From d9fd8c6e652c0c0335a0bf4dcd6cab17f330cdf1 Mon Sep 17 00:00:00 2001 From: Ben Manes Date: Sat, 19 Dec 2015 17:19:03 -0800 Subject: [PATCH] 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. --- build.gradle | 5 ++ caffeine/testing.gradle | 5 -- gradle/wrapper/gradle-wrapper.properties | 2 +- jcache/build.gradle | 3 - .../benmanes/caffeine/jcache/CacheProxy.java | 75 ++++++++++--------- .../configuration/TypesafeConfigurator.java | 18 +++-- jcache/src/main/resources/reference.conf | 15 ++-- 7 files changed, 65 insertions(+), 58 deletions(-) diff --git a/build.gradle b/build.gradle index e4590d4ed6..6c73cb0c98 100644 --- 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 index dcd3b64d2e..c98954fbcf 100644 --- 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 index 651fb444a5..71b3527d90 100644 --- 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 index 1667be502a..cfd0d2364d 100644 --- 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 index ca0e4d8674..38923dd915 100644 --- 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 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 copyMap(Map> 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/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java b/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java index 465d4308c4..c4e31a932a 100644 --- a/jcache/src/main/java/com/github/benmanes/caffeine/jcache/configuration/TypesafeConfigurator.java +++ b/jcache/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,7 +170,9 @@ 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 factory = eternal ? EternalExpiryPolicy.factoryOf() : FactoryBuilder.factoryOf(new JCacheExpiryPolicy(creation, update, access)); @@ -177,12 +180,15 @@ public void addLazyExpiration() { } /** 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 index 6f0d6a0126..cd4d2c9e47 100644 --- 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