Refactor plugin code base - The rateLimit class stores timeLapse as an integer. PeriodicRateLimiter stores timeLapse as Long, which confuses understanding and using the code. This change converts all timeLapse variables to an integer. - Remove timeLapse from warning rate limiters, since timeLapse can be accesed from delegate - Add methods getWarnLimit and getTimelapse Change-Id: I0bb96c3c2309ee911a667d138992613d3cf36d67

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java
index 1e1f7d7..db9c2e5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Configuration.java

@@ -143,7 +143,7 @@
    * @param rateLimitType type of rate limit
    * @return map of rate limits per group uuid
    */
-  Map<AccountGroup.UUID, RateLimit> getRatelimits(RateLimitType rateLimitType) {
+  Map<AccountGroup.UUID, RateLimit> getRateLimits(RateLimitType rateLimitType) {
     return rateLimits != null ? rateLimits.row(rateLimitType) : ImmutableMap.of();
   }
 

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
index 5597723..536a3d7 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/Module.java

@@ -107,9 +107,9 @@
         rateLimitType = limit.get().getType().getLimitType();
       }
 
-      long effectiveTimeLapse = PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES;
+      int effectiveTimeLapse = PeriodicRateLimiter.DEFAULT_TIME_LAPSE_IN_MINUTES;
       if (timeLapse.isPresent()) {
-        long providedTimeLapse = timeLapse.get().getRatePerHour();
+        int providedTimeLapse = timeLapse.get().getRatePerHour();
         if (providedTimeLapse > 0 && providedTimeLapse <= effectiveTimeLapse) {
           effectiveTimeLapse = providedTimeLapse;
           rateLimitType = timeLapse.get().getType().getLimitType();
@@ -124,11 +124,10 @@
 
       if (warn.isPresent()) {
         if (limit.isPresent()) {
-          return warningRateLimiterFactory.create(
-              rateLimiter, key, warn.get().getRatePerHour(), effectiveTimeLapse);
+          return warningRateLimiterFactory.create(rateLimiter, key, warn.get().getRatePerHour());
         }
         return warningUnlimitedRateLimiterFactory.create(
-            rateLimiter, key, warn.get().getRatePerHour(), effectiveTimeLapse);
+            rateLimiter, key, warn.get().getRatePerHour());
       }
       return rateLimiter;
     }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
index 5c5ced3..0e99d83 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiter.java

