Refactored rate-limiting.

If rate-limiting is disabled, we now use a simple stub implementation
that is simpler to reason with.

proxy/src/main/java/com/velocitypowered/proxy/VelocityServer.java

@@ -34,8 +34,9 @@ import com.velocitypowered.proxy.scheduler.VelocityScheduler;
 import com.velocitypowered.proxy.server.ServerMap;
 import com.velocitypowered.proxy.util.AddressUtil;
 import com.velocitypowered.proxy.util.EncryptionUtils;
-import com.velocitypowered.proxy.util.Ratelimiter;
 import com.velocitypowered.proxy.util.VelocityChannelRegistrar;
+import com.velocitypowered.proxy.util.ratelimit.Ratelimiter;
+import com.velocitypowered.proxy.util.ratelimit.Ratelimiters;
 import io.netty.bootstrap.Bootstrap;
 import java.net.InetSocketAddress;
 import java.nio.file.Files;
@@ -166,7 +167,7 @@ public class VelocityServer implements ProxyServer {
       servers.register(new ServerInfo(entry.getKey(), AddressUtil.parseAddress(entry.getValue())));
     }
 
-    ipAttemptLimiter = new Ratelimiter(configuration.getLoginRatelimit());
+    ipAttemptLimiter = Ratelimiters.createWithMilliseconds(configuration.getLoginRatelimit());
     httpClient = new NettyHttpClient(this);
     loadPlugins();
 

proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiter.java

@@ -1,6 +1,7 @@
-package com.velocitypowered.proxy.util;
+package com.velocitypowered.proxy.util.ratelimit;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Preconditions;
 import com.google.common.base.Ticker;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
@@ -11,29 +12,26 @@ import java.util.concurrent.TimeUnit;
 /**
  * A simple rate-limiter based on a Guava {@link Cache}.
  */
-public class Ratelimiter {
+public class GuavaCacheRatelimiter implements Ratelimiter {
 
   private final Cache<InetAddress, Long> expiringCache;
   private final long timeoutNanos;
 
-  public Ratelimiter(long timeoutMs) {
+  GuavaCacheRatelimiter(long time, TimeUnit unit) {
-    this(timeoutMs, Ticker.systemTicker());
+    this(time, unit, Ticker.systemTicker());
   }
 
   @VisibleForTesting
-  Ratelimiter(long timeoutMs, Ticker ticker) {
+  GuavaCacheRatelimiter(long time, TimeUnit unit, Ticker ticker) {
-    if (timeoutMs == 0) {
+    Preconditions.checkNotNull(unit, "unit");
-      this.timeoutNanos = timeoutMs;
+    Preconditions.checkNotNull(ticker, "ticker");
-      this.expiringCache = CacheBuilder.newBuilder().maximumSize(0).build();
+    this.timeoutNanos = unit.toNanos(time);
-    } else {
-      this.timeoutNanos = TimeUnit.MILLISECONDS.toNanos(timeoutMs);
     this.expiringCache = CacheBuilder.newBuilder()
         .ticker(ticker)
         .concurrencyLevel(Runtime.getRuntime().availableProcessors())
-          .expireAfterWrite(timeoutMs, TimeUnit.MILLISECONDS)
+        .expireAfterWrite(time, unit)
         .build();
   }
-  }
 
   /**
    * Attempts to rate-limit the client.
@@ -41,10 +39,9 @@ public class Ratelimiter {
    * @param address the address to rate limit
    * @return true if we should allow the client, false if we should rate-limit
    */
+  @Override
   public boolean attempt(InetAddress address) {
-    if (timeoutNanos == 0) {
+    Preconditions.checkNotNull(address, "address");
-      return true;
-    }
     long expectedNewValue = System.nanoTime() + timeoutNanos;
     long last;
     try {

proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/NoopCacheRatelimiter.java

@@ -0,0 +1,15 @@
+package com.velocitypowered.proxy.util.ratelimit;
+
+import java.net.InetAddress;
+
+/**
+ * A {@link Ratelimiter} that does no rate-limiting.
+ */
+enum NoopCacheRatelimiter implements Ratelimiter {
+  INSTANCE;
+
+  @Override
+  public boolean attempt(InetAddress address) {
+    return true;
+  }
+}

proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiter.java

@@ -0,0 +1,16 @@
+package com.velocitypowered.proxy.util.ratelimit;
+
+import java.net.InetAddress;
+
+/**
+ * Allows rate limiting of clients.
+ */
+public interface Ratelimiter {
+
+  /**
+   * Determines whether or not to allow the connection.
+   * @param address the address to rate limit
+   * @return true if allowed, false if not
+   */
+  boolean attempt(InetAddress address);
+}

proxy/src/main/java/com/velocitypowered/proxy/util/ratelimit/Ratelimiters.java

@@ -0,0 +1,14 @@
+package com.velocitypowered.proxy.util.ratelimit;
+
+import java.util.concurrent.TimeUnit;
+
+public final class Ratelimiters {
+  private Ratelimiters() {
+    throw new AssertionError();
+  }
+
+  public static Ratelimiter createWithMilliseconds(long ms) {
+    return ms <= 0 ? NoopCacheRatelimiter.INSTANCE : new GuavaCacheRatelimiter(ms,
+        TimeUnit.MILLISECONDS);
+  }
+}

proxy/src/test/java/com/velocitypowered/proxy/util/ratelimit/GuavaCacheRatelimiterTest.java

@@ -1,4 +1,4 @@
-package com.velocitypowered.proxy.util;
+package com.velocitypowered.proxy.util.ratelimit;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -9,11 +9,11 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import org.junit.jupiter.api.Test;
 
-class RatelimiterTest {
+class GuavaCacheRatelimiterTest {
 
   @Test
   void attemptZero() {
-    Ratelimiter noRatelimiter = new Ratelimiter(0);
+    Ratelimiter noRatelimiter = new GuavaCacheRatelimiter(0, TimeUnit.MILLISECONDS);
     assertTrue(noRatelimiter.attempt(InetAddress.getLoopbackAddress()));
     assertTrue(noRatelimiter.attempt(InetAddress.getLoopbackAddress()));
   }
@@ -28,7 +28,7 @@ class RatelimiterTest {
         return base + extra.get();
       }
     };
-    Ratelimiter ratelimiter = new Ratelimiter(1000, testTicker);
+    Ratelimiter ratelimiter = new GuavaCacheRatelimiter(1000, TimeUnit.MILLISECONDS, testTicker);
     assertTrue(ratelimiter.attempt(InetAddress.getLoopbackAddress()));
     assertFalse(ratelimiter.attempt(InetAddress.getLoopbackAddress()));
     extra.addAndGet(TimeUnit.SECONDS.toNanos(2));