This is an automated email from the ASF dual-hosted git repository. thiagohp pushed a commit to branch 5.4.x in repository https://gitbox.apache.org/repos/asf/tapestry-5.git
commit b8f4a672fe5a5a6709bb5ec7cd89c0c7ff0cf0be 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 | 99 ++++++++++++++++++++-- .../org/apache/tapestry5/plastic/PlasticClass.java | 11 +++ .../apache/tapestry5/plastic/PlasticManager.java | 56 +++++++++++- .../apache/tapestry5/ioc/internal/ModuleImpl.java | 7 +- .../specs/AspectInterceptorBuilderImplSpec.groovy | 2 +- .../groovy/ioc/specs/GeneralIntegrationSpec.groovy | 24 ++++++ .../tapestry5/ioc/internal/AdviceModule.java | 4 + 9 files changed, 214 insertions(+), 18 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 264825a..a7c1fc5 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 @@ -621,7 +621,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal introduceInterface(interfaceType); - for (Method m : interfaceType.getMethods()) + for (Method m : getUniqueMethods(interfaceType)) { introduceMethod(m).delegateTo(field); } @@ -968,7 +968,6 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal public PlasticMethod introduceMethod(Method method) { check(); - return introduceMethod(new MethodDescription(method)); } @@ -1401,9 +1400,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; @@ -1431,14 +1434,22 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal addClassAnnotations(interfaceClassNode); Set<PlasticMethod> introducedMethods = new HashSet<PlasticMethod>(); + Set<Method> alreadyIntroducedMethods = new HashSet<>(); - for (Method m : interfaceType.getMethods()) + Method[] sortedMethods = interfaceType.getMethods(); + Arrays.sort(sortedMethods, METHOD_COMPARATOR); + for (Method m : sortedMethods) { MethodDescription description = new MethodDescription(m); - if (!isMethodImplemented(description) && !isDefaultMethod(m)) + if (!isMethodImplemented(description) && !isDefaultMethod(m) && !contains(alreadyIntroducedMethods, m)) { - introducedMethods.add(introduceMethod(m)); + PlasticMethod introducedMethod = introduceMethod(m); + introducedMethods.add(introducedMethod); + if (method != null) { + introducedMethod.delegateTo(method); + } + alreadyIntroducedMethods.add(m); } } @@ -1446,6 +1457,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; + } @Override public PlasticClass addToString(final String toStringValue) @@ -1525,6 +1560,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/PlasticClass.java b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticClass.java index e212fe5..2168456 100644 --- a/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticClass.java +++ b/plastic/src/main/java/org/apache/tapestry5/plastic/PlasticClass.java @@ -163,6 +163,17 @@ public interface PlasticClass extends AnnotationAccess * @return this plastic class, for further configuration */ PlasticClass proxyInterface(Class interfaceType, PlasticField field); + + /** + * Introduces the interface, and then invokes {@link PlasticMethod#delegateTo(PlasticMethod)} on each method + * defined by the interface. + * + * @param interfaceType defines the interface to proxy + * @param method method to delegate to + * @return this plastic class, for further configuration + * @since 5.4.4 + */ + PlasticClass proxyInterface(Class interfaceType, PlasticMethod method); /** * Conditionally adds an implementation of <code>toString()</code> to the class, but only if it is not already 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 48bf24d..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 @@ -509,10 +509,7 @@ public class ModuleImpl implements Module } }); - for (Method m : serviceInterface.getMethods()) - { - plasticClass.introduceMethod(m).delegateTo(delegateMethod); - } + plasticClass.proxyInterface(serviceInterface, delegateMethod); plasticClass.introduceMethod(WRITE_REPLACE).changeImplementation(new InstructionBuilderCallback() { @@ -525,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/AspectInterceptorBuilderImplSpec.groovy b/tapestry-ioc/src/test/groovy/ioc/specs/AspectInterceptorBuilderImplSpec.groovy index e543482..b614c38 100644 --- a/tapestry-ioc/src/test/groovy/ioc/specs/AspectInterceptorBuilderImplSpec.groovy +++ b/tapestry-ioc/src/test/groovy/ioc/specs/AspectInterceptorBuilderImplSpec.groovy @@ -1,6 +1,6 @@ package ioc.specs -import org.apache.commons.lang.StringUtils +import org.apache.commons.lang3.StringUtils import org.apache.tapestry5.ioc.internal.services.TextTransformer import org.apache.tapestry5.ioc.services.AspectDecorator import org.apache.tapestry5.plastic.MethodAdvice 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 0e0a186..ea2e576 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 @@ -14,8 +14,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 { @@ -45,4 +48,5 @@ public class AdviceModule final MethodAdviceReceiver methodAdviceReceiver) { methodAdviceReceiver.adviseAllMethods(new TestAdvice()); } + }
