SEC-1257: Some additional API changes to use Collection instead of List...

web/src/main/java/org/springframework/security/web/access/expression/ExpressionBasedFilterInvocationSecurityMetadataSource.java

@@ -1,8 +1,8 @@
 package org.springframework.security.web.access.expression;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 
 import org.apache.commons.logging.Log;
@@ -26,23 +26,23 @@ public final class ExpressionBasedFilterInvocationSecurityMetadataSource extends
     private final static Log logger = LogFactory.getLog(ExpressionBasedFilterInvocationSecurityMetadataSource.class);
 
     public ExpressionBasedFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher,
-            LinkedHashMap<RequestKey, List<ConfigAttribute>> requestMap, WebSecurityExpressionHandler expressionHandler) {
+            LinkedHashMap<RequestKey, Collection<ConfigAttribute>> requestMap, WebSecurityExpressionHandler expressionHandler) {
         super(urlMatcher, processMap(requestMap, expressionHandler.getExpressionParser()));
         Assert.notNull(expressionHandler, "A non-null SecurityExpressionHandler is required");
     }
 
-    private static LinkedHashMap<RequestKey, List<ConfigAttribute>> processMap(
+    private static LinkedHashMap<RequestKey, Collection<ConfigAttribute>> processMap(
-            LinkedHashMap<RequestKey, List<ConfigAttribute>> requestMap, ExpressionParser parser) {
+            LinkedHashMap<RequestKey,Collection<ConfigAttribute>> requestMap, ExpressionParser parser) {
         Assert.notNull(parser, "SecurityExpressionHandler returned a null parser object");
 
-        LinkedHashMap<RequestKey, List<ConfigAttribute>> requestToExpressionAttributesMap =
+        LinkedHashMap<RequestKey, Collection<ConfigAttribute>> requestToExpressionAttributesMap =
-            new LinkedHashMap<RequestKey, List<ConfigAttribute>>(requestMap);
+            new LinkedHashMap<RequestKey, Collection<ConfigAttribute>>(requestMap);
 
-        for (Map.Entry<RequestKey, List<ConfigAttribute>> entry : requestMap.entrySet()) {
+        for (Map.Entry<RequestKey, Collection<ConfigAttribute>> entry : requestMap.entrySet()) {
             RequestKey request = entry.getKey();
             Assert.isTrue(entry.getValue().size() == 1, "Expected a single expression attribute for " + request);
             ArrayList<ConfigAttribute> attributes = new ArrayList<ConfigAttribute>(1);
-            String expression = entry.getValue().get(0).getAttribute();
+            String expression = entry.getValue().toArray(new ConfigAttribute[1])[0].getAttribute();
             logger.debug("Adding web access control expression '" + expression + "', for " + request);
             try {
                 attributes.add(new WebExpressionConfigAttribute(parser.parseExpression(expression)));

web/src/main/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSource.java

@@ -20,7 +20,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
@@ -60,8 +59,8 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
 
     //private Map<Object, List<ConfigAttribute>> requestMap = new LinkedHashMap<Object, List<ConfigAttribute>>();
     /** Stores request maps keyed by specific HTTP methods. A null key matches any method */
-    private Map<String, Map<Object, List<ConfigAttribute>>> httpMethodMap =
+    private Map<String, Map<Object, Collection<ConfigAttribute>>> httpMethodMap =
-        new HashMap<String, Map<Object, List<ConfigAttribute>>>();
+        new HashMap<String, Map<Object, Collection<ConfigAttribute>>>();
 
     private UrlMatcher urlMatcher;
 
@@ -78,10 +77,10 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
      * @param requestMap order-preserving map of request definitions to attribute lists
      */
     public DefaultFilterInvocationSecurityMetadataSource(UrlMatcher urlMatcher,
-            LinkedHashMap<RequestKey, List<ConfigAttribute>> requestMap) {
+            LinkedHashMap<RequestKey, Collection<ConfigAttribute>> requestMap) {
         this.urlMatcher = urlMatcher;
 
-        for (Map.Entry<RequestKey, List<ConfigAttribute>> entry : requestMap.entrySet()) {
+        for (Map.Entry<RequestKey, Collection<ConfigAttribute>> entry : requestMap.entrySet()) {
             addSecureUrl(entry.getKey().getUrl(), entry.getKey().getMethod(), entry.getValue());
         }
     }
@@ -94,13 +93,13 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
      * to the request map and will be passed back to the <tt>UrlMatcher</tt> when iterating through the map to find
      * a match for a particular URL.
      */
-    private void addSecureUrl(String pattern, String method, List<ConfigAttribute> attr) {
+    private void addSecureUrl(String pattern, String method, Collection<ConfigAttribute> attrs) {
-        Map<Object, List<ConfigAttribute>> mapToUse = getRequestMapForHttpMethod(method);
+        Map<Object, Collection<ConfigAttribute>> mapToUse = getRequestMapForHttpMethod(method);
 
-        mapToUse.put(urlMatcher.compile(pattern), attr);
+        mapToUse.put(urlMatcher.compile(pattern), attrs);
 
         if (logger.isDebugEnabled()) {
-            logger.debug("Added URL pattern: " + pattern + "; attributes: " + attr +
+            logger.debug("Added URL pattern: " + pattern + "; attributes: " + attrs +
                     (method == null ? "" : " for HTTP method '" + method + "'"));
         }
     }
@@ -110,15 +109,15 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
      * @param method GET, POST etc
      * @return map of URL patterns to <tt>ConfigAttribute</tt>s for this method.
      */
-    private Map<Object, List<ConfigAttribute>> getRequestMapForHttpMethod(String method) {
+    private Map<Object, Collection<ConfigAttribute>> getRequestMapForHttpMethod(String method) {
         if (method != null && !HTTP_METHODS.contains(method)) {
             throw new IllegalArgumentException("Unrecognised HTTP method: '" + method + "'");
         }
 
-        Map<Object, List<ConfigAttribute>> methodRequestMap = httpMethodMap.get(method);
+        Map<Object, Collection<ConfigAttribute>> methodRequestMap = httpMethodMap.get(method);
 
         if (methodRequestMap == null) {
-            methodRequestMap = new LinkedHashMap<Object, List<ConfigAttribute>>();
+            methodRequestMap = new LinkedHashMap<Object, Collection<ConfigAttribute>>();
             httpMethodMap.put(method, methodRequestMap);
         }
 
@@ -128,8 +127,8 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
     public Collection<ConfigAttribute> getAllConfigAttributes() {
         Set<ConfigAttribute> allAttributes = new HashSet<ConfigAttribute>();
 
-        for (Map.Entry<String, Map<Object, List<ConfigAttribute>>> entry : httpMethodMap.entrySet()) {
+        for (Map.Entry<String, Map<Object, Collection<ConfigAttribute>>> entry : httpMethodMap.entrySet()) {
-            for (List<ConfigAttribute> attrs : entry.getValue().values()) {
+            for (Collection<ConfigAttribute> attrs : entry.getValue().values()) {
                 allAttributes.addAll(attrs);
             }
         }
@@ -161,7 +160,7 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
      * @return the <code>ConfigAttribute</code>s that apply to the specified <code>FilterInvocation</code>
      * or null if no match is found
      */
-    public final List<ConfigAttribute> lookupAttributes(String url, String method) {
+    public final Collection<ConfigAttribute> lookupAttributes(String url, String method) {
         if (stripQueryStringFromUrls) {
             // Strip anything after a question mark symbol, as per SEC-161. See also SEC-321
             int firstQuestionMarkIndex = url.indexOf("?");
@@ -180,7 +179,7 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
         }
 
         // Obtain the map of request patterns to attributes for this method and lookup the url.
-        List<ConfigAttribute> attributes = extractMatchingAttributes(url, httpMethodMap.get(method));
+        Collection<ConfigAttribute> attributes = extractMatchingAttributes(url, httpMethodMap.get(method));
 
         // If no attributes found in method-specific map, use the general one stored under the null key
         if (attributes == null) {
@@ -190,14 +189,14 @@ public class DefaultFilterInvocationSecurityMetadataSource implements FilterInvo
         return attributes;
     }
 
-    private List<ConfigAttribute> extractMatchingAttributes(String url, Map<Object, List<ConfigAttribute>> requestMap) {
+    private Collection<ConfigAttribute> extractMatchingAttributes(String url, Map<Object, Collection<ConfigAttribute>> map) {
-        if (requestMap == null) {
+        if (map == null) {
             return null;
         }
 
         final boolean debug = logger.isDebugEnabled();
 
-        for (Map.Entry<Object, List<ConfigAttribute>> entry : requestMap.entrySet()) {
+        for (Map.Entry<Object, Collection<ConfigAttribute>> entry : map.entrySet()) {
             Object p = entry.getKey();
             boolean matched = urlMatcher.pathMatchesUrl(entry.getKey(), url);
 

web/src/test/java/org/springframework/security/web/access/channel/ChannelProcessingFilterTests.java

@@ -20,7 +20,6 @@ import static org.mockito.Mockito.mock;
 
 import java.io.IOException;
 import java.util.Collection;
-import java.util.List;
 
 import javax.servlet.FilterChain;
 import javax.servlet.ServletException;
@@ -120,8 +119,7 @@ public class ChannelProcessingFilterTests {
     }
 
     @Test
-    public void testDoFilterWhenNullConfigAttributeReturned()
+    public void testDoFilterWhenNullConfigAttributeReturned() throws Exception {
-        throws Exception {
         ChannelProcessingFilter filter = new ChannelProcessingFilter();
         filter.setChannelDecisionManager(new MockChannelDecisionManager(false, "NOT_USED"));
 
@@ -180,7 +178,7 @@ public class ChannelProcessingFilterTests {
     }
 
     private class MockFilterInvocationDefinitionMap implements FilterInvocationSecurityMetadataSource {
-        private List<ConfigAttribute> toReturn;
+        private Collection<ConfigAttribute> toReturn;
         private String servletPath;
         private boolean provideIterator;
 
@@ -190,7 +188,7 @@ public class ChannelProcessingFilterTests {
             this.provideIterator = provideIterator;
         }
 
-        public List<ConfigAttribute> getAttributes(Object object)
+        public Collection<ConfigAttribute> getAttributes(Object object)
             throws IllegalArgumentException {
             FilterInvocation fi = (FilterInvocation) object;
 

web/src/test/java/org/springframework/security/web/access/intercept/DefaultFilterInvocationSecurityMetadataSourceTests.java

@@ -20,7 +20,6 @@ import static org.mockito.Mockito.mock;
 
 import java.util.Collection;
 import java.util.LinkedHashMap;
-import java.util.List;
 
 import javax.servlet.FilterChain;
 
@@ -44,7 +43,7 @@ import org.springframework.security.web.util.AntUrlPathMatcher;
 @SuppressWarnings("unchecked")
 public class DefaultFilterInvocationSecurityMetadataSourceTests {
     private DefaultFilterInvocationSecurityMetadataSource fids;
-    private List<ConfigAttribute> def = SecurityConfig.createList("ROLE_ONE");
+    private Collection<ConfigAttribute> def = SecurityConfig.createList("ROLE_ONE");
 
     //~ Methods ========================================================================================================
     private void createFids(String url, String method) {
@@ -87,7 +86,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
         FilterInvocation fi = createFilterInvocation("/secure/super/somefile.html", null);
 
-        List<? extends ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
+        Collection<ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
         assertEquals(def, response);
     }
 
@@ -97,7 +96,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
         FilterInvocation fi = createFilterInvocation("/SeCuRE/super/somefile.html", null);
 
-        List<? extends ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
+        Collection<ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
         assertEquals(null, response);
     }
 
@@ -107,7 +106,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
         FilterInvocation fi = createFilterInvocation("/SeCurE/super/somefile.html", null);
 
-        List<? extends ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
+        Collection<ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
         assertEquals(def, response);
     }
 
@@ -117,7 +116,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
         FilterInvocation fi = createFilterInvocation("/someAdminPage.html?a=/test", null);
 
-        List<? extends ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
+        Collection<ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
         assertEquals(def, response); // see SEC-161 (it should truncate after ? sign)
     }
 
@@ -155,10 +154,10 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
     @Test
     public void httpMethodSpecificUrlTakesPrecedence() {
-        LinkedHashMap<RequestKey, List<ConfigAttribute>> requestMap = new LinkedHashMap<RequestKey, List<ConfigAttribute>>();
+        LinkedHashMap<RequestKey, Collection<ConfigAttribute>> requestMap = new LinkedHashMap<RequestKey, Collection<ConfigAttribute>>();
         // Even though this is added before the Http method-specific def, the latter should match
         requestMap.put(new RequestKey("/**"), def);
-        List<ConfigAttribute> postOnlyDef = SecurityConfig.createList("ROLE_TWO");
+        Collection<ConfigAttribute> postOnlyDef = SecurityConfig.createList("ROLE_TWO");
         requestMap.put(new RequestKey("/somepage**", "POST"), postOnlyDef);
         fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap);
 
@@ -170,7 +169,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
     @Test
     public void mixingPatternsWithAndWithoutHttpMethodsIsSupported() throws Exception {
         LinkedHashMap requestMap = new LinkedHashMap();
-        List<ConfigAttribute> userAttrs = SecurityConfig.createList("A");
+        Collection<ConfigAttribute> userAttrs = SecurityConfig.createList("A");
         requestMap.put(new RequestKey("/user/**", null), userAttrs);
         requestMap.put(new RequestKey("/teller/**", "GET"), SecurityConfig.createList("B"));
         fids = new DefaultFilterInvocationSecurityMetadataSource(new AntUrlPathMatcher(), requestMap);
@@ -190,7 +189,7 @@ public class DefaultFilterInvocationSecurityMetadataSourceTests {
 
         FilterInvocation fi = createFilterInvocation("/someAdminPage.html?x=2/aa?y=3", null);
 
-        List<? extends ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
+        Collection<ConfigAttribute> response = fids.lookupAttributes(fi.getRequestUrl(), null);
         assertEquals(def, response);
 
         fi = createFilterInvocation("/someAdminPage.html??", null);