From f2deb8f58e990ee87b6e00a736f6549d092da3d0 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 14 Feb 2023 12:03:47 +0100 Subject: [PATCH] HHH-14514 Fix auto eviction of collection cache --- .../internal/CollectionCacheInvalidator.java | 12 +- ...oncurrencyCollectionCacheEvictionTest.java | 269 ++++++++++++++++++ 2 files changed, 274 insertions(+), 7 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/cache/TransactionalConcurrencyCollectionCacheEvictionTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/CollectionCacheInvalidator.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/CollectionCacheInvalidator.java index 85d3d18964..b8d11ee463 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/CollectionCacheInvalidator.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/CollectionCacheInvalidator.java @@ -181,13 +181,14 @@ public class CollectionCacheInvalidator if ( LOG.isDebugEnabled() ) { LOG.debug( "Evict CollectionRegion " + collectionPersister.getRole() + " for id " + id ); } - AfterTransactionCompletionProcess afterTransactionProcess = new CollectionEvictCacheAction( + CollectionEvictCacheAction evictCacheAction = new CollectionEvictCacheAction( collectionPersister, null, id, session - ).lockCache(); - session.getActionQueue().registerProcess( afterTransactionProcess ); + ); + evictCacheAction.execute(); + session.getActionQueue().registerProcess( evictCacheAction.getAfterTransactionCompletionProcess() ); } //execute the same process as invalidation with collection operations @@ -202,11 +203,8 @@ public class CollectionCacheInvalidator @Override public void execute() throws HibernateException { - } - - public AfterTransactionCompletionProcess lockCache() { beforeExecutions(); - return getAfterTransactionCompletionProcess(); + evict(); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/cache/TransactionalConcurrencyCollectionCacheEvictionTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/cache/TransactionalConcurrencyCollectionCacheEvictionTest.java new file mode 100644 index 0000000000..ba23e9b01b --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/cache/TransactionalConcurrencyCollectionCacheEvictionTest.java @@ -0,0 +1,269 @@ +/* + * 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 . + */ +package org.hibernate.orm.test.cache; + +import java.util.HashSet; +import java.util.Set; +import jakarta.persistence.Access; +import jakarta.persistence.AccessType; +import jakarta.persistence.Cacheable; +import jakarta.persistence.Column; +import jakarta.persistence.Entity; +import jakarta.persistence.FetchType; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.GenerationType; +import jakarta.persistence.Id; +import jakarta.persistence.JoinColumn; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.OneToMany; +import jakarta.persistence.Table; + +import org.hibernate.annotations.Cache; +import org.hibernate.annotations.CacheConcurrencyStrategy; +import org.hibernate.cache.internal.CollectionCacheInvalidator; +import org.hibernate.cfg.Configuration; +import org.hibernate.cfg.Environment; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.hibernate.testing.transaction.TransactionUtil.doInHibernate; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; + +/** + * @author Christian Beikov + */ +@TestForIssue(jiraKey = "HHH-4910") +public class TransactionalConcurrencyCollectionCacheEvictionTest extends BaseCoreFunctionalTestCase { + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { Person.class, Phone.class }; + } + + @Before + public void before() { + CollectionCacheInvalidator.PROPAGATE_EXCEPTION = true; + } + + @After + public void after() { + CollectionCacheInvalidator.PROPAGATE_EXCEPTION = false; + } + + @Override + protected void configure(Configuration cfg) { + super.configure( cfg ); + cfg.setProperty( Environment.AUTO_EVICT_COLLECTION_CACHE, "true" ); + cfg.setProperty( Environment.USE_SECOND_LEVEL_CACHE, "true" ); + cfg.setProperty( Environment.USE_QUERY_CACHE, "false" ); + } + + @Override + protected void prepareTest() throws Exception { + doInHibernate( + this::sessionFactory, + s -> { + Person bart = new Person( 1L, "Bart" ); + Person lisa = new Person( 2L, "Lisa" ); + Person maggie = new Person( 3L, "Maggie" ); + s.persist( bart ); + s.persist( lisa ); + s.persist( maggie ); + + bart.addPhone( "0-1122334455" ); + bart.addPhone( "0-2233445566" ); + bart.addPhone( "0-3344556677" ); + bart.addPhone( "0-4455667788" ); + bart.addPhone( "0-5566778899" ); + + lisa.addPhone( "1-1122334455" ); + lisa.addPhone( "1-2233445566" ); + lisa.addPhone( "1-3344556677" ); + lisa.addPhone( "1-4455667788" ); + lisa.addPhone( "1-5566778899" ); + + maggie.addPhone( "2-1122334455" ); + maggie.addPhone( "2-2233445566" ); + maggie.addPhone( "2-3344556677" ); + maggie.addPhone( "2-4455667788" ); + maggie.addPhone( "2-5566778899" ); + + bart.getPhones().forEach( s::persist ); + lisa.getPhones().forEach( s::persist ); + maggie.getPhones().forEach( s::persist ); + } + ); + } + + @Override + protected void cleanupTest() throws Exception { + doInHibernate( + this::sessionFactory, + s -> { + s.createQuery( "delete from Phone" ).executeUpdate(); + s.createQuery( "delete from Person" ).executeUpdate(); + } + ); + } + + @Test + public void testCollectionCacheEvictionInsert() { + doInHibernate( + this::sessionFactory, + s -> { + Person bart = s.find( Person.class, 1L ); + assertEquals( 5, bart.getPhones().size() ); + s.persist( new Phone( "test", bart ) ); + } + ); + doInHibernate( + this::sessionFactory, + s -> { + Person bart = s.find( Person.class, 1L ); + assertEquals( 6, bart.getPhones().size() ); + } + ); + } + + @Test + public void testCollectionCacheEvictionRemove() { + Long phoneId = doInHibernate( + this::sessionFactory, + s -> { + Person bart = s.find( Person.class, 1L ); + // Lazy load phones + assertEquals( 5, bart.getPhones().size() ); + return bart.getPhones().iterator().next().getId(); + } + ); + doInHibernate( + this::sessionFactory, + s -> { + s.remove( s.getReference( Phone.class, phoneId ) ); + } + ); + doInHibernate( + this::sessionFactory, + s -> { + Person bart = s.find( Person.class, 1L ); + assertEquals( 4, bart.getPhones().size() ); + } + ); + } + + @Entity(name = "Person") + @Table(name = "PERSON") + @Cacheable + @Cache(usage = CacheConcurrencyStrategy.TRANSACTIONAL) + public static class Person { + + @Id + @Access(value = AccessType.PROPERTY) + @Column(name = "PERSONID", nullable = false) + private Long id; + + @Column(name = "NAME") + private String name; + + @OneToMany(fetch = FetchType.LAZY, mappedBy = "person") + @Cache(usage = CacheConcurrencyStrategy.TRANSACTIONAL) + private final Set phones = new HashSet<>(); + + public Person() { + } + + public Person(Long id, String name) { + this.id = id; + this.name = name; + } + + public Long getId() { + return id; + } + + public void setId(final Long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(final String name) { + this.name = name; + } + + public Set getPhones() { + return phones; + } + + public Phone addPhone(String number) { + Phone phone = new Phone( number, this ); + getPhones().add( phone ); + return phone; + } + } + + @Entity(name = "Phone") + @Table(name = "PHONE") + @Cacheable + @Cache(usage = CacheConcurrencyStrategy.TRANSACTIONAL) + public static class Phone { + + @Id + @Access(value = AccessType.PROPERTY) + @GeneratedValue(strategy = GenerationType.AUTO) + @Column(name = "PHONEID", nullable = false) + private Long id; + + @Column(name = "PHONE_NUMBER") + private String number; + + @ManyToOne(fetch = FetchType.LAZY) + @JoinColumn(name = "PERSONID") + private Person person; + + public Phone() { + } + + public Phone(String number, Person person) { + this.number = number; + this.person = person; + } + + public Long getId() { + return id; + } + + public void setId(Long id) { + this.id = id; + } + + public String getNumber() { + return number; + } + + public void setNumber(String number) { + this.number = number; + } + + public Person getPerson() { + return person; + } + + public void setPerson(Person person) { + this.person = person; + } + } + +}