From 25bf9b643a95438554a029da52b9b14723cf887e Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Thu, 4 Jan 2018 17:15:40 -0800 Subject: [PATCH] HHH-10418 : Fix inconsistency between SessionFactoryImpl and CacheImpl --- .../org/hibernate/internal/CacheImpl.java | 65 +++++++++++++++---- ...ntitiesAndCollectionsInSameRegionTest.java | 2 - 2 files changed, 54 insertions(+), 13 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java b/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java index f666ae2fa5..7cc22ec94a 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java @@ -8,6 +8,9 @@ package org.hibernate.internal; import org.hibernate.HibernateException; import org.hibernate.boot.spi.SessionFactoryOptions; +import org.hibernate.cache.spi.CollectionRegion; +import org.hibernate.cache.spi.EntityRegion; +import org.hibernate.cache.spi.NaturalIdRegion; import org.hibernate.cache.spi.QueryCache; import org.hibernate.cache.spi.Region; import org.hibernate.cache.spi.RegionFactory; @@ -39,7 +42,10 @@ public class CacheImpl implements CacheImplementor { private final transient RegionFactory regionFactory; private final transient UpdateTimestampsCache updateTimestampsCache; private final transient ConcurrentMap queryCaches; - private final transient ConcurrentMap allCacheRegions = new ConcurrentHashMap(); + private final transient ConcurrentMap entityRegionMap = new ConcurrentHashMap(); + private final transient ConcurrentMap collectionRegionMap = new ConcurrentHashMap(); + private final transient ConcurrentMap naturalIdRegionMap = new ConcurrentHashMap(); + private final transient ConcurrentMap otherRegionMap = new ConcurrentHashMap(); public CacheImpl(SessionFactoryImplementor sessionFactory) { this.sessionFactory = sessionFactory; @@ -56,8 +62,6 @@ public class CacheImpl implements CacheImplementor { queryCache = settings.getQueryCacheFactory() .getQueryCache( null, updateTimestampsCache, settings, sessionFactory.getProperties() ); queryCaches = new ConcurrentHashMap(); - allCacheRegions.put( updateTimestampsCache.getRegion().getName(), updateTimestampsCache.getRegion() ); - allCacheRegions.put( queryCache.getRegion().getName(), queryCache.getRegion() ); } else { updateTimestampsCache = null; @@ -227,7 +231,7 @@ public class CacheImpl implements CacheImplementor { } if ( sessionFactory.getSessionFactoryOptions().isQueryCacheEnabled() ) { QueryCache namedQueryCache = queryCaches.get( regionName ); - // TODO : cleanup entries in queryCaches + allCacheRegions ? + // TODO : cleanup entries in queryCaches ? if ( namedQueryCache != null ) { if ( LOG.isDebugEnabled() ) { LOG.debugf( "Evicting query cache, region: %s", regionName ); @@ -283,7 +287,7 @@ public class CacheImpl implements CacheImplementor { QueryCache currentQueryCache = queryCaches.get( regionName ); if ( currentQueryCache == null ) { - synchronized (allCacheRegions) { + synchronized (queryCaches) { currentQueryCache = queryCaches.get( regionName ); if ( currentQueryCache == null ) { currentQueryCache = settings.getQueryCacheFactory() @@ -294,7 +298,6 @@ public class CacheImpl implements CacheImplementor { sessionFactory.getProperties() ); queryCaches.put( regionName, currentQueryCache ); - allCacheRegions.put( currentQueryCache.getRegion().getName(), currentQueryCache.getRegion() ); } else { return currentQueryCache; @@ -306,7 +309,19 @@ public class CacheImpl implements CacheImplementor { @Override public void addCacheRegion(String name, Region region) { - allCacheRegions.put( name, region ); + if ( EntityRegion.class.isInstance( region ) ) { + entityRegionMap.put( name, (EntityRegion) region ); + } + else if ( CollectionRegion.class.isInstance( region ) ) { + collectionRegionMap.put( name, (CollectionRegion) region ); + } + else if ( NaturalIdRegion.class.isInstance( region ) ) { + naturalIdRegionMap.put( name, (NaturalIdRegion) region ); + } + else { + // in case an application uses this method for some other type of Region + otherRegionMap.put( name, region ); + } } @Override @@ -323,18 +338,46 @@ public class CacheImpl implements CacheImplementor { @Override public Region getSecondLevelCacheRegion(String regionName) { - return allCacheRegions.get( regionName ); + // If there is an EntityRegion and CollectionRegion with the same name, + // then the EntityRegion should be returned. + Region region = entityRegionMap.get(regionName); + if ( region != null ) { + return region; + } + region = collectionRegionMap.get( regionName ); + if ( region != null ) { + return region; + } + // Check for another type of Region of name regionName + return getNonEntityNonCollectionRegions().get( regionName ); } @Override public Region getNaturalIdCacheRegion(String regionName) { - return allCacheRegions.get( regionName ); + return naturalIdRegionMap.get( regionName ); } - @SuppressWarnings({"unchecked"}) @Override public Map getAllSecondLevelCacheRegions() { - return new HashMap( allCacheRegions ); + // Order is important! + // For example, if there is a CollectionRegion and an EntityRegion with the same name, then we + // want the EntityRegion to be in the Map that gets returned. + Map allCacheRegions = getNonEntityNonCollectionRegions(); + allCacheRegions.putAll( collectionRegionMap ); + allCacheRegions.putAll( entityRegionMap ); + return allCacheRegions; + } + + private Map getNonEntityNonCollectionRegions() { + final Map allCacheRegions = new HashMap(); + allCacheRegions.put( updateTimestampsCache.getRegion().getName(), updateTimestampsCache.getRegion() ); + allCacheRegions.put( queryCache.getRegion().getName(), queryCache.getRegion() ); + for ( QueryCache queryCacheValue : queryCaches.values() ) { + allCacheRegions.put( queryCacheValue.getRegion().getName(), queryCacheValue.getRegion() ); + } + allCacheRegions.putAll( otherRegionMap ); + allCacheRegions.putAll( naturalIdRegionMap ); + return allCacheRegions; } @Override diff --git a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/EntitiesAndCollectionsInSameRegionTest.java b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/EntitiesAndCollectionsInSameRegionTest.java index b067b51822..c6f87201de 100644 --- a/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/EntitiesAndCollectionsInSameRegionTest.java +++ b/hibernate-infinispan/src/test/java/org/hibernate/test/cache/infinispan/functional/EntitiesAndCollectionsInSameRegionTest.java @@ -19,8 +19,6 @@ import org.hibernate.Hibernate; import org.hibernate.annotations.Cache; import org.hibernate.annotations.CacheConcurrencyStrategy; import org.hibernate.cache.internal.DefaultCacheKeysFactory; -import org.hibernate.cache.spi.EntityRegion; -import org.hibernate.cache.spi.Region; import org.hibernate.cache.spi.access.EntityRegionAccessStrategy; import org.hibernate.cache.spi.access.RegionAccessStrategy; import org.hibernate.cfg.Environment;