diff --git a/core/src/main/java/org/springframework/security/access/SecurityMetadataSource.java b/core/src/main/java/org/springframework/security/access/SecurityMetadataSource.java index 89b2e1cbd3..efc018df06 100644 --- a/core/src/main/java/org/springframework/security/access/SecurityMetadataSource.java +++ b/core/src/main/java/org/springframework/security/access/SecurityMetadataSource.java @@ -35,8 +35,8 @@ public interface SecurityMetadataSource extends AopInfrastructureBean { * * @param object the object being secured * - * @return the attributes that apply to the passed in secured object. Can return either {@code null} or an - * empty collection if there are no applicable attributes. + * @return the attributes that apply to the passed in secured object. Should return an empty collection if there + * are no applicable attributes. * * @throws IllegalArgumentException if the passed object is not of a type supported by the * SecurityMetadataSource implementation diff --git a/core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java b/core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java index e84720adac..553c8f5a03 100644 --- a/core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java +++ b/core/src/main/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSource.java @@ -15,7 +15,7 @@ import org.springframework.util.ObjectUtils; /** * Automatically tries a series of method definition sources, relying on the first source of metadata - * that provides a non-null response. Provides automatic caching of the retrieved metadata. + * that provides a non-null/non-empty response. Provides automatic caching of the retrieved metadata. * * @author Ben Alex * @author Luke Taylor @@ -41,9 +41,6 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod synchronized (attributeCache) { Collection cached = attributeCache.get(cacheKey); // Check for canonical value indicating there is no config attribute, - if (cached == NULL_CONFIG_ATTRIBUTE) { - return null; - } if (cached != null) { return cached; @@ -59,13 +56,13 @@ public final class DelegatingMethodSecurityMetadataSource extends AbstractMethod } // Put it in the cache. - if (attributes == null) { + if (attributes == null || attributes.isEmpty()) { this.attributeCache.put(cacheKey, NULL_CONFIG_ATTRIBUTE); - return null; + return NULL_CONFIG_ATTRIBUTE; } if (logger.isDebugEnabled()) { - logger.debug("Adding security method [" + cacheKey + "] with attributes " + attributes); + logger.debug("Caching method [" + cacheKey + "] with attributes " + attributes); } this.attributeCache.put(cacheKey, attributes); diff --git a/core/src/test/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSourceTests.java b/core/src/test/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSourceTests.java index f23502422a..b8ecad9714 100644 --- a/core/src/test/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSourceTests.java +++ b/core/src/test/java/org/springframework/security/access/method/DelegatingMethodSecurityMetadataSourceTests.java @@ -20,7 +20,7 @@ public class DelegatingMethodSecurityMetadataSourceTests { DelegatingMethodSecurityMetadataSource mds; @Test - public void returnsNullIfDelegateReturnsNull() throws Exception { + public void returnsEmptyListIfDelegateReturnsNull() throws Exception { List sources = new ArrayList(); MethodSecurityMetadataSource delegate = mock(MethodSecurityMetadataSource.class); when(delegate.getAttributes(Matchers.any(), Matchers.any(Class.class))).thenReturn(null); @@ -29,9 +29,9 @@ public class DelegatingMethodSecurityMetadataSourceTests { assertSame(sources, mds.getMethodSecurityMetadataSources()); assertTrue(mds.getAllConfigAttributes().isEmpty()); MethodInvocation mi = new SimpleMethodInvocation(null, String.class.getMethod("toString")); - assertNull(mds.getAttributes(mi)); + assertEquals(Collections.emptyList(), mds.getAttributes(mi)); // Exercise the cached case - assertNull(mds.getAttributes(mi)); + assertEquals(Collections.emptyList(), mds.getAttributes(mi)); } @Test