HHH-8623 Cache should be up to date after region eviction

* Region clear now happens within the transaction of the caller,
if any. Otherwise, a new transaction is started in order to do the
clear within a transaction and so deal with situations where cache
statistics are queried outside of a transaction.
* Cache updates after the region eviction should be allowed to
happen, so if region eviction happened within the transaction, a
putFromLoad() is mapped to a normal put instead of a PFER call,
so that the data is accessible for the current transaction. This is
not an issue for situations where region has not evicted because
the session cache will have data that's been accessed in the
transaction.
* Transaction manager could be null, if region non-transactional

hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/access/TransactionalAccessDelegate.java

@@ -23,8 +23,6 @@
  */
 package org.hibernate.cache.infinispan.access;
 
-import javax.transaction.Transaction;
-
 import org.infinispan.AdvancedCache;
 import org.infinispan.util.logging.Log;
 import org.infinispan.util.logging.LogFactory;
@@ -138,7 +136,14 @@ public class TransactionalAccessDelegate {
 		}
 
 		try {
-			writeCache.putForExternalRead( key, value );
+			// Conditional put/putForExternalRead. If the region has been
+			// evicted in the current transaction, do a put instead of a
+			// putForExternalRead to make it stick, otherwise the current
+			// transaction won't see it.
+			if ( region.isRegionInvalidatedInCurrentTx() )
+				writeCache.put( key, value );
+			else
+				writeCache.putForExternalRead( key, value );
 		}
 		finally {
 			putValidator.releasePutFromLoadLock( key );
@@ -244,15 +249,10 @@ public class TransactionalAccessDelegate {
 		if ( !putValidator.invalidateRegion() ) {
 			throw new CacheException( "Failed to invalidate pending putFromLoad calls for region " + region.getName() );
 		}
-		final Transaction tx = region.suspend();
+
-		try {
+		// Invalidate the local region and then go remote
-			// Invalidate the local region and then go remote
+		region.invalidateRegion();
-			region.invalidateRegion();
+		Caches.broadcastEvictAll( cache );
-			Caches.broadcastEvictAll( cache );
-		}
-		finally {
-			region.resume( tx );
-		}
 	}
 
 }

hibernate-infinispan/src/main/java/org/hibernate/cache/infinispan/impl/BaseRegion.java

