From 4c4be5c04076b049f343e29be18ddc96d9954f70 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 11:25:50 +0200 Subject: [PATCH 01/17] draft of generic retry with recovery --- .../bdk/core/config/model/BdkConfig.java | 4 ++ .../impl/AbstractDatafeedService.java | 3 +- .../datafeed/impl/DatafeedServiceV1.java | 20 +++++- .../datafeed/impl/DatafeedServiceV2.java | 7 ++- .../bdk/core/util/RetryWithRecovery.java | 63 +++++++++++++++++++ .../core/util/VoidSupplierWithThrowable.java | 6 ++ 6 files changed, 100 insertions(+), 3 deletions(-) create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java index 4acbbd8ff..f75799dde 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java @@ -30,4 +30,8 @@ public class BdkConfig { public boolean isOboConfigured() { return app.isConfigured(); } + + public BdkRetryConfig getDatafeedRetryConfig() { + return datafeed.getRetry() == null ? retry : datafeed.getRetry(); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index df82fc4f4..8f1082ece 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -37,11 +37,12 @@ public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, this.listeners = new ArrayList<>(); this.authSession = authSession; this.bdkConfig = config; - BdkRetryConfig bdkRetryConfig = this.bdkConfig.getDatafeed().getRetry() == null ? this.bdkConfig.getRetry() : this.bdkConfig.getDatafeed().getRetry(); + BdkRetryConfig bdkRetryConfig = this.bdkConfig.getDatafeedRetryConfig(); this.retryConfig = RetryConfig.custom() .maxAttempts(bdkRetryConfig.getMaxAttempts()) .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) .retryOnException(e -> { + //TODO if (e instanceof ApiException) { ApiException apiException = (ApiException) e; return apiException.isServerError() || apiException.isUnauthorized(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 7cdebe0be..7e835547d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -5,13 +5,16 @@ import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; +import com.symphony.bdk.core.util.RetryWithRecovery; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; +import io.github.resilience4j.core.IntervalFunction; import io.github.resilience4j.retry.Retry; import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; +import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; @@ -44,6 +47,8 @@ public class DatafeedServiceV1 extends AbstractDatafeedService { private final AtomicBoolean started = new AtomicBoolean(); private final DatafeedIdRepository datafeedRepository; private String datafeedId; + private RetryWithRecovery> readDatafeedRetry; + private RetryWithRecovery createDatafeedRetry; public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { this(datafeedApi, authSession, config, new OnDiskDatafeedIdRepository(config)); @@ -54,6 +59,16 @@ public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkCo this.started.set(false); this.datafeedId = null; this.datafeedRepository = repository; + +// this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeedv1", () -> datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null), +// e -> { +// if (e instanceof ApiException && e.getSuppressed().length == 0) { +// ApiException apiException = (ApiException) e; +// return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); +// } +// return e instanceof ProcessingException;}, +// config.getDatafeedRetryConfig(), Collections.singletonMap(e -> e.isUnauthorized(), { +// log.info("Re-authenticate and try again"); this.authSession.refresh();})); } /** @@ -91,6 +106,7 @@ public void stop() { private void readDatafeed() throws Throwable { RetryConfig config = RetryConfig.from(this.retryConfig).retryOnException(e -> { + //TODO if (e instanceof ApiException && e.getSuppressed().length == 0) { ApiException apiException = (ApiException) e; return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); @@ -105,13 +121,14 @@ private void readDatafeed() throws Throwable { handleV4EventList(events); } } catch (ApiException e) { + //TODO if (e.isUnauthorized()) { log.info("Re-authenticate and try again"); authSession.refresh(); } else { log.error("Error {}: {}", e.getCode(), e.getMessage()); if (e.isClientError()) { - log.info("Recreate a new datafeed and try again"); + log.info("Recreate a new datafeed and try again"); //recreate in case of 400 BAD REQUEST try { datafeedId = this.createDatafeed(); } catch (Throwable throwable) { @@ -135,6 +152,7 @@ protected String createDatafeed() throws Throwable { log.debug("Datafeed: {} was created and persisted", id); return id; } catch (ApiException e) { + //TODO if (e.isUnauthorized()) { log.info("Re-authenticate and try again"); authSession.refresh(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index f89f62471..eb94c48e1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -93,6 +93,7 @@ private V5Datafeed createDatafeed() throws Throwable { try { return this.datafeedApi.createDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); } catch (ApiException e) { + //TODO if (e.isUnauthorized()) { log.info("Re-authenticate and try again"); authSession.refresh(); @@ -111,6 +112,7 @@ private V5Datafeed retrieveDatafeed() throws Throwable { try { return this.datafeedApi.listDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); } catch (ApiException e) { + //TODO if (e.isUnauthorized()) { log.info("Re-authenticate and try again"); authSession.refresh(); @@ -130,6 +132,7 @@ private void readDatafeed() throws Throwable { log.debug("Reading datafeed events from datafeed {}", datafeed.getId()); RetryConfig config = RetryConfig.from(this.retryConfig) .retryOnException(e -> { + //TODO if (e instanceof ApiException && e.getSuppressed().length == 0) { ApiException apiException = (ApiException) e; return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); @@ -150,12 +153,13 @@ private void readDatafeed() throws Throwable { this.handleV4EventList(events); } } catch (ApiException e) { + //TODO if (e.isUnauthorized()) { log.info("Re-authenticate and try again"); authSession.refresh(); } else { log.error("Error {}: {}", e.getCode(), e.getMessage()); - if (e.isClientError()) { + if (e.isClientError()) { // recreate DF if 400 BAD REQUEST try { log.info("Try to delete the faulty datafeed"); this.deleteDatafeed(); @@ -180,6 +184,7 @@ private void deleteDatafeed() throws Throwable { this.datafeedApi.deleteDatafeed(datafeed.getId(), authSession.getSessionToken(), authSession.getKeyManagerToken()); this.datafeed = null; } catch (ApiException e) { + //TODO if (e.isClientError()) { log.debug("The datafeed doesn't exist or is already removed"); } else { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java new file mode 100644 index 000000000..df251cc8e --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java @@ -0,0 +1,63 @@ +package com.symphony.bdk.core.util; + +import com.symphony.bdk.core.api.invoker.ApiException; +import com.symphony.bdk.core.config.model.BdkRetryConfig; + +import io.github.resilience4j.retry.Retry; +import io.github.resilience4j.retry.RetryConfig; +import lombok.extern.slf4j.Slf4j; + +import java.util.Map; +import java.util.function.Predicate; +import java.util.function.Supplier; + +@Slf4j +public class RetryWithRecovery { + private SupplierWithApiException supplier; + private Map, VoidSupplierWithThrowable> recoveryStrategies; + private Retry retry; + + public RetryWithRecovery(String name, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, + BdkRetryConfig bdkRetryConfig, Map, VoidSupplierWithThrowable> recoveryStrategies) { + this.supplier = supplier; + this.recoveryStrategies = recoveryStrategies; + this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); + } + + public T execute() throws Throwable { + return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); + } + + private T executeMainAndRecoveryStrategies() throws Throwable { + try { + return supplier.get(); + } catch (ApiException e) { + for (Map.Entry, VoidSupplierWithThrowable> entry : recoveryStrategies.entrySet()) { + if(entry.getKey().test(e)) { + entry.getValue().get(); + } + } + throw e; + } + } + + private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, Predicate retryOnExceptionPredicate) { + final RetryConfig retryConfig = RetryConfig.custom() + .maxAttempts(bdkRetryConfig.getMaxAttempts()) + .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) + .retryOnException(retryOnExceptionPredicate) + .build(); + + Retry retry = Retry.of(name, retryConfig); + retry.getEventPublisher().onRetry(event -> { + long intervalInMillis = event.getWaitInterval().toMillis(); + double interval = intervalInMillis / 1000.0; + if (event.getLastThrowable() != null) { + log.debug("{} service failed due to {}", name, event.getLastThrowable().getMessage()); + } + log.info("Retry in {} secs...", interval); + }); + + return retry; + } +} diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java new file mode 100644 index 000000000..f68f1b2d3 --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java @@ -0,0 +1,6 @@ +package com.symphony.bdk.core.util; + +@FunctionalInterface +public interface VoidSupplierWithThrowable { + void get() throws Throwable; +} From cd8850dbf66933593b430b43349bf00004ede704 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 13:44:23 +0200 Subject: [PATCH 02/17] Retry in DFv1 --- .../datafeed/impl/DatafeedServiceV1.java | 92 +++++++++---------- .../bdk/core/util/ConsumerWithThrowable.java | 8 ++ .../bdk/core/util/RetryWithRecovery.java | 11 ++- .../core/util/VoidSupplierWithThrowable.java | 6 -- 4 files changed, 59 insertions(+), 58 deletions(-) create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java delete mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 7e835547d..9191e7205 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -6,18 +6,20 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; import com.symphony.bdk.core.util.RetryWithRecovery; +import com.symphony.bdk.core.util.ConsumerWithThrowable; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; -import io.github.resilience4j.core.IntervalFunction; import io.github.resilience4j.retry.Retry; import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; -import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Predicate; import javax.ws.rs.ProcessingException; @@ -47,7 +49,7 @@ public class DatafeedServiceV1 extends AbstractDatafeedService { private final AtomicBoolean started = new AtomicBoolean(); private final DatafeedIdRepository datafeedRepository; private String datafeedId; - private RetryWithRecovery> readDatafeedRetry; + private RetryWithRecovery readDatafeedRetry; private RetryWithRecovery createDatafeedRetry; public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { @@ -60,17 +62,16 @@ public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkCo this.datafeedId = null; this.datafeedRepository = repository; -// this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeedv1", () -> datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null), -// e -> { -// if (e instanceof ApiException && e.getSuppressed().length == 0) { -// ApiException apiException = (ApiException) e; -// return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); -// } -// return e instanceof ProcessingException;}, -// config.getDatafeedRetryConfig(), Collections.singletonMap(e -> e.isUnauthorized(), { -// log.info("Re-authenticate and try again"); this.authSession.refresh();})); + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(); + readRecoveryStrategy.put(ApiException::isUnauthorized, e -> refresh()); + readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); + + this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeedv1", () -> {readAndHandleEvents(); return null;}, + this::retryReadOnException, this.bdkConfig.getDatafeedRetryConfig(), readRecoveryStrategy); } + + /** * {@inheritDoc} */ @@ -104,42 +105,39 @@ public void stop() { this.started.set(false); } + private boolean retryReadOnException(Throwable t) { + if (t instanceof ApiException && t.getSuppressed().length == 0) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); + } + return t instanceof ProcessingException; + } + + private void readAndHandleEvents() throws ApiException { + log.debug("Read DFv1 with RetryWithRecovery"); + List events = datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); + if (events != null && !events.isEmpty()) { + handleV4EventList(events); + } + } + + private void refresh() throws AuthUnauthorizedException { + log.info("Re-authenticate and try again"); + authSession.refresh(); + } + + private void recreateDatafeed(ApiException e) { + log.info("Recreate a new datafeed and try again"); + try { + datafeedId = this.createDatafeed(); + } catch (Throwable throwable) { + e.addSuppressed(throwable); + } + } + private void readDatafeed() throws Throwable { - RetryConfig config = RetryConfig.from(this.retryConfig).retryOnException(e -> { - //TODO - if (e instanceof ApiException && e.getSuppressed().length == 0) { - ApiException apiException = (ApiException) e; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); - } - return e instanceof ProcessingException; - }).build(); - Retry retry = this.getRetryInstance("Read Datafeed", config); - retry.executeCheckedSupplier(() -> { - try { - List events = datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); - if (events != null && !events.isEmpty()) { - handleV4EventList(events); - } - } catch (ApiException e) { - //TODO - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - if (e.isClientError()) { - log.info("Recreate a new datafeed and try again"); //recreate in case of 400 BAD REQUEST - try { - datafeedId = this.createDatafeed(); - } catch (Throwable throwable) { - e.addSuppressed(throwable); - } - } - } - throw e; - } - return null; - }); + log.debug("Read DFv1, calling RetryWithRecovery"); + this.readDatafeedRetry.execute(); } protected String createDatafeed() throws Throwable { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java new file mode 100644 index 000000000..b72f9c9a4 --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java @@ -0,0 +1,8 @@ +package com.symphony.bdk.core.util; + +import com.symphony.bdk.core.api.invoker.ApiException; + +@FunctionalInterface +public interface ConsumerWithThrowable { + void get(ApiException e) throws Throwable; +} diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java index df251cc8e..10ad20b4d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java @@ -9,22 +9,22 @@ import java.util.Map; import java.util.function.Predicate; -import java.util.function.Supplier; @Slf4j public class RetryWithRecovery { private SupplierWithApiException supplier; - private Map, VoidSupplierWithThrowable> recoveryStrategies; + private Map, ConsumerWithThrowable> recoveryStrategies; private Retry retry; public RetryWithRecovery(String name, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, - BdkRetryConfig bdkRetryConfig, Map, VoidSupplierWithThrowable> recoveryStrategies) { + BdkRetryConfig bdkRetryConfig, Map, ConsumerWithThrowable> recoveryStrategies) { this.supplier = supplier; this.recoveryStrategies = recoveryStrategies; this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); } public T execute() throws Throwable { + log.debug("RetryWithRecovery::execute"); return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); } @@ -32,9 +32,10 @@ private T executeMainAndRecoveryStrategies() throws Throwable { try { return supplier.get(); } catch (ApiException e) { - for (Map.Entry, VoidSupplierWithThrowable> entry : recoveryStrategies.entrySet()) { + log.error("Error {}: {}", e.getCode(), e.getMessage()); + for (Map.Entry, ConsumerWithThrowable> entry : recoveryStrategies.entrySet()) { if(entry.getKey().test(e)) { - entry.getValue().get(); + entry.getValue().get(e); } } throw e; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java deleted file mode 100644 index f68f1b2d3..000000000 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/VoidSupplierWithThrowable.java +++ /dev/null @@ -1,6 +0,0 @@ -package com.symphony.bdk.core.util; - -@FunctionalInterface -public interface VoidSupplierWithThrowable { - void get() throws Throwable; -} From 73f1d08189de7e2a47936826b1a24334e14a6402 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 14:17:40 +0200 Subject: [PATCH 03/17] Retry with recovery DFv1 --- .../datafeed/impl/DatafeedServiceV1.java | 240 +++++++++--------- .../bdk/core/util/ConsumerWithThrowable.java | 2 +- .../bdk/core/util/RetryWithRecovery.java | 29 ++- 3 files changed, 140 insertions(+), 131 deletions(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 9191e7205..6fd54a67d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -10,10 +10,9 @@ import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; -import io.github.resilience4j.retry.Retry; -import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -25,146 +24,145 @@ /** * A class for implementing the datafeed v1 service. - * + *

* This service will be started by calling {@link DatafeedServiceV1#start()} - * + *

* At the beginning, a datafeed will be created and the BDK bot will listen to this datafeed to receive the real-time * events. With datafeed service v1, we don't have the api endpoint to retrieve the datafeed id that a service account * is listening, so, the id of the created datafeed must be persisted in the bot side. - * + *

* The BDK bot will listen to this datafeed to get all the received real-time events. - * + *

* From the second time, the bot will firstly retrieve the datafeed that was persisted and try to read the real-time * events from this datafeed. If this datafeed is expired or faulty, the datafeed service will create the new one for * listening. - * + *

* This service will be stopped by calling {@link DatafeedServiceV1#stop()} - * + *

* If the datafeed service is stopped during a read datafeed call, it has to wait until the last read finish to be * really stopped */ @Slf4j public class DatafeedServiceV1 extends AbstractDatafeedService { - private final AtomicBoolean started = new AtomicBoolean(); - private final DatafeedIdRepository datafeedRepository; - private String datafeedId; - private RetryWithRecovery readDatafeedRetry; - private RetryWithRecovery createDatafeedRetry; - - public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { - this(datafeedApi, authSession, config, new OnDiskDatafeedIdRepository(config)); - } - - public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config, DatafeedIdRepository repository) { - super(datafeedApi, authSession, config); - this.started.set(false); - this.datafeedId = null; - this.datafeedRepository = repository; - - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(); - readRecoveryStrategy.put(ApiException::isUnauthorized, e -> refresh()); - readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - - this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeedv1", () -> {readAndHandleEvents(); return null;}, - this::retryReadOnException, this.bdkConfig.getDatafeedRetryConfig(), readRecoveryStrategy); + private final AtomicBoolean started = new AtomicBoolean(); + private final DatafeedIdRepository datafeedRepository; + private String datafeedId; + private RetryWithRecovery readDatafeedRetry; + private RetryWithRecovery createDatafeedRetry; + + public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { + this(datafeedApi, authSession, config, new OnDiskDatafeedIdRepository(config)); + } + + public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config, + DatafeedIdRepository repository) { + super(datafeedApi, authSession, config); + this.started.set(false); + this.datafeedId = null; + this.datafeedRepository = repository; + + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(); + readRecoveryStrategy.put(ApiException::isUnauthorized, e -> refresh()); + readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); + + this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V1", this::readAndHandleEvents, + this::retryReadOnException, this.bdkConfig.getDatafeedRetryConfig(), readRecoveryStrategy); + + Map, ConsumerWithThrowable> createRecoveryStrategy = + Collections.singletonMap(ApiException::isUnauthorized, e -> refresh()); + + this.createDatafeedRetry = new RetryWithRecovery<>("Create Datafeed V1", this::createDatafeedAndPersist, + this::retryCreateOnException, this.bdkConfig.getDatafeedRetryConfig(), createRecoveryStrategy); + } + + /** + * {@inheritDoc} + */ + @Override + public void start() throws AuthUnauthorizedException, ApiException { + if (this.started.get()) { + throw new IllegalStateException("The datafeed service is already started"); } - - - - /** - * {@inheritDoc} - */ - @Override - public void start() throws AuthUnauthorizedException, ApiException { - if (this.started.get()) { - throw new IllegalStateException("The datafeed service is already started"); - } - Optional persistedDatafeedId = this.retrieveDatafeed(); - - try { - this.datafeedId = persistedDatafeedId.orElse(this.createDatafeed()); - log.debug("Start reading events from datafeed {}", datafeedId); - this.started.set(true); - do { - this.readDatafeed(); - } while (this.started.get()); - } catch (AuthUnauthorizedException | ApiException exception) { - throw exception; - } catch (Throwable throwable) { - log.error("Unknown error", throwable); - } + Optional persistedDatafeedId = this.retrieveDatafeed(); + + try { + this.datafeedId = persistedDatafeedId.orElse(this.createDatafeed()); + log.debug("Start reading events from datafeed {}", datafeedId); + this.started.set(true); + do { + readDatafeedRetry.execute(); + } while (this.started.get()); + } catch (AuthUnauthorizedException | ApiException exception) { + throw exception; + } catch (Throwable throwable) { + log.error("Unknown error", throwable); } - - /** - * {@inheritDoc} - */ - @Override - public void stop() { - log.info("Stop the datafeed service"); - this.started.set(false); + } + + /** + * {@inheritDoc} + */ + @Override + public void stop() { + log.info("Stop the datafeed service"); + this.started.set(false); + } + + private boolean retryReadOnException(Throwable t) { + if (t instanceof ApiException && t.getSuppressed().length == 0) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); } + return t instanceof ProcessingException; + } - private boolean retryReadOnException(Throwable t) { - if (t instanceof ApiException && t.getSuppressed().length == 0) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); - } - return t instanceof ProcessingException; + private boolean retryCreateOnException(Throwable t) { + if (t instanceof ApiException) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized(); } - - private void readAndHandleEvents() throws ApiException { - log.debug("Read DFv1 with RetryWithRecovery"); - List events = datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); - if (events != null && !events.isEmpty()) { - handleV4EventList(events); - } + return t instanceof ProcessingException; + } + + private Void readAndHandleEvents() throws ApiException { + List events = + datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); + if (events != null && !events.isEmpty()) { + handleV4EventList(events); } - - private void refresh() throws AuthUnauthorizedException { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } - - private void recreateDatafeed(ApiException e) { - log.info("Recreate a new datafeed and try again"); - try { - datafeedId = this.createDatafeed(); - } catch (Throwable throwable) { - e.addSuppressed(throwable); - } - } - - private void readDatafeed() throws Throwable { - log.debug("Read DFv1, calling RetryWithRecovery"); - this.readDatafeedRetry.execute(); - } - - protected String createDatafeed() throws Throwable { - log.debug("Start creating a new datafeed and persisting it"); - Retry retry = this.getRetryInstance("Create Datafeed"); - return retry.executeCheckedSupplier(() -> { - try { - String id = this.datafeedApi.v4DatafeedCreatePost(authSession.getSessionToken(), authSession.getKeyManagerToken()).getId(); - this.datafeedRepository.write(id); - log.debug("Datafeed: {} was created and persisted", id); - return id; - } catch (ApiException e) { - //TODO - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - } - throw e; - } - }); - } - - protected Optional retrieveDatafeed() { - log.debug("Start retrieving datafeed id"); - return this.datafeedRepository.read(); + return null; + } + + private void refresh() throws AuthUnauthorizedException { + log.info("Re-authenticate and try again"); + authSession.refresh(); + } + + private void recreateDatafeed(ApiException e) { + log.info("Recreate a new datafeed and try again"); + try { + datafeedId = createDatafeed(); + } catch (Throwable throwable) { + e.addSuppressed(throwable); } + } + + protected String createDatafeed() throws Throwable { + log.debug("Start creating a new datafeed and persisting it"); + return createDatafeedRetry.execute(); + } + + private String createDatafeedAndPersist() throws ApiException { + String id = datafeedApi.v4DatafeedCreatePost(authSession.getSessionToken(), authSession.getKeyManagerToken()).getId(); + datafeedRepository.write(id); + log.debug("Datafeed: {} was created and persisted", id); + return id; + } + + protected Optional retrieveDatafeed() { + log.debug("Start retrieving datafeed id"); + return this.datafeedRepository.read(); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java index b72f9c9a4..242fdd572 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java @@ -4,5 +4,5 @@ @FunctionalInterface public interface ConsumerWithThrowable { - void get(ApiException e) throws Throwable; + void consume(ApiException e) throws Throwable; } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java index 10ad20b4d..bceeb2f6d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java @@ -16,7 +16,8 @@ public class RetryWithRecovery { private Map, ConsumerWithThrowable> recoveryStrategies; private Retry retry; - public RetryWithRecovery(String name, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, + public RetryWithRecovery(String name, SupplierWithApiException supplier, + Predicate retryOnExceptionPredicate, BdkRetryConfig bdkRetryConfig, Map, ConsumerWithThrowable> recoveryStrategies) { this.supplier = supplier; this.recoveryStrategies = recoveryStrategies; @@ -24,7 +25,6 @@ public RetryWithRecovery(String name, SupplierWithApiException supplier, Pred } public T execute() throws Throwable { - log.debug("RetryWithRecovery::execute"); return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); } @@ -32,17 +32,28 @@ private T executeMainAndRecoveryStrategies() throws Throwable { try { return supplier.get(); } catch (ApiException e) { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - for (Map.Entry, ConsumerWithThrowable> entry : recoveryStrategies.entrySet()) { - if(entry.getKey().test(e)) { - entry.getValue().get(e); - } - } + handleRecovery(e); throw e; } } - private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, Predicate retryOnExceptionPredicate) { + private void handleRecovery(ApiException e) throws Throwable { + boolean recoveryTriggered = false; + + for (Map.Entry, ConsumerWithThrowable> entry : recoveryStrategies.entrySet()) { + if (entry.getKey().test(e)) { + recoveryTriggered = true; + entry.getValue().consume(e); + } + } + + if (!recoveryTriggered) { + log.error("Error {}: {}", e.getCode(), e.getMessage()); + } + } + + private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, + Predicate retryOnExceptionPredicate) { final RetryConfig retryConfig = RetryConfig.custom() .maxAttempts(bdkRetryConfig.getMaxAttempts()) .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) From ecfcb035ff37cb86458e98dbad161464c60a8bea Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 15:55:15 +0200 Subject: [PATCH 04/17] Retry with recovery on DFv1 and DFv2 --- .../impl/AbstractDatafeedService.java | 161 +++++----- .../datafeed/impl/DatafeedServiceV1.java | 42 +-- .../datafeed/impl/DatafeedServiceV2.java | 289 ++++++++---------- .../bdk/core/util/RetryWithRecovery.java | 58 ++-- 4 files changed, 255 insertions(+), 295 deletions(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index 8f1082ece..d06eeb6c4 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -2,11 +2,13 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.datafeed.DatafeedService; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; import com.symphony.bdk.core.util.BdkExponentialFunction; +import com.symphony.bdk.core.util.ConsumerWithThrowable; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; @@ -15,7 +17,11 @@ import lombok.extern.slf4j.Slf4j; import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; import java.util.List; +import java.util.Map; +import java.util.function.Predicate; import javax.ws.rs.ProcessingException; @@ -26,94 +32,91 @@ @Slf4j abstract class AbstractDatafeedService implements DatafeedService { - protected final AuthSession authSession; - protected final BdkConfig bdkConfig; - protected final List listeners; - protected final RetryConfig retryConfig; - protected DatafeedApi datafeedApi; - - public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { - this.datafeedApi = datafeedApi; - this.listeners = new ArrayList<>(); - this.authSession = authSession; - this.bdkConfig = config; - BdkRetryConfig bdkRetryConfig = this.bdkConfig.getDatafeedRetryConfig(); - this.retryConfig = RetryConfig.custom() - .maxAttempts(bdkRetryConfig.getMaxAttempts()) - .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) - .retryOnException(e -> { - //TODO - if (e instanceof ApiException) { - ApiException apiException = (ApiException) e; - return apiException.isServerError() || apiException.isUnauthorized(); - } - return e instanceof ProcessingException; - }) - .build(); - } + protected final AuthSession authSession; + protected final BdkConfig bdkConfig; + protected final List listeners; + protected DatafeedApi datafeedApi; - /** - * {@inheritDoc} - */ - @Override - public void subscribe(RealTimeEventListener listener) { - listeners.add(listener); - } + public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { + this.datafeedApi = datafeedApi; + this.listeners = new ArrayList<>(); + this.authSession = authSession; + this.bdkConfig = config; + } - /** - * {@inheritDoc} - */ - @Override - public void unsubscribe(RealTimeEventListener listener) { - listeners.remove(listener); - } + /** + * {@inheritDoc} + */ + @Override + public void subscribe(RealTimeEventListener listener) { + listeners.add(listener); + } + + /** + * {@inheritDoc} + */ + @Override + public void unsubscribe(RealTimeEventListener listener) { + listeners.remove(listener); + } - /** - * Handle a received listener by using the subscribed {@link RealTimeEventListener}. - * - * @param events List of Datafeed events to be handled - * - */ - protected void handleV4EventList(List events) { - for (V4Event event : events) { - if (event == null || event.getType() == null) { - continue; - } - if (this.isSelfGeneratedEvent(event)) { - continue; - } - try { - RealTimeEventType eventType = RealTimeEventType.valueOf(event.getType()); - for (RealTimeEventListener listener : listeners) { - eventType.dispatch(listener, event); - } - } catch (IllegalArgumentException e) { - log.warn("Receive events with unknown type: {}", event.getType()); - } + /** + * Handle a received listener by using the subscribed {@link RealTimeEventListener}. + * + * @param events List of Datafeed events to be handled + */ + protected void handleV4EventList(List events) { + for (V4Event event : events) { + if (event == null || event.getType() == null) { + continue; + } + if (this.isSelfGeneratedEvent(event)) { + continue; + } + try { + RealTimeEventType eventType = RealTimeEventType.valueOf(event.getType()); + for (RealTimeEventListener listener : listeners) { + eventType.dispatch(listener, event); } + } catch (IllegalArgumentException e) { + log.warn("Receive events with unknown type: {}", event.getType()); + } } + } - private boolean isSelfGeneratedEvent(V4Event event) { - return event.getInitiator() != null && event.getInitiator().getUser() != null - && event.getInitiator().getUser().getUsername() != null - && event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); - } + private boolean isSelfGeneratedEvent(V4Event event) { + return event.getInitiator() != null && event.getInitiator().getUser() != null + && event.getInitiator().getUser().getUsername() != null + && event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); + } + + protected Map, ConsumerWithThrowable> getSessionRefreshStrategy() { + return Collections.singletonMap(ApiException::isUnauthorized, e -> refresh()); + } - protected Retry getRetryInstance(String name, RetryConfig... config) { - Retry retry = config.length == 0 ? Retry.of(name, this.retryConfig) : Retry.of(name, config[0]); - retry.getEventPublisher().onRetry(event -> { - long intervalInMillis = event.getWaitInterval().toMillis(); - double interval = intervalInMillis / 1000.0; - if (event.getLastThrowable() != null) { - log.debug("Datafeed service failed due to {}", event.getLastThrowable().getMessage()); - } - log.info("Retry in {} secs...", interval); - }); - return retry; + protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { + if (t instanceof ApiException && t.getSuppressed().length == 0) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); } + return t instanceof ProcessingException; + } - protected void setDatafeedApi(DatafeedApi datafeedApi) { - this.datafeedApi = datafeedApi; + protected boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { + if (t instanceof ApiException) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized(); } + return t instanceof ProcessingException; + } + + protected void refresh() throws AuthUnauthorizedException { + log.info("Re-authenticate and try again"); + authSession.refresh(); + } + + protected void setDatafeedApi(DatafeedApi datafeedApi) { + this.datafeedApi = datafeedApi; + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 6fd54a67d..49210604f 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -20,8 +20,6 @@ import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; -import javax.ws.rs.ProcessingException; - /** * A class for implementing the datafeed v1 service. *

@@ -47,9 +45,9 @@ public class DatafeedServiceV1 extends AbstractDatafeedService { private final AtomicBoolean started = new AtomicBoolean(); private final DatafeedIdRepository datafeedRepository; + private final RetryWithRecovery readDatafeedRetry; + private final RetryWithRecovery createDatafeedRetry; private String datafeedId; - private RetryWithRecovery readDatafeedRetry; - private RetryWithRecovery createDatafeedRetry; public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { this(datafeedApi, authSession, config, new OnDiskDatafeedIdRepository(config)); @@ -62,18 +60,14 @@ public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkCo this.datafeedId = null; this.datafeedRepository = repository; - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(); - readRecoveryStrategy.put(ApiException::isUnauthorized, e -> refresh()); + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V1", this::readAndHandleEvents, - this::retryReadOnException, this.bdkConfig.getDatafeedRetryConfig(), readRecoveryStrategy); - - Map, ConsumerWithThrowable> createRecoveryStrategy = - Collections.singletonMap(ApiException::isUnauthorized, e -> refresh()); + this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), + this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy); - this.createDatafeedRetry = new RetryWithRecovery<>("Create Datafeed V1", this::createDatafeedAndPersist, - this::retryCreateOnException, this.bdkConfig.getDatafeedRetryConfig(), createRecoveryStrategy); + this.createDatafeedRetry = new RetryWithRecovery<>("Create Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), + this::createDatafeedAndPersist, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); } /** @@ -109,22 +103,6 @@ public void stop() { this.started.set(false); } - private boolean retryReadOnException(Throwable t) { - if (t instanceof ApiException && t.getSuppressed().length == 0) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); - } - return t instanceof ProcessingException; - } - - private boolean retryCreateOnException(Throwable t) { - if (t instanceof ApiException) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized(); - } - return t instanceof ProcessingException; - } - private Void readAndHandleEvents() throws ApiException { List events = datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); @@ -134,11 +112,6 @@ private Void readAndHandleEvents() throws ApiException { return null; } - private void refresh() throws AuthUnauthorizedException { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } - private void recreateDatafeed(ApiException e) { log.info("Recreate a new datafeed and try again"); try { @@ -164,5 +137,4 @@ protected Optional retrieveDatafeed() { log.debug("Start retrieving datafeed id"); return this.datafeedRepository.read(); } - } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index eb94c48e1..6d4e44bb9 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -4,201 +4,174 @@ import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; +import com.symphony.bdk.core.util.ConsumerWithThrowable; +import com.symphony.bdk.core.util.RetryWithRecovery; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.AckId; import com.symphony.bdk.gen.api.model.V4Event; import com.symphony.bdk.gen.api.model.V5Datafeed; import com.symphony.bdk.gen.api.model.V5EventList; -import io.github.resilience4j.retry.Retry; -import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; - -import javax.ws.rs.ProcessingException; +import java.util.function.Predicate; /** * A class for implementing the datafeed v2 service. - * + *

* This service will be started by calling {@link DatafeedServiceV2#start()} - * + *

* At the beginning, the BDK bot will try to retrieve the list of datafeed to which it is listening. Since each bot * should only listening to just one datafeed, the first datafeed in the list will be used by the bot to be listened to. * If the retrieved list is empty, the BDK bot will create a new datafeed to listen. - * + *

* The BDK bot will listen to this datafeed to get all the received real-time events. - * + *

* If this datafeed becomes stale or faulty, the BDK bot will create the new one for listening. - * + *

* This service will be stopped by calling {@link DatafeedServiceV2#stop()} - * + *

* If the datafeed service is stopped during a read datafeed call, it has to wait until the last read finish to be * really stopped */ @Slf4j public class DatafeedServiceV2 extends AbstractDatafeedService { - private final AtomicBoolean started = new AtomicBoolean(); - private final AckId ackId; - private V5Datafeed datafeed; + private final AtomicBoolean started = new AtomicBoolean(); + private final AckId ackId; + private V5Datafeed datafeed; + private final RetryWithRecovery retrieveDatafeedRetry; + private final RetryWithRecovery deleteDatafeedRetry; + private final RetryWithRecovery createDatafeedRetry; + private final RetryWithRecovery readDatafeedRetry; - public DatafeedServiceV2(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { - super(datafeedApi, authSession, config); - this.ackId = new AckId().ackId(""); - } - /** - * {@inheritDoc} - */ - @Override - public void start() throws ApiException, AuthUnauthorizedException { - if (this.started.get()) { - throw new IllegalStateException("The datafeed service is already started"); - } - try { - this.datafeed = this.retrieveDatafeed(); - if (this.datafeed == null) { - this.datafeed = this.createDatafeed(); - } - log.debug("Start reading datafeed events"); - this.started.set(true); - do { - this.readDatafeed(); - } while (this.started.get()); - } catch (AuthUnauthorizedException | ApiException exception) { - throw exception; - } catch (Throwable throwable) { - log.error("Unknown error", throwable); - } - } + public DatafeedServiceV2(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { + super(datafeedApi, authSession, config); + this.ackId = new AckId().ackId(""); - protected AckId getAckId() { - return this.ackId; - } + this.retrieveDatafeedRetry = + new RetryWithRecovery<>("Retrieve Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::tryRetrieveDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); - /** - * {@inheritDoc} - */ - @Override - public void stop() { - this.started.set(false); - } + this.deleteDatafeedRetry = + new RetryWithRecovery<>("Delete Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::tryDeleteDatafeed, this::isNetworkOrServerOrUnauthorizedError, ApiException::isClientError, getSessionRefreshStrategy()); - private V5Datafeed createDatafeed() throws Throwable { - log.debug("Start creating datafeed from agent"); - Retry retry = this.getRetryInstance("Create V5Datafeed"); - return retry.executeCheckedSupplier(() -> { - try { - return this.datafeedApi.createDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); - } catch (ApiException e) { - //TODO - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - } - throw e; - } - }); + this.createDatafeedRetry = + new RetryWithRecovery<>("Create Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::tryCreateDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); + + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); + readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); + + this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy); + } + + /** + * {@inheritDoc} + */ + @Override + public void start() throws ApiException, AuthUnauthorizedException { + if (this.started.get()) { + throw new IllegalStateException("The datafeed service is already started"); + } + try { + this.datafeed = this.retrieveDatafeed(); + if (this.datafeed == null) { + this.datafeed = this.createDatafeed(); + } + log.debug("Start reading datafeed events"); + this.started.set(true); + do { + this.readDatafeed(); + } while (this.started.get()); + } catch (AuthUnauthorizedException | ApiException exception) { + throw exception; + } catch (Throwable throwable) { + log.error("Unknown error", throwable); } + } - private V5Datafeed retrieveDatafeed() throws Throwable { - log.debug("Start retrieving datafeed from agent"); - Retry retry = this.getRetryInstance("Retrieve V5Datafeed"); - List datafeeds = retry.executeCheckedSupplier(() -> { - try { - return this.datafeedApi.listDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); - } catch (ApiException e) { - //TODO - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - } - throw e; - } - }); - if (!datafeeds.isEmpty()) { - return datafeeds.get(0); - } - return null; + protected AckId getAckId() { + return this.ackId; + } + + /** + * {@inheritDoc} + */ + @Override + public void stop() { + this.started.set(false); + } + + private V5Datafeed createDatafeed() throws Throwable { + log.debug("Start creating datafeed from agent"); + return createDatafeedRetry.execute(); + } + + private V5Datafeed tryCreateDatafeed() throws ApiException { + return this.datafeedApi.createDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); + } + + private V5Datafeed retrieveDatafeed() throws Throwable { + log.debug("Start retrieving datafeed from agent"); + return retrieveDatafeedRetry.execute(); + } + + private V5Datafeed tryRetrieveDatafeed() throws ApiException { + final List datafeeds = + this.datafeedApi.listDatafeed(authSession.getSessionToken(), authSession.getKeyManagerToken()); + + if (!datafeeds.isEmpty()) { + return datafeeds.get(0); } + return null; + } - private void readDatafeed() throws Throwable { - log.debug("Reading datafeed events from datafeed {}", datafeed.getId()); - RetryConfig config = RetryConfig.from(this.retryConfig) - .retryOnException(e -> { - //TODO - if (e instanceof ApiException && e.getSuppressed().length == 0) { - ApiException apiException = (ApiException) e; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); - } - return e instanceof ProcessingException; - }).build(); - Retry retry = this.getRetryInstance("Read Datafeed", config); - retry.executeCheckedSupplier(() -> { - try { - V5EventList v5EventList = this.datafeedApi.readDatafeed( - datafeed.getId(), - authSession.getSessionToken(), - authSession.getKeyManagerToken(), - ackId); - this.ackId.setAckId(v5EventList.getAckId()); - List events = v5EventList.getEvents(); - if (events != null && !events.isEmpty()) { - this.handleV4EventList(events); - } - } catch (ApiException e) { - //TODO - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - if (e.isClientError()) { // recreate DF if 400 BAD REQUEST - try { - log.info("Try to delete the faulty datafeed"); - this.deleteDatafeed(); - log.info("Recreate a new datafeed and try again"); - this.datafeed = this.createDatafeed(); - } catch (Throwable throwable) { - e.addSuppressed(throwable); - } - } - } - throw e; - } - return null; - }); + private void readDatafeed() throws Throwable { + log.debug("Reading datafeed events from datafeed {}", datafeed.getId()); + this.readDatafeedRetry.execute(); + } + + private Void readAndHandleEvents() throws ApiException { + V5EventList v5EventList = this.datafeedApi.readDatafeed( + datafeed.getId(), + authSession.getSessionToken(), + authSession.getKeyManagerToken(), + ackId); + this.ackId.setAckId(v5EventList.getAckId()); + List events = v5EventList.getEvents(); + if (events != null && !events.isEmpty()) { + this.handleV4EventList(events); } + return null; + } - private void deleteDatafeed() throws Throwable { - log.debug("Start deleting a faulty datafeed"); - Retry retry = this.getRetryInstance("Delete Datafeed"); - retry.executeCheckedSupplier(() -> { - try { - this.datafeedApi.deleteDatafeed(datafeed.getId(), authSession.getSessionToken(), authSession.getKeyManagerToken()); - this.datafeed = null; - } catch (ApiException e) { - //TODO - if (e.isClientError()) { - log.debug("The datafeed doesn't exist or is already removed"); - } else { - if (e.isUnauthorized()) { - log.info("Re-authenticate and try again"); - authSession.refresh(); - } else { - log.error("Error {}: {}", e.getCode(), e.getMessage()); - } - throw e; - } - } - return null; - }); + private void recreateDatafeed(ApiException e) { + try { + log.info("Try to delete the faulty datafeed"); + this.deleteDatafeed(); + log.info("Recreate a new datafeed and try again"); + this.datafeed = this.createDatafeed(); + } catch (Throwable throwable) { + e.addSuppressed(throwable); } + } + + private void deleteDatafeed() throws Throwable { + log.debug("Start deleting a faulty datafeed"); + deleteDatafeedRetry.execute(); + } + private Void tryDeleteDatafeed() throws ApiException { + this.datafeedApi.deleteDatafeed(datafeed.getId(), authSession.getSessionToken(), authSession.getKeyManagerToken()); + this.datafeed = null; + return null; + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java index bceeb2f6d..1369a3e51 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java @@ -13,25 +13,58 @@ @Slf4j public class RetryWithRecovery { private SupplierWithApiException supplier; + private Predicate ignoreApiException; private Map, ConsumerWithThrowable> recoveryStrategies; private Retry retry; - public RetryWithRecovery(String name, SupplierWithApiException supplier, + public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, - BdkRetryConfig bdkRetryConfig, Map, ConsumerWithThrowable> recoveryStrategies) { + Map, ConsumerWithThrowable> recoveryStrategies) { + this(name, bdkRetryConfig, supplier, retryOnExceptionPredicate, (e) -> false, recoveryStrategies); + } + + public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, + Predicate retryOnExceptionPredicate, Predicate ignoreApiException, + Map, ConsumerWithThrowable> recoveryStrategies) { this.supplier = supplier; + this.ignoreApiException = ignoreApiException; this.recoveryStrategies = recoveryStrategies; this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); } public T execute() throws Throwable { + log.debug("RetryWithRecovery::execute"); return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); } + private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, + Predicate retryOnExceptionPredicate) { + RetryConfig retryConfig = RetryConfig.custom() + .maxAttempts(bdkRetryConfig.getMaxAttempts()) + .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) + .retryOnException(retryOnExceptionPredicate) + .build(); + + Retry retry = Retry.of(name, retryConfig); + retry.getEventPublisher().onRetry(event -> { + double interval = event.getWaitInterval().toMillis() / 1000.0; + if (event.getLastThrowable() != null) { + log.debug("{} service failed due to {}", name, event.getLastThrowable().getMessage()); + } + log.info("Retry in {}s...", interval); + }); + + return retry; + } + private T executeMainAndRecoveryStrategies() throws Throwable { try { return supplier.get(); } catch (ApiException e) { + if (ignoreApiException.test(e)) { + return null; + } + handleRecovery(e); throw e; } @@ -51,25 +84,4 @@ private void handleRecovery(ApiException e) throws Throwable { log.error("Error {}: {}", e.getCode(), e.getMessage()); } } - - private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, - Predicate retryOnExceptionPredicate) { - final RetryConfig retryConfig = RetryConfig.custom() - .maxAttempts(bdkRetryConfig.getMaxAttempts()) - .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) - .retryOnException(retryOnExceptionPredicate) - .build(); - - Retry retry = Retry.of(name, retryConfig); - retry.getEventPublisher().onRetry(event -> { - long intervalInMillis = event.getWaitInterval().toMillis(); - double interval = intervalInMillis / 1000.0; - if (event.getLastThrowable() != null) { - log.debug("{} service failed due to {}", name, event.getLastThrowable().getMessage()); - } - log.info("Retry in {} secs...", interval); - }); - - return retry; - } } From dd87889e76e2f11e891a51708811d06776043ac0 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 16:22:14 +0200 Subject: [PATCH 05/17] javadoc --- .../bdk/core/util/ConsumerWithThrowable.java | 4 +++ .../bdk/core/util/RetryWithRecovery.java | 34 ++++++++++++++++++- .../core/util/SupplierWithApiException.java | 2 +- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java index 242fdd572..c1684553c 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java @@ -2,6 +2,10 @@ import com.symphony.bdk.core.api.invoker.ApiException; +/** + * Functional interface which consumes an {@link ApiException} and may throw a {@link Throwable}. + * This is used to specify recovery functions in {@link ConsumerWithThrowable}. + */ @FunctionalInterface public interface ConsumerWithThrowable { void consume(ApiException e) throws Throwable; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java index 1369a3e51..f30bfd98a 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java @@ -10,6 +10,11 @@ import java.util.Map; import java.util.function.Predicate; +/** + * This class aims to implement a retry mechanism (on top of a{@link Retry}) + * with different recovery strategies based on predicates. + * @param the type of the object returned by {@link #execute()} + */ @Slf4j public class RetryWithRecovery { private SupplierWithApiException supplier; @@ -17,12 +22,34 @@ public class RetryWithRecovery { private Map, ConsumerWithThrowable> recoveryStrategies; private Retry retry; + /** + * Constructor with no predicate on when to ignore an {@link ApiException}, + * i.e. ApiExceptions will never be ignored. + * @param name the name of the {@link Retry} service. + * @param bdkRetryConfig the retry configuration to be used. + * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. + * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. + * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. + * If several predicates match, all corresponding consumers will be executed. + */ public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, Map, ConsumerWithThrowable> recoveryStrategies) { this(name, bdkRetryConfig, supplier, retryOnExceptionPredicate, (e) -> false, recoveryStrategies); } + /** + * Constructor with a predicate on when to ignore an {@link ApiException}, + * i.e. ApiExceptions will be ignored if the predicate matches. + * @param name the name of the {@link Retry} service. + * @param bdkRetryConfig the retry configuration to be used. + * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. + * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. + * @param ignoreApiException predicate on a thrown {@link ApiException} to know if exception should be ignored, + * which means no subsequent retry will be made and null value will be returned. + * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. + * If several predicates match, all corresponding consumers will be executed. + */ public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, Predicate retryOnExceptionPredicate, Predicate ignoreApiException, Map, ConsumerWithThrowable> recoveryStrategies) { @@ -32,8 +59,13 @@ public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWit this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); } + /** + * Executes the retry mechanism by calling the {@link RetryWithRecovery#supplier}, + * executing the potential matching recovery functions and retrying if needed. + * @return an object of param type T + * @throws Throwable + */ public T execute() throws Throwable { - log.debug("RetryWithRecovery::execute"); return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java index 27ba7f4b9..79bfb17d9 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java @@ -7,7 +7,7 @@ /** * Functional interface which supplies a T object and may throw an {@link ApiException}. - * @param the type returned by the suuplier. + * @param the type returned by the supplier. */ @FunctionalInterface @API(status = API.Status.INTERNAL) From 9193da1fc0cd58285ff6d2159eea7f67101f21fe Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 17 Sep 2020 18:19:11 +0200 Subject: [PATCH 06/17] Added unit tests for RetryWithRecovery class --- .../datafeed/impl/DatafeedServiceV1Test.java | 8 - .../bdk/core/util/RetryWithRecoveryTest.java | 218 ++++++++++++++++++ 2 files changed, 218 insertions(+), 8 deletions(-) create mode 100644 symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1Test.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1Test.java index dd3bf8966..5e811cad8 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1Test.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1Test.java @@ -273,14 +273,6 @@ void retrieveDatafeedIdFromEmptyFile(@TempDir Path tempDir) { assertFalse(datafeedId.isPresent()); } - @Test - void getRetryInstanceTest() { - Retry retry = this.datafeedService.getRetryInstance("Test retry"); - assertNotNull(retry); - assertEquals("Test retry", retry.getName()); - assertEquals(2, retry.getRetryConfig().getMaxAttempts()); - } - @Test void handleV4EventTest() { List events = new ArrayList<>(); diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java new file mode 100644 index 000000000..617d86020 --- /dev/null +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java @@ -0,0 +1,218 @@ +package com.symphony.bdk.core.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.inOrder; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; + +import com.symphony.bdk.core.api.invoker.ApiException; +import com.symphony.bdk.core.config.model.BdkRetryConfig; + +import org.junit.jupiter.api.Test; +import org.mockito.InOrder; + +import java.util.Collections; + +/** + * Test class for {@link RetryWithRecovery} + */ +class RetryWithRecoveryTest { + + //to be able to use Mockito mocks around lambdas. Otherwise, does not work, even with mockito-inline + private static class ConcreteSupplier implements SupplierWithApiException { + @Override + public String get() throws ApiException { + return ""; + } + } + + + private static class ConcreteConsumer implements ConsumerWithThrowable { + @Override + public void consume(ApiException e) throws Throwable { + return; + } + } + + BdkRetryConfig getRetryConfig() { + final BdkRetryConfig bdkRetryConfig = new BdkRetryConfig(); + bdkRetryConfig.setMultiplier(1); + bdkRetryConfig.setInitialIntervalMillis(10); + + return bdkRetryConfig; + } + + @Test + void testSupplierWithNoExceptionReturnsValue() throws Throwable { + String value = "string"; + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenReturn(value); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> false, + Collections.emptyMap()); + + assertEquals(value, r.execute()); + verify(supplier, times(1)).get(); + } + + @Test + void testSupplierWithExceptionShouldRetry() throws Throwable { + String value = "string"; + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()) + .thenThrow(new ApiException(400, "error")) + .thenReturn(value); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + (t) -> t instanceof ApiException && ((ApiException) t).isClientError(), + Collections.emptyMap()); + + assertEquals(value, r.execute()); + verify(supplier, times(2)).get(); + } + + @Test + void testSupplierWithExceptionAndNoRetryShouldFailWithException() throws Throwable { + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(400, "error")); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + (t) -> false, Collections.emptyMap()); + + assertThrows(ApiException.class, () -> r.execute()); + verify(supplier, times(1)).get(); + } + + @Test + void testMaxAttemptsReachedShouldFailWithException() throws ApiException { + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(400, "error")); + + final BdkRetryConfig retryConfig = getRetryConfig(); + + RetryWithRecovery r = new RetryWithRecovery<>("name", retryConfig, supplier, (t) -> true, + Collections.emptyMap()); + + assertThrows(ApiException.class, () -> r.execute()); + verify(supplier, times(retryConfig.getMaxAttempts())).get(); + } + + @Test + void testExceptionNotMatchingRetryPredicateShouldBeForwarded() throws ApiException { + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(400, "error")); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + (t) -> t instanceof ApiException && ((ApiException) t).isServerError(), + Collections.emptyMap()); + + assertThrows(ApiException.class, () -> r.execute()); + verify(supplier, times(1)).get(); + } + + @Test + void testIgnoredExceptionShouldReturnNull() throws Throwable { + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(400, "error")); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + (e) -> true, Collections.emptyMap()); + + assertNull(r.execute()); + verify(supplier, times(1)).get(); + } + + @Test + void testMatchingExceptionShouldTriggerRecoveryAndRetry() throws Throwable { + final String value = "string"; + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(400, "error")).thenReturn(value); + + ConcreteConsumer consumer = mock(ConcreteConsumer.class); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + Collections.singletonMap(e -> true, consumer)); + + assertEquals(value, r.execute()); + + InOrder inOrder = inOrder(supplier, consumer); + inOrder.verify(supplier).get(); + inOrder.verify(consumer).consume(any()); + inOrder.verify(supplier).get(); + verifyNoMoreInteractions(supplier, consumer); + } + + @Test + void testNonMatchingExceptionShouldNotTriggerRecoveryAndRetry() throws Throwable { + final String value = "string"; + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(new ApiException(500, "error")).thenReturn(value); + + ConcreteConsumer consumer = mock(ConcreteConsumer.class); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + Collections.singletonMap(e -> e.isClientError(), consumer)); + + assertEquals(value, r.execute()); + verify(supplier, times(2)).get(); + verifyNoInteractions(consumer); + } + + @Test + void testThrowableInRecoveryAndNotMatchingRetryPredicateShouldBeForwarded() throws Throwable { + final String value = "string"; + final ApiException error = new ApiException(400, "error"); + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(error).thenReturn(value); + + ConcreteConsumer consumer = mock(ConcreteConsumer.class); + doThrow(new IndexOutOfBoundsException()).when(consumer).consume(any()); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + (t) -> t instanceof ApiException, Collections.singletonMap(ApiException::isClientError, consumer)); + + assertThrows(IndexOutOfBoundsException.class, () -> r.execute()); + + InOrder inOrder = inOrder(supplier, consumer); + inOrder.verify(supplier).get(); + inOrder.verify(consumer).consume(eq(error)); + verifyNoMoreInteractions(supplier, consumer); + } + + @Test + void testThrowableInRecoveryAndMatchingRetryPredicateShouldLeadToRetry() throws Throwable { + final String value = "string"; + final ApiException error = new ApiException(400, "error"); + + SupplierWithApiException supplier = mock(ConcreteSupplier.class); + when(supplier.get()).thenThrow(error).thenReturn(value); + + ConcreteConsumer consumer = mock(ConcreteConsumer.class); + doThrow(new IndexOutOfBoundsException()).when(consumer).consume(any()); + + RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + (t) -> true, Collections.singletonMap(ApiException::isClientError, consumer)); + + assertEquals(value, r.execute()); + + InOrder inOrder = inOrder(supplier, consumer); + inOrder.verify(supplier).get(); + inOrder.verify(consumer).consume(eq(error)); + inOrder.verify(supplier).get(); + verifyNoMoreInteractions(supplier, consumer); + } +} From 3181687f7226bc125de2226556a996fb950b5ed9 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Fri, 18 Sep 2020 15:59:14 +0200 Subject: [PATCH 07/17] Simplified retry mechanism for DF --- .../bdk/core/service/MessageService.java | 2 +- .../datafeed/exception/NestedRetryException.java | 7 +++++++ .../datafeed/impl/AbstractDatafeedService.java | 15 ++++----------- .../service/datafeed/impl/DatafeedServiceV1.java | 10 +++++----- .../service/datafeed/impl/DatafeedServiceV2.java | 9 +++++---- .../{ => function}/ConsumerWithThrowable.java | 4 ++-- .../util/{ => function}/RetryWithRecovery.java | 6 ++++-- .../{ => function}/SupplierWithApiException.java | 2 +- .../{ => function}/RetryWithRecoveryTest.java | 16 +++++++--------- 9 files changed, 36 insertions(+), 35 deletions(-) create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/{ => function}/ConsumerWithThrowable.java (78%) rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/{ => function}/RetryWithRecovery.java (97%) rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/{ => function}/SupplierWithApiException.java (95%) rename symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/{ => function}/RetryWithRecoveryTest.java (95%) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java index 733ad4237..fdbc61098 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java @@ -1,7 +1,7 @@ package com.symphony.bdk.core.service; -import static com.symphony.bdk.core.util.SupplierWithApiException.callAndCatchApiException; +import static com.symphony.bdk.core.util.function.SupplierWithApiException.callAndCatchApiException; import com.symphony.bdk.core.api.invoker.util.ApiUtils; import com.symphony.bdk.core.service.pagination.PaginatedApi; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java new file mode 100644 index 000000000..6f2ca02f3 --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java @@ -0,0 +1,7 @@ +package com.symphony.bdk.core.service.datafeed.exception; + +public class NestedRetryException extends RuntimeException { + public NestedRetryException(String message, Throwable cause) { + super(message, cause); + } +} diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index d06eeb6c4..8645f5b14 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -4,21 +4,16 @@ import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; -import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.datafeed.DatafeedService; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; -import com.symphony.bdk.core.util.BdkExponentialFunction; -import com.symphony.bdk.core.util.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; -import io.github.resilience4j.retry.Retry; -import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.function.Predicate; @@ -85,17 +80,15 @@ protected void handleV4EventList(List events) { } private boolean isSelfGeneratedEvent(V4Event event) { - return event.getInitiator() != null && event.getInitiator().getUser() != null - && event.getInitiator().getUser().getUsername() != null - && event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); + return event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); } protected Map, ConsumerWithThrowable> getSessionRefreshStrategy() { - return Collections.singletonMap(ApiException::isUnauthorized, e -> refresh()); + return Collections.singletonMap(ApiException::isUnauthorized, this::refresh); } protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { - if (t instanceof ApiException && t.getSuppressed().length == 0) { + if (t instanceof ApiException) { ApiException apiException = (ApiException) t; return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 49210604f..f1d4c65ff 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -5,14 +5,14 @@ import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; -import com.symphony.bdk.core.util.RetryWithRecovery; -import com.symphony.bdk.core.util.ConsumerWithThrowable; +import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; +import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; import lombok.extern.slf4j.Slf4j; -import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -112,12 +112,12 @@ private Void readAndHandleEvents() throws ApiException { return null; } - private void recreateDatafeed(ApiException e) { + private void recreateDatafeed() { log.info("Recreate a new datafeed and try again"); try { datafeedId = createDatafeed(); } catch (Throwable throwable) { - e.addSuppressed(throwable); + throw new NestedRetryException("Recreation of datafeed failed", throwable); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index 6d4e44bb9..84331a306 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -4,8 +4,9 @@ import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; -import com.symphony.bdk.core.util.ConsumerWithThrowable; -import com.symphony.bdk.core.util.RetryWithRecovery; +import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.RetryWithRecovery; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.AckId; import com.symphony.bdk.gen.api.model.V4Event; @@ -153,14 +154,14 @@ private Void readAndHandleEvents() throws ApiException { return null; } - private void recreateDatafeed(ApiException e) { + private void recreateDatafeed() { try { log.info("Try to delete the faulty datafeed"); this.deleteDatafeed(); log.info("Recreate a new datafeed and try again"); this.datafeed = this.createDatafeed(); } catch (Throwable throwable) { - e.addSuppressed(throwable); + throw new NestedRetryException("Recreation of datafeed failed", throwable); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java similarity index 78% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java index c1684553c..b1080f74d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/ConsumerWithThrowable.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java @@ -1,4 +1,4 @@ -package com.symphony.bdk.core.util; +package com.symphony.bdk.core.util.function; import com.symphony.bdk.core.api.invoker.ApiException; @@ -8,5 +8,5 @@ */ @FunctionalInterface public interface ConsumerWithThrowable { - void consume(ApiException e) throws Throwable; + void consume() throws Throwable; } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java similarity index 97% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java index f30bfd98a..ca5010265 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java @@ -1,8 +1,10 @@ -package com.symphony.bdk.core.util; +package com.symphony.bdk.core.util.function; import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.BdkExponentialFunction; + import io.github.resilience4j.retry.Retry; import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; @@ -108,7 +110,7 @@ private void handleRecovery(ApiException e) throws Throwable { for (Map.Entry, ConsumerWithThrowable> entry : recoveryStrategies.entrySet()) { if (entry.getKey().test(e)) { recoveryTriggered = true; - entry.getValue().consume(e); + entry.getValue().consume(); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java similarity index 95% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java index 79bfb17d9..575cc60f2 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/SupplierWithApiException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java @@ -1,4 +1,4 @@ -package com.symphony.bdk.core.util; +package com.symphony.bdk.core.util.function; import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java similarity index 95% rename from symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java rename to symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java index 617d86020..59e4f27e3 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/RetryWithRecoveryTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java @@ -1,11 +1,9 @@ -package com.symphony.bdk.core.util; +package com.symphony.bdk.core.util.function; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doThrow; -import static org.mockito.Mockito.eq; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; @@ -38,7 +36,7 @@ public String get() throws ApiException { private static class ConcreteConsumer implements ConsumerWithThrowable { @Override - public void consume(ApiException e) throws Throwable { + public void consume() throws Throwable { return; } } @@ -149,7 +147,7 @@ void testMatchingExceptionShouldTriggerRecoveryAndRetry() throws Throwable { InOrder inOrder = inOrder(supplier, consumer); inOrder.verify(supplier).get(); - inOrder.verify(consumer).consume(any()); + inOrder.verify(consumer).consume(); inOrder.verify(supplier).get(); verifyNoMoreInteractions(supplier, consumer); } @@ -180,7 +178,7 @@ void testThrowableInRecoveryAndNotMatchingRetryPredicateShouldBeForwarded() thro when(supplier.get()).thenThrow(error).thenReturn(value); ConcreteConsumer consumer = mock(ConcreteConsumer.class); - doThrow(new IndexOutOfBoundsException()).when(consumer).consume(any()); + doThrow(new IndexOutOfBoundsException()).when(consumer).consume(); RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> t instanceof ApiException, Collections.singletonMap(ApiException::isClientError, consumer)); @@ -189,7 +187,7 @@ void testThrowableInRecoveryAndNotMatchingRetryPredicateShouldBeForwarded() thro InOrder inOrder = inOrder(supplier, consumer); inOrder.verify(supplier).get(); - inOrder.verify(consumer).consume(eq(error)); + inOrder.verify(consumer).consume(); verifyNoMoreInteractions(supplier, consumer); } @@ -202,7 +200,7 @@ void testThrowableInRecoveryAndMatchingRetryPredicateShouldLeadToRetry() throws when(supplier.get()).thenThrow(error).thenReturn(value); ConcreteConsumer consumer = mock(ConcreteConsumer.class); - doThrow(new IndexOutOfBoundsException()).when(consumer).consume(any()); + doThrow(new IndexOutOfBoundsException()).when(consumer).consume(); RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, Collections.singletonMap(ApiException::isClientError, consumer)); @@ -211,7 +209,7 @@ void testThrowableInRecoveryAndMatchingRetryPredicateShouldLeadToRetry() throws InOrder inOrder = inOrder(supplier, consumer); inOrder.verify(supplier).get(); - inOrder.verify(consumer).consume(eq(error)); + inOrder.verify(consumer).consume(); inOrder.verify(supplier).get(); verifyNoMoreInteractions(supplier, consumer); } From 8a201410565e81f8896f080e53480973c71f6db3 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Fri, 18 Sep 2020 16:34:49 +0200 Subject: [PATCH 08/17] Removed retry instantiations in DF constructors --- .../bdk/core/api/invoker/ApiException.java | 11 ++++++ .../impl/AbstractDatafeedService.java | 4 +- .../datafeed/impl/DatafeedServiceV1.java | 28 ++++++------- .../datafeed/impl/DatafeedServiceV2.java | 39 ++++++------------- 4 files changed, 39 insertions(+), 43 deletions(-) diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java index cbf010ce5..6f6f94e21 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java @@ -3,8 +3,10 @@ import lombok.Getter; import org.apiguardian.api.API; +import javax.net.ssl.HttpsURLConnection; import javax.ws.rs.core.GenericType; import java.net.HttpURLConnection; +import java.util.Arrays; import java.util.List; import java.util.Map; @@ -82,4 +84,13 @@ public boolean isClientError() { public boolean isServerError() { return this.code >= HttpURLConnection.HTTP_INTERNAL_ERROR; } + + /** + * Check if response is Bad Gateway, Service Unavailable or Gateway Timeout + * + * @return true if response status is 502, 503 or 504 + */ + public boolean isTemporaryServerError() { + return this.code >= HttpsURLConnection.HTTP_BAD_GATEWAY && this.code <= HttpsURLConnection.HTTP_GATEWAY_TIMEOUT; + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index 8645f5b14..a0aacda30 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -90,7 +90,7 @@ protected Map, ConsumerWithThrowable> getSessionRefreshS protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); + return apiException.isTemporaryServerError() || apiException.isUnauthorized() || apiException.isClientError(); } return t instanceof ProcessingException; } @@ -98,7 +98,7 @@ protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { protected boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized(); + return apiException.isTemporaryServerError() || apiException.isUnauthorized(); } return t instanceof ProcessingException; } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index f1d4c65ff..08be14070 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -45,8 +45,6 @@ public class DatafeedServiceV1 extends AbstractDatafeedService { private final AtomicBoolean started = new AtomicBoolean(); private final DatafeedIdRepository datafeedRepository; - private final RetryWithRecovery readDatafeedRetry; - private final RetryWithRecovery createDatafeedRetry; private String datafeedId; public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { @@ -59,15 +57,6 @@ public DatafeedServiceV1(DatafeedApi datafeedApi, AuthSession authSession, BdkCo this.started.set(false); this.datafeedId = null; this.datafeedRepository = repository; - - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); - readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - - this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), - this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy); - - this.createDatafeedRetry = new RetryWithRecovery<>("Create Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), - this::createDatafeedAndPersist, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); } /** @@ -85,7 +74,7 @@ public void start() throws AuthUnauthorizedException, ApiException { log.debug("Start reading events from datafeed {}", datafeedId); this.started.set(true); do { - readDatafeedRetry.execute(); + readDatafeed(); } while (this.started.get()); } catch (AuthUnauthorizedException | ApiException exception) { throw exception; @@ -105,13 +94,22 @@ public void stop() { private Void readAndHandleEvents() throws ApiException { List events = - datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), null); + datafeedApi.v4DatafeedIdReadGet(datafeedId, authSession.getSessionToken(), authSession.getKeyManagerToken(), + null); if (events != null && !events.isEmpty()) { handleV4EventList(events); } return null; } + private void readDatafeed() throws Throwable { + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); + readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); + + new RetryWithRecovery<>("Read Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), + this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy).execute(); + } + private void recreateDatafeed() { log.info("Recreate a new datafeed and try again"); try { @@ -123,7 +121,9 @@ private void recreateDatafeed() { protected String createDatafeed() throws Throwable { log.debug("Start creating a new datafeed and persisting it"); - return createDatafeedRetry.execute(); + return new RetryWithRecovery<>("Create Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), + this::createDatafeedAndPersist, this::isNetworkOrServerOrUnauthorizedError, + getSessionRefreshStrategy()).execute(); } private String createDatafeedAndPersist() throws ApiException { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index 84331a306..3cf1552de 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -45,33 +45,10 @@ public class DatafeedServiceV2 extends AbstractDatafeedService { private final AtomicBoolean started = new AtomicBoolean(); private final AckId ackId; private V5Datafeed datafeed; - private final RetryWithRecovery retrieveDatafeedRetry; - private final RetryWithRecovery deleteDatafeedRetry; - private final RetryWithRecovery createDatafeedRetry; - private final RetryWithRecovery readDatafeedRetry; - public DatafeedServiceV2(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { super(datafeedApi, authSession, config); this.ackId = new AckId().ackId(""); - - this.retrieveDatafeedRetry = - new RetryWithRecovery<>("Retrieve Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::tryRetrieveDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); - - this.deleteDatafeedRetry = - new RetryWithRecovery<>("Delete Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::tryDeleteDatafeed, this::isNetworkOrServerOrUnauthorizedError, ApiException::isClientError, getSessionRefreshStrategy()); - - this.createDatafeedRetry = - new RetryWithRecovery<>("Create Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::tryCreateDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()); - - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); - readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - - this.readDatafeedRetry = new RetryWithRecovery<>("Read Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy); } /** @@ -113,7 +90,8 @@ public void stop() { private V5Datafeed createDatafeed() throws Throwable { log.debug("Start creating datafeed from agent"); - return createDatafeedRetry.execute(); + return new RetryWithRecovery<>("Create Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::tryCreateDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()).execute(); } private V5Datafeed tryCreateDatafeed() throws ApiException { @@ -122,7 +100,8 @@ private V5Datafeed tryCreateDatafeed() throws ApiException { private V5Datafeed retrieveDatafeed() throws Throwable { log.debug("Start retrieving datafeed from agent"); - return retrieveDatafeedRetry.execute(); + return new RetryWithRecovery<>("Retrieve Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::tryRetrieveDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()).execute(); } private V5Datafeed tryRetrieveDatafeed() throws ApiException { @@ -137,7 +116,11 @@ private V5Datafeed tryRetrieveDatafeed() throws ApiException { private void readDatafeed() throws Throwable { log.debug("Reading datafeed events from datafeed {}", datafeed.getId()); - this.readDatafeedRetry.execute(); + Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); + readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); + + new RetryWithRecovery<>("Read Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), + this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy).execute(); } private Void readAndHandleEvents() throws ApiException { @@ -167,7 +150,9 @@ private void recreateDatafeed() { private void deleteDatafeed() throws Throwable { log.debug("Start deleting a faulty datafeed"); - deleteDatafeedRetry.execute(); + new RetryWithRecovery<>("Delete Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), this::tryDeleteDatafeed, + this::isNetworkOrServerOrUnauthorizedError, ApiException::isClientError, getSessionRefreshStrategy()) + .execute(); } private Void tryDeleteDatafeed() throws ApiException { From 10965e9698bfc4c576921315993233bf920cdf43 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Fri, 18 Sep 2020 18:52:01 +0200 Subject: [PATCH 09/17] Added RetryBuilder --- .../bdk/core/api/invoker/ApiException.java | 15 +--- .../impl/AbstractDatafeedService.java | 17 ++-- .../datafeed/impl/DatafeedServiceV1.java | 25 ++++-- .../datafeed/impl/DatafeedServiceV2.java | 48 +++++++--- .../Resilience4jRetryWithRecovery.java | 88 +++++++++++++++++++ .../core/util/function/RetryWithRecovery.java | 81 +++-------------- .../function/RetryWithRecoveryBuilder.java | 68 ++++++++++++++ .../datafeed/impl/DatafeedServiceV2Test.java | 15 ++-- ...=> Resilience4jRetryWithRecoveryTest.java} | 24 ++--- 9 files changed, 252 insertions(+), 129 deletions(-) create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java create mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java rename symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/{RetryWithRecoveryTest.java => Resilience4jRetryWithRecoveryTest.java} (83%) diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java index 6f6f94e21..41429ed6e 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java @@ -77,20 +77,11 @@ public boolean isClientError() { } /** - * Check if response status is server error or not + * Check if response status is server error and not an internal server error * - * @return true if response status equals or greater than 500, false otherwise + * @return true if response status strictly greater than 500, false otherwise */ public boolean isServerError() { - return this.code >= HttpURLConnection.HTTP_INTERNAL_ERROR; - } - - /** - * Check if response is Bad Gateway, Service Unavailable or Gateway Timeout - * - * @return true if response status is 502, 503 or 504 - */ - public boolean isTemporaryServerError() { - return this.code >= HttpsURLConnection.HTTP_BAD_GATEWAY && this.code <= HttpsURLConnection.HTTP_GATEWAY_TIMEOUT; + return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index a0aacda30..81bc798bf 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -7,6 +7,7 @@ import com.symphony.bdk.core.service.datafeed.DatafeedService; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; @@ -30,6 +31,7 @@ abstract class AbstractDatafeedService implements DatafeedService { protected final AuthSession authSession; protected final BdkConfig bdkConfig; protected final List listeners; + protected final RetryWithRecoveryBuilder retryWithRecoveryBuilder; protected DatafeedApi datafeedApi; public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, BdkConfig config) { @@ -37,6 +39,9 @@ public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, this.listeners = new ArrayList<>(); this.authSession = authSession; this.bdkConfig = config; + this.retryWithRecoveryBuilder = new RetryWithRecoveryBuilder<>() + .retryConfig(config.getDatafeedRetryConfig()) + .recoveryStrategy(ApiException::isUnauthorized, this::refresh); } /** @@ -80,17 +85,15 @@ protected void handleV4EventList(List events) { } private boolean isSelfGeneratedEvent(V4Event event) { - return event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); - } - - protected Map, ConsumerWithThrowable> getSessionRefreshStrategy() { - return Collections.singletonMap(ApiException::isUnauthorized, this::refresh); + return event.getInitiator() != null && event.getInitiator().getUser() != null + && event.getInitiator().getUser().getUsername() != null + && event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); } protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isTemporaryServerError() || apiException.isUnauthorized() || apiException.isClientError(); + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); } return t instanceof ProcessingException; } @@ -98,7 +101,7 @@ protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { protected boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isTemporaryServerError() || apiException.isUnauthorized(); + return apiException.isServerError() || apiException.isUnauthorized(); } return t instanceof ProcessingException; } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 08be14070..ff1b3d405 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -6,8 +6,10 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; -import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.Resilience4jRetryWithRecovery; import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; @@ -103,11 +105,13 @@ private Void readAndHandleEvents() throws ApiException { } private void readDatafeed() throws Throwable { - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); - readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - - new RetryWithRecovery<>("Read Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), - this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy).execute(); + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Read Datafeed V1") + .supplier(this::readAndHandleEvents) + .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) + .retryOnException(this::isNetworkOrServerOrUnauthorizedOrClientError) + .build(); + retry.execute(); } private void recreateDatafeed() { @@ -121,9 +125,12 @@ private void recreateDatafeed() { protected String createDatafeed() throws Throwable { log.debug("Start creating a new datafeed and persisting it"); - return new RetryWithRecovery<>("Create Datafeed V1", this.bdkConfig.getDatafeedRetryConfig(), - this::createDatafeedAndPersist, this::isNetworkOrServerOrUnauthorizedError, - getSessionRefreshStrategy()).execute(); + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Create Datafeed V1") + .supplier(this::createDatafeedAndPersist) + .retryOnException(this::isNetworkOrServerOrUnauthorizedError) + .build(); + return retry.execute(); } private String createDatafeedAndPersist() throws ApiException { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index 3cf1552de..03b519d23 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -6,7 +6,9 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.Resilience4jRetryWithRecovery; import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.AckId; import com.symphony.bdk.gen.api.model.V4Event; @@ -69,7 +71,7 @@ public void start() throws ApiException, AuthUnauthorizedException { do { this.readDatafeed(); } while (this.started.get()); - } catch (AuthUnauthorizedException | ApiException exception) { + } catch (AuthUnauthorizedException | ApiException | NestedRetryException exception) { throw exception; } catch (Throwable throwable) { log.error("Unknown error", throwable); @@ -90,8 +92,14 @@ public void stop() { private V5Datafeed createDatafeed() throws Throwable { log.debug("Start creating datafeed from agent"); - return new RetryWithRecovery<>("Create Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::tryCreateDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()).execute(); + + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Create Datafeed V2") + .supplier(this::tryCreateDatafeed) + .retryOnException(this::isNetworkOrServerOrUnauthorizedError) + .build(); + + return retry.execute(); } private V5Datafeed tryCreateDatafeed() throws ApiException { @@ -100,8 +108,14 @@ private V5Datafeed tryCreateDatafeed() throws ApiException { private V5Datafeed retrieveDatafeed() throws Throwable { log.debug("Start retrieving datafeed from agent"); - return new RetryWithRecovery<>("Retrieve Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::tryRetrieveDatafeed, this::isNetworkOrServerOrUnauthorizedError, getSessionRefreshStrategy()).execute(); + + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Retrieve Datafeed V2") + .supplier(this::tryRetrieveDatafeed) + .retryOnException(this::isNetworkOrServerOrUnauthorizedError) + .build(); + + return retry.execute(); } private V5Datafeed tryRetrieveDatafeed() throws ApiException { @@ -116,11 +130,15 @@ private V5Datafeed tryRetrieveDatafeed() throws ApiException { private void readDatafeed() throws Throwable { log.debug("Reading datafeed events from datafeed {}", datafeed.getId()); - Map, ConsumerWithThrowable> readRecoveryStrategy = new HashMap<>(getSessionRefreshStrategy()); - readRecoveryStrategy.put(ApiException::isClientError, this::recreateDatafeed); - new RetryWithRecovery<>("Read Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), - this::readAndHandleEvents, this::isNetworkOrServerOrUnauthorizedOrClientError, readRecoveryStrategy).execute(); + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Read Datafeed V2") + .supplier(this::readAndHandleEvents) + .retryOnException(this::isNetworkOrServerOrUnauthorizedOrClientError) + .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) + .build(); + + retry.execute(); } private Void readAndHandleEvents() throws ApiException { @@ -150,9 +168,15 @@ private void recreateDatafeed() { private void deleteDatafeed() throws Throwable { log.debug("Start deleting a faulty datafeed"); - new RetryWithRecovery<>("Delete Datafeed V2", this.bdkConfig.getDatafeedRetryConfig(), this::tryDeleteDatafeed, - this::isNetworkOrServerOrUnauthorizedError, ApiException::isClientError, getSessionRefreshStrategy()) - .execute(); + + final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) + .name("Delete Datafeed V2") + .supplier(this::tryDeleteDatafeed) + .retryOnException(this::isNetworkOrServerOrUnauthorizedError) + .ignoreException(ApiException::isClientError) + .build(); + + retry.execute(); } private Void tryDeleteDatafeed() throws ApiException { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java new file mode 100644 index 000000000..fe704288e --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java @@ -0,0 +1,88 @@ +package com.symphony.bdk.core.util.function; + +import com.symphony.bdk.core.api.invoker.ApiException; +import com.symphony.bdk.core.config.model.BdkRetryConfig; + +import com.symphony.bdk.core.util.BdkExponentialFunction; + +import io.github.resilience4j.retry.Retry; +import io.github.resilience4j.retry.RetryConfig; +import lombok.extern.slf4j.Slf4j; + +import java.util.Map; +import java.util.function.Predicate; + +/** + * This class aims to implement a retry mechanism (on top of a{@link Retry}) + * with different recovery strategies based on predicates. + * @param the type of the object returned by {@link #execute()} + */ +@Slf4j +public class Resilience4jRetryWithRecovery extends RetryWithRecovery { + private final Retry retry; + + /** + * Constructor with no predicate on when to ignore an {@link ApiException}, + * i.e. ApiExceptions will never be ignored. + * @param name the name of the {@link Retry} service. + * @param bdkRetryConfig the retry configuration to be used. + * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. + * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. + * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. + * If several predicates match, all corresponding consumers will be executed. + */ + public Resilience4jRetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, + Predicate retryOnExceptionPredicate, + Map, ConsumerWithThrowable> recoveryStrategies) { + this(name, bdkRetryConfig, supplier, retryOnExceptionPredicate, (e) -> false, recoveryStrategies); + } + + /** + * Constructor with a predicate on when to ignore an {@link ApiException}, + * i.e. ApiExceptions will be ignored if the predicate matches. + * @param name the name of the {@link Retry} service. + * @param bdkRetryConfig the retry configuration to be used. + * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. + * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. + * @param ignoreApiException predicate on a thrown {@link ApiException} to know if exception should be ignored, + * which means no subsequent retry will be made and null value will be returned. + * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. + * If several predicates match, all corresponding consumers will be executed. + */ + public Resilience4jRetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, + Predicate retryOnExceptionPredicate, Predicate ignoreApiException, + Map, ConsumerWithThrowable> recoveryStrategies) { + super(supplier, ignoreApiException, recoveryStrategies); + this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); + } + + /** + * Executes the retry mechanism by calling the provided supplier, + * executing the potential matching recovery functions and retrying if needed. + * @return an object of param type T + * @throws Throwable + */ + public T execute() throws Throwable { + return this.retry.executeCheckedSupplier(this::executeOnce); + } + + private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, + Predicate retryOnExceptionPredicate) { + RetryConfig retryConfig = RetryConfig.custom() + .maxAttempts(bdkRetryConfig.getMaxAttempts()) + .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) + .retryOnException(retryOnExceptionPredicate) + .build(); + + Retry retry = Retry.of(name, retryConfig); + retry.getEventPublisher().onRetry(event -> { + double interval = event.getWaitInterval().toMillis() / 1000.0; + if (event.getLastThrowable() != null) { + log.debug("{} service failed due to {}", name, event.getLastThrowable().getMessage()); + } + log.info("Retry in {}s...", interval); + }); + + return retry; + } +} diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java index ca5010265..babc2cce7 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java @@ -1,101 +1,41 @@ package com.symphony.bdk.core.util.function; import com.symphony.bdk.core.api.invoker.ApiException; -import com.symphony.bdk.core.config.model.BdkRetryConfig; -import com.symphony.bdk.core.util.BdkExponentialFunction; - -import io.github.resilience4j.retry.Retry; -import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; import java.util.Map; import java.util.function.Predicate; -/** - * This class aims to implement a retry mechanism (on top of a{@link Retry}) - * with different recovery strategies based on predicates. - * @param the type of the object returned by {@link #execute()} - */ @Slf4j -public class RetryWithRecovery { +public abstract class RetryWithRecovery { private SupplierWithApiException supplier; private Predicate ignoreApiException; private Map, ConsumerWithThrowable> recoveryStrategies; - private Retry retry; - /** - * Constructor with no predicate on when to ignore an {@link ApiException}, - * i.e. ApiExceptions will never be ignored. - * @param name the name of the {@link Retry} service. - * @param bdkRetryConfig the retry configuration to be used. - * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. - * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. - * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. - * If several predicates match, all corresponding consumers will be executed. - */ - public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, - Predicate retryOnExceptionPredicate, - Map, ConsumerWithThrowable> recoveryStrategies) { - this(name, bdkRetryConfig, supplier, retryOnExceptionPredicate, (e) -> false, recoveryStrategies); - } - - /** - * Constructor with a predicate on when to ignore an {@link ApiException}, - * i.e. ApiExceptions will be ignored if the predicate matches. - * @param name the name of the {@link Retry} service. - * @param bdkRetryConfig the retry configuration to be used. - * @param supplier the supplier responsible to provide the object of param type T and which may throw an {@link ApiException}. - * @param retryOnExceptionPredicate predicate on a thrown {@link ApiException} to know if call should be retried. - * @param ignoreApiException predicate on a thrown {@link ApiException} to know if exception should be ignored, - * which means no subsequent retry will be made and null value will be returned. - * @param recoveryStrategies mapping between {@link Predicate} and the corresponding recovery functions to be executed before retrying. - * If several predicates match, all corresponding consumers will be executed. - */ - public RetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, SupplierWithApiException supplier, - Predicate retryOnExceptionPredicate, Predicate ignoreApiException, + public RetryWithRecovery(SupplierWithApiException supplier, Predicate ignoreApiException, Map, ConsumerWithThrowable> recoveryStrategies) { this.supplier = supplier; this.ignoreApiException = ignoreApiException; this.recoveryStrategies = recoveryStrategies; - this.retry = createRetry(name, bdkRetryConfig, retryOnExceptionPredicate); } /** - * Executes the retry mechanism by calling the {@link RetryWithRecovery#supplier}, - * executing the potential matching recovery functions and retrying if needed. - * @return an object of param type T + * Method called by client which should implement the retry. + * This should call {@link #executeOnce()} which executes one actual call to the supplier, runs potential recovery + * actions and potentially throws an exception. + * + * @return * @throws Throwable */ - public T execute() throws Throwable { - return this.retry.executeCheckedSupplier(this::executeMainAndRecoveryStrategies); - } - - private Retry createRetry(String name, BdkRetryConfig bdkRetryConfig, - Predicate retryOnExceptionPredicate) { - RetryConfig retryConfig = RetryConfig.custom() - .maxAttempts(bdkRetryConfig.getMaxAttempts()) - .intervalFunction(BdkExponentialFunction.ofExponentialBackoff(bdkRetryConfig)) - .retryOnException(retryOnExceptionPredicate) - .build(); - - Retry retry = Retry.of(name, retryConfig); - retry.getEventPublisher().onRetry(event -> { - double interval = event.getWaitInterval().toMillis() / 1000.0; - if (event.getLastThrowable() != null) { - log.debug("{} service failed due to {}", name, event.getLastThrowable().getMessage()); - } - log.info("Retry in {}s...", interval); - }); - - return retry; - } + public abstract T execute() throws Throwable; - private T executeMainAndRecoveryStrategies() throws Throwable { + protected T executeOnce() throws Throwable { try { return supplier.get(); } catch (ApiException e) { if (ignoreApiException.test(e)) { + log.debug("Exception ignored: {}", e); return null; } @@ -109,6 +49,7 @@ private void handleRecovery(ApiException e) throws Throwable { for (Map.Entry, ConsumerWithThrowable> entry : recoveryStrategies.entrySet()) { if (entry.getKey().test(e)) { + log.debug("Exception recovered: {}", e); recoveryTriggered = true; entry.getValue().consume(); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java new file mode 100644 index 000000000..decf26cbf --- /dev/null +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java @@ -0,0 +1,68 @@ +package com.symphony.bdk.core.util.function; + +import com.symphony.bdk.core.api.invoker.ApiException; +import com.symphony.bdk.core.config.model.BdkRetryConfig; + +import java.util.HashMap; +import java.util.Map; +import java.util.function.Predicate; + +public class RetryWithRecoveryBuilder { + private String name; + private BdkRetryConfig retryConfig; + private SupplierWithApiException supplier; + private Predicate retryOnExceptionPredicate; + private Predicate ignoreException; + private Map, ConsumerWithThrowable> recoveryStrategies; + + public static RetryWithRecoveryBuilder from(RetryWithRecoveryBuilder from) { + RetryWithRecoveryBuilder copy = new RetryWithRecoveryBuilder(); + copy.name = from.name; + copy.retryConfig = from.retryConfig; + copy.retryOnExceptionPredicate = from.retryOnExceptionPredicate; + copy.ignoreException = from.ignoreException; + copy.recoveryStrategies = new HashMap<>(from.recoveryStrategies); + + return copy; + } + + public RetryWithRecoveryBuilder() { + this.ignoreException = (e) -> false; + this.recoveryStrategies = new HashMap<>(); + } + + public RetryWithRecoveryBuilder name(String name) { + this.name = name; + return this; + } + + public RetryWithRecoveryBuilder retryConfig(BdkRetryConfig retryConfig) { + this.retryConfig = retryConfig; + return this; + } + + public RetryWithRecoveryBuilder supplier(SupplierWithApiException supplier) { + this.supplier = supplier; + return this; + } + + public RetryWithRecoveryBuilder retryOnException(Predicate retryOnExceptionPredicate) { + this.retryOnExceptionPredicate = retryOnExceptionPredicate; + return this; + } + + public RetryWithRecoveryBuilder ignoreException(Predicate ignoreException) { + this.ignoreException = ignoreException; + return this; + } + + public RetryWithRecoveryBuilder recoveryStrategy(Predicate condition, ConsumerWithThrowable recovery) { + this.recoveryStrategies.put(condition, recovery); + return this; + } + + public RetryWithRecovery build() { + return new Resilience4jRetryWithRecovery<>(name, retryConfig, supplier, retryOnExceptionPredicate, ignoreException, + recoveryStrategies); + } +} diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java index e9a8f28bf..82845d640 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java @@ -18,6 +18,7 @@ import com.symphony.bdk.core.config.model.BdkDatafeedConfig; import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; +import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.AckId; import com.symphony.bdk.gen.api.model.V4Event; @@ -161,7 +162,7 @@ void testStartAuthRefreshListDatafeed() throws ApiException, AuthUnauthorizedExc @Test void testStartServerErrorListDatafeed() throws ApiException { - when(datafeedApi.listDatafeed("1234", "1234")).thenThrow(new ApiException(500, "server-error")); + when(datafeedApi.listDatafeed("1234", "1234")).thenThrow(new ApiException(502, "server-error")); assertThrows(ApiException.class, this.datafeedService::start); verify(datafeedApi, times(2)).listDatafeed("1234", "1234"); @@ -172,7 +173,7 @@ void testStartErrorListDatafeedThenRetrySuccess() throws ApiException, AuthUnaut AtomicInteger count = new AtomicInteger(0); when(datafeedApi.listDatafeed("1234", "1234")).thenAnswer(invocationOnMock -> { if (count.getAndIncrement() == 0) { - throw new ApiException(500, "server-error"); + throw new ApiException(502, "server-error"); } else { List datafeeds = new ArrayList<>(); datafeeds.add(new V5Datafeed().id("test-id")); @@ -213,7 +214,7 @@ void testStartClientErrorCreateDatafeed() throws ApiException { @Test void testStartServerErrorCreateDatafeed() throws ApiException { when(datafeedApi.listDatafeed("1234", "1234")).thenReturn(Collections.emptyList()); - when(datafeedApi.createDatafeed("1234", "1234")).thenThrow(new ApiException(500, "server-error")); + when(datafeedApi.createDatafeed("1234", "1234")).thenThrow(new ApiException(502, "server-error")); assertThrows(ApiException.class, this.datafeedService::start); verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); @@ -265,7 +266,7 @@ void testStartAuthErrorReadDatafeed() throws ApiException, AuthUnauthorizedExcep void testStartServerErrorReadDatafeed() throws ApiException { AckId ackId = datafeedService.getAckId(); when(datafeedApi.listDatafeed("1234", "1234")).thenReturn(Collections.singletonList(new V5Datafeed().id("test-id"))); - when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(500, "client-error")); + when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(502, "client-error")); assertThrows(ApiException.class, this.datafeedService::start); verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); @@ -297,9 +298,9 @@ void testStartServerErrorDeleteDatafeed() throws ApiException { when(datafeedApi.listDatafeed("1234", "1234")).thenReturn(Collections.singletonList(new V5Datafeed().id("test-id"))); when(datafeedApi.createDatafeed("1234", "1234")).thenReturn(new V5Datafeed().id("recreate-df-id")); when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(400, "client-error")); - when(datafeedApi.deleteDatafeed("test-id", "1234", "1234")).thenThrow(new ApiException(500, "client-error")); + when(datafeedApi.deleteDatafeed("test-id", "1234", "1234")).thenThrow(new ApiException(502, "client-error")); - assertThrows(ApiException.class, this.datafeedService::start); + assertThrows(NestedRetryException.class, this.datafeedService::start); verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); verify(datafeedApi, times(1)).readDatafeed("test-id", "1234", "1234", ackId); verify(datafeedApi, times(2)).deleteDatafeed("test-id", "1234", "1234"); @@ -314,7 +315,7 @@ void testStartAuthErrorDeleteDatafeed() throws ApiException, AuthUnauthorizedExc when(datafeedApi.deleteDatafeed("test-id", "1234", "1234")).thenThrow(new ApiException(401, "client-error")); doThrow(AuthUnauthorizedException.class).when(authSession).refresh(); - assertThrows(ApiException.class, this.datafeedService::start); + assertThrows(NestedRetryException.class, this.datafeedService::start); verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); verify(datafeedApi, times(1)).readDatafeed("test-id", "1234", "1234", ackId); verify(datafeedApi, times(1)).deleteDatafeed("test-id", "1234", "1234"); diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java similarity index 83% rename from symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java rename to symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java index 59e4f27e3..cd8e2a616 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/RetryWithRecoveryTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java @@ -21,9 +21,9 @@ import java.util.Collections; /** - * Test class for {@link RetryWithRecovery} + * Test class for {@link Resilience4jRetryWithRecovery} */ -class RetryWithRecoveryTest { +class Resilience4jRetryWithRecoveryTest { //to be able to use Mockito mocks around lambdas. Otherwise, does not work, even with mockito-inline private static class ConcreteSupplier implements SupplierWithApiException { @@ -56,7 +56,7 @@ void testSupplierWithNoExceptionReturnsValue() throws Throwable { SupplierWithApiException supplier = mock(ConcreteSupplier.class); when(supplier.get()).thenReturn(value); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> false, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> false, Collections.emptyMap()); assertEquals(value, r.execute()); @@ -72,7 +72,7 @@ void testSupplierWithExceptionShouldRetry() throws Throwable { .thenThrow(new ApiException(400, "error")) .thenReturn(value); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> t instanceof ApiException && ((ApiException) t).isClientError(), Collections.emptyMap()); @@ -85,7 +85,7 @@ void testSupplierWithExceptionAndNoRetryShouldFailWithException() throws Throwab SupplierWithApiException supplier = mock(ConcreteSupplier.class); when(supplier.get()).thenThrow(new ApiException(400, "error")); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> false, Collections.emptyMap()); assertThrows(ApiException.class, () -> r.execute()); @@ -99,7 +99,7 @@ void testMaxAttemptsReachedShouldFailWithException() throws ApiException { final BdkRetryConfig retryConfig = getRetryConfig(); - RetryWithRecovery r = new RetryWithRecovery<>("name", retryConfig, supplier, (t) -> true, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", retryConfig, supplier, (t) -> true, Collections.emptyMap()); assertThrows(ApiException.class, () -> r.execute()); @@ -111,7 +111,7 @@ void testExceptionNotMatchingRetryPredicateShouldBeForwarded() throws ApiExcepti SupplierWithApiException supplier = mock(ConcreteSupplier.class); when(supplier.get()).thenThrow(new ApiException(400, "error")); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> t instanceof ApiException && ((ApiException) t).isServerError(), Collections.emptyMap()); @@ -124,7 +124,7 @@ void testIgnoredExceptionShouldReturnNull() throws Throwable { SupplierWithApiException supplier = mock(ConcreteSupplier.class); when(supplier.get()).thenThrow(new ApiException(400, "error")); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, (e) -> true, Collections.emptyMap()); assertNull(r.execute()); @@ -140,7 +140,7 @@ void testMatchingExceptionShouldTriggerRecoveryAndRetry() throws Throwable { ConcreteConsumer consumer = mock(ConcreteConsumer.class); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, Collections.singletonMap(e -> true, consumer)); assertEquals(value, r.execute()); @@ -161,7 +161,7 @@ void testNonMatchingExceptionShouldNotTriggerRecoveryAndRetry() throws Throwable ConcreteConsumer consumer = mock(ConcreteConsumer.class); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, Collections.singletonMap(e -> e.isClientError(), consumer)); assertEquals(value, r.execute()); @@ -180,7 +180,7 @@ void testThrowableInRecoveryAndNotMatchingRetryPredicateShouldBeForwarded() thro ConcreteConsumer consumer = mock(ConcreteConsumer.class); doThrow(new IndexOutOfBoundsException()).when(consumer).consume(); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> t instanceof ApiException, Collections.singletonMap(ApiException::isClientError, consumer)); assertThrows(IndexOutOfBoundsException.class, () -> r.execute()); @@ -202,7 +202,7 @@ void testThrowableInRecoveryAndMatchingRetryPredicateShouldLeadToRetry() throws ConcreteConsumer consumer = mock(ConcreteConsumer.class); doThrow(new IndexOutOfBoundsException()).when(consumer).consume(); - RetryWithRecovery r = new RetryWithRecovery<>("name", getRetryConfig(), supplier, + Resilience4jRetryWithRecovery r = new Resilience4jRetryWithRecovery<>("name", getRetryConfig(), supplier, (t) -> true, Collections.singletonMap(ApiException::isClientError, consumer)); assertEquals(value, r.execute()); From 0db176406343557db82a187d6b98477b5f5bfd88 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Sun, 20 Sep 2020 21:19:52 +0200 Subject: [PATCH 10/17] Added 429 as HTTP code to be retried and updated unit tests --- .../bdk/core/api/invoker/ApiException.java | 4 +++ .../impl/AbstractDatafeedService.java | 5 +-- .../datafeed/impl/DatafeedServiceV1.java | 1 - .../datafeed/impl/DatafeedServiceV2.java | 3 -- .../datafeed/impl/DatafeedServiceV2Test.java | 35 ++++++++++++++----- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java index 41429ed6e..ead57567d 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java @@ -84,4 +84,8 @@ public boolean isClientError() { public boolean isServerError() { return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; } + + public boolean isTooManyRequestsError() { + return this.code == 429; + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index 81bc798bf..6a4032bc1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -41,6 +41,7 @@ public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, this.bdkConfig = config; this.retryWithRecoveryBuilder = new RetryWithRecoveryBuilder<>() .retryConfig(config.getDatafeedRetryConfig()) + .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .recoveryStrategy(ApiException::isUnauthorized, this::refresh); } @@ -93,7 +94,7 @@ private boolean isSelfGeneratedEvent(V4Event event) { protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isClientError(); + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError() || apiException.isClientError(); } return t instanceof ProcessingException; } @@ -101,7 +102,7 @@ protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { protected boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized(); + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError(); } return t instanceof ProcessingException; } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index ff1b3d405..57522e0d9 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -128,7 +128,6 @@ protected String createDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Create Datafeed V1") .supplier(this::createDatafeedAndPersist) - .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .build(); return retry.execute(); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index 03b519d23..d5b12a07c 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -96,7 +96,6 @@ private V5Datafeed createDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Create Datafeed V2") .supplier(this::tryCreateDatafeed) - .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .build(); return retry.execute(); @@ -112,7 +111,6 @@ private V5Datafeed retrieveDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Retrieve Datafeed V2") .supplier(this::tryRetrieveDatafeed) - .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .build(); return retry.execute(); @@ -172,7 +170,6 @@ private void deleteDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Delete Datafeed V2") .supplier(this::tryDeleteDatafeed) - .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .ignoreException(ApiException::isClientError) .build(); diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java index 82845d640..e003991f1 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2Test.java @@ -171,15 +171,10 @@ void testStartServerErrorListDatafeed() throws ApiException { @Test void testStartErrorListDatafeedThenRetrySuccess() throws ApiException, AuthUnauthorizedException { AtomicInteger count = new AtomicInteger(0); - when(datafeedApi.listDatafeed("1234", "1234")).thenAnswer(invocationOnMock -> { - if (count.getAndIncrement() == 0) { - throw new ApiException(502, "server-error"); - } else { - List datafeeds = new ArrayList<>(); - datafeeds.add(new V5Datafeed().id("test-id")); - return datafeeds; - } - }); + when(datafeedApi.listDatafeed("1234", "1234")) + .thenThrow(new ApiException(502, "server-error")) + .thenReturn(Collections.singletonList(new V5Datafeed().id("test-id"))); + AckId ackId = datafeedService.getAckId(); when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)) .thenReturn(new V5EventList().addEventsItem(new V4Event().type(RealTimeEventType.MESSAGESENT.name()).payload(new V4Payload())).ackId("ack-id")); @@ -273,6 +268,28 @@ void testStartServerErrorReadDatafeed() throws ApiException { verify(datafeedApi, times(2)).readDatafeed("test-id", "1234", "1234", ackId); } + @Test + void testStartInternalServerErrorReadDatafeedShouldNotBeRetried() throws ApiException { + AckId ackId = datafeedService.getAckId(); + when(datafeedApi.listDatafeed("1234", "1234")).thenReturn(Collections.singletonList(new V5Datafeed().id("test-id"))); + when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(500, "client-error")); + + assertThrows(ApiException.class, this.datafeedService::start); + verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); + verify(datafeedApi, times(1)).readDatafeed("test-id", "1234", "1234", ackId); + } + + @Test + void testStartTooManyRequestsReadDatafeedShouldBeRetried() throws ApiException { + AckId ackId = datafeedService.getAckId(); + when(datafeedApi.listDatafeed("1234", "1234")).thenReturn(Collections.singletonList(new V5Datafeed().id("test-id"))); + when(datafeedApi.readDatafeed("test-id", "1234", "1234", ackId)).thenThrow(new ApiException(429, "too-many-requests")); + + assertThrows(ApiException.class, this.datafeedService::start); + verify(datafeedApi, times(1)).listDatafeed("1234", "1234"); + verify(datafeedApi, times(2)).readDatafeed("test-id", "1234", "1234", ackId); + } + @Test void testStartClientErrorDeleteDatafeed() throws ApiException, AuthUnauthorizedException { AckId ackId = datafeedService.getAckId(); From 5d6ca3b07c05ccdf02e2216bd81e31ea8479ff56 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Mon, 21 Sep 2020 15:09:36 +0200 Subject: [PATCH 11/17] Implemented retry mechanism for remaing services --- .../core/api/invoker/ApiRuntimeException.java | 1 + .../com/symphony/bdk/core/ServiceFactory.java | 8 +- .../bdk/core/service/MessageService.java | 132 +++++---- .../bdk/core/service/SessionService.java | 10 +- .../impl/AbstractDatafeedService.java | 23 -- .../datafeed/impl/DatafeedServiceV1.java | 2 +- .../datafeed/impl/DatafeedServiceV2.java | 2 +- .../core/service/stream/OboStreamService.java | 34 ++- .../core/service/stream/StreamService.java | 119 ++++---- .../bdk/core/service/user/OboUserService.java | 124 ++++----- .../bdk/core/service/user/UserService.java | 256 ++++++++---------- .../core/util/function/RetryWithRecovery.java | 23 ++ .../function/RetryWithRecoveryBuilder.java | 21 ++ .../bdk/core/service/MessageServiceTest.java | 3 +- .../bdk/core/service/SessionServiceTest.java | 8 +- .../service/stream/StreamServiceTest.java | 3 +- .../core/service/user/UserServiceTest.java | 4 +- .../bdk/spring/config/BdkServiceConfig.java | 6 +- 18 files changed, 381 insertions(+), 398 deletions(-) diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java index 7eb64635b..0d3a7132e 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java @@ -3,6 +3,7 @@ import lombok.Getter; import org.apiguardian.api.API; +import java.util.Collections; import java.util.List; import java.util.Map; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java index 2392f57c4..2c0983d4e 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java @@ -58,7 +58,7 @@ public ServiceFactory(ApiClientFactory apiClientFactory, AuthSession authSession * @return an new {@link UserService} instance. */ public UserService getUserService() { - return new UserService(new UserApi(podClient), new UsersApi(podClient), authSession ); + return new UserService(new UserApi(podClient), new UsersApi(podClient), authSession, config.getRetry()); } /** @@ -67,7 +67,7 @@ public UserService getUserService() { * @return an new {@link StreamService} instance. */ public StreamService getStreamService() { - return new StreamService(new StreamsApi(podClient), authSession); + return new StreamService(new StreamsApi(podClient), authSession, config.getRetry()); } /** @@ -76,7 +76,7 @@ public StreamService getStreamService() { * @return an new {@link SessionService} instance. */ public SessionService getSessionService() { - return new SessionService(new SessionApi(podClient)); + return new SessionService(new SessionApi(podClient), config.getRetry()); } /** @@ -99,6 +99,6 @@ public DatafeedService getDatafeedService() { public MessageService getMessageService() { return new MessageService(new MessagesApi(this.agentClient), new MessageApi(this.podClient), new MessageSuppressionApi(this.podClient), new StreamsApi(this.podClient), new PodApi(this.podClient), - new AttachmentsApi(this.agentClient), new DefaultApi(this.podClient), this.authSession); + new AttachmentsApi(this.agentClient), new DefaultApi(this.podClient), this.authSession, this.config.getRetry()); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java index fdbc61098..8b556a318 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java @@ -4,10 +4,13 @@ import static com.symphony.bdk.core.util.function.SupplierWithApiException.callAndCatchApiException; import com.symphony.bdk.core.api.invoker.util.ApiUtils; +import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.pagination.PaginatedApi; import com.symphony.bdk.core.service.pagination.PaginatedService; -import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; +import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.AttachmentsApi; import com.symphony.bdk.gen.api.DefaultApi; import com.symphony.bdk.gen.api.MessageApi; @@ -55,23 +58,27 @@ public class MessageService { private final DefaultApi defaultApi; private final AuthSession authSession; private final TemplateResolver templateResolver; + private final BdkRetryConfig retryConfig; public MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, - StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession authSession) { + StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, + AuthSession authSession, BdkRetryConfig retryConfig) { this(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, defaultApi, authSession, - new TemplateResolver()); + new TemplateResolver(), retryConfig); } public MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, - StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession authSession, - TemplateEngine templateEngine) { + StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, + AuthSession authSession, + TemplateEngine templateEngine, BdkRetryConfig retryConfig) { this(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, defaultApi, authSession, - new TemplateResolver(templateEngine)); + new TemplateResolver(templateEngine), retryConfig); } private MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, - StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession authSession, - TemplateResolver templateResolver) { + StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, + AuthSession authSession, + TemplateResolver templateResolver, BdkRetryConfig retryConfig) { this.messagesApi = messagesApi; this.messageApi = messageApi; this.messageSuppressionApi = messageSuppressionApi; @@ -81,15 +88,16 @@ private MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSu this.authSession = authSession; this.templateResolver = templateResolver; this.defaultApi = defaultApi; + this.retryConfig = retryConfig; } /** * Get messages from an existing stream. Additionally returns any attachments associated with the message. * * @param stream the stream where to look for messages - * @param since instant of the earliest possible date of the first message returned. - * @param skip number of messages to skip. Optional and defaults to 0. - * @param limit maximum number of messages to return. Optional and defaults to 50. + * @param since instant of the earliest possible date of the first message returned. + * @param skip number of messages to skip. Optional and defaults to 0. + * @param limit maximum number of messages to return. Optional and defaults to 50. * @return the list of matching messages in the stream. * @see Messages */ @@ -101,44 +109,45 @@ public List getMessages(@Nonnull V4Stream stream, @Nonnull Instant si * Get messages from an existing stream. Additionally returns any attachments associated with the message. * * @param streamId the streamID where to look for messages - * @param since instant of the earliest possible date of the first message returned. - * @param skip number of messages to skip. Optional and defaults to 0. - * @param limit maximum number of messages to return. Optional and defaults to 50. + * @param since instant of the earliest possible date of the first message returned. + * @param skip number of messages to skip. Optional and defaults to 0. + * @param limit maximum number of messages to return. Optional and defaults to 50. * @return the list of matching messages in the stream. * @see Messages */ public List getMessages(@Nonnull String streamId, @Nonnull Instant since, Integer skip, Integer limit) { - return callAndCatchApiException(() -> messagesApi.v4StreamSidMessageGet(streamId, getEpochMillis(since), + return executeAndRetry("getMessages", () -> messagesApi.v4StreamSidMessageGet(streamId, getEpochMillis(since), authSession.getSessionToken(), authSession.getKeyManagerToken(), skip, limit)); } /** * Get messages from an existing stream. Additionally returns any attachments associated with the message. * - * @param stream the stream where to look for messages - * @param since instant of the earliest possible date of the first message returned. + * @param stream the stream where to look for messages + * @param since instant of the earliest possible date of the first message returned. * @param chunkSize size of elements to retrieve in one call. Optional and defaults to 50. * @param totalSize total maximum number of messages to return. Optional and defaults to 50. * @return a {@link Stream} of matching messages in the stream. * @see Messages */ - public Stream getMessagesStream(@Nonnull V4Stream stream, @Nonnull Instant since, Integer chunkSize, Integer totalSize) { + public Stream getMessagesStream(@Nonnull V4Stream stream, @Nonnull Instant since, Integer chunkSize, + Integer totalSize) { return getMessagesStream(stream.getStreamId(), since, chunkSize, totalSize); } /** * Get messages from an existing stream. Additionally returns any attachments associated with the message. * - * @param streamId the streamID where to look for messages - * @param since instant of the earliest possible date of the first message returned. + * @param streamId the streamID where to look for messages + * @param since instant of the earliest possible date of the first message returned. * @param chunkSize size of elements to retrieve in one call. Optional and defaults to 50. * @param totalSize total maximum number of messages to return. Optional and defaults to 50. * @return a {@link Stream} of matching messages in the stream. * @see Messages */ - public Stream getMessagesStream(@Nonnull String streamId, @Nonnull Instant since, Integer chunkSize, Integer totalSize) { - PaginatedApi api = ((offset, limit) -> messagesApi.v4StreamSidMessageGet(streamId, getEpochMillis(since), - authSession.getSessionToken(), authSession.getKeyManagerToken(), offset, limit)); + public Stream getMessagesStream(@Nonnull String streamId, @Nonnull Instant since, Integer chunkSize, + Integer totalSize) { + PaginatedApi api = ((offset, limit) -> getMessages(streamId, since, offset, limit)); final int actualChunkSize = chunkSize == null ? 50 : chunkSize.intValue(); final int actualTotalSize = totalSize == null ? 50 : totalSize.intValue(); @@ -167,7 +176,8 @@ public V4Message send(@Nonnull V4Stream stream, @Nonnull String message) { * @return a {@link V4Message} object containing the details of the sent message * @see Create Message v4 */ - public V4Message send(@Nonnull V4Stream stream, @Nonnull String template, Object parameters) throws TemplateException { + public V4Message send(@Nonnull V4Stream stream, @Nonnull String template, Object parameters) + throws TemplateException { return send(stream.getStreamId(), templateResolver.resolve(template).process(parameters)); } @@ -180,7 +190,8 @@ public V4Message send(@Nonnull V4Stream stream, @Nonnull String template, Object * @return a {@link V4Message} object containing the details of the sent message * @throws TemplateException */ - public V4Message send(@Nonnull String streamId, @Nonnull String template, Object parameters) throws TemplateException { + public V4Message send(@Nonnull String streamId, @Nonnull String template, Object parameters) + throws TemplateException { return send(streamId, templateResolver.resolve(template).process(parameters)); } @@ -193,7 +204,7 @@ public V4Message send(@Nonnull String streamId, @Nonnull String template, Object * @see Create Message v4 */ public V4Message send(@Nonnull String streamId, @Nonnull String message) { - return callAndCatchApiException(() -> messagesApi.v4StreamSidMessageCreatePost( + return executeAndRetry("send", () -> messagesApi.v4StreamSidMessageCreatePost( streamId, authSession.getSessionToken(), authSession.getKeyManagerToken(), @@ -208,15 +219,16 @@ public V4Message send(@Nonnull String streamId, @Nonnull String message) { /** * Downloads the attachment body by the stream ID, message ID and attachment ID. * - * @param streamId the stream ID where to look for the attachment - * @param messageId the ID of the message containing the attachment + * @param streamId the stream ID where to look for the attachment + * @param messageId the ID of the message containing the attachment * @param attachmentId the ID of the attachment * @return a byte array of attachment encoded in base 64 * @see Attachment */ public byte[] getAttachment(@Nonnull String streamId, @Nonnull String messageId, @Nonnull String attachmentId) { - return callAndCatchApiException(() -> attachmentsApi.v1StreamSidAttachmentGet(streamId, attachmentId, messageId, - authSession.getSessionToken(), authSession.getKeyManagerToken())); + return executeAndRetry("getAttachment", + () -> attachmentsApi.v1StreamSidAttachmentGet(streamId, attachmentId, messageId, + authSession.getSessionToken(), authSession.getKeyManagerToken())); } /** @@ -227,7 +239,7 @@ public byte[] getAttachment(@Nonnull String streamId, @Nonnull String messageId, * @see Import Message */ public List importMessages(List messages) { - return callAndCatchApiException(() -> messagesApi.v4MessageImportPost(authSession.getSessionToken(), + return executeAndRetry("importMessages", () -> messagesApi.v4MessageImportPost(authSession.getSessionToken(), authSession.getKeyManagerToken(), messages)); } @@ -239,7 +251,7 @@ public List importMessages(List messages) { * @see Suppress Message */ public MessageSuppressionResponse suppressMessage(@Nonnull String messageId) { - return callAndCatchApiException(() -> + return executeAndRetry("suppressMessage", () -> messageSuppressionApi.v1AdminMessagesuppressionIdSuppressPost(messageId, authSession.getSessionToken())); } @@ -252,7 +264,8 @@ public MessageSuppressionResponse suppressMessage(@Nonnull String messageId) { * @see Message Status */ public MessageStatus getMessageStatus(@Nonnull String messageId) { - return callAndCatchApiException(() -> messageApi.v1MessageMidStatusGet(messageId, authSession.getSessionToken())); + return executeAndRetry("getMessageStatus", + () -> messageApi.v1MessageMidStatusGet(messageId, authSession.getSessionToken())); } /** @@ -262,7 +275,7 @@ public MessageStatus getMessageStatus(@Nonnull String messageId) { * @see Attachment Types */ public List getAttachmentTypes() { - return callAndCatchApiException(() -> podApi.v1FilesAllowedTypesGet(authSession.getSessionToken())); + return executeAndRetry("getAttachmentTypes", () -> podApi.v1FilesAllowedTypesGet(authSession.getSessionToken())); } /** @@ -273,7 +286,7 @@ public List getAttachmentTypes() { * @see Get Message v1 */ public V4Message getMessage(@Nonnull String messageId) { - return callAndCatchApiException(() -> messagesApi.v1MessageIdGet(authSession.getSessionToken(), + return executeAndRetry("getMessage", () -> messagesApi.v1MessageIdGet(authSession.getSessionToken(), authSession.getKeyManagerToken(), messageId)); } @@ -281,10 +294,10 @@ public V4Message getMessage(@Nonnull String messageId) { * List attachments in a particular stream. * * @param streamId the stream ID where to look for the attachments - * @param since optional instant of the first required attachment. - * @param to optional instant of the last required attachment. - * @param limit maximum number of attachments to return. This optional value defaults to 50 and should be between 0 and 100. - * @param sort Attachment date sort direction : ASC or DESC (default to ASC) + * @param since optional instant of the first required attachment. + * @param to optional instant of the last required attachment. + * @param limit maximum number of attachments to return. This optional value defaults to 50 and should be between 0 and 100. + * @param sort Attachment date sort direction : ASC or DESC (default to ASC) * @return the list of attachments in the stream. * @see List Attachments */ @@ -292,41 +305,44 @@ public List listAttachments(@Nonnull String streamId, Inst AttachmentSort sort) { final String sortDir = sort == null ? AttachmentSort.ASC.name() : sort.name(); - return callAndCatchApiException(() -> - streamsApi.v1StreamsSidAttachmentsGet(streamId, authSession.getSessionToken(), getEpochMillis(since), getEpochMillis(to), limit, sortDir)); + return executeAndRetry("listAttachments", () -> + streamsApi.v1StreamsSidAttachmentsGet(streamId, authSession.getSessionToken(), getEpochMillis(since), + getEpochMillis(to), limit, sortDir)); } /** * Fetches message ids using timestamp. * * @param streamId the ID of the stream where to fetch messages. - * @param since optional instant of the first required messageId. - * @param to optional instant of the last required messageId. - * @param limit optional maximum number of messageIds to return. - * @param skip optional number of messageIds to skip. + * @param since optional instant of the first required messageId. + * @param to optional instant of the last required messageId. + * @param limit optional maximum number of messageIds to return. + * @param skip optional number of messageIds to skip. * @return a {@link MessageIdsFromStream} object containing the list of messageIds. * @see Get Message IDs by Timestamp */ - public MessageIdsFromStream getMessageIdsByTimestamp(@Nonnull String streamId, Instant since, Instant to, Integer limit, Integer skip) { - return callAndCatchApiException(() -> - defaultApi.v2AdminStreamsStreamIdMessageIdsGet(authSession.getSessionToken(), streamId, getEpochMillis(since), getEpochMillis(to), limit, skip)); + public MessageIdsFromStream getMessageIdsByTimestamp(@Nonnull String streamId, Instant since, Instant to, + Integer limit, Integer skip) { + return executeAndRetry("getMessageIdsByTimestamp", () -> + defaultApi.v2AdminStreamsStreamIdMessageIdsGet(authSession.getSessionToken(), streamId, getEpochMillis(since), + getEpochMillis(to), limit, skip)); } /** * Fetches message ids using timestamp. * - * @param streamId the ID of the stream where to fetch messages. - * @param since optional instant of the first required messageId. - * @param to optional instant of the last required messageId. + * @param streamId the ID of the stream where to fetch messages. + * @param since optional instant of the first required messageId. + * @param to optional instant of the last required messageId. * @param chunkSize size of elements to retrieve in one call. Optional and defaults to 50. * @param totalSize total maximum number of messages to return. Optional and defaults to 50. * @return a {@link Stream} containing the messageIds. * @see Get Message IDs by Timestamp */ - public Stream getMessageIdsByTimestampStream(@Nonnull String streamId, Instant since, Instant to, Integer chunkSize, Integer totalSize) { + public Stream getMessageIdsByTimestampStream(@Nonnull String streamId, Instant since, Instant to, + Integer chunkSize, Integer totalSize) { PaginatedApi api = ((offset, limit) -> - defaultApi.v2AdminStreamsStreamIdMessageIdsGet(authSession.getSessionToken(), streamId, getEpochMillis(since), getEpochMillis(to), limit, offset) - .getData()); + getMessageIdsByTimestamp(streamId, since, to, limit, offset).getData()); final int actualChunkSize = chunkSize == null ? 50 : chunkSize.intValue(); final int actualTotalSize = totalSize == null ? 50 : totalSize.intValue(); @@ -343,7 +359,7 @@ public Stream getMessageIdsByTimestampStream(@Nonnull String streamId, I * @see List Message Receipts */ public MessageReceiptDetailResponse listMessageReceipts(@Nonnull String messageId) { - return callAndCatchApiException(() -> + return executeAndRetry("listMessageReceipts", () -> defaultApi.v1AdminMessagesMessageIdReceiptsGet(authSession.getSessionToken(), messageId, null, null)); } @@ -357,11 +373,15 @@ public MessageReceiptDetailResponse listMessageReceipts(@Nonnull String messageI * @see Message Metadata */ public MessageMetadataResponse getMessageRelationships(@Nonnull String messageId) { - return callAndCatchApiException(() -> defaultApi.v1AdminMessagesMessageIdMetadataRelationshipsGet( + return executeAndRetry("getMessageRelationships", () -> defaultApi.v1AdminMessagesMessageIdMetadataRelationshipsGet( authSession.getSessionToken(), ApiUtils.getUserAgent(), messageId)); } private static Long getEpochMillis(Instant instant) { return instant == null ? null : instant.toEpochMilli(); } + + private T executeAndRetry(String name, SupplierWithApiException supplier) { + return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java index 4ceffafff..178b6943f 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java @@ -3,6 +3,8 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.function.RetryWithRecovery; import com.symphony.bdk.gen.api.SessionApi; import com.symphony.bdk.gen.api.model.UserV2; @@ -17,6 +19,7 @@ public class SessionService { private final SessionApi sessionApi; + private final BdkRetryConfig retryConfig; /** * Retrieves the {@link UserV2} session from the pod using an {@link AuthSession} holder. @@ -25,10 +28,7 @@ public class SessionService { * @return Bot session info. */ public UserV2 getSession(AuthSession authSession) { - try { - return this.sessionApi.v2SessioninfoGet(authSession.getSessionToken()) ; - } catch (ApiException ex) { - throw new ApiRuntimeException(ex); - } + return RetryWithRecovery.executeAndRetry("getSession", + () -> sessionApi.v2SessioninfoGet(authSession.getSessionToken()), retryConfig, authSession); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index 6a4032bc1..35d52df6e 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -6,7 +6,6 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedService; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; -import com.symphony.bdk.core.util.function.ConsumerWithThrowable; import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; @@ -14,12 +13,7 @@ import lombok.extern.slf4j.Slf4j; import java.util.ArrayList; -import java.util.Collections; import java.util.List; -import java.util.Map; -import java.util.function.Predicate; - -import javax.ws.rs.ProcessingException; /** * Base class for implementing the datafeed services. A datafeed services can help a bot subscribe or unsubscribe @@ -41,7 +35,6 @@ public AbstractDatafeedService(DatafeedApi datafeedApi, AuthSession authSession, this.bdkConfig = config; this.retryWithRecoveryBuilder = new RetryWithRecoveryBuilder<>() .retryConfig(config.getDatafeedRetryConfig()) - .retryOnException(this::isNetworkOrServerOrUnauthorizedError) .recoveryStrategy(ApiException::isUnauthorized, this::refresh); } @@ -91,22 +84,6 @@ private boolean isSelfGeneratedEvent(V4Event event) { && event.getInitiator().getUser().getUsername().equals(this.bdkConfig.getBot().getUsername()); } - protected boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { - if (t instanceof ApiException) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError() || apiException.isClientError(); - } - return t instanceof ProcessingException; - } - - protected boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { - if (t instanceof ApiException) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError(); - } - return t instanceof ProcessingException; - } - protected void refresh() throws AuthUnauthorizedException { log.info("Re-authenticate and try again"); authSession.refresh(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 57522e0d9..5c157b89b 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -109,7 +109,7 @@ private void readDatafeed() throws Throwable { .name("Read Datafeed V1") .supplier(this::readAndHandleEvents) .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) - .retryOnException(this::isNetworkOrServerOrUnauthorizedOrClientError) + .retryOnException(RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedOrClientError) .build(); retry.execute(); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index d5b12a07c..e52eeacc1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -132,7 +132,7 @@ private void readDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Read Datafeed V2") .supplier(this::readAndHandleEvents) - .retryOnException(this::isNetworkOrServerOrUnauthorizedOrClientError) + .retryOnException(RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedOrClientError) .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) .build(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java index 433622213..723a0bc1d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java @@ -3,19 +3,26 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.StreamsApi; import com.symphony.bdk.gen.api.model.StreamAttributes; import com.symphony.bdk.gen.api.model.StreamFilter; import com.symphony.bdk.gen.api.model.V2StreamAttributes; +import io.github.resilience4j.retry.RetryConfig; + import java.util.List; class OboStreamService { protected final StreamsApi streamsApi; + protected final BdkRetryConfig retryConfig; - protected OboStreamService(StreamsApi streamsApi) { + protected OboStreamService(StreamsApi streamsApi, BdkRetryConfig retryConfig) { this.streamsApi = streamsApi; + this.retryConfig = retryConfig; } /** @@ -23,15 +30,12 @@ protected OboStreamService(StreamsApi streamsApi) { * * @param authSession Bot Session or Obo Session * @param streamId The stream id - * @return The information about the stream with the given id. - * @see Stream Info V2 + * @return The information about the stream with the given id. + * @see Stream Info V2 */ public V2StreamAttributes getStreamInfo(AuthSession authSession, String streamId) { - try { - return streamsApi.v2StreamsSidInfoGet(streamId, authSession.getSessionToken()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getStreamInfo", + () -> streamsApi.v2StreamsSidInfoGet(streamId, authSession.getSessionToken()), authSession); } /** @@ -39,15 +43,15 @@ public V2StreamAttributes getStreamInfo(AuthSession authSession, String streamId * * @param authSession Bot Session or Obo Session * @param filter The stream searching criteria - * @return The list of streams retrieved according to the searching criteria. - * @see List Streams + * @return The list of streams retrieved according to the searching criteria. + * @see List Streams */ public List listStreams(AuthSession authSession, StreamFilter filter) { - try { - return streamsApi.v1StreamsListPost(authSession.getSessionToken(), null, null, filter); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("listStreams", + () -> streamsApi.v1StreamsListPost(authSession.getSessionToken(), null, null, filter), authSession); } + protected T executeAndRetry(String name, SupplierWithApiException supplier, AuthSession authSession) { + return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java index 41cef9e8d..783987a17 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java @@ -3,7 +3,9 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.StreamsApi; import com.symphony.bdk.gen.api.model.RoomDetail; import com.symphony.bdk.gen.api.model.Stream; @@ -26,39 +28,38 @@ /** * Service class for managing streams. - * + *

* This service is used for retrieving information about a particular stream or * chatroom, searching streams, listing members, attachments of a particular stream, * perform some action related to a stream like: *

    - *
  • Create a IM or MIM
    • - *
  • Create a chatroom
    • - *
  • Activate or Deactivate a chatroom
    • - *
  • + *
  • Create a IM or MIM
    • + *
  • Create a chatroom
    • + *
  • Activate or Deactivate a chatroom
    • + *
  • *

- * */ @Slf4j public class StreamService extends OboStreamService { private final AuthSession authSession; - public StreamService(StreamsApi streamsApi, AuthSession authSession) { - super(streamsApi); + public StreamService(StreamsApi streamsApi, AuthSession authSession, BdkRetryConfig retryConfig) { + super(streamsApi, retryConfig); this.authSession = authSession; } /** * Create a new single or multi party instant message conversation between the caller and specified users. - * + *

* The caller is implicitly included in the members of the created chat. - * + *

* Duplicate users will be included in the membership of the chat but * the duplication will be silently ignored. - * + *

* If there is an existing IM conversation with the same set of participants then * the id of that existing stream will be returned. - * + *

* If the given list of user ids contains only one id, an IM will be created, otherwise, a MIM will be created. * * @param uids List of user ids of the participants. @@ -93,11 +94,8 @@ public Stream create(Long... uids) { * @see Create Room V3 */ public V3RoomDetail create(V3RoomAttributes roomAttributes) { - try { - return streamsApi.v3RoomCreatePost(authSession.getSessionToken(), roomAttributes); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("createStream", + () -> streamsApi.v3RoomCreatePost(authSession.getSessionToken(), roomAttributes)); } /** @@ -108,11 +106,8 @@ public V3RoomDetail create(V3RoomAttributes roomAttributes) { * @see Search Rooms V3 */ public V3RoomSearchResults searchRooms(V2RoomSearchCriteria query) { - try { - return streamsApi.v3RoomSearchPost(authSession.getSessionToken(), query, null, null); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("searchRooms", + () -> streamsApi.v3RoomSearchPost(authSession.getSessionToken(), query, null, null)); } /** @@ -123,11 +118,8 @@ public V3RoomSearchResults searchRooms(V2RoomSearchCriteria query) { * @see Room Info V3 */ public V3RoomDetail getRoomInfo(String roomId) { - try { - return streamsApi.v3RoomIdInfoGet(roomId, authSession.getSessionToken()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getRoomInfo", + () -> streamsApi.v3RoomIdInfoGet(roomId, authSession.getSessionToken())); } /** @@ -139,27 +131,21 @@ public V3RoomDetail getRoomInfo(String roomId) { * @see De/Reactivate Room */ public RoomDetail setRoomActive(String roomId, Boolean active) { - try { - return streamsApi.v1RoomIdSetActivePost(roomId, active, authSession.getSessionToken()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("setRoomActive", + () -> streamsApi.v1RoomIdSetActivePost(roomId, active, authSession.getSessionToken())); } /** * Update the attributes of an existing chatroom. * - * @param roomId The id of the room to be updated + * @param roomId The id of the room to be updated * @param roomAttributes The attributes to be updated to the room * @return The information of the room after being updated. * @see Update Room V3 */ public V3RoomDetail updateRoom(String roomId, V3RoomAttributes roomAttributes) { - try { - return streamsApi.v3RoomIdUpdatePost(roomId, authSession.getSessionToken(), roomAttributes); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("updateRoom", + () -> streamsApi.v3RoomIdUpdatePost(roomId, authSession.getSessionToken(), roomAttributes)); } /** @@ -188,30 +174,29 @@ public V2StreamAttributes getStreamInfo(String streamId) { /** * List attachments in a particular stream. * - * @param streamId The stream id + * @param streamId The stream id * @param sinceInMillis Timestamp in millis of first required attachment - * @param toInMillis Timestamp in millis of last required attachment - * @param sort Attachment date sort direction : ASC or DESC (default to ASC) + * @param toInMillis Timestamp in millis of last required attachment + * @param sort Attachment date sort direction : ASC or DESC (default to ASC) * @return List of attachments in the stream with the given stream id. * @see List Attachments */ - public List getAttachmentsOfStream(String streamId, Long sinceInMillis, Long toInMillis, AttachmentSort sort) { - try { - return streamsApi.v1StreamsSidAttachmentsGet(streamId, authSession.getSessionToken(), sinceInMillis, toInMillis, null, sort.name()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + public List getAttachmentsOfStream(String streamId, Long sinceInMillis, Long toInMillis, + AttachmentSort sort) { + return executeAndRetry("getAttachmentsOfStream", + () -> streamsApi.v1StreamsSidAttachmentsGet(streamId, authSession.getSessionToken(), sinceInMillis, toInMillis, + null, sort.name())); } /** * Create a new single or multi party instant message conversation. * At least two user IDs must be provided or an error response will be sent. - * + *

* The caller is not included in the members of the created chat. - * + *

* Duplicate users will be included in the membership of the chat but the * duplication will be silently ignored. - * + *

* If there is an existing IM conversation with the same set of participants then * the id of that existing stream will be returned. * @@ -220,26 +205,20 @@ public List getAttachmentsOfStream(String streamId, Long s * @see Create IM or MIM Non-inclusive */ public Stream createInstantMessageAdmin(List uids) { - try { - return streamsApi.v1AdminImCreatePost(authSession.getSessionToken(), uids); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("createInstantMessageAdmin", + () -> streamsApi.v1AdminImCreatePost(authSession.getSessionToken(), uids)); } /** * Deactivate or reactivate a chatroom via AC Portal. * * @param streamId The stream id - * @param active Deactivate or activate + * @param active Deactivate or activate * @return The information of the room after being deactivated or reactivated. */ public RoomDetail setRoomActiveAdmin(String streamId, Boolean active) { - try { - return streamsApi.v1AdminRoomIdSetActivePost(streamId, active, authSession.getSessionToken()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("setRoomActiveAdmin", + () -> streamsApi.v1AdminRoomIdSetActivePost(streamId, active, authSession.getSessionToken())); } /** @@ -250,11 +229,8 @@ public RoomDetail setRoomActiveAdmin(String streamId, Boolean active) { * @see List Streams for Enterprise V2 */ public V2AdminStreamList listStreamsAdmin(V2AdminStreamFilter filter) { - try { - return streamsApi.v2AdminStreamsListPost(authSession.getSessionToken(), null, null, filter); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("listStreamsAdmin", + () -> streamsApi.v2AdminStreamsListPost(authSession.getSessionToken(), null, null, filter)); } /** @@ -266,10 +242,11 @@ public V2AdminStreamList listStreamsAdmin(V2AdminStreamFilter filter) { * @see Stream Members */ public V2MembershipList listStreamMembers(String streamId) { - try { - return streamsApi.v1AdminStreamIdMembershipListGet(streamId, authSession.getSessionToken(), null, null); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("listStreamMembers", + () -> streamsApi.v1AdminStreamIdMembershipListGet(streamId, authSession.getSessionToken(), null, null)); + } + + private T executeAndRetry(String name, SupplierWithApiException supplier) { + return executeAndRetry(name, supplier, authSession); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java index 71419dd85..9419fbfba 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java @@ -1,8 +1,9 @@ package com.symphony.bdk.core.service.user; -import com.symphony.bdk.core.api.invoker.ApiException; -import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.UserApi; import com.symphony.bdk.gen.api.UsersApi; import com.symphony.bdk.gen.api.model.UserSearchQuery; @@ -21,10 +22,12 @@ class OboUserService { protected final UserApi userApi; protected final UsersApi usersApi; + protected final BdkRetryConfig retryConfig; - protected OboUserService(UserApi userApi, UsersApi usersApi) { + protected OboUserService(UserApi userApi, UsersApi usersApi, BdkRetryConfig retryConfig) { this.userApi = userApi; this.usersApi = usersApi; + this.retryConfig = retryConfig; } /** @@ -35,17 +38,15 @@ protected OboUserService(UserApi userApi, UsersApi usersApi) { * @param local If true then a local DB search will be performed and only local pod users will be * returned. If absent or false then a directory search will be performed and users * from other pods who are visible to the calling user will also be returned. - * @return Users found by user ids - * @see Users Lookup V3 + * @return Users found by user ids + * @see Users Lookup V3 */ - public List searchUserByIds(@NonNull AuthSession authSession, @NonNull List uidList, @NonNull Boolean local) { - try { - String uids = uidList.stream().map(String::valueOf).collect(Collectors.joining(",")); - V2UserList v2UserList = usersApi.v3UsersGet(authSession.getSessionToken(), uids, null, null, local); - return v2UserList.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + public List searchUserByIds(@NonNull AuthSession authSession, @NonNull List uidList, + @NonNull Boolean local) { + String uids = uidList.stream().map(String::valueOf).collect(Collectors.joining(",")); + V2UserList v2UserList = executeAndRetry("searchUserByIds", + () -> usersApi.v3UsersGet(authSession.getSessionToken(), uids, null, null, local), authSession); + return v2UserList.getUsers(); } /** @@ -53,17 +54,14 @@ public List searchUserByIds(@NonNull AuthSession authSession, @NonNull L * * @param authSession Bot Session or Obo Session * @param uidList List of user ids - * @return Users found by user ids - * @see Users Lookup V3 + * @return Users found by user ids + * @see Users Lookup V3 */ public List searchUserByIds(@NonNull AuthSession authSession, @NonNull List uidList) { - try { - String uids = uidList.stream().map(String::valueOf).collect(Collectors.joining(",")); - V2UserList v2UserList = usersApi.v3UsersGet(authSession.getSessionToken(), uids, null, null, false); - return v2UserList.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + String uids = uidList.stream().map(String::valueOf).collect(Collectors.joining(",")); + V2UserList v2UserList = executeAndRetry("searchUserByIds", + () -> usersApi.v3UsersGet(authSession.getSessionToken(), uids, null, null, false), authSession); + return v2UserList.getUsers(); } /** @@ -74,17 +72,15 @@ public List searchUserByIds(@NonNull AuthSession authSession, @NonNull L * @param local If true then a local DB search will be performed and only local pod users will be * returned. If absent or false then a directory search will be performed and users * from other pods who are visible to the calling user will also be returned. - * @return Users found by emails. - * @see Users Lookup V3 + * @return Users found by emails. + * @see Users Lookup V3 */ - public List searchUserByEmails(@NonNull AuthSession authSession, @NonNull List emailList, @NonNull Boolean local) { - try { - String emails = String.join(",", emailList); - V2UserList v2UserList = usersApi.v3UsersGet(authSession.getSessionToken(), null, emails, null, local); - return v2UserList.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + public List searchUserByEmails(@NonNull AuthSession authSession, @NonNull List emailList, + @NonNull Boolean local) { + String emails = String.join(",", emailList); + V2UserList v2UserList = executeAndRetry("searchUserByEmails", + () -> usersApi.v3UsersGet(authSession.getSessionToken(), null, emails, null, local), authSession); + return v2UserList.getUsers(); } /** @@ -92,54 +88,50 @@ public List searchUserByEmails(@NonNull AuthSession authSession, @NonNul * * @param authSession Bot Session or Obo Session * @param emailList List of emails - * @return Users found by emails - * @see Users Lookup V3 + * @return Users found by emails + * @see Users Lookup V3 */ public List searchUserByEmails(@NonNull AuthSession authSession, @NonNull List emailList) { - try { - String emails = String.join(",", emailList); - V2UserList v2UserList = usersApi.v3UsersGet(authSession.getSessionToken(), null, emails, null, false); - return v2UserList.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + String emails = String.join(",", emailList); + V2UserList v2UserList = executeAndRetry("searchUserByEmails", + () -> usersApi.v3UsersGet(authSession.getSessionToken(), null, emails, null, false), authSession); + return v2UserList.getUsers(); } /** * {@link UserService#searchUserByUsernames(List)} * - * @param authSession Bot Session or Obo Session - * @param usernameList List of usernames - * @return Users found by usernames - * @see Users Lookup V3 + * @param authSession Bot Session or Obo Session + * @param usernameList List of usernames + * @return Users found by usernames + * @see Users Lookup V3 */ public List searchUserByUsernames(@NonNull AuthSession authSession, @NonNull List usernameList) { - try { - String usernames = String.join(",", usernameList); - V2UserList v2UserList = usersApi.v3UsersGet(authSession.getSessionToken(), null, null, usernames, true); - return v2UserList.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + String usernames = String.join(",", usernameList); + V2UserList v2UserList = executeAndRetry("searchUserByUsernames", + () -> usersApi.v3UsersGet(authSession.getSessionToken(), null, null, usernames, true), authSession); + return v2UserList.getUsers(); } /** * {@link UserService#searchUserBySearchQuery(UserSearchQuery, Boolean)} * - * @param authSession Bot Session or Obo Session - * @param query Searching query containing complicated information like title, location, company... - * @param local If true then a local DB search will be performed and only local pod users will be - * returned. If absent or false then a directory search will be performed and users - * from other pods who are visible to the calling user will also be returned. - * @return List of users found by query - * @see Search Users + * @param authSession Bot Session or Obo Session + * @param query Searching query containing complicated information like title, location, company... + * @param local If true then a local DB search will be performed and only local pod users will be + * returned. If absent or false then a directory search will be performed and users + * from other pods who are visible to the calling user will also be returned. + * @return List of users found by query + * @see Search Users */ - public List searchUserBySearchQuery(@NonNull AuthSession authSession, @NonNull UserSearchQuery query, @Nullable Boolean local) { - try { - UserSearchResults results = usersApi.v1UserSearchPost(authSession.getSessionToken(), query, null, null, local); - return results.getUsers(); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + public List searchUserBySearchQuery(@NonNull AuthSession authSession, @NonNull UserSearchQuery query, + @Nullable Boolean local) { + UserSearchResults results = executeAndRetry("searchUserBySearchQuery", + () -> usersApi.v1UserSearchPost(authSession.getSessionToken(), query, null, null, local), authSession); + return results.getUsers(); + } + + protected T executeAndRetry(String name, SupplierWithApiException supplier, AuthSession authSession) { + return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java index 31373c226..de27322a2 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java @@ -1,10 +1,10 @@ package com.symphony.bdk.core.service.user; -import com.symphony.bdk.core.api.invoker.ApiException; -import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.user.constant.RoleId; import com.symphony.bdk.core.service.user.mapper.UserDetailMapper; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.UserApi; import com.symphony.bdk.gen.api.UsersApi; import com.symphony.bdk.gen.api.model.Avatar; @@ -32,28 +32,28 @@ import javax.annotation.Nullable; + /** * Service class for managing users. - * + *

* This service is used for retrieving information about a particular user, * search users by ids, emails or usernames, perform some action related to * user like: *

    - *
  • Add or remove roles from an user
    • - *
  • Get or update avatar of an user
    • - *
  • Get, assign or unassign disclaimer to an user
    • - *
  • Get, update feature entitlements of an user
    • - *
  • Get, update status of an user
    • + *
  • Add or remove roles from an user
    • + *
  • Get or update avatar of an user
    • + *
  • Get, assign or unassign disclaimer to an user
    • + *
  • Get, update feature entitlements of an user
    • + *
  • Get, update status of an user
    • *

- * */ @Slf4j public class UserService extends OboUserService { private final AuthSession authSession; - public UserService(UserApi userApi, UsersApi usersApi, AuthSession authSession) { - super(userApi, usersApi); + public UserService(UserApi userApi, UsersApi usersApi, AuthSession authSession, BdkRetryConfig retryConfig) { + super(userApi, usersApi, retryConfig); this.authSession = authSession; } @@ -61,94 +61,76 @@ public UserService(UserApi userApi, UsersApi usersApi, AuthSession authSession) * Retrieve user details of a particular user. * * @param uid User Id - * @return Details of the user. - * @see Get User v2 + * @return Details of the user. + * @see Get User v2 */ public V2UserDetail getUserDetailByUid(@NonNull Long uid) { - try { - return userApi.v2AdminUserUidGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getUserDetailByUid", () -> userApi.v2AdminUserUidGet(authSession.getSessionToken(), uid)); } /** * Retrieve all users in the company (pod). * * @return List of retrieved users - * @see List Users V2 + * @see List Users V2 */ public List listUsersDetail() { - try { - return userApi.v2AdminUserListGet(authSession.getSessionToken(), null, null); - - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("listUsersDetail", + () -> userApi.v2AdminUserListGet(authSession.getSessionToken(), null, null)); } /** * Retrieve a list of users in the company (pod) by a filter. * - * @param filter using to filter users by - * @return List of retrieved users - * @see Find Users V1 - * @see com.symphony.bdk.core.service.user.constant.UserFeature + * @param filter using to filter users by + * @return List of retrieved users + * @see Find Users V1 + * @see com.symphony.bdk.core.service.user.constant.UserFeature */ public List listUsersDetail(@NonNull UserFilter filter) { - try { - List userDetailList = userApi.v1AdminUserFindPost(authSession.getSessionToken(), filter, null, null); - return userDetailList.stream().map(UserDetailMapper.INSTANCE::userDetailToV2UserDetail).collect(Collectors.toList()); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + List userDetailList = executeAndRetry("listUsersDetail", + () -> userApi.v1AdminUserFindPost(authSession.getSessionToken(), filter, null, null)); + return userDetailList.stream() + .map(UserDetailMapper.INSTANCE::userDetailToV2UserDetail) + .collect(Collectors.toList()); } /** * Add a role to an user. * - * @param uid User Id - * @param roleId Role Id - * @see Add Role + * @param uid User Id + * @param roleId Role Id + * @see Add Role */ public void addRoleToUser(@NonNull Long uid, @NonNull RoleId roleId) { - try { - StringId stringId = new StringId().id(roleId.name()); - userApi.v1AdminUserUidRolesAddPost(authSession.getSessionToken(), uid, stringId); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + StringId stringId = new StringId().id(roleId.name()); + executeAndRetry("addRoleToUser", + () -> userApi.v1AdminUserUidRolesAddPost(authSession.getSessionToken(), uid, stringId)); } /** * Remove a role from an user. * - * @param uid User Id - * @param roleId Role Id - * @see Remove Role + * @param uid User Id + * @param roleId Role Id + * @see Remove Role */ public void removeRoleFromUser(@NonNull Long uid, @NonNull RoleId roleId) { - try { - StringId stringId = new StringId().id(roleId.name()); - userApi.v1AdminUserUidRolesRemovePost(authSession.getSessionToken(), uid, stringId); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + StringId stringId = new StringId().id(roleId.name()); + executeAndRetry("removeRoleFromUser", + () -> userApi.v1AdminUserUidRolesRemovePost(authSession.getSessionToken(), uid, stringId)); } /** * Get the url of avatar of an user * * @param uid User Id - * @return List of avatar urls of the user - * @see User Avatar + * @return List of avatar urls of the user + * @see User Avatar */ public List getAvatarFromUser(@NonNull Long uid) { - try { - return userApi.v1AdminUserUidAvatarGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getAvatarFromUser", + () -> userApi.v1AdminUserUidAvatarGet(authSession.getSessionToken(), uid)); } /** @@ -156,15 +138,12 @@ public List getAvatarFromUser(@NonNull Long uid) { * * @param uid User Id * @param image The avatar image for the user profile picture.The image must be a base64-encoded. - * @see Update User Avatar + * @see Update User Avatar */ public void updateAvatarOfUser(@NonNull Long uid, @NonNull String image) { - try { - AvatarUpdate avatarUpdate = new AvatarUpdate().image(image); - userApi.v1AdminUserUidAvatarUpdatePost(authSession.getSessionToken(), uid, avatarUpdate); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + AvatarUpdate avatarUpdate = new AvatarUpdate().image(image); + executeAndRetry("updateAvatarOfUser", + () -> userApi.v1AdminUserUidAvatarUpdatePost(authSession.getSessionToken(), uid, avatarUpdate)); } /** @@ -172,7 +151,7 @@ public void updateAvatarOfUser(@NonNull Long uid, @NonNull String image) { * * @param uid User Id * @param image The avatar image in bytes array for the user profile picture. - * @see Update User Avatar + * @see Update User Avatar */ public void updateAvatarOfUser(@NonNull Long uid, @NonNull byte[] image) { String imageBase64 = Base64.getEncoder().encodeToString(image); @@ -184,7 +163,7 @@ public void updateAvatarOfUser(@NonNull Long uid, @NonNull byte[] image) { * * @param uid User Id * @param imageStream The avatar image input stream for the user profile picture. - * @see Update User Avatar + * @see Update User Avatar */ public void updateAvatarOfUser(@NonNull Long uid, @NonNull InputStream imageStream) throws IOException { byte[] bytes = IOUtils.toByteArray(imageStream); @@ -195,60 +174,49 @@ public void updateAvatarOfUser(@NonNull Long uid, @NonNull InputStream imageStre * Get disclaimer assigned to an user. * * @param uid User Id - * @return Disclaimer assigned to the user. - * @see User Disclaimer + * @return Disclaimer assigned to the user. + * @see User Disclaimer */ public Disclaimer getDisclaimerAssignedToUser(@NonNull Long uid) { - try { - return userApi.v1AdminUserUidDisclaimerGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getDisclaimerAssignedToUser", + () -> userApi.v1AdminUserUidDisclaimerGet(authSession.getSessionToken(), uid)); } /** * Unassign disclaimer from an user. * * @param uid User Id - * @see Unassign User Disclaimer + * @see Unassign User Disclaimer */ public void unAssignDisclaimerFromUser(@NonNull Long uid) { - try { - userApi.v1AdminUserUidDisclaimerDelete(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + executeAndRetry("unAssignDisclaimerFromUser", + () -> userApi.v1AdminUserUidDisclaimerDelete(authSession.getSessionToken(), uid)); } /** * Assign disclaimer to an user. * - * @param uid User Id - * @param disclaimerId Disclaimer to be assigned - * @see Update User Disclaimer + * @param uid User Id + * @param disclaimerId Disclaimer to be assigned + * @see Update User Disclaimer */ public void assignDisclaimerToUser(@NonNull Long uid, @NonNull String disclaimerId) { - try { - StringId stringId = new StringId().id(disclaimerId); - userApi.v1AdminUserUidDisclaimerUpdatePost(authSession.getSessionToken(), uid, stringId); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + StringId stringId = new StringId().id(disclaimerId); + executeAndRetry("assignDisclaimerToUser", + () -> userApi.v1AdminUserUidDisclaimerUpdatePost(authSession.getSessionToken(), uid, stringId)); + } /** * Get delegates assigned to an user. * * @param uid User Id - * @return List of delegates assigned to an user. - * @see User Delegates + * @return List of delegates assigned to an user. + * @see User Delegates */ public List getDelegatesAssignedToUser(@NonNull Long uid) { - try { - return userApi.v1AdminUserUidDelegatesGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getDelegatesAssignedToUser", + () -> userApi.v1AdminUserUidDelegatesGet(authSession.getSessionToken(), uid)); } /** @@ -257,75 +225,61 @@ public List getDelegatesAssignedToUser(@NonNull Long uid) { * @param uid User Id * @param delegatedUserId Delegated user Id to be assigned * @param actionEnum Action to be performed - * @see Update User Delegates + * @see Update User Delegates */ - public void updateDelegatesAssignedToUser(@NonNull Long uid, @NonNull Long delegatedUserId, @NonNull DelegateAction.ActionEnum actionEnum) { - try { - DelegateAction delegateAction = new DelegateAction().action(actionEnum).userId(delegatedUserId); - userApi.v1AdminUserUidDelegatesUpdatePost(authSession.getSessionToken(), uid, delegateAction); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + public void updateDelegatesAssignedToUser(@NonNull Long uid, @NonNull Long delegatedUserId, + @NonNull DelegateAction.ActionEnum actionEnum) { + DelegateAction delegateAction = new DelegateAction().action(actionEnum).userId(delegatedUserId); + executeAndRetry("updateDelegatesAssignedToUser", + () -> userApi.v1AdminUserUidDelegatesUpdatePost(authSession.getSessionToken(), uid, delegateAction)); } /** * Get feature entitlements of an user. * * @param uid User Id - * @return List of feature entitlements of the user. - * @see User Features + * @return List of feature entitlements of the user. + * @see User Features */ public List getFeatureEntitlementsOfUser(@NonNull Long uid) { - try { - return userApi.v1AdminUserUidFeaturesGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getFeatureEntitlementsOfUser", + () -> userApi.v1AdminUserUidFeaturesGet(authSession.getSessionToken(), uid)); } /** * Update feature entitlements of an user. * - * @param uid User Id - * @param features List of feature entitlements to be updated - * @see Update User Features + * @param uid User Id + * @param features List of feature entitlements to be updated + * @see Update User Features */ public void updateFeatureEntitlementsOfUser(@NonNull Long uid, @NonNull List features) { - try { - userApi.v1AdminUserUidFeaturesUpdatePost(authSession.getSessionToken(), uid, features); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + executeAndRetry("updateFeatureEntitlementsOfUser", + () -> userApi.v1AdminUserUidFeaturesUpdatePost(authSession.getSessionToken(), uid, features)); } /** * Get status of an user. * * @param uid User Id - * @return Status of the user. - * @see User Status + * @return Status of the user. + * @see User Status */ public UserStatus getStatusOfUser(@NonNull Long uid) { - try { - return userApi.v1AdminUserUidStatusGet(authSession.getSessionToken(), uid); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + return executeAndRetry("getStatusOfUser", + () -> userApi.v1AdminUserUidStatusGet(authSession.getSessionToken(), uid)); } /** * Update the status of an user * - * @param uid User Id - * @param status Status to be updated to the user - * @see Update User Status + * @param uid User Id + * @param status Status to be updated to the user + * @see Update User Status */ public void updateStatusOfUser(@NonNull Long uid, @NonNull UserStatus status) { - try { - userApi.v1AdminUserUidStatusUpdatePost(authSession.getSessionToken(), uid, status); - } catch (ApiException apiException) { - throw new ApiRuntimeException(apiException); - } + executeAndRetry("updateStatusOfUser", + () -> userApi.v1AdminUserUidStatusUpdatePost(authSession.getSessionToken(), uid, status)); } /** @@ -335,8 +289,8 @@ public void updateStatusOfUser(@NonNull Long uid, @NonNull UserStatus status) { * @param local If true then a local DB search will be performed and only local pod users will be * returned. If absent or false then a directory search will be performed and users * from other pods who are visible to the calling user will also be returned. - * @return Users found by user ids - * @see Users Lookup V3 + * @return Users found by user ids + * @see Users Lookup V3 */ public List searchUserByIds(@NonNull List uidList, @NonNull Boolean local) { return this.searchUserByIds(this.authSession, uidList, local); @@ -346,8 +300,8 @@ public List searchUserByIds(@NonNull List uidList, @NonNull Boolea * Search user by list of user ids * * @param uidList List of user ids - * @return Users found by user ids - * @see Users Lookup V3 + * @return Users found by user ids + * @see Users Lookup V3 */ public List searchUserByIds(@NonNull List uidList) { return this.searchUserByIds(this.authSession, uidList); @@ -360,8 +314,8 @@ public List searchUserByIds(@NonNull List uidList) { * @param local If true then a local DB search will be performed and only local pod users will be * returned. If absent or false then a directory search will be performed and users * from other pods who are visible to the calling user will also be returned. - * @return Users found by emails. - * @see Users Lookup V3 + * @return Users found by emails. + * @see Users Lookup V3 */ public List searchUserByEmails(@NonNull List emailList, @NonNull Boolean local) { return this.searchUserByEmails(this.authSession, emailList, local); @@ -371,8 +325,8 @@ public List searchUserByEmails(@NonNull List emailList, @NonNull * Search user by list of email addresses. * * @param emailList List of email addresses - * @return Users found by emails. - * @see Users Lookup V3 + * @return Users found by emails. + * @see Users Lookup V3 */ public List searchUserByEmails(@NonNull List emailList) { return this.searchUserByEmails(this.authSession, emailList); @@ -381,9 +335,9 @@ public List searchUserByEmails(@NonNull List emailList) { /** * Search user by list of usernames. * - * @param usernameList List of usernames - * @return Users found by usernames - * @see Users Lookup V3 + * @param usernameList List of usernames + * @return Users found by usernames + * @see Users Lookup V3 */ public List searchUserByUsernames(@NonNull List usernameList) { return this.searchUserByUsernames(this.authSession, usernameList); @@ -396,10 +350,14 @@ public List searchUserByUsernames(@NonNull List usernameList) { * @param local If true then a local DB search will be performed and only local pod users will be * returned. If absent or false then a directory search will be performed and users * from other pods who are visible to the calling user will also be returned. - * @return List of users found by query - * @see Search Users + * @return List of users found by query + * @see Search Users */ public List searchUserBySearchQuery(@NonNull UserSearchQuery query, @Nullable Boolean local) { return this.searchUserBySearchQuery(this.authSession, query, local); } + + private T executeAndRetry(String name, SupplierWithApiException supplier) { + return executeAndRetry(name, supplier, authSession); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java index babc2cce7..42e9aea9b 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java @@ -2,6 +2,11 @@ import com.symphony.bdk.core.api.invoker.ApiException; +import com.symphony.bdk.core.api.invoker.ApiRuntimeException; +import com.symphony.bdk.core.auth.AuthSession; + +import com.symphony.bdk.core.config.model.BdkRetryConfig; + import lombok.extern.slf4j.Slf4j; import java.util.Map; @@ -13,6 +18,24 @@ public abstract class RetryWithRecovery { private Predicate ignoreApiException; private Map, ConsumerWithThrowable> recoveryStrategies; + public static T executeAndRetry(String name, SupplierWithApiException supplier, BdkRetryConfig retryConfig, + AuthSession authSession) { + RetryWithRecovery retry = new RetryWithRecoveryBuilder() + .name(name) + .supplier(supplier) + .retryConfig(retryConfig) + .recoveryStrategy(e -> e.isUnauthorized(), () -> authSession.refresh()) + .build(); + + try { + return retry.execute(); + } catch (ApiException e) { + throw new ApiRuntimeException(e); + } catch (Throwable t) { + throw new RuntimeException(t); + } + } + public RetryWithRecovery(SupplierWithApiException supplier, Predicate ignoreApiException, Map, ConsumerWithThrowable> recoveryStrategies) { this.supplier = supplier; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java index decf26cbf..ea4b8e239 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java @@ -3,6 +3,8 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import javax.ws.rs.ProcessingException; + import java.util.HashMap; import java.util.Map; import java.util.function.Predicate; @@ -26,8 +28,27 @@ public static RetryWithRecoveryBuilder from(RetryWithRecoveryBuilder f return copy; } + //TODO rename + public static boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { + if (t instanceof ApiException) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError(); + } + return t instanceof ProcessingException; + } + + //TODO rename + public static boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { + if (t instanceof ApiException) { + ApiException apiException = (ApiException) t; + return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError() || apiException.isClientError(); + } + return t instanceof ProcessingException; + } + public RetryWithRecoveryBuilder() { this.ignoreException = (e) -> false; + this.retryOnExceptionPredicate = RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedError; this.recoveryStrategies = new HashMap<>(); } diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java index b5c982b5e..eef4e2006 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java @@ -18,6 +18,7 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; import com.symphony.bdk.core.test.JsonHelper; import com.symphony.bdk.core.test.MockApiClient; @@ -94,7 +95,7 @@ void setUp() { messageService = new MessageService(new MessagesApi(agentClient), new MessageApi(podClient), new MessageSuppressionApi(podClient), streamsApi, new PodApi(podClient), - attachmentsApi, new DefaultApi(podClient), authSession, templateEngine); + attachmentsApi, new DefaultApi(podClient), authSession, templateEngine, new BdkRetryConfig()); } @Test diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java index 17db88e9d..c77155a02 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java @@ -8,6 +8,7 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.gen.api.SessionApi; import com.symphony.bdk.gen.api.model.UserV2; @@ -35,7 +36,12 @@ class SessionServiceTest { @BeforeEach void setUp() { - this.service = new SessionService(this.sessionApi); + final BdkRetryConfig retryConfig = new BdkRetryConfig(); + retryConfig.setMaxAttempts(2); + retryConfig.setMultiplier(1); + retryConfig.setInitialIntervalMillis(10); + + this.service = new SessionService(this.sessionApi, retryConfig); } @Test diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java index fc60411c5..ee1864ca6 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java @@ -8,6 +8,7 @@ import com.symphony.bdk.core.api.invoker.ApiClient; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; import com.symphony.bdk.core.test.MockApiClient; import com.symphony.bdk.core.test.JsonHelper; @@ -58,7 +59,7 @@ void setUp() { this.mockApiClient = new MockApiClient(); AuthSession authSession = mock(AuthSession.class); ApiClient podClient = mockApiClient.getApiClient("/pod"); - this.service = new StreamService(new StreamsApi(podClient), authSession); + this.service = new StreamService(new StreamsApi(podClient), authSession, new BdkRetryConfig()); when(authSession.getSessionToken()).thenReturn("1234"); when(authSession.getKeyManagerToken()).thenReturn("1234"); diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java index 04c353724..49b454064 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java @@ -14,6 +14,7 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.service.user.constant.RoleId; import com.symphony.bdk.core.service.user.constant.UserFeature; import com.symphony.bdk.core.service.user.mapper.UserDetailMapper; @@ -63,7 +64,6 @@ class UserServiceTest { private static final String UPDATE_FEATURE_ENTITLEMENTS_OF_USER = "/pod/v1/admin/user/{uid}/features/update"; private static final String GET_STATUS_OF_USER = "/pod/v1/admin/user/{uid}/status"; private static final String UPDATE_STATUS_OF_USER = "/pod/v1/admin/user/{uid}/status/update"; - private static final String GET_USER_V2 = "/pod/v2/user"; private static final String SEARCH_USERS_V3 = "/pod/v3/users"; private static final String SEARCH_USER_BY_QUERY = "/pod/v1/user/search"; @@ -81,7 +81,7 @@ void init() { this.spiedUserApi = spy(userApi); UsersApi usersApi = new UsersApi(podClient); this.spiedUsersApi = spy(usersApi); - this.service = new UserService(this.spiedUserApi, this.spiedUsersApi, authSession); + this.service = new UserService(this.spiedUserApi, this.spiedUsersApi, authSession, new BdkRetryConfig()); when(authSession.getSessionToken()).thenReturn("1234"); when(authSession.getKeyManagerToken()).thenReturn("1234"); diff --git a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java index ebe7ac5bc..82241c3d8 100644 --- a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java +++ b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java @@ -1,6 +1,7 @@ package com.symphony.bdk.spring.config; import com.symphony.bdk.core.auth.AuthSession; +import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.MessageService; import com.symphony.bdk.gen.api.AttachmentsApi; import com.symphony.bdk.gen.api.DefaultApi; @@ -26,8 +27,9 @@ public class BdkServiceConfig { @Bean public MessageService messageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, - StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession botSession) { + StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession botSession, + BdkConfig config) { return new MessageService(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, - defaultApi, botSession); + defaultApi, botSession, config.getRetry()); } } From f454949a96f607a00f4c4eb8d43bab16690478a8 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Mon, 21 Sep 2020 16:24:29 +0200 Subject: [PATCH 12/17] Javadoc --- .../bdk/core/api/invoker/ApiException.java | 141 ++++++++++-------- .../core/api/invoker/ApiRuntimeException.java | 1 - .../bdk/core/config/model/BdkConfig.java | 67 +++++---- .../bdk/core/service/MessageService.java | 2 - .../symphony/bdk/core/service/OboEnabled.java | 16 -- .../exception/NestedRetryException.java | 4 + .../datafeed/impl/DatafeedServiceV1.java | 7 +- .../datafeed/impl/DatafeedServiceV2.java | 7 +- .../Resilience4jRetryWithRecovery.java | 5 +- .../core/util/function/RetryWithRecovery.java | 34 ++++- .../function/RetryWithRecoveryBuilder.java | 91 ++++++++++- .../function/SupplierWithApiException.java | 15 +- 12 files changed, 241 insertions(+), 149 deletions(-) delete mode 100644 symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/OboEnabled.java diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java index ead57567d..023f41da2 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java @@ -5,6 +5,7 @@ import javax.net.ssl.HttpsURLConnection; import javax.ws.rs.core.GenericType; + import java.net.HttpURLConnection; import java.util.Arrays; import java.util.List; @@ -12,80 +13,94 @@ /** * Main exception raised when invoking {@link ApiClient#invokeAPI(String, String, List, Object, Map, Map, Map, String, String, String[], GenericType)}. - * + *

* Initially generated by the OpenAPI Maven Generator */ @Getter @API(status = API.Status.STABLE) public class ApiException extends Exception { - private int code = 0; - private Map> responseHeaders = null; - private String responseBody = null; + private int code = 0; + private Map> responseHeaders = null; + private String responseBody = null; + + /** + * Creates new {@link ApiException} instance. + * + * @param message the detail message. + * @param throwable the cause. + */ + public ApiException(String message, Throwable throwable) { + super(message, throwable); + } - /** - * Creates new {@link ApiException} instance. - * - * @param message the detail message. - * @param throwable the cause. - */ - public ApiException(String message, Throwable throwable) { - super(message, throwable); - } + /** + * Creates new {@link ApiException} instance. + * + * @param code the HTTP response status code. + * @param message the detail message. + */ + public ApiException(int code, String message) { + super(message); + this.code = code; + } - /** - * Creates new {@link ApiException} instance. - * - * @param code the HTTP response status code. - * @param message the detail message. - */ - public ApiException(int code, String message) { - super(message); - this.code = code; - } + /** + * Creates new {@link ApiException} instance. + * + * @param code the HTTP response status code. + * @param message the detail message. + * @param responseHeaders list of headers returned by the server. + * @param responseBody content of the response sent back by the server. + */ + public ApiException(int code, String message, Map> responseHeaders, String responseBody) { + this(code, message); + this.responseHeaders = responseHeaders; + this.responseBody = responseBody; + } - /** - * Creates new {@link ApiException} instance. - * - * @param code the HTTP response status code. - * @param message the detail message. - * @param responseHeaders list of headers returned by the server. - * @param responseBody content of the response sent back by the server. - */ - public ApiException(int code, String message, Map> responseHeaders, String responseBody) { - this(code, message); - this.responseHeaders = responseHeaders; - this.responseBody = responseBody; - } + /** + * Indicates if the error is not a fatal one and if the api call can be subsequently retried. + * + * @return true if it error code is 401 unauthorized or 429 too many requests or 5xx greater than 500. + */ + public boolean isMinorError() { + return isServerError() || isUnauthorized() || isTooManyRequestsError(); + } - /** - * Check if response status is unauthorized or not. - * - * @return true if response status is 401, false otherwise - */ - public boolean isUnauthorized() { - return this.code == HttpURLConnection.HTTP_UNAUTHORIZED; - } + /** + * Check if response status is unauthorized or not. + * + * @return true if response status is 401, false otherwise + */ + public boolean isUnauthorized() { + return this.code == HttpURLConnection.HTTP_UNAUTHORIZED; + } - /** - * Check if response status is client error or not - * - * @return true if response status is 400, false otherwise - */ - public boolean isClientError() { - return this.code == HttpURLConnection.HTTP_BAD_REQUEST; - } + /** + * Check if response status is client error or not + * + * @return true if response status is 400, false otherwise + */ + public boolean isClientError() { + return this.code == HttpURLConnection.HTTP_BAD_REQUEST; + } - /** - * Check if response status is server error and not an internal server error - * - * @return true if response status strictly greater than 500, false otherwise - */ - public boolean isServerError() { - return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; - } + /** + * Check if response status is a server error (5xx) but not an internal server error (500) + * + * @return true if response status strictly greater than 500, false otherwise + */ + public boolean isServerError() { + return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; + } - public boolean isTooManyRequestsError() { - return this.code == 429; - } + /** + * Check if response status corresponds to a too many requests error (429) + * + * @return true if error code is 429 + */ + public boolean isTooManyRequestsError() { + return this.code == 429; + } } diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java index 0d3a7132e..7eb64635b 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiRuntimeException.java @@ -3,7 +3,6 @@ import lombok.Getter; import org.apiguardian.api.API; -import java.util.Collections; import java.util.List; import java.util.Map; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java index f75799dde..116a508c1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/config/model/BdkConfig.java @@ -3,35 +3,48 @@ import lombok.Getter; import lombok.Setter; +/** + * Class holding the whole BDK configuration + */ @Getter @Setter public class BdkConfig { - private static final String DEFAULT_SCHEME = "https"; - private static final int DEFAULT_HTTPS_PORT = 443; - - private String scheme = DEFAULT_SCHEME; - private String host; - private Integer port = DEFAULT_HTTPS_PORT; - private String context = ""; - - private BdkClientConfig agent = new BdkClientConfig(); - private BdkClientConfig pod = new BdkClientConfig(); - private BdkClientConfig keyManager = new BdkClientConfig(); - private BdkClientConfig sessionAuth = new BdkClientConfig(); - - private BdkBotConfig bot = new BdkBotConfig(); - private BdkExtAppConfig app = new BdkExtAppConfig(); - private BdkSslConfig ssl = new BdkSslConfig(); - - private BdkRetryConfig retry = new BdkRetryConfig(); - private BdkDatafeedConfig datafeed = new BdkDatafeedConfig(); - - public boolean isOboConfigured() { - return app.isConfigured(); - } - - public BdkRetryConfig getDatafeedRetryConfig() { - return datafeed.getRetry() == null ? retry : datafeed.getRetry(); - } + private static final String DEFAULT_SCHEME = "https"; + private static final int DEFAULT_HTTPS_PORT = 443; + + private String scheme = DEFAULT_SCHEME; + private String host; + private Integer port = DEFAULT_HTTPS_PORT; + private String context = ""; + + private BdkClientConfig agent = new BdkClientConfig(); + private BdkClientConfig pod = new BdkClientConfig(); + private BdkClientConfig keyManager = new BdkClientConfig(); + private BdkClientConfig sessionAuth = new BdkClientConfig(); + + private BdkBotConfig bot = new BdkBotConfig(); + private BdkExtAppConfig app = new BdkExtAppConfig(); + private BdkSslConfig ssl = new BdkSslConfig(); + + private BdkRetryConfig retry = new BdkRetryConfig(); + private BdkDatafeedConfig datafeed = new BdkDatafeedConfig(); + + /** + * Check if OBO is configured. Checks {@link BdkExtAppConfig#isConfigured()} on field {@link #app}. + * + * @return true if OBO is configured. + */ + public boolean isOboConfigured() { + return app.isConfigured(); + } + + /** + * Returns the retry configuration used for DataFeed services. + * + * @return {@link BdkDatafeedConfig#getRetry()} from {@link #datafeed} if not null, {@link #retry} otherwise + */ + public BdkRetryConfig getDatafeedRetryConfig() { + return datafeed.getRetry() == null ? retry : datafeed.getRetry(); + } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java index 8b556a318..1c7d343af 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java @@ -1,8 +1,6 @@ package com.symphony.bdk.core.service; -import static com.symphony.bdk.core.util.function.SupplierWithApiException.callAndCatchApiException; - import com.symphony.bdk.core.api.invoker.util.ApiUtils; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/OboEnabled.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/OboEnabled.java deleted file mode 100644 index cb9fb13b7..000000000 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/OboEnabled.java +++ /dev/null @@ -1,16 +0,0 @@ -package com.symphony.bdk.core.service; - -import java.lang.annotation.ElementType; -import java.lang.annotation.Retention; -import java.lang.annotation.RetentionPolicy; -import java.lang.annotation.Target; - -/** - * Indicates a service method is called in OBO mode. - * @see Get Started With OBO - */ -@Retention(RetentionPolicy.RUNTIME) -@Target(ElementType.METHOD) -public @interface OboEnabled { - -} diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java index 6f2ca02f3..03b29fa52 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java @@ -1,5 +1,9 @@ package com.symphony.bdk.core.service.datafeed.exception; +/** + * Exception thrown when recovery strategy in a {@link com.symphony.bdk.core.util.function.RetryWithRecovery} failed. + * Especially used in DataFeed services. + */ public class NestedRetryException extends RuntimeException { public NestedRetryException(String message, Throwable cause) { super(message, cause); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index 5c157b89b..bb401ecc4 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -6,8 +6,6 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; -import com.symphony.bdk.core.util.function.Resilience4jRetryWithRecovery; -import com.symphony.bdk.core.util.function.ConsumerWithThrowable; import com.symphony.bdk.core.util.function.RetryWithRecovery; import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; @@ -15,12 +13,9 @@ import lombok.extern.slf4j.Slf4j; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Optional; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Predicate; /** * A class for implementing the datafeed v1 service. @@ -109,7 +104,7 @@ private void readDatafeed() throws Throwable { .name("Read Datafeed V1") .supplier(this::readAndHandleEvents) .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) - .retryOnException(RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedOrClientError) + .retryOnException(RetryWithRecoveryBuilder::isNetworkOrMinorErrorOrClientError) .build(); retry.execute(); } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index e52eeacc1..9286021de 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -5,8 +5,6 @@ import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; -import com.symphony.bdk.core.util.function.ConsumerWithThrowable; -import com.symphony.bdk.core.util.function.Resilience4jRetryWithRecovery; import com.symphony.bdk.core.util.function.RetryWithRecovery; import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; @@ -17,11 +15,8 @@ import lombok.extern.slf4j.Slf4j; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.function.Predicate; /** * A class for implementing the datafeed v2 service. @@ -132,7 +127,7 @@ private void readDatafeed() throws Throwable { final RetryWithRecovery retry = RetryWithRecoveryBuilder.from(retryWithRecoveryBuilder) .name("Read Datafeed V2") .supplier(this::readAndHandleEvents) - .retryOnException(RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedOrClientError) + .retryOnException(RetryWithRecoveryBuilder::isNetworkOrMinorErrorOrClientError) .recoveryStrategy(ApiException::isClientError, this::recreateDatafeed) .build(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java index fe704288e..d097e3d05 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java @@ -57,10 +57,7 @@ public Resilience4jRetryWithRecovery(String name, BdkRetryConfig bdkRetryConfig, } /** - * Executes the retry mechanism by calling the provided supplier, - * executing the potential matching recovery functions and retrying if needed. - * @return an object of param type T - * @throws Throwable + * {@inheritDoc} */ public T execute() throws Throwable { return this.retry.executeCheckedSupplier(this::executeOnce); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java index 42e9aea9b..631541ddf 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java @@ -12,12 +12,31 @@ import java.util.Map; import java.util.function.Predicate; +/** + * Abstract class to implement a retry mechanism with recovery strategies, + * e.g. refresh a session in a case of session expiration. + * @param the type of the object to be eventually returned by the {@link #supplier} + */ @Slf4j public abstract class RetryWithRecovery { private SupplierWithApiException supplier; private Predicate ignoreApiException; private Map, ConsumerWithThrowable> recoveryStrategies; + /** + * This is a helper function designed to cover most of the retry cases. + * It retries on the conditions defined by {@link RetryWithRecoveryBuilder#isNetworkOrMinorError} + * and refreshes the authSession if we get an unauthorized error. + * + * @param name the name of the retry, can be any string but should specific to the function neing retried. + * @param supplier the supplier returning the desired object which may fail with an exception. + * @param retryConfig the retry configuration to be used. So far, only exponentional backoff is supported. + * @param authSession the session to the refreshed on unauthorized errors. + * @param the type of the object to be returned by the supplier. + * @return the object returned by the supplier + * @throws ApiRuntimeException if a non-handled {@link ApiException} thrown or if the max number of retries has been reached. + * @throws RuntimeException if any other exception thrown. + */ public static T executeAndRetry(String name, SupplierWithApiException supplier, BdkRetryConfig retryConfig, AuthSession authSession) { RetryWithRecovery retry = new RetryWithRecoveryBuilder() @@ -48,11 +67,22 @@ public RetryWithRecovery(SupplierWithApiException supplier, Predicate the type to be returned by {@link RetryWithRecovery#execute()}. + */ public class RetryWithRecoveryBuilder { private String name; private BdkRetryConfig retryConfig; @@ -17,6 +22,13 @@ public class RetryWithRecoveryBuilder { private Predicate ignoreException; private Map, ConsumerWithThrowable> recoveryStrategies; + /** + * Copies all fields of an existing builder except the {@link #supplier}. + * + * @param from the {@link RetryWithRecovery} to be copied. + * @param the target parametrized type. + * @return a copy of the builder passed as parameter. + */ public static RetryWithRecoveryBuilder from(RetryWithRecoveryBuilder from) { RetryWithRecoveryBuilder copy = new RetryWithRecoveryBuilder(); copy.name = from.name; @@ -28,60 +40,123 @@ public static RetryWithRecoveryBuilder from(RetryWithRecoveryBuilder f return copy; } - //TODO rename - public static boolean isNetworkOrServerOrUnauthorizedError(Throwable t) { + /** + * Checks if a throwable is a {@link ProcessingException} or a {@link ApiException} minor error. + * This is the default function used in {@link RetryWithRecovery} + * to check if a given exception thrown should lead to a retry. + * + * @param t the throwable to be checked. + * @return true if passed throwable is a {@link ProcessingException} (e.g. in case of a temporary network exception) + * or if it is a {@link ApiException} which {@link ApiException#isMinorError()}. + */ + public static boolean isNetworkOrMinorError(Throwable t) { if (t instanceof ApiException) { - ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError(); + return ((ApiException) t).isMinorError(); } return t instanceof ProcessingException; } - //TODO rename - public static boolean isNetworkOrServerOrUnauthorizedOrClientError(Throwable t) { + /** + * Checks if a throwable is a {@link ProcessingException} or a {@link ApiException} minor error or client error. + * + * @param t the throwable to be checked. + * @return true if passed throwable is a {@link ProcessingException} (e.g. in case of a temporary network exception) + * or if it is a {@link ApiException} which {@link ApiException#isMinorError()} or {@link ApiException#isClientError()}. + */ + public static boolean isNetworkOrMinorErrorOrClientError(Throwable t) { if (t instanceof ApiException) { ApiException apiException = (ApiException) t; - return apiException.isServerError() || apiException.isUnauthorized() || apiException.isTooManyRequestsError() || apiException.isClientError(); + return apiException.isMinorError() || apiException.isClientError(); } return t instanceof ProcessingException; } + /** + * Default constructor which ignores no exception + * and retries exceptions fulfilling {@link RetryWithRecoveryBuilder#isNetworkOrMinorError}. + */ public RetryWithRecoveryBuilder() { this.ignoreException = (e) -> false; - this.retryOnExceptionPredicate = RetryWithRecoveryBuilder::isNetworkOrServerOrUnauthorizedError; + this.retryOnExceptionPredicate = RetryWithRecoveryBuilder::isNetworkOrMinorError; this.recoveryStrategies = new HashMap<>(); } + /** + * Sets the name and returns the modified builder. + * + * @param name the name of the {@link RetryWithRecovery} + * @return the modified builder instance. + */ public RetryWithRecoveryBuilder name(String name) { this.name = name; return this; } + /** + * Sets the retry configuration and returns the modified builder. + * + * @param retryConfig the retry configuration to be used. + * @return the modified builder instance. + */ public RetryWithRecoveryBuilder retryConfig(BdkRetryConfig retryConfig) { this.retryConfig = retryConfig; return this; } + /** + * Sets the retry configuration and returns the modified builder. + * + * @param supplier the function to be called by the {@link RetryWithRecovery} + * which returns the desired object and which may fail. + * @return the modified builder instance. + */ public RetryWithRecoveryBuilder supplier(SupplierWithApiException supplier) { this.supplier = supplier; return this; } + /** + * Sets the conditions on which we should retry the call to the provided {@link #supplier}. + * + * @param retryOnExceptionPredicate the condition when we should retry the call + * when the {@link #supplier} throws an exception. + * @return the modified builder instance. + */ public RetryWithRecoveryBuilder retryOnException(Predicate retryOnExceptionPredicate) { this.retryOnExceptionPredicate = retryOnExceptionPredicate; return this; } + /** + * Sets the condition on which we should ignore an exception thrown by the {@link #supplier} + * and return null in {@link RetryWithRecovery#execute()}. + * + * @param ignoreException the condition when we should ignore a given exception + * @return the modified builder instance. + */ public RetryWithRecoveryBuilder ignoreException(Predicate ignoreException) { this.ignoreException = ignoreException; return this; } + /** + * Sets one recovery strategy which consists in a predicate on the thrown {@link ApiException} + * and in a corresponding recovery function to be executed when condition is met. + * + * @param condition the predicate to check if the exception should lead to the execution of the recovery function. + * @param recovery the recovery function to be executed when condition is fulfilled. + * @return + */ public RetryWithRecoveryBuilder recoveryStrategy(Predicate condition, ConsumerWithThrowable recovery) { this.recoveryStrategies.put(condition, recovery); return this; } + /** + * Builds a {@link RetryWithRecovery} based on provided fields. + * + * @return a new instance of {@link RetryWithRecovery} based on the provided fields. + */ public RetryWithRecovery build() { return new Resilience4jRetryWithRecovery<>(name, retryConfig, supplier, retryOnExceptionPredicate, ignoreException, recoveryStrategies); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java index 575cc60f2..585ca2569 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/SupplierWithApiException.java @@ -7,24 +7,11 @@ /** * Functional interface which supplies a T object and may throw an {@link ApiException}. + * * @param the type returned by the supplier. */ @FunctionalInterface @API(status = API.Status.INTERNAL) public interface SupplierWithApiException { T get() throws ApiException; - - /** - * Method which wraps {@link #get()} and throws an {@link ApiRuntimeException} if {@link ApiException} is thrown. - * @param supplier the supplier - * @param the type returned by the supplier - * @return the value returned by the supplier - */ - static T callAndCatchApiException(SupplierWithApiException supplier) { - try { - return supplier.get(); - } catch (ApiException e) { - throw new ApiRuntimeException(e); - } - } } From 0b5c6654b56b77e875691f9898320ad7d1603b4d Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Mon, 21 Sep 2020 18:00:27 +0200 Subject: [PATCH 13/17] Moved retry classes into a retry module --- .../src/main/java/com/symphony/bdk/core/SymphonyBdk.java | 3 +-- .../core/{util/function => retry}/RetryWithRecovery.java | 7 ++++++- .../function => retry}/RetryWithRecoveryBuilder.java | 8 +++++++- .../resilience4j}/Resilience4jRetryWithRecovery.java | 8 +++++++- .../com/symphony/bdk/core/service/MessageService.java | 2 +- .../com/symphony/bdk/core/service/SessionService.java | 4 +--- .../service/datafeed/exception/NestedRetryException.java | 4 +++- .../service/datafeed/impl/AbstractDatafeedService.java | 2 +- .../bdk/core/service/datafeed/impl/DatafeedServiceV1.java | 4 ++-- .../bdk/core/service/datafeed/impl/DatafeedServiceV2.java | 4 ++-- .../bdk/core/service/stream/OboStreamService.java | 6 +----- .../symphony/bdk/core/service/user/OboUserService.java | 2 +- .../bdk/core/util/function/ConsumerWithThrowable.java | 3 +++ .../resilience4j}/Resilience4jRetryWithRecoveryTest.java | 4 +++- 14 files changed, 39 insertions(+), 22 deletions(-) rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/{util/function => retry}/RetryWithRecovery.java (94%) rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/{util/function => retry}/RetryWithRecoveryBuilder.java (95%) rename symphony-bdk-core/src/main/java/com/symphony/bdk/core/{util/function => retry/resilience4j}/Resilience4jRetryWithRecovery.java (93%) rename symphony-bdk-core/src/test/java/com/symphony/bdk/core/{util/function => retry/resilience4j}/Resilience4jRetryWithRecoveryTest.java (97%) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java index 7c40aa58f..1c788fecf 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/SymphonyBdk.java @@ -48,8 +48,7 @@ protected SymphonyBdk(BdkConfig config, ApiClientFactory apiClientFactory) AuthSession botSession = authenticatorFactory.getBotAuthenticator().authenticateBot(); this.oboAuthenticator = config.isOboConfigured() ? authenticatorFactory.getOboAuthenticator() : null; this.extensionAppAuthenticator = config.isOboConfigured() ? authenticatorFactory.getExtensionAppAuthenticator() : null; - ServiceFactory serviceFactory = - new ServiceFactory(apiClientFactory, botSession, config); + ServiceFactory serviceFactory = new ServiceFactory(apiClientFactory, botSession, config); // service init this.sessionService = serviceFactory.getSessionService(); this.userService = serviceFactory.getUserService(); diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java similarity index 94% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java index 631541ddf..009bd9f70 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java @@ -1,4 +1,4 @@ -package com.symphony.bdk.core.util.function; +package com.symphony.bdk.core.retry; import com.symphony.bdk.core.api.invoker.ApiException; @@ -7,7 +7,11 @@ import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.SupplierWithApiException; + import lombok.extern.slf4j.Slf4j; +import org.apiguardian.api.API; import java.util.Map; import java.util.function.Predicate; @@ -18,6 +22,7 @@ * @param the type of the object to be eventually returned by the {@link #supplier} */ @Slf4j +@API(status = API.Status.INTERNAL) public abstract class RetryWithRecovery { private SupplierWithApiException supplier; private Predicate ignoreApiException; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java similarity index 95% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java index 59ea345fa..8cfe2aac1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/RetryWithRecoveryBuilder.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java @@ -1,7 +1,12 @@ -package com.symphony.bdk.core.util.function; +package com.symphony.bdk.core.retry; import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.resilience4j.Resilience4jRetryWithRecovery; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.SupplierWithApiException; + +import org.apiguardian.api.API; import javax.ws.rs.ProcessingException; @@ -14,6 +19,7 @@ * * @param the type to be returned by {@link RetryWithRecovery#execute()}. */ +@API(status = API.Status.INTERNAL) public class RetryWithRecoveryBuilder { private String name; private BdkRetryConfig retryConfig; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecovery.java similarity index 93% rename from symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java rename to symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecovery.java index d097e3d05..199518183 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecovery.java @@ -1,13 +1,18 @@ -package com.symphony.bdk.core.util.function; +package com.symphony.bdk.core.retry.resilience4j; import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.core.util.BdkExponentialFunction; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.SupplierWithApiException; + import io.github.resilience4j.retry.Retry; import io.github.resilience4j.retry.RetryConfig; import lombok.extern.slf4j.Slf4j; +import org.apiguardian.api.API; import java.util.Map; import java.util.function.Predicate; @@ -18,6 +23,7 @@ * @param the type of the object returned by {@link #execute()} */ @Slf4j +@API(status = API.Status.INTERNAL) public class Resilience4jRetryWithRecovery extends RetryWithRecovery { private final Retry retry; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java index 1c7d343af..eae600cf1 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java @@ -7,7 +7,7 @@ import com.symphony.bdk.core.service.pagination.PaginatedApi; import com.symphony.bdk.core.service.pagination.PaginatedService; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; -import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.AttachmentsApi; import com.symphony.bdk.gen.api.DefaultApi; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java index 178b6943f..44bac0d87 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java @@ -1,10 +1,8 @@ package com.symphony.bdk.core.service; -import com.symphony.bdk.core.api.invoker.ApiException; -import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; -import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.gen.api.SessionApi; import com.symphony.bdk.gen.api.model.UserV2; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java index 03b29fa52..2924d33b3 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java @@ -1,7 +1,9 @@ package com.symphony.bdk.core.service.datafeed.exception; +import com.symphony.bdk.core.retry.RetryWithRecovery; + /** - * Exception thrown when recovery strategy in a {@link com.symphony.bdk.core.util.function.RetryWithRecovery} failed. + * Exception thrown when recovery strategy in a {@link RetryWithRecovery} failed. * Especially used in DataFeed services. */ public class NestedRetryException extends RuntimeException { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java index 35d52df6e..200f89fe8 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/AbstractDatafeedService.java @@ -6,7 +6,7 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedService; import com.symphony.bdk.core.service.datafeed.RealTimeEventListener; -import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java index bb401ecc4..4a3b45abe 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV1.java @@ -6,8 +6,8 @@ import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.DatafeedIdRepository; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; -import com.symphony.bdk.core.util.function.RetryWithRecovery; -import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.V4Event; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java index 9286021de..d124135d9 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/impl/DatafeedServiceV2.java @@ -5,8 +5,8 @@ import com.symphony.bdk.core.auth.exception.AuthUnauthorizedException; import com.symphony.bdk.core.config.model.BdkConfig; import com.symphony.bdk.core.service.datafeed.exception.NestedRetryException; -import com.symphony.bdk.core.util.function.RetryWithRecovery; -import com.symphony.bdk.core.util.function.RetryWithRecoveryBuilder; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.DatafeedApi; import com.symphony.bdk.gen.api.model.AckId; import com.symphony.bdk.gen.api.model.V4Event; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java index 723a0bc1d..4d609fc34 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java @@ -1,18 +1,14 @@ package com.symphony.bdk.core.service.stream; -import com.symphony.bdk.core.api.invoker.ApiException; -import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; -import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.StreamsApi; import com.symphony.bdk.gen.api.model.StreamAttributes; import com.symphony.bdk.gen.api.model.StreamFilter; import com.symphony.bdk.gen.api.model.V2StreamAttributes; -import io.github.resilience4j.retry.RetryConfig; - import java.util.List; class OboStreamService { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java index 9419fbfba..28e5bdcba 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java @@ -2,7 +2,7 @@ import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; -import com.symphony.bdk.core.util.function.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.UserApi; import com.symphony.bdk.gen.api.UsersApi; diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java index b1080f74d..e5215c9ef 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/util/function/ConsumerWithThrowable.java @@ -2,11 +2,14 @@ import com.symphony.bdk.core.api.invoker.ApiException; +import org.apiguardian.api.API; + /** * Functional interface which consumes an {@link ApiException} and may throw a {@link Throwable}. * This is used to specify recovery functions in {@link ConsumerWithThrowable}. */ @FunctionalInterface +@API(status = API.Status.INTERNAL) public interface ConsumerWithThrowable { void consume() throws Throwable; } diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecoveryTest.java similarity index 97% rename from symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java rename to symphony-bdk-core/src/test/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecoveryTest.java index cd8e2a616..68d45c00c 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/util/function/Resilience4jRetryWithRecoveryTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/retry/resilience4j/Resilience4jRetryWithRecoveryTest.java @@ -1,4 +1,4 @@ -package com.symphony.bdk.core.util.function; +package com.symphony.bdk.core.retry.resilience4j; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; @@ -14,6 +14,8 @@ import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.util.function.ConsumerWithThrowable; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import org.junit.jupiter.api.Test; import org.mockito.InOrder; From 8e7094901f4fc610251f428b34619d53a9b6e65a Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Tue, 22 Sep 2020 09:23:59 +0200 Subject: [PATCH 14/17] Passed retry builder instead of retry config in service classes --- .../com/symphony/bdk/core/ServiceFactory.java | 19 +++++++++++----- .../bdk/core/retry/RetryWithRecovery.java | 10 +++------ .../core/retry/RetryWithRecoveryBuilder.java | 10 +++++++-- .../bdk/core/service/MessageService.java | 22 +++++++++---------- .../bdk/core/service/SessionService.java | 16 +++++++++++--- .../core/service/stream/OboStreamService.java | 13 +++++++---- .../core/service/stream/StreamService.java | 8 ++++--- .../bdk/core/service/user/OboUserService.java | 14 +++++++----- .../bdk/core/service/user/UserService.java | 9 ++++---- .../bdk/core/service/MessageServiceTest.java | 3 ++- .../bdk/core/service/SessionServiceTest.java | 3 ++- .../service/stream/StreamServiceTest.java | 3 ++- .../core/service/user/UserServiceTest.java | 3 ++- .../spring/config/BdkApiClientsConfig.java | 2 ++ .../bdk/spring/config/BdkServiceConfig.java | 8 ++++++- 15 files changed, 92 insertions(+), 51 deletions(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java index 2c0983d4e..9ee324a8d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java @@ -1,9 +1,12 @@ package com.symphony.bdk.core; import com.symphony.bdk.core.api.invoker.ApiClient; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.client.ApiClientFactory; import com.symphony.bdk.core.config.model.BdkConfig; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.MessageService; import com.symphony.bdk.core.service.SessionService; import com.symphony.bdk.core.service.datafeed.DatafeedService; @@ -44,12 +47,16 @@ class ServiceFactory { private final ApiClient agentClient; private final AuthSession authSession; private final BdkConfig config; + private final RetryWithRecoveryBuilder retryBuilder; public ServiceFactory(ApiClientFactory apiClientFactory, AuthSession authSession, BdkConfig config) { this.podClient = apiClientFactory.getPodClient(); this.agentClient = apiClientFactory.getAgentClient(); this.authSession = authSession; this.config = config; + this.retryBuilder = new RetryWithRecoveryBuilder<>() + .retryConfig(config.getRetry()) + .recoveryStrategy(ApiException::isUnauthorized, authSession::refresh); } /** @@ -58,7 +65,7 @@ public ServiceFactory(ApiClientFactory apiClientFactory, AuthSession authSession * @return an new {@link UserService} instance. */ public UserService getUserService() { - return new UserService(new UserApi(podClient), new UsersApi(podClient), authSession, config.getRetry()); + return new UserService(new UserApi(podClient), new UsersApi(podClient), authSession, retryBuilder); } /** @@ -67,7 +74,7 @@ public UserService getUserService() { * @return an new {@link StreamService} instance. */ public StreamService getStreamService() { - return new StreamService(new StreamsApi(podClient), authSession, config.getRetry()); + return new StreamService(new StreamsApi(podClient), authSession, retryBuilder); } /** @@ -76,7 +83,7 @@ public StreamService getStreamService() { * @return an new {@link SessionService} instance. */ public SessionService getSessionService() { - return new SessionService(new SessionApi(podClient), config.getRetry()); + return new SessionService(new SessionApi(podClient), retryBuilder); } /** @@ -97,8 +104,8 @@ public DatafeedService getDatafeedService() { * @return an new {@link MessageService} instance. */ public MessageService getMessageService() { - return new MessageService(new MessagesApi(this.agentClient), new MessageApi(this.podClient), - new MessageSuppressionApi(this.podClient), new StreamsApi(this.podClient), new PodApi(this.podClient), - new AttachmentsApi(this.agentClient), new DefaultApi(this.podClient), this.authSession, this.config.getRetry()); + return new MessageService(new MessagesApi(agentClient), new MessageApi(podClient), + new MessageSuppressionApi(podClient), new StreamsApi(podClient), new PodApi(podClient), + new AttachmentsApi(agentClient), new DefaultApi(podClient), authSession, retryBuilder); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java index 009bd9f70..85966098d 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecovery.java @@ -33,22 +33,18 @@ public abstract class RetryWithRecovery { * It retries on the conditions defined by {@link RetryWithRecoveryBuilder#isNetworkOrMinorError} * and refreshes the authSession if we get an unauthorized error. * + * @param baseRetryBuilder the {@link RetryWithRecoveryBuilder} containing the base settings for the retry mechanism. * @param name the name of the retry, can be any string but should specific to the function neing retried. * @param supplier the supplier returning the desired object which may fail with an exception. - * @param retryConfig the retry configuration to be used. So far, only exponentional backoff is supported. - * @param authSession the session to the refreshed on unauthorized errors. * @param the type of the object to be returned by the supplier. * @return the object returned by the supplier * @throws ApiRuntimeException if a non-handled {@link ApiException} thrown or if the max number of retries has been reached. * @throws RuntimeException if any other exception thrown. */ - public static T executeAndRetry(String name, SupplierWithApiException supplier, BdkRetryConfig retryConfig, - AuthSession authSession) { - RetryWithRecovery retry = new RetryWithRecoveryBuilder() + public static T executeAndRetry(RetryWithRecoveryBuilder baseRetryBuilder, String name, SupplierWithApiException supplier) { + RetryWithRecovery retry = RetryWithRecoveryBuilder.from(baseRetryBuilder) .name(name) .supplier(supplier) - .retryConfig(retryConfig) - .recoveryStrategy(e -> e.isUnauthorized(), () -> authSession.refresh()) .build(); try { diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java index 8cfe2aac1..718bb2bbb 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/retry/RetryWithRecoveryBuilder.java @@ -82,9 +82,10 @@ public static boolean isNetworkOrMinorErrorOrClientError(Throwable t) { * and retries exceptions fulfilling {@link RetryWithRecoveryBuilder#isNetworkOrMinorError}. */ public RetryWithRecoveryBuilder() { - this.ignoreException = (e) -> false; - this.retryOnExceptionPredicate = RetryWithRecoveryBuilder::isNetworkOrMinorError; this.recoveryStrategies = new HashMap<>(); + this.ignoreException = e -> false; + this.retryOnExceptionPredicate = RetryWithRecoveryBuilder::isNetworkOrMinorError; + this.retryConfig = new BdkRetryConfig(); } /** @@ -158,6 +159,11 @@ public RetryWithRecoveryBuilder recoveryStrategy(Predicate cond return this; } + public RetryWithRecoveryBuilder clearRecoveryStrategies() { + this.recoveryStrategies.clear(); + return this; + } + /** * Builds a {@link RetryWithRecovery} based on provided fields. * diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java index eae600cf1..1dc3df656 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/MessageService.java @@ -3,11 +3,11 @@ import com.symphony.bdk.core.api.invoker.util.ApiUtils; import com.symphony.bdk.core.auth.AuthSession; -import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.pagination.PaginatedApi; import com.symphony.bdk.core.service.pagination.PaginatedService; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; -import com.symphony.bdk.core.retry.RetryWithRecovery; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.AttachmentsApi; import com.symphony.bdk.gen.api.DefaultApi; @@ -56,27 +56,25 @@ public class MessageService { private final DefaultApi defaultApi; private final AuthSession authSession; private final TemplateResolver templateResolver; - private final BdkRetryConfig retryConfig; + private final RetryWithRecoveryBuilder retryBuilder; public MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, - AuthSession authSession, BdkRetryConfig retryConfig) { + AuthSession authSession, RetryWithRecoveryBuilder retryBuilder) { this(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, defaultApi, authSession, - new TemplateResolver(), retryConfig); + new TemplateResolver(), retryBuilder); } public MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, - AuthSession authSession, - TemplateEngine templateEngine, BdkRetryConfig retryConfig) { + AuthSession authSession, TemplateEngine templateEngine, RetryWithRecoveryBuilder retryBuilder) { this(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, defaultApi, authSession, - new TemplateResolver(templateEngine), retryConfig); + new TemplateResolver(templateEngine), retryBuilder); } private MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, - AuthSession authSession, - TemplateResolver templateResolver, BdkRetryConfig retryConfig) { + AuthSession authSession, TemplateResolver templateResolver, RetryWithRecoveryBuilder retryBuilder) { this.messagesApi = messagesApi; this.messageApi = messageApi; this.messageSuppressionApi = messageSuppressionApi; @@ -86,7 +84,7 @@ private MessageService(MessagesApi messagesApi, MessageApi messageApi, MessageSu this.authSession = authSession; this.templateResolver = templateResolver; this.defaultApi = defaultApi; - this.retryConfig = retryConfig; + this.retryBuilder = retryBuilder; } /** @@ -380,6 +378,6 @@ private static Long getEpochMillis(Instant instant) { } private T executeAndRetry(String name, SupplierWithApiException supplier) { - return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); + return RetryWithRecovery.executeAndRetry(retryBuilder, name, supplier); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java index 44bac0d87..bdb65bac6 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/SessionService.java @@ -1,8 +1,11 @@ package com.symphony.bdk.core.service; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; +import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.SessionApi; import com.symphony.bdk.gen.api.model.UserV2; @@ -17,7 +20,7 @@ public class SessionService { private final SessionApi sessionApi; - private final BdkRetryConfig retryConfig; + private final RetryWithRecoveryBuilder retryBuilder; /** * Retrieves the {@link UserV2} session from the pod using an {@link AuthSession} holder. @@ -26,7 +29,14 @@ public class SessionService { * @return Bot session info. */ public UserV2 getSession(AuthSession authSession) { - return RetryWithRecovery.executeAndRetry("getSession", - () -> sessionApi.v2SessioninfoGet(authSession.getSessionToken()), retryConfig, authSession); + return executeAndRetry("getSession", + () -> sessionApi.v2SessioninfoGet(authSession.getSessionToken()), authSession); + } + + protected T executeAndRetry(String name, SupplierWithApiException supplier, AuthSession authSession) { + final RetryWithRecoveryBuilder retryBuilderWithAuthSession = RetryWithRecoveryBuilder.from(retryBuilder) + .clearRecoveryStrategies() // to remove refresh on bot session put by default + .recoveryStrategy(ApiException::isUnauthorized, authSession::refresh); + return RetryWithRecovery.executeAndRetry(retryBuilderWithAuthSession, name, supplier); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java index 4d609fc34..393258b45 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/OboStreamService.java @@ -1,8 +1,10 @@ package com.symphony.bdk.core.service.stream; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.StreamsApi; import com.symphony.bdk.gen.api.model.StreamAttributes; @@ -14,11 +16,11 @@ class OboStreamService { protected final StreamsApi streamsApi; - protected final BdkRetryConfig retryConfig; + protected final RetryWithRecoveryBuilder retryBuilder; - protected OboStreamService(StreamsApi streamsApi, BdkRetryConfig retryConfig) { + protected OboStreamService(StreamsApi streamsApi, RetryWithRecoveryBuilder retryBuilder) { this.streamsApi = streamsApi; - this.retryConfig = retryConfig; + this.retryBuilder = retryBuilder; } /** @@ -48,6 +50,9 @@ public List listStreams(AuthSession authSession, StreamFilter } protected T executeAndRetry(String name, SupplierWithApiException supplier, AuthSession authSession) { - return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); + final RetryWithRecoveryBuilder retryBuilderWithAuthSession = RetryWithRecoveryBuilder.from(retryBuilder) + .clearRecoveryStrategies() // to remove refresh on bot session put by default + .recoveryStrategy(ApiException::isUnauthorized, authSession::refresh); + return RetryWithRecovery.executeAndRetry(retryBuilderWithAuthSession, name, supplier); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java index 783987a17..806a6e6a4 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/stream/StreamService.java @@ -4,6 +4,8 @@ import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.StreamsApi; @@ -44,8 +46,8 @@ public class StreamService extends OboStreamService { private final AuthSession authSession; - public StreamService(StreamsApi streamsApi, AuthSession authSession, BdkRetryConfig retryConfig) { - super(streamsApi, retryConfig); + public StreamService(StreamsApi streamsApi, AuthSession authSession, RetryWithRecoveryBuilder retryBuilder) { + super(streamsApi, retryBuilder); this.authSession = authSession; } @@ -247,6 +249,6 @@ public V2MembershipList listStreamMembers(String streamId) { } private T executeAndRetry(String name, SupplierWithApiException supplier) { - return executeAndRetry(name, supplier, authSession); + return RetryWithRecovery.executeAndRetry(retryBuilder, name, supplier); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java index 28e5bdcba..d48d6de1e 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/OboUserService.java @@ -1,8 +1,9 @@ package com.symphony.bdk.core.service.user; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; -import com.symphony.bdk.core.config.model.BdkRetryConfig; import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.util.function.SupplierWithApiException; import com.symphony.bdk.gen.api.UserApi; import com.symphony.bdk.gen.api.UsersApi; @@ -22,12 +23,12 @@ class OboUserService { protected final UserApi userApi; protected final UsersApi usersApi; - protected final BdkRetryConfig retryConfig; + protected final RetryWithRecoveryBuilder retryBuilder; - protected OboUserService(UserApi userApi, UsersApi usersApi, BdkRetryConfig retryConfig) { + protected OboUserService(UserApi userApi, UsersApi usersApi, RetryWithRecoveryBuilder retryBuilder) { this.userApi = userApi; this.usersApi = usersApi; - this.retryConfig = retryConfig; + this.retryBuilder = retryBuilder; } /** @@ -132,6 +133,9 @@ public List searchUserBySearchQuery(@NonNull AuthSession authSession, @N } protected T executeAndRetry(String name, SupplierWithApiException supplier, AuthSession authSession) { - return RetryWithRecovery.executeAndRetry(name, supplier, retryConfig, authSession); + final RetryWithRecoveryBuilder retryBuilderWithAuthSession = RetryWithRecoveryBuilder.from(retryBuilder) + .clearRecoveryStrategies() // to remove refresh on bot session put by default + .recoveryStrategy(ApiException::isUnauthorized, authSession::refresh); + return RetryWithRecovery.executeAndRetry(retryBuilderWithAuthSession, name, supplier); } } diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java index de27322a2..fb8e88943 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/user/UserService.java @@ -1,7 +1,8 @@ package com.symphony.bdk.core.service.user; import com.symphony.bdk.core.auth.AuthSession; -import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecovery; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.user.constant.RoleId; import com.symphony.bdk.core.service.user.mapper.UserDetailMapper; import com.symphony.bdk.core.util.function.SupplierWithApiException; @@ -52,8 +53,8 @@ public class UserService extends OboUserService { private final AuthSession authSession; - public UserService(UserApi userApi, UsersApi usersApi, AuthSession authSession, BdkRetryConfig retryConfig) { - super(userApi, usersApi, retryConfig); + public UserService(UserApi userApi, UsersApi usersApi, AuthSession authSession, RetryWithRecoveryBuilder retryBuilder) { + super(userApi, usersApi, retryBuilder); this.authSession = authSession; } @@ -358,6 +359,6 @@ public List searchUserBySearchQuery(@NonNull UserSearchQuery query, @Nul } private T executeAndRetry(String name, SupplierWithApiException supplier) { - return executeAndRetry(name, supplier, authSession); + return RetryWithRecovery.executeAndRetry(retryBuilder, name, supplier); } } diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java index eef4e2006..44c66ccde 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/MessageServiceTest.java @@ -19,6 +19,7 @@ import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; import com.symphony.bdk.core.test.JsonHelper; import com.symphony.bdk.core.test.MockApiClient; @@ -95,7 +96,7 @@ void setUp() { messageService = new MessageService(new MessagesApi(agentClient), new MessageApi(podClient), new MessageSuppressionApi(podClient), streamsApi, new PodApi(podClient), - attachmentsApi, new DefaultApi(podClient), authSession, templateEngine, new BdkRetryConfig()); + attachmentsApi, new DefaultApi(podClient), authSession, templateEngine, new RetryWithRecoveryBuilder()); } @Test diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java index c77155a02..5e2ffc614 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/SessionServiceTest.java @@ -9,6 +9,7 @@ import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.SessionApi; import com.symphony.bdk.gen.api.model.UserV2; @@ -41,7 +42,7 @@ void setUp() { retryConfig.setMultiplier(1); retryConfig.setInitialIntervalMillis(10); - this.service = new SessionService(this.sessionApi, retryConfig); + this.service = new SessionService(this.sessionApi, new RetryWithRecoveryBuilder().retryConfig(retryConfig)); } @Test diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java index ee1864ca6..8c0ee20bc 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/stream/StreamServiceTest.java @@ -9,6 +9,7 @@ import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.stream.constant.AttachmentSort; import com.symphony.bdk.core.test.MockApiClient; import com.symphony.bdk.core.test.JsonHelper; @@ -59,7 +60,7 @@ void setUp() { this.mockApiClient = new MockApiClient(); AuthSession authSession = mock(AuthSession.class); ApiClient podClient = mockApiClient.getApiClient("/pod"); - this.service = new StreamService(new StreamsApi(podClient), authSession, new BdkRetryConfig()); + this.service = new StreamService(new StreamsApi(podClient), authSession, new RetryWithRecoveryBuilder<>()); when(authSession.getSessionToken()).thenReturn("1234"); when(authSession.getKeyManagerToken()).thenReturn("1234"); diff --git a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java index 49b454064..411155b87 100644 --- a/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java +++ b/symphony-bdk-core/src/test/java/com/symphony/bdk/core/service/user/UserServiceTest.java @@ -15,6 +15,7 @@ import com.symphony.bdk.core.api.invoker.ApiRuntimeException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkRetryConfig; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.user.constant.RoleId; import com.symphony.bdk.core.service.user.constant.UserFeature; import com.symphony.bdk.core.service.user.mapper.UserDetailMapper; @@ -81,7 +82,7 @@ void init() { this.spiedUserApi = spy(userApi); UsersApi usersApi = new UsersApi(podClient); this.spiedUsersApi = spy(usersApi); - this.service = new UserService(this.spiedUserApi, this.spiedUsersApi, authSession, new BdkRetryConfig()); + this.service = new UserService(this.spiedUserApi, this.spiedUsersApi, authSession, new RetryWithRecoveryBuilder()); when(authSession.getSessionToken()).thenReturn("1234"); when(authSession.getKeyManagerToken()).thenReturn("1234"); diff --git a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkApiClientsConfig.java b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkApiClientsConfig.java index aef730d35..897881d68 100644 --- a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkApiClientsConfig.java +++ b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkApiClientsConfig.java @@ -1,8 +1,10 @@ package com.symphony.bdk.spring.config; import com.symphony.bdk.core.api.invoker.ApiClient; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.client.ApiClientFactory; import com.symphony.bdk.core.client.exception.ApiClientInitializationException; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.gen.api.AppEntitlementApi; import com.symphony.bdk.gen.api.ApplicationApi; import com.symphony.bdk.gen.api.AttachmentsApi; diff --git a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java index 82241c3d8..778988c3f 100644 --- a/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java +++ b/symphony-bdk-spring/bdk-core-starter/src/main/java/com/symphony/bdk/spring/config/BdkServiceConfig.java @@ -1,7 +1,9 @@ package com.symphony.bdk.spring.config; +import com.symphony.bdk.core.api.invoker.ApiException; import com.symphony.bdk.core.auth.AuthSession; import com.symphony.bdk.core.config.model.BdkConfig; +import com.symphony.bdk.core.retry.RetryWithRecoveryBuilder; import com.symphony.bdk.core.service.MessageService; import com.symphony.bdk.gen.api.AttachmentsApi; import com.symphony.bdk.gen.api.DefaultApi; @@ -29,7 +31,11 @@ public class BdkServiceConfig { public MessageService messageService(MessagesApi messagesApi, MessageApi messageApi, MessageSuppressionApi messageSuppressionApi, StreamsApi streamsApi, PodApi podApi, AttachmentsApi attachmentsApi, DefaultApi defaultApi, AuthSession botSession, BdkConfig config) { + final RetryWithRecoveryBuilder retryWithRecoveryBuilder = new RetryWithRecoveryBuilder<>() + .retryConfig(config.getRetry()) + .recoveryStrategy(ApiException::isUnauthorized, botSession::refresh); + return new MessageService(messagesApi, messageApi, messageSuppressionApi, streamsApi, podApi, attachmentsApi, - defaultApi, botSession, config.getRetry()); + defaultApi, botSession, retryWithRecoveryBuilder); } } From 3eaf6c270056b7179caffb05e4df3f57c667458c Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Tue, 22 Sep 2020 10:14:03 +0200 Subject: [PATCH 15/17] Reverted space changes in ApiException --- .../bdk/core/api/invoker/ApiException.java | 155 +++++++++--------- 1 file changed, 76 insertions(+), 79 deletions(-) diff --git a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java index 023f41da2..65c5b41a8 100644 --- a/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java +++ b/symphony-bdk-core-invokers/symphony-bdk-core-invoker-api/src/main/java/com/symphony/bdk/core/api/invoker/ApiException.java @@ -3,104 +3,101 @@ import lombok.Getter; import org.apiguardian.api.API; -import javax.net.ssl.HttpsURLConnection; import javax.ws.rs.core.GenericType; - import java.net.HttpURLConnection; -import java.util.Arrays; import java.util.List; import java.util.Map; /** * Main exception raised when invoking {@link ApiClient#invokeAPI(String, String, List, Object, Map, Map, Map, String, String, String[], GenericType)}. - *

+ * * Initially generated by the OpenAPI Maven Generator */ @Getter @API(status = API.Status.STABLE) public class ApiException extends Exception { - private int code = 0; - private Map> responseHeaders = null; - private String responseBody = null; + private int code = 0; + private Map> responseHeaders = null; + private String responseBody = null; - /** - * Creates new {@link ApiException} instance. - * - * @param message the detail message. - * @param throwable the cause. - */ - public ApiException(String message, Throwable throwable) { - super(message, throwable); - } + /** + * Creates new {@link ApiException} instance. + * + * @param message the detail message. + * @param throwable the cause. + */ + public ApiException(String message, Throwable throwable) { + super(message, throwable); + } - /** - * Creates new {@link ApiException} instance. - * - * @param code the HTTP response status code. - * @param message the detail message. - */ - public ApiException(int code, String message) { - super(message); - this.code = code; - } + /** + * Creates new {@link ApiException} instance. + * + * @param code the HTTP response status code. + * @param message the detail message. + */ + public ApiException(int code, String message) { + super(message); + this.code = code; + } - /** - * Creates new {@link ApiException} instance. - * - * @param code the HTTP response status code. - * @param message the detail message. - * @param responseHeaders list of headers returned by the server. - * @param responseBody content of the response sent back by the server. - */ - public ApiException(int code, String message, Map> responseHeaders, String responseBody) { - this(code, message); - this.responseHeaders = responseHeaders; - this.responseBody = responseBody; - } + /** + * Creates new {@link ApiException} instance. + * + * @param code the HTTP response status code. + * @param message the detail message. + * @param responseHeaders list of headers returned by the server. + * @param responseBody content of the response sent back by the server. + */ + public ApiException(int code, String message, Map> responseHeaders, String responseBody) { + this(code, message); + this.responseHeaders = responseHeaders; + this.responseBody = responseBody; + } - /** - * Indicates if the error is not a fatal one and if the api call can be subsequently retried. - * - * @return true if it error code is 401 unauthorized or 429 too many requests or 5xx greater than 500. - */ - public boolean isMinorError() { - return isServerError() || isUnauthorized() || isTooManyRequestsError(); - } + /** + * Indicates if the error is not a fatal one and if the api call can be subsequently retried. + * + * @return true if it error code is 401 unauthorized or 429 too many requests or 5xx greater than 500. + */ + public boolean isMinorError() { + return isServerError() || isUnauthorized() || isTooManyRequestsError(); + } - /** - * Check if response status is unauthorized or not. - * - * @return true if response status is 401, false otherwise - */ - public boolean isUnauthorized() { - return this.code == HttpURLConnection.HTTP_UNAUTHORIZED; - } + /** + * Check if response status is unauthorized or not. + * + * @return true if response status is 401, false otherwise + */ + public boolean isUnauthorized() { + return this.code == HttpURLConnection.HTTP_UNAUTHORIZED; + } - /** - * Check if response status is client error or not - * - * @return true if response status is 400, false otherwise - */ - public boolean isClientError() { - return this.code == HttpURLConnection.HTTP_BAD_REQUEST; - } + /** + * Check if response status is client error or not + * + * @return true if response status is 400, false otherwise + */ + public boolean isClientError() { + return this.code == HttpURLConnection.HTTP_BAD_REQUEST; + } - /** - * Check if response status is a server error (5xx) but not an internal server error (500) - * - * @return true if response status strictly greater than 500, false otherwise - */ - public boolean isServerError() { - return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; - } + /** + * Check if response status is a server error (5xx) but not an internal server error (500) + * + * @return true if response status strictly greater than 500, false otherwise + */ + public boolean isServerError() { + return this.code > HttpURLConnection.HTTP_INTERNAL_ERROR; + } - /** - * Check if response status corresponds to a too many requests error (429) - * - * @return true if error code is 429 - */ - public boolean isTooManyRequestsError() { - return this.code == 429; - } + /** + * Check if response status corresponds to a too many requests error (429) + * + * @return true if error code is 429 + */ + public boolean isTooManyRequestsError() { + return this.code == 429; + } } From a1ab641013571dfd9dd202f96c6785badd18d6f9 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Wed, 23 Sep 2020 14:44:50 +0200 Subject: [PATCH 16/17] Updated javadoc of NestedRetryException --- .../core/service/datafeed/exception/NestedRetryException.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java index 2924d33b3..6db499474 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/service/datafeed/exception/NestedRetryException.java @@ -4,7 +4,7 @@ /** * Exception thrown when recovery strategy in a {@link RetryWithRecovery} failed. - * Especially used in DataFeed services. + * Especially used in {@link com.symphony.bdk.core.service.datafeed.DatafeedService} implementations. */ public class NestedRetryException extends RuntimeException { public NestedRetryException(String message, Throwable cause) { From 04ac789ebcae021fcb8d2652120945793b64d2e4 Mon Sep 17 00:00:00 2001 From: Elias Croze Date: Thu, 24 Sep 2020 11:23:25 +0200 Subject: [PATCH 17/17] Removed default recovery strategy for SessionService in ServiceFactory --- .../src/main/java/com/symphony/bdk/core/ServiceFactory.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java index d166cf241..3c1d28e32 100644 --- a/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java +++ b/symphony-bdk-core/src/main/java/com/symphony/bdk/core/ServiceFactory.java @@ -83,7 +83,7 @@ public StreamService getStreamService() { * @return an new {@link SessionService} instance. */ public SessionService getSessionService() { - return new SessionService(new SessionApi(podClient), retryBuilder); + return new SessionService(new SessionApi(podClient), new RetryWithRecoveryBuilder<>().retryConfig(config.getRetry())); } /**