Support "no change" in JCache configuration

The expiration policy may return null for update or access queries to not change the entry's expiration time. This was not supported by the configuration file, which translated null to mean eternal. This is now fixed and the default set to "eternal" explicitly.
ben-manes · Dec 20, 2015 · d9fd8c6 · d9fd8c6
1 parent f9af06a
commit d9fd8c6
Show file tree

Hide file tree

Showing 7 changed files with 65 additions and 58 deletions.
diff --git a/build.gradle b/build.gradle
@@ -78,6 +78,11 @@ subprojects {
       rootProject.testReport.reportOn it
     }
     it.dependsOn('jar')
+
+    // ensure tasks don't overwrite the default report directories used by the 'test' task
+    reports.html.destination = "${buildDir}/reports/${name}"
+    reports.junitXml.destination = "${buildDir}/reports/${name}/results"
+    binResultsDir = file("${buildDir}/reports/${name}/results/binary/${name}")
   }
 
   task testJar(type: Jar, group: "Build") {

diff --git a/caffeine/testing.gradle b/caffeine/testing.gradle
@@ -77,11 +77,6 @@ tasks.withType(Test) {
       }
     }
   }
-
-  // ensure tasks don't overwrite the default report directories used by the 'test' task
-  reports.html.destination = "${buildDir}/reports/${name}"
-  reports.junitXml.destination = "${buildDir}/reports/${name}/results"
-  binResultsDir = file("${buildDir}/reports/${name}/results/binary/${name}")
 }
 
 task stress(type: JavaExec, group: 'Cache tests', description: 'Executes a stress test') {

diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -3,4 +3,4 @@ distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-2.10-rc-1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-2.10-rc-2-bin.zip
diff --git a/jcache/build.gradle b/jcache/build.gradle
@@ -60,9 +60,6 @@ task testCompatibilityKit(type: Test, group: 'Build', description: 'Runs the JCa
 
   useJUnit()
   testClassesDir = file("${buildDir}/tck")
-  reports.html.destination = "${buildDir}/reports/${name}"
-  reports.junitXml.destination = "${buildDir}/${name}-results"
-  binResultsDir = file("${buildDir}/${name}-results/binary/${name}")
 
   def pkg = 'com.github.benmanes.caffeine.jcache'
   systemProperty 'java.net.preferIPv4Stack', 'true'

diff --git a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheProxy.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/CacheProxy.java
@@ -145,30 +145,35 @@ public V get(K key) {
       return null;
     }
 
+    long start, millis;
     boolean statsEnabled = statistics.isEnabled();
-    long start = (statsEnabled || !expirable.isEternal()) ? ticker.read() : 0L;
-    long millis = nanosToMillis(start);
-    if (expirable.hasExpired(millis)) {
-      cache.asMap().computeIfPresent(key, (k, e) -> {
-        if (e == expirable) {
-          dispatcher.publishExpired(this, key, expirable.get());
-          statistics.recordEvictions(1);
-          return null;
-        }
-        return e;
-      });
-      dispatcher.awaitSynchronous();
-      statistics.recordMisses(1L);
-      return null;
+    if (!expirable.isEternal()) {
+      start = ticker.read();
+      millis = nanosToMillis(start);
+      if (expirable.hasExpired(millis)) {
+        cache.asMap().computeIfPresent(key, (k, e) -> {
+          if (e == expirable) {
+            dispatcher.publishExpired(this, key, expirable.get());
+            statistics.recordEvictions(1);
+            return null;
+          }
+          return e;
+        });
+        dispatcher.awaitSynchronous();
+        statistics.recordMisses(1L);
+        return null;
+      }      
+    } else if (statsEnabled) {
+      start = ticker.read();
+      millis = nanosToMillis(start);
+    } else {
+      start = millis = 0L;
     }
+
     setAccessExpirationTime(expirable, millis);
     V value = copyValue(expirable);
     if (statsEnabled) {
-      if (value == null) {
-        statistics.recordMisses(1L);
-      } else {
-        statistics.recordHits(1L);
-      }
+      statistics.recordHits(1L);
       statistics.recordGetTime(ticker.read() - start);
     }
     return value;
@@ -304,19 +309,18 @@ public V getAndPut(K key, V value) {
     int[] puts = { 0 };
     V val = putNoCopyOrAwait(key, value, true, puts);
     dispatcher.awaitSynchronous();
-    statistics.recordPuts(puts[0]);
-
-    if (val == null) {
-      statistics.recordMisses(1L);
-    } else {
-      statistics.recordHits(1L);
-    }
     V copy = copyOf(val);
 
     if (statsEnabled) {
+      if (val == null) {
+        statistics.recordMisses(1L);
+      } else {
+        statistics.recordHits(1L);
+      }
       long duration = ticker.read() - start;
       statistics.recordGetTime(duration);
       statistics.recordPutTime(duration);
+      statistics.recordPuts(puts[0]);
     }
     return copy;
   }
@@ -547,15 +551,15 @@ public V getAndRemove(K key) {
     publishToCacheWriter(writer::delete, () -> key);
     V value = removeNoCopyOrAwait(key);
     dispatcher.awaitSynchronous();
-    if (value == null) {
-      statistics.recordMisses(1L);
-    } else {
-      statistics.recordHits(1L);
-      statistics.recordRemovals(1L);
-    }
     V copy = copyOf(value);
 
     if (statsEnabled) {
+      if (value == null) {
+        statistics.recordMisses(1L);
+      } else {
+        statistics.recordHits(1L);
+        statistics.recordRemovals(1L);
+      }
       long duration = ticker.read() - start;
       statistics.recordRemoveTime(duration);
       statistics.recordGetTime(duration);
@@ -1013,10 +1017,7 @@ protected final void requireNotClosed() {
    * @return a copy of the value if storing by value or the same instance if by reference
    */
   protected final @Nullable V copyValue(@Nullable Expirable<V> expirable) {
-    if (expirable == null) {
-      return null;
-    }
-    return copier.copy(expirable.get(), cacheManager.getClassLoader());
+    return (expirable == null) ? null : copier.copy(expirable.get(), cacheManager.getClassLoader());
   }
 
   /**
@@ -1026,7 +1027,7 @@ protected final void requireNotClosed() {
    * @return a copy of the mappings if storing by value or the same instance if by reference
    */
   protected final Map<K, V> copyMap(Map<K, Expirable<V>> map) {
-    final ClassLoader classLoader = cacheManager.getClassLoader();
+    ClassLoader classLoader = cacheManager.getClassLoader();
     return map.entrySet().stream().collect(Collectors.toMap(
         entry -> (K) copier.copy(entry.getKey(), classLoader),
         entry -> (V) copier.copy(entry.getValue().get(), classLoader)));

diff --git a/...src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java b/...src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java
@@ -23,6 +23,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import javax.annotation.Nullable;
 import javax.cache.configuration.Factory;
 import javax.cache.configuration.FactoryBuilder;
 import javax.cache.configuration.MutableCacheEntryListenerConfiguration;
@@ -169,20 +170,25 @@ public void addLazyExpiration() {
       Duration update = getDurationFor("policy.lazy-expiration.update");
       Duration access = getDurationFor("policy.lazy-expiration.access");
 
-      boolean eternal = creation.isEternal() && update.isEternal() && access.isEternal();
+      boolean eternal = (creation == Duration.ETERNAL)
+          && (update == Duration.ETERNAL)
+          && (access == Duration.ETERNAL);
       Factory<? extends ExpiryPolicy> factory = eternal
           ? EternalExpiryPolicy.factoryOf()
           : FactoryBuilder.factoryOf(new JCacheExpiryPolicy(creation, update, access));
       configuration.setExpiryPolicyFactory(factory);
     }
 
     /** Returns the duration for the expiration time. */
-    private Duration getDurationFor(String path) {
-      if (config.hasPath(path)) {
-        long nanos = config.getDuration(path, TimeUnit.MILLISECONDS);
-        return new Duration(TimeUnit.MILLISECONDS, nanos);
+    private @Nullable Duration getDurationFor(String path) {
+      if (!config.hasPath(path)) {
+        return null;
       }
-      return Duration.ETERNAL;
+      if (config.getString(path).equalsIgnoreCase("eternal")) {
+        return Duration.ETERNAL;
+      }
+      long nanos = config.getDuration(path, TimeUnit.MILLISECONDS);
+      return new Duration(TimeUnit.MILLISECONDS, nanos);
     }
 
     /** Adds the Caffeine eager expiration settings. */

diff --git a/jcache/src/main/resources/reference.conf b/jcache/src/main/resources/reference.conf
@@ -53,16 +53,19 @@ caffeine.jcache {
       # entry remains in place.
       lazy-expiration {
         # The duration before a newly created entry is considered expired. If set to 0 then the
-        # entry is considered to be already expired and will not be added to the cache.
-        creation = null
+        # entry is considered to be already expired and will not be added to the cache. May be
+        # a time duration or "eternal" to indicate no expiration.
+        creation = "eternal"
 
         # The duration before a updated entry is considered expired. If set to 0 then the entry is
-        # considered immediately expired.
-        update = null
+        # considered immediately expired. May be null to indicate no change. May be a time duration,
+        # null to indicate no change, or "eternal" to indicate no expiration.
+        update = "eternal"
 
         # The duration before a read of an entry is considered expired. If set to 0 then the entry
-        # is considered immediately expired.
-        access = null
+        # is considered immediately expired. May be null to indicate no change. May be a time
+        # duration, null to indicate no change, or "eternal" to indicate no expiration.
+        access = "eternal"
       }
 
       # The expiration thresholds before eagerly evicting an entry. This settings correspond to the