Skip to content

Commit

Permalink
SYMPHONYP-939 implements review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
vaibhav-db committed Mar 15, 2023
1 parent 2d448ea commit 80509b3
Show file tree
Hide file tree
Showing 9 changed files with 82 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -20,15 +20,15 @@ public class TeamsScheduledConfig implements SchedulingConfigurer {
private TeamsResponseHandler handler;

@Autowired
private RetryHandler retryHandler;
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 configure");
LOG.info("No-operation retry handler is configured.");
} else {
Runnable runnable = () -> scheduleRetryMessage();
scheduledTaskRegistrar.addFixedDelayTask(runnable, teamsRetrySchedulerCron);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -131,7 +130,7 @@ public TeamsResponseHandler teamsResponseHandler(
ThymeleafTemplateProvider displayTemplater,
TeamsStateStorage th,
TeamsConversations tc,
RetryHandler mr) {
MessageRetryHandler mr) {
return new TeamsResponseHandler(
null, // attachment handler
markupTemplater,
Expand Down Expand Up @@ -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();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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<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;
}

protected abstract void add(MessageRetry messageRetry);


}
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -23,12 +23,11 @@ public void add(MessageRetry t) {
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());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Original file line number Diff line number Diff line change
@@ -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<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);
}

add(new MessageRetry(t, retryCount, retryAfterInt));

return true;
}
}

return false;
}
public boolean handleException(Response t, int retryCount, Throwable e);

public Optional<MessageRetry> get();

protected abstract void add(MessageRetry messageRetry);


}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import org.finos.springbot.workflow.response.Response;

public class NoOpRetryHandler implements RetryHandler {
public class NoOpRetryHandler implements MessageRetryHandler {

@Override
public Optional<MessageRetry> get() {
Expand Down

This file was deleted.

0 comments on commit 80509b3

Please sign in to comment.