Rewrite plugin version checker.

This commit removes the old version checker that used the DBO resource
page and replaces it with a custom checker that uses the "legacy" Spigot
resource API. The Spigot API is much more lightweight and doesn't
require any sort of parsing.

The new version checker uses a simple cache, keeping version checks
fresh for up to one hour, reducing the need to go fishing on every op
login. The cache resets on restarts, though, but this is acceptable.

Note that no attempt has been made to ensure correctness on multiple,
consecutive invocations when the cache is stale. If a cache refresh is
initiated, all update checks invoked before the cache refresh has ended
will behave as if no update is available. This is acceptable, because
update checks are non-essential, the time frame is extremely narrow, and
the most common result of an update check is "no updates available",
since the amount of update checks made is vastly greater than the amount
of updates released.

changelog.md

@@ -14,6 +14,7 @@ These changes will (most likely) be included in the next version.
 - It is no longer necessary to have recurrent waves for an arena to work. MobArena automatically creates a "catch all" recurrent wave in case the arena session reaches a wave number that isn't covered by any other wave definitions.
 - Entities outside of the arena can no longer target players, pets, or monsters inside of the arena.
 - Tab completion for `/ma kick` and `/ma restore` now uses actual player names instead of display names.
+- MobArena's internal version checker has been rewritten. It now uses the resource API of Spigot instead of DBO. It's also a lot more lightweight and caches results for up to one hour.
 
 ## [0.104] - 2019-08-08
 - Extended and upgraded potions are now supported in the item syntax by prepending `long_` or `strong_` to the data portion of a potion item (e.g. `potion:strong_instant_heal:1` will yield a Potion of Healing II). Check the wiki for details.

src/main/java/com/garbagemule/MobArena/MobArena.java

@@ -18,7 +18,6 @@ import com.garbagemule.MobArena.signs.ArenaSign;
 import com.garbagemule.MobArena.signs.SignBootstrap;
 import com.garbagemule.MobArena.signs.SignListeners;
 import com.garbagemule.MobArena.things.ThingManager;
-import com.garbagemule.MobArena.util.VersionChecker;
 import com.garbagemule.MobArena.util.config.ConfigUtils;
 import com.garbagemule.MobArena.waves.ability.AbilityManager;
 import net.milkbowl.vault.economy.Economy;
@@ -87,7 +86,6 @@ public class MobArena extends JavaPlugin
         }
         loadsConfigFile = null;
         ConfigurationSerialization.unregisterClass(ArenaSign.class);
