Convert cache expireAfterWrite to nullable Duration

Use the same Duration type throughout the stack. In particular, this
allows us to use the java.time API for doing expiry calculations in
H2CacheImpl.

Converting to Optional<Duration> is beyond the scope of this change, as
it would properly imply changing the other nullable fields in CacheDef.

Change-Id: I63a173ce1bef83fa84ccb8b8b6ea75ae198e5728

java/com/google/gerrit/common/TimeUtil.java

@@ -17,6 +17,7 @@ package com.google.gerrit.common;
 import com.google.common.annotations.GwtIncompatible;
 import com.google.common.annotations.VisibleForTesting;
 import java.sql.Timestamp;
+import java.time.Instant;
 import java.util.function.LongSupplier;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.storage.file.FileBasedConfig;
@@ -36,6 +37,10 @@ public class TimeUtil {
     return currentMillisSupplier.getAsLong();
   }
 
+  public static Instant now() {
+    return Instant.ofEpochMilli(nowMs());
+  }
+
   public static Timestamp nowTs() {
     return new Timestamp(nowMs());
   }

java/com/google/gerrit/httpd/GitOverHttpServlet.java

@@ -44,10 +44,10 @@ import com.google.inject.Singleton;
 import com.google.inject.TypeLiteral;
 import com.google.inject.name.Named;
 import java.io.IOException;
+import java.time.Duration;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
-import java.util.concurrent.TimeUnit;
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
 import javax.servlet.FilterConfig;
@@ -119,7 +119,7 @@ public class GitOverHttpServlet extends GitServlet {
             protected void configure() {
               cache(ID_CACHE, AdvertisedObjectsCacheKey.class, new TypeLiteral<Set<ObjectId>>() {})
                   .maximumWeight(4096)
-                  .expireAfterWrite(10, TimeUnit.MINUTES);
+                  .expireAfterWrite(Duration.ofMinutes(10));
             }
           });
     }

java/com/google/gerrit/httpd/H2CacheBasedWebSession.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.httpd;
 
-import static java.util.concurrent.TimeUnit.MINUTES;
-
 import com.google.common.cache.Cache;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.registration.DynamicItem;
@@ -30,6 +28,7 @@ import com.google.inject.Provider;
 import com.google.inject.assistedinject.FactoryModuleBuilder;
 import com.google.inject.name.Named;
 import com.google.inject.servlet.RequestScoped;
