From be8705f3178d66d00919243f8ded884c1ae27593 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 27 May 2024 19:31:23 +0200 Subject: [PATCH] HHH-16830 Ensure filters applied for by key lookups don't mess with to-one associations --- .../src/main/java/org/hibernate/Filter.java | 2 +- .../org/hibernate/annotations/FilterDef.java | 7 +- .../binder/internal/TenantIdBinder.java | 2 +- .../boot/model/internal/FilterDefBinder.java | 2 +- .../engine/spi/FilterDefinition.java | 21 +- .../engine/spi/LoadQueryInfluencers.java | 60 ++++-- .../org/hibernate/internal/FilterHelper.java | 14 ++ .../org/hibernate/internal/FilterImpl.java | 8 +- .../internal/AbstractEntityBatchLoader.java | 8 +- .../internal/EntityBatchLoaderArrayParam.java | 8 +- .../EntityBatchLoaderInPredicate.java | 8 +- .../ast/internal/LoaderSelectBuilder.java | 28 ++- .../internal/MultiIdEntityLoaderStandard.java | 3 +- .../SingleIdEntityLoaderStandardImpl.java | 11 +- .../SingleUniqueKeyEntityLoaderStandard.java | 6 +- .../internal/StandardBatchLoaderFactory.java | 11 +- .../loader/ast/spi/BatchLoaderFactory.java | 17 +- .../hibernate/loader/ast/spi/Loadable.java | 14 ++ .../metamodel/mapping/EntityMappingType.java | 5 + .../internal/PluralAttributeMappingImpl.java | 5 + .../internal/ToOneAttributeMapping.java | 24 ++- .../AbstractCollectionPersister.java | 12 ++ .../collection/CollectionPersister.java | 4 + .../entity/AbstractEntityPersister.java | 85 +++++++-- .../persister/entity/EntityPersister.java | 2 + .../sql/ast/tree/from/TableGroupJoin.java | 4 +- .../GoofyPersisterClassProvider.java | 5 + .../entitygraph/ast/LoadPlanBuilderTest.java | 4 +- .../PersisterClassProviderTest.java | 5 + .../orm/test/legacy/CustomPersister.java | 5 + .../org/hibernate/orm/test/pc/FilterTest.java | 179 +++++++++++++----- 31 files changed, 426 insertions(+), 143 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/Filter.java b/hibernate-core/src/main/java/org/hibernate/Filter.java index 990b0b44ce..868857a826 100644 --- a/hibernate-core/src/main/java/org/hibernate/Filter.java +++ b/hibernate-core/src/main/java/org/hibernate/Filter.java @@ -102,5 +102,5 @@ public interface Filter { * * @return The flag value */ - boolean isApplyToLoadById(); + boolean isApplyToLoadByKey(); } diff --git a/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java b/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java index 4536a4e3bf..b2a1194801 100644 --- a/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java +++ b/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java @@ -10,6 +10,8 @@ import java.lang.annotation.Repeatable; import java.lang.annotation.Retention; import java.lang.annotation.Target; +import org.hibernate.Incubating; + import static java.lang.annotation.ElementType.PACKAGE; import static java.lang.annotation.ElementType.TYPE; import static java.lang.annotation.RetentionPolicy.RUNTIME; @@ -101,7 +103,8 @@ public @interface FilterDef { * be applied on direct fetches or not. *

* If the flag is true, the filter will be - * applied on direct fetches, such as findById(). + * applied on direct fetches, such as {@link org.hibernate.Session#find(Class, Object)}. */ - boolean applyToLoadById() default false; + @Incubating + boolean applyToLoadByKey() default false; } diff --git a/hibernate-core/src/main/java/org/hibernate/binder/internal/TenantIdBinder.java b/hibernate-core/src/main/java/org/hibernate/binder/internal/TenantIdBinder.java index 5d9d814a1c..c0de6cb02b 100644 --- a/hibernate-core/src/main/java/org/hibernate/binder/internal/TenantIdBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/binder/internal/TenantIdBinder.java @@ -59,7 +59,7 @@ public class TenantIdBinder implements AttributeBinder { "", singletonMap( PARAMETER_NAME, tenantIdType ), Collections.emptyMap(), - true, + false, true ) ); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/FilterDefBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/FilterDefBinder.java index 29902afcad..1e968b41e6 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/FilterDefBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/FilterDefBinder.java @@ -93,7 +93,7 @@ class FilterDefBinder { explicitParamJaMappings, parameterResolvers, filterDef.autoEnabled(), - filterDef.applyToLoadById() + filterDef.applyToLoadByKey() ); LOG.debugf( "Binding filter definition: %s", filterDefinition.getFilterName() ); context.getMetadataCollector().addFilterDefinition( filterDefinition ); 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 527a1426c7..bcc75ea8bf 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 @@ -37,30 +37,37 @@ public class FilterDefinition implements Serializable { private final Map explicitParamJaMappings = new HashMap<>(); private final Map>> parameterResolverMap = new HashMap<>(); private final boolean autoEnabled; - private final boolean applyToLoadById; + private final boolean applyToLoadByKey; /** * Construct a new FilterDefinition instance. * * @param name The name of the filter for which this configuration is in effect. */ - public FilterDefinition(String name, String defaultCondition, @Nullable Map explicitParamJaMappings) { + public FilterDefinition( + String name, + String defaultCondition, + @Nullable Map explicitParamJaMappings) { this( name, defaultCondition, explicitParamJaMappings, Collections.emptyMap(), false, false); } public FilterDefinition( - String name, String defaultCondition, @Nullable Map explicitParamJaMappings, - Map>> parameterResolverMap, boolean autoEnabled, boolean applyToLoadById) { + String name, + String defaultCondition, + @Nullable Map explicitParamJaMappings, + Map>> parameterResolverMap, + boolean autoEnabled, + boolean applyToLoadByKey) { this.filterName = name; this.defaultFilterCondition = defaultCondition; if ( explicitParamJaMappings != null ) { this.explicitParamJaMappings.putAll( explicitParamJaMappings ); } - this.applyToLoadById = applyToLoadById; if ( parameterResolverMap != null ) { this.parameterResolverMap.putAll( parameterResolverMap ); } this.autoEnabled = autoEnabled; + this.applyToLoadByKey = applyToLoadByKey; } /** @@ -109,8 +116,8 @@ public class FilterDefinition implements Serializable { * * @return The flag value. */ - public boolean isApplyToLoadById() { - return applyToLoadById; + public boolean isApplyToLoadByKey() { + return applyToLoadByKey; } /** diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/LoadQueryInfluencers.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/LoadQueryInfluencers.java index 8b6396c5c3..0a88f4c2dd 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/LoadQueryInfluencers.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/LoadQueryInfluencers.java @@ -13,7 +13,6 @@ import java.util.HashSet; import java.util.Map; import java.util.Set; import java.util.function.Supplier; -import java.util.stream.Collectors; import org.hibernate.Filter; import org.hibernate.Internal; @@ -50,10 +49,10 @@ public class LoadQueryInfluencers implements Serializable { private CascadingFetchProfile enabledCascadingFetchProfile; //Lazily initialized! - private HashSet enabledFetchProfileNames; + private @Nullable HashSet enabledFetchProfileNames; //Lazily initialized! - private HashMap enabledFilters; + private @Nullable HashMap enabledFilters; private boolean subselectFetchEnabled; @@ -76,12 +75,37 @@ public class LoadQueryInfluencers implements Serializable { for (FilterDefinition filterDefinition : sessionFactory.getAutoEnabledFilters()) { FilterImpl filter = new FilterImpl( filterDefinition ); if ( enabledFilters == null ) { - this.enabledFilters = new HashMap<>(); + enabledFilters = new HashMap<>(); } enabledFilters.put( filterDefinition.getFilterName(), filter ); } } + /** + * Special constructor for {@link #copyForLoadByKey()}. + */ + private LoadQueryInfluencers(LoadQueryInfluencers original) { + this.sessionFactory = original.sessionFactory; + this.enabledCascadingFetchProfile = original.enabledCascadingFetchProfile; + this.enabledFetchProfileNames = original.enabledFetchProfileNames == null ? null : new HashSet<>( original.enabledFetchProfileNames ); + this.subselectFetchEnabled = original.subselectFetchEnabled; + this.batchSize = original.batchSize; + this.readOnly = original.readOnly; + HashMap enabledFilters; + if ( original.enabledFilters == null ) { + enabledFilters = null; + } + else { + enabledFilters = new HashMap<>( original.enabledFilters.size() ); + for ( Map.Entry entry : original.enabledFilters.entrySet() ) { + if ( entry.getValue().isApplyToLoadByKey() ) { + enabledFilters.put( entry.getKey(), entry.getValue() ); + } + } + } + this.enabledFilters = enabledFilters; + } + public EffectiveEntityGraph applyEntityGraph(@Nullable RootGraphImplementor rootGraph, @Nullable GraphSemantic graphSemantic) { final EffectiveEntityGraph effectiveEntityGraph = getEffectiveEntityGraph(); if ( graphSemantic != null ) { @@ -156,6 +180,7 @@ public class LoadQueryInfluencers implements Serializable { } public Map getEnabledFilters() { + final HashMap enabledFilters = this.enabledFilters; if ( enabledFilters == null ) { return Collections.emptyMap(); } @@ -169,20 +194,6 @@ public class LoadQueryInfluencers implements Serializable { } } - /** - * Returns a Map of enabled filters that have the applyToLoadById - * flag set to true - * @return a Map of enabled filters that have the applyToLoadById - * flag set to true - */ - public Map getEnabledFiltersForFind() { - return getEnabledFilters() - .entrySet() - .stream() - .filter(f -> f.getValue().isApplyToLoadById()) - .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - } - /** * Returns an unmodifiable Set of enabled filter names. * @return an unmodifiable Set of enabled filter names. @@ -283,8 +294,14 @@ public class LoadQueryInfluencers implements Serializable { @Internal public @Nullable HashSet adjustFetchProfiles(@Nullable Set disabledFetchProfiles, @Nullable Set enabledFetchProfiles) { - final HashSet oldFetchProfiles = - hasEnabledFetchProfiles() ? new HashSet<>( enabledFetchProfileNames ) : null; + final HashSet currentEnabledFetchProfileNames = this.enabledFetchProfileNames; + final HashSet oldFetchProfiles; + if ( currentEnabledFetchProfileNames == null || currentEnabledFetchProfileNames.isEmpty() ) { + oldFetchProfiles = null; + } + else { + oldFetchProfiles = new HashSet<>( currentEnabledFetchProfileNames ); + } if ( disabledFetchProfiles != null && enabledFetchProfileNames != null ) { enabledFetchProfileNames.removeAll( disabledFetchProfiles ); } @@ -391,4 +408,7 @@ public class LoadQueryInfluencers implements Serializable { return false; } + public LoadQueryInfluencers copyForLoadByKey() { + return new LoadQueryInfluencers( this ); + } } 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 71331dd138..ffef648536 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FilterHelper.java @@ -123,6 +123,10 @@ public class FilterHelper { return aliasTableMap.size() == 1 && aliasTableMap.containsKey( null ); } + public String[] getFilterNames() { + return filterNames; + } + public boolean isAffectedBy(Map enabledFilters) { for ( String filterName : filterNames ) { if ( enabledFilters.containsKey( filterName ) ) { @@ -132,6 +136,16 @@ public class FilterHelper { return false; } + public boolean isAffectedByApplyToLoadByKey(Map enabledFilters) { + for ( String filterName : filterNames ) { + Filter filter = enabledFilters.get( filterName ); + if ( filter != null && filter.isApplyToLoadByKey() ) { + return true; + } + } + return false; + } + public static void applyBaseRestrictions( Consumer predicateConsumer, Restrictable restrictable, 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 0cd8d9aec9..f90b16f7c0 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/FilterImpl.java @@ -32,7 +32,7 @@ public class FilterImpl implements Filter, Serializable { private final String filterName; private final Map parameters = new HashMap<>(); private final boolean autoEnabled; - private final boolean applyToLoadById; + private final boolean applyToLoadByKey; void afterDeserialize(SessionFactoryImplementor factory) { definition = factory.getFilterDefinition( filterName ); @@ -48,7 +48,7 @@ public class FilterImpl implements Filter, Serializable { this.definition = configuration; filterName = definition.getFilterName(); this.autoEnabled = definition.isAutoEnabled(); - this.applyToLoadById = definition.isApplyToLoadById(); + this.applyToLoadByKey = definition.isApplyToLoadByKey(); } public FilterDefinition getFilterDefinition() { @@ -80,8 +80,8 @@ public class FilterImpl implements Filter, Serializable { * * @return The flag value. */ - public boolean isApplyToLoadById() { - return applyToLoadById; + public boolean isApplyToLoadByKey() { + return applyToLoadByKey; } public Map getParameters() { diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractEntityBatchLoader.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractEntityBatchLoader.java index d699e19e11..0ec93a874e 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractEntityBatchLoader.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/AbstractEntityBatchLoader.java @@ -10,7 +10,7 @@ import org.hibernate.Hibernate; import org.hibernate.LockMode; import org.hibernate.LockOptions; import org.hibernate.engine.spi.EntityKey; -import org.hibernate.engine.spi.SessionFactoryImplementor; +import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.loader.ast.spi.EntityBatchLoader; import org.hibernate.metamodel.mapping.EntityMappingType; @@ -24,9 +24,9 @@ public abstract class AbstractEntityBatchLoader private final SingleIdEntityLoaderStandardImpl singleIdLoader; - public AbstractEntityBatchLoader(EntityMappingType entityDescriptor, SessionFactoryImplementor sessionFactory) { - super( entityDescriptor, sessionFactory ); - singleIdLoader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, sessionFactory ); + public AbstractEntityBatchLoader(EntityMappingType entityDescriptor, LoadQueryInfluencers loadQueryInfluencers) { + super( entityDescriptor, loadQueryInfluencers.getSessionFactory() ); + this.singleIdLoader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, loadQueryInfluencers ); } protected abstract void initializeEntities( diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderArrayParam.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderArrayParam.java index a36590b4a7..5ffff9fd24 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderArrayParam.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderArrayParam.java @@ -42,6 +42,7 @@ public class EntityBatchLoaderArrayParam implements SqlArrayMultiKeyLoader { private final int domainBatchSize; + private final LoadQueryInfluencers loadQueryInfluencers; private final BasicEntityIdentifierMapping identifierMapping; private final JdbcMapping arrayJdbcMapping; private final JdbcParameter jdbcParameter; @@ -63,8 +64,9 @@ public class EntityBatchLoaderArrayParam public EntityBatchLoaderArrayParam( int domainBatchSize, EntityMappingType entityDescriptor, - SessionFactoryImplementor sessionFactory) { - super( entityDescriptor, sessionFactory ); + LoadQueryInfluencers loadQueryInfluencers) { + super( entityDescriptor, loadQueryInfluencers ); + this.loadQueryInfluencers = loadQueryInfluencers; this.domainBatchSize = domainBatchSize; if ( MULTI_KEY_LOAD_LOGGER.isDebugEnabled() ) { @@ -89,7 +91,7 @@ public class EntityBatchLoaderArrayParam sqlAst = LoaderSelectBuilder.createSelectBySingleArrayParameter( getLoadable(), identifierMapping, - new LoadQueryInfluencers( sessionFactory ), + loadQueryInfluencers, LockOptions.NONE, jdbcParameter, sessionFactory diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderInPredicate.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderInPredicate.java index 04d27c5f54..493cb011d0 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderInPredicate.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/EntityBatchLoaderInPredicate.java @@ -46,6 +46,7 @@ public class EntityBatchLoaderInPredicate private final int domainBatchSize; private final int sqlBatchSize; + private final LoadQueryInfluencers loadQueryInfluencers; private final JdbcParametersList jdbcParameters; private final SelectStatement sqlAst; private final JdbcOperationQuerySelect jdbcSelectOperation; @@ -56,8 +57,9 @@ public class EntityBatchLoaderInPredicate public EntityBatchLoaderInPredicate( int domainBatchSize, EntityMappingType entityDescriptor, - SessionFactoryImplementor sessionFactory) { - super( entityDescriptor, sessionFactory ); + LoadQueryInfluencers loadQueryInfluencers) { + super( entityDescriptor, loadQueryInfluencers ); + this.loadQueryInfluencers = loadQueryInfluencers; this.domainBatchSize = domainBatchSize; int idColumnCount = entityDescriptor.getEntityPersister().getIdentifierType().getColumnSpan( sessionFactory ); this.sqlBatchSize = sessionFactory.getJdbcServices() @@ -85,7 +87,7 @@ public class EntityBatchLoaderInPredicate identifierMapping, null, sqlBatchSize, - new LoadQueryInfluencers( sessionFactory ), + loadQueryInfluencers, LockOptions.NONE, jdbcParametersBuilder::add, sessionFactory diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java index 8c48d89fef..17193d3f77 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/LoaderSelectBuilder.java @@ -30,6 +30,7 @@ import org.hibernate.metamodel.CollectionClassification; import org.hibernate.metamodel.mapping.AttributeMapping; import org.hibernate.metamodel.mapping.CollectionPart; import org.hibernate.metamodel.mapping.EntityIdentifierMapping; +import org.hibernate.metamodel.mapping.EntityMappingType; import org.hibernate.metamodel.mapping.EntityValuedModelPart; import org.hibernate.metamodel.mapping.ForeignKeyDescriptor; import org.hibernate.metamodel.mapping.ModelPart; @@ -67,6 +68,7 @@ import org.hibernate.sql.ast.tree.predicate.ComparisonPredicate; import org.hibernate.sql.ast.tree.predicate.InArrayPredicate; import org.hibernate.sql.ast.tree.predicate.InListPredicate; import org.hibernate.sql.ast.tree.predicate.InSubQueryPredicate; +import org.hibernate.sql.ast.tree.predicate.PredicateContainer; import org.hibernate.sql.ast.tree.select.QueryPart; import org.hibernate.sql.ast.tree.select.QuerySpec; import org.hibernate.sql.ast.tree.select.SelectStatement; @@ -708,16 +710,15 @@ public class LoaderSelectBuilder { } private void applyFiltering( - QuerySpec querySpec, + PredicateContainer predicateContainer, TableGroup tableGroup, Restrictable restrictable, SqlAstCreationState astCreationState) { restrictable.applyBaseRestrictions( - querySpec::applyPredicate, + predicateContainer::applyPredicate, tableGroup, true, - // HHH-16830 Session.find should apply filters only if specified on the filter definition - loadQueryInfluencers.getEnabledFiltersForFind(), + loadQueryInfluencers.getEnabledFilters(), null, astCreationState ); @@ -962,9 +963,9 @@ public class LoaderSelectBuilder { creationState ); - if ( fetch.getTiming() == FetchTiming.IMMEDIATE && isFetchablePluralAttributeMapping ) { - final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable; - if ( joined ) { + if ( fetch.getTiming() == FetchTiming.IMMEDIATE && joined ) { + if ( isFetchablePluralAttributeMapping ) { + final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) fetchable; final TableGroup joinTableGroup = creationState.getFromClauseAccess() .getTableGroup( fetchablePath ); final QuerySpec querySpec = creationState.getInflightQueryPart().getFirstQuerySpec(); @@ -981,6 +982,19 @@ public class LoaderSelectBuilder { creationState ); } + else if ( fetchable instanceof ToOneAttributeMapping ) { + final EntityMappingType entityType = ( (ToOneAttributeMapping) fetchable ).getEntityMappingType(); + final FromClauseAccess fromClauseAccess = creationState.getFromClauseAccess(); + final TableGroup joinTableGroup = fromClauseAccess.getTableGroup( fetchablePath ); + final TableGroupJoin join = fromClauseAccess.getTableGroup( fetchParent.getNavigablePath() ) + .findTableGroupJoin( joinTableGroup ); + applyFiltering( + join, + joinTableGroup, + entityType, + creationState + ); + } } fetches.add( fetch ); diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java index fd3646196e..4dbb45a4f8 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java @@ -30,7 +30,6 @@ import org.hibernate.loader.ast.spi.MultiIdLoadOptions; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.ast.SqlAstTranslatorFactory; -import org.hibernate.sql.ast.tree.expression.JdbcParameter; import org.hibernate.sql.ast.tree.select.SelectStatement; import org.hibernate.sql.exec.internal.JdbcParameterBindingsImpl; import org.hibernate.sql.exec.spi.JdbcOperationQuerySelect; @@ -220,7 +219,7 @@ public class MultiIdEntityLoaderStandard extends AbstractMultiIdEntityLoader< getLoadable().getIdentifierMapping(), null, numberOfIdsInBatch, - session.getLoadQueryInfluencers(), + session.getLoadQueryInfluencers().copyForLoadByKey(), lockOptions, jdbcParametersBuilder::add, getSessionFactory() diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java index df86138d55..53a08a5fac 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleIdEntityLoaderStandardImpl.java @@ -34,11 +34,11 @@ public class SingleIdEntityLoaderStandardImpl extends SingleIdEntityLoaderSup public SingleIdEntityLoaderStandardImpl( EntityMappingType entityDescriptor, - SessionFactoryImplementor sessionFactory) { + LoadQueryInfluencers loadQueryInfluencers) { this( entityDescriptor, - sessionFactory, - (lockOptions, influencers) -> createLoadPlan( entityDescriptor, lockOptions, influencers, sessionFactory ) + loadQueryInfluencers, + (lockOptions, influencers) -> createLoadPlan( entityDescriptor, lockOptions, influencers, influencers.getSessionFactory() ) ); } @@ -50,15 +50,14 @@ public class SingleIdEntityLoaderStandardImpl extends SingleIdEntityLoaderSup */ protected SingleIdEntityLoaderStandardImpl( EntityMappingType entityDescriptor, - SessionFactoryImplementor sessionFactory, + LoadQueryInfluencers influencers, BiFunction> loadPlanCreator) { // todo (6.0) : consider creating a base AST and "cloning" it - super( entityDescriptor, sessionFactory ); + super( entityDescriptor, influencers.getSessionFactory() ); this.loadPlanCreator = loadPlanCreator; // see org.hibernate.persister.entity.AbstractEntityPersister#createLoaders // we should preload a few - maybe LockMode.NONE and LockMode.READ final LockOptions lockOptions = LockOptions.NONE; - final LoadQueryInfluencers influencers = new LoadQueryInfluencers( sessionFactory ); final SingleIdLoadPlan plan = loadPlanCreator.apply( LockOptions.NONE, influencers ); if ( isLoadPlanReusable( lockOptions, influencers ) ) { selectByLockMode.put( lockOptions.getLockMode(), plan ); diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleUniqueKeyEntityLoaderStandard.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleUniqueKeyEntityLoaderStandard.java index e55437e202..242f7b1e0c 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleUniqueKeyEntityLoaderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/SingleUniqueKeyEntityLoaderStandard.java @@ -25,7 +25,6 @@ import org.hibernate.metamodel.mapping.ManagedMappingType; import org.hibernate.metamodel.mapping.ModelPart; import org.hibernate.metamodel.mapping.SingularAttributeMapping; import org.hibernate.metamodel.mapping.internal.ToOneAttributeMapping; -import org.hibernate.metamodel.model.domain.NavigableRole; import org.hibernate.query.spi.QueryOptions; import org.hibernate.sql.ast.SqlAstTranslatorFactory; import org.hibernate.sql.ast.tree.select.SelectStatement; @@ -52,7 +51,8 @@ public class SingleUniqueKeyEntityLoaderStandard implements SingleUniqueKeyEn public SingleUniqueKeyEntityLoaderStandard( EntityMappingType entityDescriptor, - SingularAttributeMapping uniqueKeyAttribute) { + SingularAttributeMapping uniqueKeyAttribute, + LoadQueryInfluencers loadQueryInfluencers) { this.entityDescriptor = entityDescriptor; this.uniqueKeyAttributePath = getAttributePath( uniqueKeyAttribute ); if ( uniqueKeyAttribute instanceof ToOneAttributeMapping ) { @@ -69,7 +69,7 @@ public class SingleUniqueKeyEntityLoaderStandard implements SingleUniqueKeyEn Collections.emptyList(), uniqueKeyAttribute, null, - new LoadQueryInfluencers( sessionFactory ), + loadQueryInfluencers, LockOptions.NONE, builder::add, sessionFactory diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java index 6c3e04931d..7b521ab052 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/StandardBatchLoaderFactory.java @@ -33,19 +33,20 @@ public class StandardBatchLoaderFactory implements BatchLoaderFactory { @Override public EntityBatchLoader createEntityBatchLoader( - int domainBatchSize, EntityMappingType entityDescriptor, - SessionFactoryImplementor factory) { - + int domainBatchSize, + EntityMappingType entityDescriptor, + LoadQueryInfluencers loadQueryInfluencers) { + final SessionFactoryImplementor factory = loadQueryInfluencers.getSessionFactory(); // NOTE : don't use the EntityIdentifierMapping here because it will not be known until later final Type identifierType = entityDescriptor.getEntityPersister().getIdentifierType(); if ( identifierType.getColumnSpan( factory ) == 1 && supportsSqlArrayType( factory.getJdbcServices().getDialect() ) && identifierType instanceof BasicType ) { // we can use a single ARRAY parameter to send all the ids - return new EntityBatchLoaderArrayParam<>( domainBatchSize, entityDescriptor, factory ); + return new EntityBatchLoaderArrayParam<>( domainBatchSize, entityDescriptor, loadQueryInfluencers ); } else { - return new EntityBatchLoaderInPredicate<>( domainBatchSize, entityDescriptor, factory ); + return new EntityBatchLoaderInPredicate<>( domainBatchSize, entityDescriptor, loadQueryInfluencers ); } } diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/BatchLoaderFactory.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/BatchLoaderFactory.java index 1e88e249b8..9f019c202c 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/BatchLoaderFactory.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/BatchLoaderFactory.java @@ -18,6 +18,21 @@ import org.hibernate.service.Service; * @author Steve Ebersole */ public interface BatchLoaderFactory extends Service { + /** + * Create a BatchLoader for batch-loadable entities. + * + * @param domainBatchSize The total number of entities (max) that will be need to be initialized + * @param entityDescriptor The entity mapping metadata + * @deprecated Use {@link #createEntityBatchLoader(int, EntityMappingType, LoadQueryInfluencers)} instead + */ + @Deprecated(forRemoval = true) + default EntityBatchLoader createEntityBatchLoader( + int domainBatchSize, + EntityMappingType entityDescriptor, + SessionFactoryImplementor factory) { + return createEntityBatchLoader( domainBatchSize, entityDescriptor, new LoadQueryInfluencers( factory ) ); + } + /** * Create a BatchLoader for batch-loadable entities. * @@ -27,7 +42,7 @@ public interface BatchLoaderFactory extends Service { EntityBatchLoader createEntityBatchLoader( int domainBatchSize, EntityMappingType entityDescriptor, - SessionFactoryImplementor factory); + LoadQueryInfluencers loadQueryInfluencers); /** * Create a BatchLoader for batch-loadable collections. diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/Loadable.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/Loadable.java index 80eb6c8041..9205cc593d 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/Loadable.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/spi/Loadable.java @@ -35,6 +35,13 @@ public interface Loadable extends ModelPart, RootTableGroupProducer { || isAffectedByBatchSize( influencers ); } + default boolean isAffectedByInfluencersForLoadByKey(LoadQueryInfluencers influencers) { + return isAffectedByEntityGraph( influencers ) + || isAffectedByEnabledFetchProfiles( influencers ) + || isAffectedByEnabledFiltersForLoadByKey( influencers ) + || isAffectedByBatchSize( influencers ); + } + default boolean isNotAffectedByInfluencers(LoadQueryInfluencers influencers) { return !isAffectedByEntityGraph( influencers ) && !isAffectedByEnabledFetchProfiles( influencers ) @@ -55,6 +62,13 @@ public interface Loadable extends ModelPart, RootTableGroupProducer { */ boolean isAffectedByEnabledFilters(LoadQueryInfluencers influencers); + /** + * Whether any of the "influencers" affect this loadable. + */ + default boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers influencers) { + return isAffectedByInfluencers( influencers.copyForLoadByKey() ); + } + /** * Whether the {@linkplain LoadQueryInfluencers#getEffectiveEntityGraph() effective entity-graph} * applies to this loadable diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java index dca086f3ba..cbb40f3338 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/EntityMappingType.java @@ -547,6 +547,11 @@ public interface EntityMappingType return getEntityPersister().isAffectedByEnabledFilters( influencers ); } + @Override + default boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers influencers) { + return getEntityPersister().isAffectedByEnabledFiltersForLoadByKey( influencers ); + } + @Override default boolean isAffectedByEntityGraph(LoadQueryInfluencers influencers) { return getEntityPersister().isAffectedByEntityGraph( influencers ); diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java index 298819e3d8..8b59d8dd54 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/PluralAttributeMappingImpl.java @@ -1077,6 +1077,11 @@ public class PluralAttributeMappingImpl return getCollectionDescriptor().isAffectedByEnabledFilters( influencers ); } + @Override + public boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers influencers) { + return getCollectionDescriptor().isAffectedByEnabledFiltersForLoadByKey( influencers ); + } + @Override public boolean isAffectedByEntityGraph(LoadQueryInfluencers influencers) { return getCollectionDescriptor().isAffectedByEntityGraph( influencers ); 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 050e756b70..52e1a8890f 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 @@ -248,7 +248,7 @@ public class ToOneAttributeMapping stateArrayPosition, fetchableIndex, attributeMetadata, - adjustFetchTiming( mappedFetchTiming, bootValue ), + adjustFetchTiming( mappedFetchTiming, bootValue, entityMappingType ), mappedFetchStyle, declaringType, propertyAccess @@ -270,7 +270,7 @@ public class ToOneAttributeMapping ); if ( bootValue instanceof ManyToOne ) { final ManyToOne manyToOne = (ManyToOne) bootValue; - this.notFoundAction = ( (ManyToOne) bootValue ).getNotFoundAction(); + this.notFoundAction = determineNotFoundAction( ( (ManyToOne) bootValue ).getNotFoundAction(), entityMappingType ); if ( manyToOne.isLogicalOneToOne() ) { cardinality = Cardinality.LOGICAL_ONE_TO_ONE; } @@ -424,7 +424,7 @@ public class ToOneAttributeMapping else { this.bidirectionalAttributePath = SelectablePath.parse( oneToOne.getMappedByProperty() ); } - notFoundAction = null; + notFoundAction = determineNotFoundAction( null, entityMappingType ); isKeyTableNullable = isNullable(); isOptional = !bootValue.isConstrained(); isInternalLoadNullable = isNullable(); @@ -569,6 +569,16 @@ 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, @@ -638,12 +648,18 @@ public class ToOneAttributeMapping return null; } - private static FetchTiming adjustFetchTiming(FetchTiming mappedFetchTiming, ToOne bootValue) { + private static FetchTiming adjustFetchTiming( + FetchTiming mappedFetchTiming, + ToOne bootValue, + EntityMappingType entityMappingType) { if ( bootValue instanceof ManyToOne ) { if ( ( (ManyToOne) bootValue ).getNotFoundAction() != null ) { return FetchTiming.IMMEDIATE; } } + if ( entityMappingType.getEntityPersister().hasFilterForLoadByKey() ) { + return FetchTiming.IMMEDIATE; + } return mappedFetchTiming; } diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java index 60803f6102..97f04e2387 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/AbstractCollectionPersister.java @@ -1677,6 +1677,18 @@ public abstract class AbstractCollectionPersister } } + @Override + public boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers influencers) { + if ( influencers.hasEnabledFilters() ) { + final Map enabledFilters = influencers.getEnabledFilters(); + return filterHelper != null && filterHelper.isAffectedByApplyToLoadByKey( enabledFilters ) + || manyToManyFilterHelper != null && manyToManyFilterHelper.isAffectedByApplyToLoadByKey( enabledFilters ); + } + else { + return false; + } + } + @Override public boolean isAffectedByEntityGraph(LoadQueryInfluencers influencers) { // todo (6.0) : anything to do here? diff --git a/hibernate-core/src/main/java/org/hibernate/persister/collection/CollectionPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/collection/CollectionPersister.java index 296dba4c82..9d8433559b 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/collection/CollectionPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/collection/CollectionPersister.java @@ -296,6 +296,10 @@ public interface CollectionPersister extends Restrictable { throw new UnsupportedOperationException( "CollectionPersister used for [" + getRole() + "] does not support SQL AST" ); } + default boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers influencers) { + throw new UnsupportedOperationException( "CollectionPersister used for [" + getRole() + "] does not support SQL AST" ); + } + default boolean isAffectedByEntityGraph(LoadQueryInfluencers influencers) { throw new UnsupportedOperationException( "CollectionPersister used for [" + getRole() + "] does not support SQL AST" ); } 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 cac8d6a7b6..14fb4190f6 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 @@ -17,7 +17,6 @@ import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.IdentityHashMap; import java.util.LinkedHashMap; import java.util.List; import java.util.Locale; @@ -26,6 +25,7 @@ import java.util.Objects; import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Supplier; @@ -536,6 +536,16 @@ public abstract class AbstractEntityPersister ? MutableEntityEntryFactory.INSTANCE : ImmutableEntityEntryFactory.INSTANCE; + // Handle any filters applied to the class level + filterHelper = isNotEmpty( persistentClass.getFilters() ) ? new FilterHelper( + persistentClass.getFilters(), + getEntityNameByTableNameMap( + persistentClass, + factory.getSqlStringGenerationContext() + ), + factory + ) : null; + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ representationStrategy = creationContext.getBootstrapContext().getRepresentationStrategySelector() @@ -804,16 +814,6 @@ public abstract class AbstractEntityPersister propertyDefinedOnSubclass = toBooleanArray( definedBySubclass ); - // Handle any filters applied to the class level - filterHelper = isNotEmpty( persistentClass.getFilters() ) ? new FilterHelper( - persistentClass.getFilters(), - getEntityNameByTableNameMap( - persistentClass, - factory.getSqlStringGenerationContext() - ), - factory - ) : null; - useReferenceCacheEntries = shouldUseReferenceCacheEntries( creationContext.getSessionFactoryOptions() ); useShallowQueryCacheLayout = shouldUseShallowCacheLayout( persistentClass.getQueryCacheLayout(), @@ -884,10 +884,10 @@ public abstract class AbstractEntityPersister final int batchSize = loadQueryInfluencers.effectiveBatchSize( this ); return factory.getServiceRegistry() .requireService( BatchLoaderFactory.class ) - .createEntityBatchLoader( batchSize, this, factory ); + .createEntityBatchLoader( batchSize, this, loadQueryInfluencers ); } else { - return new SingleIdEntityLoaderStandardImpl<>( this, factory ); + return new SingleIdEntityLoaderStandardImpl<>( this, loadQueryInfluencers ); } } @@ -1255,6 +1255,18 @@ public abstract class AbstractEntityPersister return storeDiscriminatorInShallowQueryCacheLayout; } + @Override + public boolean hasFilterForLoadByKey() { + if ( filterHelper != null ) { + for ( String filterName : filterHelper.getFilterNames() ) { + if ( factory.getFilterDefinition( filterName ).isApplyToLoadByKey() ) { + return true; + } + } + } + return false; + } + @Override public Iterable uniqueKeyEntries() { if ( this.uniqueKeyEntries == null ) { @@ -2039,7 +2051,7 @@ public abstract class AbstractEntityPersister ); } - return getUniqueKeyLoader( uniquePropertyName ).resolveId( key, session ); + return getUniqueKeyLoader( uniquePropertyName, session ).resolveId( key, session ); } @@ -2652,16 +2664,25 @@ public abstract class AbstractEntityPersister Object uniqueKey, Boolean readOnly, SharedSessionContractImplementor session) throws HibernateException { - return getUniqueKeyLoader( propertyName ).load( uniqueKey, LockOptions.NONE, readOnly, session ); + return getUniqueKeyLoader( propertyName, session ).load( uniqueKey, LockOptions.NONE, readOnly, session ); } private Map> uniqueKeyLoadersNew; - protected SingleUniqueKeyEntityLoader getUniqueKeyLoader(String attributeName) { + protected SingleUniqueKeyEntityLoader getUniqueKeyLoader(String attributeName, SharedSessionContractImplementor session) { final SingularAttributeMapping attribute = (SingularAttributeMapping) findByPath( attributeName ); + final LoadQueryInfluencers influencers = session.getLoadQueryInfluencers(); + // no subselect fetching for entities for now + if ( isAffectedByInfluencersForLoadByKey( influencers ) ) { + return new SingleUniqueKeyEntityLoaderStandard<>( + this, + attribute, + influencers.copyForLoadByKey() + ); + } final SingleUniqueKeyEntityLoader existing; if ( uniqueKeyLoadersNew == null ) { - uniqueKeyLoadersNew = new IdentityHashMap<>(); + uniqueKeyLoadersNew = new ConcurrentHashMap<>(); existing = null; } else { @@ -2673,7 +2694,7 @@ public abstract class AbstractEntityPersister } else { final SingleUniqueKeyEntityLoader loader = - new SingleUniqueKeyEntityLoaderStandard<>( this, attribute ); + new SingleUniqueKeyEntityLoaderStandard<>( this, attribute, new LoadQueryInfluencers( factory ) ); uniqueKeyLoadersNew.put( attribute, loader ); return loader; } @@ -3744,8 +3765,8 @@ public abstract class AbstractEntityPersister else { final LoadQueryInfluencers influencers = session.getLoadQueryInfluencers(); // no subselect fetching for entities for now - return isAffectedByInfluencers( influencers ) - ? buildSingleIdEntityLoader( influencers ) + return isAffectedByInfluencersForLoadByKey( influencers ) + ? buildSingleIdEntityLoader( influencers.copyForLoadByKey() ) : getSingleIdLoader(); } } @@ -3863,6 +3884,30 @@ public abstract class AbstractEntityPersister return false; } + @Override + public boolean isAffectedByEnabledFiltersForLoadByKey(LoadQueryInfluencers loadQueryInfluencers) { + if ( filterHelper != null && loadQueryInfluencers.hasEnabledFilters() ) { + if ( filterHelper.isAffectedByApplyToLoadByKey( loadQueryInfluencers.getEnabledFilters() ) ) { + return true; + } + + // we still need to verify collection fields to be eagerly loaded by join + final AttributeMappingsList attributeMappings = getAttributeMappings(); + for ( int i = 0; i < attributeMappings.size(); i++ ) { + final AttributeMapping attributeMapping = attributeMappings.get( i ); + if ( attributeMapping instanceof PluralAttributeMapping ) { + final PluralAttributeMapping pluralAttributeMapping = (PluralAttributeMapping) attributeMapping; + if ( pluralAttributeMapping.getMappedFetchOptions().getTiming() == FetchTiming.IMMEDIATE + && pluralAttributeMapping.getMappedFetchOptions().getStyle() == FetchStyle.JOIN + && pluralAttributeMapping.getCollectionDescriptor().isAffectedByEnabledFiltersForLoadByKey( loadQueryInfluencers ) ) { + return true; + } + } + } + } + return false; + } + @Override public boolean isSubclassPropertyNullable(int i) { return subclassPropertyNullabilityClosure[i]; 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 65d2ec8784..afb3cffcbe 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 @@ -1258,6 +1258,8 @@ public interface EntityPersister extends EntityMappingType, EntityMutationTarget @Incubating boolean storeDiscriminatorInShallowQueryCacheLayout(); + boolean hasFilterForLoadByKey(); + /** * The property name of the "special" identifier property in HQL * diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java index 0b3e9c4b37..4e373cca83 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/tree/from/TableGroupJoin.java @@ -14,13 +14,14 @@ import org.hibernate.sql.ast.SqlTreeCreationLogger; import org.hibernate.sql.ast.spi.SqlAstTreeHelper; import org.hibernate.sql.ast.tree.SqlAstNode; import org.hibernate.sql.ast.tree.predicate.Predicate; +import org.hibernate.sql.ast.tree.predicate.PredicateContainer; import org.hibernate.sql.results.graph.DomainResult; import org.hibernate.sql.results.graph.DomainResultCreationState; /** * @author Steve Ebersole */ -public class TableGroupJoin implements TableJoin, DomainResultProducer { +public class TableGroupJoin implements TableJoin, PredicateContainer, DomainResultProducer { private final NavigablePath navigablePath; private final TableGroup joinedGroup; @@ -78,6 +79,7 @@ public class TableGroupJoin implements TableJoin, DomainResultProducer { return predicate; } + @Override public void applyPredicate(Predicate predicate) { this.predicate = SqlAstTreeHelper.combinePredicates( this.predicate, predicate ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java b/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java index 8374ebab67..924f70b41a 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/cfg/persister/GoofyPersisterClassProvider.java @@ -760,6 +760,11 @@ public class GoofyPersisterClassProvider implements PersisterClassResolver { return false; } + @Override + public boolean hasFilterForLoadByKey() { + return false; + } + @Override public Iterable uniqueKeyEntries() { return Collections.emptyList(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/LoadPlanBuilderTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/LoadPlanBuilderTest.java index b99c591318..eee42c2fbd 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/LoadPlanBuilderTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/entitygraph/ast/LoadPlanBuilderTest.java @@ -54,7 +54,7 @@ public class LoadPlanBuilderTest { .getMappingMetamodel() .getEntityDescriptor( Message.class ); - final SingleIdEntityLoaderStandardImpl loader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, sessionFactory ); + final SingleIdEntityLoaderStandardImpl loader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, new LoadQueryInfluencers( sessionFactory ) ); final SingleIdLoadPlan loadPlan = loader.resolveLoadPlan( LockOptions.READ, @@ -89,7 +89,7 @@ public class LoadPlanBuilderTest { final SessionFactoryImplementor sessionFactory = scope.getSessionFactory(); final EntityPersister entityDescriptor = (EntityPersister) sessionFactory.getRuntimeMetamodels().getEntityMappingType( Message.class ); - final SingleIdEntityLoaderStandardImpl loader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, sessionFactory ); + final SingleIdEntityLoaderStandardImpl loader = new SingleIdEntityLoaderStandardImpl<>( entityDescriptor, new LoadQueryInfluencers( sessionFactory ) ); final LoadQueryInfluencers influencers = new LoadQueryInfluencers( sessionFactory ) { @Override diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ejb3configuration/PersisterClassProviderTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ejb3configuration/PersisterClassProviderTest.java index f7fb75d28e..3eea7a28bb 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ejb3configuration/PersisterClassProviderTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/ejb3configuration/PersisterClassProviderTest.java @@ -739,6 +739,11 @@ public class PersisterClassProviderTest { return false; } + @Override + public boolean hasFilterForLoadByKey() { + return false; + } + @Override public Iterable uniqueKeyEntries() { return Collections.emptyList(); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/legacy/CustomPersister.java b/hibernate-core/src/test/java/org/hibernate/orm/test/legacy/CustomPersister.java index 83c6440a75..c546fb226f 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/legacy/CustomPersister.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/legacy/CustomPersister.java @@ -896,6 +896,11 @@ public class CustomPersister implements EntityPersister { return false; } + @Override + public boolean hasFilterForLoadByKey() { + return false; + } + @Override public Iterable uniqueKeyEntries() { return Collections.emptyList(); 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 ac3a1521d0..2759f2d9a5 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 @@ -14,6 +14,7 @@ import java.util.function.Supplier; import jakarta.persistence.CascadeType; import jakarta.persistence.Column; import jakarta.persistence.Entity; +import jakarta.persistence.EntityGraph; import jakarta.persistence.EnumType; import jakarta.persistence.Enumerated; import jakarta.persistence.FetchType; @@ -23,13 +24,18 @@ import jakarta.persistence.NoResultException; import jakarta.persistence.OneToMany; import jakarta.persistence.Table; +import org.hibernate.FetchNotFoundException; import org.hibernate.Session; import org.hibernate.annotations.Filter; import org.hibernate.annotations.FilterDef; import org.hibernate.annotations.ParamDef; +import org.hibernate.jpa.AvailableHints; import org.hibernate.metamodel.CollectionClassification; import org.hibernate.orm.test.jpa.BaseEntityManagerFunctionalTestCase; +import org.hibernate.testing.orm.junit.JiraKey; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import static org.hibernate.cfg.AvailableSettings.DEFAULT_LIST_SEMANTICS; @@ -38,6 +44,8 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; /** * @author Vlad Mihalcea @@ -58,47 +66,59 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { options.put( DEFAULT_LIST_SEMANTICS, CollectionClassification.BAG.name() ); } - @Test - public void testLifecycle() { + @Before + public void setup() { doInJPA(this::entityManagerFactory, entityManager -> { - //tag::pc-filter-persistence-example[] Client client = new Client() - .setId(1L) - .setName("John Doe") - .setType(AccountType.DEBIT); + .setId(1L) + .setName("John Doe") + .setType(AccountType.DEBIT); + + Account account1; + client.addAccount( + account1 = new Account() + .setId(1L) + .setType(AccountType.CREDIT) + .setAmount(5000d) + .setRate(1.25 / 100) + .setActive(true) + ); client.addAccount( - new Account() - .setId(1L) - .setType(AccountType.CREDIT) - .setAmount(5000d) - .setRate(1.25 / 100) - .setActive(true) - ); + new Account() + .setId(2L) + .setType(AccountType.DEBIT) + .setAmount(0d) + .setRate(1.05 / 100) + .setActive(false) + .setParentAccount( account1 ) + ); client.addAccount( - new Account() - .setId(2L) - .setType(AccountType.DEBIT) - .setAmount(0d) - .setRate(1.05 / 100) - .setActive(false) - ); - - client.addAccount( - new Account() - .setType(AccountType.DEBIT) - .setId(3L) - .setAmount(250d) - .setRate(1.05 / 100) - .setActive(true) - ); + new Account() + .setType(AccountType.DEBIT) + .setId(3L) + .setAmount(250d) + .setRate(1.05 / 100) + .setActive(true) + ); entityManager.persist(client); //end::pc-filter-persistence-example[] }); + } + @After + public void tearDown() { + doInJPA(this::entityManagerFactory, entityManager -> { + entityManager.createQuery( "delete from Account" ).executeUpdate(); + entityManager.createQuery( "delete from Client" ).executeUpdate(); + }); + } + + @Test + public void testLifecycle() { doInJPA(this::entityManagerFactory, entityManager -> { log.infof("Activate filter [%s]", "activeAccount"); @@ -177,6 +197,33 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { //end::pc-filter-entity-query-example[] }); + doInJPA(this::entityManagerFactory, entityManager -> { + //tag::pc-no-filter-collection-query-example[] + Client client = entityManager.find(Client.class, 1L); + + assertEquals(3, client.getAccounts().size()); + //end::pc-no-filter-collection-query-example[] + }); + + doInJPA(this::entityManagerFactory, entityManager -> { + log.infof("Activate filter [%s]", "activeAccount"); + + //tag::pc-filter-collection-query-example[] + entityManager + .unwrap(Session.class) + .enableFilter("activeAccount") + .setParameter("active", true); + + Client client = entityManager.find(Client.class, 1L); + + assertEquals(2, client.getAccounts().size()); + //end::pc-filter-collection-query-example[] + }); + } + + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKey() { doInJPA(this::entityManagerFactory, entityManager -> { log.infof("Activate filter [%s]", "minimumAmount"); //tag::pc-filter-entity-example[] @@ -220,28 +267,44 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { assertEquals(1, accounts.size()); //end::pc-filter-entity-query-example[] }); + } + @Test + @JiraKey("HHH-16830") + public void testApplyToLoadByKeyAssociationFiltering() { doInJPA(this::entityManagerFactory, entityManager -> { - //tag::pc-no-filter-collection-query-example[] - Client client = entityManager.find(Client.class, 1L); - - assertEquals(3, client.getAccounts().size()); - //end::pc-no-filter-collection-query-example[] + Account account = entityManager.find(Account.class, 2L); + assertNotNull( account.getParentAccount() ); }); - doInJPA(this::entityManagerFactory, entityManager -> { - log.infof("Activate filter [%s]", "activeAccount"); + entityManager.unwrap(Session.class) + .enableFilter("accountType") + .setParameter("type", "DEBIT"); - //tag::pc-filter-collection-query-example[] - entityManager - .unwrap(Session.class) - .enableFilter("activeAccount") - .setParameter("active", true); + FetchNotFoundException exception = assertThrows( + FetchNotFoundException.class, + () -> entityManager.find( Account.class, 2L ) + ); + // 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"); + EntityGraph entityGraph = entityManager.createEntityGraph( Account.class ); + entityGraph.addAttributeNodes( "parentAccount" ); - Client client = entityManager.find(Client.class, 1L); - - assertEquals(2, client.getAccounts().size()); - //end::pc-filter-collection-query-example[] + FetchNotFoundException exception = assertThrows( + FetchNotFoundException.class, + () -> entityManager.find( + Account.class, + 2L, + Map.of( AvailableHints.HINT_SPEC_LOAD_GRAPH, entityGraph ) + ) + ); + // Account with id 1 does not exist + assertTrue( exception.getMessage().contains( "`1`" ) ); }); } @@ -333,12 +396,24 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { name="amount", type=Double.class ), - applyToLoadById = true + applyToLoadByKey = true ) @Filter( name="minimumAmount", condition="amount > :amount" ) + @FilterDef( + name="accountType", + parameters = @ParamDef( + name="type", + type=String.class + ), + applyToLoadByKey = true + ) + @Filter( + name="accountType", + condition="account_type = :type" + ) public static class Account { @@ -361,6 +436,10 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { //Getters and setters omitted for brevity //end::pc-filter-Account-example[] + + @ManyToOne(fetch = FetchType.LAZY) + private Account parentAccount; + public Long getId() { return id; } @@ -415,6 +494,14 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { return this; } + public Account getParentAccount() { + return parentAccount; + } + + public Account setParentAccount(Account parentAccount) { + this.parentAccount = parentAccount; + return this; + } //tag::pc-filter-Account-example[] } //end::pc-filter-Account-example[]