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 
Add integration tests to verify desired precedence when resolving userProject options.

Fixes #1736
  • Loading branch information
@BenWhitehead
BenWhitehead committed Feb 1, 2023
1 parent 0bca083 commit 7f2afed
Show file tree
Hide file tree
Showing 5 changed files with 302 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 7f2afed

Please sign in to comment.