Skip to content

Commit 3268e9a

Browse files
markt-asfmarkt-asf
committed
Refactor to reduce code duplication in tests
1 parent 73aa733 commit 3268e9a

5 files changed

Lines changed: 131 additions & 135 deletions

File tree

‎test/org/apache/catalina/filters/TestRateLimitFilter.java‎

Lines changed: 4 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,8 @@
1414
* See the License for the specific language governing permissions and
1515
* limitations under the License.
1616
*/
17-
1817
package org.apache.catalina.filters;
1918

20-
import java.io.IOException;
21-
import java.time.Instant;
22-
2319
import jakarta.servlet.FilterChain;
2420
import jakarta.servlet.FilterConfig;
2521
import jakarta.servlet.ServletException;
@@ -30,11 +26,9 @@
3026

3127
import org.apache.catalina.Context;
3228
import org.apache.catalina.filters.TestRemoteIpFilter.MockFilterChain;
33-
import org.apache.catalina.filters.TestRemoteIpFilter.MockHttpServletRequest;
3429
import org.apache.catalina.startup.Tomcat;
3530
import org.apache.catalina.startup.TomcatBaseTest;
3631
import org.apache.catalina.util.FastRateLimiter;
37-
import org.apache.tomcat.unittest.TesterResponse;
3832
import org.apache.tomcat.util.descriptor.web.FilterDef;
3933
import org.apache.tomcat.util.descriptor.web.FilterMap;
4034

@@ -153,68 +147,18 @@ private RateLimitFilter testRateLimitFilter(FilterDef filterDef, Context root) t
153147
return rateLimitFilter;
154148
}
155149

156-
static class TestClient extends Thread {
157-
RateLimitFilter filter;
158-
FilterChain filterChain;
159-
String ip;
150+
static class TestClient extends TesterRateLimitClientBase {
160151

161-
int requests;
162152
int sleep;
163153

164-
int[] results;
165-
volatile String[] rlpHeader;
166-
volatile String[] rlHeader;
167-
168154
TestClient(RateLimitFilter filter, FilterChain filterChain, String ip, int requests, int rps) {
169-
this.filter = filter;
170-
this.filterChain = filterChain;
171-
this.ip = ip;
172-
this.requests = requests;
173155
this.sleep = 1000 / rps;
174-
this.results = new int[requests];
175-
this.rlpHeader = new String[requests];
176-
this.rlHeader = new String[requests];
177-
super.setDaemon(true);
178-
super.start();
179-
}
180-
181-
@Override
182-
public void run() {
183-
try {
184-
for (int i = 0; i < requests; i++) {
185-
MockHttpServletRequest request = new MockHttpServletRequest();
186-
request.setRemoteAddr(ip);
187-
TesterResponse response = new TesterResponseWithStatus();
188-
response.setRequest(request);
189-
filter.doFilter(request, response, filterChain);
190-
results[i] = response.getStatus();
191-
rlpHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT_POLICY);
192-
rlHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT);
193-
System.out.printf("%s %s: %s %d, Policy:%s, Current:%s\n", ip, Instant.now(),
194-
Integer.valueOf(i + 1), Integer.valueOf(response.getStatus()), rlpHeader[i], rlHeader[i]);
195-
sleep(sleep);
196-
}
197-
} catch (Exception ex) {
198-
ex.printStackTrace();
199-
}
156+
super(filter, filterChain, ip, requests);
200157
}
201-
}
202-
203-
static class TesterResponseWithStatus extends TesterResponse {
204-
205-
int status = 200;
206-
String message = "OK";
207158

208159
@Override
209-
public void sendError(int status, String message) throws IOException {
210-
this.status = status;
211-
this.message = message;
212-
}
213-
214-
@Override
215-
public int getStatus() {
216-
return status;
160+
void waitForNextRequest(long start, int requestIndex) throws Exception {
161+
sleep(sleep);
217162
}
218163
}
219-
220164
}

