Skip to content

Commit

Permalink
fix: update grpc based Storage to defer project id validation (#2930)
Browse files Browse the repository at this point in the history
Project id is only needed as a default for some operations, don't prevent constructing an instance of GrpcStorageImpl if project id is unresolved.
  • Loading branch information
BenWhitehead authored Feb 13, 2025
1 parent 7bdef9a commit cc03784
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import com.google.cloud.storage.UnifiedOpts.ProjectId;
import com.google.cloud.storage.UnifiedOpts.UserProject;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Streams;
Expand Down Expand Up @@ -130,6 +131,7 @@
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
Expand Down Expand Up @@ -167,7 +169,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions>

// workaround for https://github.com/googleapis/java-storage/issues/1736
private final Opts<UserProject> defaultOpts;
@Deprecated private final ProjectId defaultProjectId;
@Deprecated private final Supplier<ProjectId> defaultProjectId;

GrpcStorageImpl(
GrpcStorageOptions options,
Expand All @@ -183,7 +185,7 @@ final class GrpcStorageImpl extends BaseService<StorageOptions>
this.codecs = Conversions.grpc();
this.retryAlgorithmManager = options.getRetryAlgorithmManager();
this.syntaxDecoders = new SyntaxDecoders();
this.defaultProjectId = UnifiedOpts.projectId(options.getProjectId());
this.defaultProjectId = Suppliers.memoize(() -> UnifiedOpts.projectId(options.getProjectId()));
}

@Override
Expand Down Expand Up @@ -443,6 +445,7 @@ public Page<Bucket> list(BucketListOption... options) {
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
ListBucketsRequest request =
defaultProjectId
.get()
.listBuckets()
.andThen(opts.listBucketsRequest())
.apply(ListBucketsRequest.newBuilder())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import com.google.cloud.storage.it.runner.annotations.Parameterized.ParametersProvider;
import com.google.cloud.storage.it.runner.annotations.SingleBackend;
import com.google.common.collect.ImmutableList;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;

Expand Down Expand Up @@ -73,12 +72,32 @@ public void clientShouldConstructCleanly_grpc() throws Exception {
}

@Test
@Ignore("waiting on conformation from the backend team if this should even be possible")
public void clientShouldConstructCleanly_directPath() throws Exception {
assumeTrue(
"Unable to determine environment can access directPath", TestUtils.isOnComputeEngine());
StorageOptions options =
StorageOptions.grpc().setCredentials(credentials).setAttemptDirectPath(true).build();
StorageOptions.grpc()
.setCredentials(credentials)
.setAttemptDirectPath(true)
.setEnableGrpcClientMetrics(false)
.build();
doTest(options);
}

@Test
public void lackOfProjectIdDoesNotPreventConstruction_http() throws Exception {
StorageOptions options = StorageOptions.http().setCredentials(credentials).build();
doTest(options);
}

@Test
public void lackOfProjectIdDoesNotPreventConstruction_grpc() throws Exception {
StorageOptions options =
StorageOptions.grpc()
.setCredentials(credentials)
.setAttemptDirectPath(false)
.setEnableGrpcClientMetrics(false)
.build();
doTest(options);
}

Expand Down

0 comments on commit cc03784

Please sign in to comment.