Skip to content

Commit a08dc69

Browse files
ylnxwlpylnxwlp
authored
fix: fix the bugs about http-method-specify in spring-webmvc-v6x and incorrect time unit in dashboard (#3569)
* Fix the bugs mentioned in issue #3562 and #3563. Fix the issue in the spring-webmvc-v6x-adapter where, after configuring http-method-specify:true, the prefixes for different request methods were not concatenated. Fix incorrect time unit in the tooltip when adding a new circuit breaker rule with statistical window duration. * Fix the bugs mentioned in issue #3562 and #3563. Fix the issue in the spring-webmvc-v6x-adapter where, after configuring http-method-specify:true, the prefixes for different request methods were not concatenated. Fix incorrect time unit in the tooltip when adding a new circuit breaker rule with statistical window duration. * Fixed the issue where the original test class was affected by the concatenation of resource names after adding the logic for the HTTP prefix concatenation. * remove mseHttpMethodSpecify field and update the unit test as suggested. * retrigger CI
1 parent e60f0d0 commit a08dc69

File tree

5 files changed

+138
-14
lines changed

5 files changed

+138
-14
lines changed

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/SentinelWebInterceptor.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x.config.SentinelWebMvcConfig;
1919

2020
import com.alibaba.csp.sentinel.adapter.web.common.UrlCleaner;
21+
import com.alibaba.csp.sentinel.util.StringUtil;
2122
import jakarta.servlet.http.HttpServletRequest;
2223

2324
import org.springframework.web.servlet.HandlerMapping;
@@ -62,6 +63,9 @@ protected String getResourceName(HttpServletRequest request) {
6263
if (config.isContextPathSpecify() && request.getContextPath() != null) {
6364
resourceName = request.getContextPath() + resourceName;
6465
}
66+
if (StringUtil.isNotEmpty(resourceName) && config.isHttpMethodSpecify()) {
67+
resourceName = request.getMethod().toUpperCase() + ":" + resourceName;
68+
}
6569
return resourceName;
6670
}
6771

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/main/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/config/SentinelWebMvcConfig.java‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ public class SentinelWebMvcConfig extends BaseWebMvcConfig {
3434
*/
3535
private boolean httpMethodSpecify;
3636

37-
/**
38-
* Specify whether the URL resource name should contain the HTTP method prefix in MSE (e.g. {@code GET:}).
39-
*/
40-
private static boolean mseHttpMethodSpecify;
41-
4237

4338
/**
4439
* Specify whether unify web context(i.e. use the default context name), and is true by default.
@@ -56,10 +51,6 @@ public SentinelWebMvcConfig() {
5651
super();
5752
setRequestAttributeName(DEFAULT_REQUEST_ATTRIBUTE_NAME);
5853
try {
59-
String enable = System.getProperty("spring.cloud.mse.sentinel.web.http-method-prefix","true");
60-
if(enable != null){
61-
mseHttpMethodSpecify = Boolean.parseBoolean(enable);
62-
}
6354
String enableContextPath = System.getProperty("spring.cloud.ahas.sentinel.web.context-path", "true");
6455
if (enableContextPath != null) {
6556
contextPathSpecify = Boolean.parseBoolean(enableContextPath);
@@ -86,10 +77,6 @@ public SentinelWebMvcConfig setHttpMethodSpecify(boolean httpMethodSpecify) {
8677
return this;
8778
}
8879

89-
public static boolean isMseHttpMethodSpecify() {
90-
return mseHttpMethodSpecify;
91-
}
92-
9380
public boolean isWebContextUnify() {
9481
return webContextUnify;
9582
}

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/SentinelSpringMvcIntegrationTest.java‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.Collections;
3434

3535
import org.junit.After;
36+
import org.junit.BeforeClass;
3637
import org.junit.Test;
3738
import org.junit.runner.RunWith;
3839
import org.springframework.beans.factory.annotation.Autowired;
@@ -54,6 +55,11 @@ public class SentinelSpringMvcIntegrationTest {
5455
@Autowired
5556
private MockMvc mvc;
5657

58+
@BeforeClass
59+
public static void disableMseHttpMethodPrefix() {
60+
System.setProperty("spring.cloud.mse.sentinel.web.http-method-prefix", "false");
61+
}
62+
5763
@Test
5864
public void testBase() throws Exception {
5965
String url = "/hello";

‎sentinel-adapter/sentinel-spring-webmvc-v6x-adapter/src/test/java/com/alibaba/csp/sentinel/adapter/spring/webmvc_v6x/SentinelWebInterceptorHttpMethodPrefixTest.java‎

Lines changed: 127 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,127 @@
1+
package com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x;
2+
3+
import com.alibaba.csp.sentinel.adapter.spring.webmvc_v6x.config.SentinelWebMvcConfig;
4+
import com.alibaba.csp.sentinel.adapter.web.common.UrlCleaner;
5+
import jakarta.servlet.http.HttpServletRequest;
6+
import org.junit.Before;
7+
import org.junit.Test;
8+
import org.mockito.Mockito;
9+
import org.springframework.web.servlet.HandlerMapping;
10+
11+
import static org.junit.Assert.assertEquals;
12+
import static org.junit.Assert.assertNull;
13+
import static org.mockito.Mockito.when;
14+
15+
/**
16+
* The test for the resource request prefix concatenation in spring-webmvc-v6x.
17+
*
18+
* @author ylnxwlp
19+
*/
20+
public class SentinelWebInterceptorHttpMethodPrefixTest {
21+
22+
private SentinelWebInterceptor interceptor;
23+
private HttpServletRequest mockRequest;
24+
25+
@Before
26+
public void setUp() {
27+
mockRequest = Mockito.mock(HttpServletRequest.class);
28+
}
29+
30+
@Test
31+
public void testGetResourceNameWithHttpMethodSpecifyEnabled() {
32+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
33+
config.setHttpMethodSpecify(true);
34+
interceptor = new SentinelWebInterceptor(config);
35+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
36+
.thenReturn("/test/path");
37+
when(mockRequest.getMethod()).thenReturn("POST");
38+
when(mockRequest.getContextPath()).thenReturn("");
39+
String resourceName = interceptor.getResourceName(mockRequest);
40+
assertEquals("POST:/test/path", resourceName);
41+
}
42+
43+
@Test
44+
public void testGetResourceNameWithHttpMethodSpecifyDisabled() {
45+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
46+
config.setHttpMethodSpecify(false);
47+
interceptor = new SentinelWebInterceptor(config);
48+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
49+
.thenReturn("/no/prefix");
50+
when(mockRequest.getMethod()).thenReturn("DELETE");
51+
when(mockRequest.getContextPath()).thenReturn("");
52+
String resourceName = interceptor.getResourceName(mockRequest);
53+
assertEquals("/no/prefix", resourceName);
54+
}
55+
56+
@Test
57+
public void testGetResourceNameEmptyResourceNameShouldReturnEmptyString() {
58+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
59+
config.setHttpMethodSpecify(true);
60+
interceptor = new SentinelWebInterceptor(config);
61+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
62+
.thenReturn("");
63+
when(mockRequest.getMethod()).thenReturn("PUT");
64+
when(mockRequest.getContextPath()).thenReturn("");
65+
String resourceName = interceptor.getResourceName(mockRequest);
66+
assertEquals("", resourceName);
67+
}
68+
69+
@Test
70+
public void testGetResourceNameNullResourceNameShouldReturnNull() {
71+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
72+
config.setHttpMethodSpecify(true);
73+
interceptor = new SentinelWebInterceptor(config);
74+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
75+
.thenReturn(null);
76+
when(mockRequest.getMethod()).thenReturn("PATCH");
77+
String resourceName = interceptor.getResourceName(mockRequest);
78+
assertNull(resourceName);
79+
}
80+
81+
@Test
82+
public void testGetResourceNameWithUrlCleaner() {
83+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
84+
config.setHttpMethodSpecify(true);
85+
config.setUrlCleaner(new UrlCleaner() {
86+
@Override
87+
public String clean(String originUrl) {
88+
return "/cleaned";
89+
}
90+
});
91+
interceptor = new SentinelWebInterceptor(config);
92+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
93+
.thenReturn("/dirty/path");
94+
when(mockRequest.getMethod()).thenReturn("GET");
95+
when(mockRequest.getContextPath()).thenReturn("");
96+
String resourceName = interceptor.getResourceName(mockRequest);
97+
assertEquals("GET:/cleaned", resourceName);
98+
}
99+
100+
@Test
101+
public void testGetResourceNameWithContextPath() {
102+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
103+
config.setHttpMethodSpecify(true);
104+
config.setContextPathSpecify(true);
105+
interceptor = new SentinelWebInterceptor(config);
106+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
107+
.thenReturn("/api/user");
108+
when(mockRequest.getMethod()).thenReturn("GET");
109+
when(mockRequest.getContextPath()).thenReturn("/myapp");
110+
String resourceName = interceptor.getResourceName(mockRequest);
111+
assertEquals("GET:/myapp/api/user", resourceName);
112+
}
113+
114+
@Test
115+
public void testGetResourceNameWithContextPathDisabled() {
116+
SentinelWebMvcConfig config = new SentinelWebMvcConfig();
117+
config.setHttpMethodSpecify(true);
118+
config.setContextPathSpecify(false);
119+
interceptor = new SentinelWebInterceptor(config);
120+
when(mockRequest.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE))
121+
.thenReturn("/api/user");
122+
when(mockRequest.getMethod()).thenReturn("GET");
123+
when(mockRequest.getContextPath()).thenReturn("/myapp");
124+
String resourceName = interceptor.getResourceName(mockRequest);
125+
assertEquals("GET:/api/user", resourceName);
126+
}
127+
}

0 commit comments

Comments
 (0)