Make MethodDefinitionMap query interfaces defined by secure objects, to properly support MethodDefinitionSourceAdvisor.

core/src/main/java/org/acegisecurity/intercept/method/MethodDefinitionMap.java

@@ -15,6 +15,7 @@
 
 package net.sf.acegisecurity.intercept.method;
 
+import net.sf.acegisecurity.ConfigAttribute;
 import net.sf.acegisecurity.ConfigAttributeDefinition;
 
 import org.aopalliance.intercept.MethodInvocation;
@@ -34,6 +35,32 @@ import java.util.Map;
 /**
  * Stores a {@link ConfigAttributeDefinition} for each method signature defined
  * in a bean context.
+ * 
+ * <p>
+ * For consistency with {@link MethodDefinitionAttributes} as well as support
+ * for {@link MethodDefinitionSourceAdvisor}, this implementation will return
+ * a <code>ConfigAttributeDefinition</code> containing all configuration
+ * attributes defined against:
+ * 
+ * <ul>
+ * <li>
+ * The method-specific attributes defined for the intercepted method of the
+ * intercepted class.
+ * </li>
+ * <li>
+ * The method-specific attributes defined by any explicitly implemented
+ * interface if that interface contains a method signature matching that of
+ * the intercepted method.
+ * </li>
+ * </ul>
+ * </p>
+ * 
+ * <p>
+ * In general you should therefore define the <b>interface method</b>s of your
+ * secure objects, not the implementations. For example, define
+ * <code>com.company.Foo.findAll=ROLE_TEST</code> but not
+ * <code>com.company.FooImpl.findAll=ROLE_TEST</code>.
+ * </p>
  *
  * @author Ben Alex
  * @version $Id$
@@ -53,10 +80,28 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
 
     //~ Methods ================================================================
 
+    /**
+     * Obtains the configuration attributes explicitly defined against this
+     * bean. This method will not return implicit configuration attributes
+     * that may be returned by {@link #lookupAttributes(MethodInvocation)} as
+     * it does not have access to a method invocation at this time.
+     *
+     * @return the attributes explicitly defined against this bean
+     */
     public Iterator getConfigAttributeDefinitions() {
         return methodMap.values().iterator();
     }
 
+    /**
+     * Obtains the number of configuration attributes explicitly defined
+     * against this bean. This method will not return implicit configuration
+     * attributes that may be returned by {@link
+     * #lookupAttributes(MethodInvocation)} as it does not have access to a
+     * method invocation at this time.
+     *
+     * @return the number of configuration attributes explicitly defined
+     *         against this bean
+     */
     public int getMethodMapSize() {
         return this.methodMap.size();
     }
@@ -165,7 +210,38 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
     }
 
     protected ConfigAttributeDefinition lookupAttributes(MethodInvocation mi) {
-        return (ConfigAttributeDefinition) this.methodMap.get(mi.getMethod());
+        ConfigAttributeDefinition definition = new ConfigAttributeDefinition();
+
+        // Add attributes explictly defined for this method invocation
+        ConfigAttributeDefinition directlyAssigned = (ConfigAttributeDefinition) this.methodMap
+            .get(mi.getMethod());
+        merge(definition, directlyAssigned);
+
+        // Add attributes explicitly defined for this method invocation's interfaces
+        Class[] interfaces = mi.getMethod().getDeclaringClass().getInterfaces();
+
+        for (int i = 0; i < interfaces.length; i++) {
+            Class clazz = interfaces[i];
+
+            try {
+                // Look for the method on the current interface
+                Method interfaceMethod = clazz.getDeclaredMethod(mi.getMethod()
+                                                                   .getName(),
+                        mi.getMethod().getParameterTypes());
+                ConfigAttributeDefinition interfaceAssigned = (ConfigAttributeDefinition) this.methodMap
+                    .get(interfaceMethod);
+                merge(definition, interfaceAssigned);
+            } catch (Exception e) {
+                // skip this interface
+            }
+        }
+
+        // Return null if empty, as per abstract superclass contract
+        if (definition.size() == 0) {
+            return null;
+        } else {
+            return definition;
+        }
     }
 
     /**
@@ -184,4 +260,17 @@ public class MethodDefinitionMap extends AbstractMethodDefinitionSource {
         || (mappedName.startsWith("*")
         && methodName.endsWith(mappedName.substring(1, mappedName.length())));
     }
+
+    private void merge(ConfigAttributeDefinition definition,
+        ConfigAttributeDefinition toMerge) {
+        if (toMerge == null) {
+            return;
+        }
+
+        Iterator attribs = toMerge.getConfigAttributes();
+
+        while (attribs.hasNext()) {
+            definition.addConfigAttribute((ConfigAttribute) attribs.next());
+        }
+    }
 }

core/src/test/java/org/acegisecurity/intercept/method/MethodDefinitionSourceEditorTests.java

@@ -91,6 +91,34 @@ public class MethodDefinitionSourceEditorTests extends TestCase {
         }
     }
 
+    public void testConcreteClassInvocationsAlsoReturnDefinitionsAgainstInterface()
+        throws Exception {
+        MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
+        editor.setAsText(
+            "net.sf.acegisecurity.ITargetObject.makeLower*=ROLE_FROM_INTERFACE\r\nnet.sf.acegisecurity.ITargetObject.makeUpper*=ROLE_FROM_INTERFACE\r\nnet.sf.acegisecurity.TargetObject.makeUpper*=ROLE_FROM_IMPLEMENTATION");
+
+        MethodDefinitionMap map = (MethodDefinitionMap) editor.getValue();
+        assertEquals(3, map.getMethodMapSize());
+
+        ConfigAttributeDefinition returnedMakeLower = map.getAttributes(new MockMethodInvocation(
+                    TargetObject.class, "makeLowerCase",
+                    new Class[] {String.class}));
+        ConfigAttributeDefinition expectedMakeLower = new ConfigAttributeDefinition();
+        expectedMakeLower.addConfigAttribute(new SecurityConfig(
+                "ROLE_FROM_INTERFACE"));
+        assertEquals(expectedMakeLower, returnedMakeLower);
+
+        ConfigAttributeDefinition returnedMakeUpper = map.getAttributes(new MockMethodInvocation(
+                    TargetObject.class, "makeUpperCase",
+                    new Class[] {String.class}));
+        ConfigAttributeDefinition expectedMakeUpper = new ConfigAttributeDefinition();
+        expectedMakeUpper.addConfigAttribute(new SecurityConfig(
+                "ROLE_FROM_IMPLEMENTATION"));
+        expectedMakeUpper.addConfigAttribute(new SecurityConfig(
+                "ROLE_FROM_INTERFACE"));
+        assertEquals(expectedMakeUpper, returnedMakeUpper);
+    }
+
     public void testEmptyStringReturnsEmptyMap() {
         MethodDefinitionSourceEditor editor = new MethodDefinitionSourceEditor();
         editor.setAsText("");