diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java index e4475a3a5a..2d486e0700 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java @@ -6,6 +6,7 @@ */ package org.hibernate.cache.spi.support; +import org.hibernate.Internal; import org.hibernate.cache.spi.DomainDataRegion; import org.hibernate.cache.spi.access.CachedDomainDataAccess; import org.hibernate.cache.spi.access.SoftLock; @@ -34,7 +35,8 @@ public abstract class AbstractCachedDomainDataAccess implements CachedDomainData return region; } - protected DomainDataStorageAccess getStorageAccess() { + @Internal + public DomainDataStorageAccess getStorageAccess() { return storageAccess; } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryRestrictedCollectionCachingTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryRestrictedCollectionCachingTests.java new file mode 100644 index 0000000000..c04d22df86 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/querycache/QueryRestrictedCollectionCachingTests.java @@ -0,0 +1,176 @@ +/* + * 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.orm.test.querycache; + +import java.io.Serializable; + +import org.hibernate.CacheMode; +import org.hibernate.cache.internal.BasicCacheKeyImplementation; +import org.hibernate.cache.spi.CacheImplementor; +import org.hibernate.cache.spi.access.CollectionDataAccess; +import org.hibernate.cache.spi.entry.CollectionCacheEntry; +import org.hibernate.cache.spi.support.AbstractReadWriteAccess; +import org.hibernate.cache.spi.support.CollectionReadWriteAccess; +import org.hibernate.cache.spi.support.DomainDataStorageAccess; +import org.hibernate.cfg.AvailableSettings; +import org.hibernate.metamodel.model.domain.NavigableRole; + +import org.hibernate.testing.cache.MapStorageAccessImpl; +import org.hibernate.testing.orm.domain.StandardDomainModel; +import org.hibernate.testing.orm.domain.library.Book; +import org.hibernate.testing.orm.domain.library.Person; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.FailureExpected; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.ServiceRegistry; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.Setting; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.CacheStoreMode; +import jakarta.persistence.SharedCacheMode; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.fail; + +/** + * Assertions that collections which are join fetched and restricted in a query do not get put into the + * second level cache with the filtered state + * + * @author Steve Ebersole + */ +@SuppressWarnings("JUnitMalformedDeclaration") +@Jira( "https://hibernate.atlassian.net/browse/HHH-2003" ) +@ServiceRegistry(settings = { + @Setting( name= AvailableSettings.USE_SECOND_LEVEL_CACHE, value = "true"), + @Setting( name= AvailableSettings.USE_QUERY_CACHE, value = "true"), +}) +@DomainModel(standardModels = StandardDomainModel.LIBRARY, sharedCacheMode = SharedCacheMode.ALL) +@SessionFactory +public class QueryRestrictedCollectionCachingTests { + public static final String AUTHORS_ROLE = Book.class.getName() + ".authors"; + + @Test + void testSimpleFetch(SessionFactoryScope sessions) { + final CacheImplementor cache = sessions.getSessionFactory().getCache(); + cache.evictAllRegions(); + + sessions.inTransaction( (session) -> { + final Book book = session.createSelectionQuery( "from Book b left join fetch b.authors a", Book.class ).getSingleResult(); + assertThat( book ).isNotNull(); + assertThat( book.getAuthors() ).hasSize( 2 ); + } ); + + assertThat( cache.containsCollection( AUTHORS_ROLE, 1 ) ).isTrue(); + assertThat( extractCachedCollectionKeys( cache, AUTHORS_ROLE, 1 ) ).hasSize( 2 ); + } + + @Test + void testSimpleFetch2(SessionFactoryScope sessions) { + final CacheImplementor cache = sessions.getSessionFactory().getCache(); + cache.evictAllRegions(); + + sessions.inTransaction( (session) -> { + final Book book = session.createSelectionQuery( + "from Book b left join fetch b.authors a where b.id = 1", + Book.class + ).getSingleResult(); + assertThat( book ).isNotNull(); + assertThat( book.getAuthors() ).hasSize( 2 ); + } ); + + assertThat( cache.containsCollection( AUTHORS_ROLE, 1 ) ).isTrue(); + assertThat( extractCachedCollectionKeys( cache, AUTHORS_ROLE, 1 ) ).hasSize( 2 ); + } + + @Test + void testRestrictedFetchWithCacheIgnored(SessionFactoryScope sessions) { + final CacheImplementor cache = sessions.getSessionFactory().getCache(); + cache.evictAllRegions(); + + sessions.inTransaction( (session) -> { + final Book book = session + .createSelectionQuery( "from Book b left join fetch b.authors a where a.id = 1", Book.class ) + .setCacheMode( CacheMode.IGNORE ) + .getSingleResult(); + assertThat( book ).isNotNull(); + assertThat( book.getAuthors() ).hasSize( 1 ); + } ); + + // we ignored the cache explicitly + assertThat( cache.containsCollection( AUTHORS_ROLE, 1 ) ).isFalse(); + } + + @Test + @FailureExpected + void testRestrictedFetch(SessionFactoryScope sessions) { + final CacheImplementor cache = sessions.getSessionFactory().getCache(); + cache.evictAllRegions(); + + sessions.inTransaction( (session) -> { + final Book book = session + .createSelectionQuery( "from Book b left join fetch b.authors a where a.id = 1", Book.class ) + .getSingleResult(); + assertThat( book ).isNotNull(); + assertThat( book.getAuthors() ).hasSize( 1 ); + } ); + + // This is the crux of HHH-2003. + // At the moment we put the filtered collection into the cache + assertThat( cache.containsCollection( AUTHORS_ROLE, 1 ) ).isTrue(); + // this is just some deeper checks to show that the data is "corrupt" + assertThat( extractCachedCollectionKeys( cache, AUTHORS_ROLE, 1 ) ).hasSize( 1 ); + + fail( "Really, HHH-2003 the collection to not be cached here" ); + } + + private static Serializable[] extractCachedCollectionKeys(CacheImplementor cache, String role, Integer ownerKey) { + final NavigableRole navigableRole = new NavigableRole( role ); + final CollectionReadWriteAccess authorsRegionAccess = (CollectionReadWriteAccess) cache.getCollectionRegionAccess( navigableRole ); + + final MapStorageAccessImpl storageAccess = (MapStorageAccessImpl) authorsRegionAccess.getStorageAccess(); + final BasicCacheKeyImplementation cacheKey = new BasicCacheKeyImplementation( ownerKey, role, ownerKey ); + final AbstractReadWriteAccess.Item cacheItem = (AbstractReadWriteAccess.Item) storageAccess.getFromData( cacheKey ); + assertThat( cacheItem ).isNotNull(); + + final CollectionCacheEntry cacheEntry = (CollectionCacheEntry) cacheItem.getValue(); + return cacheEntry.getState(); + } + + @BeforeEach + void createTestData(SessionFactoryScope sessions) { + sessions.inTransaction( (session) -> { + final Person poe = new Person( 1, "John Poe" ); + session.persist( poe ); + + final Person schmidt = new Person( 2, "Jacob Schmidt" ); + session.persist( schmidt ); + + final Person king = new Person( 3, "David King" ); + session.persist( king ); + + final Book nightsEdge = new Book( 1, "A Night's Edge" ); + nightsEdge.addAuthor( poe ); + nightsEdge.addAuthor( king ); + nightsEdge.addEditor( schmidt ); + session.persist( nightsEdge ); + } ); + } + + @AfterEach + void dropTestData(SessionFactoryScope sessions) { + sessions.inTransaction( (session) -> { +// session.createNativeMutationQuery( "delete book_authors" ).executeUpdate(); +// session.createNativeMutationQuery( "delete book_editors" ).executeUpdate(); + session.createMutationQuery( "delete Book" ).executeUpdate(); + session.createMutationQuery( "delete Person" ).executeUpdate(); + } ); + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/cache/MapStorageAccessImpl.java b/hibernate-testing/src/main/java/org/hibernate/testing/cache/MapStorageAccessImpl.java index a34cb9efb8..3fa69abff0 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/cache/MapStorageAccessImpl.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/cache/MapStorageAccessImpl.java @@ -9,6 +9,7 @@ package org.hibernate.testing.cache; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import org.hibernate.Internal; import org.hibernate.cache.spi.support.DomainDataStorageAccess; import org.hibernate.engine.spi.SharedSessionContractImplementor; @@ -20,6 +21,14 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; public class MapStorageAccessImpl implements DomainDataStorageAccess { private ConcurrentMap data; + @Internal + public Object getFromData(Object key) { + if ( data == null ) { + return null; + } + return data.get( key ); + } + @Override public boolean contains(Object key) { return data != null && data.containsKey( key ); @@ -27,10 +36,7 @@ public class MapStorageAccessImpl implements DomainDataStorageAccess { @Override public Object getFromCache(Object key, SharedSessionContractImplementor session) { - if ( data == null ) { - return null; - } - return data.get( key ); + return getFromData( key ); } @Override diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/StandardDomainModel.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/StandardDomainModel.java index 140e2857ba..3c76dbd005 100644 --- a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/StandardDomainModel.java +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/StandardDomainModel.java @@ -10,6 +10,7 @@ import org.hibernate.testing.orm.domain.animal.AnimalDomainModel; import org.hibernate.testing.orm.domain.contacts.ContactsDomainModel; import org.hibernate.testing.orm.domain.gambit.GambitDomainModel; import org.hibernate.testing.orm.domain.helpdesk.HelpDeskDomainModel; +import org.hibernate.testing.orm.domain.library.LibraryDomainModel; import org.hibernate.testing.orm.domain.retail.RetailDomainModel; import org.hibernate.testing.orm.domain.userguide.UserguideDomainModel; @@ -22,7 +23,8 @@ public enum StandardDomainModel { GAMBIT( GambitDomainModel.INSTANCE ), HELPDESK( HelpDeskDomainModel.INSTANCE ), RETAIL( RetailDomainModel.INSTANCE ), - USERGUIDE( UserguideDomainModel.INSTANCE ); + USERGUIDE( UserguideDomainModel.INSTANCE ), + LIBRARY( LibraryDomainModel.INSTANCE ); private final DomainModelDescriptor domainModel; diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Book.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Book.java new file mode 100644 index 0000000000..dd4f4e154c --- /dev/null +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Book.java @@ -0,0 +1,107 @@ +/* + * 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.testing.orm.domain.library; + +import java.util.HashSet; +import java.util.Set; + +import org.hibernate.annotations.Cache; +import org.hibernate.annotations.CacheConcurrencyStrategy; + +import jakarta.persistence.CollectionTable; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Basic; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.JoinTable; +import jakarta.persistence.ManyToMany; + +/** + * @author Steve Ebersole + */ +@Entity +public class Book { + @Id + private Integer id; + private String name; + private String isbn; + + @ManyToMany + @JoinTable( name = "book_authors", + joinColumns = @JoinColumn(name = "book_fk"), + inverseJoinColumns = @JoinColumn(name = "author_fk") + ) + @Cache( usage = CacheConcurrencyStrategy.READ_WRITE) + Set authors; + + @ManyToMany + @JoinTable( name = "book_editors", + joinColumns = @JoinColumn(name = "book_fk"), + inverseJoinColumns = @JoinColumn(name = "editor_fk") + ) + @Cache( usage = CacheConcurrencyStrategy.READ_WRITE) + Set editors; + + protected Book() { + // for Hibernate use + } + + public Book(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getIsbn() { + return isbn; + } + + public void setIsbn(String isbn) { + this.isbn = isbn; + } + + public Set getAuthors() { + return authors; + } + + public void setAuthors(Set authors) { + this.authors = authors; + } + + public void addAuthor(Person author) { + if ( authors == null ) { + authors = new HashSet<>(); + } + authors.add( author ); + } + + public Set getEditors() { + return editors; + } + + public void setEditors(Set editors) { + this.editors = editors; + } + + public void addEditor(Person editor) { + if ( editors == null ) { + editors = new HashSet<>(); + } + editors.add( editor ); + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/LibraryDomainModel.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/LibraryDomainModel.java new file mode 100644 index 0000000000..2026d74ffc --- /dev/null +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/LibraryDomainModel.java @@ -0,0 +1,20 @@ +/* + * 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.testing.orm.domain.library; + +import org.hibernate.testing.orm.domain.AbstractDomainModelDescriptor; + +/** + * @author Steve Ebersole + */ +public class LibraryDomainModel extends AbstractDomainModelDescriptor { + public static final LibraryDomainModel INSTANCE = new LibraryDomainModel(); + + public LibraryDomainModel() { + super( Book.class, Person.class ); + } +} diff --git a/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Person.java b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Person.java new file mode 100644 index 0000000000..31c6456887 --- /dev/null +++ b/hibernate-testing/src/main/java/org/hibernate/testing/orm/domain/library/Person.java @@ -0,0 +1,43 @@ +/* + * 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.testing.orm.domain.library; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.Basic; + +/** + * @author Steve Ebersole + */ +@Entity +public class Person { + @Id + private Integer id; + @Basic + private String name; + + protected Person() { + // for Hibernate use + } + + public Person(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +}