SEC-1012: Minor improvements to SecurityContextHolderAwareRequestFilter and conversion to use jmock for test.

core/src/main/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilter.java

@@ -32,13 +32,15 @@ import org.springframework.util.ReflectionUtils;
 
 
 /**
- * A <code>Filter</code> which populates the <code>ServletRequest</code> with a new request wrapper.<p>Several
- * request wrappers are included with the framework. The simplest version is {@link
- * SecurityContextHolderAwareRequestWrapper}. A more complex and powerful request wrapper is {@link
- * org.springframework.security.wrapper.SavedRequestAwareWrapper}. The latter is also the default.</p>
- *  <p>To modify the wrapper used, call {@link #setWrapperClass(Class)}.</p>
- *  <p>Any request wrapper configured for instantiation by this class must provide a public constructor that
- * accepts two arguments, being a <code>HttpServletRequest</code> and a <code>PortResolver</code>.</p>
+ * A <code>Filter</code> which populates the <code>ServletRequest</code> with a new request wrapper.
+ * Several request wrappers are included with the framework. The simplest version is {@link
+ * SecurityContextHolderAwareRequestWrapper}. A more complex and powerful request wrapper is
+ * {@link SavedRequestAwareWrapper}. The latter is also the default.
+ * <p>
+ * To modify the wrapper used, call {@link #setWrapperClass(Class)}.
+ * <p>
+ * Any request wrapper configured for instantiation by this class must provide a public constructor that
+ * accepts two arguments, being a <code>HttpServletRequest</code> and a <code>PortResolver</code>.
  *
  * @author Orlando Garcia Carmona
  * @author Ben Alex
@@ -47,8 +49,8 @@ import org.springframework.util.ReflectionUtils;
 public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilter {
     //~ Instance fields ================================================================================================
 
-    private Class wrapperClass = SavedRequestAwareWrapper.class;
-    private Constructor constructor;
+    private Class<? extends HttpServletRequest> wrapperClass = SavedRequestAwareWrapper.class;
+    private Constructor<? extends HttpServletRequest> constructor;
     private PortResolver portResolver = new PortResolverImpl();
     private String rolePrefix;
 
@@ -59,6 +61,7 @@ public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilte
         this.portResolver = portResolver;
     }
 
+    @SuppressWarnings("unchecked")
     public void setWrapperClass(Class wrapperClass) {
         Assert.notNull(wrapperClass, "WrapperClass required");
         Assert.isTrue(HttpServletRequest.class.isAssignableFrom(wrapperClass), "Wrapper must be a HttpServletRequest");
@@ -70,28 +73,23 @@ public class SecurityContextHolderAwareRequestFilter extends SpringSecurityFilte
         this.rolePrefix = rolePrefix.trim();
     }
 
-	protected void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
+    protected void doFilterHttp(HttpServletRequest request, HttpServletResponse response, FilterChain chain) throws IOException, ServletException {
         if (!wrapperClass.isAssignableFrom(request.getClass())) {
-            if (constructor == null) {
-                try {
-                    constructor = wrapperClass.getConstructor(
-                            new Class[] {HttpServletRequest.class, PortResolver.class, String.class});
-                } catch (Exception ex) {
-                    ReflectionUtils.handleReflectionException(ex);
+            try {
+                if (constructor == null) {
+                    constructor = wrapperClass.getConstructor(HttpServletRequest.class, PortResolver.class, String.class);
                 }
-            }
 
-            try {
-                request = (HttpServletRequest) constructor.newInstance(new Object[] {request, portResolver, rolePrefix});
+                request = constructor.newInstance(request, portResolver, rolePrefix);
             } catch (Exception ex) {
                 ReflectionUtils.handleReflectionException(ex);
             }
         }
 
         chain.doFilter(request, response);
-	}
+    }
 
-	public int getOrder() {
-		return FilterChainOrder.SERVLET_API_SUPPORT_FILTER;
-	}
+    public int getOrder() {
+        return FilterChainOrder.SERVLET_API_SUPPORT_FILTER;
+    }
 }

core/src/test/java/org/springframework/security/wrapper/SecurityContextHolderAwareRequestFilterTests.java

@@ -15,19 +15,17 @@
 
 package org.springframework.security.wrapper;
 
-import junit.framework.TestCase;
-
-import org.springframework.security.MockFilterConfig;
+import javax.servlet.FilterChain;
+import javax.servlet.FilterConfig;
+import javax.servlet.http.HttpServletResponse;
 
+import org.jmock.Expectations;
+import org.jmock.Mockery;
+import org.jmock.integration.junit4.JUnit4Mockery;
+import org.junit.Test;
 import org.springframework.mock.web.MockHttpServletRequest;
 import org.springframework.mock.web.MockHttpServletResponse;
-
-import java.io.IOException;
-
-import javax.servlet.FilterChain;
-import javax.servlet.ServletException;
-import javax.servlet.ServletRequest;
-import javax.servlet.ServletResponse;
+import org.springframework.security.util.PortResolverImpl;
 
 
 /**
@@ -36,51 +34,30 @@ import javax.servlet.ServletResponse;
  * @author Ben Alex
  * @version $Id$
  */
