Remove internal cache because it's a mess

src/main/java/com/imaginarycode/minecraft/redisbungee/DataManager.java

@@ -1,5 +1,7 @@
 package com.imaginarycode.minecraft.redisbungee;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.google.common.net.InetAddresses;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParser;
@@ -8,7 +10,6 @@ import com.imaginarycode.minecraft.redisbungee.events.PlayerChangedServerNetwork
 import com.imaginarycode.minecraft.redisbungee.events.PlayerJoinedNetworkEvent;
 import com.imaginarycode.minecraft.redisbungee.events.PlayerLeftNetworkEvent;
 import com.imaginarycode.minecraft.redisbungee.events.PubSubMessageEvent;
-import com.imaginarycode.minecraft.redisbungee.util.InternalCache;
 import lombok.Getter;
 import lombok.RequiredArgsConstructor;
 import net.md_5.bungee.api.connection.ProxiedPlayer;
@@ -19,6 +20,7 @@ import net.md_5.bungee.event.EventHandler;
 import redis.clients.jedis.Jedis;
 
 import java.net.InetAddress;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -33,29 +35,20 @@ import java.util.logging.Level;
 public class DataManager implements Listener {
     private final RedisBungee plugin;
     // TODO: Add cleanup for this.
-    private final InternalCache<UUID, String> serverCache = createCache();
+    private final Cache<UUID, String> serverCache = createCache();
-    private final InternalCache<UUID, String> proxyCache = createCache(TimeUnit.MINUTES.toMillis(60));
+    private final Cache<UUID, String> proxyCache = createCache();
-    private final InternalCache<UUID, InetAddress> ipCache = createCache(TimeUnit.MINUTES.toMillis(60));
+    private final Cache<UUID, InetAddress> ipCache = createCache();
-    private final InternalCache<UUID, Long> lastOnlineCache = createCache(TimeUnit.MINUTES.toMillis(60));
+    private final Cache<UUID, Long> lastOnlineCache = createCache();
 
     public DataManager(RedisBungee plugin) {
         this.plugin = plugin;
-        plugin.getProxy().getScheduler().schedule(plugin, new Runnable() {
-            @Override
-            public void run() {
-                proxyCache.cleanup();
-                ipCache.cleanup();
-                lastOnlineCache.cleanup();
-            }
-        }, 1, 1, TimeUnit.MINUTES);
     }
 
-    private static <K, V> InternalCache<K, V> createCache() {
+    private static <K, V> Cache<K, V> createCache() {
-        return new InternalCache<>();
+        return CacheBuilder.newBuilder()
-    }
+                .maximumSize(1000)
-
+                .expireAfterWrite(1, TimeUnit.HOURS)
-    private static <K, V> InternalCache<K, V> createCache(long entryWriteExpiry) {
+                .build();
-        return new InternalCache<>(entryWriteExpiry);
     }
 
     private final JsonParser parser = new JsonParser();
@@ -71,7 +64,7 @@ public class DataManager implements Listener {
                 @Override
                 public String call() throws Exception {
                     try (Jedis tmpRsc = plugin.getPool().getResource()) {
-                        return tmpRsc.hget("player:" + uuid, "server");
+                        return Objects.requireNonNull(tmpRsc.hget("player:" + uuid, "server"), "user not found");
                     }
                 }
             });
@@ -92,11 +85,13 @@ public class DataManager implements Listener {
                 @Override
                 public String call() throws Exception {
                     try (Jedis tmpRsc = plugin.getPool().getResource()) {
-                        return tmpRsc.hget("player:" + uuid, "proxy");
+                        return Objects.requireNonNull(tmpRsc.hget("player:" + uuid, "proxy"), "user not found");
                     }
                 }
             });
         } catch (ExecutionException e) {
+            if (e.getCause() instanceof NullPointerException && e.getCause().getMessage().equals("user not found"))
+                return null; // HACK
             plugin.getLogger().log(Level.SEVERE, "Unable to get proxy", e);
             throw new RuntimeException("Unable to get proxy for " + uuid, e);
         }
@@ -114,11 +109,15 @@ public class DataManager implements Listener {
                 public InetAddress call() throws Exception {
                     try (Jedis tmpRsc = plugin.getPool().getResource()) {
                         String result = tmpRsc.hget("player:" + uuid, "ip");
-                        return result == null ? null : InetAddresses.forString(result);
+                        if (result == null)
+                            throw new NullPointerException("user not found");
+                        return InetAddresses.forString(result);
                     }
                 }
             });
         } catch (ExecutionException e) {
+            if (e.getCause() instanceof NullPointerException && e.getCause().getMessage().equals("user not found"))
+                return null; // HACK
             plugin.getLogger().log(Level.SEVERE, "Unable to get IP", e);
             throw new RuntimeException("Unable to get IP for " + uuid, e);
         }