-        VersionChecker.shutdown();
     }
 
     private void setup() {
@@ -253,7 +251,7 @@ public class MobArena extends JavaPlugin
 
     private void checkForUpdates() {
         if (getConfig().getBoolean("global-settings.update-notification", false)) {
-            VersionChecker.checkForUpdates(this, null);
+            PluginVersionCheck.check(this, getLogger()::info);
         }
     }
 

src/main/java/com/garbagemule/MobArena/PluginVersionCheck.java

@@ -0,0 +1,136 @@
+package com.garbagemule.MobArena;
+
+import org.bukkit.plugin.Plugin;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URL;
+import java.net.URLConnection;
+import java.util.Scanner;
+import java.util.function.Consumer;
+
+public class PluginVersionCheck {
+
+    private static final String RESOURCE_ID = "34110";
+    private static final String ENDPOINT = "https://api.spigotmc.org/legacy/update.php?resource=" + RESOURCE_ID;
+    private static final int TIMEOUT = 5000;
+    private static final long CACHE_TTL = 60 * 60 * 1000;
+
+    private static long timeOfLastCheck = 0;
+    private static String messageOfLastCheck = null;
+
+    public static void check(Plugin plugin, Consumer<String> block) {
+        if (cacheIsFresh()) {
+            checkFromCache(plugin, block);
+        } else {
+            checkFromRemote(plugin, block);
+        }
+    }
+
+    private static boolean cacheIsFresh() {
+        return System.currentTimeMillis() < timeOfLastCheck + CACHE_TTL;
+    }
+
+    private static void checkFromCache(Plugin plugin, Consumer<String> block) {
+        // Run on the next tick to avoid drowning in login spam
+        plugin.getServer().getScheduler().runTask(plugin, () -> {
+            if (messageOfLastCheck != null) {
+                block.accept(messageOfLastCheck);
+            }
+        });
+    }
+
+    private static void checkFromRemote(Plugin plugin, Consumer<String> block) {
+        // Reset the cache before going fishing
+        resetCache();
+
+        // Get off the main thread for fetching the remote version
+        plugin.getServer().getScheduler().runTaskAsynchronously(plugin, () -> {
+            String local = plugin.getDescription().getVersion();
+            String remote = getRemoteVersion();
+
+            // Get back on the main thread for the result
+            plugin.getServer().getScheduler().runTask(plugin, () -> {
+                if (remote != null && lessThan(local, remote)) {
+                    // Create the update notification and cache it
+                    String message = String.format("v%s is now available! You are running v%s.", remote, local);
+                    hydrateCache(message);
+
+                    block.accept(message);
+                }
+            });
+        });
+    }
+
+    private static void resetCache() {
+        timeOfLastCheck = System.currentTimeMillis();
+        messageOfLastCheck = null;
+    }
+
+    private static void hydrateCache(String message) {
+        timeOfLastCheck = System.currentTimeMillis();
+        messageOfLastCheck = message;
+    }
+
+    private static String getRemoteVersion() {
+        try (
+            InputStream is = getEndpointStream();
+            Scanner scanner = new Scanner(is)
+        ) {
+            if (scanner.hasNext()) {
+                return scanner.next();
+            }
+        } catch (IOException e) {
+            // Update checks are non-essential, so just swallow
+        }
+        return null;
+    }
+
+    private static InputStream getEndpointStream() throws IOException {
+        // Create a new URL from the resource endpoint
+        URL url = new URL(ENDPOINT);
+
+        // Open the connection and set some timeouts
+        URLConnection connection = url.openConnection();
+        connection.setConnectTimeout(TIMEOUT);
+        connection.setReadTimeout(TIMEOUT);
+
+        // Finally, return the stream
+        return connection.getInputStream();
+    }
+
+    static boolean lessThan(String local, String remote) {
+        if (local == null || remote == null) {
+            return false;
+        }
+
+        String localVersion = local.split("-")[0];
+        String remoteVersion = remote.split("-")[0];
+
+        String[] localParts = localVersion.split("\\.");
+        String[] remoteParts = remoteVersion.split("\\.");
+
+        int length = Math.max(localParts.length, remoteParts.length);
+
+        for (int i = 0; i < length; i++) {
+            int localPart = Integer.parseInt((localParts.length > i) ? localParts[i] : "0");
+            int remotePart = Integer.parseInt((remoteParts.length > i) ? remoteParts[i] : "0");
+
+            // We skip to the next part if local and remote are identical,
+            // because we only have to short-circuit when they differ.
+            if (localPart == remotePart) {
+                continue;
+            }
+
+            // We've reached a point where local is either greater than or
+            // less than remote. Greater than means we're running a bleeding
+            // edge build. Less than means we're running an outdated build.
+            return localPart < remotePart;
+        }
+
+        // The two versions are identical, but if local is a SNAPSHOT, it
+        // is technically not the same and actually a lower version.
+        return local.endsWith("-SNAPSHOT");
+    }
+
+}

src/main/java/com/garbagemule/MobArena/listeners/MAGlobalListener.java

@@ -1,13 +1,14 @@
 package com.garbagemule.MobArena.listeners;
 
 import com.garbagemule.MobArena.MobArena;
+import com.garbagemule.MobArena.PluginVersionCheck;
 import com.garbagemule.MobArena.framework.Arena;
 import com.garbagemule.MobArena.framework.ArenaMaster;
 import com.garbagemule.MobArena.leaderboards.Stats;
-import com.garbagemule.MobArena.util.VersionChecker;
 import com.garbagemule.MobArena.util.inventory.InventoryManager;
 import org.bukkit.ChatColor;
 import org.bukkit.block.Block;
+import org.bukkit.entity.Player;
 import org.bukkit.event.EventHandler;
 import org.bukkit.event.EventPriority;
 import org.bukkit.event.Listener;
@@ -51,6 +52,7 @@ import org.bukkit.event.world.WorldUnloadEvent;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.UUID;
 
 /**
  * The point of this class is to simply redirect all events to each arena's
@@ -310,7 +312,13 @@ public class MAGlobalListener implements Listener
         InventoryManager.restoreFromFile(plugin, event.getPlayer());
         if (!am.notifyOnUpdates() || !event.getPlayer().isOp()) return;
 
-        VersionChecker.checkForUpdates(plugin, event.getPlayer());
+        UUID id = event.getPlayer().getUniqueId();
+        PluginVersionCheck.check(plugin, (message) -> {
+            Player player = plugin.getServer().getPlayer(id);
+            if (player != null) {
+                plugin.getGlobalMessenger().tell(player, message);
+            }
+        });
     }
 
     @EventHandler(priority = EventPriority.NORMAL)

src/main/java/com/garbagemule/MobArena/util/VersionChecker.java

@@ -1,102 +0,0 @@
-package com.garbagemule.MobArena.util;
-
-import com.garbagemule.MobArena.MobArena;
-import com.garbagemule.MobArena.util.Updater.UpdateResult;
-import com.garbagemule.MobArena.util.Updater.UpdateType;
-import org.bukkit.Bukkit;
-import org.bukkit.entity.Player;
-
-public class VersionChecker
-{
-    static Updater updater;
-    
-    public static void checkForUpdates(final MobArena plugin, final Player player) {
-        if (updater == null) {
-            updater = new Updater(plugin, 31265, plugin.getPluginFile(), UpdateType.NO_DOWNLOAD, false);
-        }
-
-        // Async for anti-lag
-        final Updater cache = updater;
-        Bukkit.getScheduler().runTaskAsynchronously(plugin, new Runnable() {
-            public void run() {
-                if (cache.getResult() == UpdateResult.UPDATE_AVAILABLE) {
-                    final String latest  = getLatestVersionString();
-                    final String current = plugin.getDescription().getVersion();
-
-                    if (latest == null || current == null) {
-                        String msg = "Update checker failed. Please check manually!";
-                        message(plugin, player, msg);
-                    }
-
-                    else if (isUpdateAvailable(latest, current)) {
-                        String msg1 = "MobArena v" + latest + " is now available!";
-                        String msg2 = "Your version: v" + current;
-                        message(plugin, player, msg1, msg2);
-                    }
-                }
-            }
-        });
-    }
-
-    private static String getLatestVersionString() {
-        String latestName = updater.getLatestName();
-        if (!latestName.matches("MobArena v.*")) {
-            return null;
-        }
-        return latestName.substring("MobArena v".length());
-    }
-
-    private static boolean isUpdateAvailable(String latestVersion, String currentVersion) {
-        // Split into major.minor(.patch(.build))
-        String[] latestParts  = latestVersion.split("\\.");
-        String[] currentParts = currentVersion.split("\\.");
-
-        // Figure out how many numbers to compare
-        int parts = Math.max(latestParts.length, currentParts.length);
-
-        // Check each part
-        for (int i = 0; i < parts; i++) {
-            int latest  = getPart(latestParts,  i);
-            int current = getPart(currentParts, i);
-
-            // Return early if current is more recent
-            if (current > latest) {
-                return false;
-            }
-
-            // And also if latest is more recent
-            if (latest > current) {
-                return true;
-            }
-        }
-
-        // Otherwise, we're completely up-to-date!
-        return false;
-    }
-
-    private static int getPart(String[] parts, int i) {
-        // Out of bounds or not an int? Bail with 0.
-        if (i >= parts.length || !parts[i].matches("[0-9]+")) {
-            return 0;
-        }
-        return Integer.parseInt(parts[i]);
-    }
-
-    private static void message(final MobArena plugin, final Player player, final String... messages) {
-        Bukkit.getScheduler().runTaskLater(plugin, new Runnable() {
-            public void run() {
-                for (String message : messages) {
-                    if (player == null) {
-                        plugin.getLogger().info(message);
-                    } else if (player.isOnline()) {
-                        plugin.getGlobalMessenger().tell(player, message);
-                    }
-                }
-            }
-        }, (player == null) ? 0 : 60); // Message player after login spam
-    }
-
-    public static void shutdown() {
-        updater = null;
-    }
-}

src/test/java/com/garbagemule/MobArena/PluginVersionCheckTest.java

@@ -0,0 +1,92 @@
+package com.garbagemule.MobArena;
+
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import java.util.Arrays;
+import java.util.Collection;
+
+import static org.junit.Assert.assertEquals;
+
+@RunWith(Parameterized.class)
+public class PluginVersionCheckTest {
+
+    @Parameterized.Parameters
+    public static Collection<Object[]> data() {
+        return Arrays.asList(new Object[][] {
+            // Patch: local < remote
+            { "1.1.1", "1.1.2", true },
+            // Patch: local > remote
+            { "1.1.2", "1.1.1", false },
+            // Patch: equal
+            { "1.1.2", "1.1.2", false },
+
+            // Minor: local < remote
+            { "1.1.1", "1.2.1", true },
+            { "1.1.2", "1.2.1", true },
+            // Minor: local > remote
+            { "1.2.1", "1.1.1", false },
+            { "1.2.1", "1.1.2", false },
+            // Minor: equal
+            { "1.2.1", "1.2.1", false },
+            { "1.2.2", "1.2.2", false },
+
+            // Major: local < remote
+            { "1.1.1", "2.1.1", true },
+            { "1.1.2", "2.1.1", true },
+            { "1.2.1", "2.1.1", true },
+            { "1.2.2", "2.1.1", true },
+            // Major: local > remote
+            { "2.1.1", "1.1.1", false },
+            { "2.1.1", "1.1.2", false },
+            { "2.1.1", "1.2.1", false },
+            { "2.1.1", "1.2.2", false },
+            // Major: equal
+            { "2.1.1", "2.1.1", false },
+            { "2.2.1", "2.2.1", false },
+            { "2.2.2", "2.2.2", false },
+
+            // Incomplete: local < remote
+            { "1",     "1.1.1", true },
+            { "1.1",   "1.1.1", true },
+            { "1.1.1", "2"    , true },
+            { "1.1.1", "1.2"  , true },
+            // Incomplete: local > remote
+            { "1.1.1", "1",     false },
+            { "1.1.1", "1.1",   false },
+            { "2"    , "1.1.1", false },
+            { "1.2"  , "1.1.1", false },
+
+            // Snapshot: local < remote
+            { "1.1.1-SNAPSHOT", "1.1.2", true },
+            { "1.1.1-SNAPSHOT", "1.2.1", true },
+            { "1.1.1-SNAPSHOT", "2.1.1", true },
+            // Snapshot: local > remote
+            { "1.1.2-SNAPSHOT", "1.1.1", false },
+            { "1.2.1-SNAPSHOT", "1.1.1", false },
+            { "2.1.1-SNAPSHOT", "1.1.1", false },
+            // Snapshot: equal
+            { "1.1.2-SNAPSHOT", "1.1.2", true },
+            { "1.2.1-SNAPSHOT", "1.2.1", true },
+            { "2.1.1-SNAPSHOT", "2.1.1", true },
+        });
+    }
+
+    @Parameterized.Parameter
+    public String local;
+
+    @Parameterized.Parameter(1)
+    public String remote;
+
+    @Parameterized.Parameter(2)
+    public boolean expected;
+
+    @Test
+    public void test() {
+        boolean actual = PluginVersionCheck.lessThan(local, remote);
+
+        assertEquals("Expected " + local + " < " + remote + "?",  expected, actual);
+    }
+
+}