@@ -16,6 +16,7 @@
 
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
+import java.util.Optional;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.Semaphore;
@@ -23,28 +24,33 @@
 import java.util.concurrent.atomic.AtomicInteger;
 
 class PeriodicRateLimiter implements RateLimiter {
-  static final long DEFAULT_TIME_LAPSE_IN_MINUTES = 60L;
+  static final int DEFAULT_TIME_LAPSE_IN_MINUTES = 60;
 
   private final Semaphore semaphore;
   private final int maxPermits;
   private final AtomicInteger usedPermits;
   private final ScheduledFuture<?> replenishTask;
   private final String rateLimitType;
+  private final int timeLapse;
 
   interface Factory {
-    PeriodicRateLimiter create(int permits, long timeLapse, String rateLimitType);
+    PeriodicRateLimiter create(
+        @Assisted("permits") Integer permits,
+        @Assisted("timeLapse") Integer timeLapse,
+        String rateLimitType);
   }
 
   @Inject
   PeriodicRateLimiter(
       @RateLimitExecutor ScheduledExecutorService executor,
-      @Assisted int permits,
-      @Assisted long timeLapse,
+      @Assisted("permits") Integer permits,
+      @Assisted("timeLapse") Integer timeLapse,
       @Assisted String rateLimitType) {
     this.semaphore = new Semaphore(permits);
     this.maxPermits = permits;
     this.usedPermits = new AtomicInteger();
     this.rateLimitType = rateLimitType;
+    this.timeLapse = timeLapse;
     this.replenishTask =
         executor.scheduleAtFixedRate(
             this::replenishPermits, timeLapse, timeLapse, TimeUnit.MINUTES);
@@ -91,6 +97,16 @@
   }
 
   @Override
+  public Optional<Integer> getTimeLapse() {
+    return Optional.of(timeLapse);
+  }
+
+  @Override
+  public Optional<Integer> getWarnLimit() {
+    return Optional.empty();
+  }
+
+  @Override
   public void close() {
     replenishTask.cancel(true);
   }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitFinder.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitFinder.java
index 4375f1a..0a9d8a0 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitFinder.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimitFinder.java

@@ -57,7 +57,7 @@
    */
   private Optional<RateLimit> firstMatching(RateLimitType rateLimitType, IdentifiedUser user) {
     Map<AccountGroup.UUID, RateLimit> limitsPerGroupUUID =
-        configuration.getRatelimits(rateLimitType);
+        configuration.getRateLimits(rateLimitType);
     if (!limitsPerGroupUUID.isEmpty()) {
       GroupMembership memberShip = user.getEffectiveGroups();
       for (Entry<AccountGroup.UUID, RateLimit> limitPerGroupUUID : limitsPerGroupUUID.entrySet()) {
@@ -76,7 +76,7 @@
    */
   private Optional<RateLimit> getRateLimit(
       RateLimitType rateLimitType, AccountGroup.UUID groupUUID) {
-    Map<AccountGroup.UUID, RateLimit> limits = configuration.getRatelimits(rateLimitType);
+    Map<AccountGroup.UUID, RateLimit> limits = configuration.getRateLimits(rateLimitType);
     return limits.isEmpty() ? Optional.empty() : Optional.ofNullable(limits.get(groupUUID));
   }
 }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java
index d5455b1..b9943bf 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/RateLimiter.java

@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.ratelimiter;
 
 import java.util.Comparator;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 interface RateLimiter extends Comparable<RateLimiter> {
@@ -49,6 +50,12 @@
   /** Return type of rate limiter * */
   String getType();
 
+  /** Return time lapse of rate limiter. */
+  Optional<Integer> getTimeLapse();
+
+  /** Return warning limit of rate limiter. */
+  Optional<Integer> getWarnLimit();
+
   /** Closes this RateLimiter, relinquishing any underlying resources. */
   void close();
 }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java
index d1989f4..8d4f60e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/UnlimitedRateLimiter.java

@@ -16,6 +16,7 @@
 
 import static com.googlesource.gerrit.plugins.ratelimiter.Module.DEFAULT_RATE_LIMIT_TYPE;
 
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 
 class UnlimitedRateLimiter implements RateLimiter {
@@ -55,6 +56,16 @@
   }
 
   @Override
+  public Optional<Integer> getTimeLapse() {
+    return Optional.empty();
+  }
+
+  @Override
+  public Optional<Integer> getWarnLimit() {
+    return Optional.empty();
+  }
+
+  @Override
   public int usedPermits() {
     return 0;
   }

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java
index 68cc5ee..bdd8307 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiter.java

@@ -20,13 +20,14 @@
 import com.google.inject.assistedinject.Assisted;
 import java.time.LocalTime;
 import java.time.format.DateTimeFormatter;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import org.slf4j.Logger;
 
 class WarningRateLimiter implements RateLimiter {
   @FunctionalInterface
   interface Factory {
-    WarningRateLimiter create(RateLimiter delegate, String key, int warnLimit, long timeLapse);
+    WarningRateLimiter create(RateLimiter delegate, String key, int warnLimit);
   }
 
   private static final Logger rateLimitLog = RateLimiterStatsLog.getLogger();
@@ -37,7 +38,6 @@
   private final RateLimitReachedSender.Factory rateLimitReachedSenderFactory;
   private final int warnLimit;
   private final String key;
-  private final long timeLapse;
 
   private volatile boolean wasLogged;
   private volatile boolean warningWasLogged = false;
@@ -48,14 +48,12 @@
       RateLimitReachedSender.Factory rateLimitReachedSenderFactory,
       @Assisted RateLimiter delegate,
       @Assisted String key,
-      @Assisted int warnLimit,
-      @Assisted long timeLapse) {
+      @Assisted int warnLimit) {
     this.userResolver = userResolver;
     this.delegate = delegate;
     this.rateLimitReachedSenderFactory = rateLimitReachedSenderFactory;
     this.warnLimit = warnLimit;
     this.key = key;
-    this.timeLapse = timeLapse;
   }
 
   @Override
@@ -70,7 +68,7 @@
       String emailMessage =
           String.format(
               "User %s reached the warning limit of %s %s per %s minutes.",
-              userResolver.getUserName(key).orElse(key), warnLimit, delegate.getType(), timeLapse);
+              userResolver.getUserName(key).orElse(key), warnLimit, delegate.getType(), delegate.getTimeLapse());
       rateLimitLog.info(emailMessage);
       warningWasLogged = true;
       sendEmail(key, emailMessage, acquirePermit);
@@ -83,7 +81,7 @@
               userResolver.getUserName(key).orElse(key),
               permitsPerHour(),
               delegate.getType(),
-              timeLapse,
+              delegate.getTimeLapse(),
               secondsToMsSs(remainingTime(TimeUnit.SECONDS)));
       rateLimitLog.info(emailMessage);
       wasLogged = true;
@@ -124,6 +122,11 @@
   }
 
   @Override
