This is an automated email from the ASF dual-hosted git repository.
thiagohp pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
The following commit(s) were added to refs/heads/master by this push:
new 709d282 TAP5-2582: Service creation for Hibernate Session results in
709d282 is described below
commit 709d282bfc626ce55cde07cbf909c0b86c2b4bcb
Author: Thiago H. de Paula Figueiredo <[email protected]>
AuthorDate: Sun Feb 24 00:10:42 2019 -0300
TAP5-2582: Service creation for Hibernate Session results in
ClassFormatError: Duplicate method name&signature
---
.../internal/services/PlasticProxyFactoryImpl.java | 14 ++-
.../ioc/services/PlasticProxyFactory.java | 15 +++
.../internal/plastic/PlasticClassImpl.java | 136 +++++++++++++++------
.../apache/tapestry5/plastic/PlasticManager.java | 56 ++++++++-
.../apache/tapestry5/ioc/internal/ModuleImpl.java | 2 +-
.../groovy/ioc/specs/GeneralIntegrationSpec.groovy | 24 ++++
.../tapestry5/ioc/internal/AdviceModule.java | 4 +
7 files changed, 204 insertions(+), 47 deletions(-)
diff --git
a/beanmodel/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
b/beanmodel/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
index dc5397c..f1b3f67 100644
---
a/beanmodel/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
+++
b/beanmodel/src/main/java/org/apache/tapestry5/ioc/internal/services/PlasticProxyFactoryImpl.java
@@ -65,16 +65,24 @@ public class PlasticProxyFactoryImpl implements
PlasticProxyFactory
@Override
public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
Class<? extends T> implementationType, PlasticClassTransformer callback)
{
- return manager.createProxy(interfaceType, implementationType,
callback);
+ return createProxy(interfaceType, implementationType, callback, true);
}
-
+
+ @Override
+ public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
+ Class<? extends T> implementationType,
+ PlasticClassTransformer callback,
+ boolean introduceInterface) {
+ return manager.createProxy(interfaceType, implementationType,
callback, introduceInterface);
+ }
+
+
@Override
public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
PlasticClassTransformer callback)
{
return manager.createProxy(interfaceType, callback);
}
-
@Override
public <T> PlasticClassTransformation<T>
createProxyTransformation(Class<T> interfaceType,
Class<? extends T> implementationType)
diff --git
a/commons/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
b/commons/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
index 75e93e4..99bcc55 100644
---
a/commons/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
+++
b/commons/src/main/java/org/apache/tapestry5/ioc/services/PlasticProxyFactory.java
@@ -63,6 +63,20 @@ public interface PlasticProxyFactory extends
PlasticClassListenerHub
* configures the proxy
* @return instantiator that can be used to create an instance of the
proxy class
*/
+ @IncompatibleChange(release = "5.4.5", details = "TAP5-2528")
+ <T> ClassInstantiator<T> createProxy(Class<T> interfaceType, Class<?
extends T> implementationType, PlasticClassTransformer callback, boolean
introduceInterface);
+
+ /**
+ * Same as <code>createProxy(interfacetype, implementationType, callback,
true)</code>
+ *
+ * @param interfaceType
+ * interface implemented by proxy
+ * @param implementationType
+ * a class that implements the interfaceType. It can be null.
+ * @param callback
+ * configures the proxy
+ * @return instantiator that can be used to create an instance of the
proxy class
+ */
@IncompatibleChange(release = "5.4", details = "TAP5-2029")
<T> ClassInstantiator<T> createProxy(Class<T> interfaceType, Class<?
extends T> implementationType, PlasticClassTransformer callback);
@@ -156,4 +170,5 @@ public interface PlasticProxyFactory extends
PlasticClassListenerHub
* @since 5.3.3
*/
void clearCache();
+
}
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 b86b8d0..fffd910 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
@@ -618,11 +618,9 @@ public class PlasticClassImpl extends Lockable implements
PlasticClass, Internal
introduceInterface(interfaceType);
- // TAP5-2582: avoiding adding/delegating the same method more than once
- Map<MethodSignature, MethodDescription> map =
createMethodSignatureMap(interfaceType);
- for (MethodSignature methodSignature : map.keySet())
+ for (Method m : getUniqueMethods(interfaceType))
{
- final MethodDescription description = map.get(methodSignature);
+ final MethodDescription description = new MethodDescription(m);
if(Modifier.isStatic(description.modifiers))
{
continue;
@@ -634,30 +632,6 @@ public class PlasticClassImpl extends Lockable implements
PlasticClass, Internal
}
@Override
- public PlasticClass proxyInterface(Class interfaceType, PlasticMethod
method)
- {
- check();
-
- assert method != null;
-
- introduceInterface(interfaceType);
-
- // TAP5-2582: avoiding adding/delegating the same method more than once
- Map<MethodSignature, MethodDescription> map =
createMethodSignatureMap(interfaceType);
- for (MethodSignature methodSignature : map.keySet())
- {
- final MethodDescription description = map.get(methodSignature);
- if(Modifier.isStatic(description.modifiers))
- {
- continue;
- }
- introduceMethod(map.get(methodSignature)).delegateTo(method);
- }
-
- return this;
- }
-
- @Override
public ClassInstantiator createInstantiator()
{
lock();
@@ -989,7 +963,6 @@ public class PlasticClassImpl extends Lockable implements
PlasticClass, Internal
public PlasticMethod introduceMethod(Method method)
{
check();
-
return introduceMethod(new MethodDescription(method));
}
@@ -1422,9 +1395,13 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal
return new InstructionBuilderImpl(description, mn, nameCache);
}
- @Override
public Set<PlasticMethod> introduceInterface(Class interfaceType)
{
+ return introduceInterface(interfaceType, null);
+ }
+
+ private Set<PlasticMethod> introduceInterface(Class interfaceType,
PlasticMethod method)
+ {
check();
assert interfaceType != null;
@@ -1451,20 +1428,23 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal
addClassAnnotations(interfaceClassNode);
- Set<PlasticMethod> introducedMethods = new HashSet<>();
-
- Map<MethodSignature, MethodDescription> map =
createMethodSignatureMap(interfaceType);
+ Set<PlasticMethod> introducedMethods = new HashSet<PlasticMethod>();
+ Set<Method> alreadyIntroducedMethods = new HashSet<>();
- // for (Method m : interfaceType.getMethods())
- for (MethodSignature methodSignature : map.keySet())
+ Method[] sortedMethods = interfaceType.getMethods();
+ Arrays.sort(sortedMethods, METHOD_COMPARATOR);
+ for (Method m : sortedMethods)
{
- // MethodDescription description = new MethodDescription(m);
- final MethodDescription description = map.get(methodSignature);
+ MethodDescription description = new MethodDescription(m);
- if (!isMethodImplemented(description) &&
!isDefaultMethod(methodSignature.method) &&
!Modifier.isStatic(description.modifiers))
+ if (!isMethodImplemented(description) && !isDefaultMethod(m) &&
!Modifier.isStatic(description.modifiers) &&
!contains(alreadyIntroducedMethods, m))
{
- // introducedMethods.add(introduceMethod(m));
- introducedMethods.add(introduceMethod(description));
+ PlasticMethod introducedMethod = introduceMethod(m);
+ introducedMethods.add(introducedMethod);
+ if (method != null) {
+ introducedMethod.delegateTo(method);
+ }
+ alreadyIntroducedMethods.add(m);
}
}
@@ -1472,6 +1452,30 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal
return introducedMethods;
}
+
+ @Override
+ public PlasticClass proxyInterface(Class interfaceType, PlasticMethod
method)
+ {
+ check();
+ assert method != null;
+
+ introduceInterface(interfaceType, method);
+
+ return this;
+ }
+
+ private boolean contains(Set<Method> alreadyIntroducedMethods, Method m) {
+ boolean contains = false;
+ for (Method method : alreadyIntroducedMethods)
+ {
+ if (METHOD_COMPARATOR.compare(method, m) == 0)
+ {
+ contains = true;
+ break;
+ }
+ }
+ return false;
+ }
private Map<MethodSignature, MethodDescription>
createMethodSignatureMap(Class interfaceType) {
// TAP-2582: preprocessing the method list so we don't add duplicated
@@ -1620,6 +1624,58 @@ public class PlasticClassImpl extends Lockable
implements PlasticClass, Internal
pool.setFieldReadInstrumentation(classNode.name, fieldName, fi);
}
}
+
+ final private MethodComparator METHOD_COMPARATOR = new MethodComparator();
+
+ final private class MethodComparator implements Comparator<Method>
+ {
+
+ @Override
+ public int compare(Method o1, Method o2)
+ {
+
+ int comparison = o1.getName().compareTo(o2.getName());
+
+ if (comparison == 0)
+ {
+ comparison = o1.getParameterTypes().length -
o2.getParameterTypes().length;
+ }
+
+ if (comparison == 0)
+ {
+ final int count = o1.getParameterTypes().length;
+ for (int i = 0; i < count; i++)
+ {
+ Class p1 = o1.getParameterTypes()[i];
+ Class p2 = o1.getParameterTypes()[i];
+ if (!p1.equals(p2))
+ {
+ comparison = p1.getName().compareTo(p2.getName());
+ break;
+ }
+ }
+ }
+ return comparison;
+ }
+ }
+
+ private List<Method> getUniqueMethods(Class interfaceType)
+ {
+ final List<Method> unique = new
ArrayList<>(Arrays.asList(interfaceType.getMethods()));
+ Collections.sort(unique, METHOD_COMPARATOR);
+ Method last = null;
+ Iterator<Method> iterator = unique.iterator();
+ while (iterator.hasNext())
+ {
+ Method m = iterator.next();
+ if (last != null && METHOD_COMPARATOR.compare(m, last) == 0)
+ {
+ last = m;
+ iterator.remove();
+ }
+ }
+ return unique;
+ }
@Override
public String toString()
diff --git
a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
index 5f31755..74acf21 100644
--- a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
+++ b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticManager.java
@@ -234,6 +234,21 @@ public class PlasticManager implements
PlasticClassListenerHub
*
* @param interfaceType
* class to extend from, which must be a class, not an interface
+ * @param callback
+ * used to configure the new class
+ * @return the instantiator, which allows instances of the new class to be
created
+ * @see #createProxyTransformation(Class, Class)
+ */
+ public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
PlasticClassTransformer callback, boolean introduceInterface)
+ {
+ return createProxy(interfaceType, null, callback, introduceInterface);
+ }
+
+ /**
+ * Creates an entirely new class. The class extends from Object and
implements the provided interface.
+ *
+ * @param interfaceType
+ * class to extend from, which must be a class, not an interface
* @param implementationType
* class that implements interfaceType. It can be null.
* @param callback
@@ -244,9 +259,30 @@ public class PlasticManager implements
PlasticClassListenerHub
*/
public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
Class<? extends T> implementationType, PlasticClassTransformer callback)
{
+ return createProxy(interfaceType, implementationType, callback, true);
+ }
+
+ /**
+ * Creates an entirely new class. The class extends from Object and
implements the provided interface.
+ *
+ * @param interfaceType
+ * class to extend from, which must be a class, not an interface
+ * @param implementationType
+ * class that implements interfaceType. It can be null.
+ * @param callback
+ * used to configure the new class
+ * @param introduceInterface
+ * whether to introduce the interface to the Plastic class or
not.
+ * @return the instantiator, which allows instances of the new class to be
created
+ * @see #createProxyTransformation(Class, Class)
+ * @since 5.4.5
+ */
+ public <T> ClassInstantiator<T> createProxy(Class<T> interfaceType,
Class<? extends T> implementationType, PlasticClassTransformer callback,
+ boolean introduceInterface)
+ {
assert callback != null;
- PlasticClassTransformation<T> transformation =
createProxyTransformation(interfaceType, implementationType);
+ PlasticClassTransformation<T> transformation =
createProxyTransformation(interfaceType, implementationType,
introduceInterface);
callback.transform(transformation.getPlasticClass());
@@ -254,6 +290,14 @@ public class PlasticManager implements
PlasticClassListenerHub
}
/**
+ * Returns <code>createProxyTransformation(interfaceType,
implementationType, true)</code>
+ */
+ public <T> PlasticClassTransformation<T> createProxyTransformation(Class
interfaceType, Class implementationType)
+ {
+ return createProxyTransformation(interfaceType, implementationType,
true);
+ }
+
+ /**
* Creates the underlying {@link PlasticClassTransformation} for an
interface proxy. This should only be
* used in the cases where encapsulating the PlasticClass construction
into a {@linkplain PlasticClassTransformer
* callback} is not feasible (which is the case for some of the older APIs
inside Tapestry IoC).
@@ -262,9 +306,12 @@ public class PlasticManager implements
PlasticClassListenerHub
* class proxy will extend from
* @param implementationType
* class that implements interfaceType. It can be null.
+ * @param introduceInterface
+ * whether
<code>result.getPlasticClass().introduceInterface(interfaceType);</code> should
+ * be called or not.
* @return transformation from which an instantiator may be created
*/
- public <T> PlasticClassTransformation<T> createProxyTransformation(Class
interfaceType, Class implementationType)
+ public <T> PlasticClassTransformation<T> createProxyTransformation(Class
interfaceType, Class implementationType, boolean introduceInterface)
{
assert interfaceType != null;
@@ -279,7 +326,10 @@ public class PlasticManager implements
PlasticClassListenerHub
PlasticClassTransformation<T> result =
pool.createTransformation("java.lang.Object", name,
implementationClassName);
- result.getPlasticClass().introduceInterface(interfaceType);
+ if (introduceInterface)
+ {
+ result.getPlasticClass().introduceInterface(interfaceType);
+ }
return result;
}
diff --git
a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
index 37ce2bf..98a2f8c 100644
---
a/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
+++
b/tapestry-ioc/src/main/java/org/apache/tapestry5/ioc/internal/ModuleImpl.java
@@ -522,7 +522,7 @@ public class ModuleImpl implements Module
plasticClass.addToString(description);
}
- });
+ }, false);
return instantiator.newInstance();
}
diff --git
a/tapestry-ioc/src/test/groovy/ioc/specs/GeneralIntegrationSpec.groovy
b/tapestry-ioc/src/test/groovy/ioc/specs/GeneralIntegrationSpec.groovy
index 1e30463..8fda9a4 100644
--- a/tapestry-ioc/src/test/groovy/ioc/specs/GeneralIntegrationSpec.groovy
+++ b/tapestry-ioc/src/test/groovy/ioc/specs/GeneralIntegrationSpec.groovy
@@ -1,7 +1,12 @@
package ioc.specs
+import org.apache.tapestry5.ioc.ObjectLocator
+import org.apache.tapestry5.ioc.Registry
+import org.apache.tapestry5.ioc.RegistryBuilder
import org.apache.tapestry5.ioc.internal.services.Bean
import org.apache.tapestry5.ioc.services.PropertyAccess
+import org.hibernate.Session
+import org.hibernate.cfg.Configuration
class GeneralIntegrationSpec extends AbstractSharedRegistrySpecification {
@@ -20,6 +25,25 @@ class GeneralIntegrationSpec extends
AbstractSharedRegistrySpecification {
b.value == 99
pa.get(b, "value") == 99
}
+
+ def "Avoiding duplicated method implementation in service proxies"() {
+ when:
+ Registry registry =
RegistryBuilder.buildAndStartupRegistry(TestModule.class);
+ then:
+ // Throws exception without fix.
+ Session session = registry.getService(Session.class);
+ }
+ public static final class TestModule
+ {
+ public static Session buildHibernateSession(
+ ObjectLocator objectLocator
+ ) {
+ return new Configuration()
+ .configure("hibernate.cfg.xml")
+ .buildSessionFactory()
+ .openSession();
+ }
+ }
}
diff --git
a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
index 1a90aa3..a236e70 100644
---
a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
+++
b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
@@ -15,8 +15,11 @@ package org.apache.tapestry5.ioc.internal;
import org.apache.tapestry5.ioc.MethodAdviceReceiver;
+import org.apache.tapestry5.ioc.ObjectLocator;
import org.apache.tapestry5.ioc.ServiceBinder;
import org.apache.tapestry5.ioc.annotations.Advise;
+import org.hibernate.Session;
+import org.hibernate.cfg.Configuration;
public class AdviceModule
{
@@ -46,4 +49,5 @@ public class AdviceModule
final MethodAdviceReceiver methodAdviceReceiver) {
methodAdviceReceiver.adviseAllMethods(new TestAdvice());
}
+
}
