From e5c239c7c844533f99a4b86374edaf01fff16674 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 24 Feb 2021 14:25:19 -0600 Subject: [PATCH] natural-id caching - on top of Christian's PR #3735 which fixes a problem with pulling entity snapshots from the database which effects natural-id handling and caused test failures here (b4 #3735) --- .../internal/AbstractEntityInsertAction.java | 6 +- .../action/internal/EntityDeleteAction.java | 2 +- .../action/internal/EntityUpdateAction.java | 7 +- .../java/org/hibernate/boot/BootLogging.java | 6 +- .../internal/NaturalIdResolutionsImpl.java | 100 ++++++++++++------ .../engine/internal/TwoPhaseLoad.java | 8 +- .../engine/spi/NaturalIdResolutions.java | 1 + .../internal/DefaultLoadEventListener.java | 2 +- .../org/hibernate/internal/CoreLogging.java | 4 + .../internal/DatabaseSnapshotExecutor.java | 7 +- .../metamodel/mapping/NaturalIdLogging.java | 22 ++++ .../metamodel/mapping/NaturalIdMapping.java | 4 +- .../internal/CompoundNaturalIdMapping.java | 8 +- .../internal/SimpleNaturalIdMapping.java | 16 ++- .../entity/AbstractEntityPersister.java | 8 +- .../entity/AbstractEntityInitializer.java | 4 +- .../naturalid/BasicNaturalIdCachingTests.java | 2 + .../MappedSuperclassOverrideTest.java | 9 +- .../cache/InheritedNaturalIdCacheTest.java | 51 ++++----- .../cached/CachedMutableNaturalIdTest.java | 14 ++- .../src/test/resources/log4j.properties | 2 + 21 files changed, 188 insertions(+), 95 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdLogging.java diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java index 6cc89f8adf..6099dcae6b 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/AbstractEntityInsertAction.java @@ -162,12 +162,12 @@ public abstract class AbstractEntityInsertAction extends EntityAction { * Handle sending notifications needed for natural-id before saving */ protected void handleNaturalIdPreSaveNotifications() { - // before save, we need to add a local (transactional) natural id cross-reference + // before save, we need to add a natural id cross-reference to the persistence-context final NaturalIdMapping naturalIdMapping = getPersister().getNaturalIdMapping(); if ( naturalIdMapping != null ) { getSession().getPersistenceContextInternal().getNaturalIdResolutions().manageLocalResolution( getId(), - naturalIdMapping.extractNaturalIdValues( state, getSession() ), + naturalIdMapping.extractNaturalIdFromEntityState( state, getSession() ), getPersister(), CachedNaturalIdValueSource.INSERT ); @@ -185,7 +185,7 @@ public abstract class AbstractEntityInsertAction extends EntityAction { return; } - final Object naturalIdValues = naturalIdMapping.extractNaturalIdValues( state, getSession() ); + final Object naturalIdValues = naturalIdMapping.extractNaturalIdFromEntityState( state, getSession() ); if ( isEarlyInsert() ) { // with early insert, we still need to add a local (transactional) natural id cross-reference diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java index bed4523cfc..6e7fb8359f 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityDeleteAction.java @@ -65,7 +65,7 @@ public class EntityDeleteAction extends EntityAction { if ( naturalIdMapping != null ) { naturalIdValues = session.getPersistenceContextInternal().getNaturalIdResolutions().removeLocalResolution( getId(), - naturalIdMapping.extractNaturalIdValues( state, session ), + naturalIdMapping.extractNaturalIdFromEntityState( state, session ), getPersister() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java index a61731da37..e08cc58a10 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/EntityUpdateAction.java @@ -21,7 +21,6 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.Status; import org.hibernate.event.service.spi.EventListenerGroup; -import org.hibernate.event.spi.EventType; import org.hibernate.event.spi.PostCommitUpdateEventListener; import org.hibernate.event.spi.PostUpdateEvent; import org.hibernate.event.spi.PostUpdateEventListener; @@ -94,7 +93,7 @@ public class EntityUpdateAction extends EntityAction { this.previousNaturalIdValues = determinePreviousNaturalIdValues( persister, naturalIdMapping, id, previousState, session ); session.getPersistenceContextInternal().getNaturalIdResolutions().manageLocalResolution( id, - naturalIdMapping.extractNaturalIdValues( state, session ), + naturalIdMapping.extractNaturalIdFromEntityState( state, session ), persister, CachedNaturalIdValueSource.UPDATE ); @@ -109,7 +108,7 @@ public class EntityUpdateAction extends EntityAction { SharedSessionContractImplementor session) { final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); if ( previousState != null ) { - return naturalIdMapping.extractNaturalIdValues( previousState, session ); + return naturalIdMapping.extractNaturalIdFromEntityState( previousState, session ); } return persistenceContext.getNaturalIdSnapshot( id, persister ); @@ -226,7 +225,7 @@ public class EntityUpdateAction extends EntityAction { if ( naturalIdMapping != null ) { session.getPersistenceContextInternal().getNaturalIdResolutions().manageSharedResolution( id, - naturalIdMapping.extractNaturalIdValues( state, session ), + naturalIdMapping.extractNaturalIdFromEntityState( state, session ), previousNaturalIdValues, persister, CachedNaturalIdValueSource.UPDATE diff --git a/hibernate-core/src/main/java/org/hibernate/boot/BootLogging.java b/hibernate-core/src/main/java/org/hibernate/boot/BootLogging.java index df261dfa62..53e04ae908 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/BootLogging.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/BootLogging.java @@ -8,11 +8,13 @@ package org.hibernate.boot; import org.jboss.logging.Logger; +import static org.hibernate.internal.CoreLogging.subsystemLoggerName; + /** - * @author Steve Ebersole + * Logging related to Hibernate bootstrapping */ public class BootLogging { - public static final String NAME = "org.hibernate.orm.boot"; + public static String NAME = subsystemLoggerName( "boot" ); public static final Logger LOGGER = Logger.getLogger( NAME ); diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java index fafd6852da..92304b5462 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java @@ -25,6 +25,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.event.spi.EventSource; import org.hibernate.metamodel.mapping.EntityMappingType; +import org.hibernate.metamodel.mapping.NaturalIdLogging; import org.hibernate.metamodel.mapping.NaturalIdMapping; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.sql.results.LoadingLogger; @@ -66,6 +67,13 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa @Override public void cacheResolutionFromLoad(Object id, Object naturalId, EntityMappingType entityDescriptor) { + NaturalIdLogging.LOGGER.debugf( + "Caching resolution natural-id resolution from load (%s) : `%s` -> `%s`", + entityDescriptor.getEntityName(), + naturalId, + id + ); + final EntityPersister persister = locatePersisterForKey( entityDescriptor.getEntityPersister() ); final NaturalIdMapping naturalIdMapping = entityDescriptor.getNaturalIdMapping(); @@ -88,11 +96,18 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa /** * Private, but see {@link #cacheResolution} for public version */ - private boolean cacheResolutionLocally(Object pk, Object naturalId, EntityMappingType entityDescriptor) { + private boolean cacheResolutionLocally(Object id, Object naturalId, EntityMappingType entityDescriptor) { // by the time we get here we assume that the natural-id value has already been validated so just do an assert assert entityDescriptor.getNaturalIdMapping() != null; assert isValidValue( naturalId, entityDescriptor ); + NaturalIdLogging.LOGGER.debugf( + "Locally caching natural-id resolution (%s) : `%s` -> `%s`", + entityDescriptor.getEntityName(), + naturalId, + id + ); + final EntityMappingType rootEntityDescriptor = entityDescriptor.getRootEntityDescriptor(); final EntityResolutions previousEntry = resolutionsByEntity.get( rootEntityDescriptor ); final EntityResolutions resolutions; @@ -104,7 +119,7 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa resolutionsByEntity.put( rootEntityDescriptor, resolutions ); } - return resolutions.cache( pk, naturalId ); + return resolutions.cache( id, naturalId ); } @Override @@ -157,6 +172,13 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa @Override public Object removeLocalResolution(Object id, Object naturalId, EntityMappingType entityDescriptor) { + NaturalIdLogging.LOGGER.debugf( + "Removing locally cached natural-id resolution (%s) : `%s` -> `%s`", + entityDescriptor.getEntityName(), + naturalId, + id + ); + final NaturalIdMapping naturalIdMapping = entityDescriptor.getNaturalIdMapping(); if ( naturalIdMapping == null ) { @@ -175,6 +197,11 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa return localNaturalIdValues != null ? localNaturalIdValues : naturalId; } + /** + * Removes the cross-reference from both Session cache (Resolution) as well as the + * second-level cache (NaturalIdDataAccess). Returns the natural-id value previously + * cached on the Session + */ private Object removeNaturalIdCrossReference(Object id, Object naturalIdValue, EntityPersister persister) { validateNaturalId( persister, naturalIdValue ); @@ -238,24 +265,29 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa Object naturalIdValues, Object previousNaturalIdValues, CachedNaturalIdValueSource source) { + final NaturalIdDataAccess cacheAccess = persister.getNaturalIdMapping().getCacheAccess(); + + if ( cacheAccess == null ) { + return; + } + final EntityMappingType rootEntityDescriptor = persister.getRootEntityDescriptor(); final EntityPersister rootEntityPersister = rootEntityDescriptor.getEntityPersister(); - final NaturalIdDataAccess naturalIdCacheAccessStrategy = persister.getNaturalIdCacheAccessStrategy(); - final Object naturalIdCacheKey = naturalIdCacheAccessStrategy.generateCacheKey( naturalIdValues, rootEntityPersister, session() ); + final Object cacheKey = cacheAccess.generateCacheKey( naturalIdValues, rootEntityPersister, session() ); final SessionFactoryImplementor factory = session().getFactory(); final StatisticsImplementor statistics = factory.getStatistics(); switch ( source ) { case LOAD: { - if ( CacheHelper.fromSharedCache( session(), naturalIdCacheKey, naturalIdCacheAccessStrategy ) != null ) { + if ( CacheHelper.fromSharedCache( session(), cacheKey, cacheAccess ) != null ) { // prevent identical re-cachings return; } - final boolean put = naturalIdCacheAccessStrategy.putFromLoad( + final boolean put = cacheAccess.putFromLoad( session(), - naturalIdCacheKey, + cacheKey, id, null ); @@ -263,34 +295,34 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa if ( put && statistics.isStatisticsEnabled() ) { statistics.naturalIdCachePut( rootEntityDescriptor.getNavigableRole(), - naturalIdCacheAccessStrategy.getRegion().getName() + cacheAccess.getRegion().getName() ); } break; } case INSERT: { - final boolean put = naturalIdCacheAccessStrategy.insert( session(), naturalIdCacheKey, id ); + final boolean put = cacheAccess.insert( session(), cacheKey, id ); if ( put && statistics.isStatisticsEnabled() ) { statistics.naturalIdCachePut( rootEntityDescriptor.getNavigableRole(), - naturalIdCacheAccessStrategy.getRegion().getName() + cacheAccess.getRegion().getName() ); } ( (EventSource) session() ).getActionQueue().registerProcess( (success, session) -> { if ( success ) { - final boolean put1 = naturalIdCacheAccessStrategy.afterInsert( session, naturalIdCacheKey, id ); + final boolean put1 = cacheAccess.afterInsert( session, cacheKey, id ); if ( put1 && statistics.isStatisticsEnabled() ) { statistics.naturalIdCachePut( rootEntityDescriptor.getNavigableRole(), - naturalIdCacheAccessStrategy.getRegion().getName() + cacheAccess.getRegion().getName() ); } } else { - naturalIdCacheAccessStrategy.evict( naturalIdCacheKey ); + cacheAccess.evict( cacheKey ); } } ); @@ -298,30 +330,30 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa break; } case UPDATE: { - final Object previousCacheKey = naturalIdCacheAccessStrategy.generateCacheKey( previousNaturalIdValues, rootEntityPersister, session() ); - if ( naturalIdCacheKey.equals( previousCacheKey ) ) { + final Object previousCacheKey = cacheAccess.generateCacheKey( previousNaturalIdValues, rootEntityPersister, session() ); + if ( cacheKey.equals( previousCacheKey ) ) { // prevent identical re-caching, solves HHH-7309 return; } - final SoftLock removalLock = naturalIdCacheAccessStrategy.lockItem( session(), previousCacheKey, null ); - naturalIdCacheAccessStrategy.remove( session(), previousCacheKey); + final SoftLock removalLock = cacheAccess.lockItem( session(), previousCacheKey, null ); + cacheAccess.remove( session(), previousCacheKey); - final SoftLock lock = naturalIdCacheAccessStrategy.lockItem( session(), naturalIdCacheKey, null ); - final boolean put = naturalIdCacheAccessStrategy.update( session(), naturalIdCacheKey, id ); + final SoftLock lock = cacheAccess.lockItem( session(), cacheKey, null ); + final boolean put = cacheAccess.update( session(), cacheKey, id ); if ( put && statistics.isStatisticsEnabled() ) { statistics.naturalIdCachePut( rootEntityDescriptor.getNavigableRole(), - naturalIdCacheAccessStrategy.getRegion().getName() + cacheAccess.getRegion().getName() ); } ( (EventSource) session() ).getActionQueue().registerProcess( (success, session) -> { - naturalIdCacheAccessStrategy.unlockItem( session(), previousCacheKey, removalLock ); + cacheAccess.unlockItem( session(), previousCacheKey, removalLock ); if (success) { - final boolean put12 = naturalIdCacheAccessStrategy.afterUpdate( + final boolean put12 = cacheAccess.afterUpdate( session(), - naturalIdCacheKey, + cacheKey, id, lock ); @@ -329,12 +361,12 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa if ( put12 && statistics.isStatisticsEnabled() ) { statistics.naturalIdCachePut( rootEntityDescriptor.getNavigableRole(), - naturalIdCacheAccessStrategy.getRegion().getName() + cacheAccess.getRegion().getName() ); } } else { - naturalIdCacheAccessStrategy.unlockItem( session(), naturalIdCacheKey, lock ); + cacheAccess.unlockItem( session(), cacheKey, lock ); } } ); @@ -352,7 +384,14 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa @Override public void removeSharedResolution(Object id, Object naturalId, EntityMappingType entityDescriptor) { final NaturalIdMapping naturalIdMapping = entityDescriptor.getNaturalIdMapping(); - if ( naturalIdMapping == null || naturalIdMapping.getCacheAccess() == null ) { + if ( naturalIdMapping == null ) { + // nothing to do + return; + } + + final NaturalIdDataAccess cacheAccess = naturalIdMapping.getCacheAccess(); + + if ( cacheAccess == null ) { // nothing to do return; } @@ -363,14 +402,13 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa final EntityPersister persister = locatePersisterForKey( entityDescriptor.getEntityPersister() ); - final NaturalIdDataAccess naturalIdCacheAccessStrategy = persister.getNaturalIdCacheAccessStrategy(); - final Object naturalIdCacheKey = naturalIdCacheAccessStrategy.generateCacheKey( naturalId, persister, session() ); - naturalIdCacheAccessStrategy.evict( naturalIdCacheKey ); + final Object naturalIdCacheKey = cacheAccess.generateCacheKey( naturalId, persister, session() ); + cacheAccess.evict( naturalIdCacheKey ); // if ( sessionCachedNaturalIdValues != null // && !Arrays.equals( sessionCachedNaturalIdValues, deletedNaturalIdValues ) ) { // final NaturalIdCacheKey sessionNaturalIdCacheKey = new NaturalIdCacheKey( sessionCachedNaturalIdValues, persister, session ); -// naturalIdCacheAccessStrategy.evict( sessionNaturalIdCacheKey ); +// cacheAccess.evict( sessionNaturalIdCacheKey ); // } } @@ -383,7 +421,7 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa } final EntityPersister persister = locatePersisterForKey( entityDescriptor.getEntityPersister() ); - final Object naturalIdValuesFromCurrentObjectState = naturalIdMapping.extractNaturalIdValues( entity, session() ); + final Object naturalIdValuesFromCurrentObjectState = naturalIdMapping.extractNaturalIdFromEntity( entity, session() ); final boolean changed = ! sameAsCached( persister, pk, diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/TwoPhaseLoad.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/TwoPhaseLoad.java index b44c4184f5..296f1ef4f7 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/TwoPhaseLoad.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/TwoPhaseLoad.java @@ -24,8 +24,6 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.Status; import org.hibernate.event.service.spi.EventListenerGroup; -import org.hibernate.event.service.spi.EventListenerRegistry; -import org.hibernate.event.spi.EventType; import org.hibernate.event.spi.PostLoadEvent; import org.hibernate.event.spi.PostLoadEventListener; import org.hibernate.event.spi.PreLoadEvent; @@ -279,8 +277,8 @@ public final class TwoPhaseLoad { final SessionFactoryImplementor factory = session.getFactory(); final StatisticsImplementor statistics = factory.getStatistics(); - if ( persister.canWriteToCache() && session.getCacheMode().isPutEnabled() ) { + if ( persister.canWriteToCache() && session.getCacheMode().isPutEnabled() ) { if ( debugEnabled ) { LOG.debugf( "Adding entity to second-level cache: %s", @@ -335,7 +333,9 @@ public final class TwoPhaseLoad { if ( persister.hasNaturalIdentifier() ) { persistenceContext.getNaturalIdResolutions().cacheResolutionFromLoad( - id, persister.getNaturalIdMapping().extractNaturalIdValues( hydratedState, session ), persister + id, + persister.getNaturalIdMapping().extractNaturalIdFromEntityState( hydratedState, session ), + persister ); } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/NaturalIdResolutions.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/NaturalIdResolutions.java index 3759c86c15..9b15f97c1a 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/NaturalIdResolutions.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/NaturalIdResolutions.java @@ -51,6 +51,7 @@ public interface NaturalIdResolutions { /** * Removes any local cross-reference, returning the previously cached value if one. + * * Again, this only effects the persistence-context cache, not the L2 cache */ Object removeLocalResolution(Object id, Object naturalId, EntityMappingType entityDescriptor); diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java index aa34edd074..03c42482a2 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/DefaultLoadEventListener.java @@ -533,7 +533,7 @@ public class DefaultLoadEventListener implements LoadEventListener { if ( entity != null && persister.hasNaturalIdentifier() ) { final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); persistenceContext.getNaturalIdResolutions().cacheResolutionFromLoad( - event.getEntityId(), persister.getNaturalIdMapping().extractNaturalIdValues( entity, session ), persister + event.getEntityId(), persister.getNaturalIdMapping().extractNaturalIdFromEntity( entity, session ), persister ); } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CoreLogging.java b/hibernate-core/src/main/java/org/hibernate/internal/CoreLogging.java index c1e341b2a1..3db3ccbf22 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CoreLogging.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CoreLogging.java @@ -14,6 +14,10 @@ import org.jboss.logging.Logger; * @author Steve Ebersole */ public class CoreLogging { + public static String subsystemLoggerName(String subsystem) { + return "org.hibernate.orm." + subsystem; + } + /** * Disallow instantiation */ diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java index 03319782d6..c6a57590ab 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/DatabaseSnapshotExecutor.java @@ -28,6 +28,8 @@ import org.hibernate.query.sqm.sql.FromClauseIndex; import org.hibernate.sql.ast.Clause; import org.hibernate.sql.ast.SqlAstTranslatorFactory; import org.hibernate.sql.ast.spi.SqlAliasBaseManager; +import org.hibernate.sql.ast.spi.SqlExpressionResolver; +import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.sql.ast.tree.expression.ColumnReference; import org.hibernate.sql.ast.tree.expression.JdbcParameter; import org.hibernate.sql.ast.tree.expression.QueryLiteral; @@ -105,10 +107,13 @@ class DatabaseSnapshotExecutor { // We produce the same state array as if we were creating an entity snapshot final List domainResults = new ArrayList<>(); + final SqlExpressionResolver sqlExpressionResolver = state.getSqlExpressionResolver(); + // We just need a literal to have a result set domainResults.add( new QueryLiteral<>( null, IntegerType.INSTANCE ).createDomainResult( null, state ) ); + entityDescriptor.getIdentifierMapping().forEachSelection( (columnIndex, selection) -> { final TableReference tableReference = rootTableGroup.resolveTableReference( selection.getContainingTableExpression() ); @@ -116,7 +121,7 @@ class DatabaseSnapshotExecutor { final JdbcParameter jdbcParameter = new JdbcParameterImpl( selection.getJdbcMapping() ); jdbcParameters.add( jdbcParameter ); - final ColumnReference columnReference = (ColumnReference) state.getSqlExpressionResolver() + final ColumnReference columnReference = (ColumnReference) sqlExpressionResolver .resolveSqlExpression( createColumnReferenceKey( tableReference, selection.getSelectionExpression() ), s -> new ColumnReference( diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdLogging.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdLogging.java new file mode 100644 index 0000000000..0392f70647 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdLogging.java @@ -0,0 +1,22 @@ +/* + * 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.metamodel.mapping; + +import org.hibernate.internal.CoreLogging; + +import org.jboss.logging.Logger; + +/** + * Logging related to natural-id operations + */ +public interface NaturalIdLogging { + String LOGGER_NAME = CoreLogging.subsystemLoggerName( "mapping.natural_id" ); + Logger LOGGER = Logger.getLogger( LOGGER_NAME ); + + boolean DEBUG_ENABLED = LOGGER.isDebugEnabled(); + boolean TRACE_ENABLED = LOGGER.isTraceEnabled(); +} diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java index fd1048a455..1dd5909d39 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java @@ -57,7 +57,7 @@ public interface NaturalIdMapping extends VirtualModelPart { * * @return The extracted natural id values. This is a normalized */ - Object extractNaturalIdValues(Object[] state, SharedSessionContractImplementor session); + Object extractNaturalIdFromEntityState(Object[] state, SharedSessionContractImplementor session); /** * Given an entity instance, extract the normalized natural id representation @@ -66,7 +66,7 @@ public interface NaturalIdMapping extends VirtualModelPart { * * @return The extracted natural id values */ - Object extractNaturalIdValues(Object entity, SharedSessionContractImplementor session); + Object extractNaturalIdFromEntity(Object entity, SharedSessionContractImplementor session); /** diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java index 7d425d7ef5..ee1efb7eb7 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java @@ -108,7 +108,7 @@ public class CompoundNaturalIdMapping extends AbstractNaturalIdMapping implement } @Override - public Object[] extractNaturalIdValues(Object[] state, SharedSessionContractImplementor session) { + public Object[] extractNaturalIdFromEntityState(Object[] state, SharedSessionContractImplementor session) { if ( state == null ) { return null; } @@ -128,7 +128,7 @@ public class CompoundNaturalIdMapping extends AbstractNaturalIdMapping implement } @Override - public Object[] extractNaturalIdValues(Object entity, SharedSessionContractImplementor session) { + public Object[] extractNaturalIdFromEntity(Object entity, SharedSessionContractImplementor session) { final Object[] values = new Object[ attributes.size() ]; for ( int i = 0; i < attributes.size(); i++ ) { @@ -196,11 +196,11 @@ public class CompoundNaturalIdMapping extends AbstractNaturalIdMapping implement final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); final EntityPersister persister = getDeclaringType().getEntityPersister(); - final Object[] naturalId = extractNaturalIdValues( currentState, session ); + final Object[] naturalId = extractNaturalIdFromEntityState( currentState, session ); final Object snapshot = loadedState == null ? persistenceContext.getNaturalIdSnapshot( id, persister ) - : persister.getNaturalIdMapping().extractNaturalIdValues( loadedState, session ); + : persister.getNaturalIdMapping().extractNaturalIdFromEntityState( loadedState, session ); final Object[] previousNaturalId = (Object[]) snapshot; assert naturalId.length == getNaturalIdAttributes().size(); diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java index 871a8c7af2..62ea3ea130 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java @@ -61,10 +61,10 @@ public class SimpleNaturalIdMapping extends AbstractNaturalIdMapping { final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); final EntityPersister persister = getDeclaringType().getEntityPersister(); - final Object naturalId = extractNaturalIdValues( currentState, session ); + final Object naturalId = extractNaturalIdFromEntityState( currentState, session ); final Object snapshot = loadedState == null ? persistenceContext.getNaturalIdSnapshot( id, persister ) - : persister.getNaturalIdMapping().extractNaturalIdValues( loadedState, session ); + : persister.getNaturalIdMapping().extractNaturalIdFromEntityState( loadedState, session ); if ( ! areEqual( naturalId, snapshot, session ) ) { throw new HibernateException( @@ -79,12 +79,20 @@ public class SimpleNaturalIdMapping extends AbstractNaturalIdMapping { } @Override - public Object extractNaturalIdValues(Object[] state, SharedSessionContractImplementor session) { + public Object extractNaturalIdFromEntityState(Object[] state, SharedSessionContractImplementor session) { + if ( state == null ) { + return null; + } + + if ( state.length == 1 ) { + return state[0]; + } + return state[ attribute.getStateArrayPosition() ]; } @Override - public Object extractNaturalIdValues(Object entity, SharedSessionContractImplementor session) { + public Object extractNaturalIdFromEntity(Object entity, SharedSessionContractImplementor session) { return attribute.getPropertyAccess().getGetter().get( entity ); } 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 e2170e90bd..ac3c121e3e 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 @@ -5005,11 +5005,11 @@ public abstract class AbstractEntityPersister } private void handleNaturalIdReattachment(Object entity, SharedSessionContractImplementor session) { - if ( !hasNaturalIdentifier() ) { + if ( naturalIdMapping == null ) { return; } - if ( getEntityMetamodel().hasImmutableNaturalId() ) { + if ( ! naturalIdMapping.isMutable() ) { // we assume there were no changes to natural id during detachment for now, that is validated later // during flush. return; @@ -5027,13 +5027,13 @@ public abstract class AbstractEntityPersister naturalIdSnapshot = null; } else { - naturalIdSnapshot = naturalIdMapping.extractNaturalIdValues( entitySnapshot, session ); + naturalIdSnapshot = naturalIdMapping.extractNaturalIdFromEntityState( entitySnapshot, session ); } naturalIdResolutions.removeSharedResolution( id, naturalIdSnapshot, this ); naturalIdResolutions.manageLocalResolution( id, - naturalIdMapping.extractNaturalIdValues( entity, session ), + naturalIdMapping.extractNaturalIdFromEntity( entity, session ), this, CachedNaturalIdValueSource.UPDATE ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java index 4235dabad9..19bbdbe18c 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/AbstractEntityInitializer.java @@ -25,9 +25,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.engine.spi.Status; import org.hibernate.event.service.spi.EventListenerGroup; -import org.hibernate.event.service.spi.EventListenerRegistry; import org.hibernate.event.spi.EventSource; -import org.hibernate.event.spi.EventType; import org.hibernate.event.spi.PreLoadEvent; import org.hibernate.event.spi.PreLoadEventListener; import org.hibernate.internal.util.StringHelper; @@ -727,7 +725,7 @@ public abstract class AbstractEntityInitializer extends AbstractFetchParentAcces if ( entityDescriptor.getNaturalIdMapping() != null ) { persistenceContext.getNaturalIdResolutions().cacheResolutionFromLoad( entityIdentifier, - entityDescriptor.getNaturalIdMapping().extractNaturalIdValues( resolvedEntityState, session ), + entityDescriptor.getNaturalIdMapping().extractNaturalIdFromEntityState( resolvedEntityState, session ), entityDescriptor ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/BasicNaturalIdCachingTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/BasicNaturalIdCachingTests.java index 6ff17bd7a3..a6dc9d29cf 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/BasicNaturalIdCachingTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/BasicNaturalIdCachingTests.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -74,6 +75,7 @@ public class BasicNaturalIdCachingTests { final Object cacheKey = cacheAccess.generateCacheKey( "abc", entityPersister, session ); final Object cached = cacheAccess.get( session, cacheKey ); assertThat( cached, notNullValue() ); + assertThat( cached, equalTo( 1 ) ); } ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/MappedSuperclassOverrideTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/MappedSuperclassOverrideTest.java index 4a75e61e3b..2894501941 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/MappedSuperclassOverrideTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/MappedSuperclassOverrideTest.java @@ -20,11 +20,11 @@ import org.hibernate.testing.orm.junit.SessionFactory; import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @DomainModel( annotatedClasses = { MappedSuperclassOverrideTest.MyMappedSuperclass.class, MappedSuperclassOverrideTest.MyEntity.class } ) @SessionFactory( exportSchema = false ) -@FailureExpected( jiraKey = "HHH-12085" ) public class MappedSuperclassOverrideTest { @Test public void testModel(SessionFactoryScope scope) { @@ -32,7 +32,11 @@ public class MappedSuperclassOverrideTest { .getRuntimeMetamodels() .getEntityMappingType( MyEntity.class ) .getEntityPersister(); - assertTrue( entityPersister.hasNaturalIdentifier() ); + + // defining a natural-id on a sub-entity is not allowed, only on the root. + // - here, because the root does not declare `#getName` as a natural-id + // the hierarchy does not define a natural-id + assertFalse( entityPersister.hasNaturalIdentifier() ); } @MappedSuperclass @@ -77,7 +81,6 @@ public class MappedSuperclassOverrideTest { super( id, name ); } - // this should not be allowed, and supposedly fails anyway... @Override @NaturalId public String getName() { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/cache/InheritedNaturalIdCacheTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/cache/InheritedNaturalIdCacheTest.java index 2d163210db..17af821afb 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/cache/InheritedNaturalIdCacheTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/inheritance/cache/InheritedNaturalIdCacheTest.java @@ -47,10 +47,12 @@ public class InheritedNaturalIdCacheTest { } @Test - @NotImplementedYet( reason = "natural-id caching not yet implemented", strict = false ) + @NotImplementedYet( + reason = "We do not throw `WrongClassException` atm, we just return null", + strict = false + ) public void testLoadingInheritedEntitiesByNaturalId(SessionFactoryScope scope) { // load the entities "properly" by natural-id - scope.inTransaction( (session) -> { final MyEntity entity = session.bySimpleNaturalId( MyEntity.class ).load( "base" ); @@ -61,28 +63,7 @@ public class InheritedNaturalIdCacheTest { } ); - // finally, attempt to load MyEntity#1 as an ExtendedEntity, which should - // throw a WrongClassException - - scope.inTransaction( - (session) -> { - try { - session.bySimpleNaturalId( ExtendedEntity.class ).load( "base" ); - fail( "Expecting WrongClassException" ); - } - catch (WrongClassException expected) { - // expected outcome - } - catch (Exception other) { - throw new AssertionError( - "Unexpected exception type : " + other.getClass().getName(), - other - ); - } - } - ); - - // this is functionally equivalent to loading the wrong class by id... + // baseline: loading the wrong class by id... scope.inTransaction( (session) -> { @@ -102,6 +83,28 @@ public class InheritedNaturalIdCacheTest { } ); + // finally, attempt to load MyEntity#1 as an ExtendedEntity, which should + // throw a `WrongClassException` same as above with loading by id + // + // NOTE : this is what fails currently + scope.inTransaction( + (session) -> { + try { + session.bySimpleNaturalId( ExtendedEntity.class ).load( "base" ); + fail( "Expecting WrongClassException" ); + } + catch (WrongClassException expected) { + // expected outcome + } + catch (Exception other) { + throw new AssertionError( + "Unexpected exception type : " + other.getClass().getName(), + other + ); + } + } + ); + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/mutable/cached/CachedMutableNaturalIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/mutable/cached/CachedMutableNaturalIdTest.java index faf1939847..e906f1d4b6 100755 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/mutable/cached/CachedMutableNaturalIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/mutable/cached/CachedMutableNaturalIdTest.java @@ -7,6 +7,8 @@ package org.hibernate.orm.test.mapping.naturalid.mutable.cached; import org.hibernate.LockOptions; +import org.hibernate.cache.spi.CacheImplementor; +import org.hibernate.stat.spi.StatisticsImplementor; import org.hibernate.testing.TestForIssue; import org.hibernate.testing.orm.junit.NotImplementedYet; @@ -36,6 +38,9 @@ public abstract class CachedMutableNaturalIdTest { session.createQuery( "delete from A" ).executeUpdate(); } ); + + final CacheImplementor cache = scope.getSessionFactory().getCache(); + cache.evictAllRegions(); } @Test @@ -94,8 +99,9 @@ public abstract class CachedMutableNaturalIdTest { } @Test - @NotImplementedYet( reason = "Caching is not yet implemented", strict = false ) public void testNaturalIdReCachingWhenNeeded(SessionFactoryScope scope) { + final StatisticsImplementor statistics = scope.getSessionFactory().getStatistics(); + statistics.clear(); final Integer id = scope.fromTransaction( (session) -> { @@ -119,12 +125,12 @@ public abstract class CachedMutableNaturalIdTest { (session) -> { final Another shouldBeGone = session.bySimpleNaturalId(Another.class).load("it"); assertNull( shouldBeGone ); - assertEquals( 0, session.getSessionFactory().getStatistics().getNaturalIdCacheHitCount() ); + assertEquals( 0, statistics.getNaturalIdCacheHitCount() ); } ); - // finally there should be only 2 NaturalIdCache puts : 1. insertion, 2. when updating natural-id from 'it' to 'name9' - assertEquals( 2, scope.getSessionFactory().getStatistics().getNaturalIdCachePutCount() ); + // finally there should be only 2 NaturalIdCache puts : 1. insertion, 2. when updating natural-id from 'it' to 'it2' + assertEquals( 2, statistics.getNaturalIdCachePutCount() ); } @Test diff --git a/hibernate-core/src/test/resources/log4j.properties b/hibernate-core/src/test/resources/log4j.properties index 5bdf896ae0..fccf469513 100644 --- a/hibernate-core/src/test/resources/log4j.properties +++ b/hibernate-core/src/test/resources/log4j.properties @@ -32,6 +32,8 @@ log4j.logger.org.hibernate.orm.query.sqm=debug log4j.logger.org.hibernate.orm.query.hql=trace log4j.logger.org.hibernate.sql.results.graph.DomainResultGraphPrinter=debug +log4j.logger.org.hibernate.orm.mapping.natural_id=debug + log4j.logger.org.hibernate.tool.hbm2ddl=trace log4j.logger.org.hibernate.testing.cache=debug