Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import javax.cache.CacheManager;
import javax.cache.Caching;
import javax.cache.spi.CachingProvider;

/**
* @author Steve Ebersole
Expand All @@ -19,6 +20,13 @@ public class JCacheHelper {
public static CacheManager locateStandardCacheManager() {
// unless configured differently, this is how JCacheRegionFactory
// will locate the CacheManager to use
return Caching.getCachingProvider().getCacheManager();

CachingProvider cachingProvider = Caching.getCachingProvider();

// JRegionFactory::prepareForUse retrieves the class loader service from
// the service registry and registers it as the
// Since EHCache by itself doesn't use this class loader by itself, it needs to be injected here.
// It is set via JCacheRegionFactory::prepareForUse. S.a. https://github.com/ehcache/ehcache3/issues/3252
return cachingProvider.getCacheManager( cachingProvider.getDefaultURI(), Caching.getDefaultClassLoader() );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.hibernate.cache.cfg.spi.DomainDataRegionConfig;
import org.hibernate.cache.internal.DefaultCacheKeysFactory;
import org.hibernate.cache.jcache.ConfigSettings;
import org.hibernate.cache.jcache.JCacheHelper;
import org.hibernate.cache.jcache.MissingCacheStrategy;
import org.hibernate.cache.spi.CacheKeysFactory;
import org.hibernate.cache.spi.DomainDataRegion;
Expand All @@ -43,6 +44,12 @@ public class JCacheRegionFactory extends RegionFactoryTemplate {
private volatile CacheManager cacheManager;
private volatile MissingCacheStrategy missingCacheStrategy;

// need to save the classloader from Caching.getDefaultClassLoader to reset it
// when this service is released again.
private volatile ClassLoader oldJsr107CacheClassLoader;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really necessary to keep around the old class loader? Can't we just swap the class loader when retrieving/looking-up the CacheManager?

// in case caches were already set up beforehand with a different configuration
private volatile CacheManager preInitializedCacheManager;
Comment on lines +50 to +51
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// in case caches were already set up beforehand with a different configuration
private volatile CacheManager preInitializedCacheManager;


@SuppressWarnings("unused")
public JCacheRegionFactory() {
this( DefaultCacheKeysFactory.INSTANCE );
Expand Down Expand Up @@ -85,7 +92,8 @@ protected DomainDataStorageAccess createDomainDataStorageAccess(

protected Cache<Object, Object> getOrCreateCache(String unqualifiedRegionName, SessionFactoryImplementor sessionFactory) {
verifyStarted();
assert !RegionNameQualifier.INSTANCE.isQualified( unqualifiedRegionName, sessionFactory.getSessionFactoryOptions() );
assert !RegionNameQualifier.INSTANCE.isQualified( unqualifiedRegionName,
sessionFactory.getSessionFactoryOptions() );

final String qualifiedRegionName = RegionNameQualifier.INSTANCE.qualify(
unqualifiedRegionName,
Expand All @@ -94,6 +102,13 @@ protected Cache<Object, Object> getOrCreateCache(String unqualifiedRegionName, S

final Cache<Object, Object> cache = cacheManager.getCache( qualifiedRegionName );
if ( cache == null ) {
if ( preInitializedCacheManager != null ) {
final Cache<Object, Object> cacheFromBefore = preInitializedCacheManager.getCache(
qualifiedRegionName );
if ( cacheFromBefore != null ) {
return cacheFromBefore;
}
}
Comment on lines +105 to +111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ( preInitializedCacheManager != null ) {
final Cache<Object, Object> cacheFromBefore = preInitializedCacheManager.getCache(
qualifiedRegionName );
if ( cacheFromBefore != null ) {
return cacheFromBefore;
}
}

return createCache( qualifiedRegionName );
}
return cache;
Expand All @@ -110,7 +125,8 @@ protected Cache<Object, Object> createCache(String regionName) {
case CREATE:
return cacheManager.createCache( regionName, new MutableConfiguration<>() );
case FAIL:
throw new CacheException( "On-the-fly creation of JCache Cache objects is not supported [" + regionName + "]" );
throw new CacheException(
"On-the-fly creation of JCache Cache objects is not supported [" + regionName + "]" );
default:
throw new IllegalStateException( "Unsupported missing cache strategy: " + missingCacheStrategy );
}
Expand All @@ -121,7 +137,9 @@ protected boolean cacheExists(String unqualifiedRegionName, SessionFactoryImplem
unqualifiedRegionName,
sessionFactory.getSessionFactoryOptions()
);
return cacheManager.getCache( qualifiedRegionName ) != null;
return cacheManager.getCache( qualifiedRegionName ) != null ||
(preInitializedCacheManager != null &&
preInitializedCacheManager.getCache( qualifiedRegionName ) != null);
Comment on lines +140 to +142
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return cacheManager.getCache( qualifiedRegionName ) != null ||
(preInitializedCacheManager != null &&
preInitializedCacheManager.getCache( qualifiedRegionName ) != null);
return cacheManager.getCache( qualifiedRegionName ) != null;

}

@Override
Expand Down Expand Up @@ -155,9 +173,9 @@ protected StorageAccess createTimestampsRegionStorageAccess(
}

protected final String defaultRegionName(String regionName, SessionFactoryImplementor sessionFactory,
String defaultRegionName, List<String> legacyDefaultRegionNames) {
String defaultRegionName, List<String> legacyDefaultRegionNames) {
if ( defaultRegionName.equals( regionName )
&& !cacheExists( regionName, sessionFactory ) ) {
&& !cacheExists( regionName, sessionFactory ) ) {
// Maybe the user configured caches explicitly with legacy names; try them and use the first that exists

for ( String legacyDefaultRegionName : legacyDefaultRegionNames ) {
Expand All @@ -172,7 +190,6 @@ protected final String defaultRegionName(String regionName, SessionFactoryImplem
}



// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// Lifecycle

Expand All @@ -182,7 +199,14 @@ protected boolean isStarted() {
}

@Override
protected void prepareForUse(SessionFactoryOptions settings, Map<String,Object> configValues) {
protected void prepareForUse(SessionFactoryOptions settings, Map<String, Object> configValues) {
final ClassLoader serviceClassLoader = getServiceClassLoader( settings );
if ( serviceClassLoader != null ) {
preInitializedCacheManager = JCacheHelper.locateStandardCacheManager();
oldJsr107CacheClassLoader = Caching.getDefaultClassLoader();
Caching.setDefaultClassLoader( serviceClassLoader );
}
Comment on lines +203 to +208
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, the JCacheHelper class should rather be deprecated and/or moved to the test sources than being used explicitly here.

Suggested change
final ClassLoader serviceClassLoader = getServiceClassLoader( settings );
if ( serviceClassLoader != null ) {
preInitializedCacheManager = JCacheHelper.locateStandardCacheManager();
oldJsr107CacheClassLoader = Caching.getDefaultClassLoader();
Caching.setDefaultClassLoader( serviceClassLoader );
}
final ClassLoader serviceClassLoader = getServiceClassLoader( settings );
oldJsr107CacheClassLoader = Caching.getDefaultClassLoader();
Caching.setDefaultClassLoader( serviceClassLoader );


this.cacheManager = resolveCacheManager( settings, configValues );
if ( this.cacheManager == null ) {
throw new CacheException( "Could not locate/create CacheManager" );
Expand All @@ -192,30 +216,38 @@ protected void prepareForUse(SessionFactoryOptions settings, Map<String,Object>
);
}

protected CacheManager resolveCacheManager(SessionFactoryOptions settings, Map<String,Object> properties) {
protected CacheManager resolveCacheManager(SessionFactoryOptions settings, Map<String, Object> properties) {
final Object explicitCacheManager = properties.get( ConfigSettings.CACHE_MANAGER );
if ( explicitCacheManager != null ) {
return useExplicitCacheManager( settings, explicitCacheManager );
}

final CachingProvider cachingProvider = getCachingProvider( properties );
final ClassLoader serviceClassLoader = getServiceClassLoader( settings );
final CachingProvider cachingProvider = getCachingProvider( properties, serviceClassLoader );
final CacheManager cacheManager;
final URI cacheManagerUri = getUri( settings, properties );
final ClassLoader classLoader = getClassLoader( cachingProvider, serviceClassLoader );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final ClassLoader classLoader = getClassLoader( cachingProvider, serviceClassLoader );

if ( cacheManagerUri != null ) {
cacheManager = cachingProvider.getCacheManager( cacheManagerUri, getClassLoader( cachingProvider ) );
cacheManager = cachingProvider.getCacheManager( cacheManagerUri, classLoader );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cacheManager = cachingProvider.getCacheManager( cacheManagerUri, classLoader );
cacheManager = cachingProvider.getCacheManager( cacheManagerUri, serviceClassLoader );

}
else {
cacheManager = cachingProvider.getCacheManager( cachingProvider.getDefaultURI(), getClassLoader( cachingProvider ) );
cacheManager = cachingProvider.getCacheManager( cachingProvider.getDefaultURI(), classLoader );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
cacheManager = cachingProvider.getCacheManager( cachingProvider.getDefaultURI(), classLoader );
cacheManager = cachingProvider.getCacheManager( cachingProvider.getDefaultURI(), serviceClassLoader );

}
return cacheManager;
}

protected ClassLoader getClassLoader(CachingProvider cachingProvider) {
// todo (5.3) : shouldn't this use Hibernate's AggregatedClassLoader?
return cachingProvider.getDefaultClassLoader();
private ClassLoader getServiceClassLoader(SessionFactoryOptions settings) {
final ClassLoaderService classLoaderService = settings.getServiceRegistry()
.getService( ClassLoaderService.class );
return (classLoaderService == null) ? null :
classLoaderService.workWithClassLoader( classLoader -> classLoader );
Comment on lines +240 to +243
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final ClassLoaderService classLoaderService = settings.getServiceRegistry()
.getService( ClassLoaderService.class );
return (classLoaderService == null) ? null :
classLoaderService.workWithClassLoader( classLoader -> classLoader );
return settings.getServiceRegistry()
.requireService( ClassLoaderService.class )
.workWithClassLoader( classLoader -> classLoader );

}

protected ClassLoader getClassLoader(CachingProvider cachingProvider, ClassLoader serviceClassLoader) {
return (serviceClassLoader != null) ? serviceClassLoader : cachingProvider.getDefaultClassLoader();
Comment on lines +244 to +247
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
protected ClassLoader getClassLoader(CachingProvider cachingProvider, ClassLoader serviceClassLoader) {
return (serviceClassLoader != null) ? serviceClassLoader : cachingProvider.getDefaultClassLoader();

}

protected URI getUri(SessionFactoryOptions settings, Map<String,Object> properties) {
protected URI getUri(SessionFactoryOptions settings, Map<String, Object> properties) {
String cacheManagerUri = getProp( properties, ConfigSettings.CONFIG_URI );
if ( cacheManagerUri == null ) {
return null;
Expand All @@ -237,18 +269,18 @@ protected URI getUri(SessionFactoryOptions settings, Map<String,Object> properti
}
}

private String getProp(Map<String,Object> properties, String prop) {
private String getProp(Map<String, Object> properties, String prop) {
return properties != null ? (String) properties.get( prop ) : null;
}

protected CachingProvider getCachingProvider(final Map<String,Object> properties){
protected CachingProvider getCachingProvider(final Map<String, Object> properties, ClassLoader classLoader) {
final CachingProvider cachingProvider;
final String provider = getProp( properties, ConfigSettings.PROVIDER );
if ( provider != null ) {
cachingProvider = Caching.getCachingProvider( provider );
cachingProvider = Caching.getCachingProvider( provider, classLoader );
}
else {
cachingProvider = Caching.getCachingProvider();
cachingProvider = Caching.getCachingProvider( classLoader );
}
return cachingProvider;
}
Expand Down Expand Up @@ -283,6 +315,9 @@ protected void releaseFromUse() {
// - when the explicit `setting` passed to `#useExplicitCacheManager` is
// a CacheManager instance
cacheManager.close();
if ( oldJsr107CacheClassLoader != null ) {
Caching.setDefaultClassLoader( oldJsr107CacheClassLoader );
}
}
finally {
cacheManager = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
import org.hibernate.boot.registry.StandardServiceRegistryBuilder;
import org.hibernate.cache.CacheException;
import org.hibernate.cache.jcache.ConfigSettings;
import org.hibernate.cache.jcache.JCacheHelper;
import org.hibernate.cache.spi.SecondLevelCacheLogger;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.service.spi.ServiceException;

import org.hibernate.testing.junit4.BaseUnitTestCase;
import org.hibernate.testing.logger.LoggerInspectionRule;
import org.hibernate.testing.logger.Triggerable;
import org.junit.Rule;
import org.junit.Test;
import javax.cache.CacheManager;

import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.notNullValue;
Expand Down Expand Up @@ -63,11 +64,14 @@ public void testMissingCacheStrategyFail() {
}
catch (ServiceException expected) {
assertTyping( CacheException.class, expected.getCause() );
assertThat( expected.getMessage(), startsWith( "Unable to create requested service [" + org.hibernate.cache.spi.CacheImplementor.class.getName() + "]" ) );
assertThat( expected.getCause().getMessage(), startsWith( "On-the-fly creation of JCache Cache objects is not supported" ) );
assertThat( expected.getMessage(), startsWith(
"Unable to create requested service [" + org.hibernate.cache.spi.CacheImplementor.class.getName() + "]" ) );
assertThat( expected.getCause().getMessage(),
startsWith( "On-the-fly creation of JCache Cache objects is not supported" ) );
}
catch (CacheException expected) {
assertThat( expected.getMessage(), equalTo( "On-the-fly creation of JCache Cache objects is not supported" ) );
assertThat( expected.getMessage(),
equalTo( "On-the-fly creation of JCache Cache objects is not supported" ) );
}
}

Expand Down Expand Up @@ -129,15 +133,26 @@ private void doTestMissingCacheStrategyCreateWarn(Consumer<StandardServiceRegist

@Test
public void testMissingCacheStrategyFailLegacyNames1() {
doTestMissingCacheStrategyFailLegacyNames( TestHelper.queryRegionLegacyNames1, TestHelper.queryRegionLegacyNames2 );
doTestMissingCacheStrategyFailLegacyNames( TestHelper.queryRegionLegacyNames1,
TestHelper.queryRegionLegacyNames2 );
}

@Test
public void testMissingCacheStrategyFailLegacyNames2() {
doTestMissingCacheStrategyFailLegacyNames( TestHelper.queryRegionLegacyNames2, TestHelper.queryRegionLegacyNames1 );
doTestMissingCacheStrategyFailLegacyNames( TestHelper.queryRegionLegacyNames2,
TestHelper.queryRegionLegacyNames1 );
}

private void doTestMissingCacheStrategyFailLegacyNames(String[] existingLegacyCaches, String[] nonExistingLegacyCaches) {
// clean up global state from previous tests
final CacheManager standardCacheManager = JCacheHelper.locateStandardCacheManager();
for ( String regionName : existingLegacyCaches ) {
standardCacheManager.destroyCache( TestHelper.prefix( regionName ) );
}
for ( String regionName : nonExistingLegacyCaches ) {
standardCacheManager.destroyCache( TestHelper.prefix( regionName ) );
}

Map<String, Triggerable> triggerables = new HashMap<>();

// first, lets make sure that the regions used for model caches exist
Expand Down Expand Up @@ -169,7 +184,7 @@ private void doTestMissingCacheStrategyFailLegacyNames(String[] existingLegacyCa
}

// and now let's try to build the standard testing SessionFactory
try ( SessionFactoryImplementor ignored = TestHelper.buildStandardSessionFactory(
try (SessionFactoryImplementor ignored = TestHelper.buildStandardSessionFactory(
builder -> builder.applySetting( ConfigSettings.MISSING_CACHE_STRATEGY, "fail" )
) ) {
// The session should start successfully (if we reach this line, we're good)
Expand Down