Skip to content

feat: expose BucketInfo.getProject as a BigInteger #3119

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.

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

Merged
merged 1 commit into from
May 28, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import java.io.InputStream;
import java.io.ObjectInputStream;
import java.io.Serializable;
import java.math.BigInteger;
import java.security.Key;
import java.time.Duration;
import java.time.OffsetDateTime;
Expand Down Expand Up @@ -502,7 +503,7 @@ public Builder setName(String name) {
}

@Override
Builder setProject(String project) {
Builder setProject(BigInteger project) {
infoBuilder.setProject(project);
return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.io.Serializable;
import java.math.BigInteger;
import java.time.Duration;
import java.time.OffsetDateTime;
import java.util.ArrayList;
Expand Down Expand Up @@ -80,7 +81,7 @@ public class BucketInfo implements Serializable {

private static final long serialVersionUID = 4793572058456298945L;
private final String generatedId;
private final String project;
private final BigInteger project;
private final String name;
private final Acl.Entity owner;
private final String selfLink;
Expand Down Expand Up @@ -1657,7 +1658,7 @@ public boolean isLive() {
public abstract static class Builder {
Builder() {}

abstract Builder setProject(String project);
abstract Builder setProject(BigInteger project);

/** Sets the bucket's name. */
public abstract Builder setName(String name);
Expand Down Expand Up @@ -1923,7 +1924,7 @@ public Builder setRetentionPeriodDuration(Duration retentionPeriod) {
static final class BuilderImpl extends Builder {

private String generatedId;
private String project;
private BigInteger project;
private String name;
private Acl.Entity owner;
private String selfLink;
Expand Down Expand Up @@ -2007,7 +2008,10 @@ public Builder setName(String name) {
}

@Override
Builder setProject(String project) {
Builder setProject(BigInteger project) {

Choose a reason for hiding this comment

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

Should we name it set/getProjectNumber to make it more clear?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In the v2/storage.proto it's named project in the discovery doc it's named projectNumber. Rather annoying.

We first added this field internally when doing the gRPC effort and before projectNUmber was public. In this case, I'm simply exposing the getter for the field we already have plumbed around.

if (!Objects.equals(this.project, project)) {
modifiedFields.add(BucketField.PROJECT);
}
this.project = project;
return this;
}
Expand Down Expand Up @@ -2637,7 +2641,8 @@ private Builder clearDeleteLifecycleRules() {
modifiedFields = builder.modifiedFields.build();
}

String getProject() {
/** The project number of the project the bucket belongs to */
public BigInteger getProject() {
return project;
}

Expand Down Expand Up @@ -2993,6 +2998,7 @@ public Builder toBuilder() {
public int hashCode() {
return Objects.hash(
generatedId,
project,
name,
owner,
selfLink,
Expand Down Expand Up @@ -3022,6 +3028,7 @@ public int hashCode() {
locationType,
objectRetention,
softDeletePolicy,
customPlacementConfig,
hierarchicalNamespace,
logging);
}
Expand All @@ -3036,6 +3043,7 @@ public boolean equals(Object o) {
}
BucketInfo that = (BucketInfo) o;
return Objects.equals(generatedId, that.generatedId)
&& Objects.equals(project, that.project)
&& Objects.equals(name, that.name)
&& Objects.equals(owner, that.owner)
&& Objects.equals(selfLink, that.selfLink)
Expand Down Expand Up @@ -3063,6 +3071,7 @@ public boolean equals(Object o) {
&& Objects.equals(iamConfiguration, that.iamConfiguration)
&& Objects.equals(autoclass, that.autoclass)
&& Objects.equals(locationType, that.locationType)
&& Objects.equals(customPlacementConfig, that.customPlacementConfig)
&& Objects.equals(objectRetention, that.objectRetention)
&& Objects.equals(softDeletePolicy, that.softDeletePolicy)
&& Objects.equals(hierarchicalNamespace, that.hierarchicalNamespace)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import static com.google.cloud.storage.Utils.bucketNameCodec;
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.Utils.lift;
import static com.google.cloud.storage.Utils.projectNameCodec;
import static com.google.cloud.storage.Utils.projectNumberResourceCodec;

import com.google.api.pathtemplate.PathTemplate;
import com.google.cloud.Binding;
Expand Down Expand Up @@ -204,7 +204,9 @@ 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(projectNameCodec.decode(from.getProject()));
if (!from.getProject().isEmpty()) {
to.setProject(projectNumberResourceCodec.decode(from.getProject()));
}
to.setGeneratedId(from.getBucketId());
maybeDecodeRetentionPolicy(from, to);
ifNonNull(from.getLocation(), to::setLocation);
Expand Down Expand Up @@ -304,7 +306,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.getProject(), projectNumberResourceCodec::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 @@ -26,6 +26,7 @@
import static com.google.cloud.storage.StorageV2ProtoUtils.objectAclEntityOrAltEq;
import static com.google.cloud.storage.Utils.bucketNameCodec;
import static com.google.cloud.storage.Utils.ifNonNull;
import static com.google.cloud.storage.Utils.projectNameCodec;
import static com.google.common.base.MoreObjects.firstNonNull;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -216,15 +217,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("projects/_");
if (bucketInfo.getProject() == null) {
builder.getBucketBuilder().setProject(projectNameCodec.encode(getOptions().getProjectId()));
}
CreateBucketRequest req = opts.createBucketsRequest().apply(builder).build();
GrpcCallContext merge = Utils.merge(grpcCallContext, Retrying.newCallContext());
return retrier.run(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
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 @@ -90,10 +89,6 @@
@InternalApi
final class JsonConversions {
static final JsonConversions INSTANCE = new JsonConversions();
// 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 @@ -394,7 +389,7 @@ private SoftDeletePolicy softDeletePolicyDecode(Bucket.SoftDeletePolicy from) {

private Bucket bucketInfoEncode(BucketInfo from) {
Bucket to = new Bucket();
ifNonNull(from.getProject(), projectNameCodec::encode, p -> to.set(PROJECT_ID_FIELD_NAME, p));
ifNonNull(from.getProject(), to::setProjectNumber);
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 @@ -477,10 +472,7 @@ 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.getProjectNumber(), 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 @@ -62,6 +62,7 @@
import java.io.ObjectInputStream;
import java.io.OutputStream;
import java.io.Serializable;
import java.math.BigInteger;
import java.net.URL;
import java.net.URLConnection;
import java.nio.file.Path;
Expand Down Expand Up @@ -190,7 +191,10 @@ enum BucketField implements FieldSelector, NamedField {
SOFT_DELETE_POLICY(
"softDeletePolicy",
"soft_delete_policy",
com.google.api.services.storage.model.Bucket.SoftDeletePolicy.class);
com.google.api.services.storage.model.Bucket.SoftDeletePolicy.class),

@TransportCompatibility({Transport.HTTP, Transport.GRPC})
PROJECT("projectNumber", "project", BigInteger.class);

static final List<BucketField> REQUIRED_FIELDS = ImmutableList.of(NAME);
private static final Map<String, BucketField> JSON_FIELD_NAME_INDEX;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.google.cloud.storage.Conversions.Codec;
import com.google.cloud.storage.UnifiedOpts.NamedField;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.MapDifference;
Expand All @@ -33,6 +34,7 @@
import com.google.common.primitives.Ints;
import com.google.storage.v2.BucketName;
import com.google.storage.v2.ProjectName;
import java.math.BigInteger;
import java.time.Duration;
import java.time.Instant;
import java.time.OffsetDateTime;
Expand Down Expand Up @@ -174,6 +176,27 @@ final class Utils {
}
});

/**
* Define a Codec which encapsulates the logic necessary to handle encoding and decoding project
* numbers.
*/
static final Codec<@NonNull BigInteger, @NonNull String> projectNumberResourceCodec =
Codec.of(
projectNumber -> {
requireNonNull(projectNumber, "projectNumber must be non null");
return ProjectName.format(projectNumber.toString());
},
projectNumberResource -> {
requireNonNull(projectNumberResource, "projectNumberResource must be non null");
Preconditions.checkArgument(
ProjectName.isParsableFrom(projectNumberResource),
"projectNumberResource '%s' is not parsable as a %s",
projectNumberResource,
ProjectName.class.getName());
ProjectName parse = ProjectName.parse(projectNumberResource);
return new BigInteger(parse.getProject());
});

static final Codec<Integer, String> crc32cCodec =
Codec.of(Utils::crc32cEncode, Utils::crc32cDecode);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,14 @@

package com.google.cloud.storage;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.cloud.storage.Conversions.Codec;
import java.io.IOException;
import java.math.BigInteger;
import java.util.Collections;
import org.checkerframework.checker.nullness.qual.NonNull;
import org.junit.Test;

public final class GrpcUtilsTest {
Expand All @@ -26,4 +32,41 @@ public final class GrpcUtilsTest {
public void closeAll_noNpeIfNullStream() throws IOException {
GrpcUtils.closeAll(Collections.singletonList(null));
}

@Test
public void projectNumberResourceCodec_simple() {
Codec<@NonNull BigInteger, @NonNull String> codec = Utils.projectNumberResourceCodec;

String encode = codec.encode(new BigInteger("34567892123"));
assertThat(encode).isEqualTo("projects/34567892123");

BigInteger decode = codec.decode(encode);
assertThat(decode).isEqualTo(new BigInteger("34567892123"));
}

@Test
public void projectNumberResourceCodec_decode_illegalArgumentException_whenUnParsable() {
String bad = "not-a-projects/123081892932";
IllegalArgumentException iae =
assertThrows(
IllegalArgumentException.class, () -> Utils.projectNumberResourceCodec.decode(bad));

assertThat(iae).hasMessageThat().contains(bad);
}

@Test
public void projectNumberResourceCodec_decode_nonNull() {
assertThrows(NullPointerException.class, () -> Utils.projectNumberResourceCodec.decode(null));
}

@Test
public void projectNumberResourceCodec_encode_nonNull() {
assertThrows(NullPointerException.class, () -> Utils.projectNumberResourceCodec.encode(null));
}

@Test
public void projectNumberResourceCodec_decode_notProjectNumber() {
String bad = "projects/not-a-number";
assertThrows(NumberFormatException.class, () -> Utils.projectNumberResourceCodec.decode(bad));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ public static final class BucketReadMaskTestParameters implements ParametersProv
public ImmutableList<?> parameters() {
ImmutableList<Args<BucketField, BucketInfo>> args =
ImmutableList.of(
new Args<>(BucketField.PROJECT, LazyAssertion.equal()),
new Args<>(BucketField.ACL, LazyAssertion.equal()),
new Args<>(BucketField.AUTOCLASS, LazyAssertion.equal()),
new Args<>(BucketField.BILLING, LazyAssertion.equal()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ public void storage_BucketGetOption_fields_BucketField() {
"website",
"softDeletePolicy",
"hierarchicalNamespace",
"website");
"projectNumber");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

website was defined twice, changing the second one to our new expected value.

s.get(
b.getName(),
BucketGetOption.fields(TestUtils.filterOutHttpOnlyBucketFields(BucketField.values())));
Expand Down Expand Up @@ -821,7 +821,7 @@ public void storage_BucketListOption_fields_BucketField() {
"items/website",
"items/softDeletePolicy",
"items/hierarchicalNamespace",
"items/website");
"items/projectNumber");
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

website was defined twice, changing the second one to our new expected value.

s.list(BucketListOption.fields(TestUtils.filterOutHttpOnlyBucketFields(BucketField.values())));
requestAuditing.assertQueryParam("fields", expected, splitOnCommaToSet());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

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

import com.google.cloud.storage.jqwik.StorageArbitraries.ProjectID;
import com.google.cloud.storage.jqwik.StorageArbitraries.ProjectNumber;
import com.google.storage.v2.Bucket;
import com.google.storage.v2.BucketName;
import com.google.storage.v2.ProjectName;
Expand Down Expand Up @@ -76,7 +76,7 @@ public Set<Arbitrary<?>> provideFor(TypeUsage targetType, SubtypeProvider subtyp
StorageArbitraries.etag())
.as(Tuple::of),
Combinators.combine(
StorageArbitraries.projectID().map(ProjectID::toProjectName),
StorageArbitraries.projectNumber().map(ProjectNumber::toProjectName),
StorageArbitraries
.alnum() // ignored for now, tuples can't be a single element
)
Expand Down
Loading