Skip to content

Commit 04a21c7

Browse files
elandauelandau
authored
Merge pull request #32 from stanleyrh/bugfix/servlet_partition_by_path_grouping
Use pathToGroup function argument in ServletLimiterBuilder
2 parents e42ba9c + f4f820c commit 04a21c7

File tree

2 files changed

+112
-19
lines changed

2 files changed

+112
-19
lines changed

‎concurrency-limits-servlet/src/main/java/com/netflix/concurrency/limits/servlet/ServletLimiterBuilder.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public ServletLimiterBuilder partitionByHeader(String name, Consumer<LookupParti
3939
*/
4040
public ServletLimiterBuilder partitionByUserPrincipal(Function<Principal, String> principalToGroup, Consumer<LookupPartitionStrategy.Builder<HttpServletRequest>> configurer) {
4141
return partitionByLookup(
42-
request -> Optional.ofNullable(request.getUserPrincipal()).map(principalToGroup) .orElse(null),
42+
request -> Optional.ofNullable(request.getUserPrincipal()).map(principalToGroup).orElse(null),
4343
configurer);
4444
}
4545

@@ -77,7 +77,7 @@ public ServletLimiterBuilder partitionByParameter(String name, Consumer<LookupPa
7777
*/
7878
public ServletLimiterBuilder partitionByPathInfo(Function<String, String> pathToGroup, Consumer<LookupPartitionStrategy.Builder<HttpServletRequest>> configurer) {
7979
return partitionByLookup(
80-
request -> Optional.ofNullable(request.getPathInfo()).orElse(null),
80+
request -> Optional.ofNullable(request.getPathInfo()).map(pathToGroup).orElse(null),
8181
configurer);
8282
}
8383

‎concurrency-limits-servlet/src/test/com/netflix/concurrency/limits/GroupServletLimiterTest.java‎

Lines changed: 110 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,26 +13,19 @@
1313

1414
import org.junit.Assert;
1515
import org.junit.Test;
16+
import org.junit.runner.RunWith;
17+
import org.mockito.Matchers;
1618
import org.mockito.Mockito;
19+
import org.mockito.runners.MockitoJUnitRunner;
1720

21+
@RunWith(MockitoJUnitRunner.class)
1822
public class GroupServletLimiterTest {
19-
private static final Map<String, String> principalToGroup = Mockito.spy(new HashMap<>());
20-
21-
static {
22-
principalToGroup.put("bob", "live");
23-
}
24-
25-
HttpServletRequest createMockRequestWithPrincipal(String name) {
26-
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
27-
Principal principal = Mockito.mock(Principal.class);
28-
29-
Mockito.when(request.getUserPrincipal()).thenReturn(principal);
30-
Mockito.when(principal.getName()).thenReturn(name);
31-
return request;
32-
}
33-
23+
3424
@Test
3525
public void userPrincipalMatchesGroup() {
26+
Map<String, String> principalToGroup = Mockito.spy(new HashMap<>());
27+
principalToGroup.put("bob", "live");
28+
3629
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
3730
.limit(VegasLimit.newDefault())
3831
.partitionByUserPrincipal(p -> principalToGroup.get(p.getName()), builder -> builder
@@ -49,6 +42,9 @@ public void userPrincipalMatchesGroup() {
4942

5043
@Test
5144
public void userPrincipalDoesNotMatchGroup() {
45+
Map<String, String> principalToGroup = Mockito.spy(new HashMap<>());
46+
principalToGroup.put("bob", "live");
47+
5248
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
5349
.limit(VegasLimit.newDefault())
5450
.partitionByUserPrincipal(p -> principalToGroup.get(p.getName()), builder -> builder
@@ -64,7 +60,31 @@ public void userPrincipalDoesNotMatchGroup() {
6460
}
6561

6662
@Test
67-
public void nullUserPrincipal() {
63+
public void nullUserPrincipalDoesNotMatchGroup() {
64+
Map<String, String> principalToGroup = Mockito.spy(new HashMap<>());
65+
principalToGroup.put("bob", "live");
66+
67+
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
68+
.limit(VegasLimit.newDefault())
69+
.partitionByUserPrincipal(p -> principalToGroup.get(p.getName()), builder -> builder
70+
.assign("live", 0.8)
71+
.assign("batch", 0.2))
72+
.build();
73+
74+
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
75+
Mockito.when(request.getUserPrincipal()).thenReturn(null);
76+
77+
Optional<Listener> listener = limiter.acquire(request);
78+
79+
Assert.assertTrue(listener.isPresent());
80+
Mockito.verify(principalToGroup, Mockito.times(0)).get(Mockito.<String> any());
81+
}
82+
83+
@Test
84+
public void nullUserPrincipalNameDoesNotMatchGroup() {
85+
Map<String, String> principalToGroup = Mockito.spy(new HashMap<>());
86+
principalToGroup.put("bob", "live");
87+
6888
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
6989
.limit(VegasLimit.newDefault())
7090
.partitionByUserPrincipal(p -> principalToGroup.get(p.getName()), builder -> builder
@@ -76,6 +96,79 @@ public void nullUserPrincipal() {
7696
Optional<Listener> listener = limiter.acquire(request);
7797

7898
Assert.assertTrue(listener.isPresent());
79-
Mockito.verify(principalToGroup, Mockito.never());
99+
Mockito.verify(principalToGroup, Mockito.times(1)).get(Matchers.isNull(String.class));
100+
}
101+
102+
@Test
103+
public void pathMatchesGroup() {
104+
Map<String, String> pathToGroup = Mockito.spy(new HashMap<>());
105+
pathToGroup.put("/live/path", "live");
106+
107+
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
108+
.limit(VegasLimit.newDefault())
109+
.partitionByPathInfo(pathToGroup::get, builder -> builder
110+
.assign("live", 0.8)
111+
.assign("batch", 0.2))
112+
.build();
113+
114+
HttpServletRequest request = createMockRequestWithPathInfo("/live/path");
115+
Optional<Listener> listener = limiter.acquire(request);
116+
117+
Assert.assertTrue(listener.isPresent());
118+
Mockito.verify(pathToGroup, Mockito.times(1)).get("/live/path");
119+
}
120+
121+
@Test
122+
public void pathDoesNotMatchesGroup() {
123+
Map<String, String> pathToGroup = Mockito.spy(new HashMap<>());
124+
pathToGroup.put("/live/path", "live");
125+
126+
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
127+
.limit(VegasLimit.newDefault())
128+
.partitionByPathInfo(pathToGroup::get, builder -> builder
129+
.assign("live", 0.8)
130+
.assign("batch", 0.2))
131+
.build();
132+
133+
HttpServletRequest request = createMockRequestWithPathInfo("/other/path");
134+
Optional<Listener> listener = limiter.acquire(request);
135+
136+
Assert.assertTrue(listener.isPresent());
137+
Mockito.verify(pathToGroup, Mockito.times(1)).get("/other/path");
138+
}
139+
140+
@Test
141+
public void nullPathDoesNotMatchesGroup() {
142+
Map<String, String> pathToGroup = Mockito.spy(new HashMap<>());
143+
pathToGroup.put("/live/path", "live");
144+
145+
Limiter<HttpServletRequest> limiter = new ServletLimiterBuilder()
146+
.limit(VegasLimit.newDefault())
147+
.partitionByPathInfo(pathToGroup::get, builder -> builder
148+
.assign("live", 0.8)
149+
.assign("batch", 0.2))
150+
.build();
151+
152+
HttpServletRequest request = createMockRequestWithPathInfo(null);
153+
Optional<Listener> listener = limiter.acquire(request);
154+
155+
Assert.assertTrue(listener.isPresent());
156+
Mockito.verify(pathToGroup, Mockito.times(0)).get(Mockito.<String> any());
157+
}
158+
159+
private HttpServletRequest createMockRequestWithPrincipal(String name) {
160+
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
161+
Principal principal = Mockito.mock(Principal.class);
162+
163+
Mockito.when(request.getUserPrincipal()).thenReturn(principal);
164+
Mockito.when(principal.getName()).thenReturn(name);
165+
return request;
166+
}
167+
168+
private HttpServletRequest createMockRequestWithPathInfo(String name) {
169+
HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
170+
171+
Mockito.when(request.getPathInfo()).thenReturn(name);
172+
return request;
80173
}
81174
}

0 commit comments

Comments
 (0)