@@ -53,6 +53,7 @@ import org.hibernate.cache.spi.RegionFactory;
 public abstract class BaseRegion implements Region {
 
 	private static final Log log = LogFactory.getLog( BaseRegion.class );
+	private Transaction currentTransaction;
 
 	private enum InvalidateState {
 		INVALID, CLEARING, VALID
@@ -61,9 +62,12 @@ public abstract class BaseRegion implements Region {
 	private final String name;
 	private final AdvancedCache regionClearCache;
 	private final TransactionManager tm;
+
 	private final Object invalidationMutex = new Object();
 	private final AtomicReference<InvalidateState> invalidateState =
 			new AtomicReference<InvalidateState>( InvalidateState.VALID );
+	private volatile Transaction invalidateTransaction;
+
 	private final RegionFactory factory;
 
 	protected final AdvancedCache cache;
@@ -164,26 +168,29 @@ public abstract class BaseRegion implements Region {
 		boolean valid = isValid();
 		if ( !valid ) {
 			synchronized (invalidationMutex) {
-				if ( invalidateState.compareAndSet(
+				if ( invalidateState.compareAndSet( InvalidateState.INVALID, InvalidateState.CLEARING ) ) {
-						InvalidateState.INVALID, InvalidateState.CLEARING
-				) ) {
-					final Transaction tx = suspend();
 					try {
-						// Clear region in a separate transaction
+						// Even if no transactions are running, a new transaction
-						Caches.withinTx(
+						// needs to be started to do clear the region
-								cache, new Callable<Void>() {
+						// (without forcing autoCommit cache configuration).
-							@Override
+						Transaction tx = getCurrentTransaction();
-							public Void call() throws Exception {
+						if ( tx != null ) {
-								regionClearCache.clear();
+							regionClearCache.clear();
-								return null;
+						} else {
-							}
+							Caches.withinTx( cache, new Callable<Void>() {
+								@Override
+								public Void call() throws Exception {
+									regionClearCache.clear();
+									return null;
+								}
+							} );
 						}
-						);
+
 						invalidateState.compareAndSet(
 								InvalidateState.CLEARING, InvalidateState.VALID
 						);
 					}
-					catch (Exception e) {
+					catch ( Exception e ) {
 						if ( log.isTraceEnabled() ) {
 							log.trace(
 									"Could not invalidate region: "
@@ -191,9 +198,6 @@ public abstract class BaseRegion implements Region {
 							);
 						}
 					}
-					finally {
-						resume( tx );
-					}
 				}
 			}
 			valid = isValid();
@@ -244,11 +248,20 @@ public abstract class BaseRegion implements Region {
    /**
     * Invalidates the region.
     */
-	public void invalidateRegion() {
+   public void invalidateRegion() {
-		if ( log.isTraceEnabled() ) {
+		if (log.isTraceEnabled()) {
-			log.trace( "Invalidate region: " + name );
+			log.trace("Invalidate region: " + name);
+		}
+
+		Transaction tx = getCurrentTransaction();
+		if ( tx != null ) {
+			synchronized ( invalidationMutex ) {
+				invalidateTransaction = tx;
+				invalidateState.set( InvalidateState.INVALID );
+			}
+		} else {
+			invalidateState.set( InvalidateState.INVALID );
 		}
-		invalidateState.set( InvalidateState.INVALID );
 	}
 
 	public TransactionManager getTransactionManager() {
@@ -265,4 +278,18 @@ public abstract class BaseRegion implements Region {
 		return cache;
 	}
 
+	public boolean isRegionInvalidatedInCurrentTx() {
+		Transaction tx = getCurrentTransaction();
+		return tx != null && tx.equals(invalidateTransaction);
+	}
+
+	private Transaction getCurrentTransaction() {
+		try {
+			// Transaction manager could be null
+			return tm != null ? tm.getTransaction() : null;
+		} catch (SystemException e) {
+			throw new CacheException("Unable to get current transaction", e);
+		}
+	}
+
 }

hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/AbstractFunctionalTestCase.java

@@ -72,4 +72,48 @@ public abstract class AbstractFunctionalTestCase extends SingleNodeTestCase {
       });
    }
 
+   @Test
+   public void testInsertClearCacheDeleteEntity() throws Exception {
+      final Statistics stats = sessionFactory().getStatistics();
+      stats.clear();
+
+      final Item item = new Item( "chris", "Chris's Item" );
+      withTx(tm, new Callable<Void>() {
+         @Override
+         public Void call() throws Exception {
+            Session s = openSession();
+            s.getTransaction().begin();
+            s.persist(item);
+            s.getTransaction().commit();
+            assertEquals(0, stats.getSecondLevelCacheMissCount());
+            assertEquals(0, stats.getSecondLevelCacheHitCount());
+            assertEquals(1, stats.getSecondLevelCachePutCount());
+            s.close();
+            return null;
+         }
+      });
+
+      log.info("Entry persisted, let's load and delete it.");
+
+      cleanupCache();
+
+      withTx(tm, new Callable<Void>() {
+         @Override
+         public Void call() throws Exception {
+            Session s = openSession();
+            s.getTransaction().begin();
+            Item found = (Item) s.load(Item.class, item.getId());
+            log.info(stats.toString());
+            assertEquals(item.getDescription(), found.getDescription());
+            assertEquals(1, stats.getSecondLevelCacheMissCount());
+            assertEquals(0, stats.getSecondLevelCacheHitCount());
+            assertEquals(2, stats.getSecondLevelCachePutCount());
+            s.delete(found);
+            s.getTransaction().commit();
+            s.close();
+            return null;
+         }
+      });
+   }
+
 }

hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/BasicTransactionalTestCase.java

@@ -24,11 +24,13 @@
 package org.hibernate.test.cache.infinispan.functional;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.Callable;
 import java.util.concurrent.TimeUnit;
 
+import org.hibernate.Cache;
 import org.hibernate.Criteria;
 import org.hibernate.NaturalIdLoadAccess;
 import org.hibernate.cache.infinispan.access.PutFromLoadValidator;
@@ -43,11 +45,10 @@ import org.hibernate.cfg.Configuration;
 import org.hibernate.stat.SecondLevelCacheStatistics;
 import org.hibernate.stat.Statistics;
 
+import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertNotNull;
 import static org.infinispan.test.TestingUtil.withTx;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
 /**
  * Functional entity transactional tests.
@@ -418,6 +419,9 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase {
    public void testNaturalIdCached() throws Exception {
       saveSomeCitizens();
 
+      // Clear the cache before the transaction begins
+      BasicTransactionalTestCase.this.cleanupCache();
+
       withTx(tm, new Callable<Void>() {
          @Override
          public Void call() throws Exception {
@@ -428,8 +432,6 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase {
             criteria.add( Restrictions.naturalId().set( "ssn", "1234" ).set( "state", france ) );
             criteria.setCacheable( true );
 
-            BasicTransactionalTestCase.this.cleanupCache();
-
             Statistics stats = sessionFactory().getStatistics();
             stats.setStatisticsEnabled( true );
             stats.clear();
@@ -566,7 +568,52 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase {
 
    }
 
-   private void saveSomeCitizens() throws Exception {
+   @Test
+   public void testEntityCacheContentsAfterEvictAll() throws Exception {
+      final List<Citizen> citizens = saveSomeCitizens();
+
+      withTx(tm, new Callable<Void>() {
+         @Override
+         public Void call() throws Exception {
+            Session s = openSession();
+            Transaction tx = s.beginTransaction();
+            Cache cache = s.getSessionFactory().getCache();
+
+            Statistics stats = sessionFactory().getStatistics();
+            SecondLevelCacheStatistics slcStats = stats.getSecondLevelCacheStatistics(Citizen.class.getName());
+
+            assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(0).getId(),
+                  cache.containsEntity(Citizen.class, citizens.get(0).getId()));
+            assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(1).getId(),
+                  cache.containsEntity(Citizen.class, citizens.get(1).getId()));
+            assertEquals(2, slcStats.getPutCount());
+
+            cache.evictEntityRegions();
+
+            assertEquals(0, slcStats.getElementCountInMemory());
+            assertFalse("2lc entity cache is expected to not contain Citizen id = " + citizens.get(0).getId(),
+                  cache.containsEntity(Citizen.class, citizens.get(0).getId()));
+            assertFalse("2lc entity cache is expected to not contain Citizen id = " + citizens.get(1).getId(),
+                  cache.containsEntity(Citizen.class, citizens.get(1).getId()));
+
+            Citizen citizen = (Citizen) s.load(Citizen.class, citizens.get(0).getId());
+            assertNotNull(citizen);
+            assertNotNull(citizen.getFirstname()); // proxy gets resolved
+            assertEquals(1, slcStats.getMissCount());
+            assertEquals(3, slcStats.getPutCount());
+            assertEquals(1, slcStats.getElementCountInMemory());
+            assertTrue("2lc entity cache is expected to contain Citizen id = " + citizens.get(0).getId(),
+                  cache.containsEntity(Citizen.class, citizens.get(0).getId()));
+
+            // cleanup
+            tx.rollback();
+            s.close();
+            return null;
+         }
+      });
+   }
+
+   private List<Citizen> saveSomeCitizens() throws Exception {
       final Citizen c1 = new Citizen();
       c1.setFirstname( "Emmanuel" );
       c1.setLastname( "Bernard" );
@@ -598,6 +645,11 @@ public class BasicTransactionalTestCase extends AbstractFunctionalTestCase {
             return null;
          }
       });
+
+      List<Citizen> citizens = new ArrayList<Citizen>(2);
+      citizens.add(c1);
+      citizens.add(c2);
+      return citizens;
    }
 
    private State getState(Session s, String name) {