From 9d958291d8247e5694493ae8e69ad04534365a9d Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Wed, 28 Mar 2018 11:09:04 -0500 Subject: [PATCH] HHH-11356 - Adjust the 2nd-Cache SPIs to better reflect supported uses - Fix-ups from Radim's review - Better Javadoc --- .../internal/BulkOperationCleanupAction.java | 40 +++++--- .../cache/spi/DirectAccessRegion.java | 13 ++- .../spi/access/CachedDomainDataAccess.java | 95 ++++++++++++------- .../AbstractCachedDomainDataAccess.java | 4 +- .../support/DirectAccessRegionTemplate.java | 10 -- .../cache/spi/support/StorageAccess.java | 18 ++-- 6 files changed, 107 insertions(+), 73 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/action/internal/BulkOperationCleanupAction.java b/hibernate-core/src/main/java/org/hibernate/action/internal/BulkOperationCleanupAction.java index 7834c9c492..97745d8768 100644 --- a/hibernate-core/src/main/java/org/hibernate/action/internal/BulkOperationCleanupAction.java +++ b/hibernate-core/src/main/java/org/hibernate/action/internal/BulkOperationCleanupAction.java @@ -62,15 +62,16 @@ public class BulkOperationCleanupAction implements Executable, Serializable { spacesList.addAll( Arrays.asList( (String[]) persister.getQuerySpaces() ) ); if ( persister.canWriteToCache() ) { - final EntityDataAccess entityDataAccess = factory.getCache() - .getEntityRegionAccess( persister.getNavigableRole() ); + final EntityDataAccess entityDataAccess = persister.getCacheAccessStrategy(); if ( entityDataAccess != null ) { - entityCleanups.add( new EntityCleanup( entityDataAccess ) ); + entityCleanups.add( new EntityCleanup( entityDataAccess, session ) ); } } if ( persister.hasNaturalIdentifier() && persister.hasNaturalIdCache() ) { - naturalIdCleanups.add( new NaturalIdCleanup( factory.getCache().getNaturalIdCacheRegionAccessStrategy( persister.getNavigableRole() ) ) ); + naturalIdCleanups.add( + new NaturalIdCleanup( persister.getNaturalIdCacheAccessStrategy(), session ) + ); } final Set roles = factory.getMetamodel().getCollectionRolesByEntityParticipant( persister.getEntityName() ); @@ -78,7 +79,12 @@ public class BulkOperationCleanupAction implements Executable, Serializable { for ( String role : roles ) { final CollectionPersister collectionPersister = factory.getMetamodel().collectionPersister( role ); if ( collectionPersister.hasCache() ) { - collectionCleanups.add( new CollectionCleanup( factory.getCache().getCollectionRegionAccess( collectionPersister.getNavigableRole() ) ) ); + collectionCleanups.add( + new CollectionCleanup( + collectionPersister.getCacheAccessStrategy(), + session + ) + ); } } } @@ -111,10 +117,10 @@ public class BulkOperationCleanupAction implements Executable, Serializable { spacesList.addAll( Arrays.asList( entitySpaces ) ); if ( persister.canWriteToCache() ) { - entityCleanups.add( new EntityCleanup( persister.getCacheAccessStrategy() ) ); + entityCleanups.add( new EntityCleanup( persister.getCacheAccessStrategy(), session ) ); } if ( persister.hasNaturalIdentifier() && persister.hasNaturalIdCache() ) { - naturalIdCleanups.add( new NaturalIdCleanup( persister.getNaturalIdCacheAccessStrategy() ) ); + naturalIdCleanups.add( new NaturalIdCleanup( persister.getNaturalIdCacheAccessStrategy(), session ) ); } final Set roles = session.getFactory().getMetamodel().getCollectionRolesByEntityParticipant( persister.getEntityName() ); @@ -123,7 +129,7 @@ public class BulkOperationCleanupAction implements Executable, Serializable { final CollectionPersister collectionPersister = factory.getMetamodel().collectionPersister( role ); if ( collectionPersister.hasCache() ) { collectionCleanups.add( - new CollectionCleanup( collectionPersister.getCacheAccessStrategy() ) + new CollectionCleanup( collectionPersister.getCacheAccessStrategy(), session ) ); } } @@ -208,10 +214,12 @@ public class BulkOperationCleanupAction implements Executable, Serializable { private final EntityDataAccess cacheAccess; private final SoftLock cacheLock; - private EntityCleanup(EntityDataAccess cacheAccess) { + private EntityCleanup( + EntityDataAccess cacheAccess, + SharedSessionContractImplementor session) { this.cacheAccess = cacheAccess; this.cacheLock = cacheAccess.lockRegion(); - cacheAccess.removeAll(); + cacheAccess.removeAll( session ); } private void release() { @@ -223,10 +231,12 @@ public class BulkOperationCleanupAction implements Executable, Serializable { private final CollectionDataAccess cacheAccess; private final SoftLock cacheLock; - private CollectionCleanup(CollectionDataAccess cacheAccess) { + private CollectionCleanup( + CollectionDataAccess cacheAccess, + SharedSessionContractImplementor session) { this.cacheAccess = cacheAccess; this.cacheLock = cacheAccess.lockRegion(); - cacheAccess.removeAll(); + cacheAccess.removeAll( session ); } private void release() { @@ -238,10 +248,12 @@ public class BulkOperationCleanupAction implements Executable, Serializable { private final NaturalIdDataAccess naturalIdCacheAccessStrategy; private final SoftLock cacheLock; - public NaturalIdCleanup(NaturalIdDataAccess naturalIdCacheAccessStrategy) { + public NaturalIdCleanup( + NaturalIdDataAccess naturalIdCacheAccessStrategy, + SharedSessionContractImplementor session) { this.naturalIdCacheAccessStrategy = naturalIdCacheAccessStrategy; this.cacheLock = naturalIdCacheAccessStrategy.lockRegion(); - naturalIdCacheAccessStrategy.removeAll(); + naturalIdCacheAccessStrategy.removeAll( session ); } private void release() { diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/DirectAccessRegion.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/DirectAccessRegion.java index 8511411f02..53bd3725d7 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/DirectAccessRegion.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/DirectAccessRegion.java @@ -12,14 +12,19 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; * Specialized Region whose data is accessed directly (not requiring * key/item wrapping, e.g. * + * Does not define a "remove" operation because Hibernate's query and timestamps + * caches only ever "get" and "put" + * * @author Steve Ebersole */ public interface DirectAccessRegion extends Region { - boolean contains(Object key); - + /** + * Get value by key + */ Object getFromCache(Object key, SharedSessionContractImplementor session); + /** + * Put a value by key + */ void putIntoCache(Object key, Object value, SharedSessionContractImplementor session); - - void removeFromCache(Object key, SharedSessionContractImplementor session); } diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/access/CachedDomainDataAccess.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/access/CachedDomainDataAccess.java index 2ef4c68fee..01c1e0c615 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/access/CachedDomainDataAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/access/CachedDomainDataAccess.java @@ -6,7 +6,6 @@ */ package org.hibernate.cache.spi.access; - import java.io.Serializable; import javax.persistence.Cache; @@ -18,27 +17,33 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; * Base contract for accessing the underlying cached data for a particular * Navigable of the user's domain model in a transactionally ACID manner. * + * @apiNote Note that the following methods are not considered "transactional" + * in this sense : {@link #contains}, {@link #lockRegion}, {@link #unlockRegion}, + * {@link #evict}, {@link #evictAll}. The semantics of these methods come + * from JPA's {@link Cache} contract. + * + * @implSpec The "non transactional" methods noted in the `@apiNote` should + * be implemented to ignore any locking. In other words, if {@link #evict} + * is called that item should be forcibly removed from the cache regardless of + * whether anything has locked it. + * * @author Steve Ebersole * @author Gail Badner */ public interface CachedDomainDataAccess { + /** + * The region containing the data being accessed + */ DomainDataRegion getRegion(); + /** + * The type of access implemented + */ AccessType getAccessType(); - /** - * Determine whether this region contains data for the given key. - *

- * The semantic here is whether the cache contains data visible for the - * current call context. This should be viewed as a "best effort", meaning - * blocking should be avoid if possible. - * - * @param key The cache key - * - * @return True if the underlying cache contains corresponding data; false - * otherwise. - */ - boolean contains(Object key); + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // Transactional /** * Attempt to retrieve an object from the cache. Mainly used in attempting @@ -110,15 +115,6 @@ public interface CachedDomainDataAccess { */ SoftLock lockItem(SharedSessionContractImplementor session, Object key, Object version); - /** - * Lock the entire region - * - * @return A representation of our lock on the item; or {@code null}. - * - * @throws CacheException Propagated from underlying cache provider - */ - SoftLock lockRegion(); - /** * Called when we have finished the attempted update/delete (which may or * may not have been successful), after transaction completion. This method @@ -132,16 +128,6 @@ public interface CachedDomainDataAccess { */ void unlockItem(SharedSessionContractImplementor session, Object key, SoftLock lock); - /** - * Called after we have finished the attempted invalidation of the entire - * region - * - * @param lock The lock previously obtained from {@link #lockRegion} - * - * @throws CacheException Propagated from underlying cache provider - */ - void unlockRegion(SoftLock lock); - /** * Called afterQuery an item has become stale (beforeQuery the transaction completes). * This method is used by "synchronous" concurrency strategies. @@ -154,11 +140,50 @@ public interface CachedDomainDataAccess { void remove(SharedSessionContractImplementor session, Object key); /** - * Called to evict data from the entire region + * Remove all data for this accessed type + * + * @throws CacheException Propagated from underlying cache provider + * @param session + */ + void removeAll(SharedSessionContractImplementor session); + + + + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + // Non-transactional + + /** + * Determine whether this region contains data for the given key. + *

+ * The semantic here is whether the cache contains data visible for the + * current call context. This should be viewed as a "best effort", meaning + * blocking should be avoid if possible. + * + * @param key The cache key + * + * @return True if the underlying cache contains corresponding data; false + * otherwise. + */ + boolean contains(Object key); + + /** + * Lock the entire region + * + * @return A representation of our lock on the item; or {@code null}. * * @throws CacheException Propagated from underlying cache provider */ - void removeAll(); + SoftLock lockRegion(); + + /** + * Called after we have finished the attempted invalidation of the entire + * region + * + * @param lock The lock previously obtained from {@link #lockRegion} + * + * @throws CacheException Propagated from underlying cache provider + */ + void unlockRegion(SoftLock lock); /** * Forcibly evict an item from the cache immediately without regard for transaction diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java index f63bb74eb4..7bcd27b280 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/AbstractCachedDomainDataAccess.java @@ -93,8 +93,8 @@ public abstract class AbstractCachedDomainDataAccess implements CachedDomainData } @Override - public void removeAll() { - clearCache(); + public void removeAll(SharedSessionContractImplementor session) { + getStorageAccess().clearCache( session ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/DirectAccessRegionTemplate.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/DirectAccessRegionTemplate.java index a0c9c201b5..564112e9e7 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/DirectAccessRegionTemplate.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/DirectAccessRegionTemplate.java @@ -27,11 +27,6 @@ public abstract class DirectAccessRegionTemplate extends AbstractRegion implemen return storageAccess; } - @Override - public boolean contains(Object key) { - return getStorageAccess().contains( key ); - } - @Override public Object getFromCache(Object key, SharedSessionContractImplementor session) { return getStorageAccess().getFromCache( key, session ); @@ -42,11 +37,6 @@ public abstract class DirectAccessRegionTemplate extends AbstractRegion implemen getStorageAccess().putIntoCache( key, value, session ); } - @Override - public void removeFromCache(Object key, SharedSessionContractImplementor session) { - getStorageAccess().removeFromCache( key, session ); - } - @Override public void clear() { getStorageAccess().evictData(); diff --git a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/StorageAccess.java b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/StorageAccess.java index 3b6d991411..f5c5ba1dfd 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/spi/support/StorageAccess.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/spi/support/StorageAccess.java @@ -10,16 +10,14 @@ import org.hibernate.engine.spi.SharedSessionContractImplementor; /** * A general read/write abstraction over the specific "cache" - * object from the caching provider + * object from the caching provider. + * + * @apiNote Similar to {@link org.hibernate.cache.spi.access.CachedDomainDataAccess}, + * some methods represent "transactional" (access to Session) and some are non-"transactional" * * @author Steve Ebersole */ public interface StorageAccess { - /** - * Does the cache contain this key? - */ - boolean contains(Object key); - /** * Get an item from the cache. */ @@ -44,6 +42,11 @@ public interface StorageAccess { evictData(); } + /** + * Does the cache contain this key? + */ + boolean contains(Object key); + /** * Clear all data regardless of transaction/locking */ @@ -54,9 +57,8 @@ public interface StorageAccess { */ void evictData(Object key); - /*** + /** * Release any resources. Called during cache shutdown */ void release(); - }