Skip to content

Commit

Permalink
feat: update retries for Notifications (#1734)
Browse files Browse the repository at this point in the history 
* feat: update retries for Notifications

* review comments

* format

* deps

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
  • Loading branch information
@JesseLovelace @gcf-owl-bot
JesseLovelace and gcf-owl-bot[bot] committed Oct 27, 2022
1 parent 48d0438 commit 0fb2f18
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 87 deletions.
4 changes: 2 additions & 2 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ implementation 'com.google.cloud:google-cloud-storage'
If you are using Gradle without BOM, add this to your dependencies:

```Groovy
implementation 'com.google.cloud:google-cloud-storage:2.13.1'
implementation 'com.google.cloud:google-cloud-storage:2.14.0'
```

If you are using SBT, add this to your dependencies:

```Scala
libraryDependencies += "com.google.cloud" % "google-cloud-storage" % "2.13.1"
libraryDependencies += "com.google.cloud" % "google-cloud-storage" % "2.14.0"
```

## Authentication
Expand Down
7 changes: 7 additions & 0 deletions google-cloud-storage/pom.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
</parent>
<properties>
<site.installationModule>google-cloud-storage</site.installationModule>
<pubsub-proto.version>1.102.22</pubsub-proto.version>
<kms.version>0.99.0</kms.version>
<junit-platform.version>5.9.1</junit-platform.version>
</properties>
Expand Down Expand Up @@ -185,6 +186,12 @@
<groupId>com.google.api.grpc</groupId>
<artifactId>grpc-google-iam-v1</artifactId>
</dependency>
<dependency>
<groupId>com.google.api.grpc</groupId>
<artifactId>proto-google-cloud-pubsub-v1</artifactId>
<scope>test</scope>
<version>${pubsub-proto.version}</version>
</dependency>
<dependency>
<groupId>com.google.cloud</groupId>
<artifactId>google-cloud-core</artifactId>
Expand Down
17 changes: 17 additions & 0 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/HttpRetryAlgorithmManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.api.services.storage.model.Bucket;
import com.google.api.services.storage.model.BucketAccessControl;
import com.google.api.services.storage.model.HmacKeyMetadata;
import com.google.api.services.storage.model.Notification;
import com.google.api.services.storage.model.ObjectAccessControl;
import com.google.api.services.storage.model.Policy;
import com.google.api.services.storage.model.StorageObject;
Expand Down Expand Up @@ -243,4 +244,20 @@ public ResultRetryAlgorithm<?> getForResumableUploadSessionWrite(
public ResultRetryAlgorithm<?> getForServiceAccountGet(String pb) {
return retryStrategy.getIdempotentHandler();
}

public ResultRetryAlgorithm<?> getForNotificationCreate(String bucket, Notification pb) {
return retryStrategy.getNonidempotentHandler();
}

public ResultRetryAlgorithm<?> getForNotificationGet(String bucket, String notificationId) {
return retryStrategy.getIdempotentHandler();
}

public ResultRetryAlgorithm<?> getForNotificationList(String bucket) {
return retryStrategy.getIdempotentHandler();
}

public ResultRetryAlgorithm<?> getForNotificationDelete(String bucket, String notificationId) {
return retryStrategy.getIdempotentHandler();
}
}
108 changes: 28 additions & 80 deletions google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package com.google.cloud.storage;

import static com.google.cloud.RetryHelper.runWithRetries;
import static com.google.cloud.storage.SignedUrlEncodingHelper.Rfc3986UriEncode;
import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.base.Preconditions.checkArgument;
Expand All @@ -36,7 +35,6 @@
import com.google.cloud.PageImpl.NextPageFetcher;
import com.google.cloud.Policy;
import com.google.cloud.ReadChannel;
import com.google.cloud.RetryHelper.RetryHelperException;
import com.google.cloud.WriteChannel;
import com.google.cloud.storage.Acl.Entity;
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
Expand Down Expand Up @@ -1483,96 +1481,46 @@ public Notification createNotification(
final String bucket, final NotificationInfo notificationInfo) {
final com.google.api.services.storage.model.Notification notificationPb =
codecs.notificationInfo().encode(notificationInfo);
try {
return codecs
.notificationInfo()
.decode(
runWithRetries(
new Callable<com.google.api.services.storage.model.Notification>() {
@Override
public com.google.api.services.storage.model.Notification call() {
return storageRpc.createNotification(bucket, notificationPb);
}
},
getOptions().getRetrySettings(),
EXCEPTION_HANDLER,
getOptions().getClock()))
.asNotification(this);
} catch (RetryHelperException e) {
throw StorageException.translateAndThrow(e);
}
ResultRetryAlgorithm<?> algorithm =
retryAlgorithmManager.getForNotificationCreate(bucket, notificationPb);
return run(
algorithm,
() -> storageRpc.createNotification(bucket, notificationPb),
n -> codecs.notificationInfo().decode(n).asNotification(this));
}

@Override
public Notification getNotification(final String bucket, final String notificationId) {
try {
com.google.api.services.storage.model.Notification answer =
runWithRetries(
new Callable<com.google.api.services.storage.model.Notification>() {
@Override
public com.google.api.services.storage.model.Notification call() {
return storageRpc.getNotification(bucket, notificationId);
}
},
getOptions().getRetrySettings(),
EXCEPTION_HANDLER,
getOptions().getClock());
return answer == null ? null : codecs.notificationInfo().decode(answer).asNotification(this);
} catch (RetryHelperException e) {
throw StorageException.translateAndThrow(e);
}
ResultRetryAlgorithm<?> algorithm =
retryAlgorithmManager.getForNotificationGet(bucket, notificationId);
return run(
algorithm,
() -> storageRpc.getNotification(bucket, notificationId),
n -> codecs.notificationInfo().decode(n).asNotification(this));
}

@Override
public List<Notification> listNotifications(final String bucket) {
try {
List<com.google.api.services.storage.model.Notification> answer =
runWithRetries(
new Callable<List<com.google.api.services.storage.model.Notification>>() {
@Override
public List<com.google.api.services.storage.model.Notification> call() {
return storageRpc.listNotifications(bucket);
}
},
getOptions().getRetrySettings(),
EXCEPTION_HANDLER,
getOptions().getClock());
return answer == null
? ImmutableList.<Notification>of()
: Lists.transform(
answer,
new com.google.common.base.Function<
com.google.api.services.storage.model.Notification, Notification>() {
@Override
public Notification apply(
com.google.api.services.storage.model.Notification notificationPb) {
return codecs
.notificationInfo()
.decode(notificationPb)
.asNotification(getOptions().getService());
}
});
} catch (RetryHelperException e) {
throw StorageException.translateAndThrow(e);
}
ResultRetryAlgorithm<?> algorithm = retryAlgorithmManager.getForNotificationList(bucket);
List<Notification> result =
run(
algorithm,
() -> storageRpc.listNotifications(bucket),
(answer) ->
answer.stream()
.map(n -> codecs.notificationInfo().decode(n).asNotification(this))
.collect(ImmutableList.toImmutableList()));
return result == null ? ImmutableList.of() : result;
}

@Override
public boolean deleteNotification(final String bucket, final String notificationId) {
try {
return runWithRetries(
new Callable<Boolean>() {
@Override
public Boolean call() {
return storageRpc.deleteNotification(bucket, notificationId);
}
},
getOptions().getRetrySettings(),
EXCEPTION_HANDLER,
getOptions().getClock());
} catch (RetryHelperException e) {
throw StorageException.translateAndThrow(e);
}
ResultRetryAlgorithm<?> algorithm =
retryAlgorithmManager.getForNotificationDelete(bucket, notificationId);
return run(
algorithm,
() -> storageRpc.deleteNotification(bucket, notificationId),
Function.identity());
}

@Override
Expand Down
36 changes: 36 additions & 0 deletions ...-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,18 @@
import com.google.cloud.storage.HmacKey;
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
import com.google.cloud.storage.HmacKey.HmacKeyState;
import com.google.cloud.storage.NotificationInfo;
import com.google.cloud.storage.NotificationInfo.PayloadFormat;
import com.google.cloud.storage.ServiceAccount;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BlobTargetOption;
import com.google.cloud.storage.Storage.ComposeRequest;
import com.google.cloud.storage.conformance.retry.Functions.CtxFunction;
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableMap;
import com.google.pubsub.v1.TopicName;
import java.util.HashSet;
import java.util.Map;

/**
* Define a set of {@link CtxFunction} which are used in mappings as well as general setup/tear down
Expand Down Expand Up @@ -173,6 +178,28 @@ static final class ResourceSetup {
return s.withHmacKey(hmacKey1).with(hmacKey1.getMetadata());
});

static final CtxFunction pubsubTopic =
(ctx, c) -> {
String projectId = c.getProjectId();
TopicName name = TopicName.of(projectId, c.getTopicName());
return ctx.map(s -> s.with(name));
};

static final CtxFunction notification =
(ctx, c) ->
ctx.map(
state -> {
PayloadFormat format = PayloadFormat.JSON_API_V1;
Map<String, String> attributes = ImmutableMap.of("label1", "value1");
NotificationInfo notificationInfo =
NotificationInfo.newBuilder(state.getTopicName().toString())
.setCustomAttributes(attributes)
.setPayloadFormat(format)
.build();
return state.with(
ctx.getStorage().createNotification(c.getBucketName(), notificationInfo));
});

private static final CtxFunction processResources =
(ctx, c) -> {
HashSet<Resource> resources = newHashSet(c.getMethod().getResourcesList());
Expand All @@ -192,6 +219,11 @@ static final class ResourceSetup {
resources.remove(Resource.HMAC_KEY);
}

if (resources.contains(Resource.NOTIFICATION)) {
f = f.andThen(pubsubTopic).andThen(notification);
resources.remove(Resource.NOTIFICATION);
}

if (!resources.isEmpty()) {
throw new IllegalStateException(
String.format("Unhandled Method Resource [%s]", Joiner.on(", ").join(resources)));
Expand All @@ -204,6 +236,10 @@ static final class ResourceSetup {
(ctx, c) -> ctx.map(s -> s.with(Acl.of(User.ofAllUsers(), Role.READER)));

static final CtxFunction defaultSetup = processResources.andThen(allUsersReaderAcl);

static final CtxFunction pubsubTopicSetup = defaultSetup.andThen(pubsubTopic);

static final CtxFunction notificationSetup = pubsubTopicSetup.andThen(notification);
}

static final class ResourceTeardown {
Expand Down
82 changes: 78 additions & 4 deletions ...d-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
package com.google.cloud.storage.conformance.retry;

import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.defaultSetup;
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.notificationSetup;
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.pubsubTopicSetup;
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.serviceAccount;
import static com.google.common.base.Predicates.and;
import static com.google.common.base.Predicates.not;
Expand All @@ -36,6 +38,8 @@
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
import com.google.cloud.storage.HmacKey.HmacKeyState;
import com.google.cloud.storage.HttpMethod;
import com.google.cloud.storage.NotificationInfo;
import com.google.cloud.storage.NotificationInfo.PayloadFormat;
import com.google.cloud.storage.Storage;
import com.google.cloud.storage.Storage.BlobGetOption;
import com.google.cloud.storage.Storage.BlobSourceOption;
Expand All @@ -55,6 +59,7 @@
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.buckets;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.default_object_acl;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.hmacKey;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.notifications;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.object_acl;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.objects;
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.serviceaccount;
Expand Down Expand Up @@ -89,6 +94,8 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.OptionalInt;
import java.util.Set;
Expand Down Expand Up @@ -897,13 +904,80 @@ private static void create(ArrayList<RpcMethodMapping> a) {

static final class Notification {

private static void delete(ArrayList<RpcMethodMapping> a) {}
private static void delete(ArrayList<RpcMethodMapping> a) {
a.add(
RpcMethodMapping.newBuilder(248, notifications.delete)
.withSetup(notificationSetup)
.withTest(
(ctx, c) ->
ctx.map(
state -> {
boolean success =
ctx.getStorage()
.deleteNotification(
state.getBucket().getName(),
state.getNotification().getNotificationId());
assertTrue(success);
return state.with(success);
}))
.build());
}

private static void get(ArrayList<RpcMethodMapping> a) {}
private static void get(ArrayList<RpcMethodMapping> a) {
a.add(
RpcMethodMapping.newBuilder(246, notifications.get)
.withSetup(notificationSetup)
.withTest(
(ctx, c) ->
ctx.map(
state -> {
com.google.cloud.storage.Notification notification =
ctx.getStorage()
.getNotification(
state.getBucket().getName(),
state.getNotification().getNotificationId());
return state.with(notification);
}))
.build());
}

private static void insert(ArrayList<RpcMethodMapping> a) {}
private static void insert(ArrayList<RpcMethodMapping> a) {
a.add(
RpcMethodMapping.newBuilder(247, notifications.insert)
.withSetup(pubsubTopicSetup)
.withTest(
(ctx, c) ->
ctx.map(
state -> {
PayloadFormat format = PayloadFormat.JSON_API_V1;
Map<String, String> attributes = ImmutableMap.of("label1", "value1");
NotificationInfo info =
NotificationInfo.newBuilder(state.getTopicName().toString())
.setPayloadFormat(format)
.setCustomAttributes(attributes)
.build();
com.google.cloud.storage.Notification notification =
ctx.getStorage()
.createNotification(state.getBucket().getName(), info);
return state.with(notification);
}))
.build());
}

private static void list(ArrayList<RpcMethodMapping> a) {}
private static void list(ArrayList<RpcMethodMapping> a) {
a.add(
RpcMethodMapping.newBuilder(249, notifications.list)
.withSetup(pubsubTopicSetup)
.withTest(
(ctx, c) ->
ctx.map(
state -> {
List<com.google.cloud.storage.Notification> notifications =
ctx.getStorage().listNotifications(state.getBucket().getName());
return state.with(notifications);
}))
.build());
}
}

static final class ObjectAcl {
Expand Down
Loading

0 comments on commit 0fb2f18

Please sign in to comment.