Polish spring-security-acl main code

Manually polish `spring-security-acl` following the formatting
and checkstyle fixes.

Issue gh-8945

acl/src/main/java/org/springframework/security/acls/AclEntryVoter.java

@@ -41,6 +41,7 @@ import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.SidRetrievalStrategy;
 import org.springframework.security.core.Authentication;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -100,7 +101,11 @@ public class AclEntryVoter extends AbstractAclVoter {
 
 	private static final Log logger = LogFactory.getLog(AclEntryVoter.class);
 
-	private AclService aclService;
+	private final AclService aclService;
+
+	private final String processConfigAttribute;
+
+	private final List<Permission> requirePermission;
 
 	private ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
 
@@ -108,18 +113,10 @@ public class AclEntryVoter extends AbstractAclVoter {
 
 	private String internalMethod;
 
-	private String processConfigAttribute;
-
-	private List<Permission> requirePermission;
-
 	public AclEntryVoter(AclService aclService, String processConfigAttribute, Permission[] requirePermission) {
 		Assert.notNull(processConfigAttribute, "A processConfigAttribute is mandatory");
 		Assert.notNull(aclService, "An AclService is mandatory");
-
-		if ((requirePermission == null) || (requirePermission.length == 0)) {
-			throw new IllegalArgumentException("One or more requirePermission entries is mandatory");
-		}
-
+		Assert.isTrue(!ObjectUtils.isEmpty(requirePermission), "One or more requirePermission entries is mandatory");
 		this.aclService = aclService;
 		this.processConfigAttribute = processConfigAttribute;
 		this.requirePermission = Arrays.asList(requirePermission);
@@ -164,48 +161,24 @@ public class AclEntryVoter extends AbstractAclVoter {
 
 	@Override
 	public int vote(Authentication authentication, MethodInvocation object, Collection<ConfigAttribute> attributes) {
-
 		for (ConfigAttribute attr : attributes) {
-
-			if (!this.supports(attr)) {
+			if (!supports(attr)) {
 				continue;
 			}
+
 			// Need to make an access decision on this invocation
 			// Attempt to locate the domain object instance to process
 			Object domainObject = getDomainObjectInstance(object);
 
 			// If domain object is null, vote to abstain
 			if (domainObject == null) {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Voting to abstain - domainObject is null");
-				}
-
+				logger.debug("Voting to abstain - domainObject is null");
 				return ACCESS_ABSTAIN;
 			}
 
 			// Evaluate if we are required to use an inner domain object
 			if (StringUtils.hasText(this.internalMethod)) {
-				try {
-					Class<?> clazz = domainObject.getClass();
-					Method method = clazz.getMethod(this.internalMethod, new Class[0]);
-					domainObject = method.invoke(domainObject);
-				}
-				catch (NoSuchMethodException nsme) {
-					throw new AuthorizationServiceException("Object of class '" + domainObject.getClass()
-							+ "' does not provide the requested internalMethod: " + this.internalMethod);
-				}
-				catch (IllegalAccessException iae) {
-					logger.debug("IllegalAccessException", iae);
-
-					throw new AuthorizationServiceException(
-							"Problem invoking internalMethod: " + this.internalMethod + " for object: " + domainObject);
-				}
-				catch (InvocationTargetException ite) {
-					logger.debug("InvocationTargetException", ite);
-
-					throw new AuthorizationServiceException(
-							"Problem invoking internalMethod: " + this.internalMethod + " for object: " + domainObject);
-				}
+				domainObject = invokeInternalMethod(domainObject);
 			}
 
 			// Obtain the OID applicable to the domain object
@@ -220,36 +193,21 @@ public class AclEntryVoter extends AbstractAclVoter {
 				// Lookup only ACLs for SIDs we're interested in
 				acl = this.aclService.readAclById(objectIdentity, sids);
 			}
-			catch (NotFoundException nfe) {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Voting to deny access - no ACLs apply for this principal");
-				}
-
+			catch (NotFoundException ex) {
+				logger.debug("Voting to deny access - no ACLs apply for this principal");
 				return ACCESS_DENIED;
 			}
 
 			try {
 				if (acl.isGranted(this.requirePermission, sids, false)) {
-					if (logger.isDebugEnabled()) {
-						logger.debug("Voting to grant access");
-					}
-
+					logger.debug("Voting to grant access");
 					return ACCESS_GRANTED;
 				}
-				else {
-					if (logger.isDebugEnabled()) {
-						logger.debug(
-								"Voting to deny access - ACLs returned, but insufficient permissions for this principal");
-					}
-
-					return ACCESS_DENIED;
-				}
+				logger.debug("Voting to deny access - ACLs returned, but insufficient permissions for this principal");
+				return ACCESS_DENIED;
 			}
-			catch (NotFoundException nfe) {
-				if (logger.isDebugEnabled()) {
-					logger.debug("Voting to deny access - no ACLs apply for this principal");
-				}
-
+			catch (NotFoundException ex) {
+				logger.debug("Voting to deny access - no ACLs apply for this principal");
 				return ACCESS_DENIED;
 			}
 		}
@@ -258,4 +216,26 @@ public class AclEntryVoter extends AbstractAclVoter {
 		return ACCESS_ABSTAIN;
 	}
 
+	private Object invokeInternalMethod(Object domainObject) {
+		try {
+			Class<?> domainObjectType = domainObject.getClass();
+			Method method = domainObjectType.getMethod(this.internalMethod, new Class[0]);
+			return method.invoke(domainObject);
+		}
+		catch (NoSuchMethodException ex) {
+			throw new AuthorizationServiceException("Object of class '" + domainObject.getClass()
+					+ "' does not provide the requested internalMethod: " + this.internalMethod);
+		}
+		catch (IllegalAccessException ex) {
+			logger.debug("IllegalAccessException", ex);
+			throw new AuthorizationServiceException(
+					"Problem invoking internalMethod: " + this.internalMethod + " for object: " + domainObject);
+		}
+		catch (InvocationTargetException ex) {
+			logger.debug("InvocationTargetException", ex);
+			throw new AuthorizationServiceException(
+					"Problem invoking internalMethod: " + this.internalMethod + " for object: " + domainObject);
+		}
+	}
+
 }

acl/src/main/java/org/springframework/security/acls/AclPermissionCacheOptimizer.java

@@ -23,6 +23,7 @@ import java.util.List;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.PermissionCacheOptimizer;
 import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl;
 import org.springframework.security.acls.domain.SidRetrievalStrategyImpl;
@@ -58,23 +59,15 @@ public class AclPermissionCacheOptimizer implements PermissionCacheOptimizer {
 		if (objects.isEmpty()) {
 			return;
 		}
-
 		List<ObjectIdentity> oidsToCache = new ArrayList<>(objects.size());
-
 		for (Object domainObject : objects) {
-			if (domainObject == null) {
-				continue;
+			if (domainObject != null) {
+				ObjectIdentity oid = this.oidRetrievalStrategy.getObjectIdentity(domainObject);
+				oidsToCache.add(oid);
 			}
-			ObjectIdentity oid = this.oidRetrievalStrategy.getObjectIdentity(domainObject);
-			oidsToCache.add(oid);
 		}
-
 		List<Sid> sids = this.sidRetrievalStrategy.getSids(authentication);
-
-		if (this.logger.isDebugEnabled()) {
-			this.logger.debug("Eagerly loading Acls for " + oidsToCache.size() + " objects");
-		}
-
+		this.logger.debug(LogMessage.of(() -> "Eagerly loading Acls for " + oidsToCache.size() + " objects"));
 		this.aclService.readAclsById(oidsToCache, sids);
 	}
 

acl/src/main/java/org/springframework/security/acls/AclPermissionEvaluator.java

@@ -24,6 +24,7 @@ import java.util.Locale;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.PermissionEvaluator;
 import org.springframework.security.acls.domain.DefaultPermissionFactory;
 import org.springframework.security.acls.domain.ObjectIdentityRetrievalStrategyImpl;
@@ -76,9 +77,7 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 		if (domainObject == null) {
 			return false;
 		}
-
 		ObjectIdentity objectIdentity = this.objectIdentityRetrievalStrategy.getObjectIdentity(domainObject);
-
 		return checkPermission(authentication, objectIdentity, permission);
 	}
 
@@ -86,7 +85,6 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 	public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType,
 			Object permission) {
 		ObjectIdentity objectIdentity = this.objectIdentityGenerator.createObjectIdentity(targetId, targetType);
-
 		return checkPermission(authentication, objectIdentity, permission);
 	}
 
@@ -94,72 +92,51 @@ public class AclPermissionEvaluator implements PermissionEvaluator {
 		// Obtain the SIDs applicable to the principal
 		List<Sid> sids = this.sidRetrievalStrategy.getSids(authentication);
 		List<Permission> requiredPermission = resolvePermission(permission);
-
-		final boolean debug = this.logger.isDebugEnabled();
-
-		if (debug) {
-			this.logger.debug("Checking permission '" + permission + "' for object '" + oid + "'");
-		}
-
+		this.logger.debug(LogMessage.of(() -> "Checking permission '" + permission + "' for object '" + oid + "'"));
 		try {
 			// Lookup only ACLs for SIDs we're interested in
 			Acl acl = this.aclService.readAclById(oid, sids);
-
 			if (acl.isGranted(requiredPermission, sids, false)) {
-				if (debug) {
-					this.logger.debug("Access is granted");
-				}
-
+				this.logger.debug("Access is granted");
 				return true;
 			}
-
-			if (debug) {
-				this.logger.debug("Returning false - ACLs returned, but insufficient permissions for this principal");
-			}
-
+			this.logger.debug("Returning false - ACLs returned, but insufficient permissions for this principal");
 		}
 		catch (NotFoundException nfe) {
-			if (debug) {
-				this.logger.debug("Returning false - no ACLs apply for this principal");
-			}
+			this.logger.debug("Returning false - no ACLs apply for this principal");
 		}
-
 		return false;
-
 	}
 
 	List<Permission> resolvePermission(Object permission) {
 		if (permission instanceof Integer) {
 			return Arrays.asList(this.permissionFactory.buildFromMask((Integer) permission));
 		}
-
 		if (permission instanceof Permission) {
 			return Arrays.asList((Permission) permission);
 		}
-
 		if (permission instanceof Permission[]) {
 			return Arrays.asList((Permission[]) permission);
 		}
-
 		if (permission instanceof String) {
 			String permString = (String) permission;
-			Permission p;
-
-			try {
-				p = this.permissionFactory.buildFromName(permString);
-			}
-			catch (IllegalArgumentException notfound) {
-				p = this.permissionFactory.buildFromName(permString.toUpperCase(Locale.ENGLISH));
-			}
-
+			Permission p = buildPermission(permString);
 			if (p != null) {
 				return Arrays.asList(p);
 			}
-
 		}
 		throw new IllegalArgumentException("Unsupported permission: " + permission);
 	}
 
+	private Permission buildPermission(String permString) {
+		try {
+			return this.permissionFactory.buildFromName(permString);
+		}
+		catch (IllegalArgumentException notfound) {
+			return this.permissionFactory.buildFromName(permString.toUpperCase(Locale.ENGLISH));
+		}
+	}
+
 	public void setObjectIdentityRetrievalStrategy(ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) {
 		this.objectIdentityRetrievalStrategy = objectIdentityRetrievalStrategy;
 	}

acl/src/main/java/org/springframework/security/acls/afterinvocation/AbstractAclProvider.java

@@ -32,6 +32,7 @@ import org.springframework.security.acls.model.Sid;
 import org.springframework.security.acls.model.SidRetrievalStrategy;
 import org.springframework.security.core.Authentication;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Abstract {@link AfterInvocationProvider} which provides commonly-used ACL-related
@@ -43,25 +44,21 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider {
 
 	protected final AclService aclService;
 
+	protected String processConfigAttribute;
+
 	protected Class<?> processDomainObjectClass = Object.class;
 
 	protected ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
 
 	protected SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
 
-	protected String processConfigAttribute;
-
 	protected final List<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.isEmpty()) {
-			throw new IllegalArgumentException("One or more requirePermission entries is mandatory");
-		}
-
+		Assert.isTrue(!ObjectUtils.isEmpty(requirePermission), "One or more requirePermission entries is mandatory");
 		this.aclService = aclService;
 		this.processConfigAttribute = processConfigAttribute;
 		this.requirePermission = requirePermission;
@@ -81,10 +78,9 @@ public abstract class AbstractAclProvider implements AfterInvocationProvider {
 		try {
 			// Lookup only ACLs for SIDs we're interested in
 			Acl acl = this.aclService.readAclById(objectIdentity, sids);
-
 			return acl.isGranted(this.requirePermission, sids, false);
 		}
-		catch (NotFoundException ignore) {
+		catch (NotFoundException ex) {
 			return false;
 		}
 	}

acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationCollectionFilteringProvider.java

@@ -22,6 +22,7 @@ import java.util.List;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
 import org.springframework.security.access.AccessDeniedException;
 import org.springframework.security.access.AuthorizationServiceException;
 import org.springframework.security.access.ConfigAttribute;
@@ -75,10 +76,8 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
 	@SuppressWarnings("unchecked")
 	public Object decide(Authentication authentication, Object object, Collection<ConfigAttribute> config,
 			Object returnedObject) throws AccessDeniedException {
-
 		if (returnedObject == null) {
 			logger.debug("Return object is null, skipping");
-
 			return null;
 		}
 
@@ -88,18 +87,7 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
 			}
 
 			// Need to process the Collection for this invocation
-			Filterer filterer;
-
-			if (returnedObject instanceof Collection) {
-				filterer = new CollectionFilterer((Collection) returnedObject);
-			}
-			else if (returnedObject.getClass().isArray()) {
-				filterer = new ArrayFilterer((Object[]) returnedObject);
-			}
-			else {
-				throw new AuthorizationServiceException("A Collection or an array (or null) was required as the "
-						+ "returnedObject, but the returnedObject was: " + returnedObject);
-			}
+			Filterer filterer = getFilterer(returnedObject);
 
 			// Locate unauthorised Collection elements
 			for (Object domainObject : filterer) {
@@ -108,20 +96,25 @@ public class AclEntryAfterInvocationCollectionFilteringProvider extends Abstract
 				if (domainObject == null || !getProcessDomainObjectClass().isAssignableFrom(domainObject.getClass())) {
 					continue;
 				}
-
 				if (!hasPermission(authentication, domainObject)) {
 					filterer.remove(domainObject);
-
-					if (logger.isDebugEnabled()) {
-						logger.debug("Principal is NOT authorised for element: " + domainObject);
-					}
+					logger.debug(LogMessage.of(() -> "Principal is NOT authorised for element: " + domainObject));
 				}
 			}
-
 			return filterer.getFilteredObject();
 		}
-
 		return returnedObject;
 	}
 
+	private Filterer getFilterer(Object returnedObject) {
+		if (returnedObject instanceof Collection) {
+			return new CollectionFilterer((Collection) returnedObject);
+		}
+		if (returnedObject.getClass().isArray()) {
+			return new ArrayFilterer((Object[]) returnedObject);
+		}
+		throw new AuthorizationServiceException("A Collection or an array (or null) was required as the "
+				+ "returnedObject, but the returnedObject was: " + returnedObject);
+	}
+
 }

acl/src/main/java/org/springframework/security/acls/afterinvocation/AclEntryAfterInvocationProvider.java

@@ -83,13 +83,11 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme
 			// AclManager interface contract prohibits nulls
 			// As they have permission to null/nothing, grant access
 			logger.debug("Return object is null, skipping");
-
 			return null;
 		}
 
 		if (!getProcessDomainObjectClass().isAssignableFrom(returnedObject.getClass())) {
 			logger.debug("Return object is not applicable for this provider, skipping");
-
 			return returnedObject;
 		}
 
@@ -97,14 +95,13 @@ public class AclEntryAfterInvocationProvider extends AbstractAclProvider impleme
 			if (!this.supports(attr)) {
 				continue;
 			}
-			// Need to make an access decision on this invocation
 
+			// Need to make an access decision on this invocation
 			if (hasPermission(authentication, returnedObject)) {
 				return returnedObject;
 			}
 
 			logger.debug("Denying access");
-
 			throw new AccessDeniedException(this.messages.getMessage("AclEntryAfterInvocationProvider.noPermission",
 					new Object[] { authentication.getName(), returnedObject },
 					"Authentication {0} has NO permissions to the domain object {1}"));

acl/src/main/java/org/springframework/security/acls/afterinvocation/ArrayFilterer.java

@@ -25,6 +25,8 @@ import java.util.Set;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
+
 /**
  * A filter used to filter arrays.
  *
@@ -41,17 +43,12 @@ class ArrayFilterer<T> implements Filterer<T> {
 
 	ArrayFilterer(T[] list) {
 		this.list = list;
-
 		// Collect the removed objects to a HashSet so that
 		// it is fast to lookup them when a filtered array
 		// is constructed.
 		this.removeList = new HashSet<>();
 	}
 
-	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#getFilteredObject()
-	 */
 	@Override
 	@SuppressWarnings("unchecked")
 	public T[] getFilteredObject() {
@@ -59,60 +56,48 @@ class ArrayFilterer<T> implements Filterer<T> {
 		int originalSize = this.list.length;
 		int sizeOfResultingList = originalSize - this.removeList.size();
 		T[] filtered = (T[]) Array.newInstance(this.list.getClass().getComponentType(), sizeOfResultingList);
-
 		for (int i = 0, j = 0; i < this.list.length; i++) {
 			T object = this.list[i];
-
 			if (!this.removeList.contains(object)) {
 				filtered[j] = object;
 				j++;
 			}
 		}
+		logger.debug(LogMessage.of(() -> "Original array contained " + originalSize + " elements; now contains "
+				+ sizeOfResultingList + " elements"));
+		return filtered;
+	}
 
-		if (logger.isDebugEnabled()) {
-			logger.debug("Original array contained " + originalSize + " elements; now contains " + sizeOfResultingList
-					+ " elements");
-		}
+	@Override
+	public Iterator<T> iterator() {
+		return new ArrayFiltererIterator();
+	}
 
-		return filtered;
+	@Override
+	public void remove(T object) {
+		this.removeList.add(object);
 	}
 
 	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#iterator()
+	 * Iterator for {@link ArrayFilterer} elements.
 	 */
-	@Override
-	public Iterator<T> iterator() {
-		return new Iterator<T>() {
-			private int index = 0;
+	private class ArrayFiltererIterator implements Iterator<T> {
 
-			@Override
-			public boolean hasNext() {
-				return this.index < ArrayFilterer.this.list.length;
-			}
+		private int index = 0;
 
-			@Override
-			public T next() {
-				if (!hasNext()) {
-					throw new NoSuchElementException();
-				}
-				return ArrayFilterer.this.list[this.index++];
-			}
+		@Override
+		public boolean hasNext() {
+			return this.index < ArrayFilterer.this.list.length;
+		}
 
-			@Override
-			public void remove() {
-				throw new UnsupportedOperationException();
+		@Override
+		public T next() {
+			if (hasNext()) {
+				return ArrayFilterer.this.list[this.index++];
 			}
-		};
-	}
+			throw new NoSuchElementException();
+		}
 
-	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#remove(java.lang.Object)
-	 */
-	@Override
-	public void remove(T object) {
-		this.removeList.add(object);
 	}
 
 }

acl/src/main/java/org/springframework/security/acls/afterinvocation/CollectionFilterer.java

@@ -24,6 +24,8 @@ import java.util.Set;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
+import org.springframework.core.log.LogMessage;
+
 /**
  * A filter used to filter Collections.
  *
@@ -40,7 +42,6 @@ class CollectionFilterer<T> implements Filterer<T> {
 
 	CollectionFilterer(Collection<T> collection) {
 		this.collection = collection;
-
 		// We create a Set of objects to be removed from the Collection,
 		// as ConcurrentModificationException prevents removal during
 		// iteration, and making a new Collection to be returned is
@@ -51,42 +52,24 @@ class CollectionFilterer<T> implements Filterer<T> {
 		this.removeList = new HashSet<>();
 	}
 
-	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#getFilteredObject()
-	 */
 	@Override
 	public Object getFilteredObject() {
 		// Now the Iterator has ended, remove Objects from Collection
 		Iterator<T> removeIter = this.removeList.iterator();
-
 		int originalSize = this.collection.size();
-
 		while (removeIter.hasNext()) {
 			this.collection.remove(removeIter.next());
 		}
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("Original collection contained " + originalSize + " elements; now contains "
-					+ this.collection.size() + " elements");
-		}
-
+		logger.debug(LogMessage.of(() -> "Original collection contained " + originalSize + " elements; now contains "
+				+ this.collection.size() + " elements"));
 		return this.collection;
 	}
 
-	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#iterator()
-	 */
 	@Override
 	public Iterator<T> iterator() {
 		return this.collection.iterator();
 	}
 
-	/**
-	 *
-	 * @see org.springframework.security.acls.afterinvocation.Filterer#remove(java.lang.Object)
-	 */
 	@Override
 	public void remove(T object) {
 		this.removeList.add(object);

acl/src/main/java/org/springframework/security/acls/domain/AccessControlEntryImpl.java

@@ -65,60 +65,54 @@ public class AccessControlEntryImpl implements AccessControlEntry, AuditableAcce
 		if (!(arg0 instanceof AccessControlEntryImpl)) {
 			return false;
 		}
-
-		AccessControlEntryImpl rhs = (AccessControlEntryImpl) arg0;
-
+		AccessControlEntryImpl other = (AccessControlEntryImpl) arg0;
 		if (this.acl == null) {
-			if (rhs.getAcl() != null) {
+			if (other.getAcl() != null) {
 				return false;
 			}
 			// Both this.acl and rhs.acl are null and thus equal
 		}
 		else {
 			// this.acl is non-null
-			if (rhs.getAcl() == null) {
+			if (other.getAcl() == null) {
 				return false;
 			}
 
 			// Both this.acl and rhs.acl are non-null, so do a comparison
 			if (this.acl.getObjectIdentity() == null) {
-				if (rhs.acl.getObjectIdentity() != null) {
+				if (other.acl.getObjectIdentity() != null) {
 					return false;
 				}
 				// Both this.acl and rhs.acl are null and thus equal
 			}
 			else {
 				// Both this.acl.objectIdentity and rhs.acl.objectIdentity are non-null
-				if (!this.acl.getObjectIdentity().equals(rhs.getAcl().getObjectIdentity())) {
+				if (!this.acl.getObjectIdentity().equals(other.getAcl().getObjectIdentity())) {
 					return false;
 				}
 			}
 		}
-
 		if (this.id == null) {
-			if (rhs.id != null) {
+			if (other.id != null) {
 				return false;
 			}
 			// Both this.id and rhs.id are null and thus equal
 		}
 		else {
 			// this.id is non-null
-			if (rhs.id == null) {
+			if (other.id == null) {
 				return false;
 			}
-
 			// Both this.id and rhs.id are non-null
-			if (!this.id.equals(rhs.id)) {
+			if (!this.id.equals(other.id)) {
 				return false;
 			}
 		}
-
-		if ((this.auditFailure != rhs.isAuditFailure()) || (this.auditSuccess != rhs.isAuditSuccess())
-				|| (this.granting != rhs.isGranting()) || !this.permission.equals(rhs.getPermission())
-				|| !this.sid.equals(rhs.getSid())) {
+		if ((this.auditFailure != other.isAuditFailure()) || (this.auditSuccess != other.isAuditSuccess())
+				|| (this.granting != other.isGranting()) || !this.permission.equals(other.getPermission())
+				|| !this.sid.equals(other.getSid())) {
 			return false;
 		}
-
 		return true;
 	}
 
@@ -192,7 +186,6 @@ public class AccessControlEntryImpl implements AccessControlEntry, AuditableAcce
 		sb.append("auditSuccess: ").append(this.auditSuccess).append("; ");
 		sb.append("auditFailure: ").append(this.auditFailure);
 		sb.append("]");
-
 		return sb.toString();
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/AclAuthorizationStrategyImpl.java

@@ -86,32 +86,15 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
 				|| !SecurityContextHolder.getContext().getAuthentication().isAuthenticated()) {
 			throw new AccessDeniedException("Authenticated principal required to operate with ACLs");
 		}
-
 		Authentication authentication = SecurityContextHolder.getContext().getAuthentication();
-
 		// Check if authorized by virtue of ACL ownership
 		Sid currentUser = createCurrentUser(authentication);
-
 		if (currentUser.equals(acl.getOwner())
 				&& ((changeType == CHANGE_GENERAL) || (changeType == CHANGE_OWNERSHIP))) {
 			return;
 		}
-
 		// Not authorized by ACL ownership; try via adminstrative permissions
-		GrantedAuthority requiredAuthority;
-
-		if (changeType == CHANGE_AUDITING) {
-			requiredAuthority = this.gaModifyAuditing;
-		}
-		else if (changeType == CHANGE_GENERAL) {
-			requiredAuthority = this.gaGeneralChanges;
-		}
-		else if (changeType == CHANGE_OWNERSHIP) {
-			requiredAuthority = this.gaTakeOwnership;
-		}
-		else {
-			throw new IllegalArgumentException("Unknown change type");
-		}
+		GrantedAuthority requiredAuthority = getRequiredAuthority(changeType);
 
 		// Iterate this principal's authorities to determine right
 		Set<String> authorities = AuthorityUtils.authorityListToSet(authentication.getAuthorities());
@@ -121,7 +104,6 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
 
 		// Try to get permission via ACEs within the ACL
 		List<Sid> sids = this.sidRetrievalStrategy.getSids(authentication);
-
 		if (acl.isGranted(Arrays.asList(BasePermission.ADMINISTRATION), sids, false)) {
 			return;
 		}
@@ -130,6 +112,19 @@ public class AclAuthorizationStrategyImpl implements AclAuthorizationStrategy {
 				"Principal does not have required ACL permissions to perform requested operation");
 	}
 
+	private GrantedAuthority getRequiredAuthority(int changeType) {
+		if (changeType == CHANGE_AUDITING) {
+			return this.gaModifyAuditing;
+		}
+		if (changeType == CHANGE_GENERAL) {
+			return this.gaGeneralChanges;
+		}
+		if (changeType == CHANGE_OWNERSHIP) {
+			return this.gaTakeOwnership;
+		}
+		throw new IllegalArgumentException("Unknown change type");
+	}
+
 	/**
 	 * Creates a principal-like sid from the authentication information.
 	 * @param authentication the authentication information that can provide principal and

acl/src/main/java/org/springframework/security/acls/domain/AclFormattingUtils.java

@@ -31,9 +31,7 @@ public abstract class AclFormattingUtils {
 		Assert.notNull(removeBits, "Bits To Remove string required");
 		Assert.isTrue(original.length() == removeBits.length(),
 				"Original and Bits To Remove strings must be identical length");
-
 		char[] replacement = new char[original.length()];
-
 		for (int i = 0; i < original.length(); i++) {
 			if (removeBits.charAt(i) == Permission.RESERVED_OFF) {
 				replacement[i] = original.charAt(i);
@@ -42,7 +40,6 @@ public abstract class AclFormattingUtils {
 				replacement[i] = Permission.RESERVED_OFF;
 			}
 		}
-
 		return new String(replacement);
 	}
 
@@ -51,9 +48,7 @@ public abstract class AclFormattingUtils {
 		Assert.notNull(extraBits, "Extra Bits string required");
 		Assert.isTrue(original.length() == extraBits.length(),
 				"Original and Extra Bits strings must be identical length");
-
 		char[] replacement = new char[extraBits.length()];
-
 		for (int i = 0; i < extraBits.length(); i++) {
 			if (extraBits.charAt(i) == Permission.RESERVED_OFF) {
 				replacement[i] = original.charAt(i);
@@ -62,7 +57,6 @@ public abstract class AclFormattingUtils {
 				replacement[i] = extraBits.charAt(i);
 			}
 		}
-
 		return new String(replacement);
 	}
 
@@ -92,7 +86,6 @@ public abstract class AclFormattingUtils {
 				() -> Permission.RESERVED_ON + " is a reserved character code");
 		Assert.doesNotContain(Character.toString(code), Character.toString(Permission.RESERVED_OFF),
 				() -> Permission.RESERVED_OFF + " is a reserved character code");
-
 		return printBinary(mask, Permission.RESERVED_ON, Permission.RESERVED_OFF).replace(Permission.RESERVED_ON, code);
 	}
 
@@ -100,7 +93,6 @@ public abstract class AclFormattingUtils {
 		String s = Integer.toBinaryString(i);
 		String pattern = Permission.THIRTY_TWO_RESERVED_OFF;
 		String temp2 = pattern.substring(0, pattern.length() - s.length()) + s;
-
 		return temp2.replace('0', off).replace('1', on);
 	}
 

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

@@ -53,10 +53,11 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 
 	private Serializable id;
 
-	private Sid owner; // OwnershipAcl
+	// OwnershipAcl
+	private Sid owner;
 
-	private List<Sid> loadedSids = null; // includes all SIDs the WHERE clause covered,
-											// even if there was no ACE for a SID
+	// includes all SIDs the WHERE clause covered, even if there was no ACE for a SID
+	private List<Sid> loadedSids = null;
 
 	private boolean entriesInheriting = true;
 
@@ -102,7 +103,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 		Assert.notNull(id, "Id required");
 		Assert.notNull(aclAuthorizationStrategy, "AclAuthorizationStrategy required");
 		Assert.notNull(owner, "Owner required");
-
 		this.objectIdentity = objectIdentity;
 		this.id = id;
 		this.aclAuthorizationStrategy = aclAuthorizationStrategy;
@@ -125,7 +125,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 	public void deleteAce(int aceIndex) throws NotFoundException {
 		this.aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
 		verifyAceIndexExists(aceIndex);
-
 		synchronized (this.aces) {
 			this.aces.remove(aceIndex);
 		}
@@ -154,9 +153,7 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 			throw new NotFoundException(
 					"atIndexLocation must be less than or equal to the size of the AccessControlEntry collection");
 		}
-
 		AccessControlEntryImpl ace = new AccessControlEntryImpl(null, this, sid, permission, granting, false, false);
-
 		synchronized (this.aces) {
 			this.aces.add(atIndexLocation, ace);
 		}
@@ -195,11 +192,9 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 			throws NotFoundException, UnloadedSidException {
 		Assert.notEmpty(permission, "Permissions required");
 		Assert.notEmpty(sids, "SIDs required");
-
 		if (!this.isSidLoaded(sids)) {
 			throw new UnloadedSidException("ACL was not loaded for one or more SID");
 		}
-
 		return this.permissionGrantingStrategy.isGranted(this, permission, sids, administrativeMode);
 	}
 
@@ -214,16 +209,13 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 		// This ACL applies to a SID subset only. Iterate to check it applies.
 		for (Sid sid : sids) {
 			boolean found = false;
-
 			for (Sid loadedSid : this.loadedSids) {
 				if (sid.equals(loadedSid)) {
 					// this SID is OK
 					found = true;
-
 					break; // out of loadedSids for loop
 				}
 			}
-
 			if (!found) {
 				return false;
 			}
@@ -266,7 +258,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 	public void updateAce(int aceIndex, Permission permission) throws NotFoundException {
 		this.aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL);
 		verifyAceIndexExists(aceIndex);
-
 		synchronized (this.aces) {
 			AccessControlEntryImpl ace = (AccessControlEntryImpl) this.aces.get(aceIndex);
 			ace.setPermission(permission);
@@ -277,7 +268,6 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 	public void updateAuditing(int aceIndex, boolean auditSuccess, boolean auditFailure) {
 		this.aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_AUDITING);
 		verifyAceIndexExists(aceIndex);
-
 		synchronized (this.aces) {
 			AccessControlEntryImpl ace = (AccessControlEntryImpl) this.aces.get(aceIndex);
 			ace.setAuditSuccess(auditSuccess);
@@ -327,30 +317,23 @@ public class AclImpl implements Acl, MutableAcl, AuditableAcl, OwnershipAcl {
 		sb.append("id: ").append(this.id).append("; ");
 		sb.append("objectIdentity: ").append(this.objectIdentity).append("; ");
 		sb.append("owner: ").append(this.owner).append("; ");
-
 		int count = 0;
-
 		for (AccessControlEntry ace : this.aces) {
 			count++;
-
 			if (count == 1) {
 				sb.append("\n");
 			}
-
 			sb.append(ace).append("\n");
 		}
-
 		if (count == 0) {
 			sb.append("no ACEs; ");
 		}
-
 		sb.append("inheriting: ").append(this.entriesInheriting).append("; ");
 		sb.append("parent: ").append((this.parentAcl == null) ? "Null" : this.parentAcl.getObjectIdentity().toString());
 		sb.append("; ");
 		sb.append("aclAuthorizationStrategy: ").append(this.aclAuthorizationStrategy).append("; ");
 		sb.append("permissionGrantingStrategy: ").append(this.permissionGrantingStrategy);
 		sb.append("]");
-
 		return sb.toString();
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/AuditLogger.java

@@ -22,7 +22,6 @@ import org.springframework.security.acls.model.AccessControlEntry;
  * Used by <code>AclImpl</code> to log audit events.
  *
  * @author Ben Alex
- *
  */
 public interface AuditLogger {
 

acl/src/main/java/org/springframework/security/acls/domain/ConsoleAuditLogger.java

@@ -30,10 +30,8 @@ public class ConsoleAuditLogger implements AuditLogger {
 	@Override
 	public void logIfNeeded(boolean granted, AccessControlEntry ace) {
 		Assert.notNull(ace, "AccessControlEntry required");
-
 		if (ace instanceof AuditableAccessControlEntry) {
 			AuditableAccessControlEntry auditableAce = (AuditableAccessControlEntry) ace;
-
 			if (granted && auditableAce.isAuditSuccess()) {
 				System.out.println("GRANTED due to ACE: " + ace);
 			}

acl/src/main/java/org/springframework/security/acls/domain/CumulativePermission.java

@@ -39,21 +39,18 @@ public class CumulativePermission extends AbstractPermission {
 	public CumulativePermission clear(Permission permission) {
 		this.mask &= ~permission.getMask();
 		this.pattern = AclFormattingUtils.demergePatterns(this.pattern, permission.getPattern());
-
 		return this;
 	}
 
 	public CumulativePermission clear() {
 		this.mask = 0;
 		this.pattern = THIRTY_TWO_RESERVED_OFF;
-
 		return this;
 	}
 
 	public CumulativePermission set(Permission permission) {
 		this.mask |= permission.getMask();
 		this.pattern = AclFormattingUtils.mergePatterns(this.pattern, permission.getPattern());
-
 		return this;
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionFactory.java

@@ -77,22 +77,18 @@ public class DefaultPermissionFactory implements PermissionFactory {
 	 */
 	protected void registerPublicPermissions(Class<? extends Permission> clazz) {
 		Assert.notNull(clazz, "Class required");
-
 		Field[] fields = clazz.getFields();
-
 		for (Field field : fields) {
 			try {
 				Object fieldValue = field.get(null);
-
 				if (Permission.class.isAssignableFrom(fieldValue.getClass())) {
 					// Found a Permission static field
 					Permission perm = (Permission) fieldValue;
 					String permissionName = field.getName();
-
 					registerPermission(perm, permissionName);
 				}
 			}
-			catch (Exception ignore) {
+			catch (Exception ex) {
 			}
 		}
 	}
@@ -100,7 +96,6 @@ public class DefaultPermissionFactory implements PermissionFactory {
 	protected void registerPermission(Permission perm, String permissionName) {
 		Assert.notNull(perm, "Permission required");
 		Assert.hasText(permissionName, "Permission name required");
-
 		Integer mask = perm.getMask();
 
 		// Ensure no existing Permission uses this integer or code
@@ -124,32 +119,22 @@ public class DefaultPermissionFactory implements PermissionFactory {
 
 		// To get this far, we have to use a CumulativePermission
 		CumulativePermission permission = new CumulativePermission();
-
 		for (int i = 0; i < 32; i++) {
 			int permissionToCheck = 1 << i;
-
 			if ((mask & permissionToCheck) == permissionToCheck) {
 				Permission p = this.registeredPermissionsByInteger.get(permissionToCheck);
-
-				if (p == null) {
-					throw new IllegalStateException(
-							"Mask '" + permissionToCheck + "' does not have a corresponding static Permission");
-				}
+				Assert.state(p != null,
+						() -> "Mask '" + permissionToCheck + "' does not have a corresponding static Permission");
 				permission.set(p);
 			}
 		}
-
 		return permission;
 	}
 
 	@Override
 	public Permission buildFromName(String name) {
 		Permission p = this.registeredPermissionsByName.get(name);
-
-		if (p == null) {
-			throw new IllegalArgumentException("Unknown permission '" + name + "'");
-		}
-
+		Assert.notNull(p, "Unknown permission '" + name + "'");
 		return p;
 	}
 
@@ -158,13 +143,10 @@ public class DefaultPermissionFactory implements PermissionFactory {
 		if ((names == null) || (names.size() == 0)) {
 			return Collections.emptyList();
 		}
-
 		List<Permission> permissions = new ArrayList<>(names.size());
-
 		for (String name : names) {
 			permissions.add(buildFromName(name));
 		}
-
 		return permissions;
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java

@@ -74,18 +74,13 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra
 	@Override
 	public boolean isGranted(Acl acl, List<Permission> permission, List<Sid> sids, boolean administrativeMode)
 			throws NotFoundException {
-
-		final List<AccessControlEntry> aces = acl.getEntries();
-
+		List<AccessControlEntry> aces = acl.getEntries();
 		AccessControlEntry firstRejection = null;
-
 		for (Permission p : permission) {
 			for (Sid sid : sids) {
 				// Attempt to find exact match for this permission mask and SID
 				boolean scanNextSid = true;
-
 				for (AccessControlEntry ace : aces) {
-
 					if (isGranted(ace, p) && ace.getSid().equals(sid)) {
 						// Found a matching ACE, so its authorization decision will
 						// prevail
@@ -94,7 +89,6 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra
 							if (!administrativeMode) {
 								this.auditLogger.logIfNeeded(true, ace);
 							}
-
 							return true;
 						}
 
@@ -105,13 +99,11 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra
 							// Store first rejection for auditing reasons
 							firstRejection = ace;
 						}
-
 						scanNextSid = false; // helps break the loop
 
 						break; // exit aces loop
 					}
 				}
-
 				if (!scanNextSid) {
 					break; // exit SID for loop (now try next permission)
 				}
@@ -124,7 +116,6 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra
 			if (!administrativeMode) {
 				this.auditLogger.logIfNeeded(false, firstRejection);
 			}
-
 			return false;
 		}
 
@@ -133,10 +124,9 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra
 			// We have a parent, so let them try to find a matching ACE
 			return acl.getParentAcl().isGranted(permission, sids, false);
 		}
-		else {
-			// We either have no parent, or we're the uppermost parent
-			throw new NotFoundException("Unable to locate a matching ACE for passed permissions and SIDs");
-		}
+
+		// We either have no parent, or we're the uppermost parent
+		throw new NotFoundException("Unable to locate a matching ACE for passed permissions and SIDs");
 	}
 
 	/**

acl/src/main/java/org/springframework/security/acls/domain/EhCacheBasedAclCache.java

@@ -59,9 +59,7 @@ public class EhCacheBasedAclCache implements AclCache {
 	@Override
 	public void evictFromCache(Serializable pk) {
 		Assert.notNull(pk, "Primary key (identifier) required");
-
 		MutableAcl acl = getFromCache(pk);
-
 		if (acl != null) {
 			this.cache.remove(acl.getId());
 			this.cache.remove(acl.getObjectIdentity());
@@ -71,9 +69,7 @@ public class EhCacheBasedAclCache implements AclCache {
 	@Override
 	public void evictFromCache(ObjectIdentity objectIdentity) {
 		Assert.notNull(objectIdentity, "ObjectIdentity required");
-
 		MutableAcl acl = getFromCache(objectIdentity);
-
 		if (acl != null) {
 			this.cache.remove(acl.getId());
 			this.cache.remove(acl.getObjectIdentity());
@@ -83,39 +79,25 @@ public class EhCacheBasedAclCache implements AclCache {
 	@Override
 	public MutableAcl getFromCache(ObjectIdentity objectIdentity) {
 		Assert.notNull(objectIdentity, "ObjectIdentity required");
-
-		Element element = null;
-
 		try {
-			element = this.cache.get(objectIdentity);
-		}
-		catch (CacheException ignored) {
+			Element element = this.cache.get(objectIdentity);
+			return (element != null) ? initializeTransientFields((MutableAcl) element.getValue()) : null;
 		}
-
-		if (element == null) {
+		catch (CacheException ex) {
 			return null;
 		}
-
-		return initializeTransientFields((MutableAcl) element.getValue());
 	}
 
 	@Override
 	public MutableAcl getFromCache(Serializable pk) {
 		Assert.notNull(pk, "Primary key (identifier) required");
-
-		Element element = null;
-
 		try {
-			element = this.cache.get(pk);
-		}
-		catch (CacheException ignored) {
+			Element element = this.cache.get(pk);
+			return (element != null) ? initializeTransientFields((MutableAcl) element.getValue()) : null;
 		}
-
-		if (element == null) {
+		catch (CacheException ex) {
 			return null;
 		}
-
-		return initializeTransientFields((MutableAcl) element.getValue());
 	}
 
 	@Override
@@ -123,7 +105,6 @@ public class EhCacheBasedAclCache implements AclCache {
 		Assert.notNull(acl, "Acl required");
 		Assert.notNull(acl.getObjectIdentity(), "ObjectIdentity required");
 		Assert.notNull(acl.getId(), "ID required");
-
 		if (this.aclAuthorizationStrategy == null) {
 			if (acl instanceof AclImpl) {
 				this.aclAuthorizationStrategy = (AclAuthorizationStrategy) FieldUtils
@@ -132,11 +113,9 @@ public class EhCacheBasedAclCache implements AclCache {
 						.getProtectedFieldValue("permissionGrantingStrategy", acl);
 			}
 		}
-
 		if ((acl.getParentAcl() != null) && (acl.getParentAcl() instanceof MutableAcl)) {
 			putInCache((MutableAcl) acl.getParentAcl());
 		}
-
 		this.cache.put(new Element(acl.getObjectIdentity(), acl));
 		this.cache.put(new Element(acl.getId(), acl));
 	}
@@ -146,7 +125,6 @@ public class EhCacheBasedAclCache implements AclCache {
 			FieldUtils.setProtectedFieldValue("aclAuthorizationStrategy", value, this.aclAuthorizationStrategy);
 			FieldUtils.setProtectedFieldValue("permissionGrantingStrategy", value, this.permissionGrantingStrategy);
 		}
-
 		if (value.getParentAcl() != null) {
 			initializeTransientFields((MutableAcl) value.getParentAcl());
 		}

acl/src/main/java/org/springframework/security/acls/domain/GrantedAuthoritySid.java

@@ -51,7 +51,6 @@ public class GrantedAuthoritySid implements Sid {
 		if ((object == null) || !(object instanceof GrantedAuthoritySid)) {
 			return false;
 		}
-
 		// Delegate to getGrantedAuthority() to perform actual comparison (both should be
 		// identical)
 		return ((GrantedAuthoritySid) object).getGrantedAuthority().equals(this.getGrantedAuthority());

acl/src/main/java/org/springframework/security/acls/domain/ObjectIdentityImpl.java

@@ -40,7 +40,6 @@ public class ObjectIdentityImpl implements ObjectIdentity {
 	public ObjectIdentityImpl(String type, Serializable identifier) {
 		Assert.hasText(type, "Type required");
 		Assert.notNull(identifier, "identifier required");
-
 		this.identifier = identifier;
 		this.type = type;
 	}
@@ -68,23 +67,22 @@ public class ObjectIdentityImpl implements ObjectIdentity {
 	 */
 	public ObjectIdentityImpl(Object object) throws IdentityUnavailableException {
 		Assert.notNull(object, "object cannot be null");
-
 		Class<?> typeClass = ClassUtils.getUserClass(object.getClass());
 		this.type = typeClass.getName();
+		Object result = invokeGetIdMethod(object, typeClass);
+		Assert.notNull(result, "getId() is required to return a non-null value");
+		Assert.isInstanceOf(Serializable.class, result, "Getter must provide a return value of type Serializable");
+		this.identifier = (Serializable) result;
+	}
 
-		Object result;
-
+	private Object invokeGetIdMethod(Object object, Class<?> typeClass) {
 		try {
 			Method method = typeClass.getMethod("getId", new Class[] {});
-			result = method.invoke(object);
+			return method.invoke(object);
 		}
 		catch (Exception ex) {
 			throw new IdentityUnavailableException("Could not extract identity from object " + object, ex);
 		}
-
-		Assert.notNull(result, "getId() is required to return a non-null value");
-		Assert.isInstanceOf(Serializable.class, result, "Getter must provide a return value of type Serializable");
-		this.identifier = (Serializable) result;
 	}
 
 	/**
@@ -95,17 +93,15 @@ public class ObjectIdentityImpl implements ObjectIdentity {
 	 * <p>
 	 * Numeric identities (Integer and Long values) are considered equal if they are
 	 * numerically equal. Other serializable types are evaluated using a simple equality.
-	 * @param arg0 object to compare
+	 * @param obj object to compare
 	 * @return <code>true</code> if the presented object matches this object
 	 */
 	@Override
-	public boolean equals(Object arg0) {
-		if (arg0 == null || !(arg0 instanceof ObjectIdentityImpl)) {
+	public boolean equals(Object obj) {
+		if (obj == null || !(obj instanceof ObjectIdentityImpl)) {
 			return false;
 		}
-
-		ObjectIdentityImpl other = (ObjectIdentityImpl) arg0;
-
+		ObjectIdentityImpl other = (ObjectIdentityImpl) obj;
 		if (this.identifier instanceof Number && other.identifier instanceof Number) {
 			// Integers and Longs with same value should be considered equal
 			if (((Number) this.identifier).longValue() != ((Number) other.identifier).longValue()) {
@@ -118,7 +114,6 @@ public class ObjectIdentityImpl implements ObjectIdentity {
 				return false;
 			}
 		}
-
 		return this.type.equals(other.type);
 	}
 
@@ -149,7 +144,6 @@ public class ObjectIdentityImpl implements ObjectIdentity {
 		sb.append(this.getClass().getName()).append("[");
 		sb.append("Type: ").append(this.type);
 		sb.append("; Identifier: ").append(this.identifier).append("]");
-
 		return sb.toString();
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/PermissionFactory.java

@@ -26,7 +26,6 @@ import org.springframework.security.acls.model.Permission;
  *
  * @author Ben Alex
  * @since 2.0.3
- *
  */
 public interface PermissionFactory {
 

acl/src/main/java/org/springframework/security/acls/domain/PrincipalSid.java

@@ -42,7 +42,6 @@ public class PrincipalSid implements Sid {
 	public PrincipalSid(Authentication authentication) {
 		Assert.notNull(authentication, "Authentication required");
 		Assert.notNull(authentication.getPrincipal(), "Principal required");
-
 		this.principal = authentication.getName();
 	}
 
@@ -51,7 +50,6 @@ public class PrincipalSid implements Sid {
 		if ((object == null) || !(object instanceof PrincipalSid)) {
 			return false;
 		}
-
 		// Delegate to getPrincipal() to perform actual comparison (both should be
 		// identical)
 		return ((PrincipalSid) object).getPrincipal().equals(this.getPrincipal());

acl/src/main/java/org/springframework/security/acls/domain/SidRetrievalStrategyImpl.java

@@ -56,13 +56,10 @@ public class SidRetrievalStrategyImpl implements SidRetrievalStrategy {
 		Collection<? extends GrantedAuthority> authorities = this.roleHierarchy
 				.getReachableGrantedAuthorities(authentication.getAuthorities());
 		List<Sid> sids = new ArrayList<>(authorities.size() + 1);
-
 		sids.add(new PrincipalSid(authentication));
-
 		for (GrantedAuthority authority : authorities) {
 			sids.add(new GrantedAuthoritySid(authority));
 		}
-
 		return sids;
 	}
 

acl/src/main/java/org/springframework/security/acls/domain/SpringCacheBasedAclCache.java

@@ -60,9 +60,7 @@ public class SpringCacheBasedAclCache implements AclCache {
 	@Override
 	public void evictFromCache(Serializable pk) {
 		Assert.notNull(pk, "Primary key (identifier) required");
-
 		MutableAcl acl = getFromCache(pk);
-
 		if (acl != null) {
 			this.cache.evict(acl.getId());
 			this.cache.evict(acl.getObjectIdentity());
@@ -72,9 +70,7 @@ public class SpringCacheBasedAclCache implements AclCache {
 	@Override
 	public void evictFromCache(ObjectIdentity objectIdentity) {
 		Assert.notNull(objectIdentity, "ObjectIdentity required");
-
 		MutableAcl acl = getFromCache(objectIdentity);
-
 		if (acl != null) {
 			this.cache.evict(acl.getId());
 			this.cache.evict(acl.getObjectIdentity());
@@ -98,22 +94,18 @@ public class SpringCacheBasedAclCache implements AclCache {
 		Assert.notNull(acl, "Acl required");
 		Assert.notNull(acl.getObjectIdentity(), "ObjectIdentity required");
 		Assert.notNull(acl.getId(), "ID required");
-
 		if ((acl.getParentAcl() != null) && (acl.getParentAcl() instanceof MutableAcl)) {
 			putInCache((MutableAcl) acl.getParentAcl());
 		}
-
 		this.cache.put(acl.getObjectIdentity(), acl);
 		this.cache.put(acl.getId(), acl);
 	}
 
 	private MutableAcl getFromCache(Object key) {
 		Cache.ValueWrapper element = this.cache.get(key);
-
 		if (element == null) {
 			return null;
 		}
-
 		return initializeTransientFields((MutableAcl) element.get());
 	}
 
@@ -122,7 +114,6 @@ public class SpringCacheBasedAclCache implements AclCache {
 			FieldUtils.setProtectedFieldValue("aclAuthorizationStrategy", value, this.aclAuthorizationStrategy);
 			FieldUtils.setProtectedFieldValue("permissionGrantingStrategy", value, this.permissionGrantingStrategy);
 		}
-
 		if (value.getParentAcl() != null) {
 			initializeTransientFields((MutableAcl) value.getParentAcl());
 		}

acl/src/main/java/org/springframework/security/acls/jdbc/AclClassIdUtils.java

@@ -70,26 +70,20 @@ class AclClassIdUtils {
 	Serializable identifierFrom(Serializable identifier, ResultSet resultSet) throws SQLException {
 		if (isString(identifier) && hasValidClassIdType(resultSet)
 				&& canConvertFromStringTo(classIdTypeFrom(resultSet))) {
-
-			identifier = convertFromStringTo((String) identifier, classIdTypeFrom(resultSet));
-		}
-		else {
-			// Assume it should be a Long type
-			identifier = convertToLong(identifier);
+			return convertFromStringTo((String) identifier, classIdTypeFrom(resultSet));
 		}
-
-		return identifier;
+		// Assume it should be a Long type
+		return convertToLong(identifier);
 	}
 
 	private boolean hasValidClassIdType(ResultSet resultSet) {
-		boolean hasClassIdType = false;
 		try {
-			hasClassIdType = classIdTypeFrom(resultSet) != null;
+			return classIdTypeFrom(resultSet) != null;
 		}
 		catch (SQLException ex) {
 			log.debug("Unable to obtain the class id type", ex);
+			return false;
 		}
-		return hasClassIdType;
 	}
 
 	private <T extends Serializable> Class<T> classIdTypeFrom(ResultSet resultSet) throws SQLException {
@@ -97,16 +91,16 @@ class AclClassIdUtils {
 	}
 
 	private <T extends Serializable> Class<T> classIdTypeFrom(String className) {
-		Class targetType = null;
-		if (className != null) {
-			try {
-				targetType = Class.forName(className);
-			}
-			catch (ClassNotFoundException ex) {
-				log.debug("Unable to find class id type on classpath", ex);
-			}
+		if (className == null) {
+			return null;
+		}
+		try {
+			return (Class) Class.forName(className);
+		}
+		catch (ClassNotFoundException ex) {
+			log.debug("Unable to find class id type on classpath", ex);
+			return null;
 		}
-		return targetType;
 	}
 
 	private <T> boolean canConvertFromStringTo(Class<T> targetType) {
@@ -128,14 +122,10 @@ class AclClassIdUtils {
 	 * @throws IllegalArgumentException if targetType is null
 	 */
 	private Long convertToLong(Serializable identifier) {
-		Long idAsLong;
 		if (this.conversionService.canConvert(identifier.getClass(), Long.class)) {
-			idAsLong = this.conversionService.convert(identifier, Long.class);
-		}
-		else {
-			idAsLong = Long.valueOf(identifier.toString());
+			return this.conversionService.convert(identifier, Long.class);
 		}
-		return idAsLong;
+		return Long.valueOf(identifier.toString());
 	}
 
 	private boolean isString(Serializable object) {

acl/src/main/java/org/springframework/security/acls/jdbc/BasicLookupStrategy.java

@@ -18,6 +18,7 @@ package org.springframework.security.acls.jdbc;
 
 import java.io.Serializable;
 import java.lang.reflect.Field;
+import java.sql.PreparedStatement;
 import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.util.ArrayList;
@@ -167,26 +168,19 @@ public class BasicLookupStrategy implements LookupStrategy {
 	}
 
 	private String computeRepeatingSql(String repeatingSql, int requiredRepetitions) {
-		assert requiredRepetitions > 0 : "requiredRepetitions must be > 0";
-
-		final String startSql = this.selectClause;
-
-		final String endSql = this.orderByClause;
-
+		Assert.isTrue(requiredRepetitions > 0, "requiredRepetitions must be > 0");
+		String startSql = this.selectClause;
+		String endSql = this.orderByClause;
 		StringBuilder sqlStringBldr = new StringBuilder(
 				startSql.length() + endSql.length() + requiredRepetitions * (repeatingSql.length() + 4));
 		sqlStringBldr.append(startSql);
-
 		for (int i = 1; i <= requiredRepetitions; i++) {
 			sqlStringBldr.append(repeatingSql);
-
 			if (i != requiredRepetitions) {
 				sqlStringBldr.append(" or ");
 			}
 		}
-
 		sqlStringBldr.append(endSql);
-
 		return sqlStringBldr.toString();
 	}
 
@@ -228,18 +222,9 @@ public class BasicLookupStrategy implements LookupStrategy {
 	private void lookupPrimaryKeys(final Map<Serializable, Acl> acls, final Set<Long> findNow, final List<Sid> sids) {
 		Assert.notNull(acls, "ACLs are required");
 		Assert.notEmpty(findNow, "Items to find now required");
-
 		String sql = computeRepeatingSql(this.lookupPrimaryKeysWhereClause, findNow.size());
-
-		Set<Long> parentsToLookup = this.jdbcTemplate.query(sql, (ps) -> {
-			int i = 0;
-
-			for (Long toFind : findNow) {
-				i++;
-				ps.setLong(i, toFind);
-			}
-		}, new ProcessResultSet(acls, sids));
-
+		Set<Long> parentsToLookup = this.jdbcTemplate.query(sql, (ps) -> setKeys(ps, findNow),
+				new ProcessResultSet(acls, sids));
 		// Lookup the parents, now that our JdbcTemplate has released the database
 		// connection (SEC-547)
 		if (parentsToLookup.size() > 0) {
@@ -247,6 +232,14 @@ public class BasicLookupStrategy implements LookupStrategy {
 		}
 	}
 
+	private void setKeys(PreparedStatement ps, Set<Long> findNow) throws SQLException {
+		int i = 0;
+		for (Long toFind : findNow) {
+			i++;
+			ps.setLong(i, toFind);
+		}
+	}
+
 	/**
 	 * The main method.
 	 * <p>
@@ -269,68 +262,48 @@ public class BasicLookupStrategy implements LookupStrategy {
 	public final Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids) {
 		Assert.isTrue(this.batchSize >= 1, "BatchSize must be >= 1");
 		Assert.notEmpty(objects, "Objects to lookup required");
-
 		// Map<ObjectIdentity,Acl>
-		Map<ObjectIdentity, Acl> result = new HashMap<>(); // contains
-															// FULLY
-															// loaded
-															// Acl
-															// objects
-
+		// contains FULLY loaded Acl objects
+		Map<ObjectIdentity, Acl> result = new HashMap<>();
 		Set<ObjectIdentity> currentBatchToLoad = new HashSet<>();
-
 		for (int i = 0; i < objects.size(); i++) {
 			final ObjectIdentity oid = objects.get(i);
 			boolean aclFound = false;
-
 			// Check we don't already have this ACL in the results
 			if (result.containsKey(oid)) {
 				aclFound = true;
 			}
-
 			// Check cache for the present ACL entry
 			if (!aclFound) {
 				Acl acl = this.aclCache.getFromCache(oid);
-
 				// Ensure any cached element supports all the requested SIDs
 				// (they should always, as our base impl doesn't filter on SID)
 				if (acl != null) {
-					if (acl.isSidLoaded(sids)) {
-						result.put(acl.getObjectIdentity(), acl);
-						aclFound = true;
-					}
-					else {
-						throw new IllegalStateException(
-								"Error: SID-filtered element detected when implementation does not perform SID filtering "
-										+ "- have you added something to the cache manually?");
-					}
+					Assert.state(acl.isSidLoaded(sids),
+							"Error: SID-filtered element detected when implementation does not perform SID filtering "
+									+ "- have you added something to the cache manually?");
+					result.put(acl.getObjectIdentity(), acl);
+					aclFound = true;
 				}
 			}
-
 			// Load the ACL from the database
 			if (!aclFound) {
 				currentBatchToLoad.add(oid);
 			}
-
 			// Is it time to load from JDBC the currentBatchToLoad?
 			if ((currentBatchToLoad.size() == this.batchSize) || ((i + 1) == objects.size())) {
 				if (currentBatchToLoad.size() > 0) {
 					Map<ObjectIdentity, Acl> loadedBatch = lookupObjectIdentities(currentBatchToLoad, sids);
-
 					// Add loaded batch (all elements 100% initialized) to results
 					result.putAll(loadedBatch);
-
 					// Add the loaded batch to the cache
-
 					for (Acl loadedAcl : loadedBatch.values()) {
 						this.aclCache.putInCache((AclImpl) loadedAcl);
 					}
-
 					currentBatchToLoad.clear();
 				}
 			}
 		}
-
 		return result;
 	}
 
@@ -343,37 +316,20 @@ public class BasicLookupStrategy implements LookupStrategy {
 	 * <p>
 	 * This subclass is required to return fully valid <code>Acl</code>s, including
 	 * properly-configured parent ACLs.
-	 *
 	 */
 	private Map<ObjectIdentity, Acl> lookupObjectIdentities(final Collection<ObjectIdentity> objectIdentities,
 			List<Sid> sids) {
 		Assert.notEmpty(objectIdentities, "Must provide identities to lookup");
 
-		final Map<Serializable, Acl> acls = new HashMap<>(); // contains
-																// Acls
-																// with
-																// StubAclParents
+		// contains Acls with StubAclParents
+		Map<Serializable, Acl> acls = new HashMap<>();
 
 		// Make the "acls" map contain all requested objectIdentities
 		// (including markers to each parent in the hierarchy)
 		String sql = computeRepeatingSql(this.lookupObjectIdentitiesWhereClause, objectIdentities.size());
 
-		Set<Long> parentsToLookup = this.jdbcTemplate.query(sql, (ps) -> {
-			int i = 0;
-			for (ObjectIdentity oid : objectIdentities) {
-				// Determine prepared statement values for this iteration
-				String type = oid.getType();
-
-				// No need to check for nulls, as guaranteed non-null by
-				// ObjectIdentity.getIdentifier() interface contract
-				String identifier = oid.getIdentifier().toString();
-
-				// Inject values
-				ps.setString((2 * i) + 1, identifier);
-				ps.setString((2 * i) + 2, type);
-				i++;
-			}
-		}, new ProcessResultSet(acls, sids));
+		Set<Long> parentsToLookup = this.jdbcTemplate.query(sql,
+				(ps) -> setupLookupObjectIdentitiesStatement(ps, objectIdentities), new ProcessResultSet(acls, sids));
 
 		// Lookup the parents, now that our JdbcTemplate has released the database
 		// connection (SEC-547)
@@ -383,11 +339,9 @@ public class BasicLookupStrategy implements LookupStrategy {
 
 		// Finally, convert our "acls" containing StubAclParents into true Acls
 		Map<ObjectIdentity, Acl> resultMap = new HashMap<>();
-
 		for (Acl inputAcl : acls.values()) {
 			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);
 		}
@@ -395,6 +349,24 @@ public class BasicLookupStrategy implements LookupStrategy {
 		return resultMap;
 	}
 
+	private void setupLookupObjectIdentitiesStatement(PreparedStatement ps, Collection<ObjectIdentity> objectIdentities)
+			throws SQLException {
+		int i = 0;
+		for (ObjectIdentity oid : objectIdentities) {
+			// Determine prepared statement values for this iteration
+			String type = oid.getType();
+
+			// No need to check for nulls, as guaranteed non-null by
+			// ObjectIdentity.getIdentifier() interface contract
+			String identifier = oid.getIdentifier().toString();
+
+			// Inject values
+			ps.setString((2 * i) + 1, identifier);
+			ps.setString((2 * i) + 2, type);
+			i++;
+		}
+	}
+
 	/**
 	 * The final phase of converting the <code>Map</code> of <code>AclImpl</code>
 	 * instances which contain <code>StubAclParent</code>s into proper, valid
@@ -402,7 +374,6 @@ public class BasicLookupStrategy implements LookupStrategy {
 	 * @param inputMap the unconverted <code>AclImpl</code>s
 	 * @param currentIdentity the current<code>Acl</code> that we wish to convert (this
 	 * may be
-	 *
 	 */
 	private AclImpl convert(Map<Serializable, Acl> inputMap, Long currentIdentity) {
 		Assert.notEmpty(inputMap, "InputMap required");
@@ -460,9 +431,7 @@ public class BasicLookupStrategy implements LookupStrategy {
 		if (isPrincipal) {
 			return new PrincipalSid(sid);
 		}
-		else {
-			return new GrantedAuthoritySid(sid);
-		}
+		return new GrantedAuthoritySid(sid);
 	}
 
 	/**
@@ -564,7 +533,6 @@ public class BasicLookupStrategy implements LookupStrategy {
 
 					// Now try to find it in the cache
 					MutableAcl cached = BasicLookupStrategy.this.aclCache.getFromCache(parentId);
-
 					if ((cached == null) || !cached.isSidLoaded(this.sids)) {
 						parentIdsToLookup.add(parentId);
 					}

acl/src/main/java/org/springframework/security/acls/jdbc/JdbcAclService.java

@@ -17,6 +17,8 @@
 package org.springframework.security.acls.jdbc;
 
 import java.io.Serializable;
+import java.sql.ResultSet;
+import java.sql.SQLException;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -94,18 +96,16 @@ public class JdbcAclService implements AclService {
 	@Override
 	public List<ObjectIdentity> findChildren(ObjectIdentity parentIdentity) {
 		Object[] args = { parentIdentity.getIdentifier().toString(), parentIdentity.getType() };
-		List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args, (rs, rowNum) -> {
-			String javaType = rs.getString("class");
-			Serializable identifier = (Serializable) rs.getObject("obj_id");
-			identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
-			return new ObjectIdentityImpl(javaType, identifier);
-		});
-
-		if (objects.isEmpty()) {
-			return null;
-		}
+		List<ObjectIdentity> objects = this.jdbcOperations.query(this.findChildrenSql, args,
+				(rs, rowNum) -> mapObjectIdentityRow(rs));
+		return (!objects.isEmpty()) ? objects : null;
+	}
 
-		return objects;
+	private ObjectIdentity mapObjectIdentityRow(ResultSet rs) throws SQLException {
+		String javaType = rs.getString("class");
+		Serializable identifier = (Serializable) rs.getObject("obj_id");
+		identifier = this.aclClassIdUtils.identifierFrom(identifier, rs);
+		return new ObjectIdentityImpl(javaType, identifier);
 	}
 
 	@Override
@@ -113,7 +113,6 @@ public class JdbcAclService implements AclService {
 		Map<ObjectIdentity, Acl> map = readAclsById(Collections.singletonList(object), sids);
 		Assert.isTrue(map.containsKey(object),
 				() -> "There should have been an Acl entry for ObjectIdentity " + object);
-
 		return map.get(object);
 	}
 
@@ -131,7 +130,6 @@ public class JdbcAclService implements AclService {
 	public Map<ObjectIdentity, Acl> readAclsById(List<ObjectIdentity> objects, List<Sid> sids)
 			throws NotFoundException {
 		Map<ObjectIdentity, Acl> result = this.lookupStrategy.readAclsById(objects, sids);
-
 		// Check every requested object identity was found (throw NotFoundException if
 		// needed)
 		for (ObjectIdentity oid : objects) {
@@ -139,7 +137,6 @@ public class JdbcAclService implements AclService {
 				throw new NotFoundException("Unable to find ACL information for object identity '" + oid + "'");
 			}
 		}
-
 		return result;
 	}
 

acl/src/main/java/org/springframework/security/acls/jdbc/JdbcMutableAclService.java

@@ -139,6 +139,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 			return;
 		}
 		this.jdbcOperations.batchUpdate(this.insertEntry, new BatchPreparedStatementSetter() {
+
 			@Override
 			public int getBatchSize() {
 				return acl.getEntries().size();
@@ -158,6 +159,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 				stmt.setBoolean(6, entry.isAuditSuccess());
 				stmt.setBoolean(7, entry.isAuditFailure());
 			}
+
 		});
 	}
 
@@ -216,22 +218,15 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 	 */
 	protected Long createOrRetrieveSidPrimaryKey(Sid sid, boolean allowCreate) {
 		Assert.notNull(sid, "Sid required");
-
-		String sidName;
-		boolean sidIsPrincipal = true;
-
 		if (sid instanceof PrincipalSid) {
-			sidName = ((PrincipalSid) sid).getPrincipal();
+			String sidName = ((PrincipalSid) sid).getPrincipal();
+			return createOrRetrieveSidPrimaryKey(sidName, true, allowCreate);
 		}
-		else if (sid instanceof GrantedAuthoritySid) {
-			sidName = ((GrantedAuthoritySid) sid).getGrantedAuthority();
-			sidIsPrincipal = false;
+		if (sid instanceof GrantedAuthoritySid) {
+			String sidName = ((GrantedAuthoritySid) sid).getGrantedAuthority();
+			return createOrRetrieveSidPrimaryKey(sidName, false, allowCreate);
 		}
-		else {
-			throw new IllegalArgumentException("Unsupported implementation of Sid");
-		}
-
-		return createOrRetrieveSidPrimaryKey(sidName, sidIsPrincipal, allowCreate);
+		throw new IllegalArgumentException("Unsupported implementation of Sid");
 	}
 
 	/**
@@ -243,20 +238,16 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 	 * @return the primary key or null if not found
 	 */
 	protected Long createOrRetrieveSidPrimaryKey(String sidName, boolean sidIsPrincipal, boolean allowCreate) {
-
 		List<Long> sidIds = this.jdbcOperations.queryForList(this.selectSidPrimaryKey,
 				new Object[] { sidIsPrincipal, sidName }, Long.class);
-
 		if (!sidIds.isEmpty()) {
 			return sidIds.get(0);
 		}
-
 		if (allowCreate) {
 			this.jdbcOperations.update(this.insertSid, sidIsPrincipal, sidName);
 			Assert.isTrue(TransactionSynchronizationManager.isSynchronizationActive(), "Transaction must be running");
 			return this.jdbcOperations.queryForObject(this.sidIdentityQuery, Long.class);
 		}
-
 		return null;
 	}
 
@@ -264,7 +255,6 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 	public void deleteAcl(ObjectIdentity objectIdentity, boolean deleteChildren) throws ChildrenExistException {
 		Assert.notNull(objectIdentity, "Object Identity required");
 		Assert.notNull(objectIdentity.getIdentifier(), "Object Identity doesn't provide an identifier");
-
 		if (deleteChildren) {
 			List<ObjectIdentity> children = findChildren(objectIdentity);
 			if (children != null) {
@@ -276,8 +266,7 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 		else {
 			if (!this.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
+				// do. We generally don't do this, in the interests of deadlock management
 				List<ObjectIdentity> children = findChildren(objectIdentity);
 				if (children != null) {
 					throw new ChildrenExistException(
@@ -384,21 +373,16 @@ public class JdbcMutableAclService extends JdbcAclService implements MutableAclS
 	 */
 	protected void updateObjectIdentity(MutableAcl acl) {
 		Long parentId = null;
-
 		if (acl.getParentAcl() != null) {
 			Assert.isInstanceOf(ObjectIdentityImpl.class, acl.getParentAcl().getObjectIdentity(),
 					"Implementation only supports ObjectIdentityImpl");
-
 			ObjectIdentityImpl oii = (ObjectIdentityImpl) acl.getParentAcl().getObjectIdentity();
 			parentId = retrieveObjectIdentityPrimaryKey(oii);
 		}
-
 		Assert.notNull(acl.getOwner(), "Owner is required in this implementation");
-
 		Long ownerSid = createOrRetrieveSidPrimaryKey(acl.getOwner(), true);
 		int count = this.jdbcOperations.update(this.updateObjectIdentity, parentId, ownerSid, acl.isEntriesInheriting(),
 				acl.getId());
-
 		if (count != 1) {
 			throw new NotFoundException("Unable to locate ACL to update");
 		}

acl/src/main/java/org/springframework/security/acls/model/AccessControlEntry.java

@@ -27,7 +27,6 @@ import java.io.Serializable;
  * </p>
  *
  * @author Ben Alex
- *
  */
 public interface AccessControlEntry extends Serializable {
 

acl/src/main/java/org/springframework/security/acls/model/AclCache.java

@@ -24,7 +24,6 @@ import org.springframework.security.acls.jdbc.JdbcAclService;
  * A caching layer for {@link JdbcAclService}.
  *
  * @author Ben Alex
- *
  */
 public interface AclCache {
 

acl/src/main/java/org/springframework/security/acls/model/AuditableAccessControlEntry.java

@@ -20,7 +20,6 @@ package org.springframework.security.acls.model;
  * Represents an ACE that provides auditing information.
  *
  * @author Ben Alex
- *
  */
 public interface AuditableAccessControlEntry extends AccessControlEntry {
 

acl/src/main/java/org/springframework/security/acls/model/AuditableAcl.java

@@ -20,7 +20,6 @@ package org.springframework.security.acls.model;
  * A mutable ACL that provides audit capabilities.
  *
  * @author Ben Alex
- *
  */
 public interface AuditableAcl extends MutableAcl {
 

acl/src/main/java/org/springframework/security/acls/model/ObjectIdentityRetrievalStrategy.java

@@ -21,7 +21,6 @@ package org.springframework.security.acls.model;
  * will be returned for a particular domain object
  *
  * @author Ben Alex
- *
  */
 public interface ObjectIdentityRetrievalStrategy {