+  public Optional<Integer> getTimeLapse() {
+    return delegate.getTimeLapse();
+  }
+
+  @Override
   public String getType() {
     return delegate.getType();
   }
@@ -142,6 +145,11 @@
     return LocalTime.MIN.plusSeconds(seconds).format(format);
   }
 
+  @Override
+  public Optional<Integer> getWarnLimit() {
+    return Optional.of(warnLimit);
+  }
+
   @VisibleForTesting
   public boolean getWarningFlagState() {
     return warningWasLogged;

diff --git a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java
index 7ce5be1..abf8966 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiter.java

@@ -17,14 +17,14 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import org.slf4j.Logger;
 
 class WarningUnlimitedRateLimiter implements RateLimiter {
   @FunctionalInterface
   interface Factory {
-    WarningUnlimitedRateLimiter create(
-        RateLimiter delegate, String key, int warnLimit, long timeLapse);
+    WarningUnlimitedRateLimiter create(RateLimiter delegate, String key, int warnLimit);
   }
 
   private static final Logger rateLimitLog = RateLimiterStatsLog.getLogger();
@@ -33,7 +33,6 @@
   private final RateLimiter delegate;
   private final int warnLimit;
   private final String key;
-  private final long timeLapse;
   private volatile boolean warningWasLogged = false;
 
   @Inject
@@ -41,13 +40,11 @@
       UserResolver userResolver,
       @Assisted RateLimiter delegate,
       @Assisted String key,
-      @Assisted int warnLimit,
-      @Assisted long timeLapse) {
+      @Assisted int warnLimit) {
     this.userResolver = userResolver;
     this.delegate = delegate;
     this.warnLimit = warnLimit;
     this.key = key;
-    this.timeLapse = timeLapse;
   }
 
   @Override
@@ -65,7 +62,7 @@
           userResolver.getUserName(key).orElse(key),
           warnLimit,
           delegate.getType(),
-          timeLapse);
+          delegate.getTimeLapse());
       warningWasLogged = true;
     }
     return acquirePermit;
@@ -93,6 +90,11 @@
   }
 
   @Override
+  public Optional<Integer> getTimeLapse() {
+    return delegate.getTimeLapse();
+  }
+
+  @Override
   public int usedPermits() {
     return delegate.usedPermits();
   }
@@ -102,6 +104,11 @@
     delegate.close();
   }
 