-public class SecurityContextHolderAwareRequestFilterTests extends TestCase {
-    //~ Constructors ===================================================================================================
-
-    public SecurityContextHolderAwareRequestFilterTests() {
-    }
-
-    public SecurityContextHolderAwareRequestFilterTests(String arg0) {
-        super(arg0);
-    }
+public class SecurityContextHolderAwareRequestFilterTests {
+    Mockery jmock = new JUnit4Mockery();
 
     //~ Methods ========================================================================================================
 
-    public final void setUp() throws Exception {
-        super.setUp();
-    }
-
-    public void testCorrectOperation() throws Exception {
+    @Test
+    public void expectedRequestWrapperClassIsUsed() throws Exception {
         SecurityContextHolderAwareRequestFilter filter = new SecurityContextHolderAwareRequestFilter();
-        filter.init(new MockFilterConfig());
-        filter.doFilter(new MockHttpServletRequest(null, null), new MockHttpServletResponse(),
-            new MockFilterChain(SavedRequestAwareWrapper.class));
+        filter.setPortResolver(new PortResolverImpl());
+        filter.setWrapperClass(SavedRequestAwareWrapper.class);
+        filter.setRolePrefix("ROLE_");
+        filter.init(jmock.mock(FilterConfig.class));
+        final FilterChain filterChain = jmock.mock(FilterChain.class);
 
-        // Now re-execute the filter, ensuring our replacement wrapper is still used
-        filter.doFilter(new MockHttpServletRequest(null, null), new MockHttpServletResponse(),
-            new MockFilterChain(SavedRequestAwareWrapper.class));
+        jmock.checking(new Expectations() {{
+            exactly(2).of(filterChain).doFilter(
+                    with(aNonNull(SavedRequestAwareWrapper.class)), with(aNonNull(HttpServletResponse.class)));
+        }});
 
-        filter.destroy();
-    }
+        filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain);
 
-    //~ Inner Classes ==================================================================================================
-
-    private class MockFilterChain implements FilterChain {
-        private Class expectedServletRequest;
-
-        public MockFilterChain(Class expectedServletRequest) {
-            this.expectedServletRequest = expectedServletRequest;
-        }
+        // Now re-execute the filter, ensuring our replacement wrapper is still used
+        filter.doFilter(new MockHttpServletRequest(), new MockHttpServletResponse(), filterChain);
 
-        public void doFilter(ServletRequest request, ServletResponse response)
-            throws IOException, ServletException {
-            if (request.getClass().isAssignableFrom(expectedServletRequest)) {
-                assertTrue(true);
-            } else {
-                fail("Expected class to be of type " + expectedServletRequest + " but was: " + request.getClass());
-            }
-        }
+        filter.destroy();
     }
 }