Skip to content

Commit 0fb2f18

Browse files
feat: update retries for Notifications (#1734)
* 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>
1 parent 48d0438 commit 0fb2f18

File tree

9 files changed

+221
-87
lines changed

9 files changed

+221
-87
lines changed

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,13 +56,13 @@ implementation 'com.google.cloud:google-cloud-storage'
5656
If you are using Gradle without BOM, add this to your dependencies:
5757

5858
```Groovy
59-
implementation 'com.google.cloud:google-cloud-storage:2.13.1'
59+
implementation 'com.google.cloud:google-cloud-storage:2.14.0'
6060
```
6161

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

6464
```Scala
65-
libraryDependencies += "com.google.cloud" % "google-cloud-storage" % "2.13.1"
65+
libraryDependencies += "com.google.cloud" % "google-cloud-storage" % "2.14.0"
6666
```
6767

6868
## Authentication

google-cloud-storage/pom.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
</parent>
1717
<properties>
1818
<site.installationModule>google-cloud-storage</site.installationModule>
19+
<pubsub-proto.version>1.102.22</pubsub-proto.version>
1920
<kms.version>0.99.0</kms.version>
2021
<junit-platform.version>5.9.1</junit-platform.version>
2122
</properties>
@@ -185,6 +186,12 @@
185186
<groupId>com.google.api.grpc</groupId>
186187
<artifactId>grpc-google-iam-v1</artifactId>
187188
</dependency>
189+
<dependency>
190+
<groupId>com.google.api.grpc</groupId>
191+
<artifactId>proto-google-cloud-pubsub-v1</artifactId>
192+
<scope>test</scope>
193+
<version>${pubsub-proto.version}</version>
194+
</dependency>
188195
<dependency>
189196
<groupId>com.google.cloud</groupId>
190197
<artifactId>google-cloud-core</artifactId>

google-cloud-storage/src/main/java/com/google/cloud/storage/HttpRetryAlgorithmManager.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import com.google.api.services.storage.model.Bucket;
2121
import com.google.api.services.storage.model.BucketAccessControl;
2222
import com.google.api.services.storage.model.HmacKeyMetadata;
23+
import com.google.api.services.storage.model.Notification;
2324
import com.google.api.services.storage.model.ObjectAccessControl;
2425
import com.google.api.services.storage.model.Policy;
2526
import com.google.api.services.storage.model.StorageObject;
@@ -243,4 +244,20 @@ public ResultRetryAlgorithm<?> getForResumableUploadSessionWrite(
243244
public ResultRetryAlgorithm<?> getForServiceAccountGet(String pb) {
244245
return retryStrategy.getIdempotentHandler();
245246
}
247+
248+
public ResultRetryAlgorithm<?> getForNotificationCreate(String bucket, Notification pb) {
249+
return retryStrategy.getNonidempotentHandler();
250+
}
251+
252+
public ResultRetryAlgorithm<?> getForNotificationGet(String bucket, String notificationId) {
253+
return retryStrategy.getIdempotentHandler();
254+
}
255+
256+
public ResultRetryAlgorithm<?> getForNotificationList(String bucket) {
257+
return retryStrategy.getIdempotentHandler();
258+
}
259+
260+
public ResultRetryAlgorithm<?> getForNotificationDelete(String bucket, String notificationId) {
261+
return retryStrategy.getIdempotentHandler();
262+
}
246263
}

google-cloud-storage/src/main/java/com/google/cloud/storage/StorageImpl.java

Lines changed: 28 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package com.google.cloud.storage;
1818

19-
import static com.google.cloud.RetryHelper.runWithRetries;
2019
import static com.google.cloud.storage.SignedUrlEncodingHelper.Rfc3986UriEncode;
2120
import static com.google.common.base.MoreObjects.firstNonNull;
2221
import static com.google.common.base.Preconditions.checkArgument;
@@ -36,7 +35,6 @@
3635
import com.google.cloud.PageImpl.NextPageFetcher;
3736
import com.google.cloud.Policy;
3837
import com.google.cloud.ReadChannel;
39-
import com.google.cloud.RetryHelper.RetryHelperException;
4038
import com.google.cloud.WriteChannel;
4139
import com.google.cloud.storage.Acl.Entity;
4240
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
@@ -1483,96 +1481,46 @@ public Notification createNotification(
14831481
final String bucket, final NotificationInfo notificationInfo) {
14841482
final com.google.api.services.storage.model.Notification notificationPb =
14851483
codecs.notificationInfo().encode(notificationInfo);
1486-
try {
1487-
return codecs
1488-
.notificationInfo()
1489-
.decode(
1490-
runWithRetries(
1491-
new Callable<com.google.api.services.storage.model.Notification>() {
1492-
@Override
1493-
public com.google.api.services.storage.model.Notification call() {
1494-
return storageRpc.createNotification(bucket, notificationPb);
1495-
}
1496-
},
1497-
getOptions().getRetrySettings(),
1498-
EXCEPTION_HANDLER,
1499-
getOptions().getClock()))
1500-
.asNotification(this);
1501-
} catch (RetryHelperException e) {
1502-
throw StorageException.translateAndThrow(e);
1503-
}
1484+
ResultRetryAlgorithm<?> algorithm =
1485+
retryAlgorithmManager.getForNotificationCreate(bucket, notificationPb);
1486+
return run(
1487+
algorithm,
1488+
() -> storageRpc.createNotification(bucket, notificationPb),
1489+
n -> codecs.notificationInfo().decode(n).asNotification(this));
15041490
}
15051491

15061492
@Override
15071493
public Notification getNotification(final String bucket, final String notificationId) {
1508-
try {
1509-
com.google.api.services.storage.model.Notification answer =
1510-
runWithRetries(
1511-
new Callable<com.google.api.services.storage.model.Notification>() {
1512-
@Override
1513-
public com.google.api.services.storage.model.Notification call() {
1514-
return storageRpc.getNotification(bucket, notificationId);
1515-
}
1516-
},
1517-
getOptions().getRetrySettings(),
1518-
EXCEPTION_HANDLER,
1519-
getOptions().getClock());
1520-
return answer == null ? null : codecs.notificationInfo().decode(answer).asNotification(this);
1521-
} catch (RetryHelperException e) {
1522-
throw StorageException.translateAndThrow(e);
1523-
}
1494+
ResultRetryAlgorithm<?> algorithm =
1495+
retryAlgorithmManager.getForNotificationGet(bucket, notificationId);
1496+
return run(
1497+
algorithm,
1498+
() -> storageRpc.getNotification(bucket, notificationId),
1499+
n -> codecs.notificationInfo().decode(n).asNotification(this));
15241500
}
15251501

15261502
@Override
15271503
public List<Notification> listNotifications(final String bucket) {
1528-
try {
1529-
List<com.google.api.services.storage.model.Notification> answer =
1530-
runWithRetries(
1531-
new Callable<List<com.google.api.services.storage.model.Notification>>() {
1532-
@Override
1533-
public List<com.google.api.services.storage.model.Notification> call() {
1534-
return storageRpc.listNotifications(bucket);
1535-
}
1536-
},
1537-
getOptions().getRetrySettings(),
1538-
EXCEPTION_HANDLER,
1539-
getOptions().getClock());
1540-
return answer == null
1541-
? ImmutableList.<Notification>of()
1542-
: Lists.transform(
1543-
answer,
1544-
new com.google.common.base.Function<
1545-
com.google.api.services.storage.model.Notification, Notification>() {
1546-
@Override
1547-
public Notification apply(
1548-
com.google.api.services.storage.model.Notification notificationPb) {
1549-
return codecs
1550-
.notificationInfo()
1551-
.decode(notificationPb)
1552-
.asNotification(getOptions().getService());
1553-
}
1554-
});
1555-
} catch (RetryHelperException e) {
1556-
throw StorageException.translateAndThrow(e);
1557-
}
1504+
ResultRetryAlgorithm<?> algorithm = retryAlgorithmManager.getForNotificationList(bucket);
1505+
List<Notification> result =
1506+
run(
1507+
algorithm,
1508+
() -> storageRpc.listNotifications(bucket),
1509+
(answer) ->
1510+
answer.stream()
1511+
.map(n -> codecs.notificationInfo().decode(n).asNotification(this))
1512+
.collect(ImmutableList.toImmutableList()));
1513+
return result == null ? ImmutableList.of() : result;
15581514
}
15591515

15601516
@Override
15611517
public boolean deleteNotification(final String bucket, final String notificationId) {
1562-
try {
1563-
return runWithRetries(
1564-
new Callable<Boolean>() {
1565-
@Override
1566-
public Boolean call() {
1567-
return storageRpc.deleteNotification(bucket, notificationId);
1568-
}
1569-
},
1570-
getOptions().getRetrySettings(),
1571-
EXCEPTION_HANDLER,
1572-
getOptions().getClock());
1573-
} catch (RetryHelperException e) {
1574-
throw StorageException.translateAndThrow(e);
1575-
}
1518+
ResultRetryAlgorithm<?> algorithm =
1519+
retryAlgorithmManager.getForNotificationDelete(bucket, notificationId);
1520+
return run(
1521+
algorithm,
1522+
() -> storageRpc.deleteNotification(bucket, notificationId),
1523+
Function.identity());
15761524
}
15771525

15781526
@Override

google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/CtxFunctions.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,13 +31,18 @@
3131
import com.google.cloud.storage.HmacKey;
3232
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
3333
import com.google.cloud.storage.HmacKey.HmacKeyState;
34+
import com.google.cloud.storage.NotificationInfo;
35+
import com.google.cloud.storage.NotificationInfo.PayloadFormat;
3436
import com.google.cloud.storage.ServiceAccount;
3537
import com.google.cloud.storage.Storage;
3638
import com.google.cloud.storage.Storage.BlobTargetOption;
3739
import com.google.cloud.storage.Storage.ComposeRequest;
3840
import com.google.cloud.storage.conformance.retry.Functions.CtxFunction;
3941
import com.google.common.base.Joiner;
42+
import com.google.common.collect.ImmutableMap;
43+
import com.google.pubsub.v1.TopicName;
4044
import java.util.HashSet;
45+
import java.util.Map;
4146

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

181+
static final CtxFunction pubsubTopic =
182+
(ctx, c) -> {
183+
String projectId = c.getProjectId();
184+
TopicName name = TopicName.of(projectId, c.getTopicName());
185+
return ctx.map(s -> s.with(name));
186+
};
187+
188+
static final CtxFunction notification =
189+
(ctx, c) ->
190+
ctx.map(
191+
state -> {
192+
PayloadFormat format = PayloadFormat.JSON_API_V1;
193+
Map<String, String> attributes = ImmutableMap.of("label1", "value1");
194+
NotificationInfo notificationInfo =
195+
NotificationInfo.newBuilder(state.getTopicName().toString())
196+
.setCustomAttributes(attributes)
197+
.setPayloadFormat(format)
198+
.build();
199+
return state.with(
200+
ctx.getStorage().createNotification(c.getBucketName(), notificationInfo));
201+
});
202+
176203
private static final CtxFunction processResources =
177204
(ctx, c) -> {
178205
HashSet<Resource> resources = newHashSet(c.getMethod().getResourcesList());
@@ -192,6 +219,11 @@ static final class ResourceSetup {
192219
resources.remove(Resource.HMAC_KEY);
193220
}
194221

222+
if (resources.contains(Resource.NOTIFICATION)) {
223+
f = f.andThen(pubsubTopic).andThen(notification);
224+
resources.remove(Resource.NOTIFICATION);
225+
}
226+
195227
if (!resources.isEmpty()) {
196228
throw new IllegalStateException(
197229
String.format("Unhandled Method Resource [%s]", Joiner.on(", ").join(resources)));
@@ -204,6 +236,10 @@ static final class ResourceSetup {
204236
(ctx, c) -> ctx.map(s -> s.with(Acl.of(User.ofAllUsers(), Role.READER)));
205237

206238
static final CtxFunction defaultSetup = processResources.andThen(allUsersReaderAcl);
239+
240+
static final CtxFunction pubsubTopicSetup = defaultSetup.andThen(pubsubTopic);
241+
242+
static final CtxFunction notificationSetup = pubsubTopicSetup.andThen(notification);
207243
}
208244

209245
static final class ResourceTeardown {

google-cloud-storage/src/test/java/com/google/cloud/storage/conformance/retry/RpcMethodMappings.java

Lines changed: 78 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
package com.google.cloud.storage.conformance.retry;
1818

1919
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.defaultSetup;
20+
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.notificationSetup;
21+
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.pubsubTopicSetup;
2022
import static com.google.cloud.storage.conformance.retry.CtxFunctions.ResourceSetup.serviceAccount;
2123
import static com.google.common.base.Predicates.and;
2224
import static com.google.common.base.Predicates.not;
@@ -36,6 +38,8 @@
3638
import com.google.cloud.storage.HmacKey.HmacKeyMetadata;
3739
import com.google.cloud.storage.HmacKey.HmacKeyState;
3840
import com.google.cloud.storage.HttpMethod;
41+
import com.google.cloud.storage.NotificationInfo;
42+
import com.google.cloud.storage.NotificationInfo.PayloadFormat;
3943
import com.google.cloud.storage.Storage;
4044
import com.google.cloud.storage.Storage.BlobGetOption;
4145
import com.google.cloud.storage.Storage.BlobSourceOption;
@@ -55,6 +59,7 @@
5559
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.buckets;
5660
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.default_object_acl;
5761
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.hmacKey;
62+
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.notifications;
5863
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.object_acl;
5964
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.objects;
6065
import com.google.cloud.storage.conformance.retry.RpcMethod.storage.serviceaccount;
@@ -89,6 +94,8 @@
8994
import java.util.ArrayList;
9095
import java.util.Collection;
9196
import java.util.Collections;
97+
import java.util.List;
98+
import java.util.Map;
9299
import java.util.Map.Entry;
93100
import java.util.OptionalInt;
94101
import java.util.Set;
@@ -897,13 +904,80 @@ private static void create(ArrayList<RpcMethodMapping> a) {
897904

898905
static final class Notification {
899906

900-
private static void delete(ArrayList<RpcMethodMapping> a) {}
907+
private static void delete(ArrayList<RpcMethodMapping> a) {
908+
a.add(
909+
RpcMethodMapping.newBuilder(248, notifications.delete)
910+
.withSetup(notificationSetup)
911+
.withTest(
912+
(ctx, c) ->
913+
ctx.map(
914+
state -> {
915+
boolean success =
916+
ctx.getStorage()
917+
.deleteNotification(
918+
state.getBucket().getName(),
919+
state.getNotification().getNotificationId());
920+
assertTrue(success);
921+
return state.with(success);
922+
}))
923+
.build());
924+
}
901925

902-
private static void get(ArrayList<RpcMethodMapping> a) {}
926+
private static void get(ArrayList<RpcMethodMapping> a) {
927+
a.add(
928+
RpcMethodMapping.newBuilder(246, notifications.get)
929+
.withSetup(notificationSetup)
930+
.withTest(
931+
(ctx, c) ->
932+
ctx.map(
933+
state -> {
934+
com.google.cloud.storage.Notification notification =
935+
ctx.getStorage()
936+
.getNotification(
937+
state.getBucket().getName(),
938+
state.getNotification().getNotificationId());
939+
return state.with(notification);
940+
}))
941+
.build());
942+
}
903943

904-
private static void insert(ArrayList<RpcMethodMapping> a) {}
944+
private static void insert(ArrayList<RpcMethodMapping> a) {
945+
a.add(
946+
RpcMethodMapping.newBuilder(247, notifications.insert)
947+
.withSetup(pubsubTopicSetup)
948+
.withTest(
949+
(ctx, c) ->
950+
ctx.map(
951+
state -> {
952+
PayloadFormat format = PayloadFormat.JSON_API_V1;
953+
Map<String, String> attributes = ImmutableMap.of("label1", "value1");
954+
NotificationInfo info =
955+
NotificationInfo.newBuilder(state.getTopicName().toString())
956+
.setPayloadFormat(format)
957+
.setCustomAttributes(attributes)
958+
.build();
959+
com.google.cloud.storage.Notification notification =
960+
ctx.getStorage()
961+
.createNotification(state.getBucket().getName(), info);
962+
return state.with(notification);
963+
}))
964+
.build());
965+
}
905966

906-
private static void list(ArrayList<RpcMethodMapping> a) {}
967+
private static void list(ArrayList<RpcMethodMapping> a) {
968+
a.add(
969+
RpcMethodMapping.newBuilder(249, notifications.list)
970+
.withSetup(pubsubTopicSetup)
971+
.withTest(
972+
(ctx, c) ->
973+
ctx.map(
974+
state -> {
975+
List<com.google.cloud.storage.Notification> notifications =
976+
ctx.getStorage().listNotifications(state.getBucket().getName());
977+
return state.with(notifications);
978+
}))
979+
.build());
980+
}
907981
}
908982

909983
static final class ObjectAcl {

0 commit comments

Comments
 (0)