+  @Override
+  public Optional<Integer> getWarnLimit() {
+    return Optional.of(warnLimit);
+  }
+
   @VisibleForTesting
   public boolean getWarningFlagState() {
     return warningWasLogged;

diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java
index a4046b3..9c08d11 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/ConfigurationTest.java

@@ -65,7 +65,7 @@
 
   @Test
   public void testEmptyConfig() {
-    assertThat(getConfiguration().getRatelimits(RateLimitType.UPLOAD_PACK_PER_HOUR)).isEmpty();
+    assertThat(getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR)).isEmpty();
   }
 
   @Test
@@ -77,7 +77,7 @@
         validRate);
 
     Map<AccountGroup.UUID, RateLimit> rateLimit =
-        getConfiguration().getRatelimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+        getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
     assertThat(rateLimit).hasSize(1);
     assertThat(rateLimit.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
         .isEqualTo(validRate);
@@ -129,7 +129,7 @@
         "badGroup");
 
     Map<AccountGroup.UUID, RateLimit> rateLimit =
-        getConfiguration().getRatelimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+        getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
     assertThat(rateLimit).hasSize(1);
     assertThat(rateLimit.get(someGroupDescMock.getGroupUUID()).getRatePerHour())
         .isEqualTo(validRate);
@@ -140,7 +140,7 @@
     globalPluginConfig.fromText("[group \"Administrators\"]");
 
     Map<AccountGroup.UUID, RateLimit> rateLimit =
-        getConfiguration().getRatelimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
+        getConfiguration().getRateLimits(RateLimitType.UPLOAD_PACK_PER_HOUR);
     assertThat(rateLimit).hasSize(1);
     assertThat(rateLimit.get(administratorsGroupDescMock.getGroupUUID())).isNull();
   }

diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java
index b949490..15ad0d8 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/PeriodicRateLimiterTest.java

@@ -71,8 +71,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             any(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
   }
 
@@ -82,8 +82,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             runnableCaptor.capture(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
 
     // Use all permits

diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java
index dc19df0..f03e745 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningRateLimiterTest.java

@@ -63,8 +63,8 @@
             rateLimitReachedSenderFactory,
             limiter,
             "dummy",
-            WARN_RATE,
-            DEFAULT_TIME_LAPSE_IN_MINUTES);
+            WARN_RATE
+            );
   }
 
   @Test
@@ -73,6 +73,11 @@
   }
 
   @Test
+  public void testGetWarningLimit() {
+    assertThat(warningLimiter.getWarnLimit().get()).isEqualTo(WARN_RATE);
+  }
+
+  @Test
   public void testAcquireAll() {
     assertThat(warningLimiter.availablePermits()).isEqualTo(RATE);
 
@@ -111,8 +116,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             any(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
   }
 
@@ -122,8 +127,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             runnableCaptor.capture(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
 
     replenishPermits(warningLimiter, runnableCaptor);

diff --git a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java
index 7717234..7ed1d93 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/ratelimiter/WarningUnlimitedRateLimiterTest.java

@@ -42,8 +42,7 @@
         new PeriodicRateLimiter(
             scheduledExecutorMock, RATE, DEFAULT_TIME_LAPSE_IN_MINUTES, "Any Type");
     warningUnlimitedLimiter =
-        new WarningUnlimitedRateLimiter(
-            userResolver, limiter, "dummy", WARN_RATE, DEFAULT_TIME_LAPSE_IN_MINUTES);
+        new WarningUnlimitedRateLimiter(userResolver, limiter, "dummy", WARN_RATE);
   }
 
   @Test
@@ -75,8 +74,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             any(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
   }
 
@@ -86,8 +85,8 @@
     verify(scheduledExecutorMock)
         .scheduleAtFixedRate(
             runnableCaptor.capture(),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
-            eq(DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
+            eq((long) DEFAULT_TIME_LAPSE_IN_MINUTES),
             eq(TimeUnit.MINUTES));
 
     testTriggerWarning();