Skip to content

Commit

Permalink
fix: update GrpcStorageImpl handling to be aware of quotaProjectId (#…
Browse files Browse the repository at this point in the history 
…1877)

Add integration tests to verify desired precedence when resolving userProject options.

Fixes #1736
  • Loading branch information
@BenWhitehead
BenWhitehead committed Feb 8, 2023
1 parent 9220e28 commit ca8510e
Show file tree
Hide file tree
Showing 5 changed files with 307 additions and 37 deletions.
75 changes: 43 additions & 32 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import com.google.cloud.storage.UnifiedOpts.ObjectTargetOpt;
import com.google.cloud.storage.UnifiedOpts.Opts;
import com.google.cloud.storage.UnifiedOpts.ProjectId;
import com.google.cloud.storage.UnifiedOpts.UserProject;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
Expand Down Expand Up @@ -159,11 +160,15 @@ final class GrpcStorageImpl extends BaseService<StorageOptions> implements Stora
final GrpcRetryAlgorithmManager retryAlgorithmManager;
final SyntaxDecoders syntaxDecoders;

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

GrpcStorageImpl(GrpcStorageOptions options, StorageClient storageClient) {
GrpcStorageImpl(
GrpcStorageOptions options, StorageClient storageClient, Opts<UserProject> defaultOpts) {
super(options);
this.storageClient = storageClient;
this.defaultOpts = defaultOpts;
this.codecs = Conversions.grpc();
this.retryAlgorithmManager = options.getRetryAlgorithmManager();
this.syntaxDecoders = new SyntaxDecoders();
Expand All @@ -182,7 +187,7 @@ public void close() throws Exception {

@Override
public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo);
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
com.google.storage.v2.Bucket bucket = codecs.bucketInfo().encode(bucketInfo);
Expand Down Expand Up @@ -215,7 +220,7 @@ public Blob create(
BlobInfo blobInfo, byte[] content, int offset, int length, BlobTargetOption... options) {
requireNonNull(blobInfo, "blobInfo must be non null");
requireNonNull(content, "content must be non null");
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
Expand Down Expand Up @@ -267,7 +272,7 @@ public Blob createFrom(BlobInfo blobInfo, Path path, int bufferSize, BlobWriteOp
throw new StorageException(0, path + " is a directory");
}

Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
Expand Down Expand Up @@ -335,7 +340,7 @@ public Blob createFrom(
throws IOException {
requireNonNull(blobInfo, "blobInfo must be non null");

Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
Expand Down Expand Up @@ -369,7 +374,7 @@ public Blob createFrom(

@Override
public Bucket get(String bucket, BucketGetOption... options) {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
GetBucketRequest.Builder builder =
Expand All @@ -384,7 +389,7 @@ public Bucket get(String bucket, BucketGetOption... options) {

@Override
public Bucket lockRetentionPolicy(BucketInfo bucket, BucketTargetOption... options) {
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucket);
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucket).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
LockBucketRetentionPolicyRequest.Builder builder =
Expand All @@ -406,7 +411,7 @@ public Blob get(String bucket, String blob, BlobGetOption... options) {

@Override
public Blob get(BlobId blob, BlobGetOption... options) {
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
GetObjectRequest.Builder builder =
Expand Down Expand Up @@ -435,7 +440,7 @@ public Blob get(BlobId blob) {

@Override
public Page<Bucket> list(BucketListOption... options) {
Opts<BucketListOpt> opts = Opts.unwrap(options);
Opts<BucketListOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
ListBucketsRequest request =
Expand All @@ -462,7 +467,7 @@ public Page<Bucket> list(BucketListOption... options) {

@Override
public Page<Blob> list(String bucket, BlobListOption... options) {
Opts<ObjectListOpt> opts = Opts.unwrap(options);
Opts<ObjectListOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
ListObjectsRequest.Builder builder =
Expand All @@ -481,7 +486,7 @@ public Page<Blob> list(String bucket, BlobListOption... options) {

@Override
public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) {
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo);
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
com.google.storage.v2.Bucket bucket = codecs.bucketInfo().encode(bucketInfo);
Expand All @@ -503,7 +508,7 @@ public Bucket update(BucketInfo bucketInfo, BucketTargetOption... options) {

@Override
public Blob update(BlobInfo blobInfo, BlobTargetOption... options) {
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
Object object = codecs.blobInfo().encode(blobInfo);
Expand All @@ -530,7 +535,7 @@ public Blob update(BlobInfo blobInfo) {

@Override
public boolean delete(String bucket, BucketSourceOption... options) {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
DeleteBucketRequest.Builder builder =
Expand All @@ -555,7 +560,7 @@ public boolean delete(String bucket, String blob, BlobSourceOption... options) {

@Override
public boolean delete(BlobId blob, BlobSourceOption... options) {
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
DeleteObjectRequest.Builder builder =
Expand Down Expand Up @@ -587,7 +592,9 @@ public boolean delete(BlobId blob) {
@Override
public Blob compose(ComposeRequest composeRequest) {
Opts<ObjectTargetOpt> opts =
Opts.unwrap(composeRequest.getTargetOptions()).resolveFrom(composeRequest.getTarget());
Opts.unwrap(composeRequest.getTargetOptions())
.resolveFrom(composeRequest.getTarget())
.prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
ComposeObjectRequest.Builder builder = ComposeObjectRequest.newBuilder();
Expand All @@ -609,8 +616,12 @@ public CopyWriter copy(CopyRequest copyRequest) {
BlobId src = copyRequest.getSource();
BlobInfo dst = copyRequest.getTarget();
Opts<ObjectSourceOpt> srcOpts =
Opts.unwrap(copyRequest.getSourceOptions()).projectAsSource().resolveFrom(src);
Opts<ObjectTargetOpt> dstOpts = Opts.unwrap(copyRequest.getTargetOptions()).resolveFrom(dst);
Opts.unwrap(copyRequest.getSourceOptions())
.projectAsSource()
.resolveFrom(src)
.prepend(defaultOpts);
Opts<ObjectTargetOpt> dstOpts =
Opts.unwrap(copyRequest.getTargetOptions()).resolveFrom(dst).prepend(defaultOpts);

Mapper<RewriteObjectRequest.Builder> mapper =
srcOpts.rewriteObjectsRequest().andThen(dstOpts.rewriteObjectsRequest());
Expand Down Expand Up @@ -684,7 +695,7 @@ public GrpcBlobReadChannel reader(String bucket, String blob, BlobSourceOption..

@Override
public GrpcBlobReadChannel reader(BlobId blob, BlobSourceOption... options) {
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts);
ReadObjectRequest request = getReadObjectRequest(blob, opts);
Set<StatusCode.Code> codes = resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(request));
GrpcCallContext grpcCallContext = GrpcCallContext.createDefault().withRetryableCodes(codes);
Expand Down Expand Up @@ -722,7 +733,7 @@ public void downloadTo(BlobId blob, OutputStream outputStream, BlobSourceOption.

@Override
public GrpcBlobWriteChannel writer(BlobInfo blobInfo, BlobWriteOption... options) {
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo);
Opts<ObjectTargetOpt> opts = Opts.unwrap(options).resolveFrom(blobInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
WriteObjectRequest req = getWriteObjectRequest(blobInfo, opts);
Expand Down Expand Up @@ -844,7 +855,7 @@ public List<Boolean> delete(Iterable<BlobId> blobIds) {
@Override
public Acl getAcl(String bucket, Entity entity, BucketSourceOption... options) {
try {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts);

Predicate<BucketAccessControl> entityPredicate =
Expand Down Expand Up @@ -874,7 +885,7 @@ public Acl getAcl(String bucket, Entity entity) {
@Override
public boolean deleteAcl(String bucket, Entity entity, BucketSourceOption... options) {
try {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts);
String encode = codecs.entity().encode(entity);

Expand Down Expand Up @@ -928,7 +939,7 @@ public Acl createAcl(String bucket, Acl acl) {
@Override
public Acl updateAcl(String bucket, Acl acl, BucketSourceOption... options) {
try {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts);
BucketAccessControl encode = codecs.bucketAcl().encode(acl);
String entity = encode.getEntity();
Expand Down Expand Up @@ -966,7 +977,7 @@ public Acl updateAcl(String bucket, Acl acl) {
@Override
public List<Acl> listAcls(String bucket, BucketSourceOption... options) {
try {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
com.google.storage.v2.Bucket resp = getBucketWithAcls(bucket, opts);
return resp.getAclList().stream()
.map(codecs.bucketAcl()::decode)
Expand Down Expand Up @@ -1211,7 +1222,7 @@ public List<Acl> listAcls(BlobId blob) {

@Override
public HmacKey createHmacKey(ServiceAccount serviceAccount, CreateHmacKeyOption... options) {
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options);
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
CreateHmacKeyRequest request =
Expand All @@ -1236,7 +1247,7 @@ public HmacKey createHmacKey(ServiceAccount serviceAccount, CreateHmacKeyOption.

@Override
public Page<HmacKeyMetadata> listHmacKeys(ListHmacKeysOption... options) {
Opts<HmacKeyListOpt> opts = Opts.unwrap(options);
Opts<HmacKeyListOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());

Expand Down Expand Up @@ -1264,7 +1275,7 @@ public Page<HmacKeyMetadata> listHmacKeys(ListHmacKeysOption... options) {

@Override
public HmacKeyMetadata getHmacKey(String accessId, GetHmacKeyOption... options) {
Opts<HmacKeySourceOpt> opts = Opts.unwrap(options);
Opts<HmacKeySourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
GetHmacKeyRequest request =
Expand All @@ -1283,7 +1294,7 @@ public HmacKeyMetadata getHmacKey(String accessId, GetHmacKeyOption... options)

@Override
public void deleteHmacKey(HmacKeyMetadata hmacKeyMetadata, DeleteHmacKeyOption... options) {
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options);
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
DeleteHmacKeyRequest req =
Expand All @@ -1304,7 +1315,7 @@ public void deleteHmacKey(HmacKeyMetadata hmacKeyMetadata, DeleteHmacKeyOption..
@Override
public HmacKeyMetadata updateHmacKeyState(
HmacKeyMetadata hmacKeyMetadata, HmacKeyState state, UpdateHmacKeyOption... options) {
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options);
Opts<HmacKeyTargetOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
com.google.storage.v2.HmacKeyMetadata encode =
Expand All @@ -1323,7 +1334,7 @@ public HmacKeyMetadata updateHmacKeyState(

@Override
public Policy getIamPolicy(String bucket, BucketSourceOption... options) {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
GetIamPolicyRequest.Builder builder =
Expand All @@ -1338,7 +1349,7 @@ public Policy getIamPolicy(String bucket, BucketSourceOption... options) {

@Override
public Policy setIamPolicy(String bucket, Policy policy, BucketSourceOption... options) {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
SetIamPolicyRequest req =
Expand All @@ -1356,7 +1367,7 @@ public Policy setIamPolicy(String bucket, Policy policy, BucketSourceOption... o
@Override
public List<Boolean> testIamPermissions(
String bucket, List<String> permissions, BucketSourceOption... options) {
Opts<BucketSourceOpt> opts = Opts.unwrap(options);
Opts<BucketSourceOpt> opts = Opts.unwrap(options).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
TestIamPermissionsRequest req =
Expand Down Expand Up @@ -1706,7 +1717,7 @@ private WriteObjectRequest getWriteObjectRequest(BlobInfo info, Opts<ObjectTarge

private UnbufferedReadableByteChannelSession<Object> unbufferedReadSession(
BlobId blob, BlobSourceOption[] options) {
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob);
Opts<ObjectSourceOpt> opts = Opts.unwrap(options).resolveFrom(blob).prepend(defaultOpts);
ReadObjectRequest readObjectRequest = getReadObjectRequest(blob, opts);
Set<StatusCode.Code> codes =
resultRetryAlgorithmToCodes(retryAlgorithmManager.getFor(readObjectRequest));
Expand Down
Loading

0 comments on commit ca8510e

Please sign in to comment.