Skip to content

Commit

Permalink
fix: update GrpcStorageImpl#get(BlobId) to return null on 404
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 11, 2022
1 parent 13d630e commit 7be44be
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,18 @@
import com.google.api.core.InternalApi;
import com.google.api.gax.core.CredentialsProvider;
import com.google.api.gax.core.FixedCredentialsProvider;
import com.google.api.gax.core.GaxProperties;
import com.google.api.gax.core.NoCredentialsProvider;
import com.google.api.gax.grpc.InstantiatingGrpcChannelProvider;
import com.google.api.gax.retrying.RetrySettings;
import com.google.api.gax.retrying.StreamResumptionStrategy;
import com.google.api.gax.rpc.HeaderProvider;
import com.google.api.gax.rpc.NoHeaderProvider;
import com.google.api.gax.rpc.StatusCode.Code;
import com.google.auth.Credentials;
import com.google.cloud.NoCredentials;
import com.google.cloud.ServiceFactory;
import com.google.cloud.ServiceOptions;
import com.google.cloud.ServiceRpc;
import com.google.cloud.TransportOptions;
import com.google.cloud.grpc.GrpcTransportOptions;
Expand Down Expand Up @@ -118,12 +121,22 @@ StorageSettings getStorageSettings() throws IOException {
credentialsProvider = FixedCredentialsProvider.create(credentials);
}

HeaderProvider internalHeaderProvider =
StorageSettings.defaultApiClientHeaderProviderBuilder()
.setClientLibToken(
ServiceOptions.getGoogApiClientLibName(),
GaxProperties.getLibraryVersion(this.getClass()))
.build();

StorageSettings.Builder builder =
StorageSettings.newBuilder()
new StorageSettingsBuilder(StorageSettings.newBuilder().build())
.setInternalHeaderProvider(internalHeaderProvider)
.setEndpoint(endpoint)
.setCredentialsProvider(credentialsProvider)
.setClock(getClock());

builder.setHeaderProvider(this.getMergedHeaderProvider(new NoHeaderProvider()));

InstantiatingGrpcChannelProvider.Builder channelProviderBuilder =
InstantiatingGrpcChannelProvider.newBuilder()
.setEndpoint(endpoint)
Expand Down Expand Up @@ -578,4 +591,18 @@ public boolean canResume() {
return true;
}
}

// setInternalHeaderProvider is protected so we need to open its scope in order to set it
// we are adding an entry for gccl which is set via this provider
private static final class StorageSettingsBuilder extends StorageSettings.Builder {
private StorageSettingsBuilder(StorageSettings settings) {
super(settings);
}

@Override
protected StorageSettings.Builder setInternalHeaderProvider(
HeaderProvider internalHeaderProvider) {
return super.setInternalHeaderProvider(internalHeaderProvider);
}
}
}
Original file line number Diff line number Diff line change
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();
}
}
Original file line number Diff line number Diff line change
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 7be44be

Please sign in to comment.