tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hls...@apache.org
Subject git commit: TAP5-1979: Changing the implementation of a method after adding method advice does not work; the original implementation remains
Date Thu, 02 Aug 2012 16:56:30 GMT
Updated Branches:
  refs/heads/5.3 769033e80 -> 09707fb5f


TAP5-1979: Changing the implementation of a method after adding method advice does not work;
the original implementation remains


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/09707fb5
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/09707fb5
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/09707fb5

Branch: refs/heads/5.3
Commit: 09707fb5fe0c848aa0c35708d0eab9d90d41dae7
Parents: 769033e
Author: Howard M. Lewis Ship <hlship@apache.org>
Authored: Thu Aug 2 09:52:43 2012 -0700
Committer: Howard M. Lewis Ship <hlship@apache.org>
Committed: Thu Aug 2 09:56:16 2012 -0700

----------------------------------------------------------------------
 .../internal/plastic/MethodAdviceManager.java      |    9 +++-
 .../tapestry5/plastic/MethodAdviceTests.groovy     |   34 +++++++++++++++
 .../AdviceAndImplementationSubject.java            |    5 ++
 3 files changed, 46 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/09707fb5/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
index 9642ae1..92f4d84 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
@@ -81,8 +81,6 @@ class MethodAdviceManager
 
         newMethodName = String.format("advised$%s_%s", description.methodName, PlasticUtils.nextUID());
 
-        createNewMethod();
-
         createProceedToAdvisedMethod();
     }
 
@@ -353,8 +351,15 @@ class MethodAdviceManager
         });
     }
 
+    /**
+     * Creates a new method containing the advised method's original implementation, then
rewrites the
+     * advised method to create the MethodInvocation subclass, invoke proceed() on it, and
handle
+     * the return value and/or checked exceptions.
+     */
     void rewriteOriginalMethod()
     {
+        createNewMethod();
+
         plasticClass.pool.realize(plasticClass.className, ClassType.METHOD_INVOCATION, invocationClassNode);
 
         String fieldName = String.format("methodinvocationbundle_%s_%s", description.methodName,

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/09707fb5/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
----------------------------------------------------------------------
diff --git a/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
b/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
index 3e5e5b5..727000e 100644
--- a/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
+++ b/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
@@ -11,6 +11,7 @@ import testsubjects.SingleMethod
 import java.sql.SQLException
 
 class MethodAdviceTests extends AbstractPlasticSpecification {
+
     def "advice for a void method"() {
         setup:
 
@@ -266,7 +267,40 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
         then:
 
         1 * container.magic() >> "via context"
+    }
+
+    def "method advice added before changing method implementation does not wipe out the
method implementation"() {
+        def mgr = createMgr({ PlasticClass pc ->
+            def methods = pc.introduceInterface(MagicContainer)
+
+            methods.each { PlasticMethod m ->
+                m.addAdvice({ MethodInvocation iv ->
+                    iv.proceed()
+
+                    // Can't use a GString here as the unevaluated GString gets assigned,
causing a
+                    // ClassCastException
+                    iv.returnValue = String.format("<<%s>>", iv.returnValue)
+                } as MethodAdvice)
+
+                m.changeImplementation({ InstructionBuilder b ->
+
+                    b.loadConstant "MAGIC!"
+                    b.returnResult()
+
+                } as InstructionBuilderCallback)
+            }
+        } as PlasticClassTransformer)
+
+        when:
+
+        def o = mgr.getClassInstantiator(testsubjects.AdviceAndImplementationSubject.name).newInstance()
+
+        then:
+
+        // This demonstrates that the implementation was changed and the advice wrapped around
+        // the new implementation. Before fixing TAP5-1979 this would be "<<null>>".
 
+        o.magic() == "<<MAGIC!>>"
     }
 
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/09707fb5/plastic/src/test/java/testsubjects/AdviceAndImplementationSubject.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/AdviceAndImplementationSubject.java b/plastic/src/test/java/testsubjects/AdviceAndImplementationSubject.java
new file mode 100644
index 0000000..bd69e3f
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/AdviceAndImplementationSubject.java
@@ -0,0 +1,5 @@
+package testsubjects;
+
+public class AdviceAndImplementationSubject
+{
+}


Mime
View raw message