You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2014/09/02 01:18:46 UTC
[6/6] git commit: TAP5-1736: Overriding a base class abstract event
handler method causes the sub-class method to be invoked twice
TAP5-1736: Overriding a base class abstract event handler method causes the sub-class method to be invoked twice
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/df93764a
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/df93764a
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/df93764a
Branch: refs/heads/master
Commit: df93764ad8b5ecdda365f2c205e652692993e084
Parents: 8a10194
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Sep 1 16:18:48 2014 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Sep 1 16:18:48 2014 -0700
----------------------------------------------------------------------
.../internal/plastic/InheritanceData.java | 51 ++++++++++++--------
.../internal/plastic/PlasticClassImpl.java | 11 +++--
.../internal/plastic/PlasticMethodImpl.java | 8 ++-
.../apache/tapestry5/plastic/PlasticMethod.java | 21 +++++---
.../tapestry5/plastic/MethodAdviceTests.groovy | 39 +++++++++++++--
.../java/testsubjects/AbstractPlaceholder.java | 6 +++
.../test/java/testsubjects/PlaceholderImpl.java | 8 +++
.../integration/app1/CoreBehaviorsTests.java | 9 +++-
.../base/OverrideEventHandlerDemoBaseClass.java | 26 ++++++++++
.../tapestry5/integration/app1/pages/Index.java | 2 +
.../app1/pages/OverrideEventHandlerDemo.java | 15 ++++++
.../base/OverrideEventHandlerDemoBaseClass.tml | 13 +++++
12 files changed, 171 insertions(+), 38 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
index ab7a17f..44c1734 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InheritanceData.java
@@ -1,5 +1,3 @@
-// Copyright 2011 The Apache Software Foundation
-//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
@@ -52,49 +50,58 @@ public class InheritanceData
* Returns a new MethodBundle that represents the methods of a child class
* of this bundle. The returned bundle will always be {@linkplain #isTransformed() transformed}.
*
- * @param childClassName name of subclass
* @return new method bundle
*/
- public InheritanceData createChild(String childClassName)
+ public InheritanceData createChild()
{
return new InheritanceData(this);
}
/**
- * Adds a new instance method. Only non-private, non-abstract methods should be added (that is, methods which might
+ * Adds a new instance method. Only non-private methods should be added (that is, methods which might
* be overridden in subclasses). This can later be queried to see if any base class implements the method.
*
- * @param name name of method
- * @param desc method descriptor
+ * @param name
+ * name of method
+ * @param desc
+ * describes the parameters and return value of the method
*/
public void addMethod(String name, String desc)
{
- String value = toValue(name, desc);
-
- methods.add(value);
+ methods.add(toValue(name, desc));
methodNames.add(name);
}
+
/**
- * Returns true if a transformed parent class contains the indicated method.
+ * Returns true if a transformed parent class contains an implementation of, or abstract placeholder for, the method.
*
- * @param name method name
- * @param desc method descriptor
- * @return the <em>internal name</em> of the implementing base class for this method,
- * or null if no base class implements the method
+ * @param name
+ * method name
+ * @param desc
+ * method descriptor
+ * @return true if a base class implements the method (including abstract methods)
*/
public boolean isImplemented(String name, String desc)
{
return checkForMethod(toValue(name, desc));
}
-
private boolean checkForMethod(String value)
{
- if (methods.contains(value))
- return true;
+ InheritanceData cursor = this;
- return parent == null ? false : parent.checkForMethod(value);
+ while (cursor != null)
+ {
+ if (cursor.methods.contains(value))
+ {
+ return true;
+ }
+
+ cursor = cursor.parent;
+ }
+
+ return false;
}
/**
@@ -118,8 +125,10 @@ public class InheritanceData
return false;
}
- public void addInterface(String name) {
- if (!interfaceNames.contains(name)) {
+ public void addInterface(String name)
+ {
+ if (!interfaceNames.contains(name))
+ {
interfaceNames.add(name);
}
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
index e4e3707..c07bcaf 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
@@ -198,7 +198,8 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
pool.createAnnotationAccess(superClassName));
this.parentInheritanceData = parentInheritanceData;
- inheritanceData = parentInheritanceData.createChild(className);
+
+ inheritanceData = parentInheritanceData.createChild();
for (String interfaceName : classNode.interfaces)
{
@@ -233,7 +234,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
*/
if (Modifier.isStatic(node.access))
{
- if (!Modifier.isPrivate(node.access))
+ if (isInheritableMethod(node))
{
inheritanceData.addMethod(node.name, node.desc);
}
@@ -782,8 +783,10 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
methodNames.add(methodNode.name);
- if (!Modifier.isPrivate(methodNode.access))
+ if (isInheritableMethod(methodNode))
+ {
inheritanceData.addMethod(methodNode.name, methodNode.desc);
+ }
}
private PlasticMethod createNewMethod(MethodDescription description)
@@ -1183,7 +1186,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
private boolean isInheritableMethod(MethodNode node)
{
- return (node.access & (ACC_ABSTRACT | ACC_PRIVATE)) == 0;
+ return !Modifier.isPrivate(node.access);
}
@Override
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
index 980bd84..bc3c14c 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticMethodImpl.java
@@ -1,5 +1,3 @@
-// Copyright 2011 The Apache Software Foundation
-//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
@@ -85,6 +83,12 @@ class PlasticMethodImpl extends PlasticMember implements PlasticMethod, Comparab
}
@Override
+ public boolean isAbstract()
+ {
+ return Modifier.isAbstract(node.access);
+ }
+
+ @Override
public String getMethodIdentifier()
{
plasticClass.check();
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticMethod.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticMethod.java b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticMethod.java
index f77b441..7ec9342 100644
--- a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticMethod.java
+++ b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticMethod.java
@@ -1,5 +1,3 @@
-// Copyright 2011 The Apache Software Foundation
-//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
@@ -48,7 +46,8 @@ public interface PlasticMethod extends AnnotationAccess
* If the method has advice, the advice is <em>not</em> lost but will instead wrap around the new method
* implementation.
*
- * @param callback passed the InstructionBuilder so that an implementation of the method can be created
+ * @param callback
+ * passed the InstructionBuilder so that an implementation of the method can be created
* @return this method, for further configuration
*/
PlasticMethod changeImplementation(InstructionBuilderCallback callback);
@@ -68,7 +67,8 @@ public interface PlasticMethod extends AnnotationAccess
* Note additionally that a recursive method invocation will still invoke the MethodAdvice chain on each recursive
* call (this is an intended side-effect of copying the exact bytecode of the method implementation.
*
- * @param advice advice to add to the method
+ * @param advice
+ * advice to add to the method
* @return this method, for further configuration
*/
PlasticMethod addAdvice(MethodAdvice advice);
@@ -78,7 +78,8 @@ public interface PlasticMethod extends AnnotationAccess
* correct interface (or extend the correct class). The original implementation of the method is lost,
* though (as with {@link #changeImplementation(InstructionBuilderCallback)}), method advice is retained.
*
- * @param field to delegate to
+ * @param field
+ * to delegate to
* @return this method, for further configuration
*/
PlasticMethod delegateTo(PlasticField field);
@@ -88,7 +89,8 @@ public interface PlasticMethod extends AnnotationAccess
* is dynamically computed by another method of the class. The method should take no parameters
* and must not return null, or throw any exceptions not compatible with the method being proxied.
*
- * @param method to provide the dynamic delegate
+ * @param method
+ * to provide the dynamic delegate
* @return this method, for further configuration
*/
PlasticMethod delegateTo(PlasticMethod method);
@@ -107,6 +109,13 @@ public interface PlasticMethod extends AnnotationAccess
boolean isOverride();
/**
+ * Returns true if the method is abstract.
+ *
+ * @since 5.4
+ */
+ boolean isAbstract();
+
+ /**
* Returns a short identifier for the method that includes the class name, the method name,
* and the types of all parameters. This is often used when producing debugging output
* about the method.
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/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 727000e..bd20978 100644
--- a/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
+++ b/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
@@ -109,7 +109,7 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
def "setting return value clears checked exceptions"() {
def mgr = createMgr({ PlasticClass pc ->
- findMethod(pc, "maybeThrow").addAdvice({ MethodInvocation mi ->
+ findMethod(pc, "maybeThrow").addAdvice({ MethodInvocation mi ->
mi.proceed()
@@ -221,7 +221,9 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
})
} as PlasticClassTransformer)
- if (false) { enableBytecodeDebugging(mgr) }
+ if (false) {
+ enableBytecodeDebugging(mgr)
+ }
def o = mgr.getClassInstantiator(testsubjects.FieldConduitInsideAdvisedMethod.name).with(MagicContainer, container).newInstance()
@@ -256,7 +258,9 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
})
} as PlasticClassTransformer)
- if (false) { enableBytecodeDebugging(mgr) }
+ if (false) {
+ enableBytecodeDebugging(mgr)
+ }
def o = mgr.getClassInstantiator(testsubjects.FieldConduitAdvisedMethodComplexCase.name).with(MagicContainer, container).newInstance()
@@ -303,4 +307,33 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
o.magic() == "<<MAGIC!>>"
}
+ def "abstract methods are identified as such by PlasticMethod"() {
+
+ setup:
+
+ PlasticClass pc = mgr.getPlasticClass(testsubjects.AbstractPlaceholder.name)
+
+ PlasticMethod m = findMethod pc, "placeholder"
+
+ expect:
+
+ m.isAbstract() == true
+ }
+
+ def "implementations of abstract methods are considered to be overrides"() {
+ setup:
+
+ def packages = ["testsubjects"] as Set
+
+ PlasticManager mgr = PlasticManager.withContextClassLoader().packages(packages).create()
+
+ PlasticClass pc = mgr.getPlasticClass(testsubjects.PlaceholderImpl.name)
+
+ PlasticMethod m = findMethod pc, "placeholder"
+
+ expect:
+
+ m.isOverride() == true
+ }
+
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/test/java/testsubjects/AbstractPlaceholder.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/AbstractPlaceholder.java b/plastic/src/test/java/testsubjects/AbstractPlaceholder.java
new file mode 100644
index 0000000..e18ae37
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/AbstractPlaceholder.java
@@ -0,0 +1,6 @@
+package testsubjects;
+
+public abstract class AbstractPlaceholder
+{
+ public abstract void placeholder();
+}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/plastic/src/test/java/testsubjects/PlaceholderImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/PlaceholderImpl.java b/plastic/src/test/java/testsubjects/PlaceholderImpl.java
new file mode 100644
index 0000000..a9d7ed3
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/PlaceholderImpl.java
@@ -0,0 +1,8 @@
+package testsubjects;
+
+public class PlaceholderImpl extends AbstractPlaceholder
+{
+ public void placeholder()
+ {
+ }
+}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
index cc93804..bbae36a 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/CoreBehaviorsTests.java
@@ -1,5 +1,3 @@
-// Copyright 2009-2013 The Apache Software Foundation
-//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
@@ -1796,4 +1794,11 @@ public class CoreBehaviorsTests extends App1TestCase
assertTrue(selenium.getLocation().endsWith(scenario.expectedUri));
}
}
+
+ @Test
+ public void event_handler_that_overrides_abstract_method_invoked_once() {
+ openLinks("Event Handler Override Demo", "Trigger");
+
+ assertTextSeries("//ul[@id='method-names']/li[%d]", 1, "sub-class", "DONE");
+ }
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.java
new file mode 100644
index 0000000..babfbb4
--- /dev/null
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.java
@@ -0,0 +1,26 @@
+package org.apache.tapestry5.integration.app1.base;
+
+import org.apache.tapestry5.annotations.Persist;
+import org.apache.tapestry5.annotations.Property;
+import org.apache.tapestry5.ioc.internal.util.CollectionFactory;
+
+import java.util.List;
+
+public abstract class OverrideEventHandlerDemoBaseClass
+{
+ @Persist
+ @Property
+ private List<String> methodNames;
+
+ protected void add(String methodName)
+ {
+ if (methodNames == null)
+ {
+ methodNames = CollectionFactory.newList();
+ }
+
+ methodNames.add(methodName);
+ }
+
+ public abstract Object onActionFromTrigger();
+}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
index ae49e59..f3df68f 100644
--- a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/Index.java
@@ -56,6 +56,8 @@ public class Index
private static final List<Item> ITEMS = CollectionFactory
.newList(
+ new Item("OverrideEventHandlerDemo", "Event Handler Override Demo", "Event Handler methods overridden by sub-classes invoke base-class correctly."),
+
new Item("LogoSubclass", "Base class Assets in sub-classes", "Assets are resolved for the parent class if that's where the annotations are."),
new Item("MissingRequiredARP", "Missing Query Parameter for @ActivationRequestParameter", "Activating a page with a required @ActivationRequestParameter, but no matching query parameter, is an error."),
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/OverrideEventHandlerDemo.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/OverrideEventHandlerDemo.java b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/OverrideEventHandlerDemo.java
new file mode 100644
index 0000000..0d114bf
--- /dev/null
+++ b/tapestry-core/src/test/java/org/apache/tapestry5/integration/app1/pages/OverrideEventHandlerDemo.java
@@ -0,0 +1,15 @@
+package org.apache.tapestry5.integration.app1.pages;
+
+import org.apache.tapestry5.integration.app1.base.OverrideEventHandlerDemoBaseClass;
+
+public class OverrideEventHandlerDemo extends OverrideEventHandlerDemoBaseClass
+{
+ @Override
+ public Object onActionFromTrigger()
+ {
+ add("sub-class");
+ add("DONE");
+
+ return null;
+ }
+}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/df93764a/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.tml
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.tml b/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.tml
new file mode 100644
index 0000000..9e8f696
--- /dev/null
+++ b/tapestry-core/src/test/resources/org/apache/tapestry5/integration/app1/base/OverrideEventHandlerDemoBaseClass.tml
@@ -0,0 +1,13 @@
+<html t:type="Border" xmlns:t="http://tapestry.apache.org/schema/tapestry_5_3.xsd">
+
+ <h1>Event Handler Override Demo</h1>
+
+
+ <ul id="method-names">
+ <li t:type="loop" source="methodNames" value="var:methodName">${var:methodName}</li>
+ </ul>
+
+ <t:actionlink class="btn btn-primary" t:id="trigger">Trigger</t:actionlink>
+
+</html>
+