From ac014b531aace241a423968697179737ceb4f926 Mon Sep 17 00:00:00 2001 From: Radu Cotescu Date: Tue, 18 Aug 2020 13:30:47 +0200 Subject: [PATCH] fix #447: Allow the CaffeineCachingProvider to be used with OSGi DS * annotated CaffeineCachingProvider to make it an OSGi service * when running in an OSGi container, set the CaffeineCachingProvider's classloader as the Thread context classloader for every API call on the provided CacheManagerImpl - this makes sure that the default config is read from the bundle; the default classloader will do a fallback to the app classloader, allowing to override / define configurations * added an OSGi DS test * made the bnd build reproducible (no extra bundle headers) --- build.gradle | 12 +- gradle/dependencies.gradle | 11 +- jcache/build.gradle | 15 ++- .../caffeine/jcache/CacheManagerImpl.java | 122 +++++++++++------- .../jcache/spi/CaffeineCachingProvider.java | 54 +++++++- .../benmanes/caffeine/jcache/OSGiTest.java | 19 ++- 6 files changed, 165 insertions(+), 68 deletions(-) diff --git a/build.gradle b/build.gradle index abf06b4442..778d00b65a 100644 --- a/build.gradle +++ b/build.gradle @@ -73,17 +73,9 @@ subprojects { testRuntimeOnly testLibraries.osgiRuntime } - configurations { - bundleCompile - } - - sourceSets { - bundle - } - task bundle(type: aQute.bnd.gradle.Bundle) { - from sourceSets.bundle.output - sourceSet = sourceSets.bundle + from sourceSets.main.output + sourceSet = sourceSets.main } tasks.withType(JavaCompile) { diff --git a/gradle/dependencies.gradle b/gradle/dependencies.gradle index 44c5d99a7d..95616bccee 100644 --- a/gradle/dependencies.gradle +++ b/gradle/dependencies.gradle @@ -54,6 +54,7 @@ ext { jsr330: '1', nullaway: '0.7.9', ohc: '0.6.1', + osgiComponentAnnotations: '1.4.0', picocli: '4.5.0', slf4j: '1.7.30', tcache: '2.0.1', @@ -73,6 +74,10 @@ ext { paxExam: '4.13.3', testng: '7.3.0', truth: '0.24', + felix: '6.0.3', + felixScr: '2.1.20', + osgiUtilFunction: '1.1.0', + osgiUtilPromise: '1.1.0' ] pluginVersions = [ apt: '0.21', @@ -128,6 +133,7 @@ ext { jsr330: "javax.inject:javax.inject:${versions.jsr330}", nullaway: "com.uber.nullaway:nullaway:${versions.nullaway}", ohc: "org.caffinitas.ohc:ohc-core-j8:${versions.ohc}", + osgiComponentAnnotations: "org.osgi:org.osgi.service.component.annotations:${versions.osgiComponentAnnotations}", picocli: "info.picocli:picocli:${versions.picocli}", slf4jNop: "org.slf4j:slf4j-nop:${versions.slf4j}", tcache: "com.trivago:triava:${versions.tcache}", @@ -151,10 +157,13 @@ ext { junit: "junit:junit:${testVersions.junit}", mockito: "org.mockito:mockito-core:${testVersions.mockito}", osgiCompile: [ - 'org.apache.felix:org.apache.felix.framework:6.0.3', "org.ops4j.pax.exam:pax-exam-junit4:${testVersions.paxExam}", ], osgiRuntime: [ + "org.apache.felix:org.apache.felix.framework:${testVersions.felix}", + "org.apache.felix:org.apache.felix.scr:${testVersions.felixScr}", + "org.osgi:org.osgi.util.function:${testVersions.osgiUtilFunction}", + "org.osgi:org.osgi.util.promise:${testVersions.osgiUtilPromise}", "org.ops4j.pax.exam:pax-exam-container-native:${testVersions.paxExam}", "org.ops4j.pax.exam:pax-exam-link-mvn:${testVersions.paxExam}", "org.ops4j.pax.url:pax-url-aether:2.6.2", diff --git a/jcache/build.gradle b/jcache/build.gradle index c7a233d131..710ac5d5ec 100644 --- a/jcache/build.gradle +++ b/jcache/build.gradle @@ -11,6 +11,7 @@ dependencies { api libraries.jcache api libraries.config api libraries.jsr330 + api libraries.osgiComponentAnnotations testImplementation libraries.guava testImplementation testLibraries.junit @@ -32,16 +33,16 @@ dependencies { jar.manifest { attributes 'Bundle-SymbolicName': 'com.github.ben-manes.caffeine.jcache' attributes 'Import-Package': [ - 'javax.cache.*', - 'javax.management', - 'com.typesafe.config', - 'com.github.benmanes.caffeine.cache', - 'com.github.benmanes.caffeine.cache.stats'].join(',') + '!org.checkerframework.checker.*', + '*'].join(',') attributes 'Export-Package': [ 'com.github.benmanes.caffeine.jcache.spi', 'com.github.benmanes.caffeine.jcache.copy', 'com.github.benmanes.caffeine.jcache.configuration'].join(',') attributes 'Automatic-Module-Name': 'com.github.benmanes.caffeine.jcache' + attributes '-exportcontents': '${removeall;${packages;VERSIONED};${packages;CONDITIONAL}}' + attributes '-snapshot': 'SNAPSHOT' + attributes '-noextraheaders': true } task unzipJCacheJavaDoc(type: Copy, group: 'Build', description: 'Unzips the JCache JavaDoc') { @@ -87,6 +88,10 @@ task osgiTests(type: Test, group: 'Build', description: 'Isolated OSGi tests') { tasks.test.dependsOn(it) systemProperty 'config.osgi.version', versions.config systemProperty 'jcache.osgi.version', versions.jcache + systemProperty 'checkerAnnotations.version', versions.checkerFramework + systemProperty 'felixScr.version', testVersions.felixScr + systemProperty 'osgiUtil.function', testVersions.osgiUtilFunction + systemProperty 'osgiUtil.promise', testVersions.osgiUtilPromise systemProperty 'caffeine.osgi.jar', project(':caffeine').jar.archivePath.path systemProperty 'caffeine-jcache.osgi.jar', project(':jcache').jar.archivePath.path } diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheManagerImpl.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheManagerImpl.java index 99c54f7832..4378e9ce60 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheManagerImpl.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheManagerImpl.java @@ -33,6 +33,8 @@ import org.checkerframework.checker.nullness.qual.Nullable; +import com.github.benmanes.caffeine.jcache.spi.CaffeineCachingProvider; + /** * An implementation of JSR-107 {@link CacheManager} that manages Caffeine-based caches. * @@ -45,6 +47,7 @@ public final class CacheManagerImpl implements CacheManager { private final CachingProvider cacheProvider; private final Properties properties; private final URI uri; + private final boolean runsAsAnOsgiBundle; private volatile boolean closed; @@ -55,6 +58,8 @@ public CacheManagerImpl(CachingProvider cacheProvider, this.properties = requireNonNull(properties); this.caches = new ConcurrentHashMap<>(); this.uri = requireNonNull(uri); + runsAsAnOsgiBundle = + cacheProvider instanceof CaffeineCachingProvider && ((CaffeineCachingProvider) cacheProvider).isOsgiComponent(); } @Override @@ -80,63 +85,90 @@ public Properties getProperties() { @Override public > Cache createCache( String cacheName, C configuration) { - requireNotClosed(); - requireNonNull(configuration); - - CacheProxy cache = caches.compute(cacheName, (name, existing) -> { - if ((existing != null) && !existing.isClosed()) { - throw new CacheException("Cache " + cacheName + " already exists"); - } else if (CacheFactory.isDefinedExternally(cacheName)) { - throw new CacheException("Cache " + cacheName + " is configured externally"); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + try { + if (runsAsAnOsgiBundle) { + // override the context class loader with the CachingManager's classloader + Thread.currentThread().setContextClassLoader(getClassLoader()); } - return CacheFactory.createCache(this, cacheName, configuration); - }); - enableManagement(cache.getName(), cache.getConfiguration().isManagementEnabled()); - enableStatistics(cache.getName(), cache.getConfiguration().isStatisticsEnabled()); - - @SuppressWarnings("unchecked") - Cache castedCache = (Cache) cache; - return castedCache; + requireNotClosed(); + requireNonNull(configuration); + + CacheProxy cache = caches.compute(cacheName, (name, existing) -> { + if ((existing != null) && !existing.isClosed()) { + throw new CacheException("Cache " + cacheName + " already exists"); + } else if (CacheFactory.isDefinedExternally(cacheName)) { + throw new CacheException("Cache " + cacheName + " is configured externally"); + } + return CacheFactory.createCache(this, cacheName, configuration); + }); + enableManagement(cache.getName(), cache.getConfiguration().isManagementEnabled()); + enableStatistics(cache.getName(), cache.getConfiguration().isStatisticsEnabled()); + + @SuppressWarnings("unchecked") + Cache castedCache = (Cache) cache; + return castedCache; + } finally { + Thread.currentThread().setContextClassLoader(old); + } } @Override public @Nullable Cache getCache( String cacheName, Class keyType, Class valueType) { - CacheProxy cache = getCache(cacheName); - if (cache == null) { - return null; - } - requireNonNull(keyType); - requireNonNull(valueType); - - Configuration config = cache.getConfiguration(); - if (keyType != config.getKeyType()) { - throw new ClassCastException("Incompatible cache key types specified, expected " + - config.getKeyType() + " but " + keyType + " was specified"); - } else if (valueType != config.getValueType()) { - throw new ClassCastException("Incompatible cache value types specified, expected " + - config.getValueType() + " but " + valueType + " was specified"); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + try { + if (runsAsAnOsgiBundle) { + // override the context class loader with the CachingManager's classloader + Thread.currentThread().setContextClassLoader(getClassLoader()); + } + CacheProxy cache = getCache(cacheName); + if (cache == null) { + return null; + } + requireNonNull(keyType); + requireNonNull(valueType); + + Configuration config = cache.getConfiguration(); + if (keyType != config.getKeyType()) { + throw new ClassCastException("Incompatible cache key types specified, expected " + + config.getKeyType() + " but " + keyType + " was specified"); + } else if (valueType != config.getValueType()) { + throw new ClassCastException("Incompatible cache value types specified, expected " + + config.getValueType() + " but " + valueType + " was specified"); + } + return cache; + } finally { + Thread.currentThread().setContextClassLoader(old); } - return cache; } @Override public CacheProxy getCache(String cacheName) { - requireNonNull(cacheName); - requireNotClosed(); - - CacheProxy cache = caches.computeIfAbsent(cacheName, name -> { - CacheProxy created = CacheFactory.tryToCreateFromExternalSettings(this, name); - if (created != null) { - created.enableManagement(created.getConfiguration().isManagementEnabled()); - created.enableStatistics(created.getConfiguration().isStatisticsEnabled()); + ClassLoader old = Thread.currentThread().getContextClassLoader(); + try { + if (runsAsAnOsgiBundle) { + // override the context class loader with the CachingManager's classloader + Thread.currentThread().setContextClassLoader(getClassLoader()); } - return created; - }); - - @SuppressWarnings("unchecked") - CacheProxy castedCache = (CacheProxy) cache; - return castedCache; + requireNonNull(cacheName); + requireNotClosed(); + + CacheProxy cache = caches.computeIfAbsent(cacheName, name -> { + CacheProxy created = CacheFactory.tryToCreateFromExternalSettings(this, name); + if (created != null) { + created.enableManagement(created.getConfiguration().isManagementEnabled()); + created.enableStatistics(created.getConfiguration().isStatisticsEnabled()); + } + return created; + }); + + @SuppressWarnings("unchecked") + CacheProxy castedCache = (CacheProxy) cache; + return castedCache; + } finally { + Thread.currentThread().setContextClassLoader(old); + } } @Override diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/spi/CaffeineCachingProvider.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/spi/CaffeineCachingProvider.java index 3c7eaf7e31..b8c1b34e77 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/spi/CaffeineCachingProvider.java +++ b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/spi/CaffeineCachingProvider.java @@ -37,9 +37,13 @@ import javax.cache.spi.CachingProvider; import org.checkerframework.checker.nullness.qual.Nullable; +import org.osgi.service.component.annotations.Activate; +import org.osgi.service.component.annotations.Component; import com.github.benmanes.caffeine.jcache.CacheManagerImpl; +import com.github.benmanes.caffeine.jcache.configuration.TypesafeConfigurator; import com.google.errorprone.annotations.concurrent.GuardedBy; +import com.typesafe.config.ConfigFactory; /** * A provider that produces a JCache implementation backed by Caffeine. Typically this provider is @@ -53,6 +57,7 @@ * * @author ben.manes@gmail.com (Ben Manes) */ +@Component public final class CaffeineCachingProvider implements CachingProvider { private static final ClassLoader DEFAULT_CLASS_LOADER = AccessController.doPrivileged( (PrivilegedAction) JCacheClassLoader::new); @@ -60,6 +65,8 @@ public final class CaffeineCachingProvider implements CachingProvider { @GuardedBy("itself") private final Map> cacheManagers; + private boolean isOsgiComponent; + public CaffeineCachingProvider() { this.cacheManagers = new WeakHashMap<>(1); } @@ -165,11 +172,15 @@ private ClassLoader getManagerClassLoader(ClassLoader classLoader) { */ private static final class JCacheClassLoader extends ClassLoader { + public JCacheClassLoader() { + super(Thread.currentThread().getContextClassLoader()); + } + @Override public Class loadClass(String name) throws ClassNotFoundException { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); ClassNotFoundException error = null; - if (contextClassLoader != null) { + if (contextClassLoader != null && contextClassLoader != DEFAULT_CLASS_LOADER) { try { return contextClassLoader.loadClass(name); } catch (ClassNotFoundException e) { @@ -179,7 +190,16 @@ public Class loadClass(String name) throws ClassNotFoundException { ClassLoader classClassLoader = getClass().getClassLoader(); if ((classClassLoader != null) && (classClassLoader != contextClassLoader)) { - return classClassLoader.loadClass(name); + try { + return classClassLoader.loadClass(name); + } catch (ClassNotFoundException e) { + error = e; + } + } + + ClassLoader parentClassLoader = getParent(); + if (parentClassLoader != null && parentClassLoader != contextClassLoader && parentClassLoader != classClassLoader) { + return parentClassLoader.loadClass(name); } throw (error == null) ? new ClassNotFoundException(name) : error; } @@ -187,14 +207,14 @@ public Class loadClass(String name) throws ClassNotFoundException { @Override public @Nullable URL getResource(String name) { ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); - if (contextClassLoader != null) { + if (contextClassLoader != null && contextClassLoader != DEFAULT_CLASS_LOADER) { URL resource = contextClassLoader.getResource(name); if (resource != null) { return resource; } } - ClassLoader classClassLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader classClassLoader = getClass().getClassLoader(); if ((classClassLoader != null) && (classClassLoader != contextClassLoader)) { URL resource = classClassLoader.getResource(name); if (resource != null) { @@ -202,6 +222,11 @@ public Class loadClass(String name) throws ClassNotFoundException { } } + ClassLoader parentClassLoader = getParent(); + if (parentClassLoader != null && parentClassLoader != contextClassLoader && parentClassLoader != classClassLoader) { + return parentClassLoader.getResource(name); + } + return null; } @@ -210,16 +235,33 @@ public Enumeration getResources(String name) throws IOException { List resources = new ArrayList<>(); ClassLoader contextClassLoader = Thread.currentThread().getContextClassLoader(); - if (contextClassLoader != null) { + if (contextClassLoader != null && contextClassLoader != DEFAULT_CLASS_LOADER) { resources.addAll(Collections.list(contextClassLoader.getResources(name))); } - ClassLoader classClassLoader = Thread.currentThread().getContextClassLoader(); + ClassLoader classClassLoader = getClass().getClassLoader(); if ((classClassLoader != null) && (classClassLoader != contextClassLoader)) { resources.addAll(Collections.list(classClassLoader.getResources(name))); } + ClassLoader parentClassLoader = getParent(); + if (parentClassLoader != null && parentClassLoader != contextClassLoader && parentClassLoader != classClassLoader) { + resources.addAll(Collections.list(parentClassLoader.getResources(name))); + } + return Collections.enumeration(resources); } } + + @Activate + @SuppressWarnings("unused") + private void activate() { + isOsgiComponent = true; + TypesafeConfigurator.setConfigSource(() -> ConfigFactory.load(DEFAULT_CLASS_LOADER)); + } + + public boolean isOsgiComponent() { + return isOsgiComponent; + } + } diff --git a/jcache/src/test/java/com/github/benmanes/caffeine/jcache/OSGiTest.java b/jcache/src/test/java/com/github/benmanes/caffeine/jcache/OSGiTest.java index 344509e4e8..18fbe56c44 100644 --- a/jcache/src/test/java/com/github/benmanes/caffeine/jcache/OSGiTest.java +++ b/jcache/src/test/java/com/github/benmanes/caffeine/jcache/OSGiTest.java @@ -15,6 +15,8 @@ */ package com.github.benmanes.caffeine.jcache; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.ops4j.pax.exam.CoreOptions.bundle; import static org.ops4j.pax.exam.CoreOptions.junitBundles; @@ -24,6 +26,7 @@ import javax.cache.Cache; import javax.cache.Caching; import javax.cache.spi.CachingProvider; +import javax.inject.Inject; import org.junit.Test; import org.junit.runner.RunWith; @@ -33,6 +36,8 @@ import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerMethod; +import com.github.benmanes.caffeine.jcache.spi.CaffeineCachingProvider; + /** * @author ben.manes@gmail.com (Ben Manes) */ @@ -40,6 +45,9 @@ @ExamReactorStrategy(PerMethod.class) public final class OSGiTest { + @Inject + private CachingProvider cachingProvider; + @Configuration public Option[] config() { return options( @@ -47,7 +55,10 @@ public Option[] config() { bundle("file:" + System.getProperty("caffeine.osgi.jar")), bundle("file:" + System.getProperty("caffeine-jcache.osgi.jar")), mavenBundle("com.typesafe", "config", System.getProperty("config.osgi.version")), - mavenBundle("javax.cache", "cache-api", System.getProperty("jcache.osgi.version"))); + mavenBundle("javax.cache", "cache-api", System.getProperty("jcache.osgi.version")), + mavenBundle("org.apache.felix", "org.apache.felix.scr", System.getProperty("felixScr.version")), + mavenBundle().groupId("org.osgi").artifactId("org.osgi.util.function").version(System.getProperty("osgiUtil.function")), + mavenBundle().groupId("org.osgi").artifactId("org.osgi.util.promise").version(System.getProperty("osgiUtil.promise"))); } @Test @@ -59,4 +70,10 @@ public void sanity() { .getCache("test-cache-2", String.class, Integer.class); assertNull(cache.get("a")); } + + @Test + public void testOSGIDS() { + assertNotNull("Should have found a registered CachingProvider.", cachingProvider); + assertEquals(CaffeineCachingProvider.class, cachingProvider.getClass()); + } }