Skip to content

Commit

Permalink
Pull request #30: SYMPHONYP-940 refactor retry handler
Browse files Browse the repository at this point in the history
Merge in SYMPHONYP/symphony-java-toolkit from feature/SYMPHONYP-939-ms-teams-spring-bot-retry-messages to spring-bot-master-db

* commit '3e4b60cbfa4318e45122d113b408aec181c8bf98':
  review comments changes done.
  added testcases for Message Retry.
  SYMPHONYP-939 implements review comments
  SYMPHONYP-940 refactor retry handler
  fixing formatting
  Adding roadmap
  Update LICENSE
  setting copyright name
  • Loading branch information
vaibhav-db committed Mar 27, 2023
2 parents 25a538f + 3e4b60c commit 1781441
Show file tree
Hide file tree
Showing 13 changed files with 261 additions and 77 deletions.
2 changes: 1 addition & 1 deletion LICENSE
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
same "printed page" as the copyright notice for easier
identification within third-party archives.

Copyright {yyyy} {name of copyright owner}
Copyright 2020 FINOS, Deutsche Bank

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down
6 changes: 6 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ There are two main branches:
- `spring-bot-master`: new code for the multi-platform Symphony / Microsoft Teams bot builder
- `symphony-java-toolkit-master`: legacy code for when this project only supported symphony.

## Roadmap

The spring bot roadmap is a number of tagged issues that we are working on _next_.

You can view the roadmap issues [here](https://github.com/finos/spring-bot/issues?q=is%3Aissue+is%3Aopen+label%3Aroadmap).

## Releasing This Project

In order to do a release:
Expand Down
Original file line number Diff line number Diff line change
@@ -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.MessageRetryHandler;
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 MessageRetryHandler 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 configured.");
} else {
Runnable runnable = () -> scheduleRetryMessage();
scheduledTaskRegistrar.addFixedDelayTask(runnable, teamsRetrySchedulerCron);
}
}

private void scheduleRetryMessage() {
handler.retryMessage();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.InMemoryMessageRetryHandler;
import org.finos.springbot.teams.handlers.MessageRetryHandler;
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.history.StateStorageBasedTeamsHistory;
import org.finos.springbot.teams.history.StorageIDResponseHandler;
import org.finos.springbot.teams.history.TeamsHistory;
Expand Down Expand Up @@ -230,10 +230,14 @@ public void setResourceLoaderClassLoader() {
resourceLoader.setClassLoader(this.getClass().getClassLoader());
}

/**
* set InMemoryMessageRetryHandler() if you want retry
* @return
*/
@Bean
@ConditionalOnMissingBean
public MessageRetryHandler messageRetryHandler() {
return new InMemoryMessageRetryHandler();
return new NoOpRetryHandler();
}

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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.MessageRetryHandler;
import org.finos.springbot.teams.history.StorageIDResponseHandler;
import org.finos.springbot.teams.history.TeamsHistory;
import org.finos.springbot.teams.response.templating.EntityMarkupTemplateProvider;
Expand All @@ -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;
Expand All @@ -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;
Expand All @@ -63,7 +61,7 @@ public class TeamsResponseHandler implements ResponseHandler, ApplicationContext
protected ThymeleafTemplateProvider displayTemplater;
protected TeamsStateStorage teamsState;
protected TeamsConversations teamsConversations;
protected MessageRetryHandler messageRetryHandler;
protected MessageRetryHandler retryHandler;

public TeamsResponseHandler(
AttachmentHandler attachmentHandler,
Expand All @@ -79,7 +77,7 @@ public TeamsResponseHandler(
this.displayTemplater = displayTemplater;
this.teamsState = th;
this.teamsConversations = tc;
this.messageRetryHandler = mr;
this.retryHandler = mr;
}

protected void initErrorHandler() {
Expand All @@ -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();

Expand Down Expand Up @@ -192,32 +190,10 @@ private BiFunction<? super ResourceResponse, Throwable, ResourceResponse> 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<ResponseBody> 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<? super ResourceResponse, Throwable, ResourceResponse> handleErrorAndStorage(Object out, TeamsAddressable address, Map<String, Object> 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){
Expand All @@ -239,7 +215,7 @@ private BiFunction<? super ResourceResponse, Throwable, ResourceResponse> handle
return null;
};
}

protected CompletableFuture<ResourceResponse> sendCardResponse(JsonNode json, TeamsAddressable address, Map<String, Object> data) throws Exception {
Activity out = Activity.createMessageActivity();
Attachment body = new Attachment();
Expand Down Expand Up @@ -275,7 +251,7 @@ public void retryMessage() {
int messageCount = 0;

Optional<MessageRetry> opt;
while ((opt = messageRetryHandler.get()).isPresent()) {
while ((opt = retryHandler.get()).isPresent()) {
messageCount++;
this.sendResponse(opt.get().getResponse(), opt.get().getRetryCount());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
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<ResponseBody> 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;
}


}
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -8,31 +8,36 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class InMemoryMessageRetryHandler implements MessageRetryHandler {
public class InMemoryMessageRetryHandler extends BasicMessageRetryHandler {

private static final Logger LOG = LoggerFactory.getLogger(InMemoryMessageRetryHandler.class);

Queue<MessageRetry> queue = new ConcurrentLinkedQueue<>();
private Queue<MessageRetry> queue = new ConcurrentLinkedQueue<>();

@Override
public void add(MessageRetry t) {
queue.add(t);
}

@Override
public void clearAll() {
while(queue.poll()!=null);
}

@Override
public Optional<MessageRetry> 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());
}
}

return Optional.empty();
}


}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package org.finos.springbot.teams.handlers;
package org.finos.springbot.teams.handlers.retry;

import java.time.LocalDateTime;

Expand All @@ -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() {
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
Loading

0 comments on commit 1781441

Please sign in to comment.