Ensure servlets & filters are processed in the correct order in all scenarios. Fix so that code like:
MainModule extends ServletModule {
configureServlets() {
filter("/").through(FirstFilter.class);
install(new SecondaryModule());
}
}
SecondaryModule extends ServletModule {
configureServlets() {
filter("/").through(SecondaryFilter.class);
}
}
.. ends up with a request to "/" going to FirstFilter and _then_ SecondaryFilter.
Revision created by MOE tool push_codebase.
MOE_MIGRATION=3589
diff --git a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
index 36599a6..ae6d088 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
@@ -18,7 +18,6 @@
import com.google.common.base.Throwables;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
import java.io.IOException;
import java.util.List;
diff --git a/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java b/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
index 0f8976f..4c301ad 100755
--- a/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
+++ b/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
@@ -15,8 +15,7 @@
*/
package com.google.inject.servlet;
-import com.google.common.collect.Lists;
-import com.google.inject.AbstractModule;
+import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.internal.UniqueAnnotations;
@@ -33,23 +32,12 @@
*
* @author dhanji@gmail.com (Dhanji R. Prasanna)
*/
-class FiltersModuleBuilder extends AbstractModule {
- private final List<FilterDefinition> filterDefinitions = Lists.newArrayList();
- private final List<FilterInstanceBindingEntry> filterInstanceEntries = Lists.newArrayList();
-
- //invoked on injector config
- @Override
- protected void configure() {
- // Create bindings for filter instances
- for (FilterInstanceBindingEntry entry : filterInstanceEntries) {
- bind(entry.key).toInstance(entry.filter);
- }
-
- // Bind these filter definitions to a unique random key. Doesn't matter what it is,
- // coz it's never used.
- for(FilterDefinition fd : filterDefinitions) {
- bind(FilterDefinition.class).annotatedWith(UniqueAnnotations.create()).toProvider(fd);
- }
+class FiltersModuleBuilder {
+
+ private final Binder binder;
+
+ public FiltersModuleBuilder(Binder binder) {
+ this.binder = binder;
}
public ServletModule.FilterKeyBindingBuilder filter(List<String> patterns) {
@@ -60,16 +48,6 @@
return new FilterKeyBindingBuilderImpl(regexes, UriPatternType.REGEX);
}
- private static class FilterInstanceBindingEntry {
- final Key<Filter> key;
- final Filter filter;
-
- FilterInstanceBindingEntry(Key<Filter> key, Filter filter) {
- this.key = key;
- this.filter = filter;
- }
- }
-
//non-static inner class so it can access state of enclosing module class
class FilterKeyBindingBuilderImpl implements ServletModule.FilterKeyBindingBuilder {
private final List<String> uriPatterns;
@@ -108,7 +86,7 @@
Map<String, String> initParams,
Filter filterInstance) {
for (String pattern : uriPatterns) {
- filterDefinitions.add(
+ binder.bind(FilterDefinition.class).annotatedWith(UniqueAnnotations.create()).toProvider(
new FilterDefinition(pattern, filterKey, UriPatternType.get(uriPatternType, pattern),
initParams, filterInstance));
}
@@ -117,7 +95,7 @@
public void through(Filter filter,
Map<String, String> initParams) {
Key<Filter> filterKey = Key.get(Filter.class, UniqueAnnotations.create());
- filterInstanceEntries.add(new FilterInstanceBindingEntry(filterKey, filter));
+ binder.bind(filterKey).toInstance(filter);
through(filterKey, initParams, filter);
}
}
diff --git a/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java b/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
index 62da7c7..9e5b849 100755
--- a/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
@@ -81,8 +81,8 @@
for (Binding<FilterDefinition> entry : injector.findBindingsByType(FILTER_DEFS)) {
filterDefinitions.add(entry.getProvider().get());
}
-
- // Convert to a fixed size array for speed.
+
+ // Copy to a fixed-size array for speed of iteration.
return filterDefinitions.toArray(new FilterDefinition[filterDefinitions.size()]);
}
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletModule.java b/extensions/servlet/src/com/google/inject/servlet/ServletModule.java
index 26cb2a2..c85e7ad 100644
--- a/extensions/servlet/src/com/google/inject/servlet/ServletModule.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletModule.java
@@ -45,16 +45,14 @@
protected final void configure() {
checkState(filtersModuleBuilder == null, "Re-entry is not allowed.");
checkState(servletsModuleBuilder == null, "Re-entry is not allowed.");
- filtersModuleBuilder = new FiltersModuleBuilder();
- servletsModuleBuilder = new ServletsModuleBuilder();
+ filtersModuleBuilder = new FiltersModuleBuilder(binder());
+ servletsModuleBuilder = new ServletsModuleBuilder(binder());
try {
// Install common bindings (skipped if already installed).
install(new InternalServletModule());
// Install local filter and servlet bindings.
configureServlets();
- install(filtersModuleBuilder);
- install(servletsModuleBuilder);
} finally {
filtersModuleBuilder = null;
servletsModuleBuilder = null;
diff --git a/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java b/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
index c5f973f..79f2d0b 100755
--- a/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
+++ b/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
@@ -15,9 +15,8 @@
*/
package com.google.inject.servlet;
-import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
-import com.google.inject.AbstractModule;
+import com.google.inject.Binder;
import com.google.inject.Key;
import com.google.inject.internal.UniqueAnnotations;
@@ -35,30 +34,13 @@
*
* @author Dhanji R. Prasanna (dhanji@gmail.com)
*/
-class ServletsModuleBuilder extends AbstractModule {
- private final List<ServletDefinition> servletDefinitions = Lists.newArrayList();
- private final List<ServletInstanceBindingEntry> servletInstanceEntries = Lists.newArrayList();
-
- //invoked on injector config
- @Override
- protected void configure() {
- // Create bindings for servlet instances
- for (ServletInstanceBindingEntry entry : servletInstanceEntries) {
- bind(entry.key).toInstance(entry.servlet);
- }
-
- // Ensure that servlets are not bound twice to the same pattern.
- Set<String> servletUris = Sets.newHashSet();
- for (ServletDefinition servletDefinition : servletDefinitions) {
- if (servletUris.contains(servletDefinition.getPattern())) {
- addError("More than one servlet was mapped to the same URI pattern: "
- + servletDefinition.getPattern());
- }
- else {
- bind(Key.get(ServletDefinition.class, UniqueAnnotations.create())).toProvider(servletDefinition);
- servletUris.add(servletDefinition.getPattern());
- }
- }
+class ServletsModuleBuilder {
+
+ private final Set<String> servletUris = Sets.newHashSet();
+ private final Binder binder;
+
+ public ServletsModuleBuilder(Binder binder) {
+ this.binder = binder;
}
//the first level of the EDSL--
@@ -70,16 +52,6 @@
return new ServletKeyBindingBuilderImpl(regexes, UriPatternType.REGEX);
}
- private static class ServletInstanceBindingEntry {
- final Key<HttpServlet> key;
- final HttpServlet servlet;
-
- ServletInstanceBindingEntry(Key<HttpServlet> key, HttpServlet servlet) {
- this.key = key;
- this.servlet = servlet;
- }
- }
-
//non-static inner class so it can access state of enclosing module class
class ServletKeyBindingBuilderImpl implements ServletModule.ServletKeyBindingBuilder {
private final List<String> uriPatterns;
@@ -111,21 +83,25 @@
Map<String, String> initParams) {
with(servletKey, initParams, null);
}
-
- private void with(Key<? extends HttpServlet> servletKey,
- Map<String, String> initParams,
+
+ private void with(Key<? extends HttpServlet> servletKey, Map<String, String> initParams,
HttpServlet servletInstance) {
for (String pattern : uriPatterns) {
- servletDefinitions.add(
- new ServletDefinition(pattern, servletKey, UriPatternType.get(uriPatternType, pattern),
- initParams, servletInstance));
+ // Ensure two servlets aren't bound to the same pattern.
+ if (!servletUris.add(pattern)) {
+ binder.addError("More than one servlet was mapped to the same URI pattern: " + pattern);
+ } else {
+ binder.bind(Key.get(ServletDefinition.class, UniqueAnnotations.create())).toProvider(
+ new ServletDefinition(pattern, servletKey, UriPatternType
+ .get(uriPatternType, pattern), initParams, servletInstance));
+ }
}
}
public void with(HttpServlet servlet,
Map<String, String> initParams) {
Key<HttpServlet> servletKey = Key.get(HttpServlet.class, UniqueAnnotations.create());
- servletInstanceEntries.add(new ServletInstanceBindingEntry(servletKey, servlet));
+ binder.bind(servletKey).toInstance(servlet);
with(servletKey, initParams, servlet);
}
}
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
index 1ae64e8..9d94c75 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
@@ -119,7 +119,8 @@
String pattern = "/*";
final FilterDefinition filterDef = new FilterDefinition(pattern, Key.get(Filter.class),
- UriPatternType.get(UriPatternType.SERVLET, pattern), new HashMap<String, String>(), null);
+ UriPatternType.get(UriPatternType.SERVLET, pattern),
+ new HashMap<String, String>(), null);
//should fire on mockfilter now
filterDef.init(createMock(ServletContext.class), injector,
Sets.newSetFromMap(Maps.<Filter, Boolean>newIdentityHashMap()));
@@ -129,9 +130,8 @@
final boolean proceed[] = new boolean[1];
filterDef.doFilter(request, null, new FilterChainInvocation(null, null, null) {
- public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse)
- throws IOException, ServletException {
-
+ @Override
+ public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) {
proceed[0] = true;
}
});
@@ -153,10 +153,9 @@
HttpServletRequest request = createMock(HttpServletRequest.class);
final MockFilter mockFilter = new MockFilter() {
+ @Override
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
- FilterChain filterChain) throws IOException, ServletException {
- setRun(true);
-
+ FilterChain filterChain) {
//suppress rest of chain...
}
};
@@ -179,7 +178,8 @@
String pattern = "/*";
final FilterDefinition filterDef = new FilterDefinition(pattern, Key.get(Filter.class),
- UriPatternType.get(UriPatternType.SERVLET, pattern), new HashMap<String, String>(), null);
+ UriPatternType.get(UriPatternType.SERVLET, pattern),
+ new HashMap<String, String>(), null);
//should fire on mockfilter now
filterDef.init(createMock(ServletContext.class), injector,
Sets.newSetFromMap(Maps.<Filter, Boolean>newIdentityHashMap()));
@@ -190,8 +190,8 @@
final boolean proceed[] = new boolean[1];
filterDef.doFilter(request, null, new FilterChainInvocation(null, null, null) {
- public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse)
- throws IOException, ServletException {
+ @Override
+ public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse) {
proceed[0] = true;
}
});
@@ -208,10 +208,9 @@
private static class MockFilter implements Filter {
private boolean init;
private boolean destroy;
- private boolean run;
private FilterConfig config;
- public void init(FilterConfig filterConfig) throws ServletException {
+ public void init(FilterConfig filterConfig) {
init = true;
this.config = filterConfig;
@@ -219,16 +218,10 @@
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
FilterChain filterChain) throws IOException, ServletException {
- run = true;
-
//proceed
filterChain.doFilter(servletRequest, servletResponse);
}
- protected void setRun(boolean run) {
- this.run = run;
- }
-
public void destroy() {
destroy = true;
}
@@ -241,10 +234,6 @@
return destroy;
}
- public boolean isRun() {
- return run;
- }
-
public FilterConfig getConfig() {
return config;
}
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
index 6feb82e..1d55ec1 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
@@ -1,7 +1,24 @@
+/**
+ * Copyright (C) 2011 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
package com.google.inject.servlet;
import static com.google.inject.servlet.ManagedServletPipeline.REQUEST_DISPATCHER_REQUEST;
import static com.google.inject.servlet.ServletTestUtils.newFakeHttpServletRequest;
+import static com.google.inject.servlet.ServletTestUtils.newNoOpFilterChain;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
@@ -18,12 +35,11 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
-import javax.servlet.Servlet;
-import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
@@ -199,7 +215,7 @@
@Singleton
public static class TestFilter implements Filter {
- public void init(FilterConfig filterConfig) throws ServletException {
+ public void init(FilterConfig filterConfig) {
inits++;
}
@@ -220,6 +236,7 @@
public static final String FORWARD_TO = "/forwarded.html";
public List<String> processedUris = new ArrayList<String>();
+ @Override
protected void service(HttpServletRequest httpServletRequest, HttpServletResponse httpServletResponse)
throws ServletException, IOException {
String requestUri = httpServletRequest.getRequestURI();
@@ -232,12 +249,63 @@
}
}
+ @Override
public void service(ServletRequest servletRequest, ServletResponse servletResponse)
throws ServletException, IOException {
service((HttpServletRequest) servletRequest, (HttpServletResponse) servletResponse);
}
}
+ public void testFilterOrder() throws Exception {
+ AtomicInteger counter = new AtomicInteger();
+ final CountFilter f1 = new CountFilter(counter);
+ final CountFilter f2 = new CountFilter(counter);
+
+ Injector injector = Guice.createInjector(new ServletModule() {
+ @Override
+ protected void configureServlets() {
+ filter("/").through(f1);
+ install(new ServletModule() {
+ @Override
+ protected void configureServlets() {
+ filter("/").through(f2);
+ }
+ });
+ }
+ });
+
+ HttpServletRequest request = newFakeHttpServletRequest();
+ final FilterPipeline pipeline = injector.getInstance(FilterPipeline.class);
+ pipeline.initPipeline(null);
+ pipeline.dispatch(request, null, newNoOpFilterChain());
+ assertEquals(0, f1.calledAt);
+ assertEquals(1, f2.calledAt);
+ }
+
+ /** A filter that keeps count of when it was called by increment a counter. */
+ private static class CountFilter implements Filter {
+ private final AtomicInteger counter;
+ private int calledAt = -1;
+
+ public CountFilter(AtomicInteger counter) {
+ this.counter = counter;
+ }
+
+ public void destroy() {
+ }
+
+ public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain)
+ throws ServletException, IOException {
+ if (calledAt != -1) {
+ fail("not expecting to be called twice");
+ }
+ calledAt = counter.getAndIncrement();
+ chain.doFilter(request, response);
+ }
+
+ public void init(FilterConfig filterConfig) {}
+ }
+
public final void testFilterExceptionPrunesStack() throws Exception {
Injector injector = Guice.createInjector(new ServletModule() {
@Override
@@ -295,7 +363,7 @@
@Override
protected void service(HttpServletRequest req, HttpServletResponse resp)
- throws ServletException, IOException {
+ throws ServletException {
throw new ServletException("failure!");
}
diff --git a/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java b/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
index 6ecce07..06e3990 100644
--- a/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
+++ b/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
@@ -98,7 +98,7 @@
@Singleton
public static class TestFilter implements Filter {
- public void init(FilterConfig filterConfig) throws ServletException {
+ public void init(FilterConfig filterConfig) {
}
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
@@ -112,11 +112,11 @@
@Singleton
public static class NeverFilter implements Filter {
- public void init(FilterConfig filterConfig) throws ServletException {
+ public void init(FilterConfig filterConfig) {
}
public void doFilter(ServletRequest servletRequest, ServletResponse servletResponse,
- FilterChain filterChain) throws IOException, ServletException {
+ FilterChain filterChain) {
fail("This filter should never have fired");
}
diff --git a/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java b/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
index 8657d38..e480541 100644
--- a/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
+++ b/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
@@ -11,6 +11,9 @@
import java.lang.reflect.Proxy;
import java.util.Map;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
import javax.servlet.http.HttpServletResponse;
@@ -26,12 +29,22 @@
private ServletTestUtils() {}
private static class ThrowingInvocationHandler implements InvocationHandler {
- @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
+ public Object invoke(Object proxy, Method method, Object[] args) throws Throwable {
throw new UnsupportedOperationException("No methods are supported on this object");
}
}
/**
+ * Returns a FilterChain that does nothing.
+ */
+ public static FilterChain newNoOpFilterChain() {
+ return new FilterChain() {
+ public void doFilter(ServletRequest request, ServletResponse response) {
+ }
+ };
+ }
+
+ /**
* Returns a fake, HttpServletRequest which stores attributes in a HashMap.
*/
public static HttpServletRequest newFakeHttpServletRequest() {
