فهرست منبع

SEC-1012: Java-5-ifying the ACL package.

Luke Taylor 16 سال پیش
والد
کامیت
14c50a9c96
31فایلهای تغییر یافته به همراه469 افزوده شده و 490 حذف شده
  1. 5 4
      acl/src/main/java/org/springframework/security/acls/Acl.java
  2. 9 7
      acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java
  3. 7 8
      acl/src/main/java/org/springframework/security/acls/AclService.java
  4. 2 2
      acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java
  5. 0 2
      acl/src/main/java/org/springframework/security/acls/AuditableAcl.java
  6. 2 2
      acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java
  7. 2 2
      acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java
  8. 1 3
      acl/src/main/java/org/springframework/security/acls/MutableAcl.java
  9. 0 2
      acl/src/main/java/org/springframework/security/acls/MutableAclService.java
  10. 2 2
      acl/src/main/java/org/springframework/security/acls/NotFoundException.java
  11. 0 1
      acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java
  12. 2 2
      acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java
  13. 7 4
      acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java
  14. 5 8
      acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java
  15. 4 9
      acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java
  16. 2 0
      acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java
  17. 4 7
      acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java
  18. 48 48
      acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java
  19. 76 74
      acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java
  20. 15 14
      acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java
  21. 48 70
      acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java
  22. 2 1
      acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java
  23. 3 1
      acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java
  24. 6 5
      acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java
  25. 6 5
      acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java
  26. 5 4
      acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java
  27. 86 90
      acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java
  28. 10 11
      acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java
  29. 39 40
      acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java
  30. 59 52
      acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java
  31. 12 10
      acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java

+ 5 - 4
acl/src/main/java/org/springframework/security/acls/Acl.java

@@ -18,6 +18,7 @@ import org.springframework.security.acls.objectidentity.ObjectIdentity;
 import org.springframework.security.acls.sid.Sid;
 
 import java.io.Serializable;
+import java.util.List;
 
 
 /**
@@ -55,7 +56,7 @@ public interface Acl extends Serializable {
      * part of advanced permission checking.</p>
      * 
      * <p>Do <em>NOT</em> use this method for making authorization decisions. Instead use {@link
-     * #isGranted(Permission[], Sid[], boolean)}.</p>
+     * #isGranted(List, List, boolean)}.</p>
      * 
      * <p>This method must operate correctly even if the <tt>Acl</tt> only represents a subset of
      * <tt>Sid</tt>s. The caller is responsible for correctly handling the result if only a subset of
@@ -64,7 +65,7 @@ public interface Acl extends Serializable {
      * @return the list of entries represented by the <tt>Acl</tt>, or <tt>null</tt> if there are
      * no entries presently associated with this <tt>Acl</tt>.
      */
-    AccessControlEntry[] getEntries();
+    List<AccessControlEntry> getEntries();
 
     /**
      * Obtains the domain object this <tt>Acl</tt> provides entries for. This is immutable once an
@@ -146,7 +147,7 @@ public interface Acl extends Serializable {
      * @throws UnloadedSidException thrown if the <tt>Acl</tt> does not have details for one or more of the
      *         <tt>Sid</tt>s passed as arguments
      */
-    boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode)
+    boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
         throws NotFoundException, UnloadedSidException;
 
     /**
@@ -166,5 +167,5 @@ public interface Acl extends Serializable {
      *
      * @return <tt>true</tt> if every passed <tt>Sid</tt> is represented by this <tt>Acl</tt> instance
      */
-    boolean isSidLoaded(Sid[] sids);
+    boolean isSidLoaded(List<Sid> sids);
 }

+ 9 - 7
acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java

@@ -1,6 +1,8 @@
 package org.springframework.security.acls;
 
 import java.io.Serializable;
+import java.util.Arrays;
+import java.util.List;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -60,8 +62,8 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 
     private boolean checkPermission(Authentication authentication, ObjectIdentity oid, Object permission) {
         // Obtain the SIDs applicable to the principal
-        Sid[] sids = sidRetrievalStrategy.getSids(authentication);
-        Permission[] requiredPermission = resolvePermission(permission);
+        List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
+        List<Permission> requiredPermission = resolvePermission(permission);
 
         try {
             // Lookup only ACLs for SIDs we're interested in
@@ -90,17 +92,17 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
     }
 
     // TODO: Add permission resolver/PermissionFactory rewrite
-    Permission[] resolvePermission(Object permission) {
+    List<Permission> resolvePermission(Object permission) {
         if (permission instanceof Integer) {
-            return new Permission[] {BasePermission.buildFromMask(((Integer)permission).intValue())};
+            return Arrays.asList(BasePermission.buildFromMask(((Integer)permission).intValue()));
         }
 
         if (permission instanceof Permission) {
-            return new Permission[] {(Permission)permission};
+            return Arrays.asList((Permission)permission);
         }
 
         if (permission instanceof Permission[]) {
-            return (Permission[]) permission;
+            return Arrays.asList((Permission[])permission);
         }
 
         if (permission instanceof String) {
@@ -114,7 +116,7 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
             }
 
             if (p != null) {
-                return new Permission[] {p};
+                return Arrays.asList(p);
             }
 
         }

+ 7 - 8
acl/src/main/java/org/springframework/security/acls/AclService.java

@@ -17,6 +17,7 @@ package org.springframework.security.acls;
 import org.springframework.security.acls.objectidentity.ObjectIdentity;
 import org.springframework.security.acls.sid.Sid;
 
+import java.util.List;
 import java.util.Map;
 
 
@@ -36,10 +37,10 @@ public interface AclService {
      *
      * @return the children (or <tt>null</tt> if none were found)
      */
-    ObjectIdentity[] findChildren(ObjectIdentity parentIdentity);
+    List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity);
 
     /**
-     * Same as {@link #readAclsById(ObjectIdentity[])} except it returns only a single Acl.
+     * Same as {@link #readAclsById(Java.util.List<ObjectIdentity>)} except it returns only a single Acl.
      * <p>
      * This method should not be called as it does not leverage the underlying implementation's potential ability to
      * filter <tt>Acl</tt> entries based on a {@link Sid} parameter.</p>
@@ -53,7 +54,7 @@ public interface AclService {
     Acl readAclById(ObjectIdentity object) throws NotFoundException;
 
     /**
-     * Same as {@link #readAclsById(ObjectIdentity[], Sid[])} except it returns only a single Acl.
+     * Same as {@link #readAclsById(List, List)} except it returns only a single Acl.
      *
      * @param object to locate an {@link Acl} for
      * @param sids the security identities for which  {@link Acl} information is required
@@ -63,8 +64,7 @@ public interface AclService {
      *
      * @throws NotFoundException if an {@link Acl} was not found for the requested {@link ObjectIdentity}
      */
-    Acl readAclById(ObjectIdentity object, Sid[] sids)
-        throws NotFoundException;
+    Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException;
 
     /**
      * Obtains all the <tt>Acl</tt>s that apply for the passed <tt>Object</tt>s.<p>The returned map is
@@ -77,7 +77,7 @@ public interface AclService {
      *
      * @throws NotFoundException if an {@link Acl} was not found for each requested {@link ObjectIdentity}
      */
-    Map readAclsById(ObjectIdentity[] objects) throws NotFoundException;
+    Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException;
 
     /**
      * Obtains all the <tt>Acl</tt>s that apply for the passed <tt>Object</tt>s, but only for the
@@ -97,6 +97,5 @@ public interface AclService {
      *
      * @throws NotFoundException if an {@link Acl} was not found for each requested {@link ObjectIdentity}
      */
-    Map readAclsById(ObjectIdentity[] objects, Sid[] sids)
-        throws NotFoundException;
+    Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids) throws NotFoundException;
 }

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/AlreadyExistsException.java

@@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException;
 public class AlreadyExistsException extends SpringSecurityException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>AlreadyExistsException</code> with the specified message.
      *
      * @param msg the detail message
@@ -35,7 +35,7 @@ public class AlreadyExistsException extends SpringSecurityException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>AlreadyExistsException</code> with the specified message
      * and root cause.
      *

+ 0 - 2
acl/src/main/java/org/springframework/security/acls/AuditableAcl.java

@@ -14,8 +14,6 @@
  */
 package org.springframework.security.acls;
 
-
-
 /**
  * A mutable ACL that provides audit capabilities.
  *

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/ChildrenExistException.java

@@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException;
 public class ChildrenExistException extends SpringSecurityException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>ChildrenExistException</code> with the specified
      * message.
      *
@@ -36,7 +36,7 @@ public class ChildrenExistException extends SpringSecurityException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>ChildrenExistException</code> with the specified
      * message and root cause.
      *

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/IdentityUnavailableException.java

@@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException;
 public class IdentityUnavailableException extends SpringSecurityException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>IdentityUnavailableException</code> with the specified message.
      *
      * @param msg the detail message
@@ -35,7 +35,7 @@ public class IdentityUnavailableException extends SpringSecurityException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>IdentityUnavailableException</code> with the specified message
      * and root cause.
      *

+ 1 - 3
acl/src/main/java/org/springframework/security/acls/MutableAcl.java

@@ -21,11 +21,9 @@ import org.springframework.security.acls.sid.Sid;
 
 /**
  * A mutable <tt>Acl</tt>.
- *
  * <p>
  * A mutable ACL must ensure that appropriate security checks are performed
  * before allowing access to its methods.
- * </p>
  *
  * @author Ben Alex
  * @version $Id$
@@ -47,7 +45,7 @@ public interface MutableAcl extends Acl {
 
     /**
      * Changes the present owner to a different owner.
-     * 
+     *
      * @param newOwner the new owner (mandatory; cannot be null)
      */
     void setOwner(Sid newOwner);

+ 0 - 2
acl/src/main/java/org/springframework/security/acls/MutableAclService.java

