INT-4497: Add RateLimiterRequestHandlerAdvice #2781
Conversation
| @Override | ||
| protected Object doInvoke(ExecutionCallback callback, Object target, Message<?> message) throws Exception { | ||
| CheckedFunction0<Object> restrictedCall = | ||
| RateLimiter.decorateCheckedSupplier(this.rateLimiter, callback::execute); |
There was a problem hiding this comment.
I wonder if we could refactor the ARHA to to avoid creating this object on every call?
We could provide the subclass with an alternative method, something like...
@Override
Object doInvoke(Invocation invocation) {
myInvoke(invocation);
}And then only decorate myInvoke() once during initialization,.
There was a problem hiding this comment.
I thought about that, but then it looks like we can use the same advice instance for different endpoints, so, the target and callback are going to be fully different from context to context.
Need to sleep with this more, since an internal logic of the advice is shared as well...
There was a problem hiding this comment.
Hm. Looks like we fully depend on the invocation.proceed() which is context-based against a method and actual arguments to call. So, there is no way to decorate only once during initialization. The real call must be decorated any way.
I might miss something, so feel free to share more your thoughts!
|
|
||
| private final RateLimiter rateLimiter; | ||
|
|
||
| private ApplicationEventPublisher applicationEventPublisher; |
There was a problem hiding this comment.
Yeah... I was thought to propagate RateLimiter event to Spring events, but then I realized that their API allows only to have one event consumer, so it is going to be some kind of abuse to mutate externally provided RateLimiter.
Om the other hand their even doesn't bring too much info to be propagated with our own events.
On the other hand we are good to live without them with just call we have here.
Plus that's not too hard to propagate those events by yourself configuring RateLimiter externally as a bean.
JIRA: https://jira.spring.io/browse/INT-4497