WW-4800 Executes aspects when chaining AOPed actions

Author: yasserzamani

core/src/main/java/com/opensymphony/xwork2/interceptor/ChainingInterceptor.java

@@ -163,10 +163,11 @@ private void copyStack(ActionInvocation invocation, CompoundRoot root) {
         for (Object object : list) {
             if (shouldCopy(object)) {
                 Object action = invocation.getAction();
+                Class<?> editable = null;
                 if(ProxyUtil.isSpringAopProxy(action)) {
-                    action = ProxyUtil.getSpringUltimateTargetObject(action);
+                    editable = ProxyUtil.springUltimateTargetClass(action);
                 }
-                reflectionProvider.copy(object, action, ctxMap, prepareExcludes(), includes);
+                reflectionProvider.copy(object, action, ctxMap, prepareExcludes(), includes, editable);
             }
         }
     }

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlReflectionProvider.java

@@ -70,7 +70,12 @@ public PropertyDescriptor getPropertyDescriptor(Class targetClass,
 
     public void copy(Object from, Object to, Map<String, Object> context,
             Collection<String> exclusions, Collection<String> inclusions) {
-        ognlUtil.copy(from, to, context, exclusions, inclusions);
+        copy(from, to, context, exclusions, inclusions, null);
+    }
+
+    public void copy(Object from, Object to, Map<String, Object> context,
+                     Collection<String> exclusions, Collection<String> inclusions, Class<?> editable) {
+        ognlUtil.copy(from, to, context, exclusions, inclusions, editable);
     }
 
     public Object getRealTarget(String property, Map<String, Object> context, Object root)

core/src/main/java/com/opensymphony/xwork2/ognl/OgnlUtil.java

@@ -450,6 +450,24 @@ private void checkSimpleMethod(Object tree, Map<String, Object> context) throws
      *                   note if exclusions AND inclusions are supplied and not null nothing will get copied.
      */
     public void copy(final Object from, final Object to, final Map<String, Object> context, Collection<String> exclusions, Collection<String> inclusions) {
+        copy(from, to, context, exclusions, inclusions, null);
+    }
+
+    /**
+     * Copies the properties in the object "from" and sets them in the object "to"
+     * only setting properties defined in the given "editable" class (or interface)
+     * using specified type converter, or {@link com.opensymphony.xwork2.conversion.impl.XWorkConverter} if none
+     * is specified.
+     *
+     * @param from       the source object
+     * @param to         the target object
+     * @param context    the action context we're running under
+     * @param exclusions collection of method names to excluded from copying ( can be null)
+     * @param inclusions collection of method names to included copying  (can be null)
+     *                   note if exclusions AND inclusions are supplied and not null nothing will get copied.
+     * @param editable the class (or interface) to restrict property setting to
+     */
+    public void copy(final Object from, final Object to, final Map<String, Object> context, Collection<String> exclusions, Collection<String> inclusions, Class<?> editable) {
         if (from == null || to == null) {
             LOG.warn("Attempting to copy from or to a null source. This is illegal and is bein skipped. This may be due to an error in an OGNL expression, action chaining, or some other event.");
             return;
@@ -466,7 +484,12 @@ public void copy(final Object from, final Object to, final Map<String, Object> c
 
         try {
             fromPds = getPropertyDescriptors(from);
-            toPds = getPropertyDescriptors(to);
+            if (editable != null) {
+                toPds = getPropertyDescriptors(editable);
+            }
+            else {
+                toPds = getPropertyDescriptors(to);
+            }
         } catch (IntrospectionException e) {
             LOG.error("An error occurred", e);
             return;

core/src/main/java/com/opensymphony/xwork2/util/ProxyUtil.java

@@ -29,31 +29,32 @@
 public class ProxyUtil {
     private static final String SPRING_ADVISED_CLASS_NAME = "org.springframework.aop.framework.Advised";
     private static final String SPRING_SPRINGPROXY_CLASS_NAME = "org.springframework.aop.SpringProxy";
+    private static final String SPRING_SINGLETONTARGETSOURCE_CLASS_NAME = "org.springframework.aop.target.SingletonTargetSource";
+    private static final String SPRING_TARGETCLASSAWARE_CLASS_NAME = "org.springframework.aop.TargetClassAware";
 
     /**
-     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
-     * object, unwrapping not only a top-level proxy but also any number of
-     * nested proxies.
-     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
-     * nested proxies will be returned; otherwise, the {@code candidate}
-     * will be returned <em>as is</em>.
-     * @param candidate the instance to check (potentially a Spring AOP proxy;
-     * never {@code null})
-     * @return the target object or the {@code candidate} (never {@code null})
-     * @throws IllegalStateException if an error occurs while unwrapping a proxy
+     * Determine the ultimate target class of the given spring bean instance, traversing
+     * not only a top-level spring proxy but any number of nested spring proxies as well &mdash;
+     * as long as possible without side effects, that is, just for singleton targets.
+     * @param candidate the instance to check (might be a spring AOP proxy)
+     * @return the ultimate target class (or the plain class of the given
+     * object as fallback; never {@code null})
      */
-    public static <T> T getSpringUltimateTargetObject(Object candidate) {
-        try {
-            if (isSpringAopProxy(candidate) && implementsInterface(candidate.getClass(), SPRING_ADVISED_CLASS_NAME)) {
-                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
-                Object target = MethodUtils.invokeMethod(targetSource, "getTarget");
-                return getSpringUltimateTargetObject(target);
+    public static Class<?> springUltimateTargetClass(Object candidate) {
+        Object current = candidate;
+        Class<?> result = null;
+        while (null != current && implementsInterface(current.getClass(), SPRING_TARGETCLASSAWARE_CLASS_NAME)) {
+            try {
+                result = (Class<?>) MethodUtils.invokeMethod(current, "getTargetClass");
+            } catch (Throwable ignored) {
             }
+            current = getSingletonTarget(current);
         }
-        catch (Throwable ex) {
-            throw new IllegalStateException("Failed to unwrap proxied object", ex);
+        if (result == null) {
+            Class<?> clazz = candidate.getClass();
+            result = (isCglibProxyClass(clazz) ? clazz.getSuperclass() : candidate.getClass());
         }
-        return (T) candidate;
+        return result;
     }
 
     /**
@@ -66,6 +67,26 @@ public static boolean isSpringAopProxy(Object object) {
                 || isCglibProxyClass(clazz)));
     }
 
+    /**
+     * Obtain the singleton target object behind the given spring proxy, if any.
+     * @param candidate the (potential) spring proxy to check
+     * @return the singleton target object, or {@code null} in any other case
+     * (not a spring proxy, not an existing singleton target)
+     */
+    private static Object getSingletonTarget(Object candidate) {
+        try {
+            if (implementsInterface(candidate.getClass(), SPRING_ADVISED_CLASS_NAME)) {
+                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
+                if (implementsInterface(targetSource.getClass(), SPRING_SINGLETONTARGETSOURCE_CLASS_NAME)) {
+                    return MethodUtils.invokeMethod(targetSource, "getTarget");
+                }
+            }
+        } catch (Throwable ignored) {
+        }
+
+        return null;
+    }
+
     /**
      * Check whether the specified class is a CGLIB-generated class.
      * @param clazz the class to check
@@ -81,7 +102,7 @@ private static boolean isCglibProxyClass(Class<?> clazz) {
      */
     private static boolean implementsInterface(Class<?> clazz, String ifaceClassName) {
         try {
-            Class ifaceClass = ClassLoaderUtil.loadClass(ifaceClassName, ProxyUtil.class);
+            Class<?> ifaceClass = ClassLoaderUtil.loadClass(ifaceClassName, ProxyUtil.class);
             return ifaceClass.isAssignableFrom(clazz);
         } catch (ClassNotFoundException e) {
             return false;

core/src/main/java/com/opensymphony/xwork2/util/reflection/ReflectionProvider.java

@@ -72,7 +72,23 @@ public interface ReflectionProvider {
      *                   note if exclusions AND inclusions are supplied and not null nothing will get copied.
      */
     void copy(Object from, Object to, Map<String, Object> context, Collection<String> exclusions, Collection<String> inclusions);
-    
+
+    /**
+     * Copies the properties in the object "from" and sets them in the object "to"
+     * only setting properties defined in the given "editable" class (or interface)
+     * using specified type converter, or {@link com.opensymphony.xwork2.conversion.impl.XWorkConverter} if none
+     * is specified.
+     *
+     * @param from       the source object
+     * @param to         the target object
+     * @param context    the action context we're running under
+     * @param exclusions collection of method names to excluded from copying ( can be null)
+     * @param inclusions collection of method names to included copying  (can be null)
+     *                   note if exclusions AND inclusions are supplied and not null nothing will get copied.
+     * @param editable the class (or interface) to restrict property setting to
+     */
+    void copy(Object from, Object to, Map<String, Object> context, Collection<String> exclusions, Collection<String> inclusions, Class<?> editable);
+
     /**
      * Looks for the real target with the specified property given a root Object which may be a
      * CompoundRoot.

core/src/test/java/com/opensymphony/xwork2/ognl/OgnlUtilTest.java

@@ -209,6 +209,32 @@ public void testIncudeExcludes() {
 
     }
 
+    public void testCopyEditable() {
+        Foo foo1 = new Foo();
+        Foo foo2 = new Foo();
+
+        Map<String, Object> context = ognlUtil.createDefaultContext(foo1);
+
+        Calendar cal = Calendar.getInstance();
+        cal.clear();
+        cal.set(Calendar.MONTH, Calendar.MAY);
+        cal.set(Calendar.DAY_OF_MONTH, 29);
+        cal.set(Calendar.YEAR, 2017);
+
+        foo1.setTitle("blah");
+        foo1.setNumber(1);
+        foo1.setPoints(new long[]{1, 2, 3});
+        foo1.setBirthday(cal.getTime());
+        foo1.setUseful(false);
+
+        ognlUtil.copy(foo1, foo2, context, null, null, Bar.class);
+
+        assertEquals(foo1.getTitle(), foo2.getTitle());
+        assertEquals(0, foo2.getNumber());
+        assertNull(foo2.getPoints());
+        assertNull(foo2.getBirthday());
+    }
+
 
     public void testCopyUnevenObjects() {
         Foo foo = new Foo();

core/src/test/java/com/opensymphony/xwork2/spring/ActionsFromSpringTest.java

@@ -84,14 +84,20 @@ public void testChainingProxiedActions() throws Exception {
 
         proxy.execute();
 
-        TestSubBean chaintoAOPedAction = (TestSubBean) appContext.getBean("pointcutted-test-sub-bean");
-        TestSubBean aspectState = (TestSubBean) appContext.getBean("aspected-test-sub-bean");
+        // check if AOP works
+        TestAspect aspectState = (TestAspect) appContext.getBean("test-aspect");
+        // chainedAction.actionMethodName sets name then chainedAction.getCount sets count
+        // then chaintoAction.setCount sets count2 then chainedAction.getName sets name again
+        // then chaintoAction.actionMethodName sets issueId of the aspect object.
+        assertEquals("setName(WW-4105)-setCount(1)-setCount2(1)-setName(WW-4105)-setIssueId(WW-4105)-", aspectState.log);
+        assertEquals(aspectState.getName(), aspectState.getIssueId());
+        assertEquals("WW-4105", aspectState.getIssueId());
+        assertEquals(aspectState.getCount(), aspectState.getCount2());
+        assertEquals(1, aspectState.getCount());
 
-        assertEquals(1, chaintoAOPedAction.getCount()); //check if chain
+        // check if chain works
+        TestSubBean chaintoAOPedAction = (TestSubBean) appContext.getBean("pointcutted-test-sub-bean");
+        assertEquals(1, chaintoAOPedAction.getCount());
         assertEquals("WW-4105", chaintoAOPedAction.getName());
-        assertNotNull(aspectState.getIssueId());   //and AOP proxied actions
-        assertNotNull(aspectState.getName());
-        assertEquals(aspectState.getName(), aspectState.getIssueId());
-        assertEquals("WW-4105", aspectState.getIssueId());   //work together without any problem
     }
 }

core/src/test/java/com/opensymphony/xwork2/spring/SpringProxyUtilTest.java

@@ -53,33 +53,33 @@ public void testIsSpringAopProxy() throws Exception {
         Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean");
         assertTrue(ProxyUtil.isSpringAopProxy(pointcuttedTestSubBean));
 
-        Object aspectedTestSubBean = appContext.getBean("aspected-test-sub-bean");
-        assertFalse(ProxyUtil.isSpringAopProxy(aspectedTestSubBean));
+        Object testAspect = appContext.getBean("test-aspect");
+        assertFalse(ProxyUtil.isSpringAopProxy(testAspect));
     }
 
-    public void testGetSpringUltimateTargetObject() throws Exception {
+    public void testSpringUltimateTargetClass() throws Exception {
         Object simpleAction = appContext.getBean("simple-action");
-        Object simpleActionUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(simpleAction);
-        assertEquals(simpleAction, simpleActionUltimateTargetObject);
+        Class<?> simpleActionUltimateTargetClass = ProxyUtil.springUltimateTargetClass(simpleAction);
+        assertEquals(SimpleAction.class, simpleActionUltimateTargetClass);
 
         Object proxiedAction = appContext.getBean("proxied-action");
-        Object proxiedActionUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(proxiedAction);
-        assertEquals(SimpleAction.class, proxiedActionUltimateTargetObject.getClass());
+        Class<?> proxiedActionUltimateTargetClass = ProxyUtil.springUltimateTargetClass(proxiedAction);
+        assertEquals(SimpleAction.class, proxiedActionUltimateTargetClass);
 
         Object autoProxiedAction = appContext.getBean("auto-proxied-action");
-        Object autoProxiedActionUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(autoProxiedAction);
-        assertEquals(SimpleAction.class, autoProxiedActionUltimateTargetObject.getClass());
+        Class<?> autoProxiedActionUltimateTargetClass = ProxyUtil.springUltimateTargetClass(autoProxiedAction);
+        assertEquals(SimpleAction.class, autoProxiedActionUltimateTargetClass);
 
         Object pointcuttedTestBean = appContext.getBean("pointcutted-test-bean");
-        Object pointcuttedTestBeanUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(pointcuttedTestBean);
-        assertEquals(TestBean.class, pointcuttedTestBeanUltimateTargetObject.getClass());
+        Class<?> pointcuttedTestBeanUltimateTargetClass = ProxyUtil.springUltimateTargetClass(pointcuttedTestBean);
+        assertEquals(TestBean.class, pointcuttedTestBeanUltimateTargetClass);
 
         Object pointcuttedTestSubBean = appContext.getBean("pointcutted-test-sub-bean");
-        Object pointcuttedTestSubBeanUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(pointcuttedTestSubBean);
-        assertEquals(TestSubBean.class, pointcuttedTestSubBeanUltimateTargetObject.getClass());
+        Class<?> pointcuttedTestSubBeanUltimateTargetClass = ProxyUtil.springUltimateTargetClass(pointcuttedTestSubBean);
+        assertEquals(TestSubBean.class, pointcuttedTestSubBeanUltimateTargetClass);
 
-        Object aspectedTestSubBean = appContext.getBean("aspected-test-sub-bean");
-        Object aspectedTestSubBeanUltimateTargetObject = ProxyUtil.getSpringUltimateTargetObject(aspectedTestSubBean);
-        assertEquals(aspectedTestSubBean, aspectedTestSubBeanUltimateTargetObject);
+        Object testAspect = appContext.getBean("test-aspect");
+        Class<?> testAspectUltimateTargetClass = ProxyUtil.springUltimateTargetClass(testAspect);
+        assertEquals(TestAspect.class, testAspectUltimateTargetClass);
     }
 }

core/src/test/java/com/opensymphony/xwork2/spring/TestAspect.java

@@ -0,0 +1,46 @@
+package com.opensymphony.xwork2.spring;
+
+public class TestAspect {
+	protected String log = "";
+	
+	private String issueId;
+	private int count;
+	private String name;
+	private int count2;
+
+	String getIssueId() {
+		return issueId;
+	}
+
+	public void setIssueId(String issueId) {
+		log = log + "setIssueId(" + issueId + ")-";
+		this.issueId = issueId;
+	}
+
+	public int getCount() {
+		return count;
+	}
+
+	public void setCount(int count) {
+		log = log + "setCount(" + count + ")-";
+		this.count = count;
+	}
+
+	public String getName() {
+		return name;
+	}
+
+	public void setName(String name) {
+		log = log + "setName(" + name + ")-";
+		this.name = name;
+	}
+
+	int getCount2() {
+		return count2;
+	}
+
+	public void setCount2(int count2) {
+		log = log + "setCount2(" + count2 + ")-";
+		this.count2 = count2;
+	}
+}

core/src/test/resources/com/opensymphony/xwork2/spring/actionContext-spring.xml

@@ -51,17 +51,25 @@
     <bean id="pointcutted-test-sub-bean" class="com.opensymphony.xwork2.TestSubBean">
         <property name="issueId"><value>WW-4105</value></property>
     </bean>
-    <bean id="aspected-test-sub-bean" class="com.opensymphony.xwork2.TestSubBean" />
+    <bean id="test-aspect" class="com.opensymphony.xwork2.spring.TestAspect" />
     <aop:config>
-        <aop:aspect id="myAspect" ref="aspected-test-sub-bean">
+        <aop:aspect id="myAspect" ref="test-aspect">
             <aop:pointcut id="testBeanGetName"
                 expression="execution(String com.opensymphony.xwork2.TestBean.getName()) and bean(pointcutted-test-bean)" />
             <aop:after-returning pointcut-ref="testBeanGetName"
-                method="setIssueId" returning="issueId" />
+                method="setName" returning="name" />
+            <aop:pointcut id="testBeanGetCount"
+                expression="execution(int com.opensymphony.xwork2.TestBean.getCount()) and bean(pointcutted-test-bean)" />
+            <aop:after-returning pointcut-ref="testBeanGetCount"
+                method="setCount" returning="count" />
             <aop:pointcut id="testSubBeanGetIssueId"
                 expression="execution(String com.opensymphony.xwork2.TestSubBean.getIssueId()) and bean(pointcutted-test-sub-bean)" />
             <aop:after-returning pointcut-ref="testSubBeanGetIssueId"
-                method="setName" returning="name" />
+                method="setIssueId" returning="issueId" />
+            <aop:pointcut id="testBeanSetCount"
+                expression="execution(void com.opensymphony.xwork2.TestBean.setCount(int)) and args(count2) and bean(pointcutted-test-sub-bean)" />
+            <aop:before pointcut-ref="testBeanSetCount"
+                method="setCount2" arg-names="count2"/>
         </aop:aspect>
     </aop:config>
 </beans>