From 62d8fe1c298d75fae90508383995fd43d3a6fa1c Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Mon, 8 Jan 2018 14:06:01 -0800 Subject: [PATCH] HHH-10418 : Fix inconsistency between SessionFactoryImpl and CacheImpl --- .../org/hibernate/internal/CacheImpl.java | 73 +++++++++++++++---- 1 file changed, 57 insertions(+), 16 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 7cc22ec94a..a460b6e128 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/CacheImpl.java @@ -309,16 +309,25 @@ public class CacheImpl implements CacheImplementor { @Override public void addCacheRegion(String name, Region region) { + // Add the Region to all applicable maps + boolean isOtherRegion = true; if ( EntityRegion.class.isInstance( region ) ) { entityRegionMap.put( name, (EntityRegion) region ); + isOtherRegion = false; } - else if ( CollectionRegion.class.isInstance( region ) ) { + if ( CollectionRegion.class.isInstance( region ) ) { collectionRegionMap.put( name, (CollectionRegion) region ); + isOtherRegion = false; } - else if ( NaturalIdRegion.class.isInstance( region ) ) { + if ( NaturalIdRegion.class.isInstance( region ) ) { naturalIdRegionMap.put( name, (NaturalIdRegion) region ); + isOtherRegion = false; } - else { + // If the Region is not an EntityRegion, CollectionRegion, or NaturalIdRegion, + // then add it to otherRegionMap; the only other type of Region that Hibernate knows about + // is updateTimestampsCache, queryCache, and queryCaches; those regions should not + // be added using this method. + if ( isOtherRegion ) { // in case an application uses this method for some other type of Region otherRegionMap.put( name, region ); } @@ -338,7 +347,12 @@ public class CacheImpl implements CacheImplementor { @Override public Region getSecondLevelCacheRegion(String regionName) { - // If there is an EntityRegion and CollectionRegion with the same name, + + // Order is important! + // To keep results of #getSecondLevelCacheRegion and #getAllSecondLevelCacheRegions consistent, + // the order of lookups in getSecondLevelCacheRegion needs to be the opposite of the order in + // which Map contents are added to getAllSecondLevelCacheRegions. + // In addition, 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 ) { @@ -348,8 +362,30 @@ public class CacheImpl implements CacheImplementor { if ( region != null ) { return region; } - // Check for another type of Region of name regionName - return getNonEntityNonCollectionRegions().get( regionName ); + region = naturalIdRegionMap.get( regionName ); + if ( region != null ) { + return region; + } + region = otherRegionMap.get( regionName ); + if ( region != null ) { + return region; + } + // keys in queryCaches may not be equal to the Region names + // obtained from queryCaches values; we need to be sure we are comparing + // the actual QueryCache region names with regionName. + for ( QueryCache queryCacheValue : queryCaches.values() ) { + if( queryCacheValue.getRegion().getName().equals( regionName ) ) { + return queryCacheValue.getRegion(); + } + } + if ( queryCache.getRegion().getName().equals( regionName ) ) { + return queryCache.getRegion(); + } + if ( updateTimestampsCache.getRegion().getName().equals( regionName ) ) { + return updateTimestampsCache.getRegion(); + } + // no Region with the specified name + return null; } @Override @@ -359,24 +395,29 @@ public class CacheImpl implements CacheImplementor { @Override public Map getAllSecondLevelCacheRegions() { - // 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(); + // Order is important! + // To keep results of #getSecondLevelCacheRegion and #getAllSecondLevelCacheRegions consistent, + // the order of lookups in getSecondLevelCacheRegion needs to be the opposite of the order in + // which Map contents are added to getAllSecondLevelCacheRegions. + // In addition, 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. + final Map allCacheRegions = new HashMap( + 2 + queryCaches.size() + otherRegionMap.size() + naturalIdRegionMap.size() + + collectionRegionMap.size() + entityRegionMap.size() + ); allCacheRegions.put( updateTimestampsCache.getRegion().getName(), updateTimestampsCache.getRegion() ); allCacheRegions.put( queryCache.getRegion().getName(), queryCache.getRegion() ); + // keys in queryCaches may not be equal to the Region names + // obtained from queryCaches values; we need to be sure we are adding + // the actual QueryCache region name as the key in allCacheRegions. for ( QueryCache queryCacheValue : queryCaches.values() ) { allCacheRegions.put( queryCacheValue.getRegion().getName(), queryCacheValue.getRegion() ); } allCacheRegions.putAll( otherRegionMap ); allCacheRegions.putAll( naturalIdRegionMap ); + allCacheRegions.putAll( collectionRegionMap ); + allCacheRegions.putAll( entityRegionMap ); return allCacheRegions; }