From 5c71f353dec530f720dad2ecd40fa46a370aebf6 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 5 Mar 2018 18:45:04 +0100 Subject: [PATCH] HHH-12332 - Fix for NPE in AbstractPropertyMapping.getSuperCollection --- .../org/hibernate/id/ExportableColumn.java | 5 ++ .../main/java/org/hibernate/mapping/Any.java | 13 +++ .../org/hibernate/mapping/Collection.java | 22 +++++ .../java/org/hibernate/mapping/Component.java | 15 ++++ .../org/hibernate/mapping/DependantValue.java | 11 +++ .../mapping/IdentifierCollection.java | 11 +++ .../hibernate/mapping/IndexedCollection.java | 11 +++ .../java/org/hibernate/mapping/OneToMany.java | 11 +++ .../java/org/hibernate/mapping/OneToOne.java | 15 ++++ .../hibernate/mapping/PersistentClass.java | 2 +- .../org/hibernate/mapping/SimpleValue.java | 19 +++++ .../java/org/hibernate/mapping/ToOne.java | 14 ++++ .../java/org/hibernate/mapping/Value.java | 1 + .../entity/AbstractPropertyMapping.java | 83 +------------------ .../MappedSuperclassExtendsEntityTest.java | 24 +++++- 15 files changed, 175 insertions(+), 82 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/id/ExportableColumn.java b/hibernate-core/src/main/java/org/hibernate/id/ExportableColumn.java index 6719c91af4..a5909ea18f 100644 --- a/hibernate-core/src/main/java/org/hibernate/id/ExportableColumn.java +++ b/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(); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Any.java b/hibernate-core/src/main/java/org/hibernate/mapping/Any.java index 1fda4d0298..487989640a 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Any.java +++ b/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 ); + } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java b/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java index f57d8647fd..ec96ca2051 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Collection.java +++ b/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 ) { diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java b/hibernate-core/src/main/java/org/hibernate/mapping/Component.java index 53ed74ff6c..6667d9f88f 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Component.java +++ b/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() ]; diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/DependantValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/DependantValue.java index 59c23a5007..25bcd6dc29 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/DependantValue.java +++ b/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 ); + } + } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/IdentifierCollection.java b/hibernate-core/src/main/java/org/hibernate/mapping/IdentifierCollection.java index 064c4f6cda..a2a02bf12f 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/IdentifierCollection.java +++ b/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() ); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/IndexedCollection.java b/hibernate-core/src/main/java/org/hibernate/mapping/IndexedCollection.java index bb0c3251f7..88d9db9ba8 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/IndexedCollection.java +++ b/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() ); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java b/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java index a462df9795..3b2e848ace 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/OneToMany.java +++ b/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... diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java index 429e1cacf2..d6a70c530b 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/OneToOne.java +++ b/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; @@ -146,5 +147,19 @@ public class OneToOne extends ToOne { public Object accept(ValueVisitor visitor) { 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; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/PersistentClass.java b/hibernate-core/src/main/java/org/hibernate/mapping/PersistentClass.java index 38dc218108..3f3a9d2179 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/PersistentClass.java +++ b/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( diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java b/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java index 7ebc7ab0cf..65655f5ff7 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/SimpleValue.java +++ b/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() + ')'; diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java b/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java index 2351c4e157..f6800c7d00 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/ToOne.java +++ b/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"); diff --git a/hibernate-core/src/main/java/org/hibernate/mapping/Value.java b/hibernate-core/src/main/java/org/hibernate/mapping/Value.java index 6a7be09bc6..f84c2c2c88 100644 --- a/hibernate-core/src/main/java/org/hibernate/mapping/Value.java +++ b/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(); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractPropertyMapping.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractPropertyMapping.java index b468a6d2f9..98ff676868 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractPropertyMapping.java +++ b/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.MappedSuperclass; -import org.hibernate.mapping.PersistentClass; +import org.hibernate.mapping.*; 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( - metadata, - thisCollection.getOwner(), - otherCollection.getOwner(), - thisCollection.getReferencedPropertyName() - ); - - newType = commonCollection.getType(); + throw new IllegalStateException( "Collection mapping in abstract entity type with a type variable is unsupported! Couldn't add property '" + path + "' with type: " + type ); } 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, diff --git a/hibernate-core/src/test/java/org/hibernate/test/inheritance/discriminator/MappedSuperclassExtendsEntityTest.java b/hibernate-core/src/test/java/org/hibernate/test/inheritance/discriminator/MappedSuperclassExtendsEntityTest.java index 0b8ae7c71b..dee0b23919 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/inheritance/discriminator/MappedSuperclassExtendsEntityTest.java +++ b/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 entities2; public GrandParent() { } @@ -79,6 +82,14 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas public void setId(Long id) { this.id = id; } + + public List getEntities2() { + return entities2; + } + + public void setEntities2(List entities2) { + this.entities2 = entities2; + } } @MappedSuperclass @@ -105,6 +116,8 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas private Long id; @ManyToMany private List parents; + @ManyToMany + private List parents2; public Long getId() { return id; @@ -122,6 +135,13 @@ public class MappedSuperclassExtendsEntityTest extends BaseCoreFunctionalTestCas this.parents = parents; } + public List getParents2() { + return parents2; + } + + public void setParents2(List parents2) { + this.parents2 = parents2; + } } @Entity(name = "Child1")