Skip to content

Commit

Permalink
fix: update GrpcStorageImpl#get(BlobId) to return null on 404 (#1772)
Browse files Browse the repository at this point in the history
Update GrpcStorageImpl#get(BlobId) to match the behavior of StorageImpl#get(BlobId) and return a null values when 404 is encountered.
  • Loading branch information
BenWhitehead committed Nov 16, 2022
1 parent 2770a38 commit 8c59c64
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
Expand Up @@ -417,7 +417,13 @@ public Blob get(BlobId blob, BlobGetOption... options) {
return Retrying.run(
getOptions(),
retryAlgorithmManager.getFor(req),
() -> storageClient.getObjectCallable().call(req, grpcCallContext),
() -> {
try {
return storageClient.getObjectCallable().call(req, grpcCallContext);
} catch (NotFoundException ignore) {
return null;
}
},
syntaxDecoders.blob.andThen(opts.clearBlobFields()));
}

Expand Down
Expand Up @@ -19,6 +19,7 @@
import com.google.api.services.storage.model.StorageObject;
import com.google.cloud.WriteChannel;
import com.google.cloud.storage.BucketInfo.BuilderImpl;
import com.google.common.collect.ImmutableList;
import java.util.Optional;
import java.util.function.Consumer;
import java.util.function.Function;
Expand Down Expand Up @@ -73,4 +74,8 @@ public static <T> void ifNonNull(@Nullable T t, Consumer<T> c) {
public static <T1, T2> void ifNonNull(@Nullable T1 t, Function<T1, T2> map, Consumer<T2> c) {
Utils.ifNonNull(t, map, c);
}

public static BlobInfo noAcl(BlobInfo bi) {
return bi.toBuilder().setOwner(null).setAcl(ImmutableList.of()).build();
}
}
Expand Up @@ -40,6 +40,7 @@
import com.google.cloud.storage.BucketFixture;
import com.google.cloud.storage.BucketInfo;
import com.google.cloud.storage.CopyWriter;
import com.google.cloud.storage.PackagePrivateMethodWorkarounds;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BlobField;
import com.google.cloud.storage.Storage.BlobWriteOption;
Expand Down Expand Up @@ -488,6 +489,14 @@ public void testListBlobsEmptySelectedFields() throws InterruptedException {
}
}

@Test
public void getBlobReturnNullOn404() {
String bucketName = bucketFixture.getBucketInfo().getName();
BlobId id = BlobId.of(bucketName, "__d_o_e_s__n_o_t__e_x_i_s_t__");
Blob blob = storage.get(id);
assertThat(blob).isNull();
}

@Test(timeout = 7500)
public void testListBlobRequesterPays() throws InterruptedException {
unsetRequesterPays(storage, requesterPaysBucketFixture);
Expand Down Expand Up @@ -1435,25 +1444,21 @@ public void testAttemptDeletionObjectTemporaryHold() {

@Test
public void testBlobReload() throws Exception {
// GRPC Error Handling bug
// b/247621346
assumeTrue(clientName.startsWith("JSON"));

String blobName = "test-blob-reload";
BlobId blobId = BlobId.of(bucketFixture.getBucketInfo().getName(), blobName);
BlobInfo blobInfo = BlobInfo.newBuilder(blobId).build();
Blob blob = storage.create(blobInfo, new byte[] {0, 1, 2});

Blob blobUnchanged = blob.reload();
assertEquals(blob, blobUnchanged);
// gRPC and json have differing defaults on projections b/258835631
assertThat(blobUnchanged).isAnyOf(blob, PackagePrivateMethodWorkarounds.noAcl(blob));

blob.writer().close();
try {
blob.reload(Blob.BlobSourceOption.generationMatch());
fail("StorageException was expected");
} catch (StorageException e) {
assertEquals(412, e.getCode());
assertEquals("conditionNotMet", e.getReason());
}

Blob updated = blob.reload();
Expand Down

0 comments on commit 8c59c64

Please sign in to comment.