Merge Formatting Changes

Issue gh-8945

Author: rwinch

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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.security.acls;
 
 import java.lang.reflect.InvocationTargetException;
@@ -24,6 +25,7 @@
 import org.aopalliance.intercept.MethodInvocation;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.springframework.security.access.AuthorizationServiceException;
 import org.springframework.security.access.ConfigAttribute;
 import org.springframework.security.access.vote.AbstractAclVoter;
@@ -39,6 +41,7 @@
 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;
 
 /**
@@ -92,73 +95,57 @@
  * <p>
  * All comparisons and prefixes are case sensitive.
  *
- *
  * @author Ben Alex
  */
 public class AclEntryVoter extends AbstractAclVoter {
-	// ~ Static fields/initializers
-	// =====================================================================================
 
 	private static final Log logger = LogFactory.getLog(AclEntryVoter.class);
 
-	// ~ Instance fields
-	// ================================================================================================
+	private final AclService aclService;
+
+	private final String processConfigAttribute;
+
+	private final List<Permission> requirePermission;
 
-	private AclService aclService;
 	private ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
+
 	private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
-	private String internalMethod;
-	private String processConfigAttribute;
-	private List<Permission> requirePermission;
 
-	// ~ Constructors
-	// ===================================================================================================
+	private String internalMethod;
 
-	public AclEntryVoter(AclService aclService, String processConfigAttribute,
-			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);
 	}
 
-	// ~ Methods
-	// ========================================================================================================
-
 	/**
 	 * Optionally specifies a method of the domain object that will be used to obtain a
 	 * contained domain object. That contained domain object will be used for the ACL
 	 * evaluation. This is useful if a domain object contains a parent that an ACL
 	 * evaluation should be targeted for, instead of the child domain object (which
 	 * perhaps is being created and as such does not yet have any ACL permissions)
-	 *
 	 * @return <code>null</code> to use the domain object, or the name of a method (that
 	 * requires no arguments) that should be invoked to obtain an <code>Object</code>
 	 * which will be the domain object used for ACL evaluation
 	 */
 	protected String getInternalMethod() {
-		return internalMethod;
+		return this.internalMethod;
 	}
 
 	public void setInternalMethod(String internalMethod) {
 		this.internalMethod = internalMethod;
 	}
 
 	protected String getProcessConfigAttribute() {
-		return processConfigAttribute;
+		return this.processConfigAttribute;
 	}
 
-	public void setObjectIdentityRetrievalStrategy(
-			ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) {
-		Assert.notNull(objectIdentityRetrievalStrategy,
-				"ObjectIdentityRetrievalStrategy required");
+	public void setObjectIdentityRetrievalStrategy(ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) {
+		Assert.notNull(objectIdentityRetrievalStrategy, "ObjectIdentityRetrievalStrategy required");
 		this.objectIdentityRetrievalStrategy = objectIdentityRetrievalStrategy;
 	}
 
@@ -167,108 +154,88 @@ public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) {
 		this.sidRetrievalStrategy = sidRetrievalStrategy;
 	}
 
+	@Override
 	public boolean supports(ConfigAttribute attribute) {
-		return (attribute.getAttribute() != null)
-				&& attribute.getAttribute().equals(getProcessConfigAttribute());
+		return (attribute.getAttribute() != null) && attribute.getAttribute().equals(getProcessConfigAttribute());
 	}
 
-	public int vote(Authentication authentication, MethodInvocation object,
-			Collection<ConfigAttribute> attributes) {
-
+	@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(internalMethod)) {
-				try {
-					Class<?> clazz = domainObject.getClass();
-					Method method = clazz.getMethod(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: "
-							+ internalMethod);
-				}
-				catch (IllegalAccessException iae) {
-					logger.debug("IllegalAccessException", iae);
-
-					throw new AuthorizationServiceException(
-							"Problem invoking internalMethod: " + internalMethod
-									+ " for object: " + domainObject);
-				}
-				catch (InvocationTargetException ite) {
-					logger.debug("InvocationTargetException", ite);
-
-					throw new AuthorizationServiceException(
-							"Problem invoking internalMethod: " + internalMethod
-									+ " for object: " + domainObject);
-				}
+			if (StringUtils.hasText(this.internalMethod)) {
+				domainObject = invokeInternalMethod(domainObject);
 			}
 
 			// Obtain the OID applicable to the domain object
-			ObjectIdentity objectIdentity = objectIdentityRetrievalStrategy
-					.getObjectIdentity(domainObject);
+			ObjectIdentity objectIdentity = this.objectIdentityRetrievalStrategy.getObjectIdentity(domainObject);
 
 			// Obtain the SIDs applicable to the principal
-			List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
+			List<Sid> sids = this.sidRetrievalStrategy.getSids(authentication);
 
 			Acl acl;
 
 			try {
 				// Lookup only ACLs for SIDs we're interested in
-				acl = aclService.readAclById(objectIdentity, sids);
+				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(requirePermission, sids, false)) {
-					if (logger.isDebugEnabled()) {
-						logger.debug("Voting to grant access");
-					}
-
+				if (acl.isGranted(this.requirePermission, sids, false)) {
+					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;
 			}
 		}
 
 		// No configuration attribute matched, so abstain
 		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

@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.security.acls;
 
 import java.util.ArrayList;
@@ -21,6 +22,8 @@
 
 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;
@@ -38,45 +41,42 @@
  * @since 3.1
  */
 public class AclPermissionCacheOptimizer implements PermissionCacheOptimizer {
+
 	private final Log logger = LogFactory.getLog(getClass());
+
 	private final AclService aclService;
+
 	private SidRetrievalStrategy sidRetrievalStrategy = new SidRetrievalStrategyImpl();
+
 	private ObjectIdentityRetrievalStrategy oidRetrievalStrategy = new ObjectIdentityRetrievalStrategyImpl();
 
 	public AclPermissionCacheOptimizer(AclService aclService) {
 		this.aclService = aclService;
 	}
 
+	@Override
 	public void cachePermissionsFor(Authentication authentication, Collection<?> objects) {
 		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 = oidRetrievalStrategy.getObjectIdentity(domainObject);
-			oidsToCache.add(oid);
-		}
-
-		List<Sid> sids = sidRetrievalStrategy.getSids(authentication);
-
-		if (logger.isDebugEnabled()) {
-			logger.debug("Eagerly loading Acls for " + oidsToCache.size() + " objects");
 		}
-
-		aclService.readAclsById(oidsToCache, sids);
+		List<Sid> sids = this.sidRetrievalStrategy.getSids(authentication);
+		this.logger.debug(LogMessage.of(() -> "Eagerly loading Acls for " + oidsToCache.size() + " objects"));
+		this.aclService.readAclsById(oidsToCache, sids);
 	}
 
-	public void setObjectIdentityRetrievalStrategy(
-			ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) {
+	public void setObjectIdentityRetrievalStrategy(ObjectIdentityRetrievalStrategy objectIdentityRetrievalStrategy) {
 		this.oidRetrievalStrategy = objectIdentityRetrievalStrategy;
 	}
 
 	public void setSidRetrievalStrategy(SidRetrievalStrategy sidRetrievalStrategy) {
 		this.sidRetrievalStrategy = sidRetrievalStrategy;
 	}
+
 }