Run offline placeholder lookups off the Minecraft main thread

README.md

@@ -11,7 +11,6 @@ port: 11001
 bind: 0.0.0.0
 tokens:
   - "replace-with-a-long-random-secret"
-timeout-ms: 3000
 max-concurrent: 16
 rate-limit:
   requests: 60
@@ -21,9 +20,11 @@ allowed-ips: []
 
 Tokens must contain at least 16 characters, cannot be blank or padded with spaces, and must be unique. A missing or invalid token configuration prevents startup. To rotate tokens, temporarily include old and new tokens, run `/restpapi reload`, update clients, then remove the old token and reload again. Never print the tokens or put them in browser JavaScript.
 
-`bind` selects the interface **inside the container** (default `0.0.0.0`). `allowed-ips` is an exact match list of socket peer IPs; an empty list permits any peer with a valid token. Behind Nginx it will normally see the proxy address, not the original client. It deliberately ignores `X-Forwarded-For`. The rate limit is per socket peer and uses a fixed window; no more than 4096 distinct peers are tracked. The concurrency limit returns HTTP 503 instead of queuing unbounded lookups. Placeholder evaluation runs on the Minecraft main thread; clients receive HTTP 504 if it takes longer than `timeout-ms`. A lookup already running on the main thread cannot be interrupted.
+`bind` selects the interface **inside the container** (default `0.0.0.0`). `allowed-ips` is an exact match list of socket peer IPs; an empty list permits any peer with a valid token. Behind Nginx it will normally see the proxy address, not the original client. It deliberately ignores `X-Forwarded-For`. The rate limit is per socket peer and uses a fixed window; no more than 4096 distinct peers are tracked. The concurrency limit returns HTTP 503 instead of queuing unbounded lookups.
 
-The command `/restpapi reload` reads the file again, validates it before stopping the old listener, and attempts to restore the old listener if binding the new one fails. A failed rollback disables the plugin. Listener shutdown and startup take place off the Minecraft main thread; the command reports the result after they finish. In-flight lookups receive 503 during shutdown. A successful reload updates both the port and token set.
+Placeholder evaluation, including `Bukkit.getOfflinePlayer(UUID)`, runs directly on Spark's Jetty worker thread and is not scheduled onto the Minecraft main thread. This keeps offline-player lookups and PlaceholderAPI evaluation off the server tick thread, but it also means every expansion queried through this API must support off-thread execution. PlaceholderAPI does not make third-party expansions thread-safe automatically. There is no server-side lookup timeout because a synchronous expansion running on the current Spark worker cannot be safely preempted; use `max-concurrent` to bound concurrent evaluations and configure request timeouts in the HTTP client or reverse proxy.
+
+The command `/restpapi reload` reads the file again, validates it before stopping the old listener, and attempts to restore the old listener if binding the new one fails. A failed rollback disables the plugin. Listener shutdown and startup take place off the Minecraft main thread; the command reports the result after they finish. Requests that begin after shutdown starts receive 503. A successful reload updates both the port and token set.
 
 ## Requests
 
