finos / spring-bot

Spring Boot + Java Integration for Symphony/Teams Chat Platform Bots and Apps
https://springbot.finos.org
Apache License 2.0
60 stars 35 forks source link

For Teams return conversation id on message sent #409

Closed vaibhav-db closed 1 year ago

vaibhav-db commented 1 year ago

For Teams return conversation id on message sent

Description of Problem:

When we used

TeamsResponseHandler rh;
rh.accept(new WorkResponse( new TeamsChannel("channel_id", null), object, WorkMode.VIEW));

message posted on give channel, but this method doesn't return. Many application want the posted message conversation id.

Potential Solutions:

In TeamsResponseHandler.accept method, we will add conversation id in LocalThread, similar like action then we can pull that conversation id from localThread.

@Override
    public void accept(Response t) {
    /**
        removed code....
    **/
        ResourceResponse rr = sendXMLResponse(content, attachment, ta, entities, mr.getData())
                        .handle(handleErrorAndStorage(content, ta, mr.getData(), t)).get();

                ThreadLocal<String> tc = new ThreadLocal<String>();
                tc.set(rr.getId());
}
robmoffat commented 1 year ago

could accept return a CompletableFuture? If we made it generic we could return the ResourceResponse in it

vaibhav-db commented 1 year ago

TeamsReasponseHandler.accept(Response t) is Consumer<Response> method, so we can't return CompletableFuture. We have option to add ResourceResponse in ThreadLocal like we have Action

robmoffat commented 1 year ago

What do you think about the idea of changing to be a Function instead of Consumer?

vaibhav-db commented 1 year ago

We need to modify ResponseHandler extends Consumer<Response>, PriorityOrdered It has huge impact. Many class like ButtonsResponseHandler, ChatListResponseHandler, UserListResponseHandler, ContentResponseConverter, CollectionResponseConverter, WorkResponseConverter,....

image

robmoffat commented 1 year ago

you're probably right - are you around next week? Maybe we can brainstorm a bit more then

vaibhav-db commented 1 year ago

Yes.. I am in.. If possible can we connect on Monday...

vaibhav-db commented 1 year ago

Hi @robmoffat ,

If Microsoft BDK throw error, we have three option:

  1. Existing flow init the error handle and send message to particular channel or same channel.
private BiFunction<? super ResourceResponse, Throwable, ResourceResponse> handleErrorAndStorage(Object out, TeamsAddressable address, Map<String, Object> data, Response t) {
        return (rr, e) -> {
                if (e != null) {                    
                    .....                   
                    initErrorHandler();
                    eh.handleError(e);
                    Action.CURRENT_ACTION.set(Action.NULL_ACTION);                  
                } else if(rr != null) {
                    .....
                }

                return rr;
            };
    }
  1. Create our own ErrorResourceResponse and return in case of error.

    private BiFunction<? super ResourceResponse, Throwable, ResourceResponse> handleErrorAndStorage(Object out, TeamsAddressable address, Map<String, Object> data, Response t) {
        return (rr, e) -> {
                if (e != null) {                    
                    ...
                    initErrorHandler();
                    eh.handleError(e);
                    Action.CURRENT_ACTION.set(Action.NULL_ACTION);
                    rr = new **TeamsErrorResourceResponse**(address.getKey(), e);
                } else if(rr != null) {
                    .....
                }
    
                return rr;
            };
    }
  2. We throw exception in handleErrorAndStorage method.
private BiFunction<? super ResourceResponse, Throwable, ResourceResponse> handleErrorAndStorage(Object out, TeamsAddressable address, Map<String, Object> data, Response t) {
        return (rr, e) -> {
                if (e != null) {
                    try {
                    .....
                    initErrorHandler();
                    eh.handleError(e);
                    Action.CURRENT_ACTION.set(Action.NULL_ACTION);
                    throw e;
                }catch (Throwable e1) {
                    throw new RuntimeException("Passing on exception ", e1);
                }
                .....
                return rr;
            };
    }
robmoffat commented 1 year ago

Hi @vaibhav-db,

As usual, it's pretty hard for me to get into the right headspace to figure this out without a call, but thanks for writing this up so clearly.

Can we wait until next Wednesday? Or do you want to do something sooner?

In either case, my suggested next step would be to make sure we have a unit test that we can use to ensure what whatever behaviour we agree on actually happens. We have the retry tests - is this enough? Or do we need some new ones?

speak soon

vaibhav-db commented 1 year ago

Hi @robmoffat ,

I think retry tests are enough for all scenario.

If possible can we connect tomorrow at same time? I have tested all three approach look like 2nd is suite for us.

vaibhav-db commented 1 year ago

Tomorrow we can connect 10 AM your time, or on Monday.

Raise PR for 2nd approach https://github.com/finos/spring-bot/pull/412

vaibhav-db commented 1 year ago

Added testcase.