+import java.time.Duration;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
@@ -41,10 +40,8 @@ public class H2CacheBasedWebSession extends CacheBasedWebSession {
       protected void configure() {
         persist(WebSessionManager.CACHE_NAME, String.class, Val.class)
             .maximumWeight(1024) // reasonable default for many sites
-            .expireAfterWrite(
-                CacheBasedWebSession.MAX_AGE_MINUTES,
-                MINUTES) // expire sessions if they are inactive
-        ;
+            // expire sessions if they are inactive
+            .expireAfterWrite(Duration.ofMinutes(CacheBasedWebSession.MAX_AGE_MINUTES));
         install(new FactoryModuleBuilder().build(WebSessionManagerFactory.class));
         DynamicItem.itemOf(binder(), WebSession.class);
         DynamicItem.bind(binder(), WebSession.class)

java/com/google/gerrit/server/auth/ldap/LdapModule.java

@@ -14,8 +14,6 @@
 
 package com.google.gerrit.server.auth.ldap;
 
-import static java.util.concurrent.TimeUnit.HOURS;
-
 import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.reviewdb.client.Account;
@@ -25,6 +23,7 @@ import com.google.gerrit.server.account.Realm;
 import com.google.gerrit.server.cache.CacheModule;
 import com.google.inject.Scopes;
 import com.google.inject.TypeLiteral;
+import java.time.Duration;
 import java.util.Optional;
 import java.util.Set;
 
@@ -37,18 +36,18 @@ public class LdapModule extends CacheModule {
   @Override
   protected void configure() {
     cache(GROUP_CACHE, String.class, new TypeLiteral<Set<AccountGroup.UUID>>() {})
-        .expireAfterWrite(1, HOURS)
+        .expireAfterWrite(Duration.ofHours(1))
         .loader(LdapRealm.MemberLoader.class);
 
     cache(USERNAME_CACHE, String.class, new TypeLiteral<Optional<Account.Id>>() {})
         .loader(LdapRealm.UserLoader.class);
 
     cache(GROUP_EXIST_CACHE, String.class, new TypeLiteral<Boolean>() {})
-        .expireAfterWrite(1, HOURS)
+        .expireAfterWrite(Duration.ofHours(1))
         .loader(LdapRealm.ExistenceLoader.class);
 
     cache(PARENT_GROUPS_CACHE, String.class, new TypeLiteral<ImmutableSet<String>>() {})
-        .expireAfterWrite(1, HOURS);
+        .expireAfterWrite(Duration.ofHours(1));
 
     bind(Helper.class);
     bind(Realm.class).to(LdapRealm.class).in(Scopes.SINGLETON);

java/com/google/gerrit/server/cache/CacheBinding.java

@@ -16,7 +16,7 @@ package com.google.gerrit.server.cache;
 
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.Weigher;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 /** Configure a cache declared within a {@link CacheModule} instance. */
 public interface CacheBinding<K, V> {
@@ -24,7 +24,7 @@ public interface CacheBinding<K, V> {
   CacheBinding<K, V> maximumWeight(long weight);
 
   /** Set the time an element lives before being expired. */
-  CacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits);
+  CacheBinding<K, V> expireAfterWrite(Duration duration);
 
   /** Populate the cache with items from the CacheLoader. */
   CacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz);

java/com/google/gerrit/server/cache/CacheDef.java

@@ -18,7 +18,7 @@ import com.google.common.cache.CacheLoader;
 import com.google.common.cache.Weigher;
 import com.google.gerrit.common.Nullable;
 import com.google.inject.TypeLiteral;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 public interface CacheDef<K, V> {
   /**
@@ -45,7 +45,7 @@ public interface CacheDef<K, V> {
   long maximumWeight();
 
   @Nullable
-  Long expireAfterWrite(TimeUnit unit);
+  Duration expireAfterWrite();
 
   @Nullable
   Weigher<K, V> weigher();

java/com/google/gerrit/server/cache/CacheProvider.java

@@ -16,7 +16,6 @@ package com.google.gerrit.server.cache;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
-import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.base.Strings;
 import com.google.common.cache.Cache;
@@ -27,7 +26,7 @@ import com.google.gerrit.extensions.annotations.PluginName;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>, CacheDef<K, V> {
   private final CacheModule module;
@@ -36,7 +35,7 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
   private final TypeLiteral<V> valType;
   private String configKey;
   private long maximumWeight;
-  private Long expireAfterWrite;
+  private Duration expireAfterWrite;
   private Provider<CacheLoader<K, V>> loader;
   private Provider<Weigher<K, V>> weigher;
 
@@ -69,9 +68,9 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
   }
 
   @Override
-  public CacheBinding<K, V> expireAfterWrite(long duration, TimeUnit unit) {
+  public CacheBinding<K, V> expireAfterWrite(Duration duration) {
     checkNotFrozen();
-    expireAfterWrite = SECONDS.convert(duration, unit);
+    expireAfterWrite = duration;
     return this;
   }
 
@@ -126,8 +125,8 @@ class CacheProvider<K, V> implements Provider<Cache<K, V>>, CacheBinding<K, V>,
 
   @Override
   @Nullable
-  public Long expireAfterWrite(TimeUnit unit) {
-    return expireAfterWrite != null ? unit.convert(expireAfterWrite, SECONDS) : null;
+  public Duration expireAfterWrite() {
+    return expireAfterWrite;
   }
 
   @Override

java/com/google/gerrit/server/cache/PersistentCacheBinding.java

@@ -16,7 +16,7 @@ package com.google.gerrit.server.cache;
 
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.Weigher;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 /** Configure a persistent cache declared within a {@link CacheModule} instance. */
 public interface PersistentCacheBinding<K, V> extends CacheBinding<K, V> {
@@ -24,7 +24,7 @@ public interface PersistentCacheBinding<K, V> extends CacheBinding<K, V> {
   PersistentCacheBinding<K, V> maximumWeight(long weight);
 
   @Override
-  PersistentCacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits);
+  PersistentCacheBinding<K, V> expireAfterWrite(Duration duration);
 
   @Override
   PersistentCacheBinding<K, V> loader(Class<? extends CacheLoader<K, V>> clazz);

java/com/google/gerrit/server/cache/PersistentCacheProvider.java

@@ -24,7 +24,7 @@ import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.TypeLiteral;
 import java.io.Serializable;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
     implements Provider<Cache<K, V>>, PersistentCacheBinding<K, V>, PersistentCacheDef<K, V> {
@@ -53,8 +53,8 @@ class PersistentCacheProvider<K, V> extends CacheProvider<K, V>
   }
 
   @Override
-  public PersistentCacheBinding<K, V> expireAfterWrite(long duration, TimeUnit durationUnits) {
-    return (PersistentCacheBinding<K, V>) super.expireAfterWrite(duration, durationUnits);
+  public PersistentCacheBinding<K, V> expireAfterWrite(Duration duration) {
+    return (PersistentCacheBinding<K, V>) super.expireAfterWrite(duration);
   }
 
   @Override

java/com/google/gerrit/server/cache/h2/BUILD

@@ -3,6 +3,7 @@ java_library(
     srcs = glob(["**/*.java"]),
     visibility = ["//visibility:public"],
     deps = [
+        "//java/com/google/gerrit/common:annotations",
         "//java/com/google/gerrit/common:server",
         "//java/com/google/gerrit/extensions:api",
         "//java/com/google/gerrit/lifecycle",

java/com/google/gerrit/server/cache/h2/H2CacheDefProxy.java

@@ -16,11 +16,12 @@ package com.google.gerrit.server.cache.h2;
 
 import com.google.common.cache.CacheLoader;
 import com.google.common.cache.Weigher;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.server.cache.CacheSerializer;
 import com.google.gerrit.server.cache.PersistentCacheDef;
 import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
 import com.google.inject.TypeLiteral;
-import java.util.concurrent.TimeUnit;
+import java.time.Duration;
 
 class H2CacheDefProxy<K, V> implements PersistentCacheDef<K, V> {
   private final PersistentCacheDef<K, V> source;
@@ -30,8 +31,9 @@ class H2CacheDefProxy<K, V> implements PersistentCacheDef<K, V> {
   }
 
   @Override
-  public Long expireAfterWrite(TimeUnit unit) {
-    return source.expireAfterWrite(unit);
+  @Nullable
+  public Duration expireAfterWrite() {
+    return source.expireAfterWrite();
   }
 
   @SuppressWarnings("unchecked")

java/com/google/gerrit/server/cache/h2/H2CacheFactory.java

@@ -215,7 +215,6 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
     if (h2AutoServer) {
       url.append(";AUTO_SERVER=TRUE");
     }
-    Long expireAfterWrite = def.expireAfterWrite(TimeUnit.SECONDS);
     return new SqlStore<>(
         url.toString(),
         def.keyType(),
@@ -223,6 +222,6 @@ class H2CacheFactory implements PersistentCacheFactory, LifecycleListener {
         def.valueSerializer(),
         def.version(),
         maxSize,
-        expireAfterWrite == null ? 0 : expireAfterWrite.longValue());
+        def.expireAfterWrite());
   }
 }

java/com/google/gerrit/server/cache/h2/H2CacheImpl.java

@@ -23,6 +23,7 @@ import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.hash.BloomFilter;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.server.cache.CacheSerializer;
 import com.google.gerrit.server.cache.PersistentCache;
@@ -35,6 +36,7 @@ import java.sql.ResultSet;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.sql.Timestamp;
+import java.time.Duration;
 import java.util.Calendar;
 import java.util.Map;
 import java.util.concurrent.ArrayBlockingQueue;
@@ -254,7 +256,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
     private final CacheSerializer<V> valueSerializer;
     private final int version;
     private final long maxSize;
-    private final long expireAfterWrite;
+    @Nullable private final Duration expireAfterWrite;
     private final BlockingQueue<SqlHandle> handles;
     private final AtomicLong hitCount = new AtomicLong();
     private final AtomicLong missCount = new AtomicLong();
@@ -268,7 +270,7 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
         CacheSerializer<V> valueSerializer,
         int version,
         long maxSize,
-        long expireAfterWrite) {
+        @Nullable Duration expireAfterWrite) {
       this.url = jdbcUrl;
       this.keyType = createKeyType(keyType, keySerializer);
       this.valueSerializer = valueSerializer;
@@ -421,11 +423,11 @@ public class H2CacheImpl<K, V> extends AbstractLoadingCache<K, V> implements Per
     }
 
     private boolean expired(Timestamp created) {
-      if (expireAfterWrite == 0) {
+      if (expireAfterWrite == null) {
         return false;
       }
-      long age = TimeUtil.nowMs() - created.getTime();
-      return 1000 * expireAfterWrite < age;
+      Duration age = Duration.between(created.toInstant(), TimeUtil.now());
+      return age.compareTo(expireAfterWrite) > 0;
     }
 
     private void touch(SqlHandle c, K key) throws IOException, SQLException {

java/com/google/gerrit/server/cache/mem/DefaultMemoryCacheFactory.java

@@ -26,6 +26,7 @@ import com.google.gerrit.server.cache.MemoryCacheFactory;
 import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
+import java.time.Duration;
 import java.util.concurrent.TimeUnit;
 import org.eclipse.jgit.lib.Config;
 
@@ -66,14 +67,19 @@ class DefaultMemoryCacheFactory implements MemoryCacheFactory {
     }
     builder.weigher(weigher);
 
-    Long age = def.expireAfterWrite(TimeUnit.SECONDS);
+    Duration age = def.expireAfterWrite();
     if (has(def.configKey(), "maxAge")) {
       builder.expireAfterWrite(
           ConfigUtil.getTimeUnit(
-              cfg, "cache", def.configKey(), "maxAge", age != null ? age : 0, TimeUnit.SECONDS),
+              cfg,
+              "cache",
+              def.configKey(),
+              "maxAge",
+              age != null ? age.getSeconds() : 0,
+              TimeUnit.SECONDS),
           TimeUnit.SECONDS);
     } else if (age != null) {
-      builder.expireAfterWrite(age, TimeUnit.SECONDS);
+      builder.expireAfterWrite(age.toNanos(), TimeUnit.NANOSECONDS);
     }
 
     return builder;

javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java

@@ -47,7 +47,7 @@ public class H2CacheTest {
             StringSerializer.INSTANCE,
             version,
             1 << 20,
-            0);
+            null);
     return new H2CacheImpl<>(MoreExecutors.directExecutor(), store, KEY_TYPE, mem);
   }