HHH-12332 - Fix for NPE in AbstractPropertyMapping.getSuperCollection

hibernate-core/src/main/java/org/hibernate/id/ExportableColumn.java

@@ -134,6 +134,11 @@ public class ExportableColumn extends Column {
 			return null;
 		}
 
+		@Override
+		public boolean isSame(Value value) {
+			return false;
+		}
+
 		@Override
 		public ServiceRegistry getServiceRegistry() {
 			return database.getBuildingOptions().getServiceRegistry();

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

@@ -7,6 +7,7 @@
 package org.hibernate.mapping;
 
 import java.util.Map;
+import java.util.Objects;
 
 import org.hibernate.MappingException;
 import org.hibernate.boot.spi.MetadataImplementor;
@@ -69,4 +70,16 @@ public class Any extends SimpleValue {
 	public Object accept(ValueVisitor visitor) {
 		return visitor.accept(this);
 	}
+
+	@Override
+	public boolean isSame(SimpleValue other) {
+		return other instanceof Any && isSame( (Any) other );
+	}
+
+	public boolean isSame(Any other) {
+		return super.isSame( other )
+				&& Objects.equals( identifierTypeName, other.identifierTypeName )
+				&& Objects.equals( metaTypeName, other.metaTypeName )
+				&& Objects.equals( metaValues, other.metaValues );
+	}
 }

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

@@ -12,6 +12,7 @@ import java.util.Comparator;
 import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Properties;
+import java.util.Objects;
 
 import org.hibernate.FetchMode;
 import org.hibernate.MappingException;
@@ -404,6 +405,27 @@ public abstract class Collection implements Fetchable, Value, Filterable {
 		return true;
 	}
 
+	@Override
+	public boolean isSame(Value other) {
+		return this == other || other instanceof Collection && isSame( (Collection) other );
+	}
+
+	protected static boolean isSame(Value v1, Value v2) {
+		return v1 == v2 || v1 != null && v2 != null && v1.isSame( v2 );
+	}
+
+	public boolean isSame(Collection other) {
+		return this == other || isSame( key, other.key )
+				&& isSame( element, other.element )
+				&& Objects.equals( collectionTable, other.collectionTable )
+				&& Objects.equals( where, other.where )
+				&& Objects.equals( manyToManyWhere, other.manyToManyWhere )
+				&& Objects.equals( referencedPropertyName, other.referencedPropertyName )
+				&& Objects.equals( mappedByProperty, other.mappedByProperty )
+				&& Objects.equals( typeName, other.typeName )
+				&& Objects.equals( typeParameters, other.typeParameters );
+	}
+
 	private void createForeignKeys() throws MappingException {
 		// if ( !isInverse() ) { // for inverse collections, let the "other end" handle it
 		if ( referencedPropertyName == null ) {

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

@@ -10,6 +10,7 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.Iterator;
+import java.util.Objects;
 import java.util.Map;
 
 import org.hibernate.EntityMode;
@@ -196,6 +197,20 @@ public class Component extends SimpleValue implements MetaAttributable {
 		return visitor.accept(this);
 	}
 
+	@Override
+	public boolean isSame(SimpleValue other) {
+		return other instanceof Component && isSame( (Component) other );
+	}
+
+	public boolean isSame(Component other) {
+		return super.isSame( other )
+				&& Objects.equals( properties, other.properties )
+				&& Objects.equals( componentClassName, other.componentClassName )
+				&& embedded == other.embedded
+				&& Objects.equals( parentProperty, other.parentProperty )
+				&& Objects.equals( metaAttributes, other.metaAttributes );
+	}
+
 	@Override
 	public boolean[] getColumnInsertability() {
 		boolean[] result = new boolean[ getColumnSpan() ];

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

@@ -53,4 +53,15 @@ public class DependantValue extends SimpleValue {
 	public void setUpdateable(boolean updateable) {
 		this.updateable = updateable;
 	}
+
+	@Override
+	public boolean isSame(SimpleValue other) {
+		return other instanceof DependantValue && isSame( (DependantValue) other );
+	}
+
+	public boolean isSame(DependantValue other) {
+		return super.isSame( other )
+				&& isSame( wrappedValue, other.wrappedValue );
+	}
+
 }

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

@@ -33,6 +33,17 @@ public abstract class IdentifierCollection extends Collection {
 		return true;
 	}
 
+	@Override
+	public boolean isSame(Collection other) {
+		return other instanceof IdentifierCollection
+				&& isSame( (IdentifierCollection) other );
+	}
+
+	public boolean isSame(IdentifierCollection other) {
+		return super.isSame( other )
+				&& isSame( identifier, other.identifier );
+	}
+
 	void createPrimaryKey() {
 		if ( !isOneToMany() ) {
 			PrimaryKey pk = new PrimaryKey( getCollectionTable() );

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

@@ -37,6 +37,17 @@ public abstract class IndexedCollection extends Collection {
 		return true;
 	}
 
+	@Override
+	public boolean isSame(Collection other) {
+		return other instanceof IndexedCollection
+				&& isSame( (IndexedCollection) other );
+	}
+
+	public boolean isSame(IndexedCollection other) {
+		 return super.isSame( other )
+				 && isSame( index, other.index );
+	}
+
 	void createPrimaryKey() {
 		if ( !isOneToMany() ) {
 			PrimaryKey pk = new PrimaryKey( getCollectionTable() );

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

@@ -7,6 +7,7 @@
 package org.hibernate.mapping;
 
 import java.util.Iterator;
+import java.util.Objects;
 
 import org.hibernate.FetchMode;
 import org.hibernate.MappingException;
@@ -131,6 +132,16 @@ public class OneToMany implements Value {
 		return visitor.accept( this );
 	}
 
+	@Override
+	public boolean isSame(Value other) {
+		return this == other || other instanceof OneToMany && isSame( (OneToMany) other );
+	}
+
+	public boolean isSame(OneToMany other) {
+		return Objects.equals( referencingTable, other.referencingTable )
+				&& Objects.equals( referencedEntityName, other.referencedEntityName )
+				&& Objects.equals( associatedClass, other.associatedClass );
+	}
 
 	public boolean[] getColumnInsertability() {
 		//TODO: we could just return all false...

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

@@ -8,6 +8,7 @@ package org.hibernate.mapping;
 
 import java.util.ArrayList;
 import java.util.Iterator;
+import java.util.Objects;
 
 import org.hibernate.MappingException;
 import org.hibernate.boot.spi.MetadataImplementor;
@@ -147,4 +148,18 @@ public class OneToOne extends ToOne {
 		return visitor.accept(this);
 	}
 
+	@Override
+	public boolean isSame(ToOne other) {
+		return other instanceof OneToOne && isSame( (OneToOne) other );
+	}
+
+	public boolean isSame(OneToOne other) {
+		return super.isSame( other )
+				&& Objects.equals( foreignKeyType, other.foreignKeyType )
+				&& isSame( identifier, other.identifier )
+				&& Objects.equals( propertyName, other.propertyName )
+				&& Objects.equals( entityName, other.entityName )
+				&& constrained == other.constrained;
+	}
+	
 }

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

@@ -419,7 +419,7 @@ public abstract class PersistentClass implements AttributeContainer, Serializabl
 
 	public Property getRecursiveProperty(String propertyPath) throws MappingException {
 		try {
-			return getRecursiveProperty( propertyPath, getPropertyIterator() );
+			return getRecursiveProperty( propertyPath, getPropertyClosureIterator() );
 		}
 		catch (MappingException e) {
 			throw new MappingException(

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

@@ -14,6 +14,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Locale;
 import java.util.Properties;
+import java.util.Objects;
 import javax.persistence.AttributeConverter;
 
 import org.hibernate.FetchMode;
@@ -625,6 +626,24 @@ public class SimpleValue implements KeyValue {
 		attributeConverterDescriptor = sourceValue.attributeConverterDescriptor;
 	}
 
+	@Override
+	public boolean isSame(Value other) {
+		return this == other || other instanceof SimpleValue && isSame( (SimpleValue) other );
+	}
+
+	protected static boolean isSame(Value v1, Value v2) {
+		return v1 == v2 || v1 != null && v2 != null && v1.isSame( v2 );
+	}
+
+	public boolean isSame(SimpleValue other) {
+		return Objects.equals( columns, other.columns )
+				&& Objects.equals( typeName, other.typeName )
+				&& Objects.equals( typeParameters, other.typeParameters )
+				&& Objects.equals( table, other.table )
+				&& Objects.equals( foreignKeyName, other.foreignKeyName )
+				&& Objects.equals( foreignKeyDefinition, other.foreignKeyDefinition );
+	}
+
 	@Override
 	public String toString() {
 		return getClass().getName() + '(' + columns.toString() + ')';

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

@@ -14,6 +14,8 @@ import org.hibernate.engine.spi.Mapping;
 import org.hibernate.internal.util.ReflectHelper;
 import org.hibernate.type.Type;
 
+import java.util.Objects;
+
 /**
  * A simple-point association (ie. a reference to another entity).
  * @author Gavin King
@@ -76,6 +78,18 @@ public abstract class ToOne extends SimpleValue implements Fetchable {
 		return visitor.accept(this);
 	}
 
+	@Override
+	public boolean isSame(SimpleValue other) {
+		return other instanceof ToOne && isSame( (ToOne) other );
+	}
+
+	public boolean isSame(ToOne other) {
+		return super.isSame( other )
+				&& Objects.equals( referencedPropertyName, other.referencedPropertyName )
+				&& Objects.equals( referencedEntityName, other.referencedEntityName )
+				&& embedded == other.embedded;
+	}
+
 	public boolean isValid(Mapping mapping) throws MappingException {
 		if (referencedEntityName==null) {
 			throw new MappingException("association must specify the referenced entity");

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

@@ -39,6 +39,7 @@ public interface Value extends Serializable {
 	public boolean isValid(Mapping mapping) throws MappingException;
 	public void setTypeUsingReflection(String className, String propertyName) throws MappingException;
 	public Object accept(ValueVisitor visitor);
+	public boolean isSame(Value other);
 
 	ServiceRegistry getServiceRegistry();
 }

hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractPropertyMapping.java

@@ -17,9 +17,7 @@ import org.hibernate.internal.CoreLogging;
 import org.hibernate.internal.CoreMessageLogger;
 import org.hibernate.internal.util.StringHelper;
 import org.hibernate.internal.util.collections.ArrayHelper;
-import org.hibernate.mapping.Collection;
+import org.hibernate.mapping.*;
-import org.hibernate.mapping.MappedSuperclass;
-import org.hibernate.mapping.PersistentClass;
 import org.hibernate.sql.Template;
 import org.hibernate.type.AnyType;
 import org.hibernate.type.AssociationType;
@@ -186,7 +184,7 @@ public abstract class AbstractPropertyMapping implements PropertyMapping {
 				Collection thisCollection = metadata.getCollectionBinding( ( (CollectionType) existingType ).getRole() );
 				Collection otherCollection = metadata.getCollectionBinding( ( (CollectionType) type ).getRole() );
 
-				if ( thisCollection == otherCollection ) {
+				if ( thisCollection.isSame( otherCollection ) ) {
 					logDuplicateRegistration(
 							path,
 							existingType,
@@ -195,14 +193,7 @@ public abstract class AbstractPropertyMapping implements PropertyMapping {
 					return;
 				}
 
-				Collection commonCollection = getSuperCollection(
+                throw new IllegalStateException( "Collection mapping in abstract entity type with a type variable is unsupported! Couldn't add property '" + path + "' with type: " + type );
-						metadata,
-						thisCollection.getOwner(),
-						otherCollection.getOwner(),
-						thisCollection.getReferencedPropertyName()
-				);
-
-				newType = commonCollection.getType();
 			}
 			else if ( type instanceof EntityType ) {
 				EntityType entityType1 = (EntityType) existingType;
@@ -268,78 +259,12 @@ public abstract class AbstractPropertyMapping implements PropertyMapping {
 	}
 
 	private PersistentClass getCommonPersistentClass(PersistentClass clazz1, PersistentClass clazz2) {
-		while ( !clazz2.getMappedClass().isAssignableFrom( clazz1.getMappedClass() ) ) {
+		while ( clazz2 != null && !clazz2.getMappedClass().isAssignableFrom( clazz1.getMappedClass() ) ) {
 			clazz2 = clazz2.getSuperclass();
 		}
 		return clazz2;
 	}
 
-	private Collection getSuperCollection(MetadataImplementor metadata, PersistentClass clazz1, PersistentClass commonPersistentClass, String propertyName) {
-		Class<?> c1 = clazz1.getMappedClass();
-		Class<?> c2 = commonPersistentClass.getMappedClass();
-		MappedSuperclass commonMappedSuperclass = null;
-
-		// First we traverse up the clazz2/commonPersistentClass super types until we find a common type
-		while ( !c2.isAssignableFrom( c1 ) ) {
-			if ( commonPersistentClass == null) {
-				if ( commonMappedSuperclass.getSuperPersistentClass() == null ) {
-					commonMappedSuperclass = commonMappedSuperclass.getSuperMappedSuperclass();
-					commonPersistentClass = null;
-				}
-				else {
-					commonPersistentClass = commonMappedSuperclass.getSuperPersistentClass();
-					commonMappedSuperclass = null;
-				}
-			}
-			else {
-				if ( commonPersistentClass.getSuperclass() == null ) {
-					commonMappedSuperclass = commonPersistentClass.getSuperMappedSuperclass();
-					commonPersistentClass = null;
-				}
-				else {
-					commonPersistentClass = commonPersistentClass.getSuperclass();
-					commonMappedSuperclass = null;
-				}
-			}
-		}
-
-		// Then we traverse it's types up as long as possible until we find a type that has a collection binding
-		while ( c2 != Object.class ) {
-			if ( commonMappedSuperclass != null ) {
-				Collection collection = metadata.getCollectionBinding( commonMappedSuperclass.getMappedClass().getName() + "." + propertyName );
-				if ( collection != null ) {
-					return collection;
-				}
-
-				if ( commonMappedSuperclass.getSuperPersistentClass() == null ) {
-					commonMappedSuperclass = commonMappedSuperclass.getSuperMappedSuperclass();
-					commonPersistentClass = null;
-				}
-				else {
-					commonPersistentClass = commonMappedSuperclass.getSuperPersistentClass();
-					commonMappedSuperclass = null;
-				}
-			}
-			else {
-				Collection collection = metadata.getCollectionBinding( commonPersistentClass.getEntityName() + "." + propertyName );
-				if ( collection != null ) {
-					return collection;
-				}
-
-				if ( commonPersistentClass.getSuperclass() == null ) {
-					commonMappedSuperclass = commonPersistentClass.getSuperMappedSuperclass();
-					commonPersistentClass = null;
-				}
-				else {
-					commonPersistentClass = commonPersistentClass.getSuperclass();
-					commonMappedSuperclass = null;
-				}
-			}
-		}
-
-		return null;
-	}
-
 	/*protected void initPropertyPaths(
 			final String path,
 			final Type type,

hibernate-core/src/test/java/org/hibernate/test/inheritance/discriminator/MappedSuperclassExtendsEntityTest.java

@@ -54,13 +54,14 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas
 	@Test
 	@TestForIssue(jiraKey = "HHH-12332")
 	public void testQueryingSingle() {
+		// Make sure that the produced query for th
 		doInHibernate( this::sessionFactory, s -> {
-			s.createQuery( "from TestEntity p" ).getResultList();
+			s.createQuery( "FROM TestEntity e JOIN e.parents p1 JOIN p1.entities JOIN p1.entities2 JOIN e.parents2 p2 JOIN p2.entities JOIN p2.entities2" ).getResultList();
 		} );
 	}
 
 	@Entity(name = "GrandParent")
-	@Inheritance(strategy = InheritanceType.TABLE_PER_CLASS)
+	@Inheritance
 	@DiscriminatorColumn(name = "discriminator")
 	public static abstract class GrandParent implements Serializable {
 		private static final long serialVersionUID = 1L;
@@ -68,6 +69,8 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas
         @Id
         @GeneratedValue
 		private Long id;
+		@ManyToMany(mappedBy = "parents2")
+		private List<TestEntity> entities2;
 
 		public GrandParent() {
 		}
@@ -79,6 +82,14 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas
 		public void setId(Long id) {
 			this.id = id;
 		}
+
+		public List<TestEntity> getEntities2() {
+			return entities2;
+		}
+
+		public void setEntities2(List<TestEntity> entities2) {
+			this.entities2 = entities2;
+		}
 	}
 
 	@MappedSuperclass
@@ -105,6 +116,8 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas
 		private Long id;
 		@ManyToMany
 		private List<GrandParent> parents;
+		@ManyToMany
+		private List<GrandParent> parents2;
 
 		public Long getId() {
 			return id;
@@ -122,6 +135,13 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas
 			this.parents = parents;
 		}
 
+		public List<GrandParent> getParents2() {
+			return parents2;
+		}
+
+		public void setParents2(List<GrandParent> parents2) {
+			this.parents2 = parents2;
+		}
 	}
 
 	@Entity(name = "Child1")