@@ -55,8 +55,6 @@ public interface MutableAclService extends AclService {
      *
      * @param acl to modify
      *
-     * @return DOCUMENT ME!
-     *
      * @throws NotFoundException if the relevant record could not be found (did you remember to use {@link
      *         #createAcl(ObjectIdentity)} to create the object, rather than creating it with the <code>new</code>
      *         keyword?)

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/NotFoundException.java

@@ -26,7 +26,7 @@ import org.springframework.security.SpringSecurityException;
 public class NotFoundException extends SpringSecurityException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>NotFoundException</code> with the specified message.
      *
      * @param msg the detail message
@@ -35,7 +35,7 @@ public class NotFoundException extends SpringSecurityException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>NotFoundException</code> with the specified message
      * and root cause.
      *

+ 0 - 1
acl/src/main/java/org/springframework/security/acls/OwnershipAcl.java

@@ -23,7 +23,6 @@ import org.springframework.security.acls.sid.Sid;
  * <p>
  * Generally the owner of an ACL is able to call any ACL mutator method, as
  * well as assign a new owner.
- * </p>
  *
  * @author Ben Alex
  * @version $Id$

+ 2 - 2
acl/src/main/java/org/springframework/security/acls/UnloadedSidException.java

@@ -27,7 +27,7 @@ import org.springframework.security.SpringSecurityException;
 public class UnloadedSidException extends SpringSecurityException {
     //~ Constructors ===================================================================================================
 
-/**
+    /**
      * Constructs an <code>NotFoundException</code> with the specified message.
      *
      * @param msg the detail message
@@ -36,7 +36,7 @@ public class UnloadedSidException extends SpringSecurityException {
         super(msg);
     }
 
-/**
+    /**
      * Constructs an <code>NotFoundException</code> with the specified message
      * and root cause.
      *

+ 7 - 4
acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java

@@ -15,6 +15,9 @@
 
 package org.springframework.security.acls.afterinvocation;
 
+import java.util.Arrays;
+import java.util.List;
+
 import org.springframework.security.Authentication;
 import org.springframework.security.ConfigAttribute;
 
@@ -48,15 +51,15 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider {
     protected ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
     protected SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
     protected String processConfigAttribute;
-    protected Permission[] requirePermission = {BasePermission.READ};
+    protected List<Permission> requirePermission = Arrays.asList(BasePermission.READ);
 
     //~ Constructors ===================================================================================================
 
-    public AbstractAclProvider(AclService aclService, String processConfigAttribute, Permission[] requirePermission) {
+    public AbstractAclProvider(AclService aclService, String processConfigAttribute, List<Permission> requirePermission) {
         Assert.hasText(processConfigAttribute, "A processConfigAttribute is mandatory");
         Assert.notNull(aclService, "An AclService is mandatory");
 
-        if ((requirePermission == null) || (requirePermission.length == 0)) {
+        if (requirePermission == null || requirePermission.isEmpty()) {
             throw new IllegalArgumentException("One or more requirePermission entries is mandatory");
         }
 
@@ -76,7 +79,7 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider {
         ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy.getObjectIdentity(domainObject);
 
         // Obtain the SIDs applicable to the principal
-        Sid[] sids = sidRetrievalStrategy.getSids(authentication);
+        List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
 
         Acl acl = null;
 

+ 5 - 8
acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java

@@ -15,7 +15,6 @@
 package org.springframework.security.acls.afterinvocation;
 
 import java.util.Collection;
-import java.util.Iterator;
 import java.util.List;
 
 import org.apache.commons.logging.Log;
@@ -39,8 +38,8 @@ import org.springframework.security.acls.Permission;
  * <p>
  * This after invocation provider will fire if any {@link ConfigAttribute#getAttribute()} matches the {@link
  * #processConfigAttribute}. The provider will then lookup the ACLs from the <code>AclService</code> and ensure the
- * principal is {@link org.springframework.security.acls.Acl#isGranted(org.springframework.security.acls.Permission[],
- * org.springframework.security.acls.sid.Sid[], boolean) Acl.isGranted(Permission[], Sid[], boolean)}
+ * principal is {@link org.springframework.security.acls.Acl#isGranted(List,
+ * List, boolean) Acl.isGranted(Permission[], Sid[], boolean)}
  * when presenting the {@link #requirePermission} array to that method.
  * <p>
  * If the principal does not have permission, that element will not be included in the returned
@@ -67,12 +66,13 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
 
     //~ Constructors ===================================================================================================
 
-    public AclEntryAfterInvocationCollectionFilteringProvider(AclService aclService, Permission[] requirePermission) {
+    public AclEntryAfterInvocationCollectionFilteringProvider(AclService aclService, List<Permission> requirePermission) {
         super(aclService, "AFTER_ACL_COLLECTION_READ", requirePermission);
     }
 
     //~ Methods ========================================================================================================
 
+    @SuppressWarnings("unchecked")
     public Object decide(Authentication authentication, Object object, List<ConfigAttribute> config,
             Object returnedObject) throws AccessDeniedException {
 
@@ -93,7 +93,7 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
             Filterer filterer;
 
             if (returnedObject instanceof Collection) {
-                filterer = new CollectionFilterer((Collection<?>) returnedObject);
+                filterer = new CollectionFilterer((Collection) returnedObject);
             } else if (returnedObject.getClass().isArray()) {
                 filterer = new ArrayFilterer((Object[]) returnedObject);
             } else {
@@ -102,10 +102,7 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
             }
 
             // Locate unauthorised Collection elements
-            Iterator collectionIter = filterer.iterator();
-
             for (Object domainObject : filterer) {
-
                 // Ignore nulls or entries which aren't instances of the configured domain object class
                 if (domainObject == null || !getProcessDomainObjectClass().isAssignableFrom(domainObject.getClass())) {
                     continue;

+ 4 - 9
acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java

@@ -14,7 +14,6 @@
  */
 package org.springframework.security.acls.afterinvocation;
 
-import java.util.Iterator;
 import java.util.List;
 
 import org.apache.commons.logging.Log;
@@ -39,8 +38,8 @@ import org.springframework.security.acls.Permission;
  * <p>
  * This after invocation provider will fire if any  {@link ConfigAttribute#getAttribute()} matches the {@link
  * #processConfigAttribute}. The provider will then lookup the ACLs from the <tt>AclService</tt> and ensure the
- * principal is {@link org.springframework.security.acls.Acl#isGranted(org.springframework.security.acls.Permission[],
-   org.springframework.security.acls.sid.Sid[], boolean) Acl.isGranted(Permission[], Sid[], boolean)}
+ * principal is {@link org.springframework.security.acls.Acl#isGranted(List,
+   List, boolean) Acl.isGranted(Permission[], Sid[], boolean)}
  * when presenting the {@link #requirePermission} array to that method.
  * <p>
  * Often users will setup an <code>AclEntryAfterInvocationProvider</code> with a {@link
@@ -65,7 +64,7 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme
 
     //~ Constructors ===================================================================================================
 
-    public AclEntryAfterInvocationProvider(AclService aclService, Permission[] requirePermission) {
+    public AclEntryAfterInvocationProvider(AclService aclService, List<Permission> requirePermission) {
         super(aclService, "AFTER_ACL_READ", requirePermission);
     }
 
@@ -74,8 +73,6 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme
     public Object decide(Authentication authentication, Object object, List<ConfigAttribute> config,
             Object returnedObject) throws AccessDeniedException {
 
-        Iterator iter = config.iterator();
-
         if (returnedObject == null) {
             // AclManager interface contract prohibits nulls
             // As they have permission to null/nothing, grant access
@@ -94,9 +91,7 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme
             return returnedObject;
         }
 
-        while (iter.hasNext()) {
-            ConfigAttribute attr = (ConfigAttribute) iter.next();
-
+        for (ConfigAttribute attr : config) {
             if (!this.supports(attr)) {
                 continue;
             }

+ 2 - 0
acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java

@@ -60,6 +60,7 @@ class ArrayFilterer<T> implements Filterer<T> {
      *
      * @see org.springframework.security.acls.afterinvocation.Filterer#getFilteredObject()
      */
+    @SuppressWarnings("unchecked")
     public T[] getFilteredObject() {
         // Recreate an array of same type and filter the removed objects.
         int originalSize = list.length;
@@ -87,6 +88,7 @@ class ArrayFilterer<T> implements Filterer<T> {
      *
      * @see org.springframework.security.acls.afterinvocation.Filterer#iterator()
      */
+    @SuppressWarnings("unchecked")
     public Iterator<T> iterator() {
         return new ArrayIterator(list);
     }

+ 4 - 7
acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java

@@ -15,21 +15,18 @@
 
 package org.springframework.security.acls.domain;
 
+import java.util.Arrays;
 import java.util.List;
 
 import org.springframework.security.AccessDeniedException;
 import org.springframework.security.Authentication;
 import org.springframework.security.GrantedAuthority;
-
 import org.springframework.security.acls.Acl;
-import org.springframework.security.acls.Permission;
 import org.springframework.security.acls.sid.PrincipalSid;
 import org.springframework.security.acls.sid.Sid;
 import org.springframework.security.acls.sid.SidRetrievalStrategy;
 import org.springframework.security.acls.sid.SidRetrievalStrategyImpl;
-
 import org.springframework.security.context.SecurityContextHolder;
-
 import org.springframework.util.Assert;
 
 
@@ -112,14 +109,14 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
         }
 
         // Try to get permission via ACEs within the ACL
-        Sid[] sids = sidRetrievalStrategy.getSids(authentication);
+        List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
 
-        if (acl.isGranted(new Permission[] {BasePermission.ADMINISTRATION}, sids, false)) {
+        if (acl.isGranted(Arrays.asList(BasePermission.ADMINISTRATION), sids, false)) {
             return;
         }
 
         throw new AccessDeniedException(
-            "Principal does not have required ACL permissions to perform requested operation");
+                "Principal does not have required ACL permissions to perform requested operation");
     }
 
     public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) {

+ 48 - 48
acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java

@@ -15,9 +15,10 @@
 package org.springframework.security.acls.domain;
 
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
-import java.util.Vector;
 
 import org.springframework.security.acls.AccessControlEntry;
 import org.springframework.security.acls.Acl;
@@ -44,11 +45,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
     private Acl parentAcl;
     private transient AclAuthorizationStrategy aclAuthorizationStrategy;
     private transient AuditLogger auditLogger;
-    private List aces = new Vector();
+    private List<AccessControlEntry> aces = new ArrayList<AccessControlEntry>();
     private ObjectIdentity objectIdentity;
     private Serializable id;
     private Sid owner; // OwnershipAcl
-    private Sid[] loadedSids = null; // includes all SIDs the WHERE clause covered, even if there was no ACE for a SID
+    private List<Sid> loadedSids = null; // includes all SIDs the WHERE clause covered, even if there was no ACE for a SID
     private boolean entriesInheriting = true;
 
     //~ Constructors ===================================================================================================
@@ -90,7 +91,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
      * @param owner the owner (required)
      */
     public AclImpl(ObjectIdentity objectIdentity, Serializable id, AclAuthorizationStrategy aclAuthorizationStrategy,
-                    AuditLogger auditLogger, Acl parentAcl, Sid[] loadedSids, boolean entriesInheriting, Sid owner) {
+                    AuditLogger auditLogger, Acl parentAcl, List<Sid> loadedSids, boolean entriesInheriting, Sid owner) {
         Assert.notNull(objectIdentity, "Object Identity required");
         Assert.notNull(id, "Id required");
         Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
@@ -110,19 +111,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
      * Private no-argument constructor for use by reflection-based persistence
      * tools along with field-level access.
      */
+    @SuppressWarnings("unused")
     private AclImpl() {}
 
     //~ Methods ========================================================================================================
 
-    private void verifyAceIndexExists(int aceIndex) {
-        if (aceIndex < 0) {
-            throw new NotFoundException("aceIndex must be greater than or equal to zero");
-        }
-        if (aceIndex > this.aces.size()) {
-            throw new NotFoundException("aceIndex must correctly refer to an index of the AccessControlEntry collection");
-        }
-    }
-
     public void deleteAce(int aceIndex) throws NotFoundException {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
         verifyAceIndexExists(aceIndex);
@@ -132,25 +125,13 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         }
     }
 
-    public AccessControlEntry[] getEntries() {
-        // Can safely return AccessControlEntry directly, as they're immutable outside the ACL package
-        return (AccessControlEntry[]) aces.toArray(new AccessControlEntry[] {});
-    }
-
-    public Serializable getId() {
-        return this.id;
-    }
-
-    public ObjectIdentity getObjectIdentity() {
-        return objectIdentity;
-    }
-
-    public Sid getOwner() {
-        return this.owner;
-    }
-
-    public Acl getParentAcl() {
-        return parentAcl;
+    private void verifyAceIndexExists(int aceIndex) {
+        if (aceIndex < 0) {
+            throw new NotFoundException("aceIndex must be greater than or equal to zero");
+        }
+        if (aceIndex > this.aces.size()) {
+            throw new NotFoundException("aceIndex must correctly refer to an index of the AccessControlEntry collection");
+        }
     }
 
     public void insertAce(int atIndexLocation, Permission permission, Sid sid, boolean granting) throws NotFoundException {
@@ -171,6 +152,19 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         }
     }
 
+    public List<AccessControlEntry> getEntries() {
+        // Can safely return AccessControlEntry directly, as they're immutable outside the ACL package
+        return new ArrayList<AccessControlEntry>(aces);
+    }
+
+    public Serializable getId() {
+        return this.id;
+    }
+
+    public ObjectIdentity getObjectIdentity() {
+        return objectIdentity;
+    }
+
     public boolean isEntriesInheriting() {
         return entriesInheriting;
     }
@@ -206,7 +200,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
      * @throws UnloadedSidException if the passed SIDs are unknown to this ACL because the ACL was only loaded for a
      *         subset of SIDs
      */
-    public boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode)
+    public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
             throws NotFoundException, UnloadedSidException {
         Assert.notEmpty(permission, "Permissions required");
         Assert.notEmpty(sids, "SIDs required");
@@ -217,16 +211,14 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 
         AccessControlEntry firstRejection = null;
 
-        for (int i = 0; i < permission.length; i++) {
-            for (int x = 0; x < sids.length; x++) {
+        for (Permission p : permission) {
+            for (Sid sid: sids) {
                 // Attempt to find exact match for this permission mask and SID
-                Iterator acesIterator = aces.iterator();
                 boolean scanNextSid = true;
 
-                while (acesIterator.hasNext()) {
-                    AccessControlEntry ace = (AccessControlEntry) acesIterator.next();
+                for (AccessControlEntry ace : aces ) {
 
-                    if ((ace.getPermission().getMask() == permission[i].getMask()) && ace.getSid().equals(sids[x])) {
+                    if ((ace.getPermission().getMask() == p.getMask()) && ace.getSid().equals(sid)) {
                         // Found a matching ACE, so its authorization decision will prevail
                         if (ace.isGranting()) {
                             // Success
@@ -246,7 +238,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 
                             scanNextSid = false; // helps break the loop
 
-                            break; // exit "aceIterator" while loop
+                            break; // exit aces loop
                         }
                     }
                 }
@@ -277,19 +269,19 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         }
     }
 
-    public boolean isSidLoaded(Sid[] sids) {
+    public boolean isSidLoaded(List<Sid> sids) {
         // If loadedSides is null, this indicates all SIDs were loaded
         // Also return true if the caller didn't specify a SID to find
-        if ((this.loadedSids == null) || (sids == null) || (sids.length == 0)) {
+        if ((this.loadedSids == null) || (sids == null) || (sids.size() == 0)) {
             return true;
         }
 
         // This ACL applies to a SID subset only. Iterate to check it applies.
-        for (int i = 0; i < sids.length; i++) {
+        for (Sid sid: sids) {
             boolean found = false;
 
-            for (int y = 0; y < this.loadedSids.length; y++) {
-                if (sids[i].equals(this.loadedSids[y])) {
+            for (Sid loadedSid : loadedSids) {
+                if (sid.equals(loadedSid)) {
                     // this SID is OK
                     found = true;
 
@@ -316,12 +308,20 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
         this.owner = newOwner;
     }
 
+    public Sid getOwner() {
+        return this.owner;
+    }
+
     public void setParent(Acl newParent) {
         aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
         Assert.isTrue(newParent == null || !newParent.equals(this), "Cannot be the parent of yourself");
         this.parentAcl = newParent;
     }
 
+    public Acl getParentAcl() {
+        return parentAcl;
+    }
+
     public String toString() {
         StringBuffer sb = new StringBuffer();
         sb.append("AclImpl[");
@@ -389,9 +389,9 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
                                     if ((this.loadedSids == null && rhs.loadedSids == null)) {
                                         return true;
                                     }
-                                    if (this.loadedSids.length == rhs.loadedSids.length) {
-                                        for (int i = 0; i < this.loadedSids.length; i++) {
-                                            if (!this.loadedSids[i].equals(rhs.loadedSids[i])) {
+                                    if (this.loadedSids.size() == rhs.loadedSids.size()) {
+                                        for (int i = 0; i < this.loadedSids.size(); i++) {
+                                            if (!this.loadedSids.get(i).equals(rhs.loadedSids.get(i))) {
                                                 return false;
                                             }
                                         }

+ 76 - 74
acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java

@@ -178,15 +178,14 @@ public final class BasicLookupStrategy implements LookupStrategy {
             fieldAcl.setAccessible(true);
 
             // Obtain the "aces" from the input ACL
-            Iterator i = ((List) fieldAces.get(inputAcl)).iterator();
+            List<AccessControlEntryImpl> aces = (List<AccessControlEntryImpl>) fieldAces.get(inputAcl);
 
             // Create a list in which to store the "aces" for the "result" AclImpl instance
             List<AccessControlEntryImpl> acesNew = new ArrayList<AccessControlEntryImpl>();
 
             // Iterate over the "aces" input and replace each nested AccessControlEntryImpl.getAcl() with the new "result" AclImpl instance
             // This ensures StubAclParent instances are removed, as per SEC-951
-            while(i.hasNext()) {
-                AccessControlEntryImpl ace = (AccessControlEntryImpl) i.next();
+            for (AccessControlEntryImpl ace : aces) {
                 fieldAcl.set(ace, result);
                 acesNew.add(ace);
             }
@@ -284,65 +283,6 @@ public final class BasicLookupStrategy implements LookupStrategy {
         return BasePermission.buildFromMask(mask);
     }
 
-    /**
-     * Looks up a batch of <code>ObjectIdentity</code>s directly from the database.<p>The caller is responsible
-     * for optimization issues, such as selecting the identities to lookup, ensuring the cache doesn't contain them
-     * already, and adding the returned elements to the cache etc.</p>
-     * <p>
-     * This subclass is required to return fully valid <code>Acl</code>s, including properly-configured
-     * parent ACLs.
-     *
-     */
-    private Map<ObjectIdentity, Acl> lookupObjectIdentities(final ObjectIdentity[] objectIdentities, Sid[] sids) {
-        Assert.notEmpty(objectIdentities, "Must provide identities to lookup");
-
-        final Map acls = new HashMap(); // contains Acls with StubAclParents
-
-        // Make the "acls" map contain all requested objectIdentities
-        // (including markers to each parent in the hierarchy)
-        String sql = computeRepeatingSql("(acl_object_identity.object_id_identity = ? and acl_class.class = ?)",
-                objectIdentities.length);
-
-        Set parentsToLookup = (Set) jdbcTemplate.query(sql,
-            new PreparedStatementSetter() {
-                public void setValues(PreparedStatement ps)
-                    throws SQLException {
-                    for (int i = 0; i < objectIdentities.length; i++) {
-                        // Determine prepared statement values for this iteration
-                        String javaType = objectIdentities[i].getJavaType().getName();
-
-                        // No need to check for nulls, as guaranteed non-null by ObjectIdentity.getIdentifier() interface contract
-                        String identifier = objectIdentities[i].getIdentifier().toString();
-                        long id = (Long.valueOf(identifier)).longValue();
-
-                        // Inject values
-                        ps.setLong((2 * i) + 1, id);
-                        ps.setString((2 * i) + 2, javaType);
-                    }
-                }
-            }, new ProcessResultSet(acls, sids));
-
-        // Lookup the parents, now that our JdbcTemplate has released the database connection (SEC-547)
-        if (parentsToLookup.size() > 0) {
-            lookupPrimaryKeys(acls, parentsToLookup, sids);
-        }
-
-        // Finally, convert our "acls" containing StubAclParents into true Acls
-        Map<ObjectIdentity, Acl> resultMap = new HashMap<ObjectIdentity, Acl>();
-        Iterator iter = acls.values().iterator();
-
-        while (iter.hasNext()) {
-            Acl inputAcl = (Acl) iter.next();
-            Assert.isInstanceOf(AclImpl.class, inputAcl, "Map should have contained an AclImpl");
-            Assert.isInstanceOf(Long.class, ((AclImpl) inputAcl).getId(), "Acl.getId() must be Long");
-
-            Acl result = convert(acls, (Long) ((AclImpl) inputAcl).getId());
-            resultMap.put(result.getObjectIdentity(), result);
-        }
-
-        return resultMap;
-    }
-
     /**
      * Locates the primary key IDs specified in "findNow", adding AclImpl instances with StubAclParents to the
      * "acls" Map.
@@ -351,7 +291,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
      * @param findNow Long-based primary keys to retrieve
      * @param sids
      */
-    private void lookupPrimaryKeys(final Map acls, final Set findNow, final Sid[] sids) {
+    private void lookupPrimaryKeys(final Map acls, final Set findNow, final List<Sid> sids) {
         Assert.notNull(acls, "ACLs are required");
         Assert.notEmpty(findNow, "Items to find now required");
 
@@ -395,7 +335,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
      *         should not throw {@link NotFoundException}, as a chain of {@link LookupStrategy}s may be used
      *         to automatically create entries if required)
      */
-    public Map<ObjectIdentity, Acl> readAclsById(ObjectIdentity[] objects, Sid[] sids) {
+    public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids) {
         Assert.isTrue(batchSize >= 1, "BatchSize must be >= 1");
         Assert.notEmpty(objects, "Objects to lookup required");
 
@@ -404,17 +344,17 @@ public final class BasicLookupStrategy implements LookupStrategy {
 
         Set<ObjectIdentity> currentBatchToLoad = new HashSet<ObjectIdentity>(); // contains ObjectIdentitys
 
-        for (int i = 0; i < objects.length; i++) {
+        for (int i = 0; i < objects.size(); i++) {
             boolean aclFound = false;
 
             // Check we don't already have this ACL in the results
-            if (result.containsKey(objects[i])) {
+            if (result.containsKey(objects.get(i))) {
                 aclFound = true;
             }
 
             // Check cache for the present ACL entry
             if (!aclFound) {
-                Acl acl = aclCache.getFromCache(objects[i]);
+                Acl acl = aclCache.getFromCache(objects.get(i));
 
                 // Ensure any cached element supports all the requested SIDs
                 // (they should always, as our base impl doesn't filter on SID)
@@ -432,11 +372,11 @@ public final class BasicLookupStrategy implements LookupStrategy {
 
             // Load the ACL from the database
             if (!aclFound) {
-                currentBatchToLoad.add(objects[i]);
+                currentBatchToLoad.add(objects.get(i));
             }
 
             // Is it time to load from JDBC the currentBatchToLoad?
-            if ((currentBatchToLoad.size() == this.batchSize) || ((i + 1) == objects.length)) {
+            if ((currentBatchToLoad.size() == this.batchSize) || ((i + 1) == objects.size())) {
                 if (currentBatchToLoad.size() > 0) {
                     Map<ObjectIdentity, Acl> loadedBatch = lookupObjectIdentities(currentBatchToLoad.toArray(new ObjectIdentity[] {}), sids);
 
@@ -458,6 +398,68 @@ public final class BasicLookupStrategy implements LookupStrategy {
         return result;
     }
 
+    /**
+     * Looks up a batch of <code>ObjectIdentity</code>s directly from the database.
+     * <p>
+     * The caller is responsible for optimization issues, such as selecting the identities to lookup, ensuring the
+     * cache doesn't contain them already, and adding the returned elements to the cache etc.
+     * <p>
+     * This subclass is required to return fully valid <code>Acl</code>s, including properly-configured
+     * parent ACLs.
+     *
+     */
+    @SuppressWarnings("unchecked")
+    private Map<ObjectIdentity, Acl> lookupObjectIdentities(final ObjectIdentity[] objectIdentities, List<Sid> sids) {
+        Assert.notEmpty(objectIdentities, "Must provide identities to lookup");
+
+        final Map acls = new HashMap(); // contains Acls with StubAclParents
+
+        // Make the "acls" map contain all requested objectIdentities
+        // (including markers to each parent in the hierarchy)
+        String sql = computeRepeatingSql("(acl_object_identity.object_id_identity = ? and acl_class.class = ?)",
+                objectIdentities.length);
+
+        Set parentsToLookup = (Set) jdbcTemplate.query(sql,
+            new PreparedStatementSetter() {
+                public void setValues(PreparedStatement ps)
+                    throws SQLException {
+                    for (int i = 0; i < objectIdentities.length; i++) {
+                        // Determine prepared statement values for this iteration
+                        String javaType = objectIdentities[i].getJavaType().getName();
+
+                        // No need to check for nulls, as guaranteed non-null by ObjectIdentity.getIdentifier() interface contract
+                        String identifier = objectIdentities[i].getIdentifier().toString();
+                        long id = (Long.valueOf(identifier)).longValue();
+
+                        // Inject values
+                        ps.setLong((2 * i) + 1, id);
+                        ps.setString((2 * i) + 2, javaType);
+                    }
+                }
+            }, new ProcessResultSet(acls, sids));
+
+        // Lookup the parents, now that our JdbcTemplate has released the database connection (SEC-547)
+        if (parentsToLookup.size() > 0) {
+            lookupPrimaryKeys(acls, parentsToLookup, sids);
+        }
+
+        // Finally, convert our "acls" containing StubAclParents into true Acls
+        Map<ObjectIdentity, Acl> resultMap = new HashMap<ObjectIdentity, Acl>();
+        Iterator iter = acls.values().iterator();
+
+        while (iter.hasNext()) {
+            Acl inputAcl = (Acl) iter.next();
+            Assert.isInstanceOf(AclImpl.class, inputAcl, "Map should have contained an AclImpl");
+            Assert.isInstanceOf(Long.class, ((AclImpl) inputAcl).getId(), "Acl.getId() must be Long");
+
+            Acl result = convert(acls, (Long) ((AclImpl) inputAcl).getId());
+            resultMap.put(result.getObjectIdentity(), result);
+        }
+
+        return resultMap;
+    }
+
+
     public void setBatchSize(int batchSize) {
         this.batchSize = batchSize;
     }
@@ -466,9 +468,9 @@ public final class BasicLookupStrategy implements LookupStrategy {
 
     private class ProcessResultSet implements ResultSetExtractor {
         private Map acls;
-        private Sid[] sids;
+        private List<Sid> sids;
 
-        public ProcessResultSet(Map acls, Sid[] sids) {
+        public ProcessResultSet(Map acls, List<Sid> sids) {
             Assert.notNull(acls, "ACLs cannot be null");
             this.acls = acls;
             this.sids = sids; // can be null
@@ -526,7 +528,7 @@ public final class BasicLookupStrategy implements LookupStrategy {
             this.id = id;
         }
 
-        public AccessControlEntry[] getEntries() {
+        public List<AccessControlEntry> getEntries() {
             throw new UnsupportedOperationException("Stub only");
         }
 
@@ -550,12 +552,12 @@ public final class BasicLookupStrategy implements LookupStrategy {
             throw new UnsupportedOperationException("Stub only");
         }
 
-        public boolean isGranted(Permission[] permission, Sid[] sids, boolean administrativeMode)
+        public boolean isGranted(List<Permission> permission, List<Sid> sids, boolean administrativeMode)
             throws NotFoundException, UnloadedSidException {
             throw new UnsupportedOperationException("Stub only");
         }
 
-        public boolean isSidLoaded(Sid[] sids) {
+        public boolean isSidLoaded(List<Sid> sids) {
             throw new UnsupportedOperationException("Stub only");
         }
     }

+ 15 - 14
acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java

@@ -33,6 +33,7 @@ import org.springframework.util.StringUtils;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -42,7 +43,7 @@ import javax.sql.DataSource;
 /**
  * Simple JDBC-based implementation of <code>AclService</code>.
  * <p>
- * Requires the "dirty" flags in {@link org.springframework.security.acls.domain.AclImpl} and 
+ * Requires the "dirty" flags in {@link org.springframework.security.acls.domain.AclImpl} and
  * {@link org.springframework.security.acls.domain.AccessControlEntryImpl} to be set, so that the implementation can
  * detect changed parameters easily.
  *
@@ -75,7 +76,7 @@ public class JdbcAclService implements AclService {
 
     //~ Methods ========================================================================================================
 
-    public ObjectIdentity[] findChildren(ObjectIdentity parentIdentity) {
+    public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
         Object[] args = {parentIdentity.getIdentifier(), parentIdentity.getJavaType().getName()};
         List objects = jdbcTemplate.query(selectAclObjectWithParent, args,
                 new RowMapper() {
@@ -91,12 +92,12 @@ public class JdbcAclService implements AclService {
         if (objects.size() == 0) {
             return null;
         }
-        
-        return (ObjectIdentityImpl[]) objects.toArray(new ObjectIdentityImpl[objects.size()]);
+
+        return objects;
     }
 
-    public Acl readAclById(ObjectIdentity object, Sid[] sids) throws NotFoundException {
-        Map map = readAclsById(new ObjectIdentity[] {object}, sids);
+    public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
+        Map<ObjectIdentity, Acl> map = readAclsById(Arrays.asList(object), sids);
         Assert.isTrue(map.containsKey(object), "There should have been an Acl entry for ObjectIdentity " + object);
 
         return (Acl) map.get(object);
@@ -106,21 +107,21 @@ public class JdbcAclService implements AclService {
         return readAclById(object, null);
     }
 
-    public Map readAclsById(ObjectIdentity[] objects) throws NotFoundException {
+    public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
         return readAclsById(objects, null);
     }
 
-    public Map readAclsById(ObjectIdentity[] objects, Sid[] sids) throws NotFoundException {
-        Map result = lookupStrategy.readAclsById(objects, sids);
-        
+    public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids) throws NotFoundException {
+        Map<ObjectIdentity, Acl> result = lookupStrategy.readAclsById(objects, sids);
+
         // Check every requested object identity was found (throw NotFoundException if needed)
-        for (int i = 0; i < objects.length; i++) {
-            if (!result.containsKey(objects[i])) {
+        for (int i = 0; i < objects.size(); i++) {
+            if (!result.containsKey(objects.get(i))) {
                 throw new NotFoundException("Unable to find ACL information for object identity '"
-                    + objects[i].toString() + "'");
+                    + objects.get(i).toString() + "'");
             }
         }
-        
+
         return result;
     }
 }

+ 48 - 70
acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java

@@ -14,8 +14,15 @@
  */
 package org.springframework.security.acls.jdbc;
 
-import org.springframework.security.Authentication;
+import java.sql.PreparedStatement;
+import java.sql.SQLException;
+import java.util.List;
+
+import javax.sql.DataSource;
 
+import org.springframework.dao.DataAccessException;
+import org.springframework.jdbc.core.BatchPreparedStatementSetter;
+import org.springframework.security.Authentication;
 import org.springframework.security.acls.AccessControlEntry;
 import org.springframework.security.acls.Acl;
 import org.springframework.security.acls.AlreadyExistsException;
@@ -29,26 +36,10 @@ import org.springframework.security.acls.objectidentity.ObjectIdentityImpl;
 import org.springframework.security.acls.sid.GrantedAuthoritySid;
 import org.springframework.security.acls.sid.PrincipalSid;
 import org.springframework.security.acls.sid.Sid;
-
 import org.springframework.security.context.SecurityContextHolder;
-
-import org.springframework.dao.DataAccessException;
-
-import org.springframework.jdbc.core.BatchPreparedStatementSetter;
-
 import org.springframework.transaction.support.TransactionSynchronizationManager;
-
 import org.springframework.util.Assert;
 
-import java.lang.reflect.Array;
-
-import java.sql.PreparedStatement;
-import java.sql.SQLException;
-
-import java.util.List;
-
-import javax.sql.DataSource;
-
 
 /**
  * Provides a base implementation of {@link MutableAclService}.
@@ -123,14 +114,13 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
         jdbcTemplate.batchUpdate(insertEntry,
             new BatchPreparedStatementSetter() {
                 public int getBatchSize() {
-                    return acl.getEntries().length;
+                    return acl.getEntries().size();
                 }
 
                 public void setValues(PreparedStatement stmt, int i)
                         throws SQLException {
-                    AccessControlEntry entry_ = (AccessControlEntry) Array.get(acl.getEntries(), i);
+                    AccessControlEntry entry_ = acl.getEntries().get(i);
                     Assert.isTrue(entry_ instanceof AccessControlEntryImpl, "Unknown ACE class");
-
                     AccessControlEntryImpl entry = (AccessControlEntryImpl) entry_;
 
                     stmt.setLong(1, ((Long) acl.getId()).longValue());
@@ -167,23 +157,21 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
      *
      * @return the primary key or null if not found
      */
-    protected Long createOrRetrieveClassPrimaryKey(Class clazz, boolean allowCreate) {
-        List classIds = jdbcTemplate.queryForList(selectClassPrimaryKey, new Object[] {clazz.getName()}, Long.class);
-        Long classId = null;
-
-        if (classIds.isEmpty()) {
-            if (allowCreate) {
-                classId = null;
-                jdbcTemplate.update(insertClass, new Object[] {clazz.getName()});
-                Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
-                        "Transaction must be running");
-                classId = new Long(jdbcTemplate.queryForLong(classIdentityQuery));
-            }
-        } else {
-            classId = (Long) classIds.iterator().next();
+    protected Long createOrRetrieveClassPrimaryKey(Class<?> clazz, boolean allowCreate) {
+        List<Long> classIds = jdbcTemplate.queryForList(selectClassPrimaryKey, new Object[] {clazz.getName()}, Long.class);
+
+        if (!classIds.isEmpty()) {
+            return classIds.get(0);
+        }
+
+        if (allowCreate) {
+            jdbcTemplate.update(insertClass, new Object[] {clazz.getName()});
+            Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
+                    "Transaction must be running");
+            return new Long(jdbcTemplate.queryForLong(classIdentityQuery));
         }
 
-        return classId;
+        return null;
     }
 
     /**
@@ -195,7 +183,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
      *
      * @return the primary key or null if not found
      *
-     * @throws IllegalArgumentException DOCUMENT ME!
+     * @throws IllegalArgumentException if the <tt>Sid</tt> is not a recognized implementation.
      */
     protected Long createOrRetrieveSidPrimaryKey(Sid sid, boolean allowCreate) {
         Assert.notNull(sid, "Sid required");
@@ -212,23 +200,21 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
             throw new IllegalArgumentException("Unsupported implementation of Sid");
         }
 
-        List sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey, new Object[] {new Boolean(principal), sidName},
-                Long.class);
-        Long sidId = null;
-
-        if (sidIds.isEmpty()) {
-            if (allowCreate) {
-                sidId = null;
-                jdbcTemplate.update(insertSid, new Object[] {new Boolean(principal), sidName});
-                Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
-                        "Transaction must be running");
-                sidId = new Long(jdbcTemplate.queryForLong(sidIdentityQuery));
-            }
-        } else {
-            sidId = (Long) sidIds.iterator().next();
+        List<Long> sidIds = jdbcTemplate.queryForList(selectSidPrimaryKey,
+                new Object[] {new Boolean(principal), sidName},  Long.class);
+
+        if (!sidIds.isEmpty()) {
+            return sidIds.get(0);
+        }
+
+        if (allowCreate) {
+            jdbcTemplate.update(insertSid, new Object[] {new Boolean(principal), sidName});
+            Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(),
+                    "Transaction must be running");
+            return new Long(jdbcTemplate.queryForLong(sidIdentityQuery));
         }
 
-        return sidId;
+        return null;
     }
 
     public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren)
@@ -237,26 +223,26 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
         Assert.notNull(objectIdentity.getIdentifier(), "Object Identity doesn't provide an identifier");
 
         if (deleteChildren) {
-            ObjectIdentity[] children = findChildren(objectIdentity);
+            List<ObjectIdentity> children = findChildren(objectIdentity);
             if (children != null) {
-                for (int i = 0; i < children.length; i++) {
-                    deleteAcl(children[i], true);
+                for (int i = 0; i < children.size(); i++) {
+                    deleteAcl(children.get(i), true);
                 }
             }
         } else {
             if (!foreignKeysInDatabase) {
                 // We need to perform a manual verification for what a FK would normally do
                 // We generally don't do this, in the interests of deadlock management
-                ObjectIdentity[] children = findChildren(objectIdentity);
+                List<ObjectIdentity> children = findChildren(objectIdentity);
                 if (children != null) {
-                    throw new ChildrenExistException("Cannot delete '" + objectIdentity + "' (has " + children.length
+                    throw new ChildrenExistException("Cannot delete '" + objectIdentity + "' (has " + children.size()
                             + " children)");
                 }
             }
         }
 
         Long oidPrimaryKey = retrieveObjectIdentityPrimaryKey(objectIdentity);
-        
+
         // Delete this ACL's ACEs in the acl_entry table
         deleteEntries(oidPrimaryKey);
 
@@ -279,11 +265,9 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 
     /**
      * Deletes a single row from acl_object_identity that is associated with the presented ObjectIdentity primary key.
-     * 
      * <p>
      * We do not delete any entries from acl_class, even if no classes are using that class any longer. This is a
      * deadlock avoidance approach.
-     * </p>
      *
      * @param oidPrimaryKey to delete the acl_object_identity
      */
@@ -314,12 +298,6 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
      * This implementation will simply delete all ACEs in the database and recreate them on each invocation of
      * this method. A more comprehensive implementation might use dirty state checking, or more likely use ORM
      * capabilities for create, update and delete operations of {@link MutableAcl}.
-     *
-     * @param acl DOCUMENT ME!
-     *
-     * @return DOCUMENT ME!
-     *
-     * @throws NotFoundException DOCUMENT ME!
      */
     public MutableAcl updateAcl(MutableAcl acl) throws NotFoundException {
         Assert.notNull(acl.getId(), "Object Identity doesn't provide an identifier");
@@ -339,13 +317,13 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
         // Retrieve the ACL via superclass (ensures cache registration, proper retrieval etc)
         return (MutableAcl) super.readAclById(acl.getObjectIdentity());
     }
-    
+
     private void clearCacheIncludingChildren(ObjectIdentity objectIdentity) {
         Assert.notNull(objectIdentity, "ObjectIdentity required");
-        ObjectIdentity[] children = findChildren(objectIdentity);
+        List<ObjectIdentity> children = findChildren(objectIdentity);
         if (children != null) {
-            for (int i = 0; i < children.length; i++) {
-                clearCacheIncludingChildren(children[i]);
+            for (int i = 0; i < children.size(); i++) {
+                clearCacheIncludingChildren(children.get(i));
             }
         }
         aclCache.evictFromCache(objectIdentity);
@@ -357,7 +335,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
      *
      * @param acl to modify (a row must already exist in acl_object_identity)
      *
-     * @throws NotFoundException DOCUMENT ME!
+     * @throws NotFoundException if the ACL could not be found to update.
      */
     protected void updateObjectIdentity(MutableAcl acl) {
         Long parentId = null;

+ 2 - 1
acl/src/main/java/org/springframework/security/acls/jdbc/LookupStrategy.java

@@ -19,6 +19,7 @@ import org.springframework.security.acls.NotFoundException;
 import org.springframework.security.acls.objectidentity.ObjectIdentity;
 import org.springframework.security.acls.sid.Sid;
 
+import java.util.List;
 import java.util.Map;
 
 
@@ -43,5 +44,5 @@ public interface LookupStrategy {
      *         should not throw {@link NotFoundException}, as a chain of {@link LookupStrategy}s may be used
      *         to automatically create entries if required)
      */
-    Map<ObjectIdentity, Acl> readAclsById(ObjectIdentity[] objects, Sid[] sids);
+    Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids);
 }

+ 3 - 1
acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategy.java

@@ -15,6 +15,8 @@
 
 package org.springframework.security.acls.sid;
 
+import java.util.List;
+
 import org.springframework.security.Authentication;
 
 
@@ -28,5 +30,5 @@ import org.springframework.security.Authentication;
 public interface SidRetrievalStrategy {
     //~ Methods ========================================================================================================
 
-    Sid[] getSids(Authentication authentication);
+    List<Sid> getSids(Authentication authentication);
 }

+ 6 - 5
acl/src/main/java/org/springframework/security/acls/sid/SidRetrievalStrategyImpl.java

@@ -15,6 +15,7 @@
 
 package org.springframework.security.acls.sid;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import org.springframework.security.Authentication;
@@ -32,14 +33,14 @@ import org.springframework.security.GrantedAuthority;
 public class SidRetrievalStrategyImpl implements SidRetrievalStrategy {
     //~ Methods ========================================================================================================
 
-    public Sid[] getSids(Authentication authentication) {
+    public List<Sid> getSids(Authentication authentication) {
         List<GrantedAuthority> authorities = authentication.getAuthorities();
-        Sid[] sids = new Sid[authorities.size() + 1];
+        List<Sid> sids = new ArrayList<Sid>(authorities.size() + 1);
 
-        sids[0] = new PrincipalSid(authentication);
+        sids.add(new PrincipalSid(authentication));
 
-        for (int i = 1; i <= authorities.size(); i++) {
-            sids[i] = new GrantedAuthoritySid(authorities.get(i - 1));
+        for (GrantedAuthority authority : authorities) {
+            sids.add(new GrantedAuthoritySid(authority));
         }
 
         return sids;

+ 6 - 5
acl/src/main/java/org/springframework/security/acls/vote/AclEntryVoter.java

@@ -16,6 +16,7 @@ package org.springframework.security.acls.vote;
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.Arrays;
 import java.util.List;
 
 import org.apache.commons.logging.Log;
@@ -49,8 +50,8 @@ import org.springframework.util.StringUtils;
  * The voter will vote if any  {@link ConfigAttribute#getAttribute()} matches the {@link #processConfigAttribute}.
  * The provider will then locate the first method argument of type {@link #processDomainObjectClass}. Assuming that
  * method argument is non-null, the provider will then lookup the ACLs from the <code>AclManager</code> and ensure the
- * principal is {@link Acl#isGranted(org.springframework.security.acls.Permission[],
- * org.springframework.security.acls.sid.Sid[], boolean)} when presenting the {@link #requirePermission} array to that
+ * principal is {@link Acl#isGranted(List,
+ * List, boolean)} when presenting the {@link #requirePermission} array to that
  * method.
  * <p>
  * If the method argument is <tt>null</tt>, the voter will abstain from voting. If the method argument
@@ -91,7 +92,7 @@ public class AclEntryVoter extends AbstractAclVoter {
     private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
     private String internalMethod;
     private String processConfigAttribute;
-    private Permission[] requirePermission;
+    private List<Permission> requirePermission;
 
     //~ Constructors ===================================================================================================
 
@@ -105,7 +106,7 @@ public class AclEntryVoter extends AbstractAclVoter {
 
         this.aclService = aclService;
         this.processConfigAttribute = processConfigAttribute;
-        this.requirePermission = requirePermission;
+        this.requirePermission = Arrays.asList(requirePermission);
     }
 
     //~ Methods ========================================================================================================
@@ -196,7 +197,7 @@ public class AclEntryVoter extends AbstractAclVoter {
             ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy.getObjectIdentity(domainObject);
 
             // Obtain the SIDs applicable to the principal
-            Sid[] sids = sidRetrievalStrategy.getSids(authentication);
+            List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
 
             Acl acl;
 

+ 5 - 4
acl/src/test/java/org/springframework/security/acls/AclPermissionEvaluatorTests.java

@@ -1,6 +1,8 @@
 package org.springframework.security.acls;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertTrue;
+
+import java.util.List;
 
 import org.jmock.Expectations;
 import org.jmock.Mockery;
@@ -10,7 +12,6 @@ import org.junit.Test;
 import org.springframework.security.Authentication;
 import org.springframework.security.acls.objectidentity.ObjectIdentity;
 import org.springframework.security.acls.objectidentity.ObjectIdentityRetrievalStrategy;
-import org.springframework.security.acls.sid.Sid;
 import org.springframework.security.acls.sid.SidRetrievalStrategy;
 
 /**
@@ -45,9 +46,9 @@ public class AclPermissionEvaluatorTests {
             ignoring(user);
             ignoring(oidStrategy);
             ignoring(sidStrategy);
-            oneOf(service).readAclById(with(any(ObjectIdentity.class)), with(any(Sid[].class)));
+            oneOf(service).readAclById(with(any(ObjectIdentity.class)), with(any(List.class)));
                 will(returnValue(acl));
-            oneOf(acl).isGranted(with(any(Permission[].class)), with(any(Sid[].class)), with(equal(false)));
+            oneOf(acl).isGranted(with(any(List.class)), with(any(List.class)), with(equal(false)));
                 will(returnValue(true));
         }});
 

+ 86 - 90
acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java

@@ -3,6 +3,8 @@ package org.springframework.security.acls.domain;
 import static org.junit.Assert.*;
 
 import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -41,6 +43,13 @@ import org.springframework.security.util.FieldUtils;
  * @author Andrei Stefan
  */
 public class AclImplTests {
+    private static final List<Permission> READ = Arrays.asList(BasePermission.READ );
+    private static final List<Permission> WRITE = Arrays.asList(BasePermission.WRITE);
+    private static final List<Permission> CREATE = Arrays.asList(BasePermission.CREATE );
+    private static final List<Permission> DELETE = Arrays.asList(BasePermission.DELETE );
+    private static final List<Sid> SCOTT = Arrays.asList((Sid)new PrincipalSid("scott"));
+    private static final List<Sid> BEN = Arrays.asList((Sid)new PrincipalSid("ben"));
+
     Authentication auth = new TestingAuthenticationToken("johndoe", "ignored", "ROLE_ADMINISTRATOR");
     Mockery jmockCtx = new Mockery();
     AclAuthorizationStrategy mockAuthzStrategy;
@@ -137,31 +146,31 @@ public class AclImplTests {
         acl.insertAce(0, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST1"), true);
         service.updateAcl(acl);
         // Check it was successfully added
-        assertEquals(1, acl.getEntries().length);
-        assertEquals(acl.getEntries()[0].getAcl(), acl);
-        assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ);
-        assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST1"));
+        assertEquals(1, acl.getEntries().size());
+        assertEquals(acl.getEntries().get(0).getAcl(), acl);
+        assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST1"));
 
         // Add a second permission
         acl.insertAce(1, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST2"), true);
         service.updateAcl(acl);
         // Check it was added on the last position
-        assertEquals(2, acl.getEntries().length);
-        assertEquals(acl.getEntries()[1].getAcl(), acl);
-        assertEquals(acl.getEntries()[1].getPermission(), BasePermission.READ);
-        assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
+        assertEquals(2, acl.getEntries().size());
+        assertEquals(acl.getEntries().get(1).getAcl(), acl);
+        assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
 
         // Add a third permission, after the first one
         acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_TEST3"), false);
         service.updateAcl(acl);
-        assertEquals(3, acl.getEntries().length);
+        assertEquals(3, acl.getEntries().size());
         // Check the third entry was added between the two existent ones
-        assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ);
-        assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST1"));
-        assertEquals(acl.getEntries()[1].getPermission(), BasePermission.WRITE);
-        assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST3"));
-        assertEquals(acl.getEntries()[2].getPermission(), BasePermission.READ);
-        assertEquals(acl.getEntries()[2].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
+        assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST1"));
+        assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.WRITE);
+        assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST3"));
+        assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(2).getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
     }
 
     @Test(expected=NotFoundException.class)
@@ -191,22 +200,22 @@ public class AclImplTests {
 
         // Delete first permission and check the order of the remaining permissions is kept
         acl.deleteAce(0);
-        assertEquals(2, acl.getEntries().length);
-        assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
-        assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST3"));
+        assertEquals(2, acl.getEntries().size());
+        assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
+        assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST3"));
 
         // Add one more permission and remove the permission in the middle
         acl.insertAce(2, BasePermission.READ, new GrantedAuthoritySid("ROLE_TEST4"), true);
         service.updateAcl(acl);
         acl.deleteAce(1);
-        assertEquals(2, acl.getEntries().length);
-        assertEquals(acl.getEntries()[0].getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
-        assertEquals(acl.getEntries()[1].getSid(), new GrantedAuthoritySid("ROLE_TEST4"));
+        assertEquals(2, acl.getEntries().size());
+        assertEquals(acl.getEntries().get(0).getSid(), new GrantedAuthoritySid("ROLE_TEST2"));
+        assertEquals(acl.getEntries().get(1).getSid(), new GrantedAuthoritySid("ROLE_TEST4"));
 
         // Remove remaining permissions
         acl.deleteAce(1);
         acl.deleteAce(0);
-        assertEquals(0, acl.getEntries().length);
+        assertEquals(0, acl.getEntries().size());
     }
 
     @Test
@@ -229,14 +238,15 @@ public class AclImplTests {
     public void testIsGrantingRejectsEmptyParameters() throws Exception {
         MutableAcl acl = new AclImpl(objectIdentity, new Long(1), mockAuthzStrategy, mockAuditLogger, null, null, true, new PrincipalSid(
                 "johndoe"));
+        Sid ben = new PrincipalSid("ben");
         try {
-            acl.isGranted(new Permission[] {}, new Sid[] { new PrincipalSid("ben") }, false);
+            acl.isGranted(new ArrayList<Permission>(0), Arrays.asList(ben) , false);
             fail("It should have thrown IllegalArgumentException");
         }
         catch (IllegalArgumentException expected) {
         }
         try {
-            acl.isGranted(new Permission[] { BasePermission.READ }, new Sid[] {}, false);
+            acl.isGranted(READ, new ArrayList<Sid>(0), false);
             fail("It should have thrown IllegalArgumentException");
         }
         catch (IllegalArgumentException expected) {
@@ -261,25 +271,22 @@ public class AclImplTests {
         rootAcl.insertAce(3, BasePermission.WRITE, new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), true);
 
         // Check permissions granting
-        Permission[] permissions = new Permission[] { BasePermission.READ, BasePermission.CREATE };
-        Sid[] sids = new Sid[] { new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_GUEST") };
+        List<Permission> permissions = Arrays.asList(BasePermission.READ, BasePermission.CREATE);
+        List<Sid> sids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_GUEST"));
         assertFalse(rootAcl.isGranted(permissions, sids, false));
         try {
-            rootAcl.isGranted(permissions, new Sid[] { new PrincipalSid("scott") }, false);
+            rootAcl.isGranted(permissions, SCOTT, false);
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
         }
-        assertTrue(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("scott") },
-                false));
-        assertFalse(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("rod"),
-                new GrantedAuthoritySid("WRITE_ACCESS_ROLE") }, false));
-        assertTrue(rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] {
-                new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), new PrincipalSid("rod") }, false));
+        assertTrue(rootAcl.isGranted(WRITE, SCOTT, false));
+        assertFalse(rootAcl.isGranted(WRITE,
+                Arrays.asList(new PrincipalSid("rod"), new GrantedAuthoritySid("WRITE_ACCESS_ROLE")), false));
+        assertTrue(rootAcl.isGranted(WRITE, Arrays.asList(new GrantedAuthoritySid("WRITE_ACCESS_ROLE"), new PrincipalSid("rod")), false));
         try {
             // Change the type of the Sid and check the granting process
-            rootAcl.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new GrantedAuthoritySid("rod"),
-                    new PrincipalSid("WRITE_ACCESS_ROLE") }, false);
+            rootAcl.isGranted(WRITE, Arrays.asList(new GrantedAuthoritySid("rod"), new PrincipalSid("WRITE_ACCESS_ROLE")), false);
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
@@ -326,45 +333,33 @@ public class AclImplTests {
         childAcl1.insertAce(0, BasePermission.CREATE, new PrincipalSid("scott"), true);
 
         // Check granting process for parent1
-        assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new PrincipalSid("scott") },
-                false));
-        assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new GrantedAuthoritySid(
-                "ROLE_USER_READ") }, false));
-        assertTrue(parentAcl1.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("ben") },
-                false));
-        assertFalse(parentAcl1.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") },
-                false));
-        assertFalse(parentAcl1.isGranted(new Permission[] { BasePermission.DELETE },
-                new Sid[] { new PrincipalSid("scott") }, false));
+        assertTrue(parentAcl1.isGranted(READ, SCOTT, false));
+        assertTrue(parentAcl1.isGranted(READ, Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_USER_READ")), false));
+        assertTrue(parentAcl1.isGranted(WRITE, BEN, false));
+        assertFalse(parentAcl1.isGranted(DELETE, BEN, false));
+        assertFalse(parentAcl1.isGranted(DELETE, SCOTT, false));
 
         // Check granting process for parent2
-        assertTrue(parentAcl2.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid("ben") },
-                false));
-        assertTrue(parentAcl2.isGranted(new Permission[] { BasePermission.WRITE }, new Sid[] { new PrincipalSid("ben") },
-                false));
-        assertFalse(parentAcl2.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") },
-                false));
+        assertTrue(parentAcl2.isGranted(CREATE, BEN, false));
+        assertTrue(parentAcl2.isGranted(WRITE, BEN, false));
+        assertFalse(parentAcl2.isGranted(DELETE, BEN, false));
 
         // Check granting process for child1
-        assertTrue(childAcl1.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid("scott") },
-                false));
-        assertTrue(childAcl1.isGranted(new Permission[] { BasePermission.READ }, new Sid[] { new GrantedAuthoritySid(
-                "ROLE_USER_READ") }, false));
-        assertFalse(childAcl1.isGranted(new Permission[] { BasePermission.DELETE }, new Sid[] { new PrincipalSid("ben") },
+        assertTrue(childAcl1.isGranted(CREATE, SCOTT,
                 false));
+        assertTrue(childAcl1.isGranted(READ, Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_USER_READ")), false));
+        assertFalse(childAcl1.isGranted(DELETE, BEN, false));
 
         // Check granting process for child2 (doesn't inherit the permissions from its parent)
         try {
-            assertTrue(childAcl2.isGranted(new Permission[] { BasePermission.CREATE },
-                    new Sid[] { new PrincipalSid("scott") }, false));
+            assertTrue(childAcl2.isGranted(CREATE, SCOTT, false));
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
             assertTrue(true);
         }
         try {
-            assertTrue(childAcl2.isGranted(new Permission[] { BasePermission.CREATE }, new Sid[] { new PrincipalSid(
-                    "johndoe") }, false));
+            assertTrue(childAcl2.isGranted(CREATE, Arrays.asList((Sid)new PrincipalSid("johndoe")), false));
             fail("It should have thrown NotFoundException");
         }
         catch (NotFoundException expected) {
@@ -386,9 +381,9 @@ public class AclImplTests {
         acl.insertAce(2, BasePermission.CREATE, new PrincipalSid("ben"), true);
         service.updateAcl(acl);
 
-        assertEquals(acl.getEntries()[0].getPermission(), BasePermission.READ);
-        assertEquals(acl.getEntries()[1].getPermission(), BasePermission.WRITE);
-        assertEquals(acl.getEntries()[2].getPermission(), BasePermission.CREATE);
+        assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.WRITE);
+        assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.CREATE);
 
         // Change each permission
         acl.updateAce(0, BasePermission.CREATE);
@@ -396,9 +391,9 @@ public class AclImplTests {
         acl.updateAce(2, BasePermission.READ);
 
         // Check the change was successfuly made
-        assertEquals(acl.getEntries()[0].getPermission(), BasePermission.CREATE);
-        assertEquals(acl.getEntries()[1].getPermission(), BasePermission.DELETE);
-        assertEquals(acl.getEntries()[2].getPermission(), BasePermission.READ);
+        assertEquals(acl.getEntries().get(0).getPermission(), BasePermission.CREATE);
+        assertEquals(acl.getEntries().get(1).getPermission(), BasePermission.DELETE);
+        assertEquals(acl.getEntries().get(2).getPermission(), BasePermission.READ);
     }
 
     @Test
@@ -414,20 +409,20 @@ public class AclImplTests {
         acl.insertAce(1, BasePermission.WRITE, new GrantedAuthoritySid("ROLE_USER_READ"), true);
         service.updateAcl(acl);
 
-        assertFalse(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure());
-        assertFalse(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditFailure());
-        assertFalse(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditSuccess());
-        assertFalse(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditSuccess());
+        assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditFailure());
+        assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditFailure());
+        assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditSuccess());
+        assertFalse(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditSuccess());
 
         // Change each permission
         ((AuditableAcl) acl).updateAuditing(0, true, true);
         ((AuditableAcl) acl).updateAuditing(1, true, true);
 
         // Check the change was successfuly made
-        assertTrue(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditFailure());
-        assertTrue(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditFailure());
-        assertTrue(((AuditableAccessControlEntry) acl.getEntries()[0]).isAuditSuccess());
-        assertTrue(((AuditableAccessControlEntry) acl.getEntries()[1]).isAuditSuccess());
+        assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditFailure());
+        assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditFailure());
+        assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(0)).isAuditSuccess());
+        assertTrue(((AuditableAccessControlEntry) acl.getEntries().get(1)).isAuditSuccess());
     }
 
     @Test
@@ -452,7 +447,7 @@ public class AclImplTests {
         assertEquals(acl.getOwner(), new PrincipalSid("johndoe"));
         assertNull(acl.getParentAcl());
         assertTrue(acl.isEntriesInheriting());
-        assertEquals(2, acl.getEntries().length);
+        assertEquals(2, acl.getEntries().size());
 
         acl.setParent(parentAcl);
         assertEquals(acl.getParentAcl(), parentAcl);
@@ -466,19 +461,19 @@ public class AclImplTests {
 
     @Test
     public void testIsSidLoaded() throws Exception {
-        Sid[] loadedSids = new Sid[] { new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED") };
+        List<Sid> loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED"));
         MutableAcl acl = new AclImpl(objectIdentity, new Long(1), mockAuthzStrategy, mockAuditLogger, null, loadedSids, true, new PrincipalSid(
                 "johndoe"));
 
         assertTrue(acl.isSidLoaded(loadedSids));
-        assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new PrincipalSid("ben") }));
-        assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED")}));
-        assertTrue(acl.isSidLoaded(new Sid[] { new PrincipalSid("ben") }));
+        assertTrue(acl.isSidLoaded(Arrays.asList(new GrantedAuthoritySid("ROLE_IGNORED"), new PrincipalSid("ben"))));
+        assertTrue(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED"))));
+        assertTrue(acl.isSidLoaded(BEN));
         assertTrue(acl.isSidLoaded(null));
-        assertTrue(acl.isSidLoaded(new Sid[] { }));
-        assertTrue(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_IGNORED") }));
-        assertFalse(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_GENERAL"), new GrantedAuthoritySid("ROLE_IGNORED") }));
-        assertFalse(acl.isSidLoaded(new Sid[] { new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_GENERAL") }));
+        assertTrue(acl.isSidLoaded(new ArrayList<Sid>(0)));
+        assertTrue(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_IGNORED"))));
+        assertFalse(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_GENERAL"), new GrantedAuthoritySid("ROLE_IGNORED"))));
+        assertFalse(acl.isSidLoaded(Arrays.asList((Sid)new GrantedAuthoritySid("ROLE_IGNORED"), new GrantedAuthoritySid("ROLE_GENERAL"))));
     }
 
     //~ Inner Classes ==================================================================================================
@@ -495,8 +490,9 @@ public class AclImplTests {
          * Mock implementation that populates the aces list with fully initialized AccessControlEntries
          * @see org.springframework.security.acls.MutableAclService#updateAcl(org.springframework.security.acls.MutableAcl)
          */
+        @SuppressWarnings("unchecked")
         public MutableAcl updateAcl(MutableAcl acl) throws NotFoundException {
-            AccessControlEntry[] oldAces = acl.getEntries();
+            List<AccessControlEntry> oldAces = acl.getEntries();
             Field acesField = FieldUtils.getField(AclImpl.class, "aces");
             acesField.setAccessible(true);
             List newAces;
@@ -504,8 +500,8 @@ public class AclImplTests {
                 newAces = (List) acesField.get(acl);
                 newAces.clear();
 
-                for (int i = 0; i < oldAces.length; i++) {
-                    AccessControlEntry ac = oldAces[i];
+                for (int i = 0; i < oldAces.size(); i++) {
+                    AccessControlEntry ac = oldAces.get(i);
                     // Just give an ID to all this acl's aces, rest of the fields are just copied
                     newAces.add(new AccessControlEntryImpl(new Long(i + 1), ac.getAcl(), ac.getSid(), ac.getPermission(), ac
                             .isGranting(), ((AuditableAccessControlEntry) ac).isAuditSuccess(),
@@ -519,7 +515,7 @@ public class AclImplTests {
             return acl;
         }
 
-        public ObjectIdentity[] findChildren(ObjectIdentity parentIdentity) {
+        public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
             return null;
         }
 
@@ -527,15 +523,15 @@ public class AclImplTests {
             return null;
         }
 
-        public Acl readAclById(ObjectIdentity object, Sid[] sids) throws NotFoundException {
+        public Acl readAclById(ObjectIdentity object, List<Sid> sids) throws NotFoundException {
             return null;
         }
 
-        public Map readAclsById(ObjectIdentity[] objects) throws NotFoundException {
+        public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects) throws NotFoundException {
             return null;
         }
 
-        public Map readAclsById(ObjectIdentity[] objects, Sid[] sids) throws NotFoundException {
+        public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids) throws NotFoundException {
             return null;
         }
     }

+ 10 - 11
acl/src/test/java/org/springframework/security/acls/jdbc/AclPermissionInheritanceTests.java

@@ -89,12 +89,11 @@ public class AclPermissionInheritanceTests extends TestCase {
 
         parent = (MutableAcl) child.getParentAcl();
 
-        assertEquals("Fails because child has a stale reference to its parent",
-                2, parent.getEntries().length);
-        assertEquals(1, parent.getEntries()[0].getPermission().getMask());
-        assertEquals(new PrincipalSid("john"), parent.getEntries()[0].getSid());
-        assertEquals(1, parent.getEntries()[1].getPermission().getMask());
-        assertEquals(new PrincipalSid("joe"), parent.getEntries()[1].getSid());
+        assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().size());
+        assertEquals(1, parent.getEntries().get(0).getPermission().getMask());
+        assertEquals(new PrincipalSid("john"), parent.getEntries().get(0).getSid());
+        assertEquals(1, parent.getEntries().get(1).getPermission().getMask());
+        assertEquals(new PrincipalSid("joe"), parent.getEntries().get(1).getSid());
 
     }
     public void test2() throws Exception {
@@ -121,11 +120,11 @@ public class AclPermissionInheritanceTests extends TestCase {
 
         parent = (MutableAcl) child.getParentAcl();
 
-        assertEquals(2, parent.getEntries().length);
-        assertEquals(16, parent.getEntries()[0].getPermission().getMask());
-        assertEquals(new GrantedAuthoritySid("ROLE_ADMINISTRATOR"), parent.getEntries()[0].getSid());
-        assertEquals(8, parent.getEntries()[1].getPermission().getMask());
-        assertEquals(new PrincipalSid("terry"), parent.getEntries()[1].getSid());
+        assertEquals(2, parent.getEntries().size());
+        assertEquals(16, parent.getEntries().get(0).getPermission().getMask());
+        assertEquals(new GrantedAuthoritySid("ROLE_ADMINISTRATOR"), parent.getEntries().get(0).getSid());
+        assertEquals(8, parent.getEntries().get(1).getPermission().getMask());
+        assertEquals(new PrincipalSid("terry"), parent.getEntries().get(1).getSid());
 
     }
 

+ 39 - 40
acl/src/test/java/org/springframework/security/acls/jdbc/BasicLookupStrategyTests.java

@@ -1,5 +1,7 @@
 package org.springframework.security.acls.jdbc;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 import junit.framework.Assert;
@@ -42,11 +44,8 @@ public class BasicLookupStrategyTests {
     //~ Instance fields ================================================================================================
 
     private static JdbcTemplate jdbcTemplate;
-
     private LookupStrategy strategy;
-
     private static TestDataSource dataSource;
-
     private static CacheManager cacheManager;
 
     //~ Methods ========================================================================================================
@@ -123,7 +122,7 @@ public class BasicLookupStrategyTests {
         // Deliberately use an integer for the child, to reproduce bug report in SEC-819
         ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(102));
 
-        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null);
+        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null);
         checkEntries(topParentOid, middleParentOid, childOid, map);
     }
 
@@ -134,11 +133,11 @@ public class BasicLookupStrategyTests {
         ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(102));
 
         // Objects were put in cache
-        strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null);
+        strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null);
 
         // Let's empty the database to force acls retrieval from cache
         emptyDatabase();
-        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null);
+        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null);
 
         checkEntries(topParentOid, middleParentOid, childOid, map);
     }
@@ -151,7 +150,7 @@ public class BasicLookupStrategyTests {
 
         // Set a batch size to allow multiple database queries in order to retrieve all acls
         ((BasicLookupStrategy) this.strategy).setBatchSize(1);
-        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(new ObjectIdentity[] { topParentOid, middleParentOid, childOid }, null);
+        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid), null);
         checkEntries(topParentOid, middleParentOid, childOid, map);
     }
 
@@ -174,9 +173,9 @@ public class BasicLookupStrategyTests {
         Assert.assertEquals(middleParentOid, child.getParentAcl().getObjectIdentity());
 
         // Check their ACEs were correctly retrieved
-        Assert.assertEquals(2, topParent.getEntries().length);
-        Assert.assertEquals(1, middleParent.getEntries().length);
-        Assert.assertEquals(1, child.getEntries().length);
+        Assert.assertEquals(2, topParent.getEntries().size());
+        Assert.assertEquals(1, middleParent.getEntries().size());
+        Assert.assertEquals(1, child.getEntries().size());
 
         // Check object identities were correctly retrieved
         Assert.assertEquals(topParentOid, topParent.getObjectIdentity());
@@ -187,39 +186,39 @@ public class BasicLookupStrategyTests {
         Assert.assertTrue(topParent.isEntriesInheriting());
         Assert.assertEquals(topParent.getId(), new Long(1));
         Assert.assertEquals(topParent.getOwner(), new PrincipalSid("ben"));
-        Assert.assertEquals(topParent.getEntries()[0].getId(), new Long(1));
-        Assert.assertEquals(topParent.getEntries()[0].getPermission(), BasePermission.READ);
-        Assert.assertEquals(topParent.getEntries()[0].getSid(), new PrincipalSid("ben"));
-        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[0]).isAuditFailure());
-        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[0]).isAuditSuccess());
-        Assert.assertTrue(((AuditableAccessControlEntry) topParent.getEntries()[0]).isGranting());
-
-        Assert.assertEquals(topParent.getEntries()[1].getId(), new Long(2));
-        Assert.assertEquals(topParent.getEntries()[1].getPermission(), BasePermission.WRITE);
-        Assert.assertEquals(topParent.getEntries()[1].getSid(), new PrincipalSid("ben"));
-        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isAuditFailure());
-        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isAuditSuccess());
-        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries()[1]).isGranting());
+        Assert.assertEquals(topParent.getEntries().get(0).getId(), new Long(1));
+        Assert.assertEquals(topParent.getEntries().get(0).getPermission(), BasePermission.READ);
+        Assert.assertEquals(topParent.getEntries().get(0).getSid(), new PrincipalSid("ben"));
+        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isAuditFailure());
+        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isAuditSuccess());
+        Assert.assertTrue(((AuditableAccessControlEntry) topParent.getEntries().get(0)).isGranting());
+
+        Assert.assertEquals(topParent.getEntries().get(1).getId(), new Long(2));
+        Assert.assertEquals(topParent.getEntries().get(1).getPermission(), BasePermission.WRITE);
+        Assert.assertEquals(topParent.getEntries().get(1).getSid(), new PrincipalSid("ben"));
+        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isAuditFailure());
+        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isAuditSuccess());
+        Assert.assertFalse(((AuditableAccessControlEntry) topParent.getEntries().get(1)).isGranting());
 
         Assert.assertTrue(middleParent.isEntriesInheriting());
         Assert.assertEquals(middleParent.getId(), new Long(2));
         Assert.assertEquals(middleParent.getOwner(), new PrincipalSid("ben"));
-        Assert.assertEquals(middleParent.getEntries()[0].getId(), new Long(3));
-        Assert.assertEquals(middleParent.getEntries()[0].getPermission(), BasePermission.DELETE);
-        Assert.assertEquals(middleParent.getEntries()[0].getSid(), new PrincipalSid("ben"));
-        Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isAuditFailure());
-        Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isAuditSuccess());
-        Assert.assertTrue(((AuditableAccessControlEntry) middleParent.getEntries()[0]).isGranting());
+        Assert.assertEquals(middleParent.getEntries().get(0).getId(), new Long(3));
+        Assert.assertEquals(middleParent.getEntries().get(0).getPermission(), BasePermission.DELETE);
+        Assert.assertEquals(middleParent.getEntries().get(0).getSid(), new PrincipalSid("ben"));
+        Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isAuditFailure());
+        Assert.assertFalse(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isAuditSuccess());
+        Assert.assertTrue(((AuditableAccessControlEntry) middleParent.getEntries().get(0)).isGranting());
 
         Assert.assertTrue(child.isEntriesInheriting());
         Assert.assertEquals(child.getId(), new Long(3));
         Assert.assertEquals(child.getOwner(), new PrincipalSid("ben"));
-        Assert.assertEquals(child.getEntries()[0].getId(), new Long(4));
-        Assert.assertEquals(child.getEntries()[0].getPermission(), BasePermission.DELETE);
-        Assert.assertEquals(child.getEntries()[0].getSid(), new PrincipalSid("ben"));
-        Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries()[0]).isAuditFailure());
-        Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries()[0]).isAuditSuccess());
-        Assert.assertFalse((child.getEntries()[0]).isGranting());
+        Assert.assertEquals(child.getEntries().get(0).getId(), new Long(4));
+        Assert.assertEquals(child.getEntries().get(0).getPermission(), BasePermission.DELETE);
+        Assert.assertEquals(child.getEntries().get(0).getSid(), new PrincipalSid("ben"));
+        Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries().get(0)).isAuditFailure());
+        Assert.assertFalse(((AuditableAccessControlEntry) child.getEntries().get(0)).isAuditSuccess());
+        Assert.assertFalse((child.getEntries().get(0)).isGranting());
     }
 
     @Test
@@ -233,7 +232,7 @@ public class BasicLookupStrategyTests {
         ObjectIdentity middleParent2Oid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(103));
 
         // Retrieve the child
-        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(new ObjectIdentity[] { childOid }, null);
+        Map<ObjectIdentity, Acl> map = this.strategy.readAclsById(Arrays.asList(childOid), null);
 
         // Check that the child and all its parents were retrieved
         Assert.assertNotNull(map.get(childOid));
@@ -265,9 +264,9 @@ public class BasicLookupStrategyTests {
         ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(107));
 
         // First lookup only child, thus populating the cache with grandParent, parent1 and child
-        Permission[] checkPermission = new Permission[] { BasePermission.READ };
-        Sid[] sids = new Sid[] { new PrincipalSid("ben") };
-        ObjectIdentity[] childOids = new ObjectIdentity[] { childOid };
+        List<Permission> checkPermission = Arrays.asList(BasePermission.READ);
+        List<Sid> sids = Arrays.asList((Sid)new PrincipalSid("ben"));
+        List<ObjectIdentity> childOids = Arrays.asList(childOid);
 
         ((BasicLookupStrategy) this.strategy).setBatchSize(6);
         Map<ObjectIdentity, Acl> foundAcls = strategy.readAclsById(childOids, sids);
@@ -278,7 +277,7 @@ public class BasicLookupStrategyTests {
 
         // Search for object identities has to be done in the following order: last element have to be one which
         // is already in cache and the element before it must not be stored in cache
-        ObjectIdentity[] allOids = new ObjectIdentity[] { grandParentOid, parent1Oid, parent2Oid, childOid };
+        List<ObjectIdentity> allOids = Arrays.asList(grandParentOid, parent1Oid, parent2Oid, childOid);
         try {
             foundAcls = strategy.readAclsById(allOids, sids);
             Assert.assertTrue(true);

+ 59 - 52
acl/src/test/java/org/springframework/security/acls/jdbc/JdbcAclServiceTests.java

@@ -14,6 +14,8 @@
  */
 package org.springframework.security.acls.jdbc;
 
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
 
 import org.springframework.security.Authentication;
@@ -35,7 +37,6 @@ import org.springframework.security.context.SecurityContextHolder;
 import org.springframework.security.providers.TestingAuthenticationToken;
 import org.springframework.test.AbstractTransactionalDataSourceSpringContextTests;
 
-
 /**
  * Integration tests the ACL system using an in-memory database.
  *
@@ -45,23 +46,23 @@ import org.springframework.test.AbstractTransactionalDataSourceSpringContextTest
  */
 public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringContextTests {
     //~ Constant fields ================================================================================================
-    
+
     public static final String SELECT_ALL_CLASSES = "SELECT * FROM acl_class WHERE class = ?";
-    
+
     public static final String SELECT_ALL_OBJECT_IDENTITIES = "SELECT * FROM acl_object_identity";
-    
+
     public static final String SELECT_OBJECT_IDENTITY = "SELECT * FROM acl_object_identity WHERE object_id_identity = ?";
-    
+
     public static final String SELECT_ACL_ENTRY = "SELECT * FROM acl_entry, acl_object_identity WHERE " +
             "acl_object_identity.id = acl_entry.acl_object_identity " +
             "AND acl_object_identity.object_id_identity <= ?";
 
     //~ Instance fields ================================================================================================
-    
+
     private JdbcMutableAclService jdbcMutableAclService;
-    
+
     private AclCache aclCache;
-    
+
     private LookupStrategy lookupStrategy;
 
     //~ Methods ========================================================================================================
@@ -119,7 +120,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         jdbcMutableAclService.updateAcl(child);
 
         // Let's check if we can read them back correctly
-        Map map = jdbcMutableAclService.readAclsById(new ObjectIdentity[] {topParentOid, middleParentOid, childOid});
+        Map map = jdbcMutableAclService.readAclsById(Arrays.asList(topParentOid, middleParentOid, childOid));
         assertEquals(3, map.size());
 
         // Replace our current objects with their retrieved versions
@@ -138,27 +139,33 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         assertEquals(middleParentOid, child.getParentAcl().getObjectIdentity());
 
         // Check their ACEs were correctly persisted
-        assertEquals(2, topParent.getEntries().length);
-        assertEquals(1, middleParent.getEntries().length);
-        assertEquals(1, child.getEntries().length);
+        assertEquals(2, topParent.getEntries().size());
+        assertEquals(1, middleParent.getEntries().size());
+        assertEquals(1, child.getEntries().size());
 
         // Check the retrieved rights are correct
-        assertTrue(topParent.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertFalse(topParent.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertTrue(middleParent.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
+        List<Permission> read = Arrays.asList(BasePermission.READ);
+        List<Permission> write = Arrays.asList(BasePermission.WRITE);
+        List<Permission> delete = Arrays.asList(BasePermission.DELETE);
+        List<Sid> pSid = Arrays.asList((Sid)new PrincipalSid(auth));
+
+
+        assertTrue(topParent.isGranted(read, pSid, false));
+        assertFalse(topParent.isGranted(write, pSid, false));
+        assertTrue(middleParent.isGranted(delete, pSid, false));
+        assertFalse(child.isGranted(delete, pSid, false));
 
         try {
-            child.isGranted(new Permission[] {BasePermission.ADMINISTRATION}, new Sid[] {new PrincipalSid(auth)}, false);
+            child.isGranted(Arrays.asList(BasePermission.ADMINISTRATION), pSid, false);
             fail("Should have thrown NotFoundException");
         } catch (NotFoundException expected) {
             assertTrue(true);
         }
 
         // Now check the inherited rights (when not explicitly overridden) also look OK
-        assertTrue(child.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertFalse(child.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, false));
-        assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
+        assertTrue(child.isGranted(read, pSid, false));
+        assertFalse(child.isGranted(write, pSid, false));
+        assertFalse(child.isGranted(delete, pSid, false));
 
         // Next change the child so it doesn't inherit permissions from above
         child.setEntriesInheriting(false);
@@ -167,17 +174,17 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         assertFalse(child.isEntriesInheriting());
 
         // Check the child permissions no longer inherit
-        assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, true));
+        assertFalse(child.isGranted(delete, pSid, true));
 
         try {
-            child.isGranted(new Permission[] {BasePermission.READ}, new Sid[] {new PrincipalSid(auth)}, true);
+            child.isGranted(read, pSid, true);
             fail("Should have thrown NotFoundException");
         } catch (NotFoundException expected) {
             assertTrue(true);
         }
 
         try {
-            child.isGranted(new Permission[] {BasePermission.WRITE}, new Sid[] {new PrincipalSid(auth)}, true);
+            child.isGranted(write, pSid, true);
             fail("Should have thrown NotFoundException");
         } catch (NotFoundException expected) {
             assertTrue(true);
@@ -192,19 +199,19 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         // Save the changed child
         jdbcMutableAclService.updateAcl(child);
         child = (MutableAcl) jdbcMutableAclService.readAclById(childOid);
-        assertEquals(3, child.getEntries().length);
+        assertEquals(3, child.getEntries().size());
 
         // Output permissions
-        for (int i = 0; i < child.getEntries().length; i++) {
-            System.out.println(child.getEntries()[i]);
+        for (int i = 0; i < child.getEntries().size(); i++) {
+            System.out.println(child.getEntries().get(i));
         }
 
         // Check the permissions are as they should be
-        assertFalse(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, true)); // as earlier permission overrode
-        assertTrue(child.isGranted(new Permission[] {BasePermission.CREATE}, new Sid[] {new PrincipalSid(auth)}, true));
+        assertFalse(child.isGranted(delete, pSid, true)); // as earlier permission overrode
+        assertTrue(child.isGranted(Arrays.asList(BasePermission.CREATE), pSid, true));
 
         // Now check the first ACE (index 0) really is DELETE for our Sid and is non-granting
-        AccessControlEntry entry = child.getEntries()[0];
+        AccessControlEntry entry = child.getEntries().get(0);
         assertEquals(BasePermission.DELETE.getMask(), entry.getPermission().getMask());
         assertEquals(new PrincipalSid(auth), entry.getSid());
         assertFalse(entry.isGranting());
@@ -215,12 +222,12 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
 
         // Save and check it worked
         child = jdbcMutableAclService.updateAcl(child);
-        assertEquals(2, child.getEntries().length);
-        assertTrue(child.isGranted(new Permission[] {BasePermission.DELETE}, new Sid[] {new PrincipalSid(auth)}, false));
+        assertEquals(2, child.getEntries().size());
+        assertTrue(child.isGranted(delete, pSid, false));
 
         SecurityContextHolder.clearContext();
     }
-    
+
     /**
      * Test method that demonstrates eviction failure from cache - SEC-676
      */
@@ -232,10 +239,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         // Check the childOid really is a child of middleParentOid
         Acl childAcl = jdbcMutableAclService.readAclById(childOid);
         assertEquals(middleParentOid, childAcl.getParentAcl().getObjectIdentity());
-        
+
         // Delete the mid-parent and test if the child was deleted, as well
         jdbcMutableAclService.deleteAcl(middleParentOid, true);
-        
+
         try {
             Acl acl = jdbcMutableAclService.readAclById(middleParentOid);
             fail("It should have thrown NotFoundException");
@@ -250,12 +257,12 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         catch (NotFoundException expected) {
             assertTrue(true);
         }
-        
+
         Acl acl = jdbcMutableAclService.readAclById(topParentOid);
         assertNotNull(acl);
         assertEquals(((MutableAcl) acl).getObjectIdentity(), topParentOid);
     }
-    
+
     public void testConstructorRejectsNullParameters() throws Exception {
         try {
             JdbcAclService service = new JdbcMutableAclService(null, lookupStrategy, aclCache);
@@ -264,7 +271,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         catch (IllegalArgumentException expected) {
             assertTrue(true);
         }
-        
+
         try {
             JdbcAclService service = new JdbcMutableAclService(this.getJdbcTemplate().getDataSource(), null, aclCache);
             fail("It should have thrown IllegalArgumentException");
@@ -272,7 +279,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         catch (IllegalArgumentException expected) {
             assertTrue(true);
         }
-        
+
         try {
             JdbcAclService service = new JdbcMutableAclService(this.getJdbcTemplate().getDataSource(), lookupStrategy, null);
             fail("It should have thrown IllegalArgumentException");
@@ -281,7 +288,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
             assertTrue(true);
         }
     }
-    
+
     public void testCreateAclRejectsNullParameter() throws Exception {
         try {
             jdbcMutableAclService.createAcl(null);
@@ -291,10 +298,10 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
             assertTrue(true);
         }
     }
-    
+
     public void testCreateAclForADuplicateDomainObject() throws Exception {
         ObjectIdentity duplicateOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100));
-        
+
         // Try to add the same object second time
         try {
             jdbcMutableAclService.createAcl(duplicateOid);
@@ -304,7 +311,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
             assertTrue(true);
         }
     }
-    
+
     public void testDeleteAclRejectsNullParameters() throws Exception {
         try {
             jdbcMutableAclService.deleteAcl(null, true);
@@ -314,7 +321,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
             assertTrue(true);
         }
     }
-    
+
     public void testDeleteAclWithChildrenThrowsException() throws Exception {
         try {
             ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100));
@@ -328,7 +335,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
             jdbcMutableAclService.setForeignKeysInDatabase(true); // restore to the default
         }
     }
-    
+
     public void testDeleteAclRemovesRowsFromDatabase() throws Exception {
         Authentication auth = new TestingAuthenticationToken("ben", "ignored",
                 new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ADMINISTRATOR")});
@@ -338,19 +345,19 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         ObjectIdentity topParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(100));
         ObjectIdentity middleParentOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Long(101));
         ObjectIdentity childOid = new ObjectIdentityImpl("org.springframework.security.TargetObject", new Integer(102));
-        
+
         // Remove the child and check all related database rows were removed accordingly
         jdbcMutableAclService.deleteAcl(childOid, false);
         assertEquals(1, getJdbcTemplate().queryForList(SELECT_ALL_CLASSES, new Object[] {"org.springframework.security.TargetObject"} ).size());
         assertEquals(0, getJdbcTemplate().queryForList(SELECT_OBJECT_IDENTITY, new Object[] {new Long(102)}).size());
         assertEquals(2, getJdbcTemplate().queryForList(SELECT_ALL_OBJECT_IDENTITIES).size());
         assertEquals(3, getJdbcTemplate().queryForList(SELECT_ACL_ENTRY, new Object[] {new Long(103)} ).size());
-        
+
         // Check the cache
         assertNull(aclCache.getFromCache(childOid));
         assertNull(aclCache.getFromCache(new Long(102)));
     }
-    
+
     /**
      * SEC-655
      */
@@ -365,7 +372,7 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
 
         MutableAcl parent = jdbcMutableAclService.createAcl(parentOid);
         MutableAcl child = jdbcMutableAclService.createAcl(childOid);
-        
+
         child.setParent(parent);
         jdbcMutableAclService.updateAcl(child);
 
@@ -380,13 +387,13 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
         child = (MutableAcl) jdbcMutableAclService.readAclById(childOid);
         parent = (MutableAcl) child.getParentAcl();
 
-        assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().length);
+        assertEquals("Fails because child has a stale reference to its parent", 2, parent.getEntries().size());
         assertEquals(1, parent.getEntries()[0].getPermission().getMask());
         assertEquals(new PrincipalSid("ben"), parent.getEntries()[0].getSid());
         assertEquals(1, parent.getEntries()[1].getPermission().getMask());
         assertEquals(new PrincipalSid("scott"), parent.getEntries()[1].getSid());
     }*/
-    
+
 /*    public void testCumulativePermissions() {
    setComplete();
    Authentication auth = new TestingAuthenticationToken("ben", "ignored", new GrantedAuthority[] {new GrantedAuthorityImpl("ROLE_ADMINISTRATOR")});
@@ -400,14 +407,14 @@ public class JdbcAclServiceTests extends AbstractTransactionalDataSourceSpringCo
    CumulativePermission cm = new CumulativePermission().set(BasePermission.READ).set(BasePermission.ADMINISTRATION);
    assertEquals(17, cm.getMask());
        topParent.insertAce(null, cm, new PrincipalSid(auth), true);
-       assertEquals(1, topParent.getEntries().length);
+       assertEquals(1, topParent.getEntries().size());
 
        // Explictly save the changed ACL
        topParent = jdbcMutableAclService.updateAcl(topParent);
 
        // Check the mask was retrieved correctly
        assertEquals(17, topParent.getEntries()[0].getPermission().getMask());
-       assertTrue(topParent.isGranted(new Permission[] {cm}, new Sid[] {new PrincipalSid(auth)}, true));
+       assertTrue(topParent.isGranted(new Permission[] {cm}, pSid, true));
 
        SecurityContextHolder.clearContext();
    }

+ 12 - 10
acl/src/test/java/org/springframework/security/acls/sid/SidRetrievalStrategyTests.java

@@ -1,5 +1,7 @@
 package org.springframework.security.acls.sid;
 
+import java.util.List;
+
 import junit.framework.Assert;
 import junit.framework.TestCase;
 
@@ -18,20 +20,20 @@ public class SidRetrievalStrategyTests extends TestCase {
     public void testSidsRetrieval() throws Exception {
         Authentication authentication = new TestingAuthenticationToken("scott", "password", "ROLE_1", "ROLE_2", "ROLE_3");
         SidRetrievalStrategy retrStrategy = new SidRetrievalStrategyImpl();
-        Sid[] sids = retrStrategy.getSids(authentication);
+        List<Sid> sids = retrStrategy.getSids(authentication);
 
         assertNotNull(sids);
-        assertEquals(4, sids.length);
-        assertNotNull(sids[0]);
-        assertTrue(sids[0] instanceof PrincipalSid);
+        assertEquals(4, sids.size());
+        assertNotNull(sids.get(0));
+        assertTrue(sids.get(0) instanceof PrincipalSid);
 
-        for (int i = 1; i < sids.length; i++) {
-            assertTrue(sids[i] instanceof GrantedAuthoritySid);
+        for (int i = 1; i < sids.size(); i++) {
+            assertTrue(sids.get(i) instanceof GrantedAuthoritySid);
         }
 
-        Assert.assertEquals("scott", ((PrincipalSid) sids[0]).getPrincipal());
-        Assert.assertEquals("ROLE_1", ((GrantedAuthoritySid) sids[1]).getGrantedAuthority());
-        Assert.assertEquals("ROLE_2", ((GrantedAuthoritySid) sids[2]).getGrantedAuthority());
-        Assert.assertEquals("ROLE_3", ((GrantedAuthoritySid) sids[3]).getGrantedAuthority());
+        Assert.assertEquals("scott", ((PrincipalSid) sids.get(0)).getPrincipal());
+        Assert.assertEquals("ROLE_1", ((GrantedAuthoritySid) sids.get(1)).getGrantedAuthority());
+        Assert.assertEquals("ROLE_2", ((GrantedAuthoritySid) sids.get(2)).getGrantedAuthority());
+        Assert.assertEquals("ROLE_3", ((GrantedAuthoritySid) sids.get(3)).getGrantedAuthority());
     }
 }