From 1dd862521fe119a8584951b69aadd42d1472b800 Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Wed, 1 Feb 2012 14:47:30 -0800 Subject: [PATCH] Remove Cache.getTimeToLive method Not all caches have a time to live property, a cache that never evicts its elements doesn't have this concept. The only user in Gerrit is the web session logic, so move this into that code path. Change-Id: I74cb488764bea10cde9d6eb60824585268f6be34 --- .../gerrit/ehcache/PopulatingCache.java | 10 ---------- .../google/gerrit/ehcache/SimpleCache.java | 10 ---------- .../gerrit/httpd/CacheBasedWebSession.java | 4 +++- .../gerrit/httpd/WebSessionManager.java | 19 +++++++++++++++---- .../com/google/gerrit/server/cache/Cache.java | 11 ----------- .../server/cache/ConcurrentHashMapCache.java | 6 ------ .../gerrit/server/cache/ProxyCache.java | 6 ------ 7 files changed, 18 insertions(+), 48 deletions(-) diff --git a/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/PopulatingCache.java b/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/PopulatingCache.java index 6352ca8c68..f5c6c456f0 100644 --- a/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/PopulatingCache.java +++ b/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/PopulatingCache.java @@ -14,8 +14,6 @@ package com.google.gerrit.ehcache; -import static java.util.concurrent.TimeUnit.SECONDS; - import com.google.gerrit.server.cache.Cache; import com.google.gerrit.server.cache.EntryCreator; @@ -27,8 +25,6 @@ import net.sf.ehcache.constructs.blocking.CacheEntryFactory; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.concurrent.TimeUnit; - /** * A decorator for {@link Cache} which automatically constructs missing entries. *

