From cded23436465bb5fb1233eb5cd6740b4561357f9 Mon Sep 17 00:00:00 2001 From: aozarov Date: Mon, 7 Mar 2016 15:28:21 -0800 Subject: [PATCH 1/2] Fix writes with 0 length --- .../google/gcloud/spi/DefaultStorageRpc.java | 10 +++++++- .../gcloud/storage/it/ITStorageTest.java | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java index 3d7febfd556b..4d9214ef5929 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java @@ -461,12 +461,20 @@ public Tuple read(StorageObject from, Map options, lo public void write(String uploadId, byte[] toWrite, int toWriteOffset, long destOffset, int length, boolean last) { try { + if (length == 0 && !last) { + return; + } GenericUrl url = new GenericUrl(uploadId); HttpRequest httpRequest = storage.getRequestFactory().buildPutRequest(url, new ByteArrayContent(null, toWrite, toWriteOffset, length)); long limit = destOffset + length; StringBuilder range = new StringBuilder("bytes "); - range.append(destOffset).append('-').append(limit - 1).append('/'); + if (length == 0) { + range.append('*'); + } else { + range.append(destOffset).append('-').append(limit - 1); + } + range.append('/'); if (last) { range.append(limit); } else { diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java index 38521ae5e137..a411cdd60bfb 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java @@ -823,6 +823,29 @@ public void testReadAndWriteChannels() throws IOException { assertTrue(storage.delete(BUCKET, blobName)); } + @Test + public void testReadAndWriteChannelsWithDifferentFileSize() throws IOException { + String blobNamePrefix = "test-read-and-write-channels-blob-"; + int[] blobSizes = {0, 700, 1024 * 256, 2 * 1024 * 1024, 4 * 1024 * 1024, 4 * 1024 * 1024 + 1}; + Random rnd = new Random(); + for (int blobSize : blobSizes) { + String blobName = blobNamePrefix + blobSize; + BlobInfo blob = BlobInfo.builder(BUCKET, blobName).build(); + byte[] bytes = new byte[blobSize]; + rnd.nextBytes(bytes); + try (WriteChannel writer = storage.writer(blob)) { + writer.write(ByteBuffer.wrap(BLOB_BYTE_CONTENT)); + } + ByteBuffer readBytes; + try (ReadChannel reader = storage.reader(blob.blobId())) { + readBytes = ByteBuffer.allocate(BLOB_BYTE_CONTENT.length); + reader.read(readBytes); + } + assertArrayEquals(BLOB_BYTE_CONTENT, readBytes.array()); + assertTrue(storage.delete(BUCKET, blobName)); + } + } + @Test public void testReadAndWriteCaptureChannels() throws IOException { String blobName = "test-read-and-write-capture-channels-blob"; From 36ebabda8ae8f075e5b83128344c5a92c84c3715 Mon Sep 17 00:00:00 2001 From: aozarov Date: Mon, 7 Mar 2016 17:00:03 -0800 Subject: [PATCH 2/2] Fix test and issue #723 --- .../google/gcloud/BaseServiceException.java | 23 ++++++++++++------- .../google/gcloud/spi/DefaultStorageRpc.java | 7 +++++- .../gcloud/storage/BlobReadChannel.java | 2 +- .../gcloud/storage/it/ITStorageTest.java | 15 ++++++++---- 4 files changed, 32 insertions(+), 15 deletions(-) diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index 579340f1256e..365243904436 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -97,13 +97,17 @@ public BaseServiceException(IOException exception, boolean idempotent) { String debugInfo = null; if (exception instanceof GoogleJsonResponseException) { GoogleJsonError jsonError = ((GoogleJsonResponseException) exception).getDetails(); - Error error = error(jsonError); - code = error.code; - reason = error.reason; - if (reason != null) { - GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0); - location = errorInfo.getLocation(); - debugInfo = (String) errorInfo.get("debugInfo"); + if (jsonError != null) { + Error error = error(jsonError); + code = error.code; + reason = error.reason; + if (reason != null) { + GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0); + location = errorInfo.getLocation(); + debugInfo = (String) errorInfo.get("debugInfo"); + } + } else { + code = ((GoogleJsonResponseException) exception).getStatusCode(); } } this.code = code; @@ -207,7 +211,10 @@ protected static Error error(GoogleJsonError error) { protected static String message(IOException exception) { if (exception instanceof GoogleJsonResponseException) { - return ((GoogleJsonResponseException) exception).getDetails().getMessage(); + GoogleJsonError details = ((GoogleJsonResponseException) exception).getDetails(); + if (details != null) { + return details.getMessage(); + } } return exception.getMessage(); } diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java index 4d9214ef5929..cac47b4bd248 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java @@ -31,6 +31,7 @@ import static com.google.gcloud.spi.StorageRpc.Option.PREFIX; import static com.google.gcloud.spi.StorageRpc.Option.VERSIONS; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; +import static javax.servlet.http.HttpServletResponse.SC_REQUESTED_RANGE_NOT_SATISFIABLE; import com.google.api.client.googleapis.batch.json.JsonBatchCallback; import com.google.api.client.googleapis.json.GoogleJsonError; @@ -453,7 +454,11 @@ public Tuple read(StorageObject from, Map options, lo String etag = req.getLastResponseHeaders().getETag(); return Tuple.of(etag, output.toByteArray()); } catch (IOException ex) { - throw translate(ex); + StorageException serviceException = translate(ex); + if (serviceException.code() == SC_REQUESTED_RANGE_NOT_SATISFIABLE) { + return Tuple.of(null, new byte[0]); + } + throw serviceException; } } diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java index 2b9643434ecc..52b8c39321da 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java @@ -127,7 +127,7 @@ public Tuple call() { return storageRpc.read(storageObject, requestOptions, position, toRead); } }, serviceOptions.retryParams(), StorageImpl.EXCEPTION_HANDLER); - if (lastEtag != null && !Objects.equals(result.x(), lastEtag)) { + if (result.y().length > 0 && lastEtag != null && !Objects.equals(result.x(), lastEtag)) { StringBuilder messageBuilder = new StringBuilder(); messageBuilder.append("Blob ").append(blob).append(" was updated while reading"); throw new StorageException(0, messageBuilder.toString()); diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java index a411cdd60bfb..b62a54aff818 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/it/ITStorageTest.java @@ -54,6 +54,7 @@ import org.junit.Test; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.net.URL; @@ -834,14 +835,18 @@ public void testReadAndWriteChannelsWithDifferentFileSize() throws IOException { byte[] bytes = new byte[blobSize]; rnd.nextBytes(bytes); try (WriteChannel writer = storage.writer(blob)) { - writer.write(ByteBuffer.wrap(BLOB_BYTE_CONTENT)); + writer.write(ByteBuffer.wrap(bytes)); } - ByteBuffer readBytes; + ByteArrayOutputStream output = new ByteArrayOutputStream(); try (ReadChannel reader = storage.reader(blob.blobId())) { - readBytes = ByteBuffer.allocate(BLOB_BYTE_CONTENT.length); - reader.read(readBytes); + ByteBuffer buffer = ByteBuffer.allocate(64 * 1024); + while (reader.read(buffer) > 0) { + buffer.flip(); + output.write(buffer.array(), 0, buffer.limit()); + buffer.clear(); + } } - assertArrayEquals(BLOB_BYTE_CONTENT, readBytes.array()); + assertArrayEquals(bytes, output.toByteArray()); assertTrue(storage.delete(BUCKET, blobName)); } }