From 3f2fec632e13056b0fec79bffe62697fec0e4913 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 24 Feb 2023 13:15:21 +0530 Subject: [PATCH 01/19] SYMPHONYP-940 MS Teams feature to submit the adaptive card --- .../teams/handlers/TeamsResponseHandler.java | 3 +- .../adaptivecard/AdaptiveCardPassthrough.java | 33 +++++++++++++++++++ .../AdaptiveCardTemplateProvider.java | 12 ++++++- 3 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardPassthrough.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 2a8ed171..1b80d60f 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -17,6 +17,7 @@ import org.finos.springbot.teams.response.templating.EntityMarkupTemplateProvider; import org.finos.springbot.teams.response.templating.MarkupAndEntities; import org.finos.springbot.teams.state.TeamsStateStorage; +import org.finos.springbot.teams.templating.adaptivecard.AdaptiveCardPassthrough; import org.finos.springbot.teams.templating.adaptivecard.AdaptiveCardTemplateProvider; import org.finos.springbot.teams.templating.thymeleaf.ThymeleafTemplateProvider; import org.finos.springbot.workflow.annotations.WorkMode; @@ -142,7 +143,7 @@ protected TemplateType getTemplateType(WorkResponse wr) { TemplateType tt; if (displayTemplater.hasTemplate(wr)) { tt = TemplateType.THYMELEAF; - } else if (workTemplater.hasTemplate(wr)) { + } else if (workTemplater.hasTemplate(wr) || AdaptiveCardPassthrough.isAdaptiveCard(wr)) { tt = TemplateType.ADAPTIVE_CARD; } else if (wr.getMode() == WorkMode.EDIT) { tt = TemplateType.ADAPTIVE_CARD; diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardPassthrough.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardPassthrough.java new file mode 100644 index 00000000..40f6278c --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardPassthrough.java @@ -0,0 +1,33 @@ +package org.finos.springbot.teams.templating.adaptivecard; + +import org.finos.springbot.workflow.response.WorkResponse; + +import com.fasterxml.jackson.databind.JsonNode; + +public class AdaptiveCardPassthrough { + + private final static String ADAPTIVE_CARD = "AdaptiveCard"; + private final static String ADAPTIVE_CARD_TYPE = "type"; + + private final JsonNode jsonNode; + + public AdaptiveCardPassthrough(JsonNode jsonNode) { + this.jsonNode = jsonNode; + } + + public JsonNode getJsonNode() { + return jsonNode; + } + + public static boolean isAdaptiveCard(WorkResponse wr) { + if (wr.getFormObject() instanceof AdaptiveCardPassthrough) { + AdaptiveCardPassthrough passthrough = (AdaptiveCardPassthrough) wr.getFormObject(); + JsonNode node = passthrough.getJsonNode(); + JsonNode adaptiveNode = node.get(ADAPTIVE_CARD_TYPE); + return adaptiveNode.asText().equals(ADAPTIVE_CARD); + } + + return false; + } + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardTemplateProvider.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardTemplateProvider.java index 7ca0b16b..ba8ed4a6 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardTemplateProvider.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/templating/adaptivecard/AdaptiveCardTemplateProvider.java @@ -8,6 +8,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; +import org.finos.springbot.teams.handlers.TeamsResponseHandler; import org.finos.springbot.teams.templating.MatcherUtil; import org.finos.springbot.workflow.annotations.WorkMode; import org.finos.springbot.workflow.form.ButtonList; @@ -15,6 +16,8 @@ import org.finos.springbot.workflow.response.templating.AbstractResourceTemplateProvider; import org.finos.springbot.workflow.templating.Mode; import org.finos.springbot.workflow.templating.WorkTemplater; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.core.io.ResourceLoader; import com.fasterxml.jackson.annotation.JsonInclude.Include; @@ -27,6 +30,8 @@ public class AdaptiveCardTemplateProvider extends AbstractResourceTemplateProvider { + private static final Logger LOG = LoggerFactory.getLogger(AdaptiveCardTemplateProvider.class); + public static final String FORMID_KEY = "formid"; private final WorkTemplater formConverter; @@ -58,6 +63,12 @@ public JsonNode template(WorkResponse t) { // in this case, just provide the button template return formConverter.convert(null, Mode.DISPLAY_WITH_BUTTONS); + } else if (AdaptiveCardPassthrough.isAdaptiveCard(t)) { + AdaptiveCardPassthrough passthrough = (AdaptiveCardPassthrough) t.getFormObject(); + JsonNode template = passthrough.getJsonNode(); + LOG.info("JsonNode Template: \n" + template.toPrettyString()); + + return template; } else { return super.template(t); } @@ -65,7 +76,6 @@ public JsonNode template(WorkResponse t) { - @Override protected JsonNode getDefaultTemplate(WorkResponse r) { JsonNode insert; From a9f36993e165ee91bc477d6ba7076820f90781b5 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Thu, 2 Mar 2023 15:38:17 +0530 Subject: [PATCH 02/19] SYMPHONYP-940 refactor retry handler --- .../springbot/teams/TeamsScheduledConfig.java | 38 ++++++++++---- .../springbot/teams/TeamsWorkflowConfig.java | 13 +++-- .../teams/handlers/MessageRetryHandler.java | 11 ----- .../teams/handlers/TeamsResponseHandler.java | 44 ++++------------- .../InMemoryMessageRetryHandler.java | 5 +- .../handlers/{ => retry}/MessageRetry.java | 2 +- .../handlers/retry/MessageRetryHandler.java | 49 +++++++++++++++++++ .../handlers/retry/NoOpRetryHandler.java | 20 ++++++++ .../teams/handlers/retry/RetryHandler.java | 13 +++++ 9 files changed, 133 insertions(+), 62 deletions(-) delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetryHandler.java rename libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/{ => retry}/InMemoryMessageRetryHandler.java (87%) rename libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/{ => retry}/MessageRetry.java (97%) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java index 833a49b8..74c58528 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java @@ -1,23 +1,41 @@ package org.finos.springbot.teams; -import java.util.concurrent.TimeUnit; - import org.finos.springbot.teams.handlers.TeamsResponseHandler; +import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; +import org.finos.springbot.teams.handlers.retry.RetryHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.annotation.Value; import org.springframework.scheduling.annotation.EnableScheduling; -import org.springframework.scheduling.annotation.Scheduled; - +import org.springframework.scheduling.annotation.SchedulingConfigurer; +import org.springframework.scheduling.config.ScheduledTaskRegistrar; @EnableScheduling -public class TeamsScheduledConfig { +public class TeamsScheduledConfig implements SchedulingConfigurer { + + private static final Logger LOG = LoggerFactory.getLogger(TeamsScheduledConfig.class); @Autowired private TeamsResponseHandler handler; - - @Scheduled(fixedDelay = 30, timeUnit = TimeUnit.SECONDS) - //Task to run after a fixed delay. - //the duration between the end of last execution and the start of next execution is fixed - public void scheduleRetryMessage() { + + @Autowired + private RetryHandler retryHandler; + + @Value("${teams.retry.time:30000}") + private long teamsRetrySchedulerCron; + + @Override + public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { + if (retryHandler instanceof NoOpRetryHandler) { + LOG.info("No operation retry handler is configure"); + } else { + Runnable runnable = () -> scheduleRetryMessage(); + scheduledTaskRegistrar.addFixedDelayTask(runnable, teamsRetrySchedulerCron); + } + } + + private void scheduleRetryMessage() { handler.retryMessage(); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 49143031..17fae71b 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -13,9 +13,10 @@ import org.finos.springbot.teams.conversations.TeamsConversationsConfig; import org.finos.springbot.teams.form.TeamsFormConverter; import org.finos.springbot.teams.form.TeamsFormDeserializerModule; -import org.finos.springbot.teams.handlers.InMemoryMessageRetryHandler; -import org.finos.springbot.teams.handlers.MessageRetryHandler; import org.finos.springbot.teams.handlers.TeamsResponseHandler; +import org.finos.springbot.teams.handlers.retry.InMemoryMessageRetryHandler; +import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; +import org.finos.springbot.teams.handlers.retry.RetryHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; @@ -130,7 +131,7 @@ public TeamsResponseHandler teamsResponseHandler( ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, TeamsConversations tc, - MessageRetryHandler mr) { + RetryHandler mr) { return new TeamsResponseHandler( null, // attachment handler markupTemplater, @@ -230,9 +231,13 @@ public void setResourceLoaderClassLoader() { resourceLoader.setClassLoader(this.getClass().getClassLoader()); } + /** + * set NoOpRetryHandler() if you don't want rety + * @return + */ @Bean @ConditionalOnMissingBean - public MessageRetryHandler messageRetryHandler() { + public RetryHandler messageRetryHandler() { return new InMemoryMessageRetryHandler(); } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetryHandler.java deleted file mode 100644 index a1718b36..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetryHandler.java +++ /dev/null @@ -1,11 +0,0 @@ -package org.finos.springbot.teams.handlers; - -import java.util.Optional; - -public interface MessageRetryHandler { - - public void add(MessageRetry t); - - public Optional get(); - -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 1b80d60f..7762725e 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -8,10 +8,11 @@ import java.util.concurrent.CompletionException; import java.util.function.BiFunction; -import org.apache.commons.lang3.StringUtils; import org.finos.springbot.teams.TeamsException; import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.conversations.TeamsConversations; +import org.finos.springbot.teams.handlers.retry.MessageRetry; +import org.finos.springbot.teams.handlers.retry.RetryHandler; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; import org.finos.springbot.teams.response.templating.EntityMarkupTemplateProvider; @@ -32,7 +33,6 @@ import org.springframework.beans.BeansException; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; -import org.springframework.http.HttpStatus; import org.springframework.util.ErrorHandler; import com.fasterxml.jackson.core.JsonProcessingException; @@ -46,13 +46,11 @@ import com.microsoft.bot.schema.ResourceResponse; import com.microsoft.bot.schema.TextFormatTypes; -import okhttp3.ResponseBody; - public class TeamsResponseHandler implements ResponseHandler, ApplicationContextAware { private static final Logger LOG = LoggerFactory.getLogger(TeamsResponseHandler.class); - private static final int RETRY_COUNT = 3; + private static final int INIT_RETRY_COUNT = 0; protected AttachmentHandler attachmentHandler; @@ -63,7 +61,7 @@ public class TeamsResponseHandler implements ResponseHandler, ApplicationContext protected ThymeleafTemplateProvider displayTemplater; protected TeamsStateStorage teamsState; protected TeamsConversations teamsConversations; - protected MessageRetryHandler messageRetryHandler; + protected RetryHandler retryHandler; public TeamsResponseHandler( AttachmentHandler attachmentHandler, @@ -72,14 +70,14 @@ public TeamsResponseHandler( ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, TeamsConversations tc, - MessageRetryHandler mr) { + RetryHandler mr) { this.attachmentHandler = attachmentHandler; this.messageTemplater = messageTemplater; this.workTemplater = workTemplater; this.displayTemplater = displayTemplater; this.teamsState = th; this.teamsConversations = tc; - this.messageRetryHandler = mr; + this.retryHandler = mr; } protected void initErrorHandler() { @@ -95,7 +93,7 @@ public void accept(Response t) { sendResponse(t, INIT_RETRY_COUNT); } - private void sendResponse(Response t, int retryCount) { + public void sendResponse(Response t, int retryCount) { if (t.getAddress() instanceof TeamsAddressable) { TeamsAddressable ta = (TeamsAddressable) t.getAddress(); @@ -192,32 +190,10 @@ private BiFunction handle }; } - private boolean insertIntoQueue(Response t, int retryCount, Throwable e) { - if (e instanceof CompletionException - && ((CompletionException) e).getCause() instanceof ErrorResponseException) { - ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); - retrofit2.Response response = ere.response(); - if (response.code() == HttpStatus.TOO_MANY_REQUESTS.value() && retryCount <= RETRY_COUNT) { - String retryAfter = response.headers().get("Retry-After"); - - int retryAfterInt = 1;//initiate to 1 sec - if(StringUtils.isNumeric(retryAfter)) { - retryAfterInt = Integer.parseInt(retryAfter); - } - - messageRetryHandler.add(new MessageRetry(t, retryCount, retryAfterInt)); - - return true; - } - } - - return false; - } - private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t, int retryCount) { return (rr, e) -> { if (e != null) { - boolean success = insertIntoQueue(t, retryCount, e); + boolean success = retryHandler.handleException(t, retryCount, e); if(!success) { LOG.error(e.getMessage()); if (out instanceof ObjectNode){ @@ -239,7 +215,7 @@ private BiFunction handle return null; }; } - + protected CompletableFuture sendCardResponse(JsonNode json, TeamsAddressable address, Map data) throws Exception { Activity out = Activity.createMessageActivity(); Attachment body = new Attachment(); @@ -275,7 +251,7 @@ public void retryMessage() { int messageCount = 0; Optional opt; - while ((opt = messageRetryHandler.get()).isPresent()) { + while ((opt = retryHandler.get()).isPresent()) { messageCount++; this.sendResponse(opt.get().getResponse(), opt.get().getRetryCount()); } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/InMemoryMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java similarity index 87% rename from libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/InMemoryMessageRetryHandler.java rename to libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java index e275f01d..59751dfa 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/InMemoryMessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java @@ -1,4 +1,4 @@ -package org.finos.springbot.teams.handlers; +package org.finos.springbot.teams.handlers.retry; import java.time.LocalDateTime; import java.util.Optional; @@ -8,7 +8,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class InMemoryMessageRetryHandler implements MessageRetryHandler { +public class InMemoryMessageRetryHandler extends MessageRetryHandler { private static final Logger LOG = LoggerFactory.getLogger(InMemoryMessageRetryHandler.class); @@ -34,5 +34,6 @@ public Optional get() { return Optional.empty(); } + } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetry.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java similarity index 97% rename from libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetry.java rename to libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java index d30a775f..eef3d710 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/MessageRetry.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java @@ -1,4 +1,4 @@ -package org.finos.springbot.teams.handlers; +package org.finos.springbot.teams.handlers.retry; import java.time.LocalDateTime; diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java new file mode 100644 index 00000000..7ad4c651 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java @@ -0,0 +1,49 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.util.concurrent.CompletionException; + +import org.apache.commons.lang3.StringUtils; +import org.finos.springbot.workflow.response.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; + +import com.microsoft.bot.connector.rest.ErrorResponseException; + +import okhttp3.ResponseBody; + +public abstract class MessageRetryHandler implements RetryHandler { + + private static final Logger LOG = LoggerFactory.getLogger(MessageRetryHandler.class); + + @Value("${teams.retry.count:3}") + private long teamsRetryCount; + + public boolean handleException(Response t, int retryCount, Throwable e) { + if (e instanceof CompletionException + && ((CompletionException) e).getCause() instanceof ErrorResponseException) { + ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); + retrofit2.Response response = ere.response(); + if (response.code() == HttpStatus.TOO_MANY_REQUESTS.value() && retryCount <= teamsRetryCount) { + String retryAfter = response.headers().get("Retry-After"); + LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); + + int retryAfterInt = 1;//initiate to 1 sec + if(StringUtils.isNumeric(retryAfter)) { + retryAfterInt = Integer.parseInt(retryAfter); + } + + add(new MessageRetry(t, retryCount, retryAfterInt)); + + return true; + } + } + + return false; + } + + protected abstract void add(MessageRetry messageRetry); + + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java new file mode 100644 index 00000000..c5f818ee --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java @@ -0,0 +1,20 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.util.Optional; + +import org.finos.springbot.workflow.response.Response; + +public class NoOpRetryHandler implements RetryHandler { + + @Override + public Optional get() { + return Optional.empty(); + } + + @Override + public boolean handleException(Response t, int retryCount, Throwable e) { + + return false; + } + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java new file mode 100644 index 00000000..31e1a0d6 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java @@ -0,0 +1,13 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.util.Optional; + +import org.finos.springbot.workflow.response.Response; + +public interface RetryHandler { + + public boolean handleException(Response t, int retryCount, Throwable e); + + public Optional get(); + +} From 80509b33cb26aba3792eb28941aa9777972d0297 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Wed, 15 Mar 2023 18:54:04 +0530 Subject: [PATCH 03/19] SYMPHONYP-939 implements review comments --- .../springbot/teams/TeamsScheduledConfig.java | 6 +-- .../springbot/teams/TeamsWorkflowConfig.java | 11 ++-- .../teams/handlers/TeamsResponseHandler.java | 6 +-- .../retry/BasicMessageRetryHandler.java | 50 +++++++++++++++++++ .../retry/InMemoryMessageRetryHandler.java | 7 ++- .../teams/handlers/retry/MessageRetry.java | 24 ++++----- .../handlers/retry/MessageRetryHandler.java | 46 ++--------------- .../handlers/retry/NoOpRetryHandler.java | 2 +- .../teams/handlers/retry/RetryHandler.java | 13 ----- 9 files changed, 82 insertions(+), 83 deletions(-) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java index 74c58528..92bafb2a 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java @@ -2,7 +2,7 @@ import org.finos.springbot.teams.handlers.TeamsResponseHandler; import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; -import org.finos.springbot.teams.handlers.retry.RetryHandler; +import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -20,7 +20,7 @@ public class TeamsScheduledConfig implements SchedulingConfigurer { private TeamsResponseHandler handler; @Autowired - private RetryHandler retryHandler; + private MessageRetryHandler retryHandler; @Value("${teams.retry.time:30000}") private long teamsRetrySchedulerCron; @@ -28,7 +28,7 @@ public class TeamsScheduledConfig implements SchedulingConfigurer { @Override public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { if (retryHandler instanceof NoOpRetryHandler) { - LOG.info("No operation retry handler is configure"); + LOG.info("No-operation retry handler is configured."); } else { Runnable runnable = () -> scheduleRetryMessage(); scheduledTaskRegistrar.addFixedDelayTask(runnable, teamsRetrySchedulerCron); diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 17fae71b..c54c1804 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -14,9 +14,8 @@ import org.finos.springbot.teams.form.TeamsFormConverter; import org.finos.springbot.teams.form.TeamsFormDeserializerModule; import org.finos.springbot.teams.handlers.TeamsResponseHandler; -import org.finos.springbot.teams.handlers.retry.InMemoryMessageRetryHandler; +import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; -import org.finos.springbot.teams.handlers.retry.RetryHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; @@ -131,7 +130,7 @@ public TeamsResponseHandler teamsResponseHandler( ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, TeamsConversations tc, - RetryHandler mr) { + MessageRetryHandler mr) { return new TeamsResponseHandler( null, // attachment handler markupTemplater, @@ -232,13 +231,13 @@ public void setResourceLoaderClassLoader() { } /** - * set NoOpRetryHandler() if you don't want rety + * set InMemoryMessageRetryHandler() if you want retry * @return */ @Bean @ConditionalOnMissingBean - public RetryHandler messageRetryHandler() { - return new InMemoryMessageRetryHandler(); + public MessageRetryHandler messageRetryHandler() { + return new NoOpRetryHandler(); } } \ No newline at end of file diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 7762725e..3d62a110 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -12,7 +12,7 @@ import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.conversations.TeamsConversations; import org.finos.springbot.teams.handlers.retry.MessageRetry; -import org.finos.springbot.teams.handlers.retry.RetryHandler; +import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; import org.finos.springbot.teams.response.templating.EntityMarkupTemplateProvider; @@ -61,7 +61,7 @@ public class TeamsResponseHandler implements ResponseHandler, ApplicationContext protected ThymeleafTemplateProvider displayTemplater; protected TeamsStateStorage teamsState; protected TeamsConversations teamsConversations; - protected RetryHandler retryHandler; + protected MessageRetryHandler retryHandler; public TeamsResponseHandler( AttachmentHandler attachmentHandler, @@ -70,7 +70,7 @@ public TeamsResponseHandler( ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, TeamsConversations tc, - RetryHandler mr) { + MessageRetryHandler mr) { this.attachmentHandler = attachmentHandler; this.messageTemplater = messageTemplater; this.workTemplater = workTemplater; diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java new file mode 100644 index 00000000..cbae06d1 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java @@ -0,0 +1,50 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.time.LocalDateTime; +import java.util.concurrent.CompletionException; + +import org.apache.commons.lang3.StringUtils; +import org.finos.springbot.workflow.response.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; + +import com.microsoft.bot.connector.rest.ErrorResponseException; + +import okhttp3.ResponseBody; + +public abstract class BasicMessageRetryHandler implements MessageRetryHandler { + + private static final Logger LOG = LoggerFactory.getLogger(BasicMessageRetryHandler.class); + + @Value("${teams.retry.count:3}") + private long teamsRetryCount; + + public boolean handleException(Response t, int retryCount, Throwable e) { + if (e instanceof CompletionException + && ((CompletionException) e).getCause() instanceof ErrorResponseException) { + ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); + retrofit2.Response response = ere.response(); + if (response.code() == HttpStatus.TOO_MANY_REQUESTS.value() && retryCount <= teamsRetryCount) { + String retryAfter = response.headers().get("Retry-After"); + LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); + + int retryAfterInt = 1;//initiate to 1 sec + if(StringUtils.isNumeric(retryAfter)) { + retryAfterInt = Integer.parseInt(retryAfter); + } + LocalDateTime time = LocalDateTime.now().plusSeconds(retryAfterInt); + add(new MessageRetry(t, retryCount, retryAfterInt, time)); + + return true; + } + } + + return false; + } + + protected abstract void add(MessageRetry messageRetry); + + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java index 59751dfa..cc34a02a 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java @@ -8,7 +8,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public class InMemoryMessageRetryHandler extends MessageRetryHandler { +public class InMemoryMessageRetryHandler extends BasicMessageRetryHandler { private static final Logger LOG = LoggerFactory.getLogger(InMemoryMessageRetryHandler.class); @@ -23,12 +23,11 @@ public void add(MessageRetry t) { public Optional get() { MessageRetry q; if ((q = queue.peek()) != null) { - LocalDateTime time = q.getCurrentTime().plusSeconds(q.getRetryAfter()); - if (LocalDateTime.now().isAfter(time)) { // retry now + if (LocalDateTime.now().isAfter(q.getRetryTime())) { // retry now queue.remove(q); return Optional.of(q); }else { - LOG.info("Message not retied, as retry after time {} is not getter than or equal to current time {}", time, LocalDateTime.now()); + LOG.info("Message not retried, as retry time {} for message has not passed the current time {}", q.getRetryTime(), LocalDateTime.now()); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java index eef3d710..ff672fb5 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java @@ -9,14 +9,14 @@ public class MessageRetry { private Response response; private int retryCount; private int retryAfter; - private LocalDateTime currentTime; + private LocalDateTime retryTime; - public MessageRetry(Response response, int retryCount, int retryAfter) { + public MessageRetry(Response response, int retryCount, int retryAfter, LocalDateTime retryTime) { super(); this.response = response; this.retryCount = retryCount; this.retryAfter = retryAfter; - currentTime = LocalDateTime.now(); + this.retryTime = retryTime; } public Response getResponse() { @@ -44,25 +44,25 @@ public void setRetryAfter(int retryAfter) { } - public LocalDateTime getCurrentTime() { - return currentTime; + public LocalDateTime getRetryTime() { + return retryTime; } - public void setCurrentTime(LocalDateTime currentTime) { - this.currentTime = currentTime; + public void setRetryTime(LocalDateTime retryTime) { + this.retryTime = retryTime; } @Override public String toString() { return "MessageRetry [response=" + response + ", retryCount=" + retryCount + ", retryAfter=" + retryAfter - + ", localDate=" + currentTime + "]"; + + ", localDate=" + retryTime + "]"; } @Override public int hashCode() { final int prime = 31; int result = 1; - result = prime * result + ((currentTime == null) ? 0 : currentTime.hashCode()); + result = prime * result + ((retryTime == null) ? 0 : retryTime.hashCode()); result = prime * result + ((response == null) ? 0 : response.hashCode()); result = prime * result + retryAfter; result = prime * result + retryCount; @@ -78,10 +78,10 @@ public boolean equals(Object obj) { if (getClass() != obj.getClass()) return false; MessageRetry other = (MessageRetry) obj; - if (currentTime == null) { - if (other.currentTime != null) + if (retryTime == null) { + if (other.retryTime != null) return false; - } else if (!currentTime.equals(other.currentTime)) + } else if (!retryTime.equals(other.retryTime)) return false; if (response == null) { if (other.response != null) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java index 7ad4c651..048d7638 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java @@ -1,49 +1,13 @@ package org.finos.springbot.teams.handlers.retry; -import java.util.concurrent.CompletionException; +import java.util.Optional; -import org.apache.commons.lang3.StringUtils; import org.finos.springbot.workflow.response.Response; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.HttpStatus; -import com.microsoft.bot.connector.rest.ErrorResponseException; - -import okhttp3.ResponseBody; - -public abstract class MessageRetryHandler implements RetryHandler { - - private static final Logger LOG = LoggerFactory.getLogger(MessageRetryHandler.class); +public interface MessageRetryHandler { - @Value("${teams.retry.count:3}") - private long teamsRetryCount; - - public boolean handleException(Response t, int retryCount, Throwable e) { - if (e instanceof CompletionException - && ((CompletionException) e).getCause() instanceof ErrorResponseException) { - ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); - retrofit2.Response response = ere.response(); - if (response.code() == HttpStatus.TOO_MANY_REQUESTS.value() && retryCount <= teamsRetryCount) { - String retryAfter = response.headers().get("Retry-After"); - LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); - - int retryAfterInt = 1;//initiate to 1 sec - if(StringUtils.isNumeric(retryAfter)) { - retryAfterInt = Integer.parseInt(retryAfter); - } - - add(new MessageRetry(t, retryCount, retryAfterInt)); - - return true; - } - } - - return false; - } + public boolean handleException(Response t, int retryCount, Throwable e); + + public Optional get(); - protected abstract void add(MessageRetry messageRetry); - - } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java index c5f818ee..ed3b334f 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java @@ -4,7 +4,7 @@ import org.finos.springbot.workflow.response.Response; -public class NoOpRetryHandler implements RetryHandler { +public class NoOpRetryHandler implements MessageRetryHandler { @Override public Optional get() { diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java deleted file mode 100644 index 31e1a0d6..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/RetryHandler.java +++ /dev/null @@ -1,13 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.util.Optional; - -import org.finos.springbot.workflow.response.Response; - -public interface RetryHandler { - - public boolean handleException(Response t, int retryCount, Throwable e); - - public Optional get(); - -} From b9a31ff72d2484878e4cc64ef288cd5f4aa669d1 Mon Sep 17 00:00:00 2001 From: dahiabh Date: Thu, 16 Mar 2023 14:17:25 +0530 Subject: [PATCH 04/19] added testcases for Message Retry. --- .../InMemoryMessageRetryHandlerTest.java | 66 +++++++++++++++++++ .../handlers/retry/NoOpRetryHandlerTest.java | 29 ++++++++ 2 files changed, 95 insertions(+) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java new file mode 100644 index 00000000..657c7bb6 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java @@ -0,0 +1,66 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.time.LocalDateTime; +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; + +import org.finos.springbot.teams.content.TeamsChannel; +import org.finos.springbot.workflow.data.DataHandlerConfig; +import org.finos.springbot.workflow.response.DataResponse; +import org.finos.springbot.workflow.response.Response; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; + +@SpringBootTest(classes = { + DataHandlerConfig.class, + }) + +public class InMemoryMessageRetryHandlerTest { + + InMemoryMessageRetryHandler inMemoryMessageRetryHandler = new InMemoryMessageRetryHandler(); + TeamsChannel dummyChat = new TeamsChannel(); + Object dummyObject = new Object(); + + @BeforeEach + public void setUp() { + while(inMemoryMessageRetryHandler.queue.poll()!=null); + LocalDateTime currentTime= LocalDateTime.now(); + Map data1 = new HashMap<>(); + data1.put("key1", dummyObject); + Map data2 = new HashMap<>(); + data2.put("key2", dummyObject); + Response r1 = new DataResponse(dummyChat, data1, ""); + Response r2 = new DataResponse(dummyChat, data2, ""); + MessageRetry t1 = new MessageRetry(r1,3,5,currentTime.plusSeconds(5)); + MessageRetry t2 = new MessageRetry(r2,3,45,currentTime.plusSeconds(45)); + inMemoryMessageRetryHandler.queue.add(t1); + inMemoryMessageRetryHandler.queue.add(t2); + } + @Test + public void testGet() throws InterruptedException { + Map expectedData = new HashMap<>(); + expectedData.put("key1", dummyObject); + LocalDateTime futureTime = LocalDateTime.now().plusSeconds(10); + while(LocalDateTime.now().isBefore(futureTime)) { + Thread.sleep(1000); + } + DataResponse actualResponse = (DataResponse) inMemoryMessageRetryHandler.get().get().getResponse(); + Assertions.assertTrue(actualResponse.getData().equals(expectedData)); + Optional actualResult = inMemoryMessageRetryHandler.get(); + Assertions.assertFalse(actualResult.isPresent()); + + } + + @Test + public void testGetNoData() throws InterruptedException { + LocalDateTime futureTime = LocalDateTime.now().plusSeconds(1); + while(LocalDateTime.now().isBefore(futureTime)) { + Thread.sleep(1000); + } + Optional actualResponse = inMemoryMessageRetryHandler.get(); + Assertions.assertFalse(actualResponse.isPresent()); + } +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java new file mode 100644 index 00000000..731706fc --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java @@ -0,0 +1,29 @@ +package org.finos.springbot.teams.handlers.retry; + +import org.finos.springbot.teams.content.TeamsChannel; +import org.finos.springbot.workflow.data.DataHandlerConfig; +import org.finos.springbot.workflow.response.MessageResponse; +import org.finos.springbot.workflow.response.Response; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.springframework.boot.test.context.SpringBootTest; + +@SpringBootTest(classes = { + DataHandlerConfig.class, + }) +public class NoOpRetryHandlerTest { + + public NoOpRetryHandler noOpRetryHandler = new NoOpRetryHandler(); + + @Test + public void getTest() { + Assertions.assertFalse(noOpRetryHandler.get().isPresent()); + } + + @Test + public void handleExceptionTest() { + Response t = new MessageResponse(new TeamsChannel(),""); + int retryCount=1; + Assertions.assertFalse(noOpRetryHandler.handleException(t,retryCount, new Exception())); + } +} From 3e4b60cbfa4318e45122d113b408aec181c8bf98 Mon Sep 17 00:00:00 2001 From: dahiabh Date: Tue, 21 Mar 2023 16:14:21 +0530 Subject: [PATCH 05/19] review comments changes done. --- .../retry/BasicMessageRetryHandler.java | 2 - .../retry/InMemoryMessageRetryHandler.java | 7 +++- .../handlers/retry/MessageRetryHandler.java | 4 ++ .../handlers/retry/NoOpRetryHandler.java | 10 +++++ .../InMemoryMessageRetryHandlerTest.java | 40 +++++++++---------- 5 files changed, 38 insertions(+), 25 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java index cbae06d1..9bd2b5f3 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java @@ -43,8 +43,6 @@ public boolean handleException(Response t, int retryCount, Throwable e) { return false; } - - protected abstract void add(MessageRetry messageRetry); } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java index cc34a02a..93cf9bd4 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java @@ -12,12 +12,17 @@ public class InMemoryMessageRetryHandler extends BasicMessageRetryHandler { private static final Logger LOG = LoggerFactory.getLogger(InMemoryMessageRetryHandler.class); - Queue queue = new ConcurrentLinkedQueue<>(); + private Queue queue = new ConcurrentLinkedQueue<>(); @Override public void add(MessageRetry t) { queue.add(t); } + + @Override + public void clearAll() { + while(queue.poll()!=null); + } @Override public Optional get() { diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java index 048d7638..3759503c 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java @@ -10,4 +10,8 @@ public interface MessageRetryHandler { public Optional get(); + public void add(MessageRetry messageRetry); + + public void clearAll(); + } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java index ed3b334f..47d14354 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java @@ -17,4 +17,14 @@ public boolean handleException(Response t, int retryCount, Throwable e) { return false; } + @Override + public void add(MessageRetry messageRetry) { + //Do-Nothing + } + + @Override + public void clearAll() { + //Do-Nothing + } + } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java index 657c7bb6..8ede6491 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java @@ -10,7 +10,6 @@ import org.finos.springbot.workflow.response.DataResponse; import org.finos.springbot.workflow.response.Response; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.boot.test.context.SpringBootTest; @@ -24,43 +23,40 @@ public class InMemoryMessageRetryHandlerTest { TeamsChannel dummyChat = new TeamsChannel(); Object dummyObject = new Object(); - @BeforeEach - public void setUp() { - while(inMemoryMessageRetryHandler.queue.poll()!=null); - LocalDateTime currentTime= LocalDateTime.now(); + public void setUp(LocalDateTime retryTime1,LocalDateTime retryTime2) { + inMemoryMessageRetryHandler.clearAll(); Map data1 = new HashMap<>(); data1.put("key1", dummyObject); Map data2 = new HashMap<>(); data2.put("key2", dummyObject); Response r1 = new DataResponse(dummyChat, data1, ""); Response r2 = new DataResponse(dummyChat, data2, ""); - MessageRetry t1 = new MessageRetry(r1,3,5,currentTime.plusSeconds(5)); - MessageRetry t2 = new MessageRetry(r2,3,45,currentTime.plusSeconds(45)); - inMemoryMessageRetryHandler.queue.add(t1); - inMemoryMessageRetryHandler.queue.add(t2); + MessageRetry t1 = new MessageRetry(r1,3,45,retryTime1); + MessageRetry t2 = new MessageRetry(r2,3,45,retryTime2); + inMemoryMessageRetryHandler.add(t1); + inMemoryMessageRetryHandler.add(t2); } @Test - public void testGet() throws InterruptedException { + public void testFoundMessageForRetry() { + setUp(LocalDateTime.now().minusSeconds(100),LocalDateTime.now().plusSeconds(150)); Map expectedData = new HashMap<>(); expectedData.put("key1", dummyObject); - LocalDateTime futureTime = LocalDateTime.now().plusSeconds(10); - while(LocalDateTime.now().isBefore(futureTime)) { - Thread.sleep(1000); - } DataResponse actualResponse = (DataResponse) inMemoryMessageRetryHandler.get().get().getResponse(); Assertions.assertTrue(actualResponse.getData().equals(expectedData)); - Optional actualResult = inMemoryMessageRetryHandler.get(); - Assertions.assertFalse(actualResult.isPresent()); - } @Test - public void testGetNoData() throws InterruptedException { - LocalDateTime futureTime = LocalDateTime.now().plusSeconds(1); - while(LocalDateTime.now().isBefore(futureTime)) { - Thread.sleep(1000); - } + public void testNoMessageEligibleForRetry(){ + setUp(LocalDateTime.now().plusSeconds(100),LocalDateTime.now().plusSeconds(150)); Optional actualResponse = inMemoryMessageRetryHandler.get(); Assertions.assertFalse(actualResponse.isPresent()); } + + @Test + public void testNoMessageInQueue() { + inMemoryMessageRetryHandler.clearAll(); + Optional actualResponse = inMemoryMessageRetryHandler.get(); + Assertions.assertFalse(actualResponse.isPresent()); + + } } From 18d766fd56c9bb561b2de3179c75cd57fae5e3eb Mon Sep 17 00:00:00 2001 From: dahiabh Date: Fri, 31 Mar 2023 15:20:34 +0530 Subject: [PATCH 06/19] bug fix implemented. --- .../teams/state/FileStateStorage.java | 115 ++++++++++-------- 1 file changed, 62 insertions(+), 53 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java index c386fce9..968ff638 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java @@ -33,8 +33,6 @@ public class FileStateStorage extends AbstractStateStorage { - //protected static final Logger LOG = LoggerFactory.getLogger(FileStateStorage.class); - final static String DATA_FOLDER = "data"; final static String TAG_INDEX_FOLDER = "tag_index"; final static String FILE_EXT = ".txt"; @@ -53,17 +51,16 @@ public FileStateStorage(EntityJsonConverter ejc, String filePath) { @Override public void store(String file, Map tags, Map data) { - if ((tags != null) && (tags.size() > 0)) { + String addressable = getAddressable(file); String storageId = getStorage(file); try { + Path path = checkAndCreateFolder(this.filePath + addressable); - Path dataPath = checkAndCreateFolder(path.toString() + File.separator + DATA_FOLDER); - Path tagPath = checkAndCreateFolder(path.toString() + File.separator + TAG_INDEX_FOLDER); - + Path dataPath = checkAndCreateFolder(path.toString() + File.separator + DATA_FOLDER); + Path tagPath = checkAndCreateFolder(path.toString() + File.separator + TAG_INDEX_FOLDER); createStorageFile(dataPath, storageId, data); - createTagIndexFile(tags, storageId, tagPath); } catch (IOException e) { throw new TeamsException("Error while creating or getting folder " + e); @@ -100,6 +97,7 @@ private void createTagIndexFile(Map tags, String storageId, Path @Override public Optional> retrieve(String file) { String addressable = getAddressable(file); + String storageId = getStorage(file); Optional optData = readFile( @@ -108,15 +106,16 @@ public Optional> retrieve(String file) { if (optData.isPresent()) { return Optional.ofNullable(ejc.readValue(optData.get())); } else { + return Optional.empty(); } } @Override public Iterable> retrieve(List tags, boolean singleResultOnly) { - List tagFolders = getAllTagIndex(tags);// get all tag_index files - List files = extractDataFileName(tagFolders); + List tagFiles = getAllTagIndex(tags); // get all tag_index files + List dataFiles = getDataFiles(tagFiles); Optional ftOpt = tags.stream() .filter(t -> t.key.equals(StateStorageBasedTeamsConversations.ADDRESSABLE_INFO) @@ -125,27 +124,40 @@ public Iterable> retrieve(List tags, boolean singleR // if search for addressable-info like room information or user information if (ftOpt.isPresent()) { - - List> out = files.stream() + List> out = dataFiles.stream() .map(f -> ejc.readValue(readFile(f.getAbsolutePath()).orElse(""))) .filter(filterAddressableTypeFiles()).collect(Collectors.toList()); - if ((singleResultOnly) && (files.size() > 0)) { + if ((singleResultOnly) && (dataFiles.size() > 0)) { out = out.subList(0, 1); } return out; } else { - if ((singleResultOnly) && (files.size() > 0)) { - files = files.subList(0, 1); + if ((singleResultOnly) && (dataFiles.size() > 0)) { + dataFiles = dataFiles.subList(0, 1); } - return files.stream().map(f -> ejc.readValue(readFile(f.getAbsolutePath()).orElse(""))) + return dataFiles.stream().map(f -> ejc.readValue(readFile(f.getAbsolutePath()).orElse(""))) .collect(Collectors.toList()); } } - private List extractDataFileName(List tagFolders) { - return tagFolders.stream().map(t -> getDataFiles(t)).flatMap(f -> f.stream()).distinct() - .collect(Collectors.toList()); + private List tagIndexFileIntersection(List tagFolders) { + List intersection = new ArrayList<>(); + for (int i = 0; i < tagFolders.size(); ++i) { + List tagFolderInsideFiles = getTagIndexFiles(tagFolders.get(i)); + if (i == 0) { + intersection.addAll(tagFolderInsideFiles); + } else { + intersection = intersection.stream().filter(containsTagFile(tagFolderInsideFiles)) + .collect(Collectors.toList()); + } + + } + return intersection; + } + + private Predicate containsTagFile(List files) { + return f -> files.stream().filter(file -> file.getName().equals(f.getName())).findAny().isPresent(); } private List getAllTagIndex(List filter) { @@ -180,7 +192,6 @@ private void getAllTagIndex(File node, List fileList, List tags) { } } - /** * Get tag-index files for given tag * @@ -192,45 +203,44 @@ private List filteredTagsFiles(List tags, File tagFolder) { List subNote = Arrays.asList(tagFolder.list()); Map tagMap = tags.stream().collect(Collectors.toMap(f -> getAzurePath(f.key), f -> f)); - List files = subNote.stream().map(s -> { + List tagFiles = subNote.stream().map(s -> { Filter value = tagMap.get(s); if (!Objects.isNull(value)) { File file = new File(tagFolder, getAzurePath(value.key)); List fileList = Arrays.asList(file.list()); - for (String n : fileList) { - List subSubNote = Arrays.asList(n); - List list = checkEntity(value, subSubNote); - if (!list.isEmpty()) { + for (String name : fileList) { + boolean check = checkEntity(value, name); + if (check) { tagMap.remove(s); - return list.stream().map(l -> new File(file, l)).collect(Collectors.toList()); + return new File(file, name); } } } return null; - }).filter(f -> !Objects.isNull(f)).flatMap(f -> f.stream()).collect(Collectors.toList()); + }).filter(Objects::nonNull).collect(Collectors.toList()); + + tagFiles = tagIndexFileIntersection(tagFiles); if (tagMap.isEmpty()) { - return files; + return tagFiles; } else { return Collections.emptyList(); } } - private List checkEntity(Filter f, List subSubNote) { - return subSubNote.stream().filter(s -> { - int cmp = getAzurePath(f.value).compareTo(s); + private boolean checkEntity(Filter f, String name) { + int cmp = getAzurePath(f.value).compareTo(name); - if (f.operator.contains("=") && (cmp == 0)) { - return true; - } - if (f.operator.contains(">") && (cmp < 0)) { - return true; - } - if (f.operator.contains("<") && (cmp > 0)) { - return true; - } - return false; - }).collect(Collectors.toList()); + if (f.operator.contains("=") && (cmp == 0)) { + return true; + } + if (f.operator.contains(">") && (cmp < 0)) { + return true; + } + if (f.operator.contains("<") && (cmp > 0)) { + return true; + } + return false; } private Predicate filterAddressableTypeFiles() { @@ -252,29 +262,29 @@ private Predicate filterAddressableTypeFiles() { * @param tagPath * @return */ - - private List getDataFiles(File tagPath) { - - String addressableId = tagPath.getParentFile().getParentFile().getParentFile().getName(); + private List getTagIndexFiles(File tagPath) { try (Stream stream = Files.list(tagPath.toPath())) { Set paths = stream.filter(file -> !Files.isDirectory(file)).collect(Collectors.toSet()); - List files = paths.stream().map(f -> new File(f.toString())) + return paths.stream().map(f -> new File(f.toString())) .sorted(Collections.reverseOrder(Comparator.comparingLong(File::lastModified))) .collect(Collectors.toList()); - - List list = files.stream().map(f -> new File(this.filePath + File.separator + addressableId - + File.separator + DATA_FOLDER + File.separator + f.getName())).collect(Collectors.toList()); - - return list; - } catch (IOException e) { throw new TeamsException("Error while retriving data files " + e); } } + private List getDataFiles(List tagFiles) { + return tagFiles.stream().map(f -> { + String addressableId = f.getParentFile().getParentFile().getParentFile().getParentFile().getName(); + return new File(this.filePath + File.separator + addressableId + File.separator + DATA_FOLDER + + File.separator + f.getName()); + }).collect(Collectors.toList()); + + } + private String getAddressable(String file) { Optional> split = splitString(file); if (split.isPresent()) { @@ -319,6 +329,5 @@ private String getAzurePath(String s) { } return path; } - } From add1a4d43ddd3b95d36e79af47da2b31db77cc3b Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 31 Mar 2023 17:34:54 +0530 Subject: [PATCH 07/19] refactor retry message handler --- .../springbot/teams/TeamsScheduledConfig.java | 19 +- .../springbot/teams/TeamsWorkflowConfig.java | 25 +-- .../teams/handlers/ActivityHandler.java | 13 ++ .../teams/handlers/SimpleActivityHandler.java | 24 +++ .../teams/handlers/TeamsResponseHandler.java | 35 +--- .../AbstractRetryingActivityHandler.java | 176 ++++++++++++++++++ .../retry/BasicMessageRetryHandler.java | 48 ----- ...a => InMemoryRetryingActivityHandler.java} | 30 ++- .../teams/handlers/retry/MessageRetry.java | 99 ---------- .../handlers/retry/MessageRetryHandler.java | 17 -- .../handlers/retry/NoOpRetryHandler.java | 30 --- .../handlers/retry/NoOpRetryHandlerTest.java | 26 +-- 12 files changed, 266 insertions(+), 276 deletions(-) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/ActivityHandler.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/SimpleActivityHandler.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java rename libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/{InMemoryMessageRetryHandler.java => InMemoryRetryingActivityHandler.java} (52%) delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java index 92bafb2a..7c557aa5 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java @@ -1,8 +1,8 @@ package org.finos.springbot.teams; -import org.finos.springbot.teams.handlers.TeamsResponseHandler; -import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; -import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; +import org.finos.springbot.teams.handlers.ActivityHandler; +import org.finos.springbot.teams.handlers.SimpleActivityHandler; +import org.finos.springbot.teams.handlers.retry.AbstractRetryingActivityHandler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; @@ -17,17 +17,14 @@ public class TeamsScheduledConfig implements SchedulingConfigurer { private static final Logger LOG = LoggerFactory.getLogger(TeamsScheduledConfig.class); @Autowired - private TeamsResponseHandler handler; - - @Autowired - private MessageRetryHandler retryHandler; + private ActivityHandler retryHandler; @Value("${teams.retry.time:30000}") private long teamsRetrySchedulerCron; @Override public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { - if (retryHandler instanceof NoOpRetryHandler) { + if (retryHandler instanceof SimpleActivityHandler) { LOG.info("No-operation retry handler is configured."); } else { Runnable runnable = () -> scheduleRetryMessage(); @@ -36,6 +33,10 @@ public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { } private void scheduleRetryMessage() { - handler.retryMessage(); + try { + ((AbstractRetryingActivityHandler) retryHandler).retryMessage(); + } catch (Throwable e) { + + } } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index c54c1804..52be2530 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -13,9 +13,9 @@ import org.finos.springbot.teams.conversations.TeamsConversationsConfig; import org.finos.springbot.teams.form.TeamsFormConverter; import org.finos.springbot.teams.form.TeamsFormDeserializerModule; +import org.finos.springbot.teams.handlers.ActivityHandler; import org.finos.springbot.teams.handlers.TeamsResponseHandler; -import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; -import org.finos.springbot.teams.handlers.retry.NoOpRetryHandler; +import org.finos.springbot.teams.handlers.retry.InMemoryRetryingActivityHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; @@ -129,16 +129,20 @@ public TeamsResponseHandler teamsResponseHandler( AdaptiveCardTemplateProvider formTemplater, ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, - TeamsConversations tc, - MessageRetryHandler mr) { + ActivityHandler ah) { return new TeamsResponseHandler( null, // attachment handler markupTemplater, formTemplater, displayTemplater, th, - tc, - mr); + ah); + } + + @Bean + @ConditionalOnMissingBean + public ActivityHandler activityHandler(TeamsConversations tc) { + return new InMemoryRetryingActivityHandler(tc); } @Bean @@ -230,14 +234,5 @@ public void setResourceLoaderClassLoader() { resourceLoader.setClassLoader(this.getClass().getClassLoader()); } - /** - * set InMemoryMessageRetryHandler() if you want retry - * @return - */ - @Bean - @ConditionalOnMissingBean - public MessageRetryHandler messageRetryHandler() { - return new NoOpRetryHandler(); - } } \ No newline at end of file diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/ActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/ActivityHandler.java new file mode 100644 index 00000000..f2227429 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/ActivityHandler.java @@ -0,0 +1,13 @@ +package org.finos.springbot.teams.handlers; + +import java.util.concurrent.CompletableFuture; + +import org.finos.springbot.teams.content.TeamsAddressable; + +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ResourceResponse; + +public interface ActivityHandler { + + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to); +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/SimpleActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/SimpleActivityHandler.java new file mode 100644 index 00000000..6f32ee14 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/SimpleActivityHandler.java @@ -0,0 +1,24 @@ +package org.finos.springbot.teams.handlers; + +import java.util.concurrent.CompletableFuture; + +import org.finos.springbot.teams.content.TeamsAddressable; +import org.finos.springbot.teams.conversations.TeamsConversations; + +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ResourceResponse; + +public class SimpleActivityHandler implements ActivityHandler { + + private TeamsConversations tc; + + public SimpleActivityHandler(TeamsConversations tc) { + this.tc = tc; + } + + @Override + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to) { + return tc.handleActivity(activity, to); + } + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 3d62a110..499459d5 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -3,16 +3,12 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import java.util.function.BiFunction; import org.finos.springbot.teams.TeamsException; import org.finos.springbot.teams.content.TeamsAddressable; -import org.finos.springbot.teams.conversations.TeamsConversations; -import org.finos.springbot.teams.handlers.retry.MessageRetry; -import org.finos.springbot.teams.handlers.retry.MessageRetryHandler; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; import org.finos.springbot.teams.response.templating.EntityMarkupTemplateProvider; @@ -60,8 +56,7 @@ public class TeamsResponseHandler implements ResponseHandler, ApplicationContext protected AdaptiveCardTemplateProvider workTemplater; protected ThymeleafTemplateProvider displayTemplater; protected TeamsStateStorage teamsState; - protected TeamsConversations teamsConversations; - protected MessageRetryHandler retryHandler; + protected ActivityHandler ah; public TeamsResponseHandler( AttachmentHandler attachmentHandler, @@ -69,15 +64,13 @@ public TeamsResponseHandler( AdaptiveCardTemplateProvider workTemplater, ThymeleafTemplateProvider displayTemplater, TeamsStateStorage th, - TeamsConversations tc, - MessageRetryHandler mr) { + ActivityHandler ah) { this.attachmentHandler = attachmentHandler; this.messageTemplater = messageTemplater; this.workTemplater = workTemplater; this.displayTemplater = displayTemplater; this.teamsState = th; - this.teamsConversations = tc; - this.retryHandler = mr; + this.ah = ah; } protected void initErrorHandler() { @@ -159,7 +152,7 @@ protected CompletableFuture sendXMLResponse(String xml, Object out.setEntities(entities); out.setTextFormat(TextFormatTypes.XML); out.setText(xml); - return teamsConversations.handleActivity(out, address); + return ah.handleActivity(out, address); } private BiFunction handleButtonsIfNeeded(TemplateType tt, WorkResponse wr) { @@ -192,9 +185,7 @@ private BiFunction handle private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t, int retryCount) { return (rr, e) -> { - if (e != null) { - boolean success = retryHandler.handleException(t, retryCount, e); - if(!success) { + if (e != null) { LOG.error(e.getMessage()); if (out instanceof ObjectNode){ try { @@ -206,8 +197,7 @@ private BiFunction handle } initErrorHandler(); - eh.handleError(e); - } + eh.handleError(e); } else { performStorage(address, data, teamsState); } @@ -222,7 +212,7 @@ protected CompletableFuture sendCardResponse(JsonNode json, Te body.setContentType("application/vnd.microsoft.card.adaptive"); body.setContent(json); out.getAttachments().add(body); - return teamsConversations.handleActivity(out, address); + return ah.handleActivity(out, address); } public static void performStorage(TeamsAddressable address, Map data, TeamsStateStorage teamsState) { @@ -247,17 +237,6 @@ public static Map createStorageTags(Map data, Te return out; } - public void retryMessage() { - int messageCount = 0; - - Optional opt; - while ((opt = retryHandler.get()).isPresent()) { - messageCount++; - this.sendResponse(opt.get().getResponse(), opt.get().getRetryCount()); - } - - LOG.info("Retry message queue {}" , messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); - } @Override diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java new file mode 100644 index 00000000..d909ca9e --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -0,0 +1,176 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.time.LocalDateTime; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; + +import org.apache.commons.lang3.StringUtils; +import org.finos.springbot.teams.content.TeamsAddressable; +import org.finos.springbot.teams.conversations.TeamsConversations; +import org.finos.springbot.teams.handlers.ActivityHandler; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; + +import com.microsoft.bot.connector.rest.ErrorResponseException; +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ResourceResponse; + +import okhttp3.ResponseBody; + +public abstract class AbstractRetryingActivityHandler implements ActivityHandler { + + protected static final Logger LOG = LoggerFactory.getLogger(AbstractRetryingActivityHandler.class); + + private TeamsConversations tc; + + @Value("${teams.retry.count:3}") + private long teamsRetryCount; + + public AbstractRetryingActivityHandler(TeamsConversations tc) { + this.tc = tc; + } + + public boolean isTooManyRequest(Throwable e) { + if (e instanceof CompletionException + && ((CompletionException) e).getCause() instanceof ErrorResponseException) { + ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); + retrofit2.Response response = ere.response(); + return (response.code() == HttpStatus.TOO_MANY_REQUESTS.value()); + } else { + return false; + } + } + + public long getRetryAfter(Exception e) { + ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); + retrofit2.Response response = ere.response(); + String retryAfter = response.headers().get("Retry-After"); + + LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); + long retryAfterInt = 1;// initiate to 1 sec + if (StringUtils.isNumeric(retryAfter)) { + retryAfterInt = Long.parseLong(retryAfter); + } + + return retryAfterInt; + } + + @Override + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to) { + return handleActivity(activity, to, 0); + } + + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to, Integer retryCount) { + return tc.handleActivity(activity, to).handle((rr, ex) -> { + if (ex != null) { + handleException(activity, to, retryCount, ex); + } + return rr; + }); + } + + public Boolean handleException(Activity activity, TeamsAddressable to, int retryCount, Throwable e) { + if (isTooManyRequest(e)) { + long retryAfter = getRetryAfter((CompletionException) e); + retryCount++; + if (retryCount <= teamsRetryCount) { + LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); + LocalDateTime time = LocalDateTime.now().plusSeconds(retryAfter); + add(new MessageRetry(activity, to, retryCount, retryAfter, time, e)); + return true; + } + } + return false; + } + + public void retryMessage() throws Throwable { + int messageCount = 0; + Optional opt; + while ((opt = get()).isPresent()) { + messageCount++; + MessageRetry msg = opt.get(); + this.handleActivity(msg.getActivity(), msg.getTo(), msg.getRetryCount()); + } + + LOG.info("Retry message queue {}", messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); + } + + + protected abstract void add(MessageRetry retry) ; + + protected abstract Optional get() ; + + class MessageRetry { + + private Activity activity; + private TeamsAddressable to; + private int retryCount; + private long retryAfter; + private LocalDateTime retryTime; + private Throwable e; + + + public MessageRetry(Activity activity, TeamsAddressable to, int retryCount, long retryAfter, + LocalDateTime retryTime, Throwable e) { + super(); + this.activity = activity; + this.to = to; + this.retryCount = retryCount; + this.retryAfter = retryAfter; + this.retryTime = retryTime; + this.e= e; + } + + public Activity getActivity() { + return activity; + } + + public void setActivity(Activity activity) { + this.activity = activity; + } + + public TeamsAddressable getTo() { + return to; + } + + public void setTo(TeamsAddressable to) { + this.to = to; + } + + public int getRetryCount() { + return retryCount; + } + + public void setRetryCount(int retryCount) { + this.retryCount = retryCount; + } + + public long getRetryAfter() { + return retryAfter; + } + + public void setRetryAfter(long retryAfter) { + this.retryAfter = retryAfter; + } + + public LocalDateTime getRetryTime() { + return retryTime; + } + + public void setRetryTime(LocalDateTime retryTime) { + this.retryTime = retryTime; + } + + public Throwable getE() { + return e; + } + + public void setE(Throwable e) { + this.e = e; + } + + } +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java deleted file mode 100644 index 9bd2b5f3..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/BasicMessageRetryHandler.java +++ /dev/null @@ -1,48 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.time.LocalDateTime; -import java.util.concurrent.CompletionException; - -import org.apache.commons.lang3.StringUtils; -import org.finos.springbot.workflow.response.Response; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.http.HttpStatus; - -import com.microsoft.bot.connector.rest.ErrorResponseException; - -import okhttp3.ResponseBody; - -public abstract class BasicMessageRetryHandler implements MessageRetryHandler { - - private static final Logger LOG = LoggerFactory.getLogger(BasicMessageRetryHandler.class); - - @Value("${teams.retry.count:3}") - private long teamsRetryCount; - - public boolean handleException(Response t, int retryCount, Throwable e) { - if (e instanceof CompletionException - && ((CompletionException) e).getCause() instanceof ErrorResponseException) { - ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); - retrofit2.Response response = ere.response(); - if (response.code() == HttpStatus.TOO_MANY_REQUESTS.value() && retryCount <= teamsRetryCount) { - String retryAfter = response.headers().get("Retry-After"); - LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); - - int retryAfterInt = 1;//initiate to 1 sec - if(StringUtils.isNumeric(retryAfter)) { - retryAfterInt = Integer.parseInt(retryAfter); - } - LocalDateTime time = LocalDateTime.now().plusSeconds(retryAfterInt); - add(new MessageRetry(t, retryCount, retryAfterInt, time)); - - return true; - } - } - - return false; - } - - -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java similarity index 52% rename from libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java rename to libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java index 93cf9bd4..b786c9a9 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java @@ -5,39 +5,33 @@ import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import org.finos.springbot.teams.conversations.TeamsConversations; -public class InMemoryMessageRetryHandler extends BasicMessageRetryHandler { +public class InMemoryRetryingActivityHandler extends AbstractRetryingActivityHandler { - private static final Logger LOG = LoggerFactory.getLogger(InMemoryMessageRetryHandler.class); - private Queue queue = new ConcurrentLinkedQueue<>(); - @Override - public void add(MessageRetry t) { - queue.add(t); + public InMemoryRetryingActivityHandler(TeamsConversations tc) { + super(tc); } - @Override - public void clearAll() { - while(queue.poll()!=null); + protected void add(MessageRetry retry) { + queue.add(retry); } - - @Override - public Optional get() { + + public Optional get() { MessageRetry q; if ((q = queue.peek()) != null) { if (LocalDateTime.now().isAfter(q.getRetryTime())) { // retry now queue.remove(q); return Optional.of(q); - }else { - LOG.info("Message not retried, as retry time {} for message has not passed the current time {}", q.getRetryTime(), LocalDateTime.now()); + } else { + LOG.info("Message not retried, as retry time {} for message has not passed the current time {}", + q.getRetryTime(), LocalDateTime.now()); } } - + return Optional.empty(); } - } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java deleted file mode 100644 index ff672fb5..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java +++ /dev/null @@ -1,99 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.time.LocalDateTime; - -import org.finos.springbot.workflow.response.Response; - -public class MessageRetry { - - private Response response; - private int retryCount; - private int retryAfter; - private LocalDateTime retryTime; - - public MessageRetry(Response response, int retryCount, int retryAfter, LocalDateTime retryTime) { - super(); - this.response = response; - this.retryCount = retryCount; - this.retryAfter = retryAfter; - this.retryTime = retryTime; - } - - public Response getResponse() { - return response; - } - - public void setResponse(Response response) { - this.response = response; - } - - public int getRetryCount() { - return retryCount; - } - - public void setRetryCount(int retryCount) { - this.retryCount = retryCount; - } - - public int getRetryAfter() { - return retryAfter; - } - - public void setRetryAfter(int retryAfter) { - this.retryAfter = retryAfter; - } - - - public LocalDateTime getRetryTime() { - return retryTime; - } - - public void setRetryTime(LocalDateTime retryTime) { - this.retryTime = retryTime; - } - - @Override - public String toString() { - return "MessageRetry [response=" + response + ", retryCount=" + retryCount + ", retryAfter=" + retryAfter - + ", localDate=" + retryTime + "]"; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((retryTime == null) ? 0 : retryTime.hashCode()); - result = prime * result + ((response == null) ? 0 : response.hashCode()); - result = prime * result + retryAfter; - result = prime * result + retryCount; - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - MessageRetry other = (MessageRetry) obj; - if (retryTime == null) { - if (other.retryTime != null) - return false; - } else if (!retryTime.equals(other.retryTime)) - return false; - if (response == null) { - if (other.response != null) - return false; - } else if (!response.equals(other.response)) - return false; - if (retryAfter != other.retryAfter) - return false; - if (retryCount != other.retryCount) - return false; - return true; - } - - -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java deleted file mode 100644 index 3759503c..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetryHandler.java +++ /dev/null @@ -1,17 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.util.Optional; - -import org.finos.springbot.workflow.response.Response; - -public interface MessageRetryHandler { - - public boolean handleException(Response t, int retryCount, Throwable e); - - public Optional get(); - - public void add(MessageRetry messageRetry); - - public void clearAll(); - -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java deleted file mode 100644 index 47d14354..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandler.java +++ /dev/null @@ -1,30 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.util.Optional; - -import org.finos.springbot.workflow.response.Response; - -public class NoOpRetryHandler implements MessageRetryHandler { - - @Override - public Optional get() { - return Optional.empty(); - } - - @Override - public boolean handleException(Response t, int retryCount, Throwable e) { - - return false; - } - - @Override - public void add(MessageRetry messageRetry) { - //Do-Nothing - } - - @Override - public void clearAll() { - //Do-Nothing - } - -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java index 731706fc..ee755498 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java @@ -1,29 +1,31 @@ package org.finos.springbot.teams.handlers.retry; import org.finos.springbot.teams.content.TeamsChannel; +import org.finos.springbot.teams.conversations.TeamsConversations; +import org.finos.springbot.teams.handlers.SimpleActivityHandler; import org.finos.springbot.workflow.data.DataHandlerConfig; -import org.finos.springbot.workflow.response.MessageResponse; -import org.finos.springbot.workflow.response.Response; -import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.Mockito; import org.springframework.boot.test.context.SpringBootTest; +import com.microsoft.bot.schema.Activity; + @SpringBootTest(classes = { DataHandlerConfig.class, }) public class NoOpRetryHandlerTest { - public NoOpRetryHandler noOpRetryHandler = new NoOpRetryHandler(); + @Mock + TeamsConversations tc; - @Test - public void getTest() { - Assertions.assertFalse(noOpRetryHandler.get().isPresent()); - } + @InjectMocks + private SimpleActivityHandler handler = new SimpleActivityHandler(tc); @Test - public void handleExceptionTest() { - Response t = new MessageResponse(new TeamsChannel(),""); - int retryCount=1; - Assertions.assertFalse(noOpRetryHandler.handleException(t,retryCount, new Exception())); + public void testHandleActivity() { + Activity activity = Mockito.mock(Activity.class); + handler.handleActivity(activity , new TeamsChannel()); } } From 4b9e8488beec1e28732af92e5bcd229a3b5e2fdd Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 31 Mar 2023 17:38:39 +0530 Subject: [PATCH 08/19] refactor retry message handler --- .../springbot/teams/TeamsWorkflowConfig.java | 4 +- .../InMemoryMessageRetryHandlerTest.java | 62 ------------------- .../InMemoryRetryingActivityHandlerTest.java | 41 ++++++++++++ ...st.java => SimpleActivityHandlerTest.java} | 2 +- 4 files changed, 44 insertions(+), 65 deletions(-) delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java rename libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/{NoOpRetryHandlerTest.java => SimpleActivityHandlerTest.java} (95%) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 52be2530..0d3267eb 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -14,8 +14,8 @@ import org.finos.springbot.teams.form.TeamsFormConverter; import org.finos.springbot.teams.form.TeamsFormDeserializerModule; import org.finos.springbot.teams.handlers.ActivityHandler; +import org.finos.springbot.teams.handlers.SimpleActivityHandler; import org.finos.springbot.teams.handlers.TeamsResponseHandler; -import org.finos.springbot.teams.handlers.retry.InMemoryRetryingActivityHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; @@ -142,7 +142,7 @@ public TeamsResponseHandler teamsResponseHandler( @Bean @ConditionalOnMissingBean public ActivityHandler activityHandler(TeamsConversations tc) { - return new InMemoryRetryingActivityHandler(tc); + return new SimpleActivityHandler(tc); } @Bean diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java deleted file mode 100644 index 8ede6491..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryMessageRetryHandlerTest.java +++ /dev/null @@ -1,62 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.time.LocalDateTime; -import java.util.HashMap; -import java.util.Map; -import java.util.Optional; - -import org.finos.springbot.teams.content.TeamsChannel; -import org.finos.springbot.workflow.data.DataHandlerConfig; -import org.finos.springbot.workflow.response.DataResponse; -import org.finos.springbot.workflow.response.Response; -import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.Test; -import org.springframework.boot.test.context.SpringBootTest; - -@SpringBootTest(classes = { - DataHandlerConfig.class, - }) - -public class InMemoryMessageRetryHandlerTest { - - InMemoryMessageRetryHandler inMemoryMessageRetryHandler = new InMemoryMessageRetryHandler(); - TeamsChannel dummyChat = new TeamsChannel(); - Object dummyObject = new Object(); - - public void setUp(LocalDateTime retryTime1,LocalDateTime retryTime2) { - inMemoryMessageRetryHandler.clearAll(); - Map data1 = new HashMap<>(); - data1.put("key1", dummyObject); - Map data2 = new HashMap<>(); - data2.put("key2", dummyObject); - Response r1 = new DataResponse(dummyChat, data1, ""); - Response r2 = new DataResponse(dummyChat, data2, ""); - MessageRetry t1 = new MessageRetry(r1,3,45,retryTime1); - MessageRetry t2 = new MessageRetry(r2,3,45,retryTime2); - inMemoryMessageRetryHandler.add(t1); - inMemoryMessageRetryHandler.add(t2); - } - @Test - public void testFoundMessageForRetry() { - setUp(LocalDateTime.now().minusSeconds(100),LocalDateTime.now().plusSeconds(150)); - Map expectedData = new HashMap<>(); - expectedData.put("key1", dummyObject); - DataResponse actualResponse = (DataResponse) inMemoryMessageRetryHandler.get().get().getResponse(); - Assertions.assertTrue(actualResponse.getData().equals(expectedData)); - } - - @Test - public void testNoMessageEligibleForRetry(){ - setUp(LocalDateTime.now().plusSeconds(100),LocalDateTime.now().plusSeconds(150)); - Optional actualResponse = inMemoryMessageRetryHandler.get(); - Assertions.assertFalse(actualResponse.isPresent()); - } - - @Test - public void testNoMessageInQueue() { - inMemoryMessageRetryHandler.clearAll(); - Optional actualResponse = inMemoryMessageRetryHandler.get(); - Assertions.assertFalse(actualResponse.isPresent()); - - } -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java new file mode 100644 index 00000000..d3baff26 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java @@ -0,0 +1,41 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.time.LocalDateTime; +import java.util.Optional; + +import org.finos.springbot.teams.content.TeamsAddressable; +import org.finos.springbot.teams.content.TeamsChannel; +import org.finos.springbot.teams.handlers.retry.AbstractRetryingActivityHandler.MessageRetry; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class InMemoryRetryingActivityHandlerTest { + + private InMemoryRetryingActivityHandler inhandler = new InMemoryRetryingActivityHandler(null); + TeamsChannel dummyChat1 = new TeamsChannel("dummy_id_1", "dummy_name"); + TeamsChannel dummyChat2 = new TeamsChannel("dummy_id_2", "dummy_name"); + + private void setUp(LocalDateTime retryTime) { + MessageRetry mr = inhandler.new MessageRetry(null,dummyChat1,3, 45, retryTime, null); + inhandler.add(mr); + } + + @Test + public void testFoundMessageForRetry() { + setUp(LocalDateTime.now().minusSeconds(100)); + setUp(LocalDateTime.now().plusSeconds(100)); + + TeamsAddressable address = inhandler.get().get().getTo(); + Assertions.assertTrue(address.getKey().equals(dummyChat1.getKey())); + } + + @Test + public void testNoMessageEligibleForRetry(){ + setUp(LocalDateTime.now().plusSeconds(100)); + setUp(LocalDateTime.now().plusSeconds(150)); + + Optional actualResponse = inhandler.get(); + Assertions.assertFalse(actualResponse.isPresent()); + } + +} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/SimpleActivityHandlerTest.java similarity index 95% rename from libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java rename to libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/SimpleActivityHandlerTest.java index ee755498..e56f26b4 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/NoOpRetryHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/SimpleActivityHandlerTest.java @@ -15,7 +15,7 @@ @SpringBootTest(classes = { DataHandlerConfig.class, }) -public class NoOpRetryHandlerTest { +public class SimpleActivityHandlerTest { @Mock TeamsConversations tc; From 5c475cae120765a865405eeda4d45ab215aa10e4 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 31 Mar 2023 19:04:50 +0530 Subject: [PATCH 09/19] refactor retry message handler --- .../springbot/teams/TeamsWorkflowConfig.java | 4 +- .../teams/handlers/TeamsResponseHandler.java | 15 ++-- .../AbstractRetryingActivityHandler.java | 73 ++++++++----------- .../InMemoryRetryingActivityHandler.java | 4 +- .../InMemoryRetryingActivityHandlerTest.java | 4 +- 5 files changed, 44 insertions(+), 56 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 0d3267eb..dc094aae 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -16,6 +16,7 @@ import org.finos.springbot.teams.handlers.ActivityHandler; import org.finos.springbot.teams.handlers.SimpleActivityHandler; import org.finos.springbot.teams.handlers.TeamsResponseHandler; +import org.finos.springbot.teams.handlers.retry.InMemoryRetryingActivityHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; @@ -139,10 +140,11 @@ public TeamsResponseHandler teamsResponseHandler( ah); } + //For retry message on TOO_MANY_REQUESTS exception used InMemoryRetryingActivityHandler @Bean @ConditionalOnMissingBean public ActivityHandler activityHandler(TeamsConversations tc) { - return new SimpleActivityHandler(tc); + return new InMemoryRetryingActivityHandler(tc); } @Bean diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 499459d5..06c194d6 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -47,7 +47,7 @@ public class TeamsResponseHandler implements ResponseHandler, ApplicationContext private static final Logger LOG = LoggerFactory.getLogger(TeamsResponseHandler.class); - private static final int INIT_RETRY_COUNT = 0; + protected AttachmentHandler attachmentHandler; protected ApplicationContext ctx; @@ -83,10 +83,6 @@ enum TemplateType { ADAPTIVE_CARD, THYMELEAF, BOTH }; @Override public void accept(Response t) { - sendResponse(t, INIT_RETRY_COUNT); - } - - public void sendResponse(Response t, int retryCount) { if (t.getAddress() instanceof TeamsAddressable) { TeamsAddressable ta = (TeamsAddressable) t.getAddress(); @@ -103,7 +99,7 @@ public void sendResponse(Response t, int retryCount) { } sendXMLResponse(content, attachment, ta, entities, mr.getData()) - .handle(handleErrorAndStorage(content, ta, mr.getData(), t, ++retryCount)); + .handle(handleErrorAndStorage(content, ta, mr.getData(), t)); } else if (t instanceof WorkResponse) { WorkResponse wr = (WorkResponse) t; @@ -112,7 +108,7 @@ public void sendResponse(Response t, int retryCount) { if (tt == TemplateType.ADAPTIVE_CARD) { JsonNode cardJson = workTemplater.template(wr); sendCardResponse(cardJson, ta, wr.getData()) - .handle(handleErrorAndStorage(cardJson, ta, wr.getData(), t, ++retryCount)); + .handle(handleErrorAndStorage(cardJson, ta, wr.getData(), t)); ; } else { MarkupAndEntities mae = displayTemplater.template(wr); @@ -120,7 +116,7 @@ public void sendResponse(Response t, int retryCount) { List entities = mae.getEntities(); sendXMLResponse(content, null, ta, entities, wr.getData()) .handle(handleButtonsIfNeeded(tt, wr)) - .handle(handleErrorAndStorage(content, ta, wr.getData(), t, ++retryCount)); + .handle(handleErrorAndStorage(content, ta, wr.getData(), t)); } } @@ -130,6 +126,7 @@ public void sendResponse(Response t, int retryCount) { } } + protected TemplateType getTemplateType(WorkResponse wr) { TemplateType tt; if (displayTemplater.hasTemplate(wr)) { @@ -183,7 +180,7 @@ private BiFunction handle }; } - private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t, int retryCount) { + private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t) { return (rr, e) -> { if (e != null) { LOG.error(e.getMessage()); diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java index d909ca9e..edf301cc 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -6,6 +6,7 @@ import java.util.concurrent.CompletionException; import org.apache.commons.lang3.StringUtils; +import org.finos.springbot.teams.TeamsException; import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.conversations.TeamsConversations; import org.finos.springbot.teams.handlers.ActivityHandler; @@ -29,6 +30,8 @@ public abstract class AbstractRetryingActivityHandler implements ActivityHandler @Value("${teams.retry.count:3}") private long teamsRetryCount; + private static final int INIT_RETRY_COUNT = 0; + public AbstractRetryingActivityHandler(TeamsConversations tc) { this.tc = tc; } @@ -44,12 +47,11 @@ public boolean isTooManyRequest(Throwable e) { } } - public long getRetryAfter(Exception e) { + public long getRetryAfter(CompletionException e) { ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); retrofit2.Response response = ere.response(); String retryAfter = response.headers().get("Retry-After"); - LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); long retryAfterInt = 1;// initiate to 1 sec if (StringUtils.isNumeric(retryAfter)) { retryAfterInt = Long.parseLong(retryAfter); @@ -60,15 +62,22 @@ public long getRetryAfter(Exception e) { @Override public CompletableFuture handleActivity(Activity activity, TeamsAddressable to) { - return handleActivity(activity, to, 0); + return handleActivity(activity, to, INIT_RETRY_COUNT); } public CompletableFuture handleActivity(Activity activity, TeamsAddressable to, Integer retryCount) { return tc.handleActivity(activity, to).handle((rr, ex) -> { if (ex != null) { - handleException(activity, to, retryCount, ex); + Boolean success = handleException(activity, to, retryCount, ex); + if(!success) { + throw new TeamsException("Couldn't handle response ", ex); + }else { + return null; + } + }else { + return rr; } - return rr; + }); } @@ -77,9 +86,9 @@ public Boolean handleException(Activity activity, TeamsAddressable to, int retry long retryAfter = getRetryAfter((CompletionException) e); retryCount++; if (retryCount <= teamsRetryCount) { - LOG.info("MessageRetryHandler request retryAfter {}", retryAfter); - LocalDateTime time = LocalDateTime.now().plusSeconds(retryAfter); - add(new MessageRetry(activity, to, retryCount, retryAfter, time, e)); + LOG.info("AbstractRetryingActivityHandler request retryAfter {}", retryAfter); + LocalDateTime retryAfterTime = LocalDateTime.now().plusSeconds(retryAfter); + add(new MessageRetry(activity, to, retryCount, retryAfterTime)); return true; } } @@ -92,7 +101,7 @@ public void retryMessage() throws Throwable { while ((opt = get()).isPresent()) { messageCount++; MessageRetry msg = opt.get(); - this.handleActivity(msg.getActivity(), msg.getTo(), msg.getRetryCount()); + this.handleActivity(msg.getActivity(), msg.getAddressable(), msg.getRetryCount()); } LOG.info("Retry message queue {}", messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); @@ -106,22 +115,17 @@ public void retryMessage() throws Throwable { class MessageRetry { private Activity activity; - private TeamsAddressable to; + private TeamsAddressable addressable; private int retryCount; - private long retryAfter; - private LocalDateTime retryTime; - private Throwable e; + private LocalDateTime retryAfterTime; - public MessageRetry(Activity activity, TeamsAddressable to, int retryCount, long retryAfter, - LocalDateTime retryTime, Throwable e) { + public MessageRetry(Activity activity, TeamsAddressable addressable, int retryCount, LocalDateTime retryAfterTime) { super(); this.activity = activity; - this.to = to; + this.addressable = addressable; this.retryCount = retryCount; - this.retryAfter = retryAfter; - this.retryTime = retryTime; - this.e= e; + this.retryAfterTime = retryAfterTime; } public Activity getActivity() { @@ -132,12 +136,12 @@ public void setActivity(Activity activity) { this.activity = activity; } - public TeamsAddressable getTo() { - return to; + public TeamsAddressable getAddressable() { + return addressable; } - public void setTo(TeamsAddressable to) { - this.to = to; + public void setAddressable(TeamsAddressable addressable) { + this.addressable = addressable; } public int getRetryCount() { @@ -148,29 +152,14 @@ public void setRetryCount(int retryCount) { this.retryCount = retryCount; } - public long getRetryAfter() { - return retryAfter; - } - - public void setRetryAfter(long retryAfter) { - this.retryAfter = retryAfter; - } - - public LocalDateTime getRetryTime() { - return retryTime; - } - - public void setRetryTime(LocalDateTime retryTime) { - this.retryTime = retryTime; + public LocalDateTime getRetryAfterTime() { + return retryAfterTime; } - public Throwable getE() { - return e; + public void setRetryAfterTime(LocalDateTime retryAfterTime) { + this.retryAfterTime = retryAfterTime; } - public void setE(Throwable e) { - this.e = e; - } } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java index b786c9a9..b10ee99d 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java @@ -22,12 +22,12 @@ protected void add(MessageRetry retry) { public Optional get() { MessageRetry q; if ((q = queue.peek()) != null) { - if (LocalDateTime.now().isAfter(q.getRetryTime())) { // retry now + if (LocalDateTime.now().isAfter(q.getRetryAfterTime())) { // retry now queue.remove(q); return Optional.of(q); } else { LOG.info("Message not retried, as retry time {} for message has not passed the current time {}", - q.getRetryTime(), LocalDateTime.now()); + q.getRetryAfterTime(), LocalDateTime.now()); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java index d3baff26..c29771e0 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java @@ -16,7 +16,7 @@ public class InMemoryRetryingActivityHandlerTest { TeamsChannel dummyChat2 = new TeamsChannel("dummy_id_2", "dummy_name"); private void setUp(LocalDateTime retryTime) { - MessageRetry mr = inhandler.new MessageRetry(null,dummyChat1,3, 45, retryTime, null); + MessageRetry mr = inhandler.new MessageRetry(null,dummyChat1,3, retryTime); inhandler.add(mr); } @@ -25,7 +25,7 @@ public void testFoundMessageForRetry() { setUp(LocalDateTime.now().minusSeconds(100)); setUp(LocalDateTime.now().plusSeconds(100)); - TeamsAddressable address = inhandler.get().get().getTo(); + TeamsAddressable address = inhandler.get().get().getAddressable(); Assertions.assertTrue(address.getKey().equals(dummyChat1.getKey())); } From a5496fe9d79a058560b3655acda627216e079420 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Tue, 4 Apr 2023 14:40:20 +0530 Subject: [PATCH 10/19] refactor retry message handler --- .../springbot/teams/TeamsScheduledConfig.java | 5 +-- .../springbot/teams/TeamsWorkflowConfig.java | 2 +- .../teams/handlers/TeamsResponseHandler.java | 5 +-- .../AbstractRetryingActivityHandler.java | 39 +++++++++---------- 4 files changed, 23 insertions(+), 28 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java index 7c557aa5..fde4de8b 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java @@ -33,10 +33,7 @@ public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { } private void scheduleRetryMessage() { - try { - ((AbstractRetryingActivityHandler) retryHandler).retryMessage(); - } catch (Throwable e) { - } + ((AbstractRetryingActivityHandler) retryHandler).retryMessage(); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index dc094aae..438be41c 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -144,7 +144,7 @@ public TeamsResponseHandler teamsResponseHandler( @Bean @ConditionalOnMissingBean public ActivityHandler activityHandler(TeamsConversations tc) { - return new InMemoryRetryingActivityHandler(tc); + return new SimpleActivityHandler(tc); } @Bean diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 06c194d6..58fd6b67 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -109,7 +109,6 @@ public void accept(Response t) { JsonNode cardJson = workTemplater.template(wr); sendCardResponse(cardJson, ta, wr.getData()) .handle(handleErrorAndStorage(cardJson, ta, wr.getData(), t)); - ; } else { MarkupAndEntities mae = displayTemplater.template(wr); String content = mae.getContents(); @@ -163,7 +162,7 @@ private BiFunction handle JsonNode expandedJson = workTemplater.applyTemplate(buttonsJson, wr); return sendCardResponse(expandedJson, (TeamsAddressable) wr.getAddress(), wr.getData()).get(); } else { - return null; + return rr; } } else { @@ -195,7 +194,7 @@ private BiFunction handle initErrorHandler(); eh.handleError(e); - } else { + } else if(rr != null) { performStorage(address, data, teamsState); } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java index edf301cc..287c47cb 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -31,7 +31,7 @@ public abstract class AbstractRetryingActivityHandler implements ActivityHandler private long teamsRetryCount; private static final int INIT_RETRY_COUNT = 0; - + public AbstractRetryingActivityHandler(TeamsConversations tc) { this.tc = tc; } @@ -65,23 +65,24 @@ public CompletableFuture handleActivity(Activity activity, Tea return handleActivity(activity, to, INIT_RETRY_COUNT); } - public CompletableFuture handleActivity(Activity activity, TeamsAddressable to, Integer retryCount) { + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to, + Integer retryCount) { return tc.handleActivity(activity, to).handle((rr, ex) -> { if (ex != null) { - Boolean success = handleException(activity, to, retryCount, ex); - if(!success) { + Boolean success = handleToManyRequestException(activity, to, retryCount, ex); + if (!success) { throw new TeamsException("Couldn't handle response ", ex); - }else { - return null; + } else { + return null;//don't do any think, error response is inserted into queue } - }else { + } else { return rr; } - + }); } - public Boolean handleException(Activity activity, TeamsAddressable to, int retryCount, Throwable e) { + public Boolean handleToManyRequestException(Activity activity, TeamsAddressable to, int retryCount, Throwable e) { if (isTooManyRequest(e)) { long retryAfter = getRetryAfter((CompletionException) e); retryCount++; @@ -94,8 +95,8 @@ public Boolean handleException(Activity activity, TeamsAddressable to, int retry } return false; } - - public void retryMessage() throws Throwable { + + public void retryMessage() { int messageCount = 0; Optional opt; while ((opt = get()).isPresent()) { @@ -107,20 +108,19 @@ public void retryMessage() throws Throwable { LOG.info("Retry message queue {}", messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); } - - protected abstract void add(MessageRetry retry) ; - - protected abstract Optional get() ; - + protected abstract void add(MessageRetry retry); + + protected abstract Optional get(); + class MessageRetry { private Activity activity; private TeamsAddressable addressable; private int retryCount; private LocalDateTime retryAfterTime; - - - public MessageRetry(Activity activity, TeamsAddressable addressable, int retryCount, LocalDateTime retryAfterTime) { + + public MessageRetry(Activity activity, TeamsAddressable addressable, int retryCount, + LocalDateTime retryAfterTime) { super(); this.activity = activity; this.addressable = addressable; @@ -160,6 +160,5 @@ public void setRetryAfterTime(LocalDateTime retryAfterTime) { this.retryAfterTime = retryAfterTime; } - } } From e2c1e8ee069157328340bcb388da8ba19a374d7b Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Tue, 4 Apr 2023 18:47:26 +0530 Subject: [PATCH 11/19] refactor retry message handler --- .../teams/handlers/retry/AbstractRetryingActivityHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java index 287c47cb..47eda5ad 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -73,7 +73,7 @@ public CompletableFuture handleActivity(Activity activity, Tea if (!success) { throw new TeamsException("Couldn't handle response ", ex); } else { - return null;//don't do any think, error response is inserted into queue + return null; } } else { return rr; From 320488b2b314041fab0498f244763a1c2250973c Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Wed, 5 Apr 2023 14:38:45 +0530 Subject: [PATCH 12/19] refactor retry message handler --- .../java/org/finos/springbot/teams/TeamsWorkflowConfig.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 438be41c..7d065c4f 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -140,7 +140,9 @@ public TeamsResponseHandler teamsResponseHandler( ah); } - //For retry message on TOO_MANY_REQUESTS exception used InMemoryRetryingActivityHandler + /** + If you want to include retry logic for activities, override this bean and return an instance of InMemoryRetryingActivityHandler + */ @Bean @ConditionalOnMissingBean public ActivityHandler activityHandler(TeamsConversations tc) { From 115d93e82c634cd81af45ba566caf84972134f97 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 7 Apr 2023 10:26:36 +0530 Subject: [PATCH 13/19] Fixed state storage testcases --- .../teams/state/AbstractStateStorageTest.java | 22 ++++++++++ .../state/AzureBlobStateStorageTest.java | 43 +++++++++++++++++-- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java index 42fd5a61..99e0b0e0 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java @@ -72,6 +72,28 @@ public void testStoreWithTagDates() throws IOException { Assertions.assertEquals(2, hoover(tss.retrieve(tagList4, false)).size()); } + @Test + public void testSlashStoreWithMultTags() throws IOException { + Map somedata = Collections.singletonMap("a", "b"); + Map tags1 = new HashMap(); + tags1.put("addressable", "thefile"); + tags1.put("object1", "tag"); + + Map tags2 = new HashMap(); + tags2.put("addressable", "thefile"); + tags2.put("object2", "tag"); + + List tagList1 = Arrays.asList( + new Filter("addressable", "thefile", "="), + new Filter("object1", "tag", "=") + ); + + tss.store("thefile/thefile", tags1, somedata); + tss.store("thefile/theotherfile", tags2, somedata); + + Assertions.assertEquals(1, hoover(tss.retrieve(tagList1, false)).size()); + } + public List> hoover(Iterable> iterable) { List> result = StreamSupport.stream(iterable.spliterator(), false) .collect(Collectors.toList()); diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java index 47a2a072..e46866e6 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java @@ -58,6 +58,8 @@ public void setup() { BlobClient theFileBlobClient; BlobClient theOtherFileBlobClient; BlobClient nonFileBlobClient; + BlobClient theFileWithSlashBlobClient; + BlobClient theOtherFileWithSlashBlobClient; private BlobServiceClient createBlobServiceClientMock() { bsc = Mockito.mock(BlobServiceClient.class); @@ -65,7 +67,8 @@ private BlobServiceClient createBlobServiceClientMock() { theFileBlobClient = Mockito.mock(BlobClient.class); theOtherFileBlobClient = Mockito.mock(BlobClient.class); nonFileBlobClient = Mockito.mock(BlobClient.class); - + theFileWithSlashBlobClient = Mockito.mock(BlobClient.class); + theOtherFileWithSlashBlobClient = Mockito.mock(BlobClient.class); Mockito.when(bsc.getBlobContainerClient("test")) .thenReturn(bcc); @@ -75,7 +78,8 @@ private BlobServiceClient createBlobServiceClientMock() { Mockito.when(bcc.getBlobClient("thefile")).thenReturn(theFileBlobClient); Mockito.when(bcc.getBlobClient("theotherfile")).thenReturn(theOtherFileBlobClient); Mockito.when(bcc.getBlobClient("nonfile")).thenReturn(nonFileBlobClient); - + Mockito.when(bcc.getBlobClient("thefile/thefile")).thenReturn(theFileWithSlashBlobClient); + Mockito.when(bcc.getBlobClient("thefile/theotherfile")).thenReturn(theOtherFileWithSlashBlobClient); return bsc; } @@ -118,6 +122,15 @@ private void allowReadingFromTheFile() throws IOException { private void allowReadingFromTheOtherFile() throws IOException { Mockito.when(theOtherFileBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); } + + private void allowReadingFromTheSlashFile() throws IOException { + Mockito.when(theFileWithSlashBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); + } + + private void allowReadingFromTheOtherSlashFile() throws IOException { + Mockito.when(theOtherFileWithSlashBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); + } + private BlobInputStream setupBlobInputStream() throws IOException { BlobInputStream bis = Mockito.mock(BlobInputStream.class); @@ -144,6 +157,16 @@ private void allowWritingToTheOtherFile() { mockBlobOutputStream(theOtherFileBlobClient); } + private void allowWritingToTheSlashFile() { + mockBlobOutputStream(theFileWithSlashBlobClient); + } + + private void allowWritingToTheOtherSlashFile() { + mockBlobOutputStream(theOtherFileWithSlashBlobClient); + } + + + private void mockBlobOutputStream(BlobClient bc) { Mockito.doAnswer(new Answer() { public Void answer(InvocationOnMock a) { @@ -176,6 +199,20 @@ public void testStoreWithTagDates() throws IOException { super.testStoreWithTagDates(); } - + @Override + @Test + public void testSlashStoreWithMultTags() throws IOException { + allowWritingToTheSlashFile(); + allowWritingToTheOtherSlashFile(); + allowReadingFromTheSlashFile(); + allowReadingFromTheOtherSlashFile(); + + Map> queries = new HashMap<>(); + queries.put("@container='test' AND addressable='thefile' AND object1='tag'", Arrays.asList("thefile/thefile")); + + setupBlobSearch(queries); + + super.testSlashStoreWithMultTags(); + } } From eb8de4aa9141d70a9822b22d2083def1c94f8a0d Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 14 Apr 2023 12:18:47 +0530 Subject: [PATCH 14/19] State storage testcases. --- .../springbot/teams/state/AbstractStateStorageTest.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java index 99e0b0e0..1b6f01e2 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java @@ -88,10 +88,16 @@ public void testSlashStoreWithMultTags() throws IOException { new Filter("object1", "tag", "=") ); + List tagList2 = Arrays.asList( + new Filter("addressable", "theotherfile", "="), + new Filter("object2", "tag", "=") + ); + tss.store("thefile/thefile", tags1, somedata); tss.store("thefile/theotherfile", tags2, somedata); - Assertions.assertEquals(1, hoover(tss.retrieve(tagList1, false)).size()); + Assertions.assertEquals(1, hoover(tss.retrieve(tagList1, false)).size()); + Assertions.assertEquals(0, hoover(tss.retrieve(tagList2, false)).size()); } public List> hoover(Iterable> iterable) { From 8f823611da53e44a55210f7169bf2dbc66fd87ae Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Fri, 21 Apr 2023 13:30:45 +0530 Subject: [PATCH 15/19] refactor file stage storage and testcase --- .../springbot/teams/TeamsWorkflowConfig.java | 1 - .../AbstractRetryingActivityHandler.java | 55 +--------- .../teams/handlers/retry/MessageRetry.java | 93 ++++++++++++++++ .../teams/state/FileStateStorage.java | 9 +- .../InMemoryRetryingActivityHandlerTest.java | 101 +++++++++++++----- .../teams/state/AbstractStateStorageTest.java | 32 +++--- .../state/AzureBlobStateStorageTest.java | 35 +++--- .../teams/state/FileStateStorageTest.java | 16 +++ 8 files changed, 229 insertions(+), 113 deletions(-) create mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 7d065c4f..9c7f51bd 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -16,7 +16,6 @@ import org.finos.springbot.teams.handlers.ActivityHandler; import org.finos.springbot.teams.handlers.SimpleActivityHandler; import org.finos.springbot.teams.handlers.TeamsResponseHandler; -import org.finos.springbot.teams.handlers.retry.InMemoryRetryingActivityHandler; import org.finos.springbot.teams.history.StateStorageBasedTeamsHistory; import org.finos.springbot.teams.history.StorageIDResponseHandler; import org.finos.springbot.teams.history.TeamsHistory; diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java index 47eda5ad..f0bdd570 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -88,7 +88,7 @@ public Boolean handleToManyRequestException(Activity activity, TeamsAddressable retryCount++; if (retryCount <= teamsRetryCount) { LOG.info("AbstractRetryingActivityHandler request retryAfter {}", retryAfter); - LocalDateTime retryAfterTime = LocalDateTime.now().plusSeconds(retryAfter); + LocalDateTime retryAfterTime = LocalDateTime.now().plusSeconds(retryAfter); add(new MessageRetry(activity, to, retryCount, retryAfterTime)); return true; } @@ -102,63 +102,16 @@ public void retryMessage() { while ((opt = get()).isPresent()) { messageCount++; MessageRetry msg = opt.get(); - this.handleActivity(msg.getActivity(), msg.getAddressable(), msg.getRetryCount()); + this.handleActivity(msg.getActivity(), msg.getTeamsAddressable(), msg.getRetryCount()); } LOG.info("Retry message queue {}", messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); } + protected abstract void add(MessageRetry retry); protected abstract Optional get(); - class MessageRetry { - - private Activity activity; - private TeamsAddressable addressable; - private int retryCount; - private LocalDateTime retryAfterTime; - - public MessageRetry(Activity activity, TeamsAddressable addressable, int retryCount, - LocalDateTime retryAfterTime) { - super(); - this.activity = activity; - this.addressable = addressable; - this.retryCount = retryCount; - this.retryAfterTime = retryAfterTime; - } - - public Activity getActivity() { - return activity; - } - - public void setActivity(Activity activity) { - this.activity = activity; - } - - public TeamsAddressable getAddressable() { - return addressable; - } - - public void setAddressable(TeamsAddressable addressable) { - this.addressable = addressable; - } - - public int getRetryCount() { - return retryCount; - } - - public void setRetryCount(int retryCount) { - this.retryCount = retryCount; - } - - public LocalDateTime getRetryAfterTime() { - return retryAfterTime; - } - - public void setRetryAfterTime(LocalDateTime retryAfterTime) { - this.retryAfterTime = retryAfterTime; - } - - } + } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java new file mode 100644 index 00000000..5bcda074 --- /dev/null +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java @@ -0,0 +1,93 @@ +package org.finos.springbot.teams.handlers.retry; + +import java.time.LocalDateTime; + +import org.finos.springbot.teams.content.TeamsAddressable; +import org.finos.springbot.teams.content.TeamsChannel; +import org.finos.springbot.teams.content.TeamsUser; + +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; +import com.microsoft.bot.schema.Activity; + +@JsonIgnoreProperties(ignoreUnknown = true) +public class MessageRetry { + + private Activity activity; + private String teamsAddressableId; + private int retryCount; + private LocalDateTime retryAfterTime; + private Class teamsAddressableClass; + + public MessageRetry() { + + } + + public MessageRetry(Activity activity, TeamsAddressable teamsAddressable, int retryCount, LocalDateTime retryAfterTime) { + this.activity = activity; + this.teamsAddressableId = teamsAddressable.getKey(); + this.retryCount = retryCount; + this.retryAfterTime = retryAfterTime; + this.teamsAddressableClass = teamsAddressable.getClass(); + } + + public Activity getActivity() { + return activity; + } + + public void setActivity(Activity activity) { + this.activity = activity; + } + + public int getRetryCount() { + return retryCount; + } + + public void setRetryCount(int retryCount) { + this.retryCount = retryCount; + } + + public LocalDateTime getRetryAfterTime() { + return retryAfterTime; + } + + public void setRetryAfterTime(LocalDateTime retryAfterTime) { + this.retryAfterTime = retryAfterTime; + } + + public String getTeamsAddressableId() { + return teamsAddressableId; + } + + public void setTeamsAddressableId(String teamsAddressableId) { + this.teamsAddressableId = teamsAddressableId; + } + + public Class getTeamsAddressableClass() { + return teamsAddressableClass; + } + + public void setTeamsAddressableClass(Class teamsAddressableClass) { + this.teamsAddressableClass = teamsAddressableClass; + } + + public TeamsAddressable getTeamsAddressable() { + try { + TeamsAddressable instance = teamsAddressableClass.newInstance(); + + if (instance instanceof TeamsUser || instance instanceof TeamsChannel) { + ((TeamsChannel)instance).setKey(teamsAddressableId); + } + + return instance; + } catch (InstantiationException | IllegalAccessException e) { + throw new RuntimeException("Can't create TeamsAddressable", e); + } + } + + @Override + public String toString() { + return "MessageRetry [activity=" + activity + ", teamsAddressableId=" + teamsAddressableId + ", retryCount=" + + retryCount + ", retryAfterTime=" + retryAfterTime + "]"; + } + +} \ No newline at end of file diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java index 968ff638..7e5c1279 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/state/FileStateStorage.java @@ -304,13 +304,18 @@ private String getStorage(String file) { private Optional> splitString(String s) { if (s.contains("/")) { String[] data = s.split("/"); - if (data.length > 1) { + if (data.length == 0) { + return Optional.empty(); + } else if (data.length == 2) { return Optional.of(Arrays.asList(data)); + } else { + throw new UnsupportedOperationException("Can't handle multiple paths with file state storage: "+s); } } return Optional.empty(); } - + + private String getAzureTag(String s) { return s.replaceAll("[^0-9a-zA-Z]", "_"); } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java index c29771e0..c88184dc 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java @@ -1,41 +1,84 @@ package org.finos.springbot.teams.handlers.retry; -import java.time.LocalDateTime; -import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; -import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.content.TeamsChannel; -import org.finos.springbot.teams.handlers.retry.AbstractRetryingActivityHandler.MessageRetry; +import org.finos.springbot.teams.conversations.TeamsConversations; +import org.finos.springbot.teams.handlers.ActivityHandler; import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.HttpStatus; +import org.springframework.test.context.ActiveProfiles; +import org.springframework.test.context.junit.jupiter.SpringExtension; +import com.microsoft.bot.connector.rest.ErrorResponseException; +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ResourceResponse; + +import okhttp3.MediaType; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.ResponseBody; +import retrofit2.Response; + +@ActiveProfiles("teams") +@ExtendWith(SpringExtension.class) public class InMemoryRetryingActivityHandlerTest { - private InMemoryRetryingActivityHandler inhandler = new InMemoryRetryingActivityHandler(null); - TeamsChannel dummyChat1 = new TeamsChannel("dummy_id_1", "dummy_name"); - TeamsChannel dummyChat2 = new TeamsChannel("dummy_id_2", "dummy_name"); - - private void setUp(LocalDateTime retryTime) { - MessageRetry mr = inhandler.new MessageRetry(null,dummyChat1,3, retryTime); - inhandler.add(mr); + @MockBean + TeamsConversations conv; + + int go = 0; + + @BeforeEach + public void mockSetup() { + ArgumentCaptor msg = ArgumentCaptor.forClass(Activity.class); + + Mockito.when(conv.handleActivity(msg.capture(), Mockito.any())).thenAnswer(a -> { + ResourceResponse arg1 = new ResourceResponse("done"); + + if (go % 3 != 2) { + // fails + ResponseBody out = ResponseBody.create("{}", MediaType.get("application/json")); + Response r = Response.error(out, + new okhttp3.Response.Builder() + .code(HttpStatus.TOO_MANY_REQUESTS.value()) + .message("Response.error()").addHeader("Retry-After", "1000") + .protocol(Protocol.HTTP_1_1) + .request(new Request.Builder().url("http://localhost/").build()).build()); + go++; + return CompletableFuture.completedFuture(new ErrorResponseException("Failed", r)); + } else { + go++; + return CompletableFuture.completedFuture(arg1); + } + }); } - + + TeamsChannel dummyChat1 = new TeamsChannel("dummy_id_1", "dummy_name"); + @Test - public void testFoundMessageForRetry() { - setUp(LocalDateTime.now().minusSeconds(100)); - setUp(LocalDateTime.now().plusSeconds(100)); - - TeamsAddressable address = inhandler.get().get().getAddressable(); - Assertions.assertTrue(address.getKey().equals(dummyChat1.getKey())); - } - - @Test - public void testNoMessageEligibleForRetry(){ - setUp(LocalDateTime.now().plusSeconds(100)); - setUp(LocalDateTime.now().plusSeconds(150)); - - Optional actualResponse = inhandler.get(); - Assertions.assertFalse(actualResponse.isPresent()); + public void testRetryWorks() throws InterruptedException, ExecutionException { + long now = System.currentTimeMillis(); + + ActivityHandler retry = new InMemoryRetryingActivityHandler(conv); + + CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); + + ResourceResponse rr = cf.get(); + + Assertions.assertEquals("done", rr.getId()); + long doneTime = System.currentTimeMillis(); + + Assertions.assertTrue(doneTime - now > 2000); + Assertions.assertEquals(3, go); + } - -} + +} \ No newline at end of file diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java index 1b6f01e2..a2f48f17 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AbstractStateStorageTest.java @@ -73,31 +73,37 @@ public void testStoreWithTagDates() throws IOException { } @Test - public void testSlashStoreWithMultTags() throws IOException { + public void testSlashStoreWithMultipleDirectories() throws IOException { Map somedata = Collections.singletonMap("a", "b"); - Map tags1 = new HashMap(); - tags1.put("addressable", "thefile"); - tags1.put("object1", "tag"); + Map tagsForTheFile = new HashMap(); + tagsForTheFile.put("addressable", "one"); + tagsForTheFile.put("object1", "tag"); - Map tags2 = new HashMap(); - tags2.put("addressable", "thefile"); - tags2.put("object2", "tag"); + Map tagsForTheFileA = new HashMap(); + tagsForTheFileA.put("addressable", "one"); + tagsForTheFileA.put("object2", "tag"); + + Map tagsForTheFileB = new HashMap(); + tagsForTheFileB.put("addressable", "two"); + tagsForTheFileB.put("object2", "tag"); List tagList1 = Arrays.asList( - new Filter("addressable", "thefile", "="), + new Filter("addressable", "one", "="), new Filter("object1", "tag", "=") ); List tagList2 = Arrays.asList( - new Filter("addressable", "theotherfile", "="), + new Filter("addressable", "two", "="), new Filter("object2", "tag", "=") ); - tss.store("thefile/thefile", tags1, somedata); - tss.store("thefile/theotherfile", tags2, somedata); + tss.store("thefile", tagsForTheFile, somedata); + //tss.store("thefile/thefile", tagsForTheFile, somedata); // this won't work + tss.store("thefile/a", tagsForTheFileA, somedata); + tss.store("thefile/b", tagsForTheFileB, somedata); - Assertions.assertEquals(1, hoover(tss.retrieve(tagList1, false)).size()); - Assertions.assertEquals(0, hoover(tss.retrieve(tagList2, false)).size()); + Assertions.assertEquals(1, hoover(tss.retrieve(tagList1, false)).size()); + Assertions.assertEquals(1, hoover(tss.retrieve(tagList2, false)).size()); } public List> hoover(Iterable> iterable) { diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java index e46866e6..14c50680 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/AzureBlobStateStorageTest.java @@ -58,8 +58,8 @@ public void setup() { BlobClient theFileBlobClient; BlobClient theOtherFileBlobClient; BlobClient nonFileBlobClient; - BlobClient theFileWithSlashBlobClient; - BlobClient theOtherFileWithSlashBlobClient; + BlobClient theFileABlobClient; + BlobClient theFileBBlobClient; private BlobServiceClient createBlobServiceClientMock() { bsc = Mockito.mock(BlobServiceClient.class); @@ -67,8 +67,8 @@ private BlobServiceClient createBlobServiceClientMock() { theFileBlobClient = Mockito.mock(BlobClient.class); theOtherFileBlobClient = Mockito.mock(BlobClient.class); nonFileBlobClient = Mockito.mock(BlobClient.class); - theFileWithSlashBlobClient = Mockito.mock(BlobClient.class); - theOtherFileWithSlashBlobClient = Mockito.mock(BlobClient.class); + theFileABlobClient = Mockito.mock(BlobClient.class); + theFileBBlobClient = Mockito.mock(BlobClient.class); Mockito.when(bsc.getBlobContainerClient("test")) .thenReturn(bcc); @@ -78,8 +78,8 @@ private BlobServiceClient createBlobServiceClientMock() { Mockito.when(bcc.getBlobClient("thefile")).thenReturn(theFileBlobClient); Mockito.when(bcc.getBlobClient("theotherfile")).thenReturn(theOtherFileBlobClient); Mockito.when(bcc.getBlobClient("nonfile")).thenReturn(nonFileBlobClient); - Mockito.when(bcc.getBlobClient("thefile/thefile")).thenReturn(theFileWithSlashBlobClient); - Mockito.when(bcc.getBlobClient("thefile/theotherfile")).thenReturn(theOtherFileWithSlashBlobClient); + Mockito.when(bcc.getBlobClient("thefile/a")).thenReturn(theFileABlobClient); + Mockito.when(bcc.getBlobClient("thefile/b")).thenReturn(theFileBBlobClient); return bsc; } @@ -123,12 +123,12 @@ private void allowReadingFromTheOtherFile() throws IOException { Mockito.when(theOtherFileBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); } - private void allowReadingFromTheSlashFile() throws IOException { - Mockito.when(theFileWithSlashBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); + private void allowReadingFromTheFileA() throws IOException { + Mockito.when(theFileABlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); } - private void allowReadingFromTheOtherSlashFile() throws IOException { - Mockito.when(theOtherFileWithSlashBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); + private void allowReadingFromTheFileB() throws IOException { + Mockito.when(theFileBBlobClient.openInputStream()).thenAnswer((a) -> setupBlobInputStream()); } @@ -158,11 +158,11 @@ private void allowWritingToTheOtherFile() { } private void allowWritingToTheSlashFile() { - mockBlobOutputStream(theFileWithSlashBlobClient); + mockBlobOutputStream(theFileABlobClient); } private void allowWritingToTheOtherSlashFile() { - mockBlobOutputStream(theOtherFileWithSlashBlobClient); + mockBlobOutputStream(theFileBBlobClient); } @@ -202,17 +202,18 @@ public void testStoreWithTagDates() throws IOException { @Override @Test - public void testSlashStoreWithMultTags() throws IOException { + public void testSlashStoreWithMultipleDirectories() throws IOException { allowWritingToTheSlashFile(); allowWritingToTheOtherSlashFile(); - allowReadingFromTheSlashFile(); - allowReadingFromTheOtherSlashFile(); + allowReadingFromTheFileA(); + allowReadingFromTheFileB(); Map> queries = new HashMap<>(); - queries.put("@container='test' AND addressable='thefile' AND object1='tag'", Arrays.asList("thefile/thefile")); + queries.put("@container='test' AND addressable='one' AND object1='tag'", Arrays.asList("thefile/a")); + queries.put("@container='test' AND addressable='two' AND object2='tag'", Arrays.asList("thefile/b")); setupBlobSearch(queries); - super.testSlashStoreWithMultTags(); + super.testSlashStoreWithMultipleDirectories(); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/FileStateStorageTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/FileStateStorageTest.java index 6e6f6bda..a802cf37 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/FileStateStorageTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/state/FileStateStorageTest.java @@ -5,13 +5,18 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; import org.apache.commons.io.FileUtils; import org.finos.springbot.teams.MockTeamsConfiguration; import org.finos.springbot.workflow.data.DataHandlerConfig; import org.finos.springbot.workflow.data.EntityJsonConverter; import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; import org.junit.jupiter.api.TestInstance; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.SpringBootTest; @@ -37,6 +42,17 @@ public void setup() throws IOException { this.tss = new FileStateStorage(ejc, tmpdir); } + @Test + public void testCantStoreMultipleNestedDirectories() throws IOException { + Map somedata = Collections.singletonMap("a", "b"); + + Map tagsForTheFileB = new HashMap(); + tagsForTheFileB.put("addressable", "two"); + tagsForTheFileB.put("object2", "tag"); + + Assertions.assertThrows(UnsupportedOperationException.class, () -> tss.store("thefile/c/b", tagsForTheFileB, somedata)); + } + @AfterEach public void cleanUp() throws IOException { Path path = Paths.get(tmpdir); From 3bf0ea73488018fe7e0c524cc233abd42ace71d2 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Thu, 27 Apr 2023 15:20:01 +0530 Subject: [PATCH 16/19] Teams retry InMemory handler --- .../springbot/teams/TeamsScheduledConfig.java | 39 --------- .../AbstractRetryingActivityHandler.java | 80 +++---------------- .../InMemoryRetryingActivityHandler.java | 58 ++++++++------ .../InMemoryRetryingActivityHandlerTest.java | 63 +++++++++++++-- 4 files changed, 106 insertions(+), 134 deletions(-) delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java deleted file mode 100644 index fde4de8b..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsScheduledConfig.java +++ /dev/null @@ -1,39 +0,0 @@ -package org.finos.springbot.teams; - -import org.finos.springbot.teams.handlers.ActivityHandler; -import org.finos.springbot.teams.handlers.SimpleActivityHandler; -import org.finos.springbot.teams.handlers.retry.AbstractRetryingActivityHandler; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; -import org.springframework.scheduling.annotation.EnableScheduling; -import org.springframework.scheduling.annotation.SchedulingConfigurer; -import org.springframework.scheduling.config.ScheduledTaskRegistrar; - -@EnableScheduling -public class TeamsScheduledConfig implements SchedulingConfigurer { - - private static final Logger LOG = LoggerFactory.getLogger(TeamsScheduledConfig.class); - - @Autowired - private ActivityHandler retryHandler; - - @Value("${teams.retry.time:30000}") - private long teamsRetrySchedulerCron; - - @Override - public void configureTasks(ScheduledTaskRegistrar scheduledTaskRegistrar) { - if (retryHandler instanceof SimpleActivityHandler) { - LOG.info("No-operation retry handler is configured."); - } else { - Runnable runnable = () -> scheduleRetryMessage(); - scheduledTaskRegistrar.addFixedDelayTask(runnable, teamsRetrySchedulerCron); - } - } - - private void scheduleRetryMessage() { - - ((AbstractRetryingActivityHandler) retryHandler).retryMessage(); - } -} diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java index f0bdd570..77710205 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/AbstractRetryingActivityHandler.java @@ -1,13 +1,10 @@ package org.finos.springbot.teams.handlers.retry; -import java.time.LocalDateTime; -import java.util.Optional; + import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; import org.apache.commons.lang3.StringUtils; -import org.finos.springbot.teams.TeamsException; -import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.conversations.TeamsConversations; import org.finos.springbot.teams.handlers.ActivityHandler; import org.slf4j.Logger; @@ -16,8 +13,6 @@ import org.springframework.http.HttpStatus; import com.microsoft.bot.connector.rest.ErrorResponseException; -import com.microsoft.bot.schema.Activity; -import com.microsoft.bot.schema.ResourceResponse; import okhttp3.ResponseBody; @@ -25,18 +20,16 @@ public abstract class AbstractRetryingActivityHandler implements ActivityHandler protected static final Logger LOG = LoggerFactory.getLogger(AbstractRetryingActivityHandler.class); - private TeamsConversations tc; - @Value("${teams.retry.count:3}") - private long teamsRetryCount; - - private static final int INIT_RETRY_COUNT = 0; + protected long teamsRetryCount = 3; + protected TeamsConversations tc; + public AbstractRetryingActivityHandler(TeamsConversations tc) { this.tc = tc; } - public boolean isTooManyRequest(Throwable e) { + protected boolean isTooManyRequest(Throwable e) { if (e instanceof CompletionException && ((CompletionException) e).getCause() instanceof ErrorResponseException) { ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); @@ -47,71 +40,24 @@ public boolean isTooManyRequest(Throwable e) { } } - public long getRetryAfter(CompletionException e) { + protected int getRetryAfter(CompletionException e) { ErrorResponseException ere = (ErrorResponseException) ((CompletionException) e).getCause(); retrofit2.Response response = ere.response(); String retryAfter = response.headers().get("Retry-After"); - long retryAfterInt = 1;// initiate to 1 sec + int retryAfterInt = 1;// initiate to 1 sec if (StringUtils.isNumeric(retryAfter)) { - retryAfterInt = Long.parseLong(retryAfter); + retryAfterInt = Integer.parseInt(retryAfter); } return retryAfterInt; } - @Override - public CompletableFuture handleActivity(Activity activity, TeamsAddressable to) { - return handleActivity(activity, to, INIT_RETRY_COUNT); + protected static CompletableFuture failed(Throwable error) { + CompletableFuture future = new CompletableFuture<>(); + future.completeExceptionally(error); + return future; } - public CompletableFuture handleActivity(Activity activity, TeamsAddressable to, - Integer retryCount) { - return tc.handleActivity(activity, to).handle((rr, ex) -> { - if (ex != null) { - Boolean success = handleToManyRequestException(activity, to, retryCount, ex); - if (!success) { - throw new TeamsException("Couldn't handle response ", ex); - } else { - return null; - } - } else { - return rr; - } - - }); - } - - public Boolean handleToManyRequestException(Activity activity, TeamsAddressable to, int retryCount, Throwable e) { - if (isTooManyRequest(e)) { - long retryAfter = getRetryAfter((CompletionException) e); - retryCount++; - if (retryCount <= teamsRetryCount) { - LOG.info("AbstractRetryingActivityHandler request retryAfter {}", retryAfter); - LocalDateTime retryAfterTime = LocalDateTime.now().plusSeconds(retryAfter); - add(new MessageRetry(activity, to, retryCount, retryAfterTime)); - return true; - } - } - return false; - } - - public void retryMessage() { - int messageCount = 0; - Optional opt; - while ((opt = get()).isPresent()) { - messageCount++; - MessageRetry msg = opt.get(); - this.handleActivity(msg.getActivity(), msg.getTeamsAddressable(), msg.getRetryCount()); - } - - LOG.info("Retry message queue {}", messageCount == 0 ? "is empty" : "has messages, count: " + messageCount); - } - - - protected abstract void add(MessageRetry retry); - - protected abstract Optional get(); - -} +} \ No newline at end of file diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java index b10ee99d..6467390a 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandler.java @@ -1,37 +1,49 @@ package org.finos.springbot.teams.handlers.retry; -import java.time.LocalDateTime; -import java.util.Optional; -import java.util.Queue; -import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; +import java.util.concurrent.Executor; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.function.Function; +import org.finos.springbot.teams.content.TeamsAddressable; import org.finos.springbot.teams.conversations.TeamsConversations; -public class InMemoryRetryingActivityHandler extends AbstractRetryingActivityHandler { +import com.microsoft.bot.schema.Activity; +import com.microsoft.bot.schema.ResourceResponse; + +public class InMemoryRetryingActivityHandler extends AbstractRetryingActivityHandler { + + static final ScheduledExecutorService SCHEDULER = new ScheduledThreadPoolExecutor(10); - private Queue queue = new ConcurrentLinkedQueue<>(); - public InMemoryRetryingActivityHandler(TeamsConversations tc) { super(tc); } - - protected void add(MessageRetry retry) { - queue.add(retry); - } - - public Optional get() { - MessageRetry q; - if ((q = queue.peek()) != null) { - if (LocalDateTime.now().isAfter(q.getRetryAfterTime())) { // retry now - queue.remove(q); - return Optional.of(q); - } else { - LOG.info("Message not retried, as retry time {} for message has not passed the current time {}", - q.getRetryAfterTime(), LocalDateTime.now()); - } + + @Override + public CompletableFuture handleActivity(Activity activity, TeamsAddressable to) { + CompletableFuture f = tc.handleActivity(activity, to); + for (int i = 0; i < teamsRetryCount; i++) { + f = f.thenApply(CompletableFuture::completedFuture).exceptionally(t -> { + if (isTooManyRequest(t)) { + int ra = getRetryAfter((CompletionException) t); + Executor afterRetryTime = createDelayedExecutor(ra, TimeUnit.SECONDS); + return CompletableFuture.supplyAsync(() -> null, afterRetryTime) + .thenCompose(m -> tc.handleActivity(activity, to)); + } else { + return failed(t); + } + + }).thenCompose(Function.identity()); } + return f; + } + - return Optional.empty(); + private Executor createDelayedExecutor(long delay, TimeUnit unit) { + return r -> SCHEDULER.schedule(r, delay, unit); } } diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java index c88184dc..b87fb621 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java @@ -1,11 +1,13 @@ package org.finos.springbot.teams.handlers.retry; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import org.finos.springbot.teams.content.TeamsChannel; import org.finos.springbot.teams.conversations.TeamsConversations; -import org.finos.springbot.teams.handlers.ActivityHandler; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -35,31 +37,44 @@ public class InMemoryRetryingActivityHandlerTest { TeamsConversations conv; int go = 0; + int passEvery = 3; + + Set allUsedThreads = new HashSet<>(); @BeforeEach public void mockSetup() { + go = 0; + allUsedThreads.clear(); ArgumentCaptor msg = ArgumentCaptor.forClass(Activity.class); Mockito.when(conv.handleActivity(msg.capture(), Mockito.any())).thenAnswer(a -> { + allUsedThreads.add(Thread.currentThread()); + System.out.println("Trying "+go+" with thread "+Thread.currentThread()); ResourceResponse arg1 = new ResourceResponse("done"); - if (go % 3 != 2) { + if (go % passEvery != (passEvery -1)) { // fails ResponseBody out = ResponseBody.create("{}", MediaType.get("application/json")); Response r = Response.error(out, new okhttp3.Response.Builder() .code(HttpStatus.TOO_MANY_REQUESTS.value()) - .message("Response.error()").addHeader("Retry-After", "1000") + .message("Response.error()").addHeader("Retry-After", "2") .protocol(Protocol.HTTP_1_1) .request(new Request.Builder().url("http://localhost/").build()).build()); go++; - return CompletableFuture.completedFuture(new ErrorResponseException("Failed", r)); + return failed(new ErrorResponseException("Failed", r)); } else { go++; return CompletableFuture.completedFuture(arg1); } }); } + + public static CompletableFuture failed(Throwable error) { + CompletableFuture future = new CompletableFuture<>(); + future.completeExceptionally(error); + return future; + } TeamsChannel dummyChat1 = new TeamsChannel("dummy_id_1", "dummy_name"); @@ -67,7 +82,7 @@ public void mockSetup() { public void testRetryWorks() throws InterruptedException, ExecutionException { long now = System.currentTimeMillis(); - ActivityHandler retry = new InMemoryRetryingActivityHandler(conv); + InMemoryRetryingActivityHandler retry = new InMemoryRetryingActivityHandler(conv); CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); @@ -80,5 +95,43 @@ public void testRetryWorks() throws InterruptedException, ExecutionException { Assertions.assertEquals(3, go); } + + @Test + public void testRetryGivesUp() throws InterruptedException, ExecutionException { + Assertions.assertThrows(Exception.class, () -> { + passEvery = 1000; // never succeeds + long now = System.currentTimeMillis(); + + InMemoryRetryingActivityHandler retry = new InMemoryRetryingActivityHandler(conv); + + CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); + + ResourceResponse rr = cf.get(); + }); + } + + @Test + public void testWorksFirstTime() throws InterruptedException, ExecutionException { + passEvery = 1; // always succeeds + long now = System.currentTimeMillis(); + + RetryingActivityHandler retry = new RetryingActivityHandler(conv); + + CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); + + ResourceResponse rr = cf.get(); + Assertions.assertEquals("done", rr.getId()); + } + + @Test + public void testMultipleTimes() throws Exception { + for (int i = 0; i <10; i++) { + RetryingActivityHandler retry = new RetryingActivityHandler(conv); + CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); + ResourceResponse rr = cf.get(); + } + + System.out.println(allUsedThreads.size()); + } } \ No newline at end of file From 784b2c78806a6aff03b20c5316e3d1fe63a523ea Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Thu, 27 Apr 2023 15:23:18 +0530 Subject: [PATCH 17/19] Teams retry InMemory handler --- .../springbot/teams/TeamsWorkflowConfig.java | 3 +- .../teams/handlers/retry/MessageRetry.java | 93 ------------------- 2 files changed, 1 insertion(+), 95 deletions(-) delete mode 100644 libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java index 9c7f51bd..f344ec23 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/TeamsWorkflowConfig.java @@ -75,8 +75,7 @@ ThymeleafEngineConfig.class, AdaptiveCardConverterConfig.class, ThymeleafConverterConfig.class, - TeamsConversationsConfig.class, - TeamsScheduledConfig.class + TeamsConversationsConfig.class }) @Profile("teams") public class TeamsWorkflowConfig { diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java deleted file mode 100644 index 5bcda074..00000000 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/retry/MessageRetry.java +++ /dev/null @@ -1,93 +0,0 @@ -package org.finos.springbot.teams.handlers.retry; - -import java.time.LocalDateTime; - -import org.finos.springbot.teams.content.TeamsAddressable; -import org.finos.springbot.teams.content.TeamsChannel; -import org.finos.springbot.teams.content.TeamsUser; - -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; -import com.microsoft.bot.schema.Activity; - -@JsonIgnoreProperties(ignoreUnknown = true) -public class MessageRetry { - - private Activity activity; - private String teamsAddressableId; - private int retryCount; - private LocalDateTime retryAfterTime; - private Class teamsAddressableClass; - - public MessageRetry() { - - } - - public MessageRetry(Activity activity, TeamsAddressable teamsAddressable, int retryCount, LocalDateTime retryAfterTime) { - this.activity = activity; - this.teamsAddressableId = teamsAddressable.getKey(); - this.retryCount = retryCount; - this.retryAfterTime = retryAfterTime; - this.teamsAddressableClass = teamsAddressable.getClass(); - } - - public Activity getActivity() { - return activity; - } - - public void setActivity(Activity activity) { - this.activity = activity; - } - - public int getRetryCount() { - return retryCount; - } - - public void setRetryCount(int retryCount) { - this.retryCount = retryCount; - } - - public LocalDateTime getRetryAfterTime() { - return retryAfterTime; - } - - public void setRetryAfterTime(LocalDateTime retryAfterTime) { - this.retryAfterTime = retryAfterTime; - } - - public String getTeamsAddressableId() { - return teamsAddressableId; - } - - public void setTeamsAddressableId(String teamsAddressableId) { - this.teamsAddressableId = teamsAddressableId; - } - - public Class getTeamsAddressableClass() { - return teamsAddressableClass; - } - - public void setTeamsAddressableClass(Class teamsAddressableClass) { - this.teamsAddressableClass = teamsAddressableClass; - } - - public TeamsAddressable getTeamsAddressable() { - try { - TeamsAddressable instance = teamsAddressableClass.newInstance(); - - if (instance instanceof TeamsUser || instance instanceof TeamsChannel) { - ((TeamsChannel)instance).setKey(teamsAddressableId); - } - - return instance; - } catch (InstantiationException | IllegalAccessException e) { - throw new RuntimeException("Can't create TeamsAddressable", e); - } - } - - @Override - public String toString() { - return "MessageRetry [activity=" + activity + ", teamsAddressableId=" + teamsAddressableId + ", retryCount=" - + retryCount + ", retryAfterTime=" + retryAfterTime + "]"; - } - -} \ No newline at end of file From 7e210353f3dd22b320053c74e25936e2ec4d66ef Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Tue, 2 May 2023 12:36:43 +0530 Subject: [PATCH 18/19] Corrected InMemoryRetryingActivityHandler reference --- .../retry/InMemoryRetryingActivityHandlerTest.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java index b87fb621..16a750dd 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/test/java/org/finos/springbot/teams/handlers/retry/InMemoryRetryingActivityHandlerTest.java @@ -1,7 +1,6 @@ package org.finos.springbot.teams.handlers.retry; import java.util.HashSet; -import java.util.Iterator; import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; @@ -115,7 +114,7 @@ public void testWorksFirstTime() throws InterruptedException, ExecutionException passEvery = 1; // always succeeds long now = System.currentTimeMillis(); - RetryingActivityHandler retry = new RetryingActivityHandler(conv); + InMemoryRetryingActivityHandler retry = new InMemoryRetryingActivityHandler(conv); CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); @@ -125,8 +124,8 @@ public void testWorksFirstTime() throws InterruptedException, ExecutionException @Test public void testMultipleTimes() throws Exception { - for (int i = 0; i <10; i++) { - RetryingActivityHandler retry = new RetryingActivityHandler(conv); + for (int i = 0; i <5; i++) { + InMemoryRetryingActivityHandler retry = new InMemoryRetryingActivityHandler(conv); CompletableFuture cf = retry.handleActivity(new Activity("dummy"), dummyChat1); ResourceResponse rr = cf.get(); } From 9007a164514fd899ccebb60e2339326541e404c7 Mon Sep 17 00:00:00 2001 From: Vaibhav-a Mankar Date: Wed, 3 May 2023 18:41:07 +0530 Subject: [PATCH 19/19] Error stream id in log --- .../finos/springbot/teams/handlers/TeamsResponseHandler.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java index 58fd6b67..ebc15b4d 100644 --- a/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java +++ b/libs/teams/teams-chat-workflow-spring-boot-starter/src/main/java/org/finos/springbot/teams/handlers/TeamsResponseHandler.java @@ -182,7 +182,7 @@ private BiFunction handle private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t) { return (rr, e) -> { if (e != null) { - LOG.error(e.getMessage()); + LOG.error("Error message for stream id {} , message: {} ", address.getKey() , e.getMessage()); if (out instanceof ObjectNode){ try { LOG.error("json:\n"+new ObjectMapper().writeValueAsString(out));