@@ -111,12 +107,6 @@ class PopulatingCache implements Cache { self.put(new Element(key, value)); } - @Override - public long getTimeToLive(final TimeUnit unit) { - final long maxAge = self.getCacheConfiguration().getTimeToLiveSeconds(); - return unit.convert(maxAge, SECONDS); - } - @Override public String toString() { return "Cache[" + self.getName() + "]"; diff --git a/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/SimpleCache.java b/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/SimpleCache.java index 791488133d..e4428e3ed1 100644 --- a/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/SimpleCache.java +++ b/gerrit-ehcache/src/main/java/com/google/gerrit/ehcache/SimpleCache.java @@ -14,8 +14,6 @@ package com.google.gerrit.ehcache; -import static java.util.concurrent.TimeUnit.SECONDS; - import com.google.gerrit.server.cache.Cache; import net.sf.ehcache.CacheException; @@ -25,8 +23,6 @@ import net.sf.ehcache.Element; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.util.concurrent.TimeUnit; - /** * A fast in-memory and/or on-disk based cache. * @@ -78,12 +74,6 @@ final class SimpleCache implements Cache { self.removeAll(); } - @Override - public long getTimeToLive(final TimeUnit unit) { - final long maxAge = self.getCacheConfiguration().getTimeToLiveSeconds(); - return unit.convert(maxAge, SECONDS); - } - @Override public String toString() { return "Cache[" + self.getName() + "]"; diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java index d9203f57d8..270b4762e6 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/CacheBasedWebSession.java @@ -15,6 +15,7 @@ package com.google.gerrit.httpd; import static java.util.concurrent.TimeUnit.HOURS; +import static java.util.concurrent.TimeUnit.MINUTES; import com.google.gerrit.httpd.WebSessionManager.Key; import com.google.gerrit.httpd.WebSessionManager.Val; @@ -42,6 +43,7 @@ import javax.servlet.http.HttpServletResponse; @RequestScoped public final class CacheBasedWebSession implements WebSession { private static final String ACCOUNT_COOKIE = "GerritAccount"; + static final long MAX_AGE_MINUTES = HOURS.toMinutes(12); public static Module module() { return new CacheModule() { @@ -52,7 +54,7 @@ public final class CacheBasedWebSession implements WebSession { new TypeLiteral>() {}; disk(type, cacheName) // .memoryLimit(1024) // reasonable default for many sites - .maxAge(12, HOURS) // expire sessions if they are inactive + .maxAge(MAX_AGE_MINUTES, MINUTES) // expire sessions if they are inactive .evictionPolicy(EvictionPolicy.LRU) // keep most recently used ; bind(WebSessionManager.class); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java index 4a3e113c93..186d5da86c 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/WebSessionManager.java @@ -21,17 +21,22 @@ import static com.google.gerrit.server.ioutil.BasicSerialization.writeBytes; import static com.google.gerrit.server.ioutil.BasicSerialization.writeFixInt64; import static com.google.gerrit.server.ioutil.BasicSerialization.writeString; import static com.google.gerrit.server.ioutil.BasicSerialization.writeVarInt32; +import static com.google.gerrit.httpd.CacheBasedWebSession.MAX_AGE_MINUTES; import static java.util.concurrent.TimeUnit.HOURS; import static java.util.concurrent.TimeUnit.MILLISECONDS; -import static java.util.concurrent.TimeUnit.SECONDS; +import static java.util.concurrent.TimeUnit.MINUTES; import com.google.gerrit.reviewdb.Account; import com.google.gerrit.reviewdb.AccountExternalId; import com.google.gerrit.server.cache.Cache; +import com.google.gerrit.server.config.ConfigUtil; +import com.google.gerrit.server.config.GerritServerConfig; import com.google.inject.Inject; import com.google.inject.Singleton; import com.google.inject.name.Named; +import org.eclipse.jgit.lib.Config; + import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.ObjectInputStream; @@ -47,13 +52,19 @@ class WebSessionManager { return System.currentTimeMillis(); } + private final long sessionMaxAgeMillis; private final SecureRandom prng; private final Cache self; @Inject - WebSessionManager(@Named(CACHE_NAME) final Cache cache) { + WebSessionManager(@GerritServerConfig Config cfg, + @Named(CACHE_NAME) final Cache cache) { prng = new SecureRandom(); self = cache; + + sessionMaxAgeMillis = MINUTES.toMillis(ConfigUtil.getTimeUnit(cfg, + "cache", CACHE_NAME, "maxAge", + MAX_AGE_MINUTES, MINUTES)); } Key createKey(final Account.Id who) { @@ -90,7 +101,7 @@ class WebSessionManager { // early but also avoids us needing to refresh the cookie on // every single request. // - final long halfAgeRefresh = self.getTimeToLive(MILLISECONDS) >>> 1; + final long halfAgeRefresh = sessionMaxAgeMillis >>> 1; final long minRefresh = MILLISECONDS.convert(1, HOURS); final long refresh = Math.min(halfAgeRefresh, minRefresh); final long refreshCookieAt = now() + refresh; @@ -114,7 +125,7 @@ class WebSessionManager { // Client may store the cookie until we would remove it from our // own cache, after which it will certainly be invalid. // - return (int) self.getTimeToLive(SECONDS); + return (int) MILLISECONDS.toSeconds(sessionMaxAgeMillis); } else { // Client should not store the cookie, as the user asked for us // to not remember them long-term. Sending -1 as the age will diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/cache/Cache.java b/gerrit-server/src/main/java/com/google/gerrit/server/cache/Cache.java index 7159501af7..7892ea1e06 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/cache/Cache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/cache/Cache.java @@ -14,9 +14,6 @@ package com.google.gerrit.server.cache; -import java.util.concurrent.TimeUnit; - - /** * A fast in-memory and/or on-disk based cache. * @@ -35,12 +32,4 @@ public interface Cache { /** Remove all cached items. */ public void removeAll(); - - /** - * Get the time an element will survive in the cache. - * - * @param unit desired units of the return value. - * @return time an item can live before being purged. - */ - public long getTimeToLive(TimeUnit unit); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/cache/ConcurrentHashMapCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/cache/ConcurrentHashMapCache.java index 268fe2ba5a..bafdc49836 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/cache/ConcurrentHashMapCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/cache/ConcurrentHashMapCache.java @@ -15,7 +15,6 @@ package com.google.gerrit.server.cache; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.TimeUnit; /** * An infinitely sized cache backed by java.util.ConcurrentHashMap. @@ -46,9 +45,4 @@ public class ConcurrentHashMapCache implements Cache { public void removeAll() { map.clear(); } - - @Override - public long getTimeToLive(TimeUnit unit) { - return 0; - } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/cache/ProxyCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/cache/ProxyCache.java index 1fbc819e0c..c1b029292f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/cache/ProxyCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/cache/ProxyCache.java @@ -14,8 +14,6 @@ package com.google.gerrit.server.cache; -import java.util.concurrent.TimeUnit; - /** Proxy around a cache which has not yet been created. */ public final class ProxyCache implements Cache { private volatile Cache self; @@ -28,10 +26,6 @@ public final class ProxyCache implements Cache { return self.get(key); } - public long getTimeToLive(TimeUnit unit) { - return self.getTimeToLive(unit); - } - public void put(K key, V value) { self.put(key, value); }