From d7d6e2882f47fdf50c56449eeb37b4ea04811af1 Mon Sep 17 00:00:00 2001 From: Chris Cranford Date: Mon, 22 Oct 2018 18:21:07 -0400 Subject: [PATCH] HHH-13042 HHH-13044 HHH-13053 - Fix to short-circuit delayed identifier insert forcing them to insert early. --- .../action/internal/EntityAction.java | 4 +- .../SessionFactoryOptionsBuilder.java | 15 ++- .../boot/spi/SessionFactoryOptions.java | 4 + .../org/hibernate/cfg/AvailableSettings.java | 15 +++ .../internal/AbstractSaveEventListener.java | 28 ++++- .../entity/AbstractEntityPersister.java | 64 +++++++++++ .../persister/entity/EntityPersister.java | 4 + ...lushModeWithIdentitySelfReferenceTest.java | 106 ++++++++++++++++-- 8 files changed, 222 insertions(+), 18 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java index 35d2ce984c..90d9b41db5 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityAction.java @@ -12,6 +12,7 @@ import org.hibernate.AssertionFailure; import org.hibernate.action.spi.AfterTransactionCompletionProcess; import org.hibernate.action.spi.BeforeTransactionCompletionProcess; import org.hibernate.action.spi.Executable; +import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.service.spi.EventListenerGroup; import org.hibernate.event.service.spi.EventListenerRegistry; @@ -100,7 +101,8 @@ public abstract class EntityAction */ public final Serializable getId() { if ( id instanceof DelayedPostInsertIdentifier ) { - final Serializable eeId = session.getPersistenceContext().getEntry( instance ).getId(); + final EntityEntry entry = session.getPersistenceContext().getEntry( instance ); + final Serializable eeId = entry == null ? null : entry.getId(); return eeId instanceof DelayedPostInsertIdentifier ? null : eeId; } return id; diff --git a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java index a142ab5b7a..9712744e77 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/internal/SessionFactoryOptionsBuilder.java @@ -83,6 +83,7 @@ import static org.hibernate.cfg.AvailableSettings.CUSTOM_ENTITY_DIRTINESS_STRATE import static org.hibernate.cfg.AvailableSettings.DEFAULT_BATCH_FETCH_SIZE; import static org.hibernate.cfg.AvailableSettings.DEFAULT_ENTITY_MODE; import static org.hibernate.cfg.AvailableSettings.DELAY_ENTITY_LOADER_CREATIONS; +import static org.hibernate.cfg.AvailableSettings.DISABLE_DELAYED_IDENTIFIER_POST_INSERTS; import static org.hibernate.cfg.AvailableSettings.ENABLE_LAZY_LOAD_NO_TRANS; import static org.hibernate.cfg.AvailableSettings.FAIL_ON_PAGINATION_OVER_COLLECTION_FETCH; import static org.hibernate.cfg.AvailableSettings.FLUSH_BEFORE_COMPLETION; @@ -193,7 +194,7 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions { private NullPrecedence defaultNullPrecedence; private boolean orderUpdatesEnabled; private boolean orderInsertsEnabled; - + private boolean postInsertIdentifierDelayed; // multi-tenancy private MultiTenancyStrategy multiTenancyStrategy; @@ -341,6 +342,9 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions { this.defaultNullPrecedence = NullPrecedence.parse( defaultNullPrecedence ); this.orderUpdatesEnabled = ConfigurationHelper.getBoolean( ORDER_UPDATES, configurationSettings ); this.orderInsertsEnabled = ConfigurationHelper.getBoolean( ORDER_INSERTS, configurationSettings ); + this.postInsertIdentifierDelayed = !ConfigurationHelper.getBoolean( + DISABLE_DELAYED_IDENTIFIER_POST_INSERTS, configurationSettings, false + ); this.jtaTrackByThread = cfgService.getSetting( JTA_TRACK_BY_THREAD, BOOLEAN, true ); @@ -1044,6 +1048,11 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions { return queryStatisticsMaxSize; } + @Override + public boolean isPostInsertIdentifierDelayableEnabled() { + return postInsertIdentifierDelayed; + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // In-flight mutation access @@ -1175,6 +1184,10 @@ public class SessionFactoryOptionsBuilder implements SessionFactoryOptions { this.orderUpdatesEnabled = enabled; } + public void enableDelayedIdentityInserts(boolean enabled) { + this.postInsertIdentifierDelayed = enabled; + } + public void applyMultiTenancyStrategy(MultiTenancyStrategy strategy) { this.multiTenancyStrategy = strategy; } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java index a8e0695f47..dd65549056 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/spi/SessionFactoryOptions.java @@ -295,4 +295,8 @@ public interface SessionFactoryOptions { default int getQueryStatisticsMaxSize() { return Statistics.DEFAULT_QUERY_STATISTICS_MAX_SIZE; } + + default boolean isPostInsertIdentifierDelayableEnabled() { + return true; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java index 013124408f..64bdd79fb3 100644 --- a/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java +++ b/hibernate-core/src/main/java/org/hibernate/cfg/AvailableSettings.java @@ -1992,4 +1992,19 @@ public interface AvailableSettings extends org.hibernate.jpa.AvailableSettings { * @since 5.4 */ String SEQUENCE_INCREMENT_SIZE_MISMATCH_STRATEGY = "hibernate.id.sequence.increment_size_mismatch_strategy"; + + /** + * This setting controls whether the default behavior for delayed identity inserts is disabled. + *

+ * Hibernate defines a set of rules that are used to determine if an insert statement that has + * an identity-based column can be delayed until flush/commit or if the statement must be + * executed early because access to the identity column is needed. + *

+ * In the event that those defined rules are insufficient for a given mapping scenario, this + * setting can be set to {@code true} to permanently disable delayed identity inserts for all + * transactions as a short-term workaround. + *

+ * The default value is {@code false}. + */ + String DISABLE_DELAYED_IDENTIFIER_POST_INSERTS = "hibernate.id.disable_delayed_identifier_post_inserts"; } diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java index 120f6c4e0d..4f99ae5bc8 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractSaveEventListener.java @@ -248,7 +248,7 @@ public abstract class AbstractSaveEventListener Serializable id = key == null ? null : key.getIdentifier(); - boolean shouldDelayIdentityInserts = shouldDelayIdentityInserts( requiresImmediateIdAccess, source ); + boolean shouldDelayIdentityInserts = shouldDelayIdentityInserts( requiresImmediateIdAccess, source, persister ); // Put a placeholder in entries, so we don't recurse back and try to save() the // same object again. QUESTION: should this be done before onSave() is called? @@ -320,14 +320,19 @@ public abstract class AbstractSaveEventListener return id; } - private static boolean shouldDelayIdentityInserts(boolean requiresImmediateIdAccess, EventSource source) { - return shouldDelayIdentityInserts( requiresImmediateIdAccess, isPartOfTransaction( source ), source.getHibernateFlushMode() ); + private static boolean shouldDelayIdentityInserts(boolean requiresImmediateIdAccess, EventSource source, EntityPersister persister) { + return shouldDelayIdentityInserts( requiresImmediateIdAccess, isPartOfTransaction( source ), source.getHibernateFlushMode(), persister ); } private static boolean shouldDelayIdentityInserts( boolean requiresImmediateIdAccess, boolean partOfTransaction, - FlushMode flushMode) { + FlushMode flushMode, + EntityPersister persister) { + if ( !persister.getFactory().getSessionFactoryOptions().isPostInsertIdentifierDelayableEnabled() ) { + return false; + } + if ( requiresImmediateIdAccess ) { // todo : make this configurable? as a way to support this behavior with Session#save etc return false; @@ -336,8 +341,19 @@ public abstract class AbstractSaveEventListener // otherwise we should delay the IDENTITY insertions if either: // 1) we are not part of a transaction // 2) we are in FlushMode MANUAL or COMMIT (not AUTO nor ALWAYS) - return !partOfTransaction || flushMode == MANUAL || flushMode == COMMIT; - + if ( !partOfTransaction || flushMode == MANUAL || flushMode == COMMIT ) { + if ( persister.canIdentityInsertBeDelayed() ) { + return true; + } + LOG.debugf( + "Identity insert for entity [%s] should be delayed; however the persister requested early insert.", + persister.getEntityName() + ); + return false; + } + else { + return false; + } } private static boolean isPartOfTransaction(EventSource source) { diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 747701e884..3f759cc6f9 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -76,9 +76,11 @@ import org.hibernate.engine.spi.PersistentAttributeInterceptable; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.ValueInclusion; +import org.hibernate.id.ForeignGenerator; import org.hibernate.id.IdentifierGenerator; import org.hibernate.id.PostInsertIdentifierGenerator; import org.hibernate.id.PostInsertIdentityPersister; +import org.hibernate.id.enhanced.SequenceStyleGenerator; import org.hibernate.id.insert.Binder; import org.hibernate.id.insert.InsertGeneratedIdentifierDelegate; import org.hibernate.internal.CoreLogging; @@ -106,6 +108,7 @@ import org.hibernate.mapping.Table; import org.hibernate.metadata.ClassMetadata; import org.hibernate.metamodel.model.domain.NavigableRole; import org.hibernate.persister.collection.CollectionPersister; +import org.hibernate.persister.collection.QueryableCollection; import org.hibernate.persister.spi.PersisterCreationContext; import org.hibernate.persister.walking.internal.EntityIdentifierDefinitionHelper; import org.hibernate.persister.walking.spi.AttributeDefinition; @@ -294,6 +297,8 @@ public abstract class AbstractEntityPersister private final boolean useReferenceCacheEntries; + private boolean canIdentityInsertBeDelayed; + protected void addDiscriminatorToInsert(Insert insert) { } @@ -956,6 +961,11 @@ public abstract class AbstractEntityPersister return useReferenceCacheEntries; } + @Override + public boolean canIdentityInsertBeDelayed() { + return canIdentityInsertBeDelayed; + } + protected static String getTemplateFromString(String string, SessionFactoryImplementor factory) { return string == null ? null : @@ -4148,6 +4158,58 @@ public abstract class AbstractEntityPersister return new SubstituteBracketSQLQueryParser( sql, getFactory() ).process(); } + private void resolveIdentityInsertDelayable() { + // By default they can + // The remainder of this method checks use cases where we shouldn't permit it. + canIdentityInsertBeDelayed = true; + + if ( getEntityMetamodel().getIdentifierProperty().isIdentifierAssignedByInsert() ) { + // if the persister's identifier is assigned by insert, we need to see if we must force non-delay mode. + for ( NonIdentifierAttribute attribute : getEntityMetamodel().getProperties() ) { + if ( isTypeSelfReferencing( attribute.getType() ) ) { + canIdentityInsertBeDelayed = false; + } + } + } + } + + private boolean isTypeSelfReferencing(Type propertyType) { + if ( propertyType.isAssociationType() ) { + if ( propertyType.isEntityType() ) { + Class entityClass = propertyType.getReturnedClass(); + if ( getMappedClass().equals( propertyType.getReturnedClass() ) ) { + return true; + } + } + else if ( propertyType.isCollectionType() ) { + // Association is a collection where owner needs identifier up-front + final CollectionType collection = (CollectionType) propertyType; + final CollectionPersister collectionPersister = getFactory().getMetamodel().collectionPersister( collection.getRole() ); + if ( collectionPersister.isInverse() ) { + final EntityPersister entityPersister = collectionPersister.getOwnerEntityPersister(); + if ( collectionPersister.getOwnerEntityPersister().equals( this ) ) { + final QueryableCollection queryableCollection = (QueryableCollection) collectionPersister; + final IdentifierGenerator identifierGenerator = queryableCollection.getElementPersister().getIdentifierGenerator(); + // todo - perhaps this can be simplified + if ( ( identifierGenerator instanceof ForeignGenerator ) || ( identifierGenerator instanceof SequenceStyleGenerator ) ) { + return true; + } + } + } + } + } + else if ( propertyType.isComponentType() ) { + final CompositeType componentType = (CompositeType) propertyType; + for ( Type componentSubType : componentType.getSubtypes() ) { + if ( isTypeSelfReferencing( componentSubType ) ) { + return true; + } + } + } + + return false; + } + public final void postInstantiate() throws MappingException { doLateInit(); @@ -4155,6 +4217,8 @@ public abstract class AbstractEntityPersister createUniqueKeyLoaders(); createQueryLoader(); + resolveIdentityInsertDelayable(); + doPostInstantiate(); } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java index eb47a19382..dafe858c46 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/EntityPersister.java @@ -811,4 +811,8 @@ public interface EntityPersister extends EntityDefinition { int[] resolveAttributeIndexes(String[] attributeNames); boolean canUseReferenceCacheEntries(); + + default boolean canIdentityInsertBeDelayed() { + return false; + } } diff --git a/hibernate-core/src/test/java/org/hibernate/test/flush/TestFlushModeWithIdentitySelfReferenceTest.java b/hibernate-core/src/test/java/org/hibernate/test/flush/TestFlushModeWithIdentitySelfReferenceTest.java index c5d2b0c5e6..c598634a64 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/flush/TestFlushModeWithIdentitySelfReferenceTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/flush/TestFlushModeWithIdentitySelfReferenceTest.java @@ -1,18 +1,14 @@ /* - * Copyright 2014 JBoss Inc + * Hibernate, Relational Persistence for Idiomatic Java * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on - * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the - * specific language governing permissions and limitations under the License. + * 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.test.flush; import javax.persistence.CascadeType; +import javax.persistence.Embeddable; +import javax.persistence.Embedded; import javax.persistence.Entity; import javax.persistence.GeneratedValue; import javax.persistence.GenerationType; @@ -40,7 +36,7 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction @Override protected Class[] getAnnotatedClasses() { - return new Class[] { SelfRefEntity.class }; + return new Class[] { SelfRefEntity.class, SelfRefEntityWithEmbeddable.class }; } @Test @@ -49,6 +45,7 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction try { session.setHibernateFlushMode( FlushMode.COMMIT ); loadAndAssert( session, createAndInsertEntity( session ) ); + loadAndInsert( session, createAndInsertEntityEmbeddable( session ) ); } finally { session.close(); @@ -61,6 +58,7 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction try { session.setHibernateFlushMode( FlushMode.MANUAL ); loadAndAssert( session, createAndInsertEntity( session ) ); + loadAndInsert( session, createAndInsertEntityEmbeddable( session ) ); } finally { session.close(); @@ -73,6 +71,7 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction try { session.setHibernateFlushMode( FlushMode.AUTO ); loadAndAssert( session, createAndInsertEntity( session ) ); + loadAndInsert( session, createAndInsertEntityEmbeddable( session ) ); } finally { session.close(); @@ -86,6 +85,7 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction SelfRefEntity entity = new SelfRefEntity(); entity.setSelfRefEntity( entity ); entity.setData( "test" ); + entity = (SelfRefEntity) session.merge( entity ); // only during manual flush do we want to force a flush prior to commit @@ -105,6 +105,37 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction } } + private SelfRefEntityWithEmbeddable createAndInsertEntityEmbeddable(Session session) { + try { + session.getTransaction().begin(); + + SelfRefEntityWithEmbeddable entity = new SelfRefEntityWithEmbeddable(); + entity.setData( "test" ); + + SelfRefEntityInfo info = new SelfRefEntityInfo(); + info.setSeflRefEntityEmbedded( entity ); + + entity.setInfo( info ); + + entity = (SelfRefEntityWithEmbeddable) session.merge( entity ); + + // only during manual flush do we want to force a flush prior to commit + if ( session.getHibernateFlushMode().equals( FlushMode.MANUAL ) ) { + session.flush(); + } + + session.getTransaction().commit(); + + return entity; + } + catch ( Exception e ) { + if ( session.getTransaction().isActive() ) { + session.getTransaction().rollback(); + } + throw e; + } + } + private void loadAndAssert(Session session, SelfRefEntity mergedEntity) { final SelfRefEntity loadedEntity = session.get( SelfRefEntity.class, mergedEntity.getId() ); assertNotNull( "Expected to find the merged entity but did not.", loadedEntity ); @@ -112,6 +143,13 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction assertNotNull( "Expected a non-null self reference", loadedEntity.getSelfRefEntity() ); } + private void loadAndInsert(Session session, SelfRefEntityWithEmbeddable mergedEntity) { + final SelfRefEntityWithEmbeddable loadedEntity = session.get( SelfRefEntityWithEmbeddable.class, mergedEntity.getId() ); + assertNotNull( "Expected to find the merged entity but did not.", loadedEntity ); + assertEquals( "test", loadedEntity.getData() ); + assertNotNull( "Expected a non-null self reference in embeddable", loadedEntity.getInfo().getSeflRefEntityEmbedded() ); + } + @Entity(name = "SelfRefEntity") public static class SelfRefEntity { @Id @@ -145,4 +183,52 @@ public class TestFlushModeWithIdentitySelfReferenceTest extends BaseCoreFunction this.data = data; } } + + @Entity(name = "SelfRefEntityWithEmbeddable") + public static class SelfRefEntityWithEmbeddable { + @Id + @GeneratedValue(strategy = GenerationType.IDENTITY) + private Integer id; + @Embedded + private SelfRefEntityInfo info; + private String data; + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public SelfRefEntityInfo getInfo() { + return info; + } + + public void setInfo(SelfRefEntityInfo info) { + this.info = info; + } + + public String getData() { + return data; + } + + public void setData(String data) { + this.data = data; + } + } + + @Embeddable + public static class SelfRefEntityInfo { + @OneToOne(cascade = CascadeType.ALL, optional = true) + private SelfRefEntityWithEmbeddable seflRefEntityEmbedded; + + public SelfRefEntityWithEmbeddable getSeflRefEntityEmbedded() { + return seflRefEntityEmbedded; + } + + public void setSeflRefEntityEmbedded(SelfRefEntityWithEmbeddable seflRefEntityEmbedded) { + this.seflRefEntityEmbedded = seflRefEntityEmbedded; + } + } }