Skip to content
Open
Show file tree
Hide file tree
Changes from 9 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
6 changes: 6 additions & 0 deletions google-cloud-storage/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,12 @@
<method>com.google.cloud.storage.BucketInfo$Builder setGoogleManagedEncryptionEnforcementConfig(com.google.cloud.storage.BucketInfo$GoogleManagedEncryptionEnforcementConfig)</method>
</difference>

<difference>
<differenceType>7013</differenceType>
<className>com/google/cloud/storage/BucketInfo$Builder</className>
<method>com.google.cloud.storage.BucketInfo$Builder setIsUnreachable(java.lang.Boolean)</method>
</difference>

<!-- make beta api constructors private, they still retain their factory methods. -->
<difference>
<differenceType>7004</differenceType>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,12 @@ public Builder setCustomerSuppliedEncryptionEnforcementConfig(
return this;
}

@Override
public Builder setIsUnreachable(Boolean isUnreachable) {
infoBuilder.setIsUnreachable(isUnreachable);
return this;
}

@Override
public Bucket build() {
return new Bucket(storage, infoBuilder);
Expand Down Expand Up @@ -997,6 +1003,12 @@ public Builder clearCustomerSuppliedEncryptionEnforcementConfig() {
infoBuilder.clearCustomerSuppliedEncryptionEnforcementConfig();
return this;
}

@Override
Builder clearIsUnreachable() {
infoBuilder.clearIsUnreachable();
return this;
}
}

