diff --git a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannel.java b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannel.java index 77211b6dbd..20009921c1 100644 --- a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannel.java +++ b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannel.java @@ -554,7 +554,7 @@ private BlobSourceOption[] generateReadOptions(BlobId blobId) { // To get decoded content blobReadOptions.add(BlobSourceOption.shouldReturnRawInputStream(false)); - if (blobId.getGeneration() != null) { + if (blobId.getGeneration() != null && blobId.getGeneration() > 0) { blobReadOptions.add(BlobSourceOption.generationMatch(blobId.getGeneration())); } if (storageOptions.getEncryptionKey() != null) { diff --git a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannel.java b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannel.java index b18272e2e9..c50107ac7d 100644 --- a/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannel.java +++ b/gcsio/src/main/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannel.java @@ -775,7 +775,7 @@ protected void initMetadata(@Nullable String encoding, long sizeFromMetadata, lo size = gzipEncoded ? Long.MAX_VALUE : sizeFromMetadata; checkEncodingAndAccess(); - if (resourceId.hasGenerationId()) { + if (resourceId.hasGenerationId() && resourceId.getGenerationId() > 0) { checkState( resourceId.getGenerationId() == generation, "Provided generation (%s) should be equal to fetched generation (%s) for '%s'", @@ -1134,7 +1134,7 @@ protected Storage.Objects.Get createDataRequest() throws IOException { Storage.Objects.Get getData = storageRequestFactory.objectsGetData( resourceId.getBucketName(), resourceId.getObjectName()); - if (resourceId.hasGenerationId()) { + if (resourceId.hasGenerationId() && resourceId.getGenerationId() > 0) { getData.setGeneration(resourceId.getGenerationId()); } return getData; @@ -1149,7 +1149,7 @@ protected Storage.Objects.Get createMetadataRequest() throws IOException { Storage.Objects.Get getMetadata = storageRequestFactory.objectsGetMetadata( resourceId.getBucketName(), resourceId.getObjectName()); - if (resourceId.hasGenerationId()) { + if (resourceId.hasGenerationId() && resourceId.getGenerationId() > 0) { getMetadata.setGeneration(resourceId.getGenerationId()); } return getMetadata; diff --git a/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannelTest.java b/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannelTest.java index baec24438e..99e7536d55 100644 --- a/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannelTest.java +++ b/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageClientReadChannelTest.java @@ -33,7 +33,9 @@ import com.google.cloud.hadoop.gcsio.GoogleCloudStorageReadOptions.Fadvise; import com.google.cloud.hadoop.gcsio.integration.GoogleCloudStorageTestHelper; import com.google.cloud.hadoop.util.GrpcErrorTypeExtractor; +import com.google.cloud.storage.BlobId; import com.google.cloud.storage.Storage; +import com.google.cloud.storage.Storage.BlobSourceOption; import com.google.common.collect.ImmutableList; import com.google.protobuf.ByteString; import java.io.EOFException; @@ -200,6 +202,59 @@ public void readFullObject() throws IOException { verifyNoMoreInteractions(fakeReadChannel); } + @Test + public void read_withPositiveGeneration_usesGenerationMatchPrecondition() throws IOException { + long generation = 12345L; + GoogleCloudStorageItemInfo itemInfo = + GoogleCloudStorageItemInfo.createObject( + new StorageResourceId(V1_BUCKET_NAME, OBJECT_NAME, generation), + /* creationTime= */ 10L, + /* modificationTime= */ 15L, + /* size= */ OBJECT_SIZE, + /* contentType= */ "text/plain", + /* contentEncoding= */ "text", + /* metadata= */ null, + /* contentGeneration= */ generation, + /* metaGeneration= */ 2L, + /* verificationAttributes= */ null); + + readChannel = getJavaStorageChannel(itemInfo, DEFAULT_READ_OPTION); + readChannel.read(ByteBuffer.allocate(1)); + + ArgumentCaptor optionsCaptor = + ArgumentCaptor.forClass(BlobSourceOption.class); + verify(mockedStorage).reader(any(BlobId.class), optionsCaptor.capture()); + + assertThat(optionsCaptor.getAllValues()).contains(BlobSourceOption.generationMatch(generation)); + } + + @Test + public void read_withZeroGeneration_doesNotUseGenerationMatchPrecondition() throws IOException { + long generation = 0L; + GoogleCloudStorageItemInfo itemInfo = + GoogleCloudStorageItemInfo.createObject( + new StorageResourceId(V1_BUCKET_NAME, OBJECT_NAME, generation), + /* creationTime= */ 10L, + /* modificationTime= */ 15L, + /* size= */ OBJECT_SIZE, + /* contentType= */ "text/plain", + /* contentEncoding= */ "text", + /* metadata= */ null, + /* contentGeneration= */ generation, + /* metaGeneration= */ 2L, + /* verificationAttributes= */ null); + + readChannel = getJavaStorageChannel(itemInfo, DEFAULT_READ_OPTION); + readChannel.read(ByteBuffer.allocate(1)); + + ArgumentCaptor optionsCaptor = + ArgumentCaptor.forClass(BlobSourceOption.class); + verify(mockedStorage).reader(any(BlobId.class), optionsCaptor.capture()); + + assertThat(optionsCaptor.getAllValues()) + .doesNotContain(BlobSourceOption.generationMatch(generation)); + } + @Test public void fadviseAuto_onForwardRead_switchesToRandom() throws IOException { int chunkSize = FakeReadChannel.CHUNK_SIZE; diff --git a/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannelTest.java b/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannelTest.java index 98e3eb5804..8487399de7 100644 --- a/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannelTest.java +++ b/gcsio/src/test/java/com/google/cloud/hadoop/gcsio/GoogleCloudStorageReadChannelTest.java @@ -603,6 +603,55 @@ public void read_gzipEncoded_shouldReadAllBytes() throws IOException { assertThat(readChannel.size()).isEqualTo(testData.length); } + @Test + public void read_withPositiveGeneration_usesGenerationMatchPrecondition() throws IOException { + long generation = 12345L; + byte[] testData = {0x01, 0x02, 0x03}; + MockHttpTransport transport = + mockTransport( + jsonDataResponse(newStorageObject(BUCKET_NAME, OBJECT_NAME).setGeneration(generation)), + dataResponse(testData)); + List requests = new ArrayList<>(); + Storage storage = new Storage(transport, GsonFactory.getDefaultInstance(), requests::add); + GoogleCloudStorageReadChannel readChannel = + createReadChannel(storage, GoogleCloudStorageReadOptions.builder().build(), generation); + + readChannel.read(ByteBuffer.allocate(testData.length)); + + // The first request is for metadata, the second is for data. + // Verify the data request URL contains the generation parameter. + String mediaRequestUrl = getMediaRequestString(BUCKET_NAME, OBJECT_NAME, generation); + assertThat("GET:" + requests.get(1).getUrl().toString()).isEqualTo(mediaRequestUrl); + } + + @Test + public void read_withZeroGeneration_doesNotUseGenerationMatchPrecondition() throws IOException { + long requestedGeneration = 0L; + // The actual object has a real, positive generation ID. + long actualGeneration = 54321L; + byte[] testData = {0x04, 0x05, 0x06}; + MockHttpTransport transport = + mockTransport( + jsonDataResponse( + newStorageObject(BUCKET_NAME, OBJECT_NAME).setGeneration(actualGeneration)), + dataResponse(testData)); + List requests = new ArrayList<>(); + Storage storage = new Storage(transport, GsonFactory.getDefaultInstance(), requests::add); + GoogleCloudStorageReadChannel readChannel = + createReadChannel( + storage, + GoogleCloudStorageReadOptions.builder().setFastFailOnNotFoundEnabled(false).build(), + requestedGeneration); + + readChannel.read(ByteBuffer.allocate(testData.length)); + + // The data request URL should not contain a generation parameter, + // because the requested generation was 0. The generation parameter in getMediaRequestString + // will be null when the check for generation > 0 fails. + String mediaRequestUrl = getMediaRequestString(BUCKET_NAME, OBJECT_NAME); + assertThat("GET:" + requests.get(0).getUrl().toString()).isEqualTo(mediaRequestUrl); + } + @Test public void open_gzipContentEncoding_succeeds_whenContentEncodingSupported() throws Exception { MockHttpTransport transport =