Reduce stack size and simplify control flow in FilterChainInvocation.
Revision created by MOE tool push_codebase.
MOE_MIGRATION=4878
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
index ae6d088..ed5f2df 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
@@ -23,6 +23,7 @@
import java.util.List;
import java.util.Set;
+import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
@@ -45,7 +46,6 @@
class FilterChainInvocation implements FilterChain {
private static final Set<String> SERVLET_INTERNAL_METHODS = ImmutableSet.of(
- FilterDefinition.class.getName() + ".doFilter",
FilterChainInvocation.class.getName() + ".doFilter");
private final FilterDefinition[] filterDefinitions;
@@ -67,8 +67,6 @@
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse)
throws IOException, ServletException {
- index++;
-
GuiceFilter.Context previous = GuiceFilter.localContext.get();
HttpServletRequest request = (HttpServletRequest) servletRequest;
HttpServletResponse response = (HttpServletResponse) servletResponse;
@@ -76,9 +74,12 @@
= (previous != null) ? previous.getOriginalRequest() : request;
GuiceFilter.localContext.set(new GuiceFilter.Context(originalRequest, request, response));
try {
- //dispatch down the chain while there are more filters
- if (index < filterDefinitions.length) {
- filterDefinitions[index].doFilter(servletRequest, servletResponse, this);
+ Filter filter = findNextFilter(servletRequest, servletResponse);
+ if (filter != null) {
+ // call to the filter, which can either consume the request or
+ // recurse back into this method. (in which case we will go to find the next filter,
+ // or dispatch to the servlet if no more filters are left)
+ filter.doFilter(servletRequest, servletResponse, this);
} else {
//we've reached the end of the filterchain, let's try to dispatch to a servlet
final boolean serviced = servletPipeline.service(servletRequest, servletResponse);
@@ -103,6 +104,20 @@
GuiceFilter.localContext.set(previous);
}
}
+
+ /**
+ * Iterates over the remaining filter definitions.
+ * Returns the first applicable filter, or null if none apply.
+ */
+ private Filter findNextFilter(ServletRequest servletRequest, ServletResponse servletResponse) {
+ while (++index < filterDefinitions.length) {
+ Filter filter = filterDefinitions[index].getFilterIfMatching(servletRequest, servletResponse);
+ if (filter != null) {
+ return filter;
+ }
+ }
+ return null;
+ }
/**
* Removes stacktrace elements related to AOP internal mechanics from the
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
index ee42b14..501a351 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterDefinition.java
@@ -23,7 +23,6 @@
import com.google.inject.spi.ProviderInstanceBinding;
import com.google.inject.spi.ProviderWithExtensionVisitor;
-import java.io.IOException;
import java.util.Collections;
import java.util.Enumeration;
import java.util.HashMap;
@@ -153,20 +152,16 @@
}
}
- public void doFilter(ServletRequest servletRequest,
- ServletResponse servletResponse, FilterChainInvocation filterChainInvocation)
- throws IOException, ServletException {
+ public Filter getFilterIfMatching(ServletRequest servletRequest,
+ ServletResponse servletResponse) {
final HttpServletRequest request = (HttpServletRequest) servletRequest;
final String path = request.getRequestURI().substring(request.getContextPath().length());
if (shouldFilter(path)) {
- filter.get()
- .doFilter(servletRequest, servletResponse, filterChainInvocation);
-
+ return filter.get();
} else {
- //otherwise proceed down chain anyway
- filterChainInvocation.doFilter(servletRequest, servletResponse);
+ return null;
}
}
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
index 9d94c75..115a4b0 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
@@ -128,8 +128,11 @@
assertTrue("Init did not fire", mockFilter.isInit());
+ Filter matchingFilter = filterDef.getFilterIfMatching(request, null);
+ assertSame(mockFilter, matchingFilter);
+
final boolean proceed[] = new boolean[1];
- filterDef.doFilter(request, null, new FilterChainInvocation(null, null, null) {
+ matchingFilter.doFilter(request, null, new FilterChainInvocation(null, null, null) {
@Override
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) {
proceed[0] = true;
@@ -188,8 +191,11 @@
assertTrue("init did not fire", mockFilter.isInit());
+ Filter matchingFilter = filterDef.getFilterIfMatching(request, null);
+ assertSame(mockFilter, matchingFilter);
+
final boolean proceed[] = new boolean[1];
- filterDef.doFilter(request, null, new FilterChainInvocation(null, null, null) {
+ matchingFilter.doFilter(request, null, new FilterChainInvocation(null, null, null) {
@Override
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) {
proceed[0] = true;
