Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: update GrpcStorageImpl handling to be aware of quotaProjectId #1877

Merged
merged 2 commits into from
Feb 8, 2023
Merged

fix: update GrpcStorageImpl handling to be aware of quotaProjectId #1877

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c4a4cd2
fix: update GrpcStorageImpl handling to be aware of quotaProjectId
BenWhitehead Feb 1, 2023
e1cbed8
chore: review cleanup
BenWhitehead Feb 8, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 43 additions & 32 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/GrpcStorageImpl.java
View file
Open in desktop
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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

defaultOpts will only ever contain a single userProject correct? it seems confusing to call it defaultOpts alluding that it may have more than one value. Unless we are assuming we might have to do this again?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Opts itself is a collection, so there can theoretically be multiple UserProject instances in it, it felt weird to name it defaultUserProject when from the standpoint of the usage of this value the operations themselves don't really care about it being UserProject just that it can be prepended as a lower priority in case a particular Opt isn't already defined.

@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