From c14543003081ec381d6a2e8178b8af3a3d0b155e Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 14 Feb 2019 13:08:33 +0000 Subject: [PATCH 1/3] 1101: Fixed infinite cycle with MediaHttpDownloader setContentRange download. --- .../api/client/googleapis/media/MediaHttpDownloader.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java b/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java index 7e67bc036..ed58a9117 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java +++ b/google-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 if it is smaller than the chunk size. currentRequestLastBytePos = Math.min(lastBytePos, currentRequestLastBytePos); } HttpResponse response = executeCurrentRequest( @@ -204,6 +204,13 @@ public void download(GenericUrl requestUrl, HttpHeaders requestHeaders, OutputSt String contentRange = response.getHeaders().getContentRange(); long nextByteIndex = getNextByteIndex(contentRange); setMediaContentLength(contentRange); + // If last byte position specified then complete when less than nextByteIndex. + if (lastBytePos != -1 && lastBytePos < nextByteIndex) { + // All required bytes from the range have been downloaded from the server. + bytesDownloaded = nextByteIndex - 1; + updateStateAndNotifyListener(DownloadState.MEDIA_COMPLETE); + return; + } if (mediaContentLength <= nextByteIndex) { // All required bytes have been downloaded from the server. From 4aa9aec95821c8bcdd95160eec446e8c7a8257af Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 14 Feb 2019 14:10:20 +0000 Subject: [PATCH 2/3] 1101: Added unit tests. --- .../googleapis/media/MediaHttpDownloader.java | 4 +- .../media/MediaHttpDownloaderTest.java | 41 ++++++++++++++++++- 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java b/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java index ed58a9117..37c856fa0 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java +++ b/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java @@ -205,9 +205,9 @@ public void download(GenericUrl requestUrl, HttpHeaders requestHeaders, OutputSt long nextByteIndex = getNextByteIndex(contentRange); setMediaContentLength(contentRange); // If last byte position specified then complete when less than nextByteIndex. - if (lastBytePos != -1 && lastBytePos < nextByteIndex) { + if (lastBytePos != -1 && lastBytePos <= nextByteIndex) { // All required bytes from the range have been downloaded from the server. - bytesDownloaded = nextByteIndex - 1; + bytesDownloaded = lastBytePos; updateStateAndNotifyListener(DownloadState.MEDIA_COMPLETE); return; } diff --git a/google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java b/google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java index 17d7eb343..a5ecb028b 100644 --- a/google-api-client/src/test/java/com/google/api/client/googleapis/media/MediaHttpDownloaderTest.java +++ b/google-api-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); From 9e21ccba4e20a9bc0b62f167da1b5bef0a5899b1 Mon Sep 17 00:00:00 2001 From: andrey-qlogic Date: Thu, 14 Feb 2019 16:34:39 +0000 Subject: [PATCH 3/3] 1101: Fixed comments. --- .../api/client/googleapis/media/MediaHttpDownloader.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java b/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java index 37c856fa0..3f465cbfd 100644 --- a/google-api-client/src/main/java/com/google/api/client/googleapis/media/MediaHttpDownloader.java +++ b/google-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 if it is smaller than the chunk size. + // 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,7 +204,8 @@ public void download(GenericUrl requestUrl, HttpHeaders requestHeaders, OutputSt String contentRange = response.getHeaders().getContentRange(); long nextByteIndex = getNextByteIndex(contentRange); setMediaContentLength(contentRange); - // If last byte position specified then complete when less than nextByteIndex. + // 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;