HHH-10264 - Values weren't cached after persist

HHH-9140 - Allow to look for id outside of context
ERROR CollectionCacheInvalidator:145 - org.hibernate.TransientObjectException: The instance was not associated with this session
	at org.hibernate.internal.SessionImpl.getIdentifier(SessionImpl.java:1511)
(cherry picked from commit f0d8fcd)

Added property to propagate error in test case

Test case without mappedBy
org.hibernate.HibernateException: Unable to resolve property:
	at org.hibernate.tuple.entity.EntityMetamodel.getPropertyIndex(EntityMetamodel.java:926)

(cherry picked from commit bf2eb01856)

hibernate-core/src/main/java/org/hibernate/cache/internal/CollectionCacheInvalidator.java

@@ -9,9 +9,14 @@ package org.hibernate.cache.internal;
 import java.io.Serializable;
 import java.util.Set;
 
+import org.hibernate.HibernateException;
+import org.hibernate.action.internal.CollectionAction;
+import org.hibernate.action.spi.AfterTransactionCompletionProcess;
 import org.hibernate.boot.Metadata;
-import org.hibernate.cache.spi.access.CollectionRegionAccessStrategy;
+import org.hibernate.cache.spi.access.SoftLock;
+import org.hibernate.collection.spi.PersistentCollection;
 import org.hibernate.engine.spi.SessionFactoryImplementor;
+import org.hibernate.engine.spi.SessionImplementor;
 import org.hibernate.event.service.spi.EventListenerRegistry;
 import org.hibernate.event.spi.EventSource;
 import org.hibernate.event.spi.EventType;
@@ -40,6 +45,8 @@ import org.jboss.logging.Logger;
 public class CollectionCacheInvalidator
 		implements Integrator, PostInsertEventListener, PostDeleteEventListener, PostUpdateEventListener {
 	private static final Logger LOG = Logger.getLogger( CollectionCacheInvalidator.class.getName() );
+	public static final String PROPAGATE_EXCEPTION = "hibernate.test.auto_evict_collection_cache.propagate_exception";
+	private boolean propagateException;
 
 	@Override
 	public void integrate(Metadata metadata, SessionFactoryImplementor sessionFactory,
@@ -80,6 +87,8 @@ public class CollectionCacheInvalidator
 			// Nothing to do, if caching is disabled
 			return;
 		}
+		propagateException = Boolean.parseBoolean(
+				sessionFactory.getProperties().getProperty( PROPAGATE_EXCEPTION ) );
 		EventListenerRegistry eventListenerRegistry = serviceRegistry.getService( EventListenerRegistry.class );
 		eventListenerRegistry.appendListeners( EventType.POST_INSERT, this );
 		eventListenerRegistry.appendListeners( EventType.POST_DELETE, this );
@@ -95,14 +104,14 @@ public class CollectionCacheInvalidator
 				return;
 			}
 			for ( String role : collectionRoles ) {
-				CollectionPersister collectionPersister = factory.getCollectionPersister( role );
+				final CollectionPersister collectionPersister = factory.getCollectionPersister( role );
 				if ( !collectionPersister.hasCache() ) {
 					// ignore collection if no caching is used
 					continue;
 				}
 				// this is the property this OneToMany relation is mapped by
 				String mappedBy = collectionPersister.getMappedByProperty();
-				if ( mappedBy != null ) {
+				if ( mappedBy != null && !mappedBy.isEmpty() ) {
 					int i = persister.getEntityMetamodel().getPropertyIndex( mappedBy );
 					Serializable oldId = null;
 					if ( oldState != null ) {
@@ -113,7 +122,11 @@ public class CollectionCacheInvalidator
 					Object ref = persister.getPropertyValue( entity, i );
 					Serializable id = null;
 					if ( ref != null ) {
-						id = session.getIdentifier( ref );
+						id = session.getContextEntityIdentifier( ref );
+						if ( id == null ) {
+							id = session.getSessionFactory().getClassMetadata( ref.getClass() )
+									.getIdentifier( ref, session );
+						}
 					}
 					// only evict if the related entity has changed
 					if ( id != null && !id.equals( oldId ) ) {
@@ -125,11 +138,20 @@ public class CollectionCacheInvalidator
 				}
 				else {
 					LOG.debug( "Evict CollectionRegion " + role );
-					collectionPersister.getCacheAccessStrategy().evictAll();
+					final SoftLock softLock = collectionPersister.getCacheAccessStrategy().lockRegion();
+					session.getActionQueue().registerProcess( new AfterTransactionCompletionProcess() {
+						@Override
+						public void doAfterTransactionCompletion(boolean success, SessionImplementor session) {
+							collectionPersister.getCacheAccessStrategy().unlockRegion( softLock );
+						}
+					} );
 				}
 			}
 		}
 		catch ( Exception e ) {
+			if ( propagateException ) {
+				throw new IllegalStateException( e );
+			}
 			// don't let decaching influence other logic
 			LOG.error( "", e );
 		}
@@ -139,13 +161,32 @@ public class CollectionCacheInvalidator
 		if ( LOG.isDebugEnabled() ) {
 			LOG.debug( "Evict CollectionRegion " + collectionPersister.getRole() + " for id " + id );
 		}
-		CollectionRegionAccessStrategy cache = collectionPersister.getCacheAccessStrategy();
+		AfterTransactionCompletionProcess afterTransactionProcess = new CollectionEvictCacheAction(
-		Object key = cache.generateCacheKey(
-				id,
 				collectionPersister,
-				session.getFactory(),
+				null,
-				session.getTenantIdentifier()
+				id,
-		);
+				session
-		cache.evict( key );
+		).lockCache();
+		session.getActionQueue().registerProcess( afterTransactionProcess );
+	}
+
+	//execute the same process as invalidation with collection operations
+	private static final class CollectionEvictCacheAction extends CollectionAction {
+		protected CollectionEvictCacheAction(
+				CollectionPersister persister,
+				PersistentCollection collection,
+				Serializable key,
+				SessionImplementor session) {
+			super( persister, collection, key, session );
+		}
+
+		@Override
+		public void execute() throws HibernateException {
+		}
+
+		public AfterTransactionCompletionProcess lockCache() {
+			beforeExecutions();
+			return getAfterTransactionCompletionProcess();
+		}
 	}
 }

