From 75f4b7726e32ce5c3bfcb9e1a38091fff98b365f Mon Sep 17 00:00:00 2001 From: Jason Joo Date: Thu, 20 Aug 2020 00:49:04 +0800 Subject: [PATCH] Improve compatibility for dispatched servlet request and take the initial request as target resource --- .../webmvc/AbstractSentinelInterceptor.java | 90 ++++++++++++------- .../webmvc/config/BaseWebMvcConfig.java | 12 ++- .../controller/WebMvcTestController.java | 17 +++- 3 files changed, 86 insertions(+), 33 deletions(-) diff --git a/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java b/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java index c4de7847af..e793aac73c 100644 --- a/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java +++ b/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/AbstractSentinelInterceptor.java @@ -34,6 +34,21 @@ import org.springframework.web.servlet.ModelAndView; /** + * 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 + * deal with the initial request. So we use reference count to track in + * dispathing "onion" though which we could figure out whether we are in initial type "REQUEST". + * That means the sub-requests which we rarely meet in practice will NOT be recorded in Sentinel. + *

+ * How to implement a forward sub-request in your action: + * 

+ * initalRequest() {
+ *     ModelAndView mav = new ModelAndView();
+ *     mav.setViewName("another");
+ *     return mav;
+ * }
+ *
+ * * @author kaizi2009 * @since 1.7.1 */ @@ -49,20 +64,46 @@ public AbstractSentinelInterceptor(BaseWebMvcConfig config) { AssertUtil.assertNotBlank(config.getRequestAttributeName(), "requestAttributeName should not be blank"); this.baseWebMvcConfig = config; } - + + /** + * @param request + * @param rcKey + * @param step + * @return reference count after increasing (initial value as zero to be increased) + */ + private Integer increaseReferece(HttpServletRequest request, String rcKey, int step) { + Object obj = request.getAttribute(rcKey); + + if (obj == null) { + // initial + obj = Integer.valueOf(0); + } + + Integer newRc = (Integer)obj + step; + request.setAttribute(rcKey, newRc); + return newRc; + } + @Override public boolean preHandle(HttpServletRequest request, HttpServletResponse response, Object handler) throws Exception { try { String resourceName = getResourceName(request); - if (StringUtil.isNotEmpty(resourceName)) { - // Parse the request origin using registered origin parser. - String origin = parseOrigin(request); - String contextName = getContextName(request); - ContextUtil.enter(contextName, origin); - setEntryInRequest(request, baseWebMvcConfig.getRequestAttributeName(), resourceName); + if (StringUtil.isEmpty(resourceName)) { + return true; } + + if (increaseReferece(request, this.baseWebMvcConfig.getRequestRefName(), 1) != 1) { + return true; + } + + // Parse the request origin using registered origin parser. + String origin = parseOrigin(request); + String contextName = getContextName(request); + ContextUtil.enter(contextName, origin); + Entry entry = SphU.entry(resourceName, ResourceTypeConstants.COMMON_WEB, EntryType.IN); + request.setAttribute(baseWebMvcConfig.getRequestAttributeName(), entry); return true; } catch (BlockException e) { try { @@ -95,11 +136,20 @@ 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) { + return; + } + Entry entry = getEntryInRequest(request, baseWebMvcConfig.getRequestAttributeName()); - if (entry != null) { - traceExceptionAndExit(entry, ex); - removeEntryInRequest(request); + if (entry == null) { + // should not happen + RecordLog.warn("[{}] No entry found in request, key: {}", + getClass().getSimpleName(), baseWebMvcConfig.getRequestAttributeName()); + return; } + + traceExceptionAndExit(entry, ex); + removeEntryInRequest(request); ContextUtil.exit(); } @@ -108,26 +158,6 @@ public void postHandle(HttpServletRequest request, HttpServletResponse response, ModelAndView modelAndView) throws Exception { } - /** - * Note: - * If the attribute key already exists in request, don't create new {@link Entry}, - * to guarantee the order of {@link Entry} in pair and avoid {@link com.alibaba.csp.sentinel.ErrorEntryFreeException}. - * - * Refer to: - * https://github.com/alibaba/Sentinel/issues/1531 - * https://github.com/alibaba/Sentinel/issues/1482 - */ - protected void setEntryInRequest(HttpServletRequest request, String name, String resourceName) throws BlockException { - Object attrVal = request.getAttribute(name); - if (attrVal != null) { - RecordLog.warn("[{}] The attribute key '{}' already exists in request, please set `requestAttributeName`", - getClass().getSimpleName(), name); - } else { - Entry entry = SphU.entry(resourceName, ResourceTypeConstants.COMMON_WEB, EntryType.IN); - request.setAttribute(name, entry); - } - } - protected Entry getEntryInRequest(HttpServletRequest request, String attrKey) { Object entryObject = request.getAttribute(attrKey); return entryObject == null ? null : (Entry)entryObject; diff --git a/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/config/BaseWebMvcConfig.java b/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/config/BaseWebMvcConfig.java index 03ff0ebc7b..e1bd1542fd 100644 --- a/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/config/BaseWebMvcConfig.java +++ b/sentinel-adapter/sentinel-spring-webmvc-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc/config/BaseWebMvcConfig.java @@ -17,7 +17,6 @@ import com.alibaba.csp.sentinel.adapter.spring.webmvc.callback.BlockExceptionHandler; import com.alibaba.csp.sentinel.adapter.spring.webmvc.callback.RequestOriginParser; -import com.alibaba.csp.sentinel.util.AssertUtil; /** * Common base configuration for Spring Web MVC adapter. @@ -28,6 +27,7 @@ public abstract class BaseWebMvcConfig { protected String requestAttributeName; + protected String requestRefName; protected BlockExceptionHandler blockExceptionHandler; protected RequestOriginParser originParser; @@ -37,6 +37,16 @@ public String getRequestAttributeName() { public void setRequestAttributeName(String requestAttributeName) { this.requestAttributeName = requestAttributeName; + this.requestRefName = this.requestAttributeName + "-rc"; + } + + /** + * Paired with attr name used to track reference count. + * + * @return + */ + public String getRequestRefName() { + return requestRefName; } public BlockExceptionHandler getBlockExceptionHandler() { diff --git a/sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/controller/WebMvcTestController.java b/sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/controller/WebMvcTestController.java index c70342c860..ac2aa97635 100644 --- a/sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/controller/WebMvcTestController.java +++ b/sentinel-demo/sentinel-demo-spring-webmvc/src/main/java/com/alibaba/csp/sentinel/demo/spring/webmvc/controller/WebMvcTestController.java @@ -17,40 +17,53 @@ import java.util.Random; import java.util.concurrent.TimeUnit; +import org.springframework.stereotype.Controller; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; -import org.springframework.web.bind.annotation.RestController; +import org.springframework.web.bind.annotation.ResponseBody; +import org.springframework.web.servlet.ModelAndView; /** * Test controller * @author kaizi2009 */ -@RestController +@Controller public class WebMvcTestController { @GetMapping("/hello") + @ResponseBody public String apiHello() { doBusiness(); return "Hello!"; } @GetMapping("/err") + @ResponseBody public String apiError() { doBusiness(); return "Oops..."; } @GetMapping("/foo/{id}") + @ResponseBody public String apiFoo(@PathVariable("id") Long id) { doBusiness(); return "Hello " + id; } @GetMapping("/exclude/{id}") + @ResponseBody public String apiExclude(@PathVariable("id") Long id) { doBusiness(); return "Exclude " + id; } + + @GetMapping("/forward") + public ModelAndView apiForward() { + ModelAndView mav = new ModelAndView(); + mav.setViewName("hello"); + return mav; + } private void doBusiness() { Random random = new Random(1);