Improved default layering rules for JCache configuration (fixes #171)

Previously defaults were only honored for resource configured caches. An
explicit cache configuration (via CacheManager#createCache) was not did
not use the defaults for non-JSR settings. This made default a poorly
named template only used by resource configurations.

It is now used in these cases as well, which reduces surprises. A nice
benefit is that anonymous caches created by the annotations are not
stuck on the JSR's defaults (unbounded, serialized). A safe threshold
can be set in the defaults to avoid memory leaks, etc.

Restricted CacheManager#createCache from overriding the externalized
configuration. As the caches are lazily loaded or destroyed, creating
a defined cache could be forced to a setting. That caused confusion on
when the layering, e.g. would the explicit layer on the externalized?

Added missing key-type and value-type configuration settings for getting
a cache by its name+types.

jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheFactory.java

@@ -60,6 +60,16 @@ public CacheFactory(CacheManager cacheManager, Config rootConfig) {
     this.rootConfig = requireNonNull(rootConfig);
   }
 
+  /**
+   * Returns a if the cache definition is found in the external settings file.
+   *
+   * @param cacheName the name of the cache
+   * @return {@code true} if a definition exists
+   */
+  public boolean isDefinedExternally(String cacheName) {
+    return TypesafeConfigurator.cacheNames(rootConfig).contains(cacheName);
+  }
+
   /**
    * Returns a newly created cache instance if a definition is found in the external settings file.
    *
@@ -85,23 +95,32 @@ public <K, V> CacheProxy<K, V> createCache(String cacheName, Configuration<K, V>
   }
 
   /** Copies the configuration and overlays it on top of the default settings. */
+  @SuppressWarnings("PMD.AccessorMethodGeneration")
   private <K, V> CaffeineConfiguration<K, V> resolveConfigurationFor(
       Configuration<K, V> configuration) {
     if (configuration instanceof CaffeineConfiguration<?, ?>) {
       return new CaffeineConfiguration<>((CaffeineConfiguration<K, V>) configuration);
     }
 
-    CaffeineConfiguration<K, V> defaults = TypesafeConfigurator.defaults(rootConfig);
+    CaffeineConfiguration<K, V> template = TypesafeConfigurator.defaults(rootConfig);
     if (configuration instanceof CompleteConfiguration<?, ?>) {
-      CaffeineConfiguration<K, V> config = new CaffeineConfiguration<>(
-          (CompleteConfiguration<K, V>) configuration);
-      config.setCopierFactory(defaults.getCopierFactory());
-      return config;
+      CompleteConfiguration<K, V> complete = (CompleteConfiguration<K, V>) configuration;
+      template.setReadThrough(complete.isReadThrough());
+      template.setWriteThrough(complete.isWriteThrough());
+      template.setManagementEnabled(complete.isManagementEnabled());
+      template.setStatisticsEnabled(complete.isStatisticsEnabled());
+      template.getCacheEntryListenerConfigurations()
+          .forEach(template::removeCacheEntryListenerConfiguration);
+      complete.getCacheEntryListenerConfigurations()
+          .forEach(template::addCacheEntryListenerConfiguration);
+      template.setCacheLoaderFactory(complete.getCacheLoaderFactory());
+      template.setCacheWriterFactory(complete.getCacheWriterFactory());
+      template.setExpiryPolicyFactory(complete.getExpiryPolicyFactory());
     }
 
-    defaults.setTypes(configuration.getKeyType(), configuration.getValueType());
-    defaults.setStoreByValue(configuration.isStoreByValue());
-    return defaults;
+    template.setTypes(configuration.getKeyType(), configuration.getValueType());
+    template.setStoreByValue(configuration.isStoreByValue());
+    return template;
   }
 
   /** A one-shot builder for creating a cache instance. */

jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheManagerImpl.java

@@ -94,6 +94,8 @@ public <K, V, C extends Configuration<K, V>> Cache<K, V> createCache(String cach
     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(cacheName, configuration);
     });

jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java

@@ -19,9 +19,11 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.NANOSECONDS;
 
+import java.util.Collections;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.OptionalLong;
+import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
@@ -54,6 +56,18 @@ public final class TypesafeConfigurator {
 
   private TypesafeConfigurator() {}
 
+  /**
+   * Retrieves the names of the caches defined in the configuration resource.
+   *
+   * @param config the configuration resource
+   * @return the names of the configured caches
+   */
+  public static Set<String> cacheNames(Config config) {
+    return config.hasPath("caffeine.jcache")
+        ? Collections.unmodifiableSet(config.getObject("caffeine.jcache").keySet())
+        : Collections.emptySet();
+  }
+
   /**
    * Retrieves the default cache settings from the configuration resource.
    *
@@ -114,6 +128,7 @@ private static final class Configurator<K, V> {
 
     /** Returns a configuration built from the external settings. */
     CaffeineConfiguration<K, V> configure() {
+      addKeyValueTypes();
       addStoreByValue();
       addListeners();
       addReadThrough();
@@ -127,6 +142,19 @@ CaffeineConfiguration<K, V> configure() {
       return configuration;
     }
 
+    /** Adds the key and value class types. */
+    private void addKeyValueTypes() {
+      try {
+        @SuppressWarnings("unchecked")
+        Class<K> keyType = (Class<K>) Class.forName(merged.getString("key-type"));
+        @SuppressWarnings("unchecked")
+        Class<V> valueType = (Class<V>) Class.forName(merged.getString("value-type"));
+        configuration.setTypes(keyType, valueType);
+      } catch (ClassNotFoundException e) {
+        throw new IllegalStateException(e);
+      }
+    }
+
     /** Adds the store-by-value settings. */
     private void addStoreByValue() {
       configuration.setStoreByValue(merged.getBoolean("store-by-value.enabled"));

jcache/src/main/resources/reference.conf

@@ -6,6 +6,12 @@ caffeine.jcache {
   # A named cache is configured by nesting a new definition under the caffeine.jcache namespace. The
   # per-cache configuration is overlaid on top of the default configuration.
   default {
+
+    # The required type of the keys
+    key-type = java.lang.Object
+    # The required type of the values
+    value-type = java.lang.Object
+
     # The strategy for copying the cache entry for value-based storage
     store-by-value {
 

jcache/src/test/java/com/github/benmanes/caffeine/jcache/configuration/JCacheConfigurationTest.java

@@ -0,0 +1,85 @@
+/*
+ * Copyright 2017 Nick Robison. All Rights Reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.github.benmanes.caffeine.jcache.configuration;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
+
+import java.util.OptionalLong;
+import java.util.function.Supplier;
+
+import javax.cache.Cache;
+import javax.cache.CacheException;
+import javax.cache.CacheManager;
+import javax.cache.Caching;
+import javax.cache.configuration.MutableConfiguration;
+import javax.cache.spi.CachingProvider;
+
+import org.testng.Assert;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.github.benmanes.caffeine.jcache.configuration.CaffeineConfiguration;
+import com.github.benmanes.caffeine.jcache.spi.CaffeineCachingProvider;
+
+/**
+ * @author Nick Robison (github.com/nickrobison)
+ */
+public final class JCacheConfigurationTest {
+  private static final String PROVIDER_NAME = CaffeineCachingProvider.class.getName();
+
+  private MutableConfiguration<String, String> cacheConfig;
+  private CacheManager cacheManager;
+
+  @BeforeClass
+  public void beforeClass() {
+    final CachingProvider provider = Caching.getCachingProvider(PROVIDER_NAME);
+    cacheManager = provider.getCacheManager();
+    cacheManager.destroyCache("cache-not-in-config-file");
+
+    cacheConfig = new MutableConfiguration<>();
+    cacheConfig.setTypes(String.class, String.class);
+    cacheConfig.setStatisticsEnabled(true);
+  }
+
+  @Test
+  public void anonymousCache() {
+    checkConfiguration(() ->
+        cacheManager.createCache("cache-not-in-config-file", cacheConfig), 500L);
+    checkConfiguration(() ->
+        cacheManager.getCache("cache-not-in-config-file", String.class, String.class), 500L);
+  }
+
+  @Test
+  public void definedCache() {
+    try {
+      cacheManager.createCache("test-cache-2", cacheConfig);
+      Assert.fail();
+    } catch (CacheException ignored) {}
+
+    checkConfiguration(() ->
+        cacheManager.getCache("test-cache-2", String.class, Integer.class), 1000L);
+  }
+
+  private void checkConfiguration(Supplier<Cache<?, ?>> cacheSupplier, long expectedValue) {
+    Cache<?, ?> cache = cacheSupplier.get();
+
+    @SuppressWarnings("unchecked")
+    CaffeineConfiguration<?, ?> configuration =
+        cache.getConfiguration(CaffeineConfiguration.class);
+    assertThat(configuration.getMaximumSize(), is(OptionalLong.of(expectedValue)));
+  }
+}

jcache/src/test/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurationTest.java

@@ -48,6 +48,8 @@ public void defaults() {
     CaffeineConfiguration<Integer, Integer> defaults =
         TypesafeConfigurator.defaults(ConfigFactory.load());
     assertThat(defaults.getMaximumSize(), is(OptionalLong.of(500)));
+    assertThat(defaults.getKeyType(), is(Object.class));
+    assertThat(defaults.getValueType(), is(Object.class));
   }
 
   @Test
@@ -61,8 +63,14 @@ public void testCache() {
 
   @Test
   public void testCache2() {
-    assertThat(TypesafeConfigurator.from(ConfigFactory.load(), "test-cache-2"),
-        is(not(equalTo(TypesafeConfigurator.from(ConfigFactory.load(), "test-cache").get()))));
+    Optional<CaffeineConfiguration<Integer, Integer>> config1 =
+        TypesafeConfigurator.from(ConfigFactory.load(), "test-cache");
+    Optional<CaffeineConfiguration<Integer, Integer>> config2 =
+        TypesafeConfigurator.from(ConfigFactory.load(), "test-cache-2");
+    assertThat(config1, is(not(equalTo(config2))));
+
+    assertThat(config2.get().getKeyType(), is(String.class));
+    assertThat(config2.get().getValueType(), is(Integer.class));
   }
 
   @Test
@@ -81,6 +89,8 @@ static void checkTestCache(CaffeineConfiguration<?, ?> config) {
     checkStoreByValue(config);
     checkListener(config);
 
+    assertThat(config.getKeyType(), is(Object.class));
+    assertThat(config.getValueType(), is(Object.class));
     assertThat(config.getCacheLoaderFactory().create(),
         instanceOf(TestCacheLoader.class));
     assertThat(config.getCacheWriter(), instanceOf(TestCacheWriter.class));

jcache/src/test/resources/application.conf

@@ -49,6 +49,9 @@ caffeine.jcache {
   }
 
   test-cache-2 {
+    key-type = java.lang.String
+    value-type = java.lang.Integer
+
     policy.maximum.size = 1000
   }