‎test/org/apache/catalina/filters/TestRateLimitFilterWithExactRateLimiter.java‎

Lines changed: 12 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
*/
1717
package org.apache.catalina.filters;
1818

19-
import java.io.IOException;
20-
2119
import jakarta.servlet.FilterChain;
2220
import jakarta.servlet.http.HttpServletResponse;
2321

@@ -26,11 +24,9 @@
2624

2725
import org.apache.catalina.Context;
2826
import org.apache.catalina.filters.TestRemoteIpFilter.MockFilterChain;
29-
import org.apache.catalina.filters.TestRemoteIpFilter.MockHttpServletRequest;
3027
import org.apache.catalina.startup.Tomcat;
3128
import org.apache.catalina.startup.TomcatBaseTest;
3229
import org.apache.catalina.util.ExactRateLimiter;
33-
import org.apache.tomcat.unittest.TesterResponse;
3430
import org.apache.tomcat.util.descriptor.web.FilterDef;
3531
import org.apache.tomcat.util.descriptor.web.FilterMap;
3632

@@ -49,10 +45,10 @@ private void testRateLimitWith1Clients(boolean exposeHeaders, boolean enforce) t
4945

5046
Tomcat tomcat = getTomcatInstance();
5147
Context root = tomcat.addContext("", TEMP_DIR);
48+
tomcat.start();
5249

5350
MockFilterChain filterChain = new MockFilterChain();
5451
RateLimitFilter rateLimitFilter = testRateLimitFilter(filterDef, root);
55-
tomcat.start();
5652

5753
ExactRateLimiter exactRateLimiter = (ExactRateLimiter) rateLimitFilter.rateLimiter;
5854

@@ -149,80 +145,26 @@ private RateLimitFilter testRateLimitFilter(FilterDef filterDef, Context root) {
149145
return rateLimitFilter;
150146
}
151147

152-
static class TestClient extends Thread {
153-
RateLimitFilter filter;
154-
FilterChain filterChain;
155-
String ip;
148+
static class TestClient extends TesterRateLimitClientBase {
156149

157-
int requests;
158150
int timePerRequest;
159151

160-
int[] results;
161-
volatile String[] rlpHeader;
162-
volatile String[] rlHeader;
163-
164152
TestClient(RateLimitFilter filter, FilterChain filterChain, String ip, int requests, int rps) {
165-
this.filter = filter;
166-
this.filterChain = filterChain;
167-
this.ip = ip;
168-
this.requests = requests;
169153
this.timePerRequest = 1000 / rps;
170-
this.results = new int[requests];
171-
this.rlpHeader = new String[requests];
172-
this.rlHeader = new String[requests];
173-
super.setDaemon(true);
174-
super.start();
154+
super(filter, filterChain, ip, requests);
175155
}
176156

177157
@Override
178-
public void run() {
179-
long start = System.nanoTime();
180-
181-
try {
182-
for (int i = 0; i < requests; i++) {
183-
MockHttpServletRequest request = new MockHttpServletRequest();
184-
request.setRemoteAddr(ip);
185-
TesterResponse response = new TesterResponseWithStatus();
186-
response.setRequest(request);
187-
filter.doFilter(request, response, filterChain);
188-
results[i] = response.getStatus();
189-
190-
rlpHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT_POLICY);
191-
rlHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT);
192-
193-
if (results[i] != 200) {
194-
break;
195-
}
196-
/*
197-
* Ensure requests are evenly spaced through time irrespective of how long each request takes to
198-
* complete. Do comparisons in milliseconds.
199-
*/
200-
long expectedDuration = (i + 1) * timePerRequest;
201-
long duration = (System.nanoTime() - start) / 1000000;
202-
if (expectedDuration > duration) {
203-
sleep(expectedDuration - duration);
204-
}
205-
}
206-
} catch (Exception ex) {
207-
ex.printStackTrace();
158+
void waitForNextRequest(long start, int requestIndex) throws Exception {
159+
/*
160+
* Ensure requests are evenly spaced through time irrespective of how long each request takes to
161+
* complete. Do comparisons in milliseconds.
162+
*/
163+
long expectedDuration = (requestIndex + 1) * timePerRequest;
164+
long duration = (System.nanoTime() - start) / 1000000;
165+
if (expectedDuration > duration) {
166+
sleep(expectedDuration - duration);
208167
}
209168
}
210169
}
211-
212-
static class TesterResponseWithStatus extends TesterResponse {
213-
214-
int status = 200;
215-
String message = "OK";
216-
217-
@Override
218-
public void sendError(int status, String message) throws IOException {
219-
this.status = status;
220-
this.message = message;
221-
}
222-
223-
@Override
224-
public int getStatus() {
225-
return status;
226-
}
227-
}
228170
}

