HHH-9485 - Duplicate Property with AccessType.PROPERTY and MappedSuperclass

(cherry picked from commit a5cbe326d6)

hibernate-core/src/main/java/org/hibernate/cfg/AnnotationBinder.java

@@ -1495,8 +1495,17 @@ public final class AnnotationBinder {
 	private static int addProperty(
 			PropertyContainer propertyContainer,
 			XProperty property,
-			List<PropertyData> annElts,
+			List<PropertyData> inFlightPropertyDataList,
 			MetadataBuildingContext context) {
+		// see if inFlightPropertyDataList already contains a PropertyData for this name,
+		// and if so, skip it..
+		for ( PropertyData propertyData : inFlightPropertyDataList ) {
+			if ( propertyData.getPropertyName().equals( property.getName() ) ) {
+				// EARLY EXIT!!!
+				return 0;
+			}
+		}
+
 		final XClass declaringClass = propertyContainer.getDeclaringClass();
 		final XClass entity = propertyContainer.getEntityAtStake();
 		int idPropertyCounter = 0;
@@ -1513,7 +1522,7 @@ public final class AnnotationBinder {
 		 */
 		final XAnnotatedElement element = propertyAnnotatedElement.getProperty();
 		if ( element.isAnnotationPresent( Id.class ) || element.isAnnotationPresent( EmbeddedId.class ) ) {
-			annElts.add( 0, propertyAnnotatedElement );
+			inFlightPropertyDataList.add( 0, propertyAnnotatedElement );
 			/**
 			 * The property must be put in hibernate.properties as it's a system wide property. Fixable?
 			 * TODO support true/false/default on the property instead of present / not present
@@ -1571,7 +1580,7 @@ public final class AnnotationBinder {
 			idPropertyCounter++;
 		}
 		else {
-			annElts.add( propertyAnnotatedElement );
+			inFlightPropertyDataList.add( propertyAnnotatedElement );
 		}
 		if ( element.isAnnotationPresent( MapsId.class ) ) {
 			context.getMetadataCollector().addPropertyAnnotatedWithMapsId( entity, propertyAnnotatedElement );
@@ -1595,6 +1604,16 @@ public final class AnnotationBinder {
 			boolean inSecondPass,
 			MetadataBuildingContext context,
 			Map<XClass, InheritanceState> inheritanceStatePerClass) throws MappingException {
+
+		if ( entityBinder.isPropertyDefinedInSuperHierarchy( inferredData.getPropertyName() ) ) {
+			LOG.debugf(
+					"Skipping attribute [%s : %s] as it was already processed as part of super hierarchy",
+					inferredData.getClassOrElementName(),
+					inferredData.getPropertyName()
+			);
+			return;
+		}
+
 		/**
 		 * inSecondPass can only be used to apply right away the second pass of a composite-element
 		 * Because it's a value type, there is no bidirectional association, hence second pass

hibernate-core/src/main/java/org/hibernate/cfg/annotations/EntityBinder.java

@@ -79,7 +79,9 @@ import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.mapping.DependantValue;
 import org.hibernate.mapping.Join;
+import org.hibernate.mapping.MappedSuperclass;
 import org.hibernate.mapping.PersistentClass;
+import org.hibernate.mapping.Property;
 import org.hibernate.mapping.RootClass;
 import org.hibernate.mapping.SimpleValue;
 import org.hibernate.mapping.SingleTableSubclass;
@@ -158,6 +160,24 @@ public class EntityBinder {
 		bindHibernateAnnotation( hibAnn );
 	}
 
+	/**
+	 * For the most part, this is a simple delegation to {@link PersistentClass#isPropertyDefinedInHierarchy},
+	 * after verifying that PersistentClass is indeed set here.
+	 *
+	 * @param name The name of the property to check
+	 *
+	 * @return {@code true} if a property by that given name does already exist in the super hierarchy.
+	 */
+	@SuppressWarnings("SimplifiableIfStatement")
+	public boolean isPropertyDefinedInSuperHierarchy(String name) {
+		// Yes, yes... persistentClass can be null because EntityBinder can be used
+		// to bind components as well, of course...
+		if ( persistentClass == null ) {
+			return false;
+		}
+
+		return persistentClass.isPropertyDefinedInSuperHierarchy( name );
+	}
 
 	@SuppressWarnings("SimplifiableConditionalExpression")
 	private void bindHibernateAnnotation(org.hibernate.annotations.Entity hibAnn) {

hibernate-core/src/main/java/org/hibernate/mapping/MappedSuperclass.java

@@ -163,4 +163,51 @@ public class MappedSuperclass {
 	public void setDeclaredIdentifierMapper(Component identifierMapper) {
 		this.identifierMapper = identifierMapper;
 	}
+
+	/**
+	 * Check to see if this MappedSuperclass defines a property with the given name.
+	 *
+	 * @param name The property name to check
+	 *
+	 * @return {@code true} if a property with that name exists; {@code false} if not
+	 */
+	@SuppressWarnings("WeakerAccess")
+	public boolean hasProperty(String name) {
+		final Iterator itr = getDeclaredPropertyIterator();
+		while ( itr.hasNext() ) {
+			final Property property = (Property) itr.next();
+			if ( property.getName().equals( name ) ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Check to see if a property with the given name exists in this MappedSuperclass
+	 * or in any of its super hierarchy.
+	 *
+	 * @param name The property name to check
+	 *
+	 * @return {@code true} if a property with that name exists; {@code false} if not
+	 */
+	@SuppressWarnings({"WeakerAccess", "RedundantIfStatement"})
+	public boolean isPropertyDefinedInHierarchy(String name) {
+		if ( hasProperty( name ) ) {
+			return true;
+		}
+
+		if ( getSuperMappedSuperclass() != null
+				&& getSuperMappedSuperclass().isPropertyDefinedInHierarchy( name ) ) {
+			return true;
+		}
+
+		if ( getSuperPersistentClass() != null
+				&& getSuperPersistentClass().isPropertyDefinedInHierarchy( name ) ) {
+			return true;
+		}
+
+		return false;
+	}
 }

hibernate-core/src/main/java/org/hibernate/mapping/PersistentClass.java

@@ -22,7 +22,6 @@ import org.hibernate.engine.OptimisticLockStyle;
 import org.hibernate.engine.spi.ExecuteUpdateResultCheckStyle;
 import org.hibernate.engine.spi.Mapping;
 import org.hibernate.internal.FilterConfiguration;
-import org.hibernate.internal.util.ReflectHelper;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.EmptyIterator;
 import org.hibernate.internal.util.collections.JoinedIterator;
@@ -498,6 +497,73 @@ public abstract class PersistentClass implements AttributeContainer, Serializabl
 		}
 	}
 
+	/**
+	 * Check to see if this PersistentClass defines a property with the given name.
+	 *
+	 * @param name The property name to check
+	 *
+	 * @return {@code true} if a property with that name exists; {@code false} if not
+	 */
+	@SuppressWarnings("WeakerAccess")
+	public boolean hasProperty(String name) {
+		final Property identifierProperty = getIdentifierProperty();
+		if ( identifierProperty != null && identifierProperty.getName().equals( name ) ) {
+			return true;
+		}
+
+		final Iterator itr = getPropertyClosureIterator();
+		while ( itr.hasNext() ) {
+			final Property property = (Property) itr.next();
+			if ( property.getName().equals( name ) ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
+	/**
+	 * Check to see if a property with the given name exists in the super hierarchy
+	 * of this PersistentClass.  Does not check this PersistentClass, just up the
+	 * hierarchy
+	 *
+	 * @param name The property name to check
+	 *
+	 * @return {@code true} if a property with that name exists; {@code false} if not
+	 */
+	public boolean isPropertyDefinedInSuperHierarchy(String name) {
+		return getSuperclass() != null && getSuperclass().isPropertyDefinedInHierarchy( name );
+
+	}
+
+	/**
+	 * Check to see if a property with the given name exists in this PersistentClass
+	 * or in any of its super hierarchy.  Unlike {@link #isPropertyDefinedInSuperHierarchy},
+	 * this method does check this PersistentClass
+	 *
+	 * @param name The property name to check
+	 *
+	 * @return {@code true} if a property with that name exists; {@code false} if not
+	 */
+	@SuppressWarnings({"WeakerAccess", "RedundantIfStatement"})
+	public boolean isPropertyDefinedInHierarchy(String name) {
+		if ( hasProperty( name ) ) {
+			return true;
+		}
+
+		if ( getSuperMappedSuperclass() != null
+				&& getSuperMappedSuperclass().isPropertyDefinedInHierarchy( name ) ) {
+			return true;
+		}
+
+		if ( getSuperclass() != null
+				&& getSuperclass().isPropertyDefinedInHierarchy( name ) ) {
+			return true;
+		}
+
+		return false;
+	}
+
 	/**
 	 * @deprecated prefer {@link #getOptimisticLockStyle}
 	 */

hibernate-core/src/test/java/org/hibernate/test/annotations/override/InheritedAttributeOverridingTest.java

@@ -0,0 +1,138 @@
+/*
+ * 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 <http://www.gnu.org/licenses/lgpl-2.1.html>.
+ */
+package org.hibernate.test.annotations.override;
+
+import javax.persistence.Entity;
+import javax.persistence.Id;
+import javax.persistence.MappedSuperclass;
+
+import org.hibernate.boot.Metadata;
+import org.hibernate.boot.MetadataSources;
+import org.hibernate.boot.registry.StandardServiceRegistry;
+import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
+import org.hibernate.boot.spi.MetadataImplementor;
+
+import org.hibernate.testing.FailureExpected;
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.junit4.BaseUnitTestCase;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+/**
+ * @author Steve Ebersole
+ */
+public class InheritedAttributeOverridingTest extends BaseUnitTestCase {
+	private StandardServiceRegistry standardServiceRegistry;
+
+	@Before
+	public void buildServiceRegistry() {
+		standardServiceRegistry = new StandardServiceRegistryBuilder().build();
+	}
+
+	@After
+	public void releaseServiceRegistry() {
+		if ( standardServiceRegistry != null ) {
+			StandardServiceRegistryBuilder.destroy( standardServiceRegistry );
+		}
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-9485" )
+	public void testInheritedAttributeOverridingMappedsuperclass() {
+		Metadata metadata = new MetadataSources( standardServiceRegistry )
+				.addAnnotatedClass( A.class )
+				.addAnnotatedClass( B.class )
+				.buildMetadata();
+
+		( (MetadataImplementor) metadata ).validate();
+	}
+
+	@Test
+	@TestForIssue( jiraKey = "HHH-9485" )
+	public void testInheritedAttributeOverridingEntity() {
+		Metadata metadata = new MetadataSources( standardServiceRegistry )
+				.addAnnotatedClass( C.class )
+				.addAnnotatedClass( D.class )
+				.buildMetadata();
+
+		( (MetadataImplementor) metadata ).validate();
+	}
+
+	@MappedSuperclass
+	public static class A {
+		private Integer id;
+		private String name;
+
+		@Id
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+
+	@Entity( name = "B" )
+	public static class B extends A {
+		@Override
+		public String getName() {
+			return super.getName();
+		}
+
+		@Override
+		public void setName(String name) {
+			super.setName( name );
+		}
+	}
+
+
+	@Entity( name = "C" )
+	public static class C {
+		private Integer id;
+		private String name;
+
+		@Id
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+
+	@Entity( name = "D" )
+	public static class D extends C {
+		@Override
+		public String getName() {
+			return super.getName();
+		}
+
+		@Override
+		public void setName(String name) {
+			super.setName( name );
+		}
+	}
+}