Infinite cycle with MediaHttpDownloader setContentRange download (#1242)

* 1101: Fixed infinite cycle with MediaHttpDownloader setContentRange download. * 1101: Added unit tests. * 1101: Fixed comments.
googleapis · Feb 22, 2019 · cba5f27 · cba5f27
1 parent 81ef217
commit cba5f27
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 2 deletions.
diff --git a/...-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java b/...-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java
@@ -195,7 +195,7 @@ public void download(GenericUrl requestUrl, HttpHeaders requestHeaders, OutputSt
     while (true) {
       long currentRequestLastBytePos = bytesDownloaded + chunkSize - 1;
       if (lastBytePos != -1) {
-        // If last byte position has been specified use it iff it is smaller than the chunksize.
+        // If last byte position has been specified, use it iff it is smaller than the chunk size.
         currentRequestLastBytePos = Math.min(lastBytePos, currentRequestLastBytePos);
       }
       HttpResponse response = executeCurrentRequest(
@@ -204,6 +204,14 @@ public void download(GenericUrl requestUrl, HttpHeaders requestHeaders, OutputSt
       String contentRange = response.getHeaders().getContentRange();
       long nextByteIndex = getNextByteIndex(contentRange);
       setMediaContentLength(contentRange);
+      // If the last byte position is specified, complete the download when it is less than
+      // nextByteIndex.
+      if (lastBytePos != -1 && lastBytePos <= nextByteIndex) {
+        // All required bytes from the range have been downloaded from the server.
+        bytesDownloaded = lastBytePos;
+        updateStateAndNotifyListener(DownloadState.MEDIA_COMPLETE);
+        return;
+      }
 
       if (mediaContentLength <= nextByteIndex) {
         // All required bytes have been downloaded from the server.

diff --git a/...-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java b/...-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java
@@ -109,7 +109,12 @@ public LowLevelHttpResponse execute() {
           }
 
           response.setStatusCode(206);
-          int upper = Math.min(bytesDownloaded + TEST_CHUNK_SIZE, contentLength) - 1;
+          int upper;
+          if (lastBytePos != -1) {
+            upper = Math.min(lastBytePos, contentLength) - 1;
+          } else {
+            upper = Math.min(bytesDownloaded + TEST_CHUNK_SIZE, contentLength) - 1;
+          }
           response.addHeader(
               "Content-Range", "bytes " + bytesDownloaded + "-" + upper + "/" + contentLength);
           int bytesDownloadedCur = upper - bytesDownloaded + 1;
@@ -377,6 +382,40 @@ public void testSetContentRangeWithResumableDownload() throws Exception {
     assertEquals(1, fakeTransport.lowLevelExecCalls);
   }
 
+  public void testSetContentRangeFromStartWithResumableDownload() throws Exception {
+    MediaTransport fakeTransport = new MediaTransport(MediaHttpDownloader.MAXIMUM_CHUNK_SIZE);
+    fakeTransport.bytesDownloaded = 0;
+    fakeTransport.lastBytePos = 1024;
+
+    MediaHttpDownloader downloader = new MediaHttpDownloader(fakeTransport, null);
+    downloader.setContentRange(0, 1024);
+
+    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+    downloader.download(new GenericUrl(TEST_REQUEST_URL), outputStream);
+    assertEquals(1024, downloader.getLastBytePosition());
+    assertEquals(1024, outputStream.size());
+
+    // There should be 1 download call made.
+    assertEquals(1, fakeTransport.lowLevelExecCalls);
+  }
+
+  public void testSetContentRangeFromMiddletWithResumableDownload() throws Exception {
+    MediaTransport fakeTransport = new MediaTransport(MediaHttpDownloader.MAXIMUM_CHUNK_SIZE);
+    fakeTransport.bytesDownloaded = 512;
+    fakeTransport.lastBytePos = 1024;
+
+    MediaHttpDownloader downloader = new MediaHttpDownloader(fakeTransport, null);
+    downloader.setContentRange(512, 1024);
+
+    ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+    downloader.download(new GenericUrl(TEST_REQUEST_URL), outputStream);
+    assertEquals(1024, downloader.getLastBytePosition());
+    assertEquals(512, outputStream.size());
+
+    // There should be 1 download call made.
+    assertEquals(1, fakeTransport.lowLevelExecCalls);
+  }
+
   public void testSetContentRangeWithDirectDownload() throws Exception {
     int contentLength = MediaHttpDownloader.MAXIMUM_CHUNK_SIZE;
     MediaTransport fakeTransport = new MediaTransport(contentLength);