@@ -209,11 +208,11 @@ public class DataManager implements Listener {
             case SERVER_CHANGE:
                 final DataManagerMessage<ServerChangePayload> message3 = RedisBungee.getGson().fromJson(jsonObject, new TypeToken<DataManagerMessage<ServerChangePayload>>() {
                 }.getType());
-                final String oldServer = serverCache.put(message3.getTarget(), message3.getPayload().getServer());
+                serverCache.put(message3.getTarget(), message3.getPayload().getServer());
                 plugin.getProxy().getScheduler().runAsync(plugin, new Runnable() {
                     @Override
                     public void run() {
-                        plugin.getProxy().getPluginManager().callEvent(new PlayerChangedServerNetworkEvent(message3.getTarget(), oldServer, message3.getPayload().getServer()));
+                        plugin.getProxy().getPluginManager().callEvent(new PlayerChangedServerNetworkEvent(message3.getTarget(), message3.getPayload().getOldServer(), message3.getPayload().getServer()));
                     }
                 });
                 break;
@@ -245,6 +244,7 @@ public class DataManager implements Listener {
     @RequiredArgsConstructor
     static class ServerChangePayload {
         private final String server;
+        private final String oldServer;
     }
 
     @Getter

src/main/java/com/imaginarycode/minecraft/redisbungee/RedisBungee.java

@@ -1,5 +1,7 @@
 package com.imaginarycode.minecraft.redisbungee;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.*;
 import com.google.common.io.ByteStreams;
 import com.google.gson.Gson;
@@ -64,7 +66,9 @@ public final class RedisBungee extends Plugin {
     private LuaManager.Script getServerPlayersScript;
 
     private static final Object SERVER_TO_PLAYERS_KEY = new Object();
-    private final InternalCache<Object, Multimap<String, UUID>> serverToPlayersCache = new InternalCache<>(2000);
+    private final Cache<Object, Multimap<String, UUID>> serverToPlayersCache = CacheBuilder.newBuilder()
+            .expireAfterWrite(5, TimeUnit.SECONDS)
+            .build();
 
     /**
      * Fetch the {@link RedisBungeeAPI} object created on plugin start.

src/main/java/com/imaginarycode/minecraft/redisbungee/RedisBungeeListener.java

@@ -120,13 +120,14 @@ public class RedisBungeeListener implements Listener {
 
     @EventHandler
     public void onServerChange(final ServerConnectedEvent event) {
+        final String currentServer = event.getPlayer().getServer() == null ? null : event.getPlayer().getServer().getInfo().getName();
         plugin.getProxy().getScheduler().runAsync(plugin, new RedisCallable<Void>(plugin) {
             @Override
             protected Void call(Jedis jedis) {
                 jedis.hset("player:" + event.getPlayer().getUniqueId().toString(), "server", event.getServer().getInfo().getName());
                 jedis.publish("redisbungee-data", RedisBungee.getGson().toJson(new DataManager.DataManagerMessage<>(
                         event.getPlayer().getUniqueId(), DataManager.DataManagerMessage.Action.SERVER_CHANGE,
-                        new DataManager.ServerChangePayload(event.getServer().getInfo().getName()))));
+                        new DataManager.ServerChangePayload(event.getServer().getInfo().getName(), currentServer))));
                 return null;
             }
         });

src/main/java/com/imaginarycode/minecraft/redisbungee/util/InternalCache.java

@@ -1,77 +0,0 @@
-package com.imaginarycode.minecraft.redisbungee.util;
-
-import lombok.Data;
-
-import java.util.Iterator;
-import java.util.Map;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
-import java.util.concurrent.ExecutionException;
-
-// I would use the Guava cache, but can't because I need a few more properties.
-public class InternalCache<K, V> {
-    private final ConcurrentMap<K, Holder> map = new ConcurrentHashMap<>(128, 0.75f, 4);
-    private final long entryWriteExpiry;
-
-    public InternalCache() {
-        this.entryWriteExpiry = 0;
-    }
-
-    public InternalCache(long entryWriteExpiry) {
-        this.entryWriteExpiry = entryWriteExpiry;
-    }
-
-    public V get(K key, Callable<V> loader) throws ExecutionException {
-        Holder value = map.get(key);
-
-        if (value == null || (entryWriteExpiry > 0 && System.currentTimeMillis() > value.expiry)) {
-            V freshValue;
-
-            try {
-                freshValue = loader.call();
-            } catch (Exception e) {
-                throw new ExecutionException(e);
-            }
-
-            if (freshValue == null)
-                return null;
-
-            map.putIfAbsent(key, value = new Holder(freshValue, System.currentTimeMillis() + entryWriteExpiry));
-        }
-
-        return value.value;
-    }
-
-    public V put(K key, V value) {
-        Holder holder = map.put(key, new Holder(value, System.currentTimeMillis() + entryWriteExpiry));
-
-        if (holder == null)
-            return null;
-
-        return holder.value;
-    }
-
-    public void invalidate(K key) {
-        map.remove(key);
-    }
-
-    // Run periodically to clean up the cache mappings.
-    public void cleanup() {
-        if (entryWriteExpiry <= 0)
-            return;
-
-        long fixedReference = System.currentTimeMillis();
-        for (Iterator<Map.Entry<K, Holder>> it = map.entrySet().iterator(); it.hasNext(); ) {
-            Map.Entry<K, Holder> entry = it.next();
-            if (entry.getValue().expiry <= fixedReference)
-                it.remove();
-        }
-    }
-
-    @Data
-    private class Holder {
-        private final V value;
-        private final long expiry;
-    }
-}

src/test/java/com/imaginarycode/minecraft/redisbungee/test/InternalCacheTest.java

@@ -1,127 +0,0 @@
-package com.imaginarycode.minecraft.redisbungee.test;
-
-import com.imaginarycode.minecraft.redisbungee.util.InternalCache;
-import org.junit.Assert;
-import org.junit.Test;
-
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutionException;
-
-public class InternalCacheTest {
-    @Test
-    public void testNonCached() {
-        InternalCache<String, String> cache = new InternalCache<>();
-        try {
-            Assert.assertEquals("hi", cache.get("hi", new Callable<String>() {
-                @Override
-                public String call() throws Exception {
-                    return "hi";
-                }
-            }));
-        } catch (ExecutionException e) {
-            throw new AssertionError(e);
-        }
-    }
-
-    @Test
-    public void testCached() {
-        InternalCache<String, String> cache = new InternalCache<>();
-        try {
-            Assert.assertEquals("hi", cache.get("hi", new Callable<String>() {
-                @Override
-                public String call() throws Exception {
-                    return "hi";
-                }
-            }));
-            Assert.assertEquals("hi", cache.get("hi", new Callable<String>() {
-                @Override
-                public String call() throws Exception {
-                    Assert.fail("Cache is using loader!");
-                    return null;
-                }
-            }));
-        } catch (ExecutionException e) {
-            throw new AssertionError(e);
-        }
-    }
-
-    @Test
-    public void testWriteExpiry() {
-        final Object one = new Object();
-        InternalCache<String, Object> cache = new InternalCache<>(100); // not very long
-        try {
-            // Successive calls should always work.
-            Assert.assertEquals(one, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    return one;
-                }
-            }));
-            Assert.assertEquals(one, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    Assert.fail("Cache is using loader!");
-                    return null;
-                }
-            }));
-
-            // But try again in a second and a bit:
-            try {
-                Thread.sleep(150);
-            } catch (InterruptedException e) {
-                throw new AssertionError(e);
-            }
-
-            final Object two = new Object();
-            Assert.assertEquals(two, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    return two;
-                }
-            }));
-        } catch (ExecutionException e) {
-            throw new AssertionError(e);
-        }
-    }
-
-    @Test
-    public void testCleanup() {
-        InternalCache<String, Object> cache = new InternalCache<>(10);
-        final Object one = new Object();
-        final Object two = new Object();
-        try {
-            Assert.assertEquals(one, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    return one;
-                }
-            }));
-            try {
-                Thread.sleep(5);
-            } catch (InterruptedException e) {
-                throw new AssertionError(e);
-            }
-            cache.cleanup();
-            Assert.assertEquals(one, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    return two;
-                }
-            }));
-            try {
-                Thread.sleep(5);
-            } catch (InterruptedException e) {
-                throw new AssertionError(e);
-            }
-            cache.cleanup();
-            Assert.assertEquals(two, cache.get("hi", new Callable<Object>() {
-                @Override
-                public Object call() throws Exception {
-                    return two;
-                }
-            }));
-        } catch (ExecutionException e) {
-            throw new AssertionError(e);
-        }
-    }
-}