@@ -34,7 +35,7 @@ curl -H "Token: YOUR_SECRET" "http://127.0.0.1:11001/da8a8993-adfa-4d29-99b1-9d0
 curl -H "Token: YOUR_SECRET" "http://127.0.0.1:11001/server/server_online"
 ```
 
-Use the placeholder name without percent signs. Unknown placeholders return 406, missing player data 400, bad UUID 400, unauthorized requests 401, blocked peers 403, unknown routes 404, excess requests 429, overload or shutdown 503, and timed out lookups 504. Empty resolved values are valid. Offline results depend on each PlaceholderAPI expansion's support for offline players.
+Use the placeholder name without percent signs. Unknown placeholders return 406, missing player data 400, bad UUID 400, unauthorized requests 401, blocked peers 403, unknown routes 404, excess requests 429, and overload or shutdown 503. Empty resolved values are valid. Offline results depend on each PlaceholderAPI expansion's support for offline players and off-thread evaluation.
 
 ## Docker, Pterodactyl and external bots
 

src/main/java/me/fredthedoggy/restpapi/RestConfig.java

@@ -2,29 +2,27 @@
 
 import org.bukkit.configuration.file.FileConfiguration;
 
-import java.util.HashSet;
 import java.util.ArrayList;
 import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
 final class RestConfig {
-    private final int port, timeoutMillis, rateLimit, rateWindowSeconds, maxConcurrent;
+    private final int port, rateLimit, rateWindowSeconds, maxConcurrent;
     private final String bind;
     private final List<String> tokens;
     private final Set<String> allowedIps;
 
     RestConfig(FileConfiguration yaml) {
         port = yaml.getInt("port", 8080);
         bind = yaml.getString("bind", "0.0.0.0");
-        timeoutMillis = yaml.getInt("timeout-ms", 3000);
         rateLimit = yaml.getInt("rate-limit.requests", 60);
         rateWindowSeconds = yaml.getInt("rate-limit.window-seconds", 60);
         maxConcurrent = yaml.getInt("max-concurrent", 16);
         tokens = Collections.unmodifiableList(new ArrayList<>(yaml.getStringList("tokens")));
         allowedIps = Collections.unmodifiableSet(new HashSet<>(yaml.getStringList("allowed-ips")));
         if (port < 1 || port > 65535 || bind == null || bind.trim().isEmpty()
-                || timeoutMillis < 100 || timeoutMillis > 30000
                 || rateLimit < 1 || rateLimit > 10000 || rateWindowSeconds < 1
                 || rateWindowSeconds > 3600 || maxConcurrent < 1 || maxConcurrent > 24
                 || tokens.isEmpty() || tokens.stream().anyMatch(t -> t == null || t.length() < 16 || !t.equals(t.trim()))
@@ -36,7 +34,6 @@ final class RestConfig {
 
     int port() { return port; }
     String bind() { return bind; }
-    int timeoutMillis() { return timeoutMillis; }
     int rateLimit() { return rateLimit; }
     int rateWindowSeconds() { return rateWindowSeconds; }
     int maxConcurrent() { return maxConcurrent; }

src/main/java/me/fredthedoggy/restpapi/RestPapiLoader.java

@@ -25,7 +25,6 @@ void enable() {
             yaml.set("port", 8080);
             yaml.set("bind", "0.0.0.0");
             yaml.set("tokens", Arrays.asList(UUID.randomUUID().toString(), UUID.randomUUID().toString()));
-            yaml.set("timeout-ms", 3000);
             yaml.set("max-concurrent", 16);
             yaml.set("rate-limit.requests", 60);
             yaml.set("rate-limit.window-seconds", 60);

src/main/java/me/fredthedoggy/restpapi/SparkWrapper.java

@@ -4,21 +4,14 @@
 import me.clip.placeholderapi.PlaceholderAPI;
 import org.bukkit.Bukkit;
 import org.bukkit.OfflinePlayer;
-import org.bukkit.scheduler.BukkitTask;
 import spark.Request;
 import spark.Response;
 import spark.Service;
 
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.util.UUID;
-import java.util.Set;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Semaphore;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.logging.Level;
@@ -33,7 +26,6 @@ final class SparkWrapper {
     private final RequestLimiter limiter;
     private final AtomicBoolean accepting = new AtomicBoolean(true);
     private final AtomicBoolean stopped = new AtomicBoolean(false);
-    private final Set<CompletableFuture<Lookup>> pending = ConcurrentHashMap.newKeySet();
     private Service http;
 
     SparkWrapper(Restpapi plugin, RestConfig config) {
@@ -67,8 +59,7 @@ void start() {
             if (routesRegistered) {
                 stop();
                 awaitStop();
-            }
-            else {
+            } else {
                 accepting.set(false);
                 http = null;
             }
@@ -86,58 +77,43 @@ private String handle(Request request, Response response, boolean playerRoute) {
         }
         if (!limiter.allow(request.ip())) return json(response, 429, "Too Many Requests");
         if (!concurrent.tryAcquire()) return json(response, 503, "Service Busy");
-        CompletableFuture<Lookup> future = new CompletableFuture<>();
+
         try {
             String name = request.params(":placeholder");
             if (name == null || name.isEmpty() || name.length() > 256 || name.indexOf('%') >= 0) {
                 return json(response, 400, "Invalid Placeholder");
             }
-            UUID uuid = null;
+
+            UUID playerId = null;
             if (playerRoute) {
                 try {
-                    uuid = UUID.fromString(request.params(":uuid"));
+                    playerId = UUID.fromString(request.params(":uuid"));
                 } catch (IllegalArgumentException exception) {
                     return json(response, 400, "Invalid UUID");
                 }
             }
-            final UUID playerId = uuid;
-            final String expression = "%" + name + "%";
-            track(future);
-            BukkitTask task = Bukkit.getScheduler().runTask(plugin, () -> {
-                if (!accepting.get() || future.isDone()) return;
-                try {
-                    OfflinePlayer player = playerId == null ? null : Bukkit.getOfflinePlayer(playerId);
-                    if (player != null && !player.hasPlayedBefore() && !player.isOnline()) {
-                        future.complete(new Lookup(400, "Player Has Not Played Before"));
-                        return;
-                    }
-                    String result = PlaceholderAPI.setPlaceholders(player, expression);
-                    future.complete(resolveResult(expression, result));
-                } catch (Exception exception) {
-                    plugin.getLogger().log(Level.WARNING, "Placeholder lookup failed", exception);
-                    future.complete(new Lookup(500, "Internal Server Error"));
-                }
-            });
-            try {
-                Lookup result = future.get(config.timeoutMillis(), TimeUnit.MILLISECONDS);
-                return json(response, Integer.parseInt(result.status), result.message);
-            } catch (TimeoutException exception) {
-                future.cancel(false);
-                task.cancel();
-                return json(response, 504, "Lookup Timed Out");
-            } catch (InterruptedException exception) {
-                Thread.currentThread().interrupt();
-                task.cancel();
+
+            String expression = "%" + name + "%";
+            OfflinePlayer player = playerId == null ? null : Bukkit.getOfflinePlayer(playerId);
+            if (player != null && !player.hasPlayedBefore() && !player.isOnline()) {
+                return json(response, 400, "Player Has Not Played Before");
+            }
+
+            // Spark invokes routes on its Jetty worker pool. Keep both OfflinePlayer lookup and
+            // PlaceholderAPI evaluation on that worker instead of moving the request onto the
+            // Minecraft main thread. Placeholder expansions queried through this endpoint must
+            // therefore support off-thread evaluation.
+            String result = PlaceholderAPI.setPlaceholders(player, expression);
+            if (!accepting.get() || !plugin.isEnabled()) {
                 return json(response, 503, "Service Unavailable");
-            } catch (ExecutionException exception) {
-                plugin.getLogger().log(Level.WARNING, "Placeholder lookup failed", exception);
-                return json(response, 500, "Internal Server Error");
             }
-        } catch (RuntimeException exception) {
-            plugin.getLogger().log(Level.WARNING, "Could not schedule placeholder lookup", exception);
-            return json(response, 503, "Service Unavailable");
+
+            Lookup lookup = resolveResult(expression, result);
+            return json(response, Integer.parseInt(lookup.status), lookup.message);
+        } catch (Exception exception) {
+            plugin.getLogger().log(Level.WARNING, "Placeholder lookup failed", exception);
+            return json(response, 500, "Internal Server Error");
         } finally {
-            pending.remove(future);
             concurrent.release();
         }
     }
@@ -161,11 +137,6 @@ private static Lookup resolveResult(String expression, String result) {
         return new Lookup(status, status == 406 ? "Invalid Placeholder" : result);
     }
 
-    void track(CompletableFuture<Lookup> future) {
-        pending.add(future);
-        if (!accepting.get()) future.complete(new Lookup(503, "Service Unavailable"));
-    }
-
     static String json(Response response, int status, String message) {
         response.status(status);
         response.type("application/json");
@@ -175,9 +146,6 @@ static String json(Response response, int status, String message) {
     void stop() {
         if (!stopped.compareAndSet(false, true)) return;
         accepting.set(false);
-        for (CompletableFuture<Lookup> future : pending) {
-            future.complete(new Lookup(503, "Service Unavailable"));
-        }
         if (http != null) http.stop();
     }
 

src/test/java/me/fredthedoggy/restpapi/RestSecurityTest.java

@@ -7,8 +7,6 @@
 
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.TimeUnit;
 
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
@@ -81,15 +79,9 @@ void rejectsOutOfRangeNetworkSettings() {
     }
 
     @Test
-    void shutdownUnblocksAnAwaitingHttpRequestWithoutRunningItsBukkitTask() throws Exception {
+    void shutdownIsIdempotentBeforeHttpStartup() {
         SparkWrapper server = new SparkWrapper(mock(Restpapi.class), new RestConfig(config()));
-        CompletableFuture<SparkWrapper.Lookup> lookup = new CompletableFuture<>();
-        server.track(lookup);
-        server.stop();
-        assertNotNull(lookup.get(100, TimeUnit.MILLISECONDS));
-
-        CompletableFuture<SparkWrapper.Lookup> lateLookup = new CompletableFuture<>();
-        server.track(lateLookup);
-        assertNotNull(lateLookup.get(100, TimeUnit.MILLISECONDS));
+        assertDoesNotThrow(server::stop);
+        assertDoesNotThrow(server::stop);
     }
 }