From 5a549ea5b4d8b4fd55be456293093e686764fc3a Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 31 Jan 2022 21:36:44 +0100 Subject: [PATCH] fix initialization of SingleTableEntityPersister to be eager --- .../util/collections/ArrayHelper.java | 35 +-- .../entity/SingleTableEntityPersister.java | 215 ++++++++---------- .../java/org/hibernate/type/BasicType.java | 2 - .../DuplicatedDiscriminatorValueTest.java | 2 +- 4 files changed, 120 insertions(+), 134 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/ArrayHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/ArrayHelper.java index 06f491140a..81000afa2e 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/ArrayHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/ArrayHelper.java @@ -36,6 +36,7 @@ public final class ArrayHelper { return -1; } + @SuppressWarnings("unchecked") public static T[] filledArray(T value, Class valueJavaType, int size) { final T[] array = (T[]) Array.newInstance( valueJavaType, size ); Arrays.fill( array, value ); @@ -79,6 +80,10 @@ public final class ArrayHelper { return coll.toArray( EMPTY_STRING_ARRAY ); } + public static Object[] toObjectArray(Collection coll) { + return coll.toArray( EMPTY_OBJECT_ARRAY ); + } + public static String[][] to2DStringArray(Collection coll) { return coll.toArray( new String[0][] ); } @@ -102,11 +107,11 @@ public final class ArrayHelper { } public static boolean[] toBooleanArray(Collection coll) { - Iterator iter = coll.iterator(); + Iterator iter = coll.iterator(); boolean[] arr = new boolean[coll.size()]; int i = 0; while ( iter.hasNext() ) { - arr[i++] = (Boolean) iter.next(); + arr[i++] = iter.next(); } return arr; } @@ -116,17 +121,17 @@ public final class ArrayHelper { } //Arrays.asList doesn't do primitive arrays - public static List toList(Object array) { - if ( array instanceof Object[] ) { - return Arrays.asList( (Object[]) array ); //faster? - } - int size = Array.getLength( array ); - ArrayList list = new ArrayList( size ); - for ( int i = 0; i < size; i++ ) { - list.add( Array.get( array, i ) ); - } - return list; - } +// public static List toList(Object array) { +// if ( array instanceof Object[] ) { +// return Arrays.asList( (Object[]) array ); //faster? +// } +// int size = Array.getLength( array ); +// ArrayList list = new ArrayList<>( size ); +// for ( int i = 0; i < size; i++ ) { +// list.add( Array.get( array, i ) ); +// } +// return list; +// } public static String[] slice(String[] strings, int begin, int length) { String[] result = new String[length]; @@ -140,8 +145,8 @@ public final class ArrayHelper { return result; } - public static List toList(Iterator iter) { - List list = new ArrayList(); + public static List toList(Iterator iter) { + List list = new ArrayList<>(); while ( iter.hasNext() ) { list.add( iter.next() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java index aaa3116450..55a117a0ca 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/SingleTableEntityPersister.java @@ -25,6 +25,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.internal.DynamicFilterAliasGenerator; import org.hibernate.internal.FilterAliasGenerator; import org.hibernate.internal.util.MarkerObject; +import org.hibernate.internal.util.ReflectHelper; import org.hibernate.internal.util.StringHelper; import org.hibernate.internal.util.collections.ArrayHelper; import org.hibernate.internal.util.collections.CollectionHelper; @@ -128,6 +129,9 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { //private final Map propertyTableNumbersByName = new HashMap(); // private final Map propertyTableNumbersByNameAndSubclass; + private final String[] fullDiscriminatorSQLValues; + private final Object[] fullDiscriminatorValues; + private static final Object NULL_DISCRIMINATOR = new MarkerObject( "" ); private static final Object NOT_NULL_DISCRIMINATOR = new MarkerObject( "" ); @@ -293,7 +297,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { // DISCRIMINATOR if ( persistentClass.isPolymorphic() ) { - Value discrimValue = persistentClass.getDiscriminator(); + final Value discrimValue = persistentClass.getDiscriminator(); if ( discrimValue == null ) { throw new MappingException( "discriminator mapping required for single table polymorphic persistence" ); } @@ -318,37 +322,10 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { // discriminatorFormula = null; discriminatorFormulaTemplate = null; } - discriminatorType = (BasicType) persistentClass.getDiscriminator().getType(); - if ( persistentClass.isDiscriminatorValueNull() ) { - discriminatorValue = NULL_DISCRIMINATOR; - discriminatorSQLValue = InFragment.NULL; - discriminatorInsertable = false; - } - else if ( persistentClass.isDiscriminatorValueNotNull() ) { - discriminatorValue = NOT_NULL_DISCRIMINATOR; - discriminatorSQLValue = InFragment.NOT_NULL; - discriminatorInsertable = false; - } - else { - discriminatorInsertable = persistentClass.isDiscriminatorInsertable() && !discrimValue.hasFormula(); - try { - discriminatorValue = discriminatorType.getJavaTypeDescriptor() - .fromString( persistentClass.getDiscriminatorValue() ); - JdbcLiteralFormatter literalFormatter = discriminatorType.getJdbcType() - .getJdbcLiteralFormatter( discriminatorType.getJavaTypeDescriptor() ); - discriminatorSQLValue = literalFormatter.toJdbcLiteral( - discriminatorValue, - dialect, - factory.getWrapperOptions() - ); - } - catch (ClassCastException cce) { - throw new MappingException( "Illegal discriminator type: " + discriminatorType.getName() ); - } - catch (Exception e) { - throw new MappingException( "Could not format discriminator value to SQL string", e ); - } - } + discriminatorType = getDiscriminatorType( persistentClass ); + discriminatorValue = getDiscriminatorValue( persistentClass ); + discriminatorSQLValue = getDiscriminatorSQLValue( persistentClass, dialect, factory ); + discriminatorInsertable = isDiscriminatorInsertable( persistentClass ); } else { forceDiscriminator = false; @@ -407,6 +384,8 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { // subclassFormulaTableNumberClosure = ArrayHelper.toIntArray( formulaJoinedNumbers ); subclassPropertyTableNumberClosure = ArrayHelper.toIntArray( propertyJoinNumbers ); + final List values = new ArrayList<>(); + final List sqlValues = new ArrayList<>(); int subclassSpan = persistentClass.getSubclassSpan() + 1; subclassClosure = new String[subclassSpan]; subclassClosure[0] = getEntityName(); @@ -416,53 +395,106 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { discriminatorValue, getEntityName() ); - } - // SUBCLASSES - if ( persistentClass.isPolymorphic() ) { + if ( !getEntityMetamodel().isAbstract() ) { + values.add( discriminatorValue ); + sqlValues.add( discriminatorSQLValue ); + } + + // SUBCLASSES int k = 1; for ( Subclass subclass : persistentClass.getSubclasses() ) { subclassClosure[k++] = subclass.getEntityName(); - if ( subclass.isDiscriminatorValueNull() ) { - addSubclassByDiscriminatorValue( - subclassesByDiscriminatorValueLocal, - NULL_DISCRIMINATOR, - subclass.getEntityName() - ); - } - else if ( subclass.isDiscriminatorValueNotNull() ) { - addSubclassByDiscriminatorValue( - subclassesByDiscriminatorValueLocal, - NOT_NULL_DISCRIMINATOR, - subclass.getEntityName() - ); - } - else { - try { - addSubclassByDiscriminatorValue( - subclassesByDiscriminatorValueLocal, - discriminatorType.getJavaTypeDescriptor() - .fromString( subclass.getDiscriminatorValue() ), - subclass.getEntityName() - ); - } - catch (ClassCastException cce) { - throw new MappingException( "Illegal discriminator type: " + discriminatorType.getName() ); - } - catch (Exception e) { - throw new MappingException( "Error parsing discriminator value", e ); - } + Object subclassDiscriminatorValue = getDiscriminatorValue( subclass ); + addSubclassByDiscriminatorValue( + subclassesByDiscriminatorValueLocal, + subclassDiscriminatorValue, + subclass.getEntityName() + ); + + //copy/paste from EntityMetamodel: + boolean subclassAbstract = subclass.isAbstract() == null + ? subclass.hasPojoRepresentation() && ReflectHelper.isAbstractClass( subclass.getMappedClass() ) + : subclass.isAbstract(); + + if ( !subclassAbstract ) { + values.add(subclassDiscriminatorValue); + sqlValues.add( getDiscriminatorSQLValue( subclass, dialect, factory ) ); } } } // Don't hold a reference to an empty HashMap: subclassesByDiscriminatorValue = CollectionHelper.toSmallMap( subclassesByDiscriminatorValueLocal ); + fullDiscriminatorSQLValues = ArrayHelper.toStringArray( sqlValues ); + fullDiscriminatorValues = ArrayHelper.toObjectArray( values ); initSubclassPropertyAliasesMap( persistentClass ); postConstruct( creationContext.getMetadata() ); + } + private static BasicType getDiscriminatorType(PersistentClass persistentClass) { + Type discriminatorType = persistentClass.getDiscriminator().getType(); + if ( discriminatorType instanceof BasicType ) { + return (BasicType) discriminatorType; + } + else { + throw new MappingException( "Illegal discriminator type: " + discriminatorType.getName() ); + } + } + + private static String getDiscriminatorSQLValue( + PersistentClass persistentClass, + Dialect dialect, + SessionFactoryImplementor factory) { + if ( persistentClass.isDiscriminatorValueNull() ) { + return InFragment.NULL; + } + else if ( persistentClass.isDiscriminatorValueNotNull() ) { + return InFragment.NOT_NULL; + } + else { + BasicType discriminatorType = getDiscriminatorType( persistentClass ); + Object discriminatorValue = getDiscriminatorValue( persistentClass ); + try { + JdbcLiteralFormatter literalFormatter = discriminatorType.getJdbcType() + .getJdbcLiteralFormatter( discriminatorType.getJavaTypeDescriptor() ); + return literalFormatter.toJdbcLiteral( + discriminatorValue, + dialect, + factory.getWrapperOptions() + ); + } + catch (Exception e) { + throw new MappingException( "Could not format discriminator value to SQL string", e ); + } + } + } + + private static Object getDiscriminatorValue(PersistentClass persistentClass) { + if ( persistentClass.isDiscriminatorValueNull() ) { + return NULL_DISCRIMINATOR; + } + else if ( persistentClass.isDiscriminatorValueNotNull() ) { + return NOT_NULL_DISCRIMINATOR; + } + else { + BasicType discriminatorType = getDiscriminatorType( persistentClass ); + try { + return discriminatorType.getJavaTypeDescriptor().fromString( persistentClass.getDiscriminatorValue() ); + } + catch (Exception e) { + throw new MappingException( "Could not parse discriminator value", e ); + } + } + } + + private static boolean isDiscriminatorInsertable(PersistentClass persistentClass) { + return !persistentClass.isDiscriminatorValueNull() + && !persistentClass.isDiscriminatorValueNotNull() + && persistentClass.isDiscriminatorInsertable() + && !persistentClass.getDiscriminator().hasFormula(); } private static void addSubclassByDiscriminatorValue(Map subclassesByDiscriminatorValue, Object discriminatorValue, String entityName) { @@ -632,12 +664,7 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { frag.setColumn( alias, getDiscriminatorColumnName() ); } - if ( hasTreatAs ) { - frag.addValues( decodeTreatAsRequests( treatAsDeclarations ) ); - } - else { - frag.addValues( fullDiscriminatorSQLValues() ); - } + frag.addValues( hasTreatAs ? decodeTreatAsRequests( treatAsDeclarations ) : fullDiscriminatorSQLValues ); return frag.toFragmentString(); } @@ -649,7 +676,6 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { private String[] decodeTreatAsRequests(Set treatAsDeclarations) { final List values = new ArrayList<>(); for ( String subclass : treatAsDeclarations ) { - //TODO: move getDiscriminatorSQLValue() to Loadable to get rid of Queryable final Queryable queryable = (Queryable) getFactory() .getRuntimeMetamodels() .getMappingMetamodel() @@ -679,48 +705,6 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { return ArrayHelper.toStringArray( values ); } - private String[] fullDiscriminatorSQLValues; - - private String[] fullDiscriminatorSQLValues() { - String[] fullDiscriminatorSQLValues = this.fullDiscriminatorSQLValues; - if ( fullDiscriminatorSQLValues == null ) { - // first access; build it - final List values = new ArrayList<>(); - for ( String subclass : getSubclassClosure() ) { - final Queryable queryable = (Queryable) getFactory().getRuntimeMetamodels() - .getMappingMetamodel() - .getEntityDescriptor( subclass ); - if ( !queryable.isAbstract() ) { - values.add( queryable.getDiscriminatorSQLValue() ); - } - } - this.fullDiscriminatorSQLValues = fullDiscriminatorSQLValues = ArrayHelper.toStringArray( values ); - } - - return fullDiscriminatorSQLValues; - } - - private Object[] fullDiscriminatorValues; - - private Object[] fullDiscriminatorValues() { - Object[] fullDiscriminatorValues = this.fullDiscriminatorValues; - if ( fullDiscriminatorValues == null ) { - // first access; build it - final List values = new ArrayList<>(); - for ( String subclass : getSubclassClosure() ) { - final Loadable queryable = (Loadable) getFactory().getRuntimeMetamodels() - .getMappingMetamodel() - .getEntityDescriptor( subclass ); - if ( !queryable.isAbstract() ) { - values.add( queryable.getDiscriminatorValue() ); - } - } - this.fullDiscriminatorValues = fullDiscriminatorValues = values.toArray(new Object[0]); - } - - return fullDiscriminatorValues; - } - @Override public String getSubclassPropertyTableName(int i) { return subclassTableNameClosure[subclassPropertyTableNumberClosure[i]]; @@ -903,10 +887,9 @@ public class SingleTableEntityPersister extends AbstractEntityPersister { ); if ( hasSubclasses() ) { - final Object[] discriminatorValues = fullDiscriminatorValues(); - final List values = new ArrayList<>( discriminatorValues.length ); + final List values = new ArrayList<>( fullDiscriminatorValues.length ); boolean hasNull = false, hasNonNull = false; - for ( Object discriminatorValue : discriminatorValues ) { + for ( Object discriminatorValue : fullDiscriminatorValues) { if ( discriminatorValue == NULL_DISCRIMINATOR ) { hasNull = true; } diff --git a/hibernate-core/src/main/java/org/hibernate/type/BasicType.java b/hibernate-core/src/main/java/org/hibernate/type/BasicType.java index b8205d5231..ad9f204b3e 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/BasicType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/BasicType.java @@ -48,8 +48,6 @@ public interface BasicType extends Type, BasicDomainType, MappingType, Bas return getJavaTypeDescriptor(); } - - @Override default int forEachJdbcType(IndexedConsumer action) { action.accept( 0, this ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java index 8a6bd67662..34d80b8576 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/annotations/inheritance/singletable/DuplicatedDiscriminatorValueTest.java @@ -43,7 +43,7 @@ public class DuplicatedDiscriminatorValueTest { fail( MappingException.class.getName() + " expected when two subclasses are mapped with the same discriminator value." ); } catch ( MappingException e ) { - final String errorMsg = e.getCause().getMessage(); + final String errorMsg = e.getMessage(); // Check if error message contains descriptive information. assertTrue( errorMsg.contains( Building1.class.getName() ) ); assertTrue( errorMsg.contains( Building2.class.getName() ) );