restlet / restlet-framework-java

The first REST API framework for Java
https://restlet.talend.com
Other
650 stars 284 forks source link

Current application cleared before response is sent #1317

Open rmbrad opened 6 years ago

rmbrad commented 6 years ago

We recently upgraded from v2.3.5 and noticed the call-backs on our custom ConnectorService were no longer being called. The issue seems to have been introduced by 6afed5c70ae36316af4cf471b077d219bc022a00, which sets the current application to null when there is no previous current application.

thomasjtaylor commented 3 years ago

+1 This is still a problem in v2.3.12 - custom ConnectorService is not called due to Application replacing the helped application with the current application (or null) before the HTTP call has been handled.

As @rmbrad mentioned in #1318:

Setting the current application to null in ApplicationHelper#handle actually clears the thread local before the HTTP call has been completely handled (i.e. before the response has been sent to the client.)

thboileau commented 5 months ago

hi @thomasjtaylor and @rmbrad just out of curiosity, could you tell us why you need to extend the connector service behavior? what is your use case?

rmbrad commented 5 months ago

No problem, we had groups of resources that needed differing common logic around the request. Since we use dependency injection we had modules that would define the resources and bind their required services, but needed to extend the connector service to add the logic around the request. So we just extended the connector service so we could inject request listeners allowing the modules to bind a listener with whatever logic was needed, keeping them more self contained and removing the need to modify the connector service in each of our servers according to the installed modules.

thboileau commented 5 months ago

thanks @rmbrad for your answer. I wonder also if the logic requires to update the response's entity, or if the logic has to happen at a certain moment?

rmbrad commented 5 months ago

@thboileau We weren't updating the response entity, just had to ensure the logic would always occur at the correct time

jlouvel commented 5 months ago

After reviewing all related issues and discussing with @thboileau as well, we have two core issues at hand:

As @thboileau suggested to me, one way to solve both issues would be to store the Application instance that handled a given call in the Request and Response instances themselves as they will retain the information in a reliable way after the ApplicationHelper#handle() method has returned. We could use the attributes map for that purpose or a new instance variable Message#application.

When we are retrieving the ConnectorService in ServerCall or ClientCall classes, we should then attempt to retrieve the Application instance from the Message (Request or Response) that needs it, rather than from the current thread. We could for that purpose add a Message parameter to the ConnectorHelper.getConnectorService() static method that would be used to look up the application from the Message by preference and only fallback on Application.getCurrent() if necessary.