Skip to content

Commit 4571953

Browse files
sapessisapessi
committed
changed the filter chain cache to only store FilterHolder object and return a new FilterChainHolder each time. This addresses issue #56 
1 parent c72f07c commit 4571953

File tree

6 files changed

+148
-36
lines changed

6 files changed

+148
-36
lines changed

‎aws-serverless-java-container-core/pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,13 @@
7474
<scope>test</scope>
7575
</dependency>
7676

77+
<!-- https://mvnrepository.com/artifact/org.slf4j/slf4j-simple -->
78+
<dependency>
79+
<groupId>org.slf4j</groupId>
80+
<artifactId>slf4j-simple</artifactId>
81+
<version>1.7.25</version>
82+
<scope>test</scope>
83+
</dependency>
7784
</dependencies>
7885

7986
</project>

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainHolder.java‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
*/
1313
package com.amazonaws.serverless.proxy.internal.servlet;
1414

15+
import org.slf4j.Logger;
16+
import org.slf4j.LoggerFactory;
17+
1518
import javax.servlet.FilterChain;
1619
import javax.servlet.ServletException;
1720
import javax.servlet.ServletRequest;
@@ -33,7 +36,9 @@ public class FilterChainHolder implements FilterChain {
3336
//-------------------------------------------------------------
3437

3538
private List<FilterHolder> filters;
36-
private int currentFilter;
39+
int currentFilter;
40+
41+
private Logger log = LoggerFactory.getLogger(FilterChainHolder.class);
3742

3843

3944
//-------------------------------------------------------------
@@ -43,7 +48,7 @@ public class FilterChainHolder implements FilterChain {
4348
/**
4449
* Creates a new empty <code>FilterChainHolder</code>
4550
*/
46-
public FilterChainHolder() {
51+
FilterChainHolder() {
4752
this(new ArrayList<>());
4853
}
4954

@@ -52,9 +57,9 @@ public FilterChainHolder() {
5257
* Creates a new instance of a filter chain holder
5358
* @param allFilters A populated list of <code>FilterHolder</code> objects
5459
*/
55-
public FilterChainHolder(List<FilterHolder> allFilters) {
60+
FilterChainHolder(List<FilterHolder> allFilters) {
5661
filters = allFilters;
57-
currentFilter = -1;
62+
resetHolder();
5863
}
5964

6065

@@ -66,16 +71,20 @@ public FilterChainHolder(List<FilterHolder> allFilters) {
6671
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) throws IOException, ServletException {
6772
currentFilter++;
6873
if (filters == null || filters.size() == 0 || currentFilter > filters.size() - 1) {
74+
log.debug("Could not find filters to execute, returning");
6975
return;
7076
}
7177
// TODO: We do not check for async filters here
7278

7379
FilterHolder holder = filters.get(currentFilter);
7480

81+
// lazily initialize filters when they are needed
7582
if (!holder.isFilterInitialized()) {
7683
holder.init();
7784
}
85+
log.debug("Starting filter " + holder.getFilterName());
7886
holder.getFilter().doFilter(servletRequest, servletResponse, this);
87+
log.debug("Executed filter " + holder.getFilterName());
7988
}
8089

8190

@@ -87,7 +96,7 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
8796
* Add a filter to the chain.
8897
* @param newFilter The filter to be added at the end of the chain
8998
*/
90-
public void addFilter(FilterHolder newFilter) {
99+
void addFilter(FilterHolder newFilter) {
91100
filters.add(newFilter);
92101
}
93102

@@ -96,7 +105,7 @@ public void addFilter(FilterHolder newFilter) {
96105
* Returns the number of filters loaded in the chain holder
97106
* @return The number of filters in the chain holder. If the filter chain is null then this will return 0
98107
*/
99-
public int filterCount() {
108+
int filterCount() {
100109
if (filters == null) {
101110
return 0;
102111
} else {
@@ -110,11 +119,29 @@ public int filterCount() {
110119
* @param idx The index in the chain. Use the <code>filterCount</code> method to get the filter count
111120
* @return A populated FilterHolder object
112121
*/
113-
public FilterHolder getFilter(int idx) {
122+
FilterHolder getFilter(int idx) {
114123
if (filters == null) {
115124
return null;
116125
} else {
117126
return filters.get(idx);
118127
}
119128
}
129+
130+
131+
/**
132+
* Returns the list of filters in this chain.
133+
* @return The list of filters
134+
*/
135+
public List<FilterHolder> getFilters() {
136+
return filters;
137+
}
138+
139+
140+
/**
141+
* Resets the chain holder to the beginning of the filter chain. This method is used from the constructor as well as when
142+
* the {@link FilterChainManager} return a holder from the cache.
143+
*/
144+
private void resetHolder() {
145+
currentFilter = -1;
146+
}
120147
}

‎aws-serverless-java-container-core/src/main/java/com/amazonaws/serverless/proxy/internal/servlet/FilterChainManager.java‎

Lines changed: 22 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,12 @@
1313
package com.amazonaws.serverless.proxy.internal.servlet;
1414

1515
import javax.servlet.DispatcherType;
16+
import javax.servlet.ServletContext;
1617
import javax.servlet.http.HttpServletRequest;
18+
1719
import java.util.Collections;
1820
import java.util.HashMap;
21+
import java.util.List;
1922
import java.util.Map;
2023

2124
/**
@@ -26,13 +29,13 @@
2629
* For example, the Spring implementation creates the ServletContext when the application is initialized the first time
2730
* and creates a FitlerChainManager to execute its filters for each request.
2831
*/
29-
public abstract class FilterChainManager<ServletContextType> {
32+
public abstract class FilterChainManager<ServletContextType extends ServletContext> {
3033

3134
//-------------------------------------------------------------
3235
// Variables - Protected
3336
//-------------------------------------------------------------
3437

35-
protected static final String PATH_PART_SEPARATOR = "/";
38+
static final String PATH_PART_SEPARATOR = "/";
3639

3740

3841
//-------------------------------------------------------------
@@ -41,7 +44,7 @@ public abstract class FilterChainManager<ServletContextType> {
4144

4245
// we use the synchronizedMap because we do not expect high concurrency on this object. Lambda only allows one
4346
// event at a time per container
44-
private Map<TargetCacheKey, FilterChainHolder> filterCache = Collections.synchronizedMap(new HashMap<TargetCacheKey, FilterChainHolder>());
47+
private Map<TargetCacheKey, List<FilterHolder>> filterCache = Collections.synchronizedMap(new HashMap<TargetCacheKey, List<FilterHolder>>());
4548
private int filtersSize = -1;
4649
protected ServletContextType servletContext;
4750

@@ -50,7 +53,7 @@ public abstract class FilterChainManager<ServletContextType> {
5053
// Constructors
5154
//-------------------------------------------------------------
5255

53-
protected FilterChainManager(ServletContextType context) {
56+
FilterChainManager(ServletContextType context) {
5457
servletContext = context;
5558
}
5659

@@ -80,7 +83,7 @@ protected FilterChainManager(ServletContextType context) {
8083
* @param request The incoming servlet request
8184
* @return A <code>FilterChainHolder</code> object that can be used to apply the filters to the request
8285
*/
83-
public FilterChainHolder getFilterChain(final HttpServletRequest request) {
86+
FilterChainHolder getFilterChain(final HttpServletRequest request) {
8487
String targetPath = request.getServletPath();
8588
DispatcherType type = request.getDispatcherType();
8689

@@ -127,17 +130,22 @@ public FilterChainHolder getFilterChain(final HttpServletRequest request) {
127130

128131
/**
129132
* Retrieves a filter chain from the cache. The cache is lazily loaded as filter chains are requested. If the chain
130-
* is not available in the cache, the method returns null.
133+
* is not available in the cache, the method returns null. This method returns a new instance of FilterChainHolder
134+
* initialized with the cached list of {@link FilterHolder} objects
131135
* @param type The dispatcher type for the incoming request
132136
* @param targetPath The request path - this is extracted with the <code>getPath</code> method of the request object
133137
* @return A populated FilterChainHolder
134138
*/
135-
protected FilterChainHolder getFilterChainCache(final DispatcherType type, final String targetPath) {
139+
private FilterChainHolder getFilterChainCache(final DispatcherType type, final String targetPath) {
136140
TargetCacheKey key = new TargetCacheKey();
137141
key.setDispatcherType(type);
138142
key.setTargetPath(targetPath);
139143

140-
return filterCache.get(key);
144+
if (!filterCache.containsKey(key)) {
145+
return null;
146+
}
147+
148+
return new FilterChainHolder(filterCache.get(key));
141149
}
142150

143151

@@ -150,7 +158,7 @@ protected FilterChainHolder getFilterChainCache(final DispatcherType type, final
150158
* @param targetPath The target path in the API
151159
* @param holder The FilterChainHolder object to save in the cache
152160
*/
153-
protected void putFilterChainCache(final DispatcherType type, final String targetPath, final FilterChainHolder holder) {
161+
private void putFilterChainCache(final DispatcherType type, final String targetPath, final FilterChainHolder holder) {
154162
TargetCacheKey key = new TargetCacheKey();
155163
key.setDispatcherType(type);
156164
key.setTargetPath(targetPath);
@@ -159,8 +167,8 @@ protected void putFilterChainCache(final DispatcherType type, final String targe
159167
if (key.hashCode() == -1) {
160168
return;
161169
}
170+
filterCache.put(key, holder.getFilters());
162171

163-
filterCache.put(key, holder);
164172
}
165173

166174

@@ -171,7 +179,7 @@ protected void putFilterChainCache(final DispatcherType type, final String targe
171179
* @param mapping The mapping path stored in the filter registration
172180
* @return true if the given mapping path can apply to the target, false otherwise.
173181
*/
174-
protected boolean pathMatches(final String target, final String mapping) {
182+
boolean pathMatches(final String target, final String mapping) {
175183
// easiest case, they are exactly the same
176184
if (target.toLowerCase().equals(mapping.toLowerCase())) {
177185
return true;
@@ -275,34 +283,20 @@ public int hashCode() {
275283

276284
@Override
277285
public boolean equals(Object key) {
278-
if (!key.getClass().isAssignableFrom(TargetCacheKey.class)) {
279-
return false;
280-
} else {
281-
return hashCode() == key.hashCode();
282-
}
286+
return key.getClass().isAssignableFrom(TargetCacheKey.class) && hashCode() == key.hashCode();
283287
}
284288

285289

286290
//-------------------------------------------------------------
287291
// Methods - Getter/Setter
288292
//-------------------------------------------------------------
289293

290-
public String getTargetPath() {
291-
return targetPath;
292-
}
293-
294-
295-
public void setTargetPath(String targetPath) {
294+
void setTargetPath(String targetPath) {
296295
this.targetPath = targetPath;
297296
}
298297

299298

300-
public DispatcherType getDispatcherType() {
301-
return dispatcherType;
302-
}
303-
304-
305-
public void setDispatcherType(DispatcherType dispatcherType) {
299+
void setDispatcherType(DispatcherType dispatcherType) {
306300
this.dispatcherType = dispatcherType;
307301
}
308302
}

‎aws-serverless-java-container-core/src/test/java/com/amazonaws/serverless/proxy/internal/servlet/AwsFilterChainManagerTest.java‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,27 @@
55
import com.amazonaws.services.lambda.runtime.Context;
66
import org.junit.BeforeClass;
77
import org.junit.Test;
8+
import org.slf4j.Logger;
9+
import org.slf4j.LoggerFactory;
810

911
import javax.servlet.*;
1012

1113
import java.io.IOException;
1214
import java.util.EnumSet;
15+
import java.util.concurrent.CountDownLatch;
1316

1417
import static org.junit.Assert.*;
1518

1619
public class AwsFilterChainManagerTest {
20+
private static final String REQUEST_CUSTOM_ATTRIBUTE_NAME = "X-Custom-Attribute";
21+
private static final String REQUEST_CUSTOM_ATTRIBUTE_VALUE = "CustomAttrValue";
1722

1823
private static AwsFilterChainManager chainManager;
1924
private static Context lambdaContext = new MockLambdaContext();
2025
private static ServletContext servletContext;
2126

27+
private Logger log = LoggerFactory.getLogger(AwsFilterChainManagerTest.class);
28+
2229
@BeforeClass
2330
public static void setUp() {
2431
servletContext = new AwsServletContext( null);//AwsServletContext.getInstance(lambdaContext, null);
@@ -127,6 +134,76 @@ public void filterChain_getFilterChain_subsetOfFilters() {
127134
assertEquals("Filter2", fcHolder.getFilter(0).getFilterName());
128135
}
129136

137+
@Test
138+
public void filterChain_matchMultipleTimes_expectSameMatch() {
139+
AwsProxyHttpServletRequest req = new AwsProxyHttpServletRequest(
140+
new AwsProxyRequestBuilder("/first/second", "GET").build(), lambdaContext, null
141+
);
142+
req.setServletContext(servletContext);
143+
FilterChainHolder fcHolder = chainManager.getFilterChain(req);
144+
assertEquals(1, fcHolder.filterCount());
145+
assertEquals("Filter1", fcHolder.getFilter(0).getFilterName());
146+
147+
AwsProxyHttpServletRequest req2 = new AwsProxyHttpServletRequest(
148+
new AwsProxyRequestBuilder("/first/second", "GET").build(), lambdaContext, null
149+
);
150+
req.setServletContext(servletContext);
151+
FilterChainHolder fcHolder2 = chainManager.getFilterChain(req2);
152+
assertEquals(1, fcHolder2.filterCount());
153+
assertEquals("Filter1", fcHolder2.getFilter(0).getFilterName());
154+
}
155+
156+
@Test
157+
public void filerChain_executeMultipleFilters_expectRunEachTime() {
158+
AwsProxyHttpServletRequest req = new AwsProxyHttpServletRequest(
159+
new AwsProxyRequestBuilder("/first/second", "GET").build(), lambdaContext, null
160+
);
161+
req.setServletContext(servletContext);
162+
FilterChainHolder fcHolder = chainManager.getFilterChain(req);
163+
assertEquals(1, fcHolder.filterCount());
164+
assertEquals("Filter1", fcHolder.getFilter(0).getFilterName());
165+
AwsHttpServletResponse resp = new AwsHttpServletResponse(req, new CountDownLatch(1));
166+
167+
try {
168+
fcHolder.doFilter(req, resp);
169+
} catch (IOException e) {
170+
fail("IO Exception while executing filters");
171+
e.printStackTrace();
172+
} catch (ServletException e) {
173+
fail("Servlet exception while executing filters");
174+
e.printStackTrace();
175+
}
176+
177+
assertTrue(req.getAttribute(REQUEST_CUSTOM_ATTRIBUTE_NAME) != null);
178+
assertEquals(REQUEST_CUSTOM_ATTRIBUTE_VALUE, req.getAttribute(REQUEST_CUSTOM_ATTRIBUTE_NAME));
179+
180+
log.debug("Starting second request");
181+
182+
AwsProxyHttpServletRequest req2 = new AwsProxyHttpServletRequest(
183+
new AwsProxyRequestBuilder("/first/second", "GET").build(), lambdaContext, null
184+
);
185+
req2.setServletContext(servletContext);
186+
FilterChainHolder fcHolder2 = chainManager.getFilterChain(req2);
187+
assertEquals(1, fcHolder2.filterCount());
188+
assertEquals("Filter1", fcHolder2.getFilter(0).getFilterName());
189+
assertEquals(-1, fcHolder2.currentFilter);
190+
191+
AwsHttpServletResponse resp2 = new AwsHttpServletResponse(req, new CountDownLatch(1));
192+
193+
try {
194+
fcHolder2.doFilter(req2, resp2);
195+
} catch (IOException e) {
196+
fail("IO Exception while executing filters");
197+
e.printStackTrace();
198+
} catch (ServletException e) {
199+
fail("Servlet exception while executing filters");
200+
e.printStackTrace();
201+
}
202+
203+
assertTrue(req2.getAttribute(REQUEST_CUSTOM_ATTRIBUTE_NAME) != null);
204+
assertEquals(REQUEST_CUSTOM_ATTRIBUTE_VALUE, req2.getAttribute(REQUEST_CUSTOM_ATTRIBUTE_NAME));
205+
}
206+
130207
@Test
131208
public void filterChain_getFilterChain_multipleFilters() {
132209
AwsProxyHttpServletRequest req = new AwsProxyHttpServletRequest(
@@ -159,6 +236,7 @@ public void init(FilterConfig filterConfig) throws ServletException {
159236
@Override
160237
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain) throws IOException, ServletException {
161238
System.out.println("DoFilter");
239+
servletRequest.setAttribute(REQUEST_CUSTOM_ATTRIBUTE_NAME, REQUEST_CUSTOM_ATTRIBUTE_VALUE);
162240
filterChain.doFilter(servletRequest, servletResponse);
163241
}
164242

‎aws-serverless-java-container-spark/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131
<groupId>com.sparkjava</groupId>
3232
<artifactId>spark-core</artifactId>
3333
<version>${spark.version}</version>
34+
<exclusions>
35+
<exclusion>
36+
<groupId>org.slf4j</groupId>
37+
<artifactId>slf4j-api</artifactId>
38+
</exclusion>
39+
</exclusions>
3440
</dependency>
3541

3642
<!-- https://mvnrepository.com/artifact/junit/junit -->

0 commit comments

Comments
 (0)