From 13a1e4145a3cd651b448b167025c73a74d6557cd Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Thu, 9 Apr 2020 14:40:12 +0200 Subject: [PATCH] HV-1763 Switch to a string less approach for signature generation --- .../metadata/aggregated/BeanMetaDataImpl.java | 33 +++++++------- .../aggregated/ExecutableMetaData.java | 9 ++-- .../descriptor/BeanDescriptorImpl.java | 9 ++-- .../internal/properties/Callable.java | 2 +- .../internal/properties/Signature.java | 44 +++++++++++++++++++ .../javabean/JavaBeanExecutable.java | 3 +- .../internal/util/ExecutableHelper.java | 25 ++--------- .../aggregated/ExecutableMetaDataTest.java | 17 +++---- 8 files changed, 88 insertions(+), 54 deletions(-) create mode 100644 engine/src/main/java/org/hibernate/validator/internal/properties/Signature.java diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java index c837b046e2..34adb58879 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/BeanMetaDataImpl.java @@ -36,6 +36,7 @@ import org.hibernate.validator.internal.metadata.descriptor.ExecutableDescriptorImpl; import org.hibernate.validator.internal.metadata.facets.Cascadable; import org.hibernate.validator.internal.metadata.location.ConstraintLocation.ConstraintLocationKind; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.util.CollectionHelper; import org.hibernate.validator.internal.util.ExecutableHelper; import org.hibernate.validator.internal.util.classhierarchy.ClassHierarchyHelper; @@ -100,14 +101,14 @@ public final class BeanMetaDataImpl implements BeanMetaData { * meta-data will be stored). */ @Immutable - private final Map executableMetaDataMap; + private final Map executableMetaDataMap; /** * The set of unconstrained executables of the bean. It contains all the relevant signatures, following the same * rules as {@code executableMetaDataMap}. */ @Immutable - private final Set unconstrainedExecutables; + private final Set unconstrainedExecutables; /** * Property meta data keyed against the property name @@ -181,7 +182,7 @@ public BeanMetaDataImpl(Class beanClass, Set propertyMetaDataSet = newHashSet(); Set executableMetaDataSet = newHashSet(); - Set tmpUnconstrainedExecutables = newHashSet(); + Set tmpUnconstrainedExecutables = newHashSet(); boolean hasConstraints = false; Set> allMetaConstraints = newHashSet(); @@ -302,7 +303,7 @@ public Set> getDirectMetaConstraints() { @Override public Optional getMetaDataFor(Executable executable) { - String signature = ExecutableHelper.getSignature( executable ); + Signature signature = ExecutableHelper.getSignature( executable ); if ( unconstrainedExecutables.contains( signature ) ) { return Optional.empty(); @@ -357,7 +358,8 @@ public List> getClassHierarchy() { } private static BeanDescriptor createBeanDescriptor(Class beanClass, Set> allMetaConstraints, - Map propertyMetaDataMap, Map executableMetaDataMap, boolean defaultGroupSequenceRedefined, + Map propertyMetaDataMap, Map executableMetaDataMap, + boolean defaultGroupSequenceRedefined, List> resolvedDefaultGroupSequence) { Map propertyDescriptors = getConstrainedPropertiesAsDescriptors( propertyMetaDataMap, @@ -365,13 +367,13 @@ private static BeanDescriptor createBeanDescriptor(Class beanClass, Set methodsDescriptors = getConstrainedMethodsAsDescriptors( + Map methodsDescriptors = getConstrainedMethodsAsDescriptors( executableMetaDataMap, defaultGroupSequenceRedefined, resolvedDefaultGroupSequence ); - Map constructorsDescriptors = getConstrainedConstructorsAsDescriptors( + Map constructorsDescriptors = getConstrainedConstructorsAsDescriptors( executableMetaDataMap, defaultGroupSequenceRedefined, resolvedDefaultGroupSequence @@ -414,9 +416,10 @@ private static Map getConstrainedPropertiesAsDescrip return theValue; } - private static Map getConstrainedMethodsAsDescriptors(Map executableMetaDataMap, + private static Map getConstrainedMethodsAsDescriptors( + Map executableMetaDataMap, boolean defaultGroupSequenceIsRedefined, List> resolvedDefaultGroupSequence) { - Map constrainedMethodDescriptors = newHashMap(); + Map constrainedMethodDescriptors = newHashMap(); for ( ExecutableMetaData executableMetaData : executableMetaDataMap.values() ) { if ( executableMetaData.getKind() == ElementKind.METHOD @@ -426,7 +429,7 @@ private static Map getConstrainedMethodsAsDesc resolvedDefaultGroupSequence ); - for ( String signature : executableMetaData.getSignatures() ) { + for ( Signature signature : executableMetaData.getSignatures() ) { constrainedMethodDescriptors.put( signature, descriptor ); } } @@ -435,9 +438,9 @@ private static Map getConstrainedMethodsAsDesc return constrainedMethodDescriptors; } - private static Map getConstrainedConstructorsAsDescriptors(Map executableMetaDataMap, + private static Map getConstrainedConstructorsAsDescriptors(Map executableMetaDataMap, boolean defaultGroupSequenceIsRedefined, List> resolvedDefaultGroupSequence) { - Map constrainedMethodDescriptors = newHashMap(); + Map constrainedMethodDescriptors = newHashMap(); for ( ExecutableMetaData executableMetaData : executableMetaDataMap.values() ) { if ( executableMetaData.getKind() == ElementKind.CONSTRUCTOR && executableMetaData.isConstrained() ) { @@ -501,11 +504,11 @@ private Set> getDirectConstraints() { * Builds up the method meta data for this type; each meta-data entry will be stored under the signature of the * represented method and all the methods it overrides. */ - private Map bySignature(Set executables) { - Map theValue = newHashMap(); + private Map bySignature(Set executables) { + Map theValue = newHashMap(); for ( ExecutableMetaData executableMetaData : executables ) { - for ( String signature : executableMetaData.getSignatures() ) { + for ( Signature signature : executableMetaData.getSignatures() ) { theValue.put( signature, executableMetaData ); } } diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ExecutableMetaData.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ExecutableMetaData.java index 9bf4614d19..fb3108ffaa 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ExecutableMetaData.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/aggregated/ExecutableMetaData.java @@ -31,6 +31,7 @@ import org.hibernate.validator.internal.metadata.raw.ConstrainedExecutable; import org.hibernate.validator.internal.metadata.raw.ConstrainedParameter; import org.hibernate.validator.internal.properties.Callable; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.util.CollectionHelper; import org.hibernate.validator.internal.util.ExecutableHelper; import org.hibernate.validator.internal.util.ExecutableParameterNameProvider; @@ -72,7 +73,7 @@ public class ExecutableMetaData extends AbstractConstraintMetaData { * overrides a super type method with generic parameters, in which case the signature of the super-type and the * sub-type method will differ. */ - private final Set signatures; + private final Set signatures; private final ReturnValueMetaData returnValueMetaData; private final ElementKind kind; @@ -82,7 +83,7 @@ private ExecutableMetaData( Type returnType, Class[] parameterTypes, ElementKind kind, - Set signatures, + Set signatures, Set> returnValueConstraints, Set> returnValueContainerElementConstraints, List parameterMetaDataList, @@ -137,7 +138,7 @@ public Class[] getParameterTypes() { * method represents a sub-type method overriding a super-type method using a generic type parameter in its * parameters. */ - public Set getSignatures() { + public Set getSignatures() { return signatures; } @@ -249,7 +250,7 @@ public boolean equals(Object obj) { * @author Kevin Pollet <kevin.pollet@serli.com> (C) 2011 SERLI */ public static class Builder extends MetaDataBuilder { - private final Set signatures = newHashSet(); + private final Set signatures = newHashSet(); /** * Either CONSTRUCTOR, METHOD or GETTER. diff --git a/engine/src/main/java/org/hibernate/validator/internal/metadata/descriptor/BeanDescriptorImpl.java b/engine/src/main/java/org/hibernate/validator/internal/metadata/descriptor/BeanDescriptorImpl.java index 96a5797476..8bce45a2f4 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/metadata/descriptor/BeanDescriptorImpl.java +++ b/engine/src/main/java/org/hibernate/validator/internal/metadata/descriptor/BeanDescriptorImpl.java @@ -21,6 +21,7 @@ import jakarta.validation.metadata.MethodType; import jakarta.validation.metadata.PropertyDescriptor; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.util.CollectionHelper; import org.hibernate.validator.internal.util.Contracts; import org.hibernate.validator.internal.util.ExecutableHelper; @@ -37,15 +38,15 @@ public class BeanDescriptorImpl extends ElementDescriptorImpl implements BeanDes @Immutable private final Map constrainedProperties; @Immutable - private final Map constrainedMethods; + private final Map constrainedMethods; @Immutable - private final Map constrainedConstructors; + private final Map constrainedConstructors; public BeanDescriptorImpl(Type beanClass, Set> classLevelConstraints, Map constrainedProperties, - Map constrainedMethods, - Map constrainedConstructors, + Map constrainedMethods, + Map constrainedConstructors, boolean defaultGroupSequenceRedefined, List> defaultGroupSequence) { super( beanClass, classLevelConstraints, defaultGroupSequenceRedefined, defaultGroupSequence ); diff --git a/engine/src/main/java/org/hibernate/validator/internal/properties/Callable.java b/engine/src/main/java/org/hibernate/validator/internal/properties/Callable.java index 9ef259f46d..12fa48cd91 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/properties/Callable.java +++ b/engine/src/main/java/org/hibernate/validator/internal/properties/Callable.java @@ -31,7 +31,7 @@ public interface Callable extends Constrainable { boolean isPrivate(); - String getSignature(); + Signature getSignature(); boolean overrides(ExecutableHelper executableHelper, Callable superTypeMethod); diff --git a/engine/src/main/java/org/hibernate/validator/internal/properties/Signature.java b/engine/src/main/java/org/hibernate/validator/internal/properties/Signature.java new file mode 100644 index 0000000000..0a1087f434 --- /dev/null +++ b/engine/src/main/java/org/hibernate/validator/internal/properties/Signature.java @@ -0,0 +1,44 @@ +/* + * Hibernate Validator, declare and validate application constraints + * + * License: Apache License, Version 2.0 + * See the license.txt file in the root directory or . + */ +package org.hibernate.validator.internal.properties; + +import java.util.Arrays; + +public final class Signature { + + private final String name; + + private final Class[] parameterTypes; + + public Signature(String name, Class... parameterTypes) { + this.name = name; + this.parameterTypes = parameterTypes; + } + + @Override + public boolean equals(Object obj) { + if ( obj == null ) { + return false; + } + + // we can safely assume the type will always be the right one + Signature other = (Signature) obj; + + if ( !this.name.equals( other.name ) ) { + return false; + } + + return Arrays.equals( this.parameterTypes, other.parameterTypes ); + } + + @Override + public int hashCode() { + int result = name.hashCode(); + result = 31 * result + Arrays.hashCode( parameterTypes ); + return result; + } +} diff --git a/engine/src/main/java/org/hibernate/validator/internal/properties/javabean/JavaBeanExecutable.java b/engine/src/main/java/org/hibernate/validator/internal/properties/javabean/JavaBeanExecutable.java index 11e84f89d6..fb53587d9b 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/properties/javabean/JavaBeanExecutable.java +++ b/engine/src/main/java/org/hibernate/validator/internal/properties/javabean/JavaBeanExecutable.java @@ -20,6 +20,7 @@ import java.util.List; import org.hibernate.validator.internal.properties.Callable; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.util.CollectionHelper; import org.hibernate.validator.internal.util.ExecutableHelper; import org.hibernate.validator.internal.util.ExecutableParameterNameProvider; @@ -91,7 +92,7 @@ public boolean isPrivate() { } @Override - public String getSignature() { + public Signature getSignature() { return ExecutableHelper.getSignature( executable ); } diff --git a/engine/src/main/java/org/hibernate/validator/internal/util/ExecutableHelper.java b/engine/src/main/java/org/hibernate/validator/internal/util/ExecutableHelper.java index da6ad4547e..d7f65241c5 100644 --- a/engine/src/main/java/org/hibernate/validator/internal/util/ExecutableHelper.java +++ b/engine/src/main/java/org/hibernate/validator/internal/util/ExecutableHelper.java @@ -16,6 +16,7 @@ import java.security.PrivilegedAction; import org.hibernate.validator.internal.properties.Callable; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.util.classhierarchy.Filters; import org.hibernate.validator.internal.util.logging.Log; import org.hibernate.validator.internal.util.logging.LoggerFactory; @@ -175,30 +176,12 @@ public static String getSimpleName(Executable executable) { return executable instanceof Constructor ? executable.getDeclaringClass().getSimpleName() : executable.getName(); } - public static String getSignature(Executable executable) { + public static Signature getSignature(Executable executable) { return getSignature( getSimpleName( executable ), executable.getParameterTypes() ); } - /** - * This method needs to be fast. - *

- * The purpose of it is to get uniqueness, we don't really care about prettiness. - */ - public static String getSignature(String name, Class[] parameterTypes) { - StringBuilder signature = new StringBuilder( name.length() + 2 + parameterTypes.length * 25 ); - signature.append( name ).append( '(' ); - boolean separator = false; - for ( Class parameterType : parameterTypes ) { - if ( separator ) { - signature.append( ',' ); - } - else { - separator = true; - } - signature.append( parameterType.getName() ); - } - signature.append( ')' ); - return signature.toString(); + public static Signature getSignature(String name, Class[] parameterTypes) { + return new Signature( name, parameterTypes ); } /** diff --git a/engine/src/test/java/org/hibernate/validator/test/internal/metadata/aggregated/ExecutableMetaDataTest.java b/engine/src/test/java/org/hibernate/validator/test/internal/metadata/aggregated/ExecutableMetaDataTest.java index aeac834bbb..851b3a0843 100644 --- a/engine/src/test/java/org/hibernate/validator/test/internal/metadata/aggregated/ExecutableMetaDataTest.java +++ b/engine/src/test/java/org/hibernate/validator/test/internal/metadata/aggregated/ExecutableMetaDataTest.java @@ -35,6 +35,7 @@ import org.hibernate.validator.internal.metadata.descriptor.ConstraintDescriptorImpl; import org.hibernate.validator.internal.metadata.provider.MetaDataProvider; import org.hibernate.validator.internal.properties.DefaultGetterPropertySelectionStrategy; +import org.hibernate.validator.internal.properties.Signature; import org.hibernate.validator.internal.properties.javabean.JavaBeanHelper; import org.hibernate.validator.internal.util.ExecutableHelper; import org.hibernate.validator.internal.util.ExecutableParameterNameProvider; @@ -152,7 +153,7 @@ public void getIdentifierForMethod() throws Exception { ExecutableMetaData methodMetaData = beanMetaData.getMetaDataFor( method ).get(); assertThat( methodMetaData.getSignatures() ).containsOnly( - "createCustomer(java.lang.CharSequence,java.lang.String)" + new Signature( "createCustomer", CharSequence.class, String.class ) ); } @@ -161,7 +162,7 @@ public void getIdentifierForParameterlessMethod() throws Exception { Method method = CustomerRepositoryExt.class.getMethod( "foo" ); ExecutableMetaData methodMetaData = beanMetaData.getMetaDataFor( method ).get(); - assertThat( methodMetaData.getSignatures() ).containsOnly( "foo()" ); + assertThat( methodMetaData.getSignatures() ).containsOnly( new Signature( "foo" ) ); } @Test @@ -169,7 +170,7 @@ public void getIdentifierForConstructor() throws Exception { Constructor constructor = CustomerRepositoryExt.class.getConstructor( String.class ); ExecutableMetaData constructorMetaData = beanMetaData.getMetaDataFor( constructor ).get(); - assertThat( constructorMetaData.getSignatures() ).containsOnly( "CustomerRepositoryExt(java.lang.String)" ); + assertThat( constructorMetaData.getSignatures() ).containsOnly( new Signature( "CustomerRepositoryExt", String.class ) ); } @Test @@ -180,8 +181,8 @@ public void getIdentifierForOverridingGenericMethod() throws Exception { ExecutableMetaData methodMetaData = beanMetaData.getMetaDataFor( method ).get(); assertThat( methodMetaData.getSignatures() ) - .describedAs( "Expecting super-type and sub-type method signatures" ) - .containsOnly( "createJob(java.lang.Object)", "createJob(java.util.UUID)" ); + .describedAs( "Expecting super-type and sub-type method signatures" ) + .containsOnly( new Signature( "createJob", Object.class ), new Signature( "createJob", UUID.class ) ); method = JobRepository.class.getMethod( "createJob", Object.class ); beanMetaData = beanMetaDataManager.getBeanMetaData( JobRepository.class ); @@ -189,7 +190,7 @@ public void getIdentifierForOverridingGenericMethod() throws Exception { assertThat( methodMetaData.getSignatures() ) .describedAs( "Expecting only super-type method signature" ) - .containsOnly( "createJob(java.lang.Object)" ); + .containsOnly( new Signature( "createJob", Object.class ) ); method = SpecialJobRepositoryImpl.class.getMethod( "createJob", Object.class ); beanMetaData = beanMetaDataManager.getBeanMetaData( SpecialJobRepositoryImpl.class ); @@ -197,7 +198,7 @@ public void getIdentifierForOverridingGenericMethod() throws Exception { assertThat( methodMetaData.getSignatures() ) .describedAs( "Expecting method signatures from super-types" ) - .containsOnly( "createJob(java.lang.Object)", "createJob(java.util.UUID)" ); + .containsOnly( new Signature( "createJob", Object.class ), new Signature( "createJob", UUID.class ) ); } @Test @@ -209,7 +210,7 @@ public void getIdentifierForOverloadedMethod() throws Exception { assertThat( methodMetaData.getSignatures() ) .describedAs( "Expecting sub-type method signature which overloads but not overrides super-type methods" ) - .containsOnly( "createJob(java.lang.String)" ); + .containsOnly( new Signature( "createJob", String.class ) ); } @Test