From e2f7d5d516ed3fba3899d18946e744d9baf8f5ab Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 11 Jun 2024 18:13:16 +0200 Subject: [PATCH] HHH-16830 Custom exception handling for applyToLoadByKey associations --- .../chapters/pc/PersistenceContext.adoc | 15 +- .../org/hibernate/EntityFilterException.java | 54 ++++ .../src/main/java/org/hibernate/Filter.java | 4 +- .../engine/spi/FilterDefinition.java | 2 +- .../org/hibernate/internal/FilterHelper.java | 4 +- .../org/hibernate/internal/FilterImpl.java | 4 +- .../org/hibernate/internal/SessionImpl.java | 8 + .../internal/ToOneAttributeMapping.java | 41 +-- .../entity/AbstractEntityPersister.java | 4 +- .../results/complete/EntityResultImpl.java | 1 + ...ractBatchEntitySelectFetchInitializer.java | 28 +- ...nsideEmbeddableSelectFetchInitializer.java | 5 +- .../BatchEntitySelectFetchInitializer.java | 5 +- ...nitializeEntitySelectFetchInitializer.java | 5 +- .../internal/EntityFetchJoinedImpl.java | 12 +- .../internal/EntityFetchSelectImpl.java | 10 + .../internal/EntityInitializerImpl.java | 11 + .../entity/internal/EntityResultImpl.java | 1 + ...titySelectFetchByUniqueKeyInitializer.java | 29 +- .../EntitySelectFetchInitializer.java | 30 +- .../EntitySelectFetchInitializerBuilder.java | 7 + .../internal/domain/CircularFetchImpl.java | 1 + .../org/hibernate/orm/test/pc/FilterTest.java | 277 ++++++++++++++++-- 23 files changed, 472 insertions(+), 86 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/EntityFilterException.java diff --git a/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc b/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc index cc6471b394..65e62aec11 100644 --- a/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc +++ b/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc @@ -558,11 +558,12 @@ include::{example-dir-pc}/FilterTest.java[tags=pc-filter-resolver-Account-exampl [IMPORTANT] ==== -Filters apply to entity queries, but not to direct fetching, unless otherwise configured using the `applyToLoadById` flag +Filters apply to entity queries, but not to direct fetching, unless otherwise configured using the `applyToLoadByKey` flag on the `@FilterDef`, that should be set to `true` in order to activate the filter with direct fetching. +==== -Therefore, in the following example, the `activeAccount` filter is not taken into consideration when fetching an entity from the Persistence Context. -On the other hand, the `minimumAmount` filter is taken into consideration, because its `applyToLoadById` flag is set to `true`. +In the following example, the `activeAccount` filter is not taken into consideration when fetching an entity from the Persistence Context. +On the other hand, the `minimumAmount` filter is taken into consideration, because its `applyToLoadByKey` flag is set to `true`. [[pc-filter-entity-example]] .Fetching entities mapped with `@Filter` @@ -581,9 +582,15 @@ include::{extrasdir}/pc-filter-entity-example.sql[] include::{extrasdir}/pc-filter-entity-find-example.sql[] ---- +[WARNING] +==== +Using `@NotFound(action = NotFoundAction.IGNORE)` on associations that are filtered via a `FilterDef` with `applyToLoadByKey` set to `true` +is dangerous, because the association will be set to `null` if the filter excludes the target row. +On flush, this can lead to the foreign key column be set to `null` and hence lead to data loss. +==== + As you can see from the example above, contrary to an entity query, the `activeAccount` filter does not prevent the entity from being loaded, but the `minimumAmount` filter limits the results to the ones with an amount that is greater than the specified one. -==== Just like with entity queries, collections can be filtered as well, but only if the filter is enabled on the currently running Hibernate `Session`, either if the filter is enabled explicitly or by setting `autoEnabled` to `true`. diff --git a/hibernate-core/src/main/java/org/hibernate/EntityFilterException.java b/hibernate-core/src/main/java/org/hibernate/EntityFilterException.java new file mode 100644 index 0000000000..ce518f4511 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/EntityFilterException.java @@ -0,0 +1,54 @@ +/* + * 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; + +import java.util.Locale; + +import org.hibernate.annotations.FilterDef; + +import jakarta.persistence.EntityNotFoundException; + +/** + * Exception thrown if a filter would make a to-one association {@code null}, + * which could lead to data loss. + * Even though filters are applied to load-by-key operations, + * a to-one association should never refer to an entity that is filtered. + * + * @see FilterDef#applyToLoadByKey() + */ +public class EntityFilterException extends EntityNotFoundException { + private final String entityName; + private final Object identifier; + private final String role; + + public EntityFilterException(String entityName, Object identifier, String role) { + super( + String.format( + Locale.ROOT, + "Entity `%s` with identifier value `%s` is filtered for association `%s`", + entityName, + identifier, + role + ) + ); + this.entityName = entityName; + this.identifier = identifier; + this.role = role; + } + + public String getEntityName() { + return entityName; + } + + public Object getIdentifier() { + return identifier; + } + + public String getRole() { + return role; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/Filter.java b/hibernate-core/src/main/java/org/hibernate/Filter.java index 868857a826..e9cb3bdef4 100644 --- a/hibernate-core/src/main/java/org/hibernate/Filter.java +++ b/hibernate-core/src/main/java/org/hibernate/Filter.java @@ -97,10 +97,10 @@ public interface Filter { boolean isAutoEnabled(); /** - * Get the associated {@link FilterDefinition applyToLoadById} of this + * Get the associated {@link FilterDefinition applyToLoadByKey} of this * named filter. * * @return The flag value */ - boolean isApplyToLoadByKey(); + boolean isAppliedToLoadByKey(); } diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/FilterDefinition.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/FilterDefinition.java index bcc75ea8bf..7f2b0f49c9 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/FilterDefinition.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/FilterDefinition.java @@ -116,7 +116,7 @@ public class FilterDefinition implements Serializable { * * @return The flag value. */ - public boolean isApplyToLoadByKey() { + public boolean isAppliedToLoadByKey() { return applyToLoadByKey; } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java index 7a060eaaeb..2ba8e9619f 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java @@ -134,7 +134,7 @@ public class FilterHelper { public boolean isAffectedBy(Map enabledFilters, boolean onlyApplyForLoadByKey) { for ( String filterName : filterNames ) { Filter filter = enabledFilters.get( filterName ); - if ( filter != null && ( !onlyApplyForLoadByKey || filter.isApplyToLoadByKey() ) ) { + if ( filter != null && ( !onlyApplyForLoadByKey || filter.isAppliedToLoadByKey() ) ) { return true; } } @@ -189,7 +189,7 @@ public class FilterHelper { for ( int i = 0, max = filterNames.length; i < max; i++ ) { final String filterName = filterNames[i]; final FilterImpl enabledFilter = (FilterImpl) enabledFilters.get( filterName ); - if ( enabledFilter != null && ( !onlyApplyLoadByKeyFilters || enabledFilter.isApplyToLoadByKey() ) ) { + if ( enabledFilter != null && ( !onlyApplyLoadByKeyFilters || enabledFilter.isAppliedToLoadByKey() ) ) { filterPredicate.applyFragment( render( aliasGenerator, i, tableGroup, creationState ), enabledFilter, parameterNames[i] ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java index f90b16f7c0..acbcc51614 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java @@ -48,7 +48,7 @@ public class FilterImpl implements Filter, Serializable { this.definition = configuration; filterName = definition.getFilterName(); this.autoEnabled = definition.isAutoEnabled(); - this.applyToLoadByKey = definition.isApplyToLoadByKey(); + this.applyToLoadByKey = definition.isAppliedToLoadByKey(); } public FilterDefinition getFilterDefinition() { @@ -80,7 +80,7 @@ public class FilterImpl implements Filter, Serializable { * * @return The flag value. */ - public boolean isApplyToLoadByKey() { + public boolean isAppliedToLoadByKey() { return applyToLoadByKey; } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java index 7906136ab1..e636475e7a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/SessionImpl.java @@ -24,6 +24,7 @@ import java.util.TimeZone; import org.hibernate.CacheMode; import org.hibernate.ConnectionAcquisitionMode; +import org.hibernate.EntityFilterException; import org.hibernate.FetchNotFoundException; import org.hibernate.FlushMode; import org.hibernate.HibernateException; @@ -2465,6 +2466,13 @@ public class SessionImpl if ( enfe instanceof FetchNotFoundException ) { throw enfe; } + /* + This may happen if the entity has an associations which is filtered by a FilterDef + and this associated entity is not found. + */ + if ( enfe instanceof EntityFilterException ) { + throw enfe; + } // DefaultLoadEventListener#returnNarrowedProxy() may throw ENFE (see HHH-7861 for details), // which find() should not throw. Find() should return null if the entity was not found. if ( log.isDebugEnabled() ) { diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java index a379f373cb..859cee1046 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/ToOneAttributeMapping.java @@ -21,6 +21,7 @@ import org.hibernate.annotations.NotFoundAction; import org.hibernate.cache.MutableCacheKeyBuilder; import org.hibernate.engine.FetchStyle; import org.hibernate.engine.FetchTiming; +import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.internal.util.IndexedConsumer; @@ -270,7 +271,7 @@ public class ToOneAttributeMapping ); if ( bootValue instanceof ManyToOne ) { final ManyToOne manyToOne = (ManyToOne) bootValue; - this.notFoundAction = determineNotFoundAction( ( (ManyToOne) bootValue ).getNotFoundAction(), entityMappingType ); + this.notFoundAction = ( (ManyToOne) bootValue ).getNotFoundAction(); if ( manyToOne.isLogicalOneToOne() ) { cardinality = Cardinality.LOGICAL_ONE_TO_ONE; } @@ -424,7 +425,7 @@ public class ToOneAttributeMapping else { this.bidirectionalAttributePath = SelectablePath.parse( oneToOne.getMappedByProperty() ); } - notFoundAction = determineNotFoundAction( null, entityMappingType ); + notFoundAction = null; isKeyTableNullable = isNullable(); isOptional = !bootValue.isConstrained(); isInternalLoadNullable = isNullable(); @@ -569,16 +570,6 @@ public class ToOneAttributeMapping } } - private NotFoundAction determineNotFoundAction(NotFoundAction notFoundAction, EntityMappingType entityMappingType) { - // When a filter exists that affects a singular association, we have to enable NotFound handling - // to force an exception if the filter would result in the entity not being found. - // If we silently just read null, this could lead to data loss on flush - if ( entityMappingType.getEntityPersister().hasFilterForLoadByKey() && notFoundAction == null ) { - return NotFoundAction.EXCEPTION; - } - return notFoundAction; - } - private static SelectablePath findBidirectionalOneToManyAttributeName( String propertyPath, ManagedMappingType declaringType, @@ -657,9 +648,6 @@ public class ToOneAttributeMapping return FetchTiming.IMMEDIATE; } } - if ( entityMappingType.getEntityPersister().hasFilterForLoadByKey() ) { - return FetchTiming.IMMEDIATE; - } return mappedFetchTiming; } @@ -1330,6 +1318,7 @@ public class ToOneAttributeMapping this, tableGroup, keyDomainResult, + false, fetchablePath, creationState ); @@ -1371,6 +1360,7 @@ public class ToOneAttributeMapping fetchablePath, domainResult, isSelectByUniqueKey( sideNature ), + false, creationState ); } @@ -1419,6 +1409,7 @@ public class ToOneAttributeMapping NavigablePath navigablePath, DomainResult keyResult, boolean selectByUniqueKey, + boolean isAffectedByFilter, @SuppressWarnings("unused") DomainResultCreationState creationState) { return new EntityFetchSelectImpl( fetchParent, @@ -1426,6 +1417,7 @@ public class ToOneAttributeMapping navigablePath, keyResult, selectByUniqueKey, + isAffectedByFilter, creationState ); } @@ -1438,6 +1430,7 @@ public class ToOneAttributeMapping ToOneAttributeMapping toOneMapping, TableGroup tableGroup, DomainResult keyResult, + boolean isAffectedByFilter, NavigablePath navigablePath, DomainResultCreationState creationState) { return new EntityFetchJoinedImpl( @@ -1445,6 +1438,7 @@ public class ToOneAttributeMapping toOneMapping, tableGroup, keyResult, + isAffectedByFilter, navigablePath, creationState ); @@ -1576,11 +1570,15 @@ public class ToOneAttributeMapping return withRegisteredAssociationKeys( () -> { + // When a filter exists that affects a singular association, we have to enable NotFound handling + // to force an exception if the filter would result in the entity not being found. + // If we silently just read null, this could lead to data loss on flush + final boolean affectedByEnabledFilters = isAffectedByEnabledFilters( creationState ); DomainResult keyResult = null; if ( sideNature == ForeignKeyDescriptor.Nature.KEY ) { // If the key side is non-nullable we also need to add the keyResult // to be able to manually check invalid foreign key references - if ( hasNotFoundAction() || !isInternalLoadNullable ) { + if ( hasNotFoundAction() || !isInternalLoadNullable || affectedByEnabledFilters ) { keyResult = foreignKeyDescriptor.createKeyDomainResult( fetchablePath, tableGroup, @@ -1590,7 +1588,8 @@ public class ToOneAttributeMapping } } else if ( hasNotFoundAction() - || getAssociatedEntityMappingType().getSoftDeleteMapping() != null ) { + || getAssociatedEntityMappingType().getSoftDeleteMapping() != null + || affectedByEnabledFilters ) { // For the target side only add keyResult when a not-found action is present keyResult = foreignKeyDescriptor.createTargetDomainResult( fetchablePath, @@ -1605,6 +1604,7 @@ public class ToOneAttributeMapping this, tableGroup, keyResult, + affectedByEnabledFilters, fetchablePath, creationState ); @@ -1679,6 +1679,7 @@ public class ToOneAttributeMapping fetchablePath, keyResult, selectByUniqueKey, + isAffectedByEnabledFilters( creationState ), creationState ); } @@ -1697,6 +1698,12 @@ public class ToOneAttributeMapping ); } + private boolean isAffectedByEnabledFilters(DomainResultCreationState creationState) { + final LoadQueryInfluencers loadQueryInfluencers = creationState.getSqlAstCreationState() + .getLoadQueryInfluencers(); + return entityMappingType.isAffectedByEnabledFilters( loadQueryInfluencers, true ); + } + private boolean needsImmediateFetch(FetchTiming fetchTiming) { if ( fetchTiming == FetchTiming.IMMEDIATE ) { return true; 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 9a36c896d4..5f17390e73 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 @@ -72,7 +72,6 @@ import org.hibernate.classic.Lifecycle; import org.hibernate.collection.spi.PersistentCollection; import org.hibernate.dialect.Dialect; import org.hibernate.dialect.lock.LockingStrategy; -import org.hibernate.engine.FetchStyle; import org.hibernate.engine.FetchTiming; import org.hibernate.engine.OptimisticLockStyle; import org.hibernate.engine.internal.CacheHelper; @@ -280,7 +279,6 @@ import org.hibernate.sql.model.ast.builder.TableInsertBuilder; import org.hibernate.sql.results.graph.DomainResult; import org.hibernate.sql.results.graph.DomainResultCreationState; import org.hibernate.sql.results.graph.Fetch; -import org.hibernate.sql.results.graph.FetchOptions; import org.hibernate.sql.results.graph.FetchParent; import org.hibernate.sql.results.graph.Fetchable; import org.hibernate.sql.results.graph.FetchableContainer; @@ -1260,7 +1258,7 @@ public abstract class AbstractEntityPersister public boolean hasFilterForLoadByKey() { if ( filterHelper != null ) { for ( String filterName : filterHelper.getFilterNames() ) { - if ( factory.getFilterDefinition( filterName ).isApplyToLoadByKey() ) { + if ( factory.getFilterDefinition( filterName ).isAppliedToLoadByKey() ) { return true; } } diff --git a/hibernate-core/src/main/java/org/hibernate/query/results/complete/EntityResultImpl.java b/hibernate-core/src/main/java/org/hibernate/query/results/complete/EntityResultImpl.java index b00cf59f5f..bec6b32711 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/results/complete/EntityResultImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/results/complete/EntityResultImpl.java @@ -165,6 +165,7 @@ public class EntityResultImpl implements EntityResult, InitializerProducer keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { - super( parent, toOneMapping, fetchedNavigable, concreteDescriptor, keyResult, creationState ); + super( parent, toOneMapping, fetchedNavigable, concreteDescriptor, keyResult, affectedByFilter, creationState ); //noinspection unchecked this.owningEntityInitializer = (EntityInitializer) Initializer.findOwningEntityInitializer( parent ); assert owningEntityInitializer != null : "This initializer requires an owning parent entity initializer"; @@ -231,14 +235,30 @@ public abstract class AbstractBatchEntitySelectFetchInitializer keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { - super( parentAccess, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, creationState ); + super( parentAccess, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, affectedByFilter, creationState ); this.referencedModelPartSetter = referencedModelPart.getAttributeMetadata().getPropertyAccess().getSetter(); final String rootEmbeddablePropertyName = getRootEmbeddablePropertyName( @@ -172,7 +173,7 @@ public class BatchEntityInsideEmbeddableSelectFetchInitializer extends AbstractB data.toBatchLoad.forEach( (entityKey, parentInfos) -> { final SharedSessionContractImplementor session = data.getRowProcessingState().getSession(); - final Object loadedInstance = loadInstance( entityKey, toOneMapping, session ); + final Object loadedInstance = loadInstance( entityKey, toOneMapping, affectedByFilter, session ); for ( ParentInfo parentInfo : parentInfos ) { final PersistenceContext persistenceContext = session.getPersistenceContext(); final EntityEntry parentEntityEntry = persistenceContext.getEntry( parentInfo.parentEntityInstance ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchEntitySelectFetchInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchEntitySelectFetchInitializer.java index 9f86dd537c..ec6885d8c7 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchEntitySelectFetchInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchEntitySelectFetchInitializer.java @@ -46,8 +46,9 @@ public class BatchEntitySelectFetchInitializer extends AbstractBatchEntitySelect NavigablePath fetchedNavigable, EntityPersister concreteDescriptor, DomainResult keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { - super( parentAccess, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, creationState ); + super( parentAccess, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, affectedByFilter, creationState ); this.parentAttributes = getParentEntityAttributes( referencedModelPart.getAttributeName() ); this.referencedModelPartSetter = referencedModelPart.getPropertyAccess().getSetter(); this.referencedModelPartType = referencedModelPart.findContainingEntityMapping().getEntityPersister() @@ -95,7 +96,7 @@ public class BatchEntitySelectFetchInitializer extends AbstractBatchEntitySelect data.toBatchLoad.forEach( (entityKey, parentInfos) -> { final SharedSessionContractImplementor session = data.getRowProcessingState().getSession(); - final Object instance = loadInstance( entityKey, toOneMapping, session ); + final Object instance = loadInstance( entityKey, toOneMapping, affectedByFilter, session ); for ( ParentInfo parentInfo : parentInfos ) { final Object parentInstance = parentInfo.parentInstance; final EntityEntry entry = session.getPersistenceContext().getEntry( parentInstance ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchInitializeEntitySelectFetchInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchInitializeEntitySelectFetchInitializer.java index 330f0d8b29..abe125cff0 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchInitializeEntitySelectFetchInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/BatchInitializeEntitySelectFetchInitializer.java @@ -41,8 +41,9 @@ public class BatchInitializeEntitySelectFetchInitializer extends AbstractBatchEn NavigablePath fetchedNavigable, EntityPersister concreteDescriptor, DomainResult keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { - super( parent, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, creationState ); + super( parent, referencedModelPart, fetchedNavigable, concreteDescriptor, keyResult, affectedByFilter, creationState ); } @Override @@ -73,7 +74,7 @@ public class BatchInitializeEntitySelectFetchInitializer extends AbstractBatchEn super.endLoading( data ); final SharedSessionContractImplementor session = data.getRowProcessingState().getSession(); for ( EntityKey key : data.toBatchLoad ) { - loadInstance( key, toOneMapping, session ); + loadInstance( key, toOneMapping, affectedByFilter, session ); } data.toBatchLoad.clear(); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java index 25907625b6..c79712a6f0 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchJoinedImpl.java @@ -39,6 +39,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali private final EntityResultImpl entityResult; private final DomainResult keyResult; private final NotFoundAction notFoundAction; + private final boolean isAffectedByFilter; private final String sourceAlias; @@ -47,6 +48,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali ToOneAttributeMapping toOneMapping, TableGroup tableGroup, DomainResult keyResult, + boolean isAffectedByFilter, NavigablePath navigablePath, DomainResultCreationState creationState) { this.fetchContainer = toOneMapping; @@ -54,7 +56,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali this.keyResult = keyResult; this.notFoundAction = toOneMapping.getNotFoundAction(); this.sourceAlias = tableGroup.getSourceAlias(); - + this.isAffectedByFilter = isAffectedByFilter; this.entityResult = new EntityResultImpl( navigablePath, toOneMapping, @@ -76,7 +78,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali this.notFoundAction = collectionPart.getNotFoundAction(); this.keyResult = null; this.sourceAlias = tableGroup.getSourceAlias(); - + this.isAffectedByFilter = false; this.entityResult = new EntityResultImpl( navigablePath, collectionPart, @@ -96,6 +98,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali this.entityResult = original.entityResult; this.keyResult = original.keyResult; this.notFoundAction = original.notFoundAction; + this.isAffectedByFilter = original.isAffectedByFilter; this.sourceAlias = original.sourceAlias; } @@ -153,6 +156,7 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali keyResult, entityResult.getRowIdResult(), notFoundAction, + isAffectedByFilter, parent, false, creationState @@ -214,6 +218,10 @@ public class EntityFetchJoinedImpl implements EntityFetch, FetchParent, Initiali return notFoundAction; } + protected boolean isAffectedByFilter() { + return isAffectedByFilter; + } + protected String getSourceAlias() { return sourceAlias; } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java index 34ea79b932..a23dace718 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityFetchSelectImpl.java @@ -23,14 +23,18 @@ import org.hibernate.sql.results.graph.entity.EntityInitializer; */ public class EntityFetchSelectImpl extends AbstractNonJoinedEntityFetch { + private final boolean isAffectedByFilter; + public EntityFetchSelectImpl( FetchParent fetchParent, ToOneAttributeMapping fetchedAttribute, NavigablePath navigablePath, DomainResult keyResult, boolean selectByUniqueKey, + boolean isAffectedByFilter, DomainResultCreationState creationState) { super( navigablePath, fetchedAttribute, fetchParent, keyResult, false, selectByUniqueKey, creationState ); + this.isAffectedByFilter = isAffectedByFilter; } /** @@ -45,6 +49,7 @@ public class EntityFetchSelectImpl extends AbstractNonJoinedEntityFetch { original.getDiscriminatorFetch(), original.isSelectByUniqueKey() ); + this.isAffectedByFilter = original.isAffectedByFilter(); } @Override @@ -52,6 +57,10 @@ public class EntityFetchSelectImpl extends AbstractNonJoinedEntityFetch { return FetchTiming.IMMEDIATE; } + public boolean isAffectedByFilter() { + return isAffectedByFilter; + } + @Override public EntityInitializer createInitializer(InitializerParent parent, AssemblerCreationState creationState) { return EntitySelectFetchInitializerBuilder.createInitializer( @@ -61,6 +70,7 @@ public class EntityFetchSelectImpl extends AbstractNonJoinedEntityFetch { getKeyResult(), getNavigablePath(), isSelectByUniqueKey(), + isAffectedByFilter(), creationState ); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java index d1348994dc..701d982dad 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntityInitializerImpl.java @@ -9,6 +9,7 @@ package org.hibernate.sql.results.graph.entity.internal; import java.util.Collection; import java.util.function.BiConsumer; +import org.hibernate.EntityFilterException; import org.hibernate.FetchNotFoundException; import org.hibernate.Hibernate; import org.hibernate.HibernateException; @@ -100,6 +101,7 @@ public class EntityInitializerImpl extends AbstractInitializer parent; private final NotFoundAction notFoundAction; + private final boolean affectedByFilter; private final boolean isPartOfKey; private final boolean isResultInitializer; private final boolean hasKeyManyToOne; @@ -141,6 +143,7 @@ public class EntityInitializerImpl extends AbstractInitializer keyResult, @Nullable DomainResult rowIdResult, NotFoundAction notFoundAction, + boolean affectedByFilter, @Nullable InitializerParent parent, boolean isResultInitializer, AssemblerCreationState creationState) { @@ -249,6 +252,7 @@ public class EntityInitializerImpl extends AbstractInitializer keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { - super( parent, fetchedAttribute, fetchedNavigable, concreteDescriptor, keyResult, creationState ); + super( parent, fetchedAttribute, fetchedNavigable, concreteDescriptor, keyResult, affectedByFilter, creationState ); this.fetchedAttribute = fetchedAttribute; } @@ -51,17 +55,30 @@ public class EntitySelectFetchByUniqueKeyInitializer extends EntitySelectFetchIn final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); data.setInstance( persistenceContext.getEntity( euk ) ); if ( data.getInstance() == null ) { - data.setInstance( concreteDescriptor.loadByUniqueKey( + final Object instance = concreteDescriptor.loadByUniqueKey( uniqueKeyPropertyName, data.entityIdentifier, session - ) ); + ); + data.setInstance( instance ); + if ( instance == null ) { + if ( toOneMapping.getNotFoundAction() != NotFoundAction.IGNORE ) { + if ( affectedByFilter ) { + throw new EntityFilterException( + entityName, + data.entityIdentifier, + toOneMapping.getNavigableRole().getFullPath() + ); + } + if ( toOneMapping.getNotFoundAction() == NotFoundAction.EXCEPTION ) { + throw new FetchNotFoundException( entityName, data.entityIdentifier ); + } + } + } // If the entity was not in the Persistence Context, but was found now, // add it to the Persistence Context - if ( data.getInstance() != null ) { - persistenceContext.addEntity( euk, data.getInstance() ); - } + persistenceContext.addEntity( euk, instance ); } if ( data.getInstance() != null ) { data.setInstance( persistenceContext.proxyFor( data.getInstance() ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntitySelectFetchInitializer.java b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntitySelectFetchInitializer.java index 79981cc4b0..b62fc16a90 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntitySelectFetchInitializer.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/graph/entity/internal/EntitySelectFetchInitializer.java @@ -8,6 +8,7 @@ package org.hibernate.sql.results.graph.entity.internal; import java.util.function.BiConsumer; +import org.hibernate.EntityFilterException; import org.hibernate.FetchNotFoundException; import org.hibernate.Hibernate; import org.hibernate.annotations.NotFoundAction; @@ -53,6 +54,7 @@ public class EntitySelectFetchInitializer keyAssembler; protected final ToOneAttributeMapping toOneMapping; + protected final boolean affectedByFilter; public static class EntitySelectFetchInitializerData extends InitializerData { // per-row state @@ -70,6 +72,7 @@ public class EntitySelectFetchInitializer keyResult, + boolean affectedByFilter, AssemblerCreationState creationState) { super( creationState ); this.parent = parent; @@ -79,6 +82,7 @@ public class EntitySelectFetchInitializer keyResult, NavigablePath navigablePath, boolean selectByUniqueKey, + boolean affectedByFilter, AssemblerCreationState creationState) { if ( selectByUniqueKey ) { return new EntitySelectFetchByUniqueKeyInitializer( @@ -40,6 +41,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); } @@ -51,6 +53,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); } @@ -63,6 +66,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); case BATCH_LOAD: @@ -73,6 +77,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); } @@ -83,6 +88,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); } @@ -93,6 +99,7 @@ public class EntitySelectFetchInitializerBuilder { navigablePath, entityPersister, keyResult, + affectedByFilter, creationState ); } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularFetchImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularFetchImpl.java index aa81deb9ae..a8b5a413e9 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularFetchImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/domain/CircularFetchImpl.java @@ -141,6 +141,7 @@ public class CircularFetchImpl extends AbstractNonJoinedEntityFetch implements B keyResult, navigablePath, selectByUniqueKey, + false, creationState ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/pc/FilterTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/pc/FilterTest.java index ecd29411e4..e14f878ab8 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/pc/FilterTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/pc/FilterTest.java @@ -15,6 +15,7 @@ import jakarta.persistence.CascadeType; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.EntityGraph; +import jakarta.persistence.EntityNotFoundException; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; import jakarta.persistence.FetchType; @@ -24,10 +25,13 @@ import jakarta.persistence.NoResultException; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; -import org.hibernate.FetchNotFoundException; +import org.hibernate.EntityFilterException; +import org.hibernate.Hibernate; import org.hibernate.Session; import org.hibernate.annotations.Filter; import org.hibernate.annotations.FilterDef; +import org.hibernate.annotations.NotFound; +import org.hibernate.annotations.NotFoundAction; import org.hibernate.annotations.ParamDef; import org.hibernate.jpa.AvailableHints; import org.hibernate.metamodel.CollectionClassification; @@ -56,7 +60,10 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { protected Class[] getAnnotatedClasses() { return new Class[] { Client.class, - Account.class + Account.class, + AccountEager.class, + AccountNotFound.class, + AccountNotFoundException.class }; } @@ -106,6 +113,21 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { entityManager.persist(client); //end::pc-filter-persistence-example[] + entityManager.persist( + new AccountEager() + .setId(2L) + .setParentAccount( account1 ) + ); + entityManager.persist( + new AccountNotFound() + .setId(2L) + .setParentAccount( account1 ) + ); + entityManager.persist( + new AccountNotFoundException() + .setId(2L) + .setParentAccount( account1 ) + ); }); } @@ -113,6 +135,9 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { public void tearDown() { doInJPA(this::entityManagerFactory, entityManager -> { entityManager.createQuery( "update Account set parentAccount = null" ).executeUpdate(); + entityManager.createQuery( "delete from AccountEager" ).executeUpdate(); + entityManager.createQuery( "delete from AccountNotFound" ).executeUpdate(); + entityManager.createQuery( "delete from AccountNotFoundException" ).executeUpdate(); entityManager.createQuery( "delete from Account" ).executeUpdate(); entityManager.createQuery( "delete from Client" ).executeUpdate(); }); @@ -273,31 +298,42 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { @Test @JiraKey("HHH-16830") public void testApplyToLoadByKeyAssociationFiltering() { - doInJPA(this::entityManagerFactory, entityManager -> { - Account account = entityManager.find(Account.class, 2L); + doInJPA( this::entityManagerFactory, entityManager -> { + Account account = entityManager.find( Account.class, 2L ); assertNotNull( account.getParentAccount() ); - }); - doInJPA(this::entityManagerFactory, entityManager -> { - entityManager.unwrap(Session.class) - .enableFilter("accountType") - .setParameter("type", "DEBIT"); + } ); + } - FetchNotFoundException exception = assertThrows( - FetchNotFoundException.class, - () -> entityManager.find( Account.class, 2L ) + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringLazyInitialization() { + doInJPA( this::entityManagerFactory, entityManager -> { + entityManager.unwrap( Session.class ) + .enableFilter( "accountType" ) + .setParameter( "type", "DEBIT" ); + + Account account = entityManager.find( Account.class, 2L ); + EntityNotFoundException exception = assertThrows( + EntityNotFoundException.class, + () -> Hibernate.initialize( account.getParentAccount() ) ); // Account with id 1 does not exist - assertTrue( exception.getMessage().contains( "`1`" ) ); - }); - doInJPA(this::entityManagerFactory, entityManager -> { - entityManager.unwrap(Session.class) - .enableFilter("accountType") - .setParameter("type", "DEBIT"); + assertTrue( exception.getMessage().endsWith( " 1" ) ); + } ); + } + + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringAccountLoadGraphInitializer() { + doInJPA( this::entityManagerFactory, entityManager -> { + entityManager.unwrap( Session.class ) + .enableFilter( "accountType" ) + .setParameter( "type", "DEBIT" ); EntityGraph entityGraph = entityManager.createEntityGraph( Account.class ); entityGraph.addAttributeNodes( "parentAccount" ); - FetchNotFoundException exception = assertThrows( - FetchNotFoundException.class, + EntityFilterException exception = assertThrows( + EntityFilterException.class, () -> entityManager.find( Account.class, 2L, @@ -305,22 +341,116 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { ) ); // Account with id 1 does not exist - assertTrue( exception.getMessage().contains( "`1`" ) ); - }); - doInJPA(this::entityManagerFactory, entityManager -> { - entityManager.unwrap(Session.class) - .enableFilter("accountType") - .setParameter("type", "DEBIT"); + assertTrue( exception.getRole().endsWith( "parentAccount" ) ); + assertEquals( 1L, exception.getIdentifier() ); + } ); + } - FetchNotFoundException exception = assertThrows( - FetchNotFoundException.class, + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringAccountJoinInitializer() { + doInJPA( this::entityManagerFactory, entityManager -> { + entityManager.unwrap( Session.class ) + .enableFilter( "accountType" ) + .setParameter( "type", "DEBIT" ); + + EntityFilterException exception = assertThrows( + EntityFilterException.class, () -> entityManager.createQuery( "select a from Account a left join fetch a.parentAccount where a.id = 2", Account.class ).getResultList() ); // Account with id 1 does not exist - assertTrue( exception.getMessage().contains( "`1`" ) ); + assertTrue( exception.getRole().contains( "parentAccount" ) ); + assertEquals( 1L, exception.getIdentifier() ); + } ); + } + + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringAccountSelectInitializer() { + doInJPA( this::entityManagerFactory, entityManager -> { + entityManager.unwrap( Session.class ) + .enableFilter( "accountType" ) + .setParameter( "type", "DEBIT" ); + + EntityFilterException exception = assertThrows( + EntityFilterException.class, + () -> entityManager.createQuery( + "select a from AccountEager a where a.id = 2", + AccountEager.class + ).getResultList() + ); + // Account with id 1 does not exist + assertTrue( exception.getRole().contains( "parentAccount" ) ); + assertEquals( 1L, exception.getIdentifier() ); + } ); + } + + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringAccountNotFoundException() { + doInJPA(this::entityManagerFactory, entityManager -> { + entityManager.unwrap(Session.class) + .enableFilter("accountType") + .setParameter("type", "DEBIT"); + + EntityFilterException exception = assertThrows( + EntityFilterException.class, + () -> entityManager.createQuery( + "select a from AccountNotFoundException a where a.id = 2", + AccountNotFoundException.class + ).getSingleResult() + ); + // Account with id 1 does not exist + assertTrue( exception.getRole().contains( "parentAccount" ) ); + assertEquals( 1L, exception.getIdentifier() ); + }); + doInJPA(this::entityManagerFactory, entityManager -> { + entityManager.unwrap(Session.class) + .enableFilter("accountType") + .setParameter("type", "DEBIT"); + + EntityFilterException exception = assertThrows( + EntityFilterException.class, + () -> entityManager.createQuery( + "select a from AccountNotFoundException a left join fetch a.parentAccount where a.id = 2", + AccountNotFoundException.class + ).getSingleResult() + ); + // Account with id 1 does not exist + assertTrue( exception.getRole().contains( "parentAccount" ) ); + assertEquals( 1L, exception.getIdentifier() ); + }); + } + + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFilteringAccountNotFoundIgnore() { + doInJPA(this::entityManagerFactory, entityManager -> { + entityManager.unwrap(Session.class) + .enableFilter("accountType") + .setParameter("type", "DEBIT"); + + AccountNotFound account = entityManager.createQuery( + "select a from AccountNotFound a where a.id = 2", + AccountNotFound.class + ).getSingleResult(); + // No exception, since we use NotFoundAction.IGNORE + assertNull( account.getParentAccount() ); + }); + doInJPA(this::entityManagerFactory, entityManager -> { + entityManager.unwrap(Session.class) + .enableFilter("accountType") + .setParameter("type", "DEBIT"); + + AccountNotFound account = entityManager.createQuery( + "select a from AccountNotFound a left join fetch a.parentAccount where a.id = 2", + AccountNotFound.class + ).getSingleResult(); + // No exception, since we use NotFoundAction.IGNORE + assertNull( account.getParentAccount() ); }); } @@ -586,4 +716,93 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { } } //end::pc-filter-resolver-Account-example[] + + @Entity(name = "AccountEager") + @Table(name = "account_eager") + public static class AccountEager { + + @Id + private Long id; + + @ManyToOne + private Account parentAccount; + + public Long getId() { + return id; + } + + public AccountEager setId(Long id) { + this.id = id; + return this; + } + + public Account getParentAccount() { + return parentAccount; + } + + public AccountEager setParentAccount(Account parentAccount) { + this.parentAccount = parentAccount; + return this; + } + } + + @Entity(name = "AccountNotFound") + @Table(name = "account_not_found") + public static class AccountNotFound { + + @Id + private Long id; + + @ManyToOne + @NotFound(action = NotFoundAction.IGNORE) + private Account parentAccount; + + public Long getId() { + return id; + } + + public AccountNotFound setId(Long id) { + this.id = id; + return this; + } + + public Account getParentAccount() { + return parentAccount; + } + + public AccountNotFound setParentAccount(Account parentAccount) { + this.parentAccount = parentAccount; + return this; + } + } + + @Entity(name = "AccountNotFoundException") + @Table(name = "account_not_found_exception") + public static class AccountNotFoundException { + + @Id + private Long id; + + @ManyToOne + @NotFound + private Account parentAccount; + + public Long getId() { + return id; + } + + public AccountNotFoundException setId(Long id) { + this.id = id; + return this; + } + + public Account getParentAccount() { + return parentAccount; + } + + public AccountNotFoundException setParentAccount(Account parentAccount) { + this.parentAccount = parentAccount; + return this; + } + } }