Revision: fee2d237bd4d
Author: Sam Berlin <[email protected]>
Date: Mon Oct 31 13:35:33 2011
Log: 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
http://code.google.com/p/google-guice/source/detail?r=fee2d237bd4d
Modified:
/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
/extensions/servlet/src/com/google/inject/servlet/ServletModule.java
/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
=======================================
---
/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
Mon Oct 31 13:34:04 2011
+++
/extensions/servlet/src/com/google/inject/servlet/FilterChainInvocation.java
Mon Oct 31 13:35:33 2011
@@ -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;
=======================================
---
/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
Thu Jul 7 17:34:16 2011
+++
/extensions/servlet/src/com/google/inject/servlet/FiltersModuleBuilder.java
Mon Oct 31 13:35:33 2011
@@ -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 [email protected] (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) {
@@ -59,16 +47,6 @@
public ServletModule.FilterKeyBindingBuilder filterRegex(List<String>
regexes) {
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 {
@@ -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);
}
}
=======================================
---
/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
Tue Sep 27 08:36:19 2011
+++
/extensions/servlet/src/com/google/inject/servlet/ManagedFilterPipeline.java
Mon Oct 31 13:35:33 2011
@@ -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()]);
}
=======================================
--- /extensions/servlet/src/com/google/inject/servlet/ServletModule.java
Tue Oct 18 13:43:18 2011
+++ /extensions/servlet/src/com/google/inject/servlet/ServletModule.java
Mon Oct 31 13:35:33 2011
@@ -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;
=======================================
---
/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
Thu Jul 7 17:34:16 2011
+++
/extensions/servlet/src/com/google/inject/servlet/ServletsModuleBuilder.java
Mon Oct 31 13:35:33 2011
@@ -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 ([email protected])
*/
-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--
@@ -69,16 +51,6 @@
public ServletModule.ServletKeyBindingBuilder serveRegex(List<String>
regexes) {
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 {
@@ -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);
}
}
=======================================
---
/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
Thu Jul 7 17:34:16 2011
+++
/extensions/servlet/test/com/google/inject/servlet/FilterDefinitionTest.java
Mon Oct 31 13:35:33 2011
@@ -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,15 +218,9 @@
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;
@@ -240,10 +233,6 @@
public boolean isDestroy() {
return destroy;
}
-
- public boolean isRun() {
- return run;
- }
public FilterConfig getConfig() {
return config;
=======================================
---
/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
Mon Oct 31 13:34:04 2011
+++
/extensions/servlet/test/com/google/inject/servlet/FilterDispatchIntegrationTest.java
Mon Oct 31 13:35:33 2011
@@ -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,11 +249,62 @@
}
}
+ @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() {
@@ -295,7 +363,7 @@
@Override
protected void service(HttpServletRequest req, HttpServletResponse
resp)
- throws ServletException, IOException {
+ throws ServletException {
throw new ServletException("failure!");
}
=======================================
---
/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
Thu Jul 7 17:34:16 2011
+++
/extensions/servlet/test/com/google/inject/servlet/FilterPipelineTest.java
Mon Oct 31 13:35:33 2011
@@ -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");
}
=======================================
---
/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
Mon Oct 31 13:34:04 2011
+++
/extensions/servlet/test/com/google/inject/servlet/ServletTestUtils.java
Mon Oct 31 13:35:33 2011
@@ -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,10 +29,20 @@
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.
--
You received this message because you are subscribed to the Google Groups
"google-guice-dev" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/google-guice-dev?hl=en.