Bucket(Storage storage, BucketInfo.BuilderImpl infoBuilder) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public class BucketInfo implements Serializable {
customerManagedEncryptionEnforcementConfig;
private final @Nullable CustomerSuppliedEncryptionEnforcementConfig
customerSuppliedEncryptionEnforcementConfig;
private final Boolean isUnreachable;

private final transient ImmutableSet<NamedField> modifiedFields;

Expand Down Expand Up @@ -2638,6 +2639,8 @@ public Builder setRetentionPeriodDuration(Duration retentionPeriod) {
*/
public abstract Builder setIpFilter(IpFilter ipFilter);

public abstract Builder setIsUnreachable(Boolean isUnreachable);

/** Creates a {@code BucketInfo} object. */
public abstract BucketInfo build();

Expand Down Expand Up @@ -2708,6 +2711,8 @@ public Builder setRetentionPeriodDuration(Duration retentionPeriod) {
abstract Builder clearCustomerManagedEncryptionEnforcementConfig();

abstract Builder clearCustomerSuppliedEncryptionEnforcementConfig();

abstract Builder clearIsUnreachable();
}

static final class BuilderImpl extends Builder {
Expand Down Expand Up @@ -2751,6 +2756,7 @@ static final class BuilderImpl extends Builder {
private GoogleManagedEncryptionEnforcementConfig googleManagedEncryptionEnforcementConfig;
private CustomerManagedEncryptionEnforcementConfig customerManagedEncryptionEnforcementConfig;
private CustomerSuppliedEncryptionEnforcementConfig customerSuppliedEncryptionEnforcementConfig;
private Boolean isUnreachable;
private final ImmutableSet.Builder<NamedField> modifiedFields = ImmutableSet.builder();

BuilderImpl(String name) {
Expand Down Expand Up @@ -2799,6 +2805,7 @@ static final class BuilderImpl extends Builder {
bucketInfo.customerManagedEncryptionEnforcementConfig;
customerSuppliedEncryptionEnforcementConfig =
bucketInfo.customerSuppliedEncryptionEnforcementConfig;
isUnreachable = bucketInfo.isUnreachable;
}

@Override
Expand Down Expand Up @@ -3250,6 +3257,13 @@ public Builder setIpFilter(IpFilter ipFilter) {
return this;
}

@Override
public Builder setIsUnreachable(Boolean isUnreachable) {
Boolean tmp = firstNonNull(isUnreachable, Data.<Boolean>nullOf(Boolean.class));
this.isUnreachable = tmp;
return this;
}

@Override
public BucketInfo build() {
checkNotNull(name);
Expand Down Expand Up @@ -3460,6 +3474,12 @@ BuilderImpl clearCustomerSuppliedEncryptionEnforcementConfig() {
return this;
}

@Override
BuilderImpl clearIsUnreachable() {
this.isUnreachable = null;
return this;
}

private Builder clearDeleteLifecycleRules() {
if (lifecycleRules != null && !lifecycleRules.isEmpty()) {
ImmutableList<LifecycleRule> nonDeleteRules =
Expand Down Expand Up @@ -3513,6 +3533,7 @@ private Builder clearDeleteLifecycleRules() {
customerManagedEncryptionEnforcementConfig = builder.customerManagedEncryptionEnforcementConfig;
customerSuppliedEncryptionEnforcementConfig =
builder.customerSuppliedEncryptionEnforcementConfig;
isUnreachable = builder.isUnreachable;
modifiedFields = builder.modifiedFields.build();
}

Expand Down Expand Up @@ -3886,6 +3907,16 @@ public HierarchicalNamespace getHierarchicalNamespace() {
return customerSuppliedEncryptionEnforcementConfig;
}

/**
* Returns a {@code Boolean} with {@code true} if the bucket is unreachable, else {@code null}
*
* <p>A bucket may be unreachable if the region in which it resides is experiencing an outage or
* if there are other temporary access issues.
*/
public Boolean isUnreachable() {
return Data.isNull(isUnreachable) ? null : isUnreachable;
}

/** Returns a builder for the current bucket. */
public Builder toBuilder() {
return new BuilderImpl(this);
Expand Down Expand Up @@ -3931,7 +3962,8 @@ public int hashCode() {
ipFilter,
googleManagedEncryptionEnforcementConfig,
customerManagedEncryptionEnforcementConfig,
customerSuppliedEncryptionEnforcementConfig);
customerSuppliedEncryptionEnforcementConfig,
isUnreachable);
}

@Override
Expand Down Expand Up @@ -3985,7 +4017,8 @@ public boolean equals(Object o) {
that.customerManagedEncryptionEnforcementConfig)
&& Objects.equals(
customerSuppliedEncryptionEnforcementConfig,
that.customerSuppliedEncryptionEnforcementConfig);
that.customerSuppliedEncryptionEnforcementConfig)
&& Objects.equals(isUnreachable, that.isUnreachable);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -674,6 +674,9 @@ private BucketInfo bucketInfoDecode(com.google.api.services.storage.model.Bucket
ifNonNull(from.getObjectRetention(), this::objectRetentionDecode, to::setObjectRetention);
ifNonNull(from.getSoftDeletePolicy(), this::softDeletePolicyDecode, to::setSoftDeletePolicy);
ifNonNull(from.getIpFilter(), ipFilterCodec::decode, to::setIpFilter);
if (from.containsKey("isUnreachable")) {
to.setIsUnreachable(Boolean.TRUE);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Spoke about this offline as well, but we need to update the logic here to handle the bucket resource names the same way grpc does.

On line https://github.com/googleapis/java-storage/pull/3404/files#diff-02854c6353038f671563a52b22658f9dcb49e66d8eb45a822439e90aa60b11f6R612 we need to add change the first line to be BucketInfo.Builder to = new BucketInfo.BuilderImpl(bucketNameCodec.decode(from.getName())); like in

BucketInfo.Builder to = new BucketInfo.BuilderImpl(bucketNameCodec.decode(from.getName()));

Even in grpc we don't expose projects/_/buckets/ everything is normalized to json style bucket names.

Copy link
Collaborator

Choose a reason for hiding this comment

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

While this does deviate from how the API returns the value, and possibly how other languages have exposed things. This change does mean that the Storage sdk here is internally consistent, and customers can reliably use bucket names regardless of unavailability.

return to.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2476,6 +2476,11 @@ public static BucketListOption pageToken(@NonNull String pageToken) {
return new BucketListOption(UnifiedOpts.pageToken(pageToken));
}

@TransportCompatibility({Transport.HTTP})
public static BucketListOption returnPartialSuccess(boolean returnPartialSuccess) {
return new BucketListOption(UnifiedOpts.returnPartialSuccess(returnPartialSuccess));
}

/**
* Returns an option to set a prefix to filter results to buckets whose names begin with this
* prefix.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,10 @@ static PageToken pageToken(@NonNull String pageToken) {
return new PageToken(pageToken);
}

static ReturnPartialSuccess returnPartialSuccess(boolean returnPartialSuccess) {
return new ReturnPartialSuccess(returnPartialSuccess);
}

static PredefinedAcl predefinedAcl(Storage.@NonNull PredefinedAcl predefinedAcl) {
requireNonNull(predefinedAcl, "predefinedAcl must be non null");
return new PredefinedAcl(predefinedAcl.getEntry());
Expand Down Expand Up @@ -1639,6 +1643,19 @@ public Mapper<ListObjectsRequest.Builder> listObjects() {
}
}

static final class ReturnPartialSuccess extends RpcOptVal<Boolean> implements BucketListOpt {
private static final long serialVersionUID = -1370658416509499277L;

private ReturnPartialSuccess(boolean val) {
super(StorageRpc.Option.RETURN_PARTIAL_SUCCESS, val);
}

@Override
public Mapper<ListBucketsRequest.Builder> listBuckets() {
return b -> b.setReturnPartialSuccess(val);
}
}

static final class PredefinedAcl extends RpcOptVal<String>
implements BucketTargetOpt, ObjectTargetOpt {
private static final long serialVersionUID = -1743736785228368741L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -461,11 +461,18 @@ public Tuple<String, Iterable<Bucket>> list(Map<Option, ?> options) {
.setPrefix(Option.PREFIX.getString(options))
.setMaxResults(Option.MAX_RESULTS.getLong(options))
.setPageToken(Option.PAGE_TOKEN.getString(options))
.setReturnPartialSuccess(Option.RETURN_PARTIAL_SUCCESS.getBoolean(options))
.setFields(Option.FIELDS.getString(options))
.setUserProject(Option.USER_PROJECT.getString(options));
setExtraHeaders(list, options);
com.google.api.services.storage.model.Buckets buckets = list.execute();
return Tuple.<String, Iterable<Bucket>>of(buckets.getNextPageToken(), buckets.getItems());
com.google.api.services.storage.model.Buckets bucketList = list.execute();
Iterable<Bucket> buckets =
Iterables.concat(
firstNonNull(bucketList.getItems(), ImmutableList.<Bucket>of()),
bucketList.getUnreachable() != null
? Lists.transform(bucketList.getUnreachable(), createUnreachableBucket())
: ImmutableList.<Bucket>of());
return Tuple.<String, Iterable<Bucket>>of(bucketList.getNextPageToken(), buckets);
} catch (IOException ex) {
span.setStatus(Status.UNKNOWN.withDescription(ex.getMessage()));
throw translate(ex);
Expand Down Expand Up @@ -530,6 +537,10 @@ private static String detectContentType(StorageObject object, Map<Option, ?> opt
return firstNonNull(contentType, "application/octet-stream");
}

private static Function<String, Bucket> createUnreachableBucket() {
return bucketName -> new Bucket().setName(bucketName).set("isUnreachable", "true");
}

private static Function<String, StorageObject> objectFromPrefix(final String bucket) {
return new Function<String, StorageObject>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ enum Option {
PROJECTION("projection"),
MAX_RESULTS("maxResults"),
PAGE_TOKEN("pageToken"),
RETURN_PARTIAL_SUCCESS("returnPartialSuccess"),
DELIMITER("delimiter"),
START_OFF_SET("startOffset"),
END_OFF_SET("endOffset"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,8 @@ public class StorageImplMockitoTest {
Storage.BucketListOption.fields();
private static final Map<StorageRpc.Option, ?> BUCKET_LIST_OPTIONS =
ImmutableMap.of(StorageRpc.Option.MAX_RESULTS, PAGE_SIZE, StorageRpc.Option.PREFIX, "prefix");
private static final Map<StorageRpc.Option, ?> BUCKET_LIST_PARTIAL_SUCCESS_OPTION =
ImmutableMap.of(StorageRpc.Option.RETURN_PARTIAL_SUCCESS, true);

// Blob list options
private static final Storage.BlobListOption BLOB_LIST_PAGE_SIZE =
Expand Down Expand Up @@ -785,6 +787,47 @@ public void testListBuckets() {
assertArrayEquals(bucketList.toArray(), Iterables.toArray(page.getValues(), Bucket.class));
}

@Test
public void testListBucketWithPartialSuccess() {
String cursor = "cursor";
String unreachableName = "unreachable_bucket";

com.google.api.services.storage.model.Bucket modelUnreachableBucket =
new com.google.api.services.storage.model.Bucket()
.setName(unreachableName)
.set("isUnreachable", "true");
com.google.api.services.storage.model.Bucket modelBucket1 =
Conversions.json().bucketInfo().encode(BUCKET_INFO1);
com.google.api.services.storage.model.Bucket modelBucket2 =
Conversions.json().bucketInfo().encode(BUCKET_INFO2);

List<com.google.api.services.storage.model.Bucket> bucketList =
ImmutableList.of(modelBucket1, modelBucket2, modelUnreachableBucket);
Tuple<String, Iterable<com.google.api.services.storage.model.Bucket>> result =
Tuple.of(cursor, bucketList);

doReturn(result)
.doThrow(UNEXPECTED_CALL_EXCEPTION)
.when(storageRpcMock)
.list(BUCKET_LIST_PARTIAL_SUCCESS_OPTION);

initializeService();
Page<Bucket> page = storage.list(Storage.BucketListOption.returnPartialSuccess(true));

Bucket expectedUnreachableBucket =
new Bucket(
storage,
new BucketInfo.BuilderImpl(
BucketInfo.newBuilder(unreachableName).setIsUnreachable(true).build()));

ImmutableList<Bucket> expectedBucketList =
ImmutableList.of(expectedBucket1, expectedBucket2, expectedUnreachableBucket);

assertEquals(cursor, page.getNextPageToken());
assertArrayEquals(
expectedBucketList.toArray(), Iterables.toArray(page.getValues(), Bucket.class));
}

@Test
public void testListBucketsEmpty() {
doReturn(Tuple.<String, Iterable<com.google.api.services.storage.model.Bucket>>of(null, null))
Expand Down
Loading