‎test/org/apache/catalina/filters/TestRemoteCIDRFilter.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.apache.catalina.startup.Tomcat;
3131
import org.apache.catalina.startup.TomcatBaseTest;
3232
import org.apache.tomcat.unittest.TesterResponse;
33+
import org.apache.tomcat.unittest.TesterResponseWithStatus;
3334
import org.apache.tomcat.util.descriptor.web.FilterDef;
3435
import org.apache.tomcat.util.descriptor.web.FilterMap;
3536

@@ -56,7 +57,7 @@ public void testAllowOnly() throws Exception {
5657
for (int j = 0; j < 256; j += 11) {
5758
ipAddr = String.format("192.168.%s.%s", Integer.valueOf(i), Integer.valueOf(j));
5859
request = new TestRemoteIpFilter.MockHttpServletRequest(ipAddr);
59-
response = new TestRateLimitFilter.TesterResponseWithStatus();
60+
response = new TesterResponseWithStatus();
6061
expected = (i == 10 || i == 20) ? HttpServletResponse.SC_OK : HttpServletResponse.SC_FORBIDDEN;
6162
filter.doFilter(request, response, filterChain);
6263
Assert.assertEquals(expected, response.getStatus());
@@ -85,7 +86,7 @@ public void testDenyOnly() throws Exception {
8586
for (int j = 0; j < 256; j += 11) {
8687
ipAddr = String.format("192.168.%s.%s", Integer.valueOf(i), Integer.valueOf(j));
8788
request = new TestRemoteIpFilter.MockHttpServletRequest(ipAddr);
88-
response = new TestRateLimitFilter.TesterResponseWithStatus();
89+
response = new TesterResponseWithStatus();
8990
expected = (i != 10 && i != 20) ? HttpServletResponse.SC_OK : HttpServletResponse.SC_FORBIDDEN;
9091
filter.doFilter(request, response, filterChain);
9192
Assert.assertEquals(expected, response.getStatus());
@@ -115,7 +116,7 @@ public void testAllowDeny() throws Exception {
115116
for (int j = 0; j < 256; j += 11) {
116117
ipAddr = String.format("10.10.%s.%s", Integer.valueOf(i), Integer.valueOf(j));
117118
request = new TestRemoteIpFilter.MockHttpServletRequest(ipAddr);
118-
response = new TestRateLimitFilter.TesterResponseWithStatus();
119+
response = new TesterResponseWithStatus();
119120
expected = (i != 10 && i != 20) ? HttpServletResponse.SC_OK : HttpServletResponse.SC_FORBIDDEN;
120121
filter.doFilter(request, response, filterChain);
121122
Assert.assertEquals(expected, response.getStatus());
@@ -145,7 +146,7 @@ public void testAllowDenySetAsNull() throws Exception {
145146
for (int j = 0; j < 256; j += 11) {
146147
ipAddr = String.format("192.168.%s.%s", Integer.valueOf(i), Integer.valueOf(j));
147148
request = new TestRemoteIpFilter.MockHttpServletRequest(ipAddr);
148-
response = new TestRateLimitFilter.TesterResponseWithStatus();
149+
response = new TesterResponseWithStatus();
149150
expected = HttpServletResponse.SC_FORBIDDEN;
150151
filter.doFilter(request, response, filterChain);
151152
Assert.assertEquals(expected, response.getStatus());
@@ -179,7 +180,7 @@ public void testAllowDenySetAsEmptyString() throws Exception {
179180
for (int j = 0; j < 256; j += 11) {
180181
ipAddr = String.format("192.168.%s.%s", Integer.valueOf(i), Integer.valueOf(j));
181182
request = new TestRemoteIpFilter.MockHttpServletRequest(ipAddr);
182-
response = new TestRateLimitFilter.TesterResponseWithStatus();
183+
response = new TesterResponseWithStatus();
183184
expected = HttpServletResponse.SC_FORBIDDEN;
184185
filter.doFilter(request, response, filterChain);
185186
Assert.assertEquals(expected, response.getStatus());

‎test/org/apache/catalina/filters/TesterRateLimitClientBase.java‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.catalina.filters;
18+
19+
import jakarta.servlet.FilterChain;
20+
21+
import org.apache.catalina.filters.TestRemoteIpFilter.MockHttpServletRequest;
22+
import org.apache.tomcat.unittest.TesterResponse;
23+
import org.apache.tomcat.unittest.TesterResponseWithStatus;
24+
25+
public abstract class TesterRateLimitClientBase extends Thread {
26+
RateLimitFilter filter;
27+
FilterChain filterChain;
28+
String ip;
29+
30+
int requests;
31+
32+
int[] results;
33+
volatile String[] rlpHeader;
34+
volatile String[] rlHeader;
35+
36+
TesterRateLimitClientBase(RateLimitFilter filter, FilterChain filterChain, String ip, int requests) {
37+
this.filter = filter;
38+
this.filterChain = filterChain;
39+
this.ip = ip;
40+
this.requests = requests;
41+
this.results = new int[requests];
42+
this.rlpHeader = new String[requests];
43+
this.rlHeader = new String[requests];
44+
super.setDaemon(true);
45+
super.start();
46+
}
47+
48+
@Override
49+
public void run() {
50+
long start = System.nanoTime();
51+
try {
52+
for (int i = 0; i < requests; i++) {
53+
MockHttpServletRequest request = new MockHttpServletRequest();
54+
request.setRemoteAddr(ip);
55+
TesterResponse response = new TesterResponseWithStatus();
56+
response.setRequest(request);
57+
filter.doFilter(request, response, filterChain);
58+
results[i] = response.getStatus();
59+
rlpHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT_POLICY);
60+
rlHeader[i] = response.getHeader(RateLimitFilter.HEADER_RATE_LIMIT);
61+
//System.out.printf("%s %s: %s %d, Policy:%s, Current:%s\n", ip, Instant.now(),
62+
// Integer.valueOf(i + 1), Integer.valueOf(response.getStatus()), rlpHeader[i], rlHeader[i]);
63+
64+
waitForNextRequest(start, i);
65+
}
66+
} catch (Exception ex) {
67+
ex.printStackTrace();
68+
}
69+
}
70+
71+
72+
abstract void waitForNextRequest(long start, int requestIndex) throws Exception;
73+
}

‎test/org/apache/tomcat/unittest/TesterResponseWithStatus.java‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tomcat.unittest;
18+
19+
import java.io.IOException;
20+
21+
public class TesterResponseWithStatus extends TesterResponse {
22+
23+
int status = 200;
24+
String message = "OK";
25+
26+
@Override
27+
public void sendError(int status, String message) throws IOException {
28+
this.status = status;
29+
this.message = message;
30+
}
31+
32+
@Override
33+
public int getStatus() {
34+
return status;
35+
}
36+
}

0 commit comments

Comments
 (0)