Skip to content

Commit

Permalink
fix: update bucket creation to set project as bucket.project (#1912)
Browse files Browse the repository at this point in the history
Creation of a bucket is modifying which field is used to specify the project a bucket
is associated with. Changing from `parent` to `bucket.project`. This change updates
our handling to use this new field.

The existing `parent` is set to the sentinel value `projects/_` which was previously
only implicitly found in the bucket name.

Update testbench tag to v0.35.0 which includes the new field behavior.

ApiaryConversions have been updated to track the project attribute when cross converting
as the property `.x_project`; this value should avoid any possible collision if the field
name project is supported in the future while the client is operating in this intervening
time. If/when StorageObject receives its own project field we should switch to using it.

Fix associated with b/254678990
  • Loading branch information
BenWhitehead committed Mar 28, 2023
1 parent ca82828 commit 65993c0
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.Utils.lift;
import static com.google.cloud.storage.Utils.nullableDateTimeCodec;
import static com.google.cloud.storage.Utils.projectNameCodec;
import static com.google.common.base.MoreObjects.firstNonNull;

import com.google.api.client.util.Data;
Expand Down Expand Up @@ -84,6 +85,10 @@
@InternalApi
final class ApiaryConversions {
static final ApiaryConversions INSTANCE = new ApiaryConversions();
// gRPC has a Bucket.project property that apiary doesn't have yet.
// when converting from gRPC to apiary or vice-versa we want to preserve this property. Until
// such a time as the apiary model has a project field, we manually apply it with this name.
private static final String PROJECT_ID_FIELD_NAME = "x_project";

private final Codec<Entity, String> entityCodec =
Codec.of(this::entityEncode, this::entityDecode);
Expand Down Expand Up @@ -323,6 +328,7 @@ private CustomerEncryption customerEncryptionDecode(StorageObject.CustomerEncryp

private Bucket bucketInfoEncode(BucketInfo from) {
Bucket to = new Bucket();
ifNonNull(from.getProject(), projectNameCodec::encode, p -> to.set(PROJECT_ID_FIELD_NAME, p));
ifNonNull(from.getAcl(), toListOf(bucketAcl()::encode), to::setAcl);
ifNonNull(from.getCors(), toListOf(cors()::encode), to::setCors);
ifNonNull(from.getCreateTimeOffsetDateTime(), dateTimeCodec::encode, to::setTimeCreated);
Expand Down Expand Up @@ -395,6 +401,10 @@ private Bucket bucketInfoEncode(BucketInfo from) {
@SuppressWarnings("deprecation")
private BucketInfo bucketInfoDecode(com.google.api.services.storage.model.Bucket from) {
BucketInfo.Builder to = new BucketInfo.BuilderImpl(from.getName());
ifNonNull(
from.get(PROJECT_ID_FIELD_NAME),
lift(String.class::cast).andThen(projectNameCodec::decode),
to::setProject);
ifNonNull(from.getAcl(), toListOf(bucketAcl()::decode), to::setAcl);
ifNonNull(from.getCors(), toListOf(cors()::decode), to::setCors);
ifNonNull(from.getDefaultObjectAcl(), toListOf(objectAcl()::decode), to::setDefaultAcl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ Codec<Policy, com.google.iam.v1.Policy> policyCodec() {

private BucketInfo bucketInfoDecode(Bucket from) {
BucketInfo.Builder to = new BucketInfo.BuilderImpl(bucketNameCodec.decode(from.getName()));
to.setProject(from.getProject());
to.setProject(projectNameCodec.decode(from.getProject()));
to.setGeneratedId(from.getBucketId());
maybeDecodeRetentionPolicy(from, to);
ifNonNull(from.getLocation(), to::setLocation);
Expand Down Expand Up @@ -302,6 +302,7 @@ private BucketInfo bucketInfoDecode(Bucket from) {
private Bucket bucketInfoEncode(BucketInfo from) {
Bucket.Builder to = Bucket.newBuilder();
to.setName(bucketNameCodec.encode(from.getName()));
ifNonNull(from.getProject(), projectNameCodec::encode, to::setProject);
ifNonNull(from.getGeneratedId(), to::setBucketId);
maybeEncodeRetentionPolicy(from, to);
ifNonNull(from.getLocation(), to::setLocation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
import com.google.storage.v2.Object;
import com.google.storage.v2.ObjectAccessControl;
import com.google.storage.v2.ObjectChecksums;
import com.google.storage.v2.ProjectName;
import com.google.storage.v2.ReadObjectRequest;
import com.google.storage.v2.RewriteObjectRequest;
import com.google.storage.v2.RewriteResponse;
Expand Down Expand Up @@ -190,12 +189,15 @@ public Bucket create(BucketInfo bucketInfo, BucketTargetOption... options) {
Opts<BucketTargetOpt> opts = Opts.unwrap(options).resolveFrom(bucketInfo).prepend(defaultOpts);
GrpcCallContext grpcCallContext =
opts.grpcMetadataMapper().apply(GrpcCallContext.createDefault());
if (bucketInfo.getProject() == null || bucketInfo.getProject().trim().isEmpty()) {
bucketInfo = bucketInfo.toBuilder().setProject(getOptions().getProjectId()).build();
}
com.google.storage.v2.Bucket bucket = codecs.bucketInfo().encode(bucketInfo);
CreateBucketRequest.Builder builder =
CreateBucketRequest.newBuilder()
.setBucket(bucket)
.setBucketId(bucketInfo.getName())
.setParent(ProjectName.format(getOptions().getProjectId()));
.setParent("projects/_");
CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build();
return Retrying.run(
getOptions(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ static final class Builder {
private static final String DEFAULT_GRPC_BASE_URI = "http://localhost:9005";
private static final String DEFAULT_IMAGE_NAME =
"gcr.io/cloud-devrel-public-resources/storage-testbench";
private static final String DEFAULT_IMAGE_TAG = "v0.33.0";
private static final String DEFAULT_IMAGE_TAG = "v0.35.0";
private static final String DEFAULT_CONTAINER_NAME = "default";

private boolean ignorePullError;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,10 @@

import static com.google.cloud.storage.PackagePrivateMethodWorkarounds.ifNonNull;

import com.google.cloud.storage.jqwik.StorageArbitraries.ProjectID;
import com.google.storage.v2.Bucket;
import com.google.storage.v2.BucketName;
import com.google.storage.v2.ProjectName;
import java.util.Collections;
import java.util.Set;
import javax.annotation.ParametersAreNonnullByDefault;
Expand Down Expand Up @@ -72,9 +74,15 @@ public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtyp
StorageArbitraries.buckets().iamConfig().injectNull(0.5),
StorageArbitraries.buckets().labels(),
StorageArbitraries.etag())
.as(Tuple::of),
Combinators.combine(
StorageArbitraries.projectID().map(ProjectID::toProjectName),
StorageArbitraries
.alnum() // ignored for now, tuples can't be a single element
)
.as(Tuple::of))
.as(
(t1, t2, t3) -> {
(t1, t2, t3, t4) -> {
Bucket.Builder b = Bucket.newBuilder();
ifNonNull(t1.get1(), BucketName::getBucket, b::setBucketId);
ifNonNull(t1.get2(), BucketName::toString, b::setName);
Expand All @@ -100,6 +108,7 @@ public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtyp
ifNonNull(t3.get6(), b::setIamConfig);
ifNonNull(t3.get7(), b::putAllLabels);
ifNonNull(t3.get8(), b::setEtag);
ifNonNull(t4.get1(), ProjectName::toString, b::setProject);
// TODO: add CustomPlacementConfig
return b.build();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import com.google.storage.v2.ObjectAccessControl;
import com.google.storage.v2.ObjectChecksums;
import com.google.storage.v2.Owner;
import com.google.storage.v2.ProjectName;
import com.google.storage.v2.ProjectTeam;
import com.google.type.Date;
import java.math.BigInteger;
Expand Down Expand Up @@ -437,6 +438,10 @@ private ProjectID(String value) {
public String get() {
return value;
}

public ProjectName toProjectName() {
return ProjectName.of(value);
}
}

public static Objects objects() {
Expand Down

0 comments on commit 65993c0

Please sign in to comment.