From e721a3769134eb3bdd5ebf80404be52706b657b5 Mon Sep 17 00:00:00 2001 From: Dmitrii Karmanov Date: Fri, 24 May 2024 17:20:09 +0200 Subject: [PATCH] HHH-16830: apply filters to find() method --- .../chapters/pc/PersistenceContext.adoc | 14 ++++- .../extras/pc-filter-entity-find-example.sql | 14 +++++ .../src/main/java/org/hibernate/Filter.java | 8 +++ .../org/hibernate/annotations/FilterDef.java | 9 +++ .../binder/internal/TenantIdBinder.java | 3 +- .../boot/model/internal/AnnotationBinder.java | 8 +-- .../boot/model/internal/FilterDefBinder.java | 3 +- .../internal/hbm/FilterDefinitionBinder.java | 1 + .../engine/spi/FilterDefinition.java | 16 ++++- .../engine/spi/LoadQueryInfluencers.java | 15 +++++ .../org/hibernate/internal/FilterImpl.java | 17 ++++- .../ast/internal/LoaderSelectBuilder.java | 5 +- .../hibernate/xsd/mapping/mapping-3.1.0.xsd | 1 + .../org/hibernate/orm/test/pc/FilterTest.java | 63 ++++++++++++++++++- .../orm/test/tenantid/TenantIdTest.java | 8 ++- .../test/tenantlongid/TenantLongIdTest.java | 3 +- .../orm/test/tenantuuid/TenantUuidTest.java | 4 +- 17 files changed, 168 insertions(+), 24 deletions(-) create mode 100644 documentation/src/main/asciidoc/userguide/chapters/pc/extras/pc-filter-entity-find-example.sql diff --git a/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc b/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc index 2d1e595271..bcd9b72441 100644 --- a/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc +++ b/documentation/src/main/asciidoc/userguide/chapters/pc/PersistenceContext.adoc @@ -558,9 +558,11 @@ include::{example-dir-pc}/FilterTest.java[tags=pc-filter-resolver-Account-exampl [IMPORTANT] ==== -Filters apply to entity queries, but not to direct fetching. +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 filter is not taken into consideration when fetching an entity from the Persistence Context. +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 `applyToLoadByKey` flag is set to `true`. [[pc-filter-entity-example]] .Fetching entities mapped with `@Filter` @@ -574,7 +576,13 @@ include::{example-dir-pc}/FilterTest.java[tags=pc-filter-entity-example] include::{extrasdir}/pc-filter-entity-example.sql[] ---- -As you can see from the example above, contrary to an entity query, the filter does not prevent the entity from being loaded. +[source, SQL, indent=0] +---- +include::{extrasdir}/pc-filter-entity-find-example.sql[] +---- + +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`, diff --git a/documentation/src/main/asciidoc/userguide/chapters/pc/extras/pc-filter-entity-find-example.sql b/documentation/src/main/asciidoc/userguide/chapters/pc/extras/pc-filter-entity-find-example.sql new file mode 100644 index 0000000000..4c803059b1 --- /dev/null +++ b/documentation/src/main/asciidoc/userguide/chapters/pc/extras/pc-filter-entity-find-example.sql @@ -0,0 +1,14 @@ +SELECT + a.id as id1_0_0_, + a.active_status as active2_0_0_, + a.amount as amount3_0_0_, + a.client_id as client_i6_0_0_, + a.rate as rate4_0_0_, + a.account_type as account_5_0_0_, + c.id as id1_1_1_, + c.name as name2_1_1_ +FROM + Account a +WHERE + a.id = 1 + AND a.amount > 9000 \ No newline at end of file diff --git a/hibernate-core/src/main/java/org/hibernate/Filter.java b/hibernate-core/src/main/java/org/hibernate/Filter.java index 38a89f0784..ef1683227d 100644 --- a/hibernate-core/src/main/java/org/hibernate/Filter.java +++ b/hibernate-core/src/main/java/org/hibernate/Filter.java @@ -95,4 +95,12 @@ public interface Filter { * @return The flag value */ boolean isAutoEnabled(); + + /** + * Get the associated {@link FilterDefinition applyToLoadByKey} of this + * named filter. + * + * @return The flag value + */ + 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 6560454e39..154b8d4d7a 100644 --- a/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java +++ b/hibernate-core/src/main/java/org/hibernate/annotations/FilterDef.java @@ -95,4 +95,13 @@ public @interface FilterDef { * The flag used to auto-enable the filter on the session. */ boolean autoEnabled() default false; + + /** + * The flag used to decide if the filter will + * be applied on direct fetches or not. + *

+ * If the flag is true, the filter will be + * applied on direct fetches, such as findById(). + */ + 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 35e0347932..3f235981cc 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,8 @@ public class TenantIdBinder implements AttributeBinder { "", singletonMap( PARAMETER_NAME, tenantIdType ), Collections.emptyMap(), - false + true, + true ) ); } diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotationBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotationBinder.java index c0d55e21ca..16d26b6824 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotationBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/internal/AnnotationBinder.java @@ -416,11 +416,11 @@ public final class AnnotationBinder { //@Entity and @MappedSuperclass on the same class leads to a NPE down the road if ( annotatedClass.isAnnotationPresent( Entity.class ) ) { throw new AnnotationException( "Type '" + annotatedClass.getName() - + "' is annotated both '@Entity' and '@MappedSuperclass'" ); + + "' is annotated both '@Entity' and '@MappedSuperclass'" ); } if ( annotatedClass.isAnnotationPresent( Table.class ) ) { throw new AnnotationException( "Mapped superclass '" + annotatedClass.getName() - + "' may not specify a '@Table'" ); + + "' may not specify a '@Table'" ); } if ( annotatedClass.isAnnotationPresent( Inheritance.class ) ) { throw new AnnotationException( "Mapped superclass '" + annotatedClass.getName() @@ -651,8 +651,8 @@ public final class AnnotationBinder { for ( FetchOverride fetch : fetchProfile.fetchOverrides() ) { if ( fetch.fetch() == FetchType.LAZY && fetch.mode() == FetchMode.JOIN ) { throw new AnnotationException( "Fetch profile '" + name - + "' has a '@FetchOverride' with 'fetch=LAZY' and 'mode=JOIN'" - + " (join fetching is eager by nature)"); + + "' has a '@FetchOverride' with 'fetch=LAZY' and 'mode=JOIN'" + + " (join fetching is eager by nature)"); } context.getMetadataCollector() .addSecondPass( new FetchOverrideSecondPass( name, fetch, context ) ); 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 62c609496f..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 @@ -92,7 +92,8 @@ class FilterDefBinder { filterDef.defaultCondition(), explicitParamJaMappings, parameterResolvers, - filterDef.autoEnabled() + filterDef.autoEnabled(), + filterDef.applyToLoadByKey() ); LOG.debugf( "Binding filter definition: %s", filterDefinition.getFilterName() ); context.getMetadataCollector().addFilterDefinition( filterDefinition ); diff --git a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/FilterDefinitionBinder.java b/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/FilterDefinitionBinder.java index 2fea0b9f0c..d5e7c3d62b 100644 --- a/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/FilterDefinitionBinder.java +++ b/hibernate-core/src/main/java/org/hibernate/boot/model/source/internal/hbm/FilterDefinitionBinder.java @@ -7,6 +7,7 @@ package org.hibernate.boot.model.source.internal.hbm; import java.io.Serializable; +import java.util.Collections; import java.util.HashMap; import java.util.Map; 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 f597441c9d..484f3bc2fc 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,6 +37,7 @@ public class FilterDefinition implements Serializable { private final Map explicitParamJaMappings = new HashMap<>(); private final Map>> parameterResolverMap = new HashMap<>(); private final boolean autoEnabled; + private final boolean applyToLoadByKey; /** * Construct a new FilterDefinition instance. @@ -44,17 +45,18 @@ public class FilterDefinition implements Serializable { * @param name The name of the filter for which this configuration is in effect. */ public FilterDefinition(String name, String defaultCondition, @Nullable Map explicitParamJaMappings) { - this( name, defaultCondition, explicitParamJaMappings, Collections.emptyMap(), false); + this( name, defaultCondition, explicitParamJaMappings, Collections.emptyMap(), false, false); } public FilterDefinition( String name, String defaultCondition, @Nullable Map explicitParamJaMappings, - Map>> parameterResolverMap, boolean autoEnabled) { + Map>> parameterResolverMap, boolean autoEnabled, boolean applyToLoadByKey) { this.filterName = name; this.defaultFilterCondition = defaultCondition; if ( explicitParamJaMappings != null ) { this.explicitParamJaMappings.putAll( explicitParamJaMappings ); } + this.applyToLoadByKey = applyToLoadByKey; if ( parameterResolverMap != null ) { this.parameterResolverMap.putAll( parameterResolverMap ); } @@ -101,6 +103,16 @@ public class FilterDefinition implements Serializable { return defaultFilterCondition; } + /** + * Get a flag that defines if the filter should be applied + * on direct fetches or not. + * + * @return The flag value. + */ + public boolean isApplyToLoadByKey() { + return applyToLoadByKey; + } + /** * Called before binding a JDBC parameter * 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 c55efc1f2d..a52c8e680a 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,6 +13,7 @@ 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; @@ -168,6 +169,20 @@ public class LoadQueryInfluencers implements Serializable { } } + /** + * Returns a Map of enabled filters that have the applyToLoadByKey + * flag set to true + * @return a Map of enabled filters that have the applyToLoadByKey + * flag set to true + */ + public Map getEnabledFiltersForFind() { + return getEnabledFilters() + .entrySet() + .stream() + .filter(f -> f.getValue().isApplyToLoadByKey()) + .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. 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 61970b47a1..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,8 @@ public class FilterImpl implements Filter, Serializable { private final String filterName; private final Map parameters = new HashMap<>(); private final boolean autoEnabled; - + private final boolean applyToLoadByKey; + void afterDeserialize(SessionFactoryImplementor factory) { definition = factory.getFilterDefinition( filterName ); validate(); @@ -47,6 +48,7 @@ public class FilterImpl implements Filter, Serializable { this.definition = configuration; filterName = definition.getFilterName(); this.autoEnabled = definition.isAutoEnabled(); + this.applyToLoadByKey = definition.isApplyToLoadByKey(); } public FilterDefinition getFilterDefinition() { @@ -70,7 +72,18 @@ public class FilterImpl implements Filter, Serializable { public boolean isAutoEnabled() { return autoEnabled; } - + + + /** + * Get a flag that defines if the filter should be applied + * on direct fetches or not. + * + * @return The flag value. + */ + public boolean isApplyToLoadByKey() { + return applyToLoadByKey; + } + public Map getParameters() { return parameters; } 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 341f7ff3bf..8c48d89fef 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 @@ -7,7 +7,6 @@ package org.hibernate.loader.ast.internal; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Objects; @@ -717,8 +716,8 @@ public class LoaderSelectBuilder { querySpec::applyPredicate, tableGroup, true, - // HHH-16179 Session.find should not apply filters - Collections.emptyMap(),//loadQueryInfluencers.getEnabledFilters(), + // HHH-16830 Session.find should apply filters only if specified on the filter definition + loadQueryInfluencers.getEnabledFiltersForFind(), null, astCreationState ); diff --git a/hibernate-core/src/main/resources/org/hibernate/xsd/mapping/mapping-3.1.0.xsd b/hibernate-core/src/main/resources/org/hibernate/xsd/mapping/mapping-3.1.0.xsd index 181de30b7c..7f07e8fbfa 100644 --- a/hibernate-core/src/main/resources/org/hibernate/xsd/mapping/mapping-3.1.0.xsd +++ b/hibernate-core/src/main/resources/org/hibernate/xsd/mapping/mapping-3.1.0.xsd @@ -2281,6 +2281,7 @@ + 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 3fffa75d3c..db1894fd72 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 @@ -177,6 +177,50 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { //end::pc-filter-entity-query-example[] }); + doInJPA(this::entityManagerFactory, entityManager -> { + log.infof("Activate filter [%s]", "minimumAmount"); + //tag::pc-filter-entity-example[] + entityManager + .unwrap(Session.class) + .enableFilter("minimumAmount") + .setParameter("amount", 9000d); + + Account account = entityManager.find(Account.class, 1L); + + assertNull( account ); + //end::pc-filter-entity-example[] + }); + + doInJPA(this::entityManagerFactory, entityManager -> { + log.infof("Activate filter [%s]", "minimumAmount"); + //tag::pc-filter-entity-example[] + entityManager + .unwrap(Session.class) + .enableFilter("minimumAmount") + .setParameter("amount", 100d); + + Account account = entityManager.find(Account.class, 1L); + + assertNotNull( account ); + //end::pc-filter-entity-example[] + }); + + doInJPA(this::entityManagerFactory, entityManager -> { + log.infof("Activate filter [%s]", "minimumAmount"); + //tag::pc-filter-entity-query-example[] + entityManager + .unwrap(Session.class) + .enableFilter("minimumAmount") + .setParameter("amount", 500d); + + List accounts = entityManager.createQuery( + "select a from Account a", Account.class) + .getResultList(); + + assertEquals(1, accounts.size()); + //end::pc-filter-entity-query-example[] + }); + doInJPA(this::entityManagerFactory, entityManager -> { //tag::pc-no-filter-collection-query-example[] Client client = entityManager.find(Client.class, 1L); @@ -280,9 +324,22 @@ public class FilterTest extends BaseEntityManagerFunctionalTestCase { ) ) @Filter( - name="activeAccount", - condition="active_status = :active" - ) + name="activeAccount", + condition="active_status = :active" + ) + @FilterDef( + name="minimumAmount", + parameters = @ParamDef( + name="amount", + type=Double.class + ), + applyToLoadByKey = true + ) + @Filter( + name="minimumAmount", + condition="amount > :amount" + ) + public static class Account { @Id diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantid/TenantIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantid/TenantIdTest.java index 2753163072..b4b45ef599 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantid/TenantIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantid/TenantIdTest.java @@ -40,6 +40,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.hibernate.cfg.AvailableSettings.JAKARTA_HBM2DDL_DATABASE_ACTION; import static org.hibernate.internal.util.collections.CollectionHelper.toMap; import static org.hibernate.jpa.HibernateHints.HINT_TENANT_ID; +import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; @@ -110,7 +111,8 @@ public class TenantIdTest implements SessionFactoryProducer { currentTenant = "yours"; scope.inTransaction( session -> { - assertNotNull( session.find(Account.class, acc.id) ); + //HHH-16830 Sessions applies tenantId filter on find() + assertNull( session.find(Account.class, acc.id) ); assertEquals( 0, session.createQuery("from Account", Account.class).getResultList().size() ); session.disableFilter(TenantIdBinder.FILTER_NAME); assertNotNull( session.find(Account.class, acc.id) ); @@ -247,9 +249,9 @@ public class TenantIdTest implements SessionFactoryProducer { Record r = em.find( Record.class, record.id ); assertEquals( "mine", r.state.tenantId ); - // Session seems to not apply tenant-id on #find + // HHH-16830 Session applies tenant-id on #find Record yours = em.find( Record.class, record2.id ); - assertEquals( "yours", yours.state.tenantId ); + assertNull(yours); em.createQuery( "from Record where id = :id", Record.class ) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantlongid/TenantLongIdTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantlongid/TenantLongIdTest.java index 9249135bd2..0d98864d0d 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantlongid/TenantLongIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantlongid/TenantLongIdTest.java @@ -79,7 +79,8 @@ public class TenantLongIdTest implements SessionFactoryProducer { currentTenant = yours; scope.inTransaction( session -> { - assertNotNull( session.find(Account.class, acc.id) ); + //HHH-16830 Sessions applies tenantId filter on find() + assertNull( session.find(Account.class, acc.id) ); assertEquals( 0, session.createQuery("from Account").getResultList().size() ); session.disableFilter(TenantIdBinder.FILTER_NAME); assertNotNull( session.find(Account.class, acc.id) ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantuuid/TenantUuidTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantuuid/TenantUuidTest.java index bc8c8a3430..377235ee00 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/tenantuuid/TenantUuidTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/tenantuuid/TenantUuidTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.api.Test; import java.util.UUID; import static org.hibernate.cfg.AvailableSettings.JAKARTA_HBM2DDL_DATABASE_ACTION; +import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -86,7 +87,8 @@ public class TenantUuidTest implements SessionFactoryProducer { currentTenant = yours; scope.inTransaction( session -> { - assertNotNull( session.find(Account.class, acc.id) ); + //HHH-16830 Sessions applies tenantId filter on find() + assertNull( session.find(Account.class, acc.id) ); assertEquals( 0, session.createQuery("from Account").getResultList().size() ); session.disableFilter(TenantIdBinder.FILTER_NAME); assertNotNull( session.find(Account.class, acc.id) );