Support Spring MVC asynchronous requests, fix #2716. #2781
Conversation
sczyh30
added
kind/enhancement
| @@ -15,28 +15,29 @@ | |||
| */ | |||
| package com.alibaba.csp.sentinel.adapter.spring.webmvc; | |||
|
|
|||
| import javax.servlet.http.HttpServletRequest; | |||
There was a problem hiding this comment.
please remove the unnecessary changes.
| /** | ||
| * Since request may be reprocessed in flow if any forwarding or including or other action | ||
| * happened (see {@link javax.servlet.ServletRequest#getDispatcherType()}) we will only |
There was a problem hiding this comment.
please remove the unnecessary changes.
| @@ -48,11 +49,11 @@ | |||
| * return mav; | |||
| * } | |||
| * </pre> | |||
| * | |||
There was a problem hiding this comment.
please remove the unnecessary changes.
| @@ -64,54 +65,76 @@ public AbstractSentinelInterceptor(BaseWebMvcConfig config) { | |||
| AssertUtil.assertNotBlank(config.getRequestAttributeName(), "requestAttributeName should not be blank"); | |||
| this.baseWebMvcConfig = config; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
please remove the unnecessary changes.
| /** | ||
| * @param request | ||
| * @param rcKey | ||
| * @param step | ||
| * @return reference count after increasing (initial value as zero to be increased) |
There was a problem hiding this comment.
please remove the unnecessary changes.
| } | ||
|
|
There was a problem hiding this comment.
please remove the unnecessary changes.
| @@ -139,15 +162,15 @@ public void afterCompletion(HttpServletRequest request, HttpServletResponse resp | |||
| if (increaseReferece(request, this.baseWebMvcConfig.getRequestRefName(), -1) != 0) { | |||
| return; | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
please remove the unnecessary changes.
| Entry entry = getEntryInRequest(request, baseWebMvcConfig.getRequestAttributeName()); | ||
| if (entry == null) { | ||
| // should not happen | ||
| RecordLog.warn("[{}] No entry found in request, key: {}", | ||
| getClass().getSimpleName(), baseWebMvcConfig.getRequestAttributeName()); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
please remove the unnecessary changes.
| } catch (BlockException e) { | ||
| try { | ||
| handleBlockException(request, response, e); | ||
| } finally { | ||
| ContextUtil.exit(); | ||
| } | ||
| return false; |
There was a problem hiding this comment.
the return false logic missed.
|
|
||
| HandlerMethod handlerMethod = (HandlerMethod) handler; | ||
| Class<?> returnType = handlerMethod.getReturnType().getParameterType(); | ||
| return WebAsyncTask.class.isAssignableFrom(returnType) || |
There was a problem hiding this comment.
Can you give some case about the return type is void?
|
我刚开始以为这个问题是异步请求 对于异步请求， |
|
Ok，可以把评论里面的那些缩进问题修复一下吗？ |
brotherlu-xcq
changed the title
| @@ -136,18 +136,14 @@ protected String getContextName(HttpServletRequest request) { | |||
| @Override | |||
| public void afterCompletion(HttpServletRequest request, HttpServletResponse response, | |||
| Object handler, Exception ex) throws Exception { | |||
| if (increaseReferece(request, this.baseWebMvcConfig.getRequestRefName(), -1) != 0) { | |||
There was a problem hiding this comment.
maybe don't remove this, but add the code to preHandle is better.
increaseReferece(request, this.baseWebMvcConfig.getRequestRefName(), -1)There was a problem hiding this comment.
这个PR太乱了，我重新提交了一个 #2795
# Conflicts: # sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java
Describe what this PR does / why we need it
支持Spring MVC的异步请求
Does this pull request fix one issue?
(#2716)
Describe how you did it
1 更改了
AbstractSentinelInterceptor的继承接口为AsyncHandlerInterceptor将限流的逻辑移动到了afterConcurrentHandlingStarted 中实现。异步请求的拦截逻辑 request -> afterConcurrentHandlingStarted -> preHandle -> afterCompletion -> resopnse
其中preHandle 做了方法返回值判断，如果是异步方法就不会执行了。
同步请求的拦截逻辑 request -> preHandle -> afterConcurrentHandlingStarted -> afterCompletion -> resopnse
支持的异步返回值，与Spring的 handleReturnValue 中的一致，WebAsyncTask、Callable、DeferredResult、ListenableFuture、CompletionStage
Describe how to verify it
test case
Special notes for reviews
@sczyh30