From 1203acbdc0044f877b65f6e25c167db4a28ca857 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 7 Oct 2015 12:26:07 -0500 Subject: [PATCH] HHH-10172 - Throw MappingException when entity/component class defines multiple matching getters by stem name --- .../internal/util/ReflectHelper.java | 69 +++++++++++++-- .../property/GetAndIsVariantGetterTest.java | 83 +++++++++++++++++++ .../org/hibernate/property/TheEntity.hbm.xml | 12 +++ 3 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/property/GetAndIsVariantGetterTest.java create mode 100644 hibernate-core/src/test/resources/org/hibernate/property/TheEntity.hbm.xml diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java index bf425a9174..6954c0fa33 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/ReflectHelper.java @@ -426,18 +426,21 @@ public final class ReflectHelper { // try "get" if ( methodName.startsWith( "get" ) ) { - String testStdMethod = Introspector.decapitalize( methodName.substring( 3 ) ); - String testOldMethod = methodName.substring( 3 ); - if ( testStdMethod.equals( propertyName ) || testOldMethod.equals( propertyName ) ) { + final String stemName = methodName.substring( 3 ); + final String decapitalizedStemName = Introspector.decapitalize( stemName ); + if ( stemName.equals( propertyName ) || decapitalizedStemName.equals( propertyName ) ) { + verifyNoIsVariantExists( containerClass, propertyName, method, stemName ); return method; } + } // if not "get", then try "is" if ( methodName.startsWith( "is" ) ) { - String testStdMethod = Introspector.decapitalize( methodName.substring( 2 ) ); - String testOldMethod = methodName.substring( 2 ); - if ( testStdMethod.equals( propertyName ) || testOldMethod.equals( propertyName ) ) { + final String stemName = methodName.substring( 2 ); + String decapitalizedStemName = Introspector.decapitalize( stemName ); + if ( stemName.equals( propertyName ) || decapitalizedStemName.equals( propertyName ) ) { + verifyNoGetVariantExists( containerClass, propertyName, method, stemName ); return method; } } @@ -446,6 +449,60 @@ public final class ReflectHelper { return null; } + private static void verifyNoIsVariantExists( + Class containerClass, + String propertyName, + Method getMethod, + String stemName) { + // verify that the Class does not also define a method with the same stem name with 'is' + try { + final Method isMethod = containerClass.getDeclaredMethod( "is" + stemName ); + // No such method should throw the caught exception. So if we get here, there was + // such a method. + checkGetAndIsVariants( containerClass, propertyName, getMethod, isMethod ); + } + catch (NoSuchMethodException ignore) { + } + } + + private static void checkGetAndIsVariants( + Class containerClass, + String propertyName, + Method getMethod, + Method isMethod) { + // Check the return types. If they are the same, its ok. If they are different + // we are in a situation where we could not reasonably know which to use. + if ( !isMethod.getReturnType().equals( getMethod.getReturnType() ) ) { + throw new MappingException( + String.format( + Locale.ROOT, + "In trying to locate getter for property [%s], Class [%s] defined " + + "both a `get` [%s] and `is` [%s] variant", + propertyName, + containerClass.getName(), + getMethod.toString(), + isMethod.toString() + ) + ); + } + } + + private static void verifyNoGetVariantExists( + Class containerClass, + String propertyName, + Method isMethod, + String stemName) { + // verify that the Class does not also define a method with the same stem name with 'is' + try { + final Method getMethod = containerClass.getDeclaredMethod( "get" + stemName ); + // No such method should throw the caught exception. So if we get here, there was + // such a method. + checkGetAndIsVariants( containerClass, propertyName, getMethod, isMethod ); + } + catch (NoSuchMethodException ignore) { + } + } + public static Method findSetterMethod(Class containerClass, String propertyName, Class propertyType) { Class checkClass = containerClass; Method setter = null; diff --git a/hibernate-core/src/test/java/org/hibernate/property/GetAndIsVariantGetterTest.java b/hibernate-core/src/test/java/org/hibernate/property/GetAndIsVariantGetterTest.java new file mode 100644 index 0000000000..6e502ece46 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/property/GetAndIsVariantGetterTest.java @@ -0,0 +1,83 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later. + * See the lgpl.txt file in the root directory or . + */ +package org.hibernate.property; + +import javax.persistence.Entity; +import javax.persistence.Id; + +import org.hibernate.MappingException; +import org.hibernate.boot.MetadataSources; +import org.hibernate.boot.registry.StandardServiceRegistry; +import org.hibernate.boot.registry.StandardServiceRegistryBuilder; + +import org.hibernate.testing.TestForIssue; +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +import static org.junit.Assert.fail; + +/** + * Originally written to verify fix for HHH-10172 + * + * @author Steve Ebersole + */ +public class GetAndIsVariantGetterTest { + private static StandardServiceRegistry ssr; + + @BeforeClass + public static void prepare() { + ssr = new StandardServiceRegistryBuilder( ).build(); + } + + @AfterClass + public static void release() { + if ( ssr != null ) { + StandardServiceRegistryBuilder.destroy( ssr ); + } + } + + @Test + @TestForIssue( jiraKey = "HHH-10172" ) + public void testHbmXml() { + try { + new MetadataSources( ssr ) + .addResource( "org/hibernate/property/TheEntity.hbm.xml" ) + .buildMetadata(); + fail( "Expecting a failure" ); + } + catch (MappingException ignore) { + // expected + } + } + + @Test + @TestForIssue( jiraKey = "HHH-10172" ) + public void testAnnotations() { + new MetadataSources( ssr ) + .addAnnotatedClass( TheEntity.class ) + .buildMetadata(); + } + + @Entity + public static class TheEntity { + private Integer id; + + public boolean isId() { + return false; + } + + @Id + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + } +} diff --git a/hibernate-core/src/test/resources/org/hibernate/property/TheEntity.hbm.xml b/hibernate-core/src/test/resources/org/hibernate/property/TheEntity.hbm.xml new file mode 100644 index 0000000000..84ae7abf0f --- /dev/null +++ b/hibernate-core/src/test/resources/org/hibernate/property/TheEntity.hbm.xml @@ -0,0 +1,12 @@ + + + + + + + \ No newline at end of file