hibernate-core/src/test/java/org/hibernate/test/cache/CollectionCacheEvictionTest.java

@@ -8,15 +8,20 @@ package org.hibernate.test.cache;
 
 import org.hibernate.ObjectNotFoundException;
 import org.hibernate.Session;
+import org.hibernate.cache.internal.CollectionCacheInvalidator;
+import org.hibernate.cache.spi.access.CollectionRegionAccessStrategy;
 import org.hibernate.cfg.Configuration;
 import org.hibernate.cfg.Environment;
-
+import org.hibernate.engine.spi.SessionImplementor;
-import org.junit.Test;
+import org.hibernate.persister.collection.CollectionPersister;
 
 import org.hibernate.testing.TestForIssue;
 import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
 /**
@@ -36,6 +41,7 @@ public class CollectionCacheEvictionTest extends BaseCoreFunctionalTestCase {
 		cfg.setProperty( Environment.USE_SECOND_LEVEL_CACHE, "true" );
 		cfg.setProperty( Environment.USE_QUERY_CACHE, "true" );
 		cfg.setProperty( Environment.CACHE_PROVIDER_CONFIG, "true" );
+		cfg.setProperty( CollectionCacheInvalidator.PROPAGATE_EXCEPTION, "true" );
 	}
 
 	@Override
@@ -68,6 +74,31 @@ public class CollectionCacheEvictionTest extends BaseCoreFunctionalTestCase {
 		s.close();
 	}
 
+	@Test
+	public void testCachedValueAfterEviction() {
+		CollectionPersister persister = sessionFactory().getCollectionPersister( Company.class.getName() + ".users" );
+
+		Session session = openSession();
+		SessionImplementor sessionImplementor = (SessionImplementor) session;
+
+		CollectionRegionAccessStrategy cache = persister.getCacheAccessStrategy();
+		Object key = cache.generateCacheKey( 1, persister, sessionFactory(), session.getTenantIdentifier() );
+		Object cachedValue = cache.get( sessionImplementor, key, sessionImplementor.getTimestamp() );
+		assertNull( cachedValue );
+
+		Company company = session.get( Company.class, 1 );
+		//should add in cache
+		assertEquals( 1, company.getUsers().size() );
+		session.close();
+
+		session = openSession();
+		sessionImplementor = (SessionImplementor) session;
+		key = cache.generateCacheKey( 1, persister, sessionFactory(), session.getTenantIdentifier() );
+		cachedValue = cache.get( sessionImplementor, key, sessionImplementor.getTimestamp() );
+		assertNotNull( "Collection wasn't cached", cachedValue );
+		session.close();
+	}
+
 	@Test
 	public void testCollectionCacheEvictionInsert() {
 		Session s = openSession();
@@ -93,6 +124,29 @@ public class CollectionCacheEvictionTest extends BaseCoreFunctionalTestCase {
 		s.close();
 	}
 
+	@Test
+	public void testCollectionCacheEvictionInsertWithEntityOutOfContext() {
+		Session s = openSession();
+		Company company = s.get( Company.class, 1 );
+		assertEquals( 1, company.getUsers().size() );
+		s.close();
+
+		s = openSession();
+		s.beginTransaction();
+
+		User user = new User( 2, company );
+		s.save( user );
+
+		s.getTransaction().commit();
+		s.close();
+
+		s = openSession();
+
+		company = s.get( Company.class, 1 );
+		assertEquals( 2, company.getUsers().size() );
+		s.close();
+	}
+
 	@Test
 	public void testCollectionCacheEvictionRemove() {
 		Session s = openSession();
@@ -121,6 +175,32 @@ public class CollectionCacheEvictionTest extends BaseCoreFunctionalTestCase {
 		s.close();
 	}
 
+	@Test
+	public void testCollectionCacheEvictionRemoveWithEntityOutOfContext() {
+		Session s = openSession();
+		Company company = s.get( Company.class, 1 );
+		assertEquals( 1, company.getUsers().size() );
+		s.close();
+
+		s = openSession();
+		s.beginTransaction();
+		s.delete( company.getUsers().get( 0 ) );
+
+		s.getTransaction().commit();
+		s.close();
+
+		s = openSession();
+
+		company = s.get( Company.class, 1 );
+		try {
+			assertEquals( 0, company.getUsers().size() );
+		}
+		catch ( ObjectNotFoundException e ) {
+			fail( "Cached element not found" );
+		}
+		s.close();
+	}
+
 	@Test
 	public void testCollectionCacheEvictionUpdate() {
 		Session s = openSession();
@@ -157,4 +237,39 @@ public class CollectionCacheEvictionTest extends BaseCoreFunctionalTestCase {
 
 		s.close();
 	}
+
+	@Test
+	public void testCollectionCacheEvictionUpdateWithEntityOutOfContext() {
+		Session s = openSession();
+		Company company1 = s.get( Company.class, 1 );
+		Company company2 = s.get( Company.class, 2 );
+
+		assertEquals( 1, company1.getUsers().size() );
+		assertEquals( 0, company2.getUsers().size() );
+
+		s.close();
+		s = openSession();
+		s.beginTransaction();
+
+		User user = s.get( User.class, 1 );
+		user.setCompany( company2 );
+
+		s.getTransaction().commit();
+		s.close();
+
+		s = openSession();
+
+		company1 = s.get( Company.class, 1 );
+		company2 = s.get( Company.class, 2 );
+
+		assertEquals( 1, company2.getUsers().size() );
+
+		try {
+			assertEquals( 0, company1.getUsers().size() );
+		}
+		catch ( ObjectNotFoundException e ) {
+			fail( "Cached element not found" );
+		}
+		s.close();
+	}
 }

hibernate-core/src/test/java/org/hibernate/test/cache/CollectionCacheEvictionWithoutMappedByTest.java

@@ -0,0 +1,180 @@
+/*
+ * 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.test.cache;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.persistence.Cacheable;
+import javax.persistence.CascadeType;
+import javax.persistence.Entity;
+import javax.persistence.GeneratedValue;
+import javax.persistence.Id;
+import javax.persistence.OneToMany;
+import javax.persistence.Table;
+
+import org.hibernate.Session;
+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.junit4.BaseCoreFunctionalTestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+/**
+ * @author Janario Oliveira
+ */
+public class CollectionCacheEvictionWithoutMappedByTest extends BaseCoreFunctionalTestCase {
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class[] {Person.class, People.class};
+	}
+
+	@Override
+	protected void configure(Configuration cfg) {
+		cfg.setProperty( Environment.AUTO_EVICT_COLLECTION_CACHE, "true" );
+		cfg.setProperty( Environment.USE_SECOND_LEVEL_CACHE, "true" );
+		cfg.setProperty( Environment.USE_QUERY_CACHE, "true" );
+		cfg.setProperty( Environment.CACHE_PROVIDER_CONFIG, "true" );
+		cfg.setProperty( CollectionCacheInvalidator.PROPAGATE_EXCEPTION, "true" );
+	}
+
+	private People createPeople() {
+		Session session = openSession();
+		session.beginTransaction();
+		People people = new People();
+		people.people.add( new Person() );
+		people.people.add( new Person() );
+		session.persist( people );
+
+		session.getTransaction().commit();
+		session.close();
+		return people;
+	}
+
+	private People initCache(int id) {
+		Session session = openSession();
+		People people = session.get( People.class, id );
+		//should add in cache
+		assertEquals( 2, people.people.size() );
+		session.close();
+		return people;
+	}
+
+	@Test
+	public void testCollectionCacheEvictionInsert() {
+		People people = createPeople();
+		people = initCache( people.id );
+
+		Session session = openSession();
+		session.beginTransaction();
+
+		people = session.get( People.class, people.id );
+		Person person = new Person();
+		session.save( person );
+		people.people.add( person );
+
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+
+		people = session.get( People.class, people.id );
+		assertEquals( 3, people.people.size() );
+
+		session.close();
+	}
+
+	@Test
+	public void testCollectionCacheEvictionRemove() {
+		People people = createPeople();
+		people = initCache( people.id );
+
+		Session session = openSession();
+		session.beginTransaction();
+
+		people = session.get( People.class, people.id );
+		Person person = people.people.remove( 0 );
+		session.delete( person );
+
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+
+		people = session.get( People.class, people.id );
+		assertEquals( 1, people.people.size() );
+
+		session.close();
+	}
+
+	@Test
+	public void testCollectionCacheEvictionUpdate() {
+		People people1 = createPeople();
+		people1 = initCache( people1.id );
+		People people2 = createPeople();
+		people2 = initCache( people2.id );
+
+
+		Session session = openSession();
+		session.beginTransaction();
+
+		people1 = session.get( People.class, people1.id );
+		people2 = session.get( People.class, people2.id );
+
+		Person person1 = people1.people.remove( 0 );
+		Person person2 = people1.people.remove( 0 );
+		Person person3 = people2.people.remove( 0 );
+		session.flush();//avoid: Unique index or primary key violation
+		people1.people.add( person3 );
+		people2.people.add( person2 );
+		people2.people.add( person1 );
+
+		session.getTransaction().commit();
+		session.close();
+
+		session = openSession();
+
+		people1 = session.get( People.class, people1.id );
+		people2 = session.get( People.class, people2.id );
+		assertEquals( 1, people1.people.size() );
+		assertEquals( 3, people2.people.size() );
+
+		session.close();
+	}
+
+	@Entity
+	@Table(name = "people_group")
+	@Cacheable
+	@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
+	public static class People {
+		@Id
+		@GeneratedValue
+		private Integer id;
+
+		@OneToMany(cascade = CascadeType.ALL)
+		@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
+		private List<Person> people = new ArrayList<Person>();
+	}
+
+	@Entity
+	@Table(name = "person")
+	@Cacheable
+	@Cache(usage = CacheConcurrencyStrategy.READ_WRITE)
+	public static class Person {
+		@Id
+		@GeneratedValue
+		private Integer id;
+
+		protected Person() {
+		}
+	}
+}