Fix optional PlaceholderAPI failures and release 2.2.3

docs/guides/CONFIGURATION-AND-LOCALIZATION.md

@@ -42,6 +42,34 @@ Define stable message keys with `MessageMap` and use the platform localization a
 
 Keep formatting at the presentation boundary. Domain services should return useful results or models rather than preformatted chat strings.
 
+### Optional PlaceholderAPI support on Paper
+
+Hosts can opt into `PlaceholderMessageDecorator.create()` from `featureframework-paper-integrations`.
+PAPI is checked on each resolution call. Missing, disabled or incompatible PAPI and failing expansions leave the
+original text unchanged; normal message substitutions and formatting still run. Failures emit rate-limited warnings
+and do not permanently disable later resolution attempts. Tokens requiring an unavailable Vault economy provider
+remain unresolved while other expansions can still resolve.
+
+Features exporting an expansion should use a lazy factory and keep the returned PAPI-independent handle:
+
+```java
+private PlaceholderExpansionRegistration placeholders;
+
+// During initialization, after creating the state used by the expansion:
+placeholders = PlaceholderExpansionRegistration.register("example", () -> new ExampleExpansion(this));
+
+// During disable, before releasing that state:
+if (placeholders != null) {
+    placeholders.close();
+    placeholders = null;
+}
+```
+
+`PlaceholderExpansionRegistration` contains runtime and linkage failures from availability checks, construction,
+registration and removal. Its `close()` is idempotent, including when registration failed. Retain only this handle
+in feature fields so feature discovery does not need to load PAPI expansion types. Retry failed registration by
+reloading the feature after repairing PAPI. Fatal JVM errors are not suppressed.
+
 ## Transactional feature-file reset
 
 Concrete Paper and Velocity hosts expose `previewFileReset(...)` and `resetFiles(...)` for administrative tooling.

featureframework-paper-integrations/src/main/java/nl/hauntedmc/featureframework/paper/integration/placeholder/PlaceholderAPIHook.java

@@ -36,7 +36,7 @@ public static String applyPlaceholders(String text, Player player) {
                 text,
                 player,
                 pluginName -> Bukkit.getPluginManager().isPluginEnabled(pluginName),
-                me.clip.placeholderapi.PlaceholderAPI::setPlaceholders,
+                (context, message) -> me.clip.placeholderapi.PlaceholderAPI.setPlaceholders(context, message),
                 PlaceholderAPIHook::isVaultEconomyAvailable,
                 PlaceholderAPIHook::logWarning,
                 System::nanoTime
@@ -83,14 +83,14 @@ static String applyPlaceholders(
                 return text;
             }
         } catch (RuntimeException | LinkageError failure) {
-            warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong());
+            warnSafely(text, player, failure, warningSink, nanoTime);
             return text;
         }
 
         MaskedText input = MaskedText.unchanged(text);
         if (VAULT_ECONOMY_PATTERN.matcher(text).find() && !safeEconomyAvailability(vaultEconomyAvailable)) {
             input = MaskedText.maskVaultEconomy(text);
-            warnRateLimited(text, player, null, warningSink, nanoTime.getAsLong());
+            warnSafely(text, player, null, warningSink, nanoTime);
         }
 
         try {
@@ -100,7 +100,7 @@ static String applyPlaceholders(
             }
             return input.restore(resolved);
         } catch (RuntimeException | LinkageError failure) {
-            warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong());
+            warnSafely(text, player, failure, warningSink, nanoTime);
             return text;
         }
     }
@@ -160,6 +160,20 @@ private static boolean providerReportsEnabled(Object provider) {
         }
     }
 
+    private static void warnSafely(
+            String text,
+            Player player,
+            Throwable failure,
+            BiConsumer<String, Throwable> warningSink,
+            LongSupplier nanoTime
+    ) {
+        try {
+            warnRateLimited(text, player, failure, warningSink, nanoTime.getAsLong());
+        } catch (RuntimeException | LinkageError ignored) {
+            // Diagnostics must never prevent the original message from being rendered.
+        }
+    }
+
     private static void warnRateLimited(
             String text,
             Player player,
@@ -248,11 +262,15 @@ private static String safePlayerName(Player player) {
         return "unknown";
     }
 
-    private static void logWarning(String message, Throwable failure) {
-        if (failure == null) {
-            Bukkit.getLogger().warning("[FeatureFramework] " + message);
-        } else {
-            Bukkit.getLogger().log(Level.WARNING, "[FeatureFramework] " + message, failure);
+    static void logWarning(String message, Throwable failure) {
+        try {
+            if (failure == null) {
+                Bukkit.getLogger().warning("[FeatureFramework] " + message);
+            } else {
+                Bukkit.getLogger().log(Level.WARNING, "[FeatureFramework] " + message, failure);
+            }
+        } catch (RuntimeException | LinkageError ignored) {
+            // Optional integration failures must remain isolated even if logging is unavailable.
         }
     }
 

featureframework-paper-integrations/src/main/java/nl/hauntedmc/featureframework/paper/integration/placeholder/PlaceholderExpansionRegistration.java

@@ -0,0 +1,103 @@
+package nl.hauntedmc.featureframework.paper.integration.placeholder;
+
+import org.bukkit.Bukkit;
+
+import java.util.Objects;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.function.Function;
+import java.util.function.Supplier;
+
+/** Owns an optional expansion without exposing PlaceholderAPI types to feature lifecycles. */
+public final class PlaceholderExpansionRegistration implements AutoCloseable {
+    private final String identifier;
+    private final Runnable unregister;
+    private final AtomicBoolean closed = new AtomicBoolean();
+
+    private PlaceholderExpansionRegistration(String identifier, Runnable unregister) {
+        this.identifier = identifier;
+        this.unregister = unregister;
+    }
+
+    /**
+     * Constructs and registers an expansion only when PlaceholderAPI is enabled.
+     * The factory must lazily create a PlaceholderExpansion; use a lambda rather than constructing it first.
+     * Failures return an inactive handle and leave the owning feature operational.
+     */
+    public static PlaceholderExpansionRegistration register(String identifier, Supplier<?> factory) {
+        return register(identifier, factory, name -> Bukkit.getPluginManager().isPluginEnabled("PlaceholderAPI")
+                ? AvailableExpansion.current(name) : null);
+    }
+
+    static PlaceholderExpansionRegistration register(
+            String identifier, Supplier<?> factory, Function<String, ?> registeredExpansion
+    ) {
+        Objects.requireNonNull(identifier, "identifier");
+        Objects.requireNonNull(factory, "factory");
+        Objects.requireNonNull(registeredExpansion, "registeredExpansion");
+        Runnable cleanup = null;
+        try {
+            if (Bukkit.getPluginManager().isPluginEnabled("PlaceholderAPI")) {
+                cleanup = AvailableExpansion.install(identifier, factory, registeredExpansion);
+            }
+        } catch (RuntimeException | LinkageError failure) {
+            PlaceholderAPIHook.logWarning("Could not register PlaceholderAPI expansion '" + identifier
+                    + "'; the feature will continue without its placeholders.", failure);
+        }
+        return new PlaceholderExpansionRegistration(identifier, cleanup);
+    }
+
+    /** Best-effort cleanup, safe to call repeatedly even after PlaceholderAPI is disabled or removed. */
+    @Override
+    public void close() {
+        if (!closed.compareAndSet(false, true) || unregister == null) {
+            return;
+        }
+        try {
+            unregister.run();
+        } catch (RuntimeException | LinkageError failure) {
+            PlaceholderAPIHook.logWarning("Could not unregister PlaceholderAPI expansion '" + identifier
+                    + "'; continuing feature cleanup.", failure);
+        }
+    }
+
+    /** Loaded only inside the guarded registration path; the outer handle has no PAPI signatures. */
+    private static final class AvailableExpansion {
+        private static Runnable install(String identifier, Supplier<?> factory, Function<String, ?> registeredExpansion) {
+            var expansion = (me.clip.placeholderapi.expansion.PlaceholderExpansion)
+                    Objects.requireNonNull(factory.get(), "expansion");
+            try {
+                if (expansion.register()) {
+                    return () -> remove(expansion, registeredExpansion);
+                }
+            } catch (RuntimeException | LinkageError failure) {
+                try {
+                    remove(expansion, registeredExpansion);
+                } catch (RuntimeException | LinkageError cleanupFailure) {
+                    if (cleanupFailure != failure) {
+                        failure.addSuppressed(cleanupFailure);
+                    }
+                }
+                throw failure;
+            }
+            PlaceholderAPIHook.logWarning("Could not register PlaceholderAPI expansion '" + identifier
+                    + "'; registration was refused. The feature will continue without its placeholders.", null);
+            return null;
+        }
+
+        private static Object current(String identifier) {
+            return me.clip.placeholderapi.PlaceholderAPIPlugin.getInstance().getLocalExpansionManager()
+                    .findExpansionByIdentifier(identifier).orElse(null);
+        }
+
+        private static void remove(
+                me.clip.placeholderapi.expansion.PlaceholderExpansion expansion, Function<String, ?> registeredExpansion
+        ) {
+            // A replacement may already own this identifier after a PAPI reload.
+            Object registered = registeredExpansion.apply(expansion.getIdentifier());
+            if (registered == expansion && !expansion.unregister()) {
+                PlaceholderAPIHook.logWarning("PlaceholderAPI refused to unregister expansion '"
+                        + expansion.getIdentifier() + "'; continuing feature cleanup.", null);
+            }
+        }
+    }
+}

featureframework-paper-integrations/src/test/java/nl/hauntedmc/featureframework/paper/integration/placeholder/PlaceholderAPIHookTest.java

@@ -0,0 +1,138 @@
+package nl.hauntedmc.featureframework.paper.integration.placeholder;
+
+import org.bukkit.entity.Player;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+import java.util.UUID;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.function.BiConsumer;
+import java.util.function.BiFunction;
+import java.util.function.Predicate;
+
+import static org.junit.jupiter.api.Assertions.*;
+import static org.mockito.Mockito.*;
+
+class PlaceholderAPIHookTest {
+    private final Player player = mock(Player.class);
+
+    @BeforeEach
+    void resetWarnings() {
+        PlaceholderAPIHook.clearWarningStateForTests();
+        when(player.getUniqueId()).thenReturn(UUID.fromString("00000000-0000-0000-0000-000000000001"));
+        when(player.getName()).thenReturn("Alice");
+    }
+
+    @Test
+    void skipsUnavailablePapiAndInputsWithoutPlayerPlaceholders() {
+        Predicate<String> unavailable = name -> false;
+        BiFunction<Player, String, String> resolver = (context, message) -> {
+            fail("Unavailable PAPI must not be called");
+            return message;
+        };
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, unavailable, resolver));
+        Predicate<String> unexpectedLookup = name -> {
+            fail("Plain messages and missing context must not look up PAPI");
+            return true;
+        };
+        assertNull(PlaceholderAPIHook.applyPlaceholders(null, player, unexpectedLookup, resolver));
+        assertEquals("Hello", PlaceholderAPIHook.applyPlaceholders("Hello", player, unexpectedLookup, resolver));
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", null, unexpectedLookup, resolver));
+    }
+
+    @Test
+    void containsRuntimeAndLinkageFailuresFromLookupAndResolution() {
+        for (Throwable failure : List.of(new IllegalStateException("broken expansion"),
+                new NoClassDefFoundError("missing PAPI"), new NoSuchMethodError("incompatible PAPI"),
+                new ExceptionInInitializerError("broken initialization"))) {
+            assertEquals("Hello %player_name%", PlaceholderAPIHook.applyPlaceholders(
+                    "Hello %player_name%", player, name -> true,
+                    (context, message) -> { throwUnchecked(failure); return message; }));
+            assertEquals("Hello %player_name%", PlaceholderAPIHook.applyPlaceholders(
+                    "Hello %player_name%", player,
+                    name -> { throwUnchecked(failure); return true; }, (context, message) -> "unexpected"));
+        }
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, (context, message) -> null));
+    }
+
+    @Test
+    void retriesAfterFailureAndRechecksAvailabilityOnEveryCall() {
+        AtomicInteger attempts = new AtomicInteger();
+        BiFunction<Player, String, String> resolver = (context, message) -> {
+            if (attempts.getAndIncrement() == 0) throw new IllegalStateException("not ready");
+            return message.replace("%player_name%", "Alice");
+        };
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, resolver));
+        assertEquals("Alice", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, resolver));
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> false, resolver));
+        assertEquals(2, attempts.get());
+    }
+
+    @Test
+    void preservesUnavailableVaultTokensWhileResolvingOtherExpansions() {
+        String text = "%vault_eco_balance% / %player_name%";
+        BiFunction<Player, String, String> resolver = (context, message) -> {
+            assertFalse(message.contains("%vault_eco_balance%"));
+            return message.replace("%player_name%", "Alice");
+        };
+        assertEquals("%vault_eco_balance% / Alice", PlaceholderAPIHook.applyPlaceholders(
+                text, player, name -> true, resolver, () -> false, (message, failure) -> { }, () -> 0L));
+        assertEquals("%vault_eco_balance% / Alice", PlaceholderAPIHook.applyPlaceholders(
+                text, player, name -> true, resolver, () -> { throw new NoClassDefFoundError("Vault"); },
+                (message, failure) -> { }, () -> 0L));
+        assertEquals("10 / Alice", PlaceholderAPIHook.applyPlaceholders(
+                text, player, name -> true,
+                (context, message) -> message.replace("%vault_eco_balance%", "10").replace("%player_name%", "Alice"),
+                () -> true, (message, failure) -> { }, () -> 0L));
+    }
+
+    @Test
+    void throttlesWarningsAndContainsDiagnosticFailures() {
+        AtomicInteger warnings = new AtomicInteger();
+        AtomicLong time = new AtomicLong();
+        BiFunction<Player, String, String> broken = (context, message) -> {
+            throw new IllegalStateException("broken expansion");
+        };
+        BiConsumer<String, Throwable> sink = (message, failure) -> warnings.incrementAndGet();
+        for (int i = 0; i < 2; i++) {
+            assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                    "%player_name%", player, name -> true, broken, () -> true, sink, time::get));
+        }
+        assertEquals(1, warnings.get());
+        time.set(TimeUnit.MINUTES.toNanos(5));
+        PlaceholderAPIHook.applyPlaceholders("%player_name%", player, name -> true, broken,
+                () -> true, sink, time::get);
+        assertEquals(2, warnings.get());
+
+        PlaceholderAPIHook.clearWarningStateForTests();
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, broken, () -> true,
+                (message, failure) -> { throw new NoClassDefFoundError("logger"); }, time::get));
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, broken, () -> true, sink,
+                () -> { throw new IllegalStateException("clock"); }));
+        when(player.getUniqueId()).thenThrow(new NoClassDefFoundError("player diagnostic"));
+        assertEquals("%player_name%", PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, broken, () -> true, sink, time::get));
+    }
+
+    @Test
+    void doesNotSuppressFatalVmFailures() {
+        assertThrows(OutOfMemoryError.class, () -> PlaceholderAPIHook.applyPlaceholders(
+                "%player_name%", player, name -> true, (context, message) -> { throw new OutOfMemoryError(); }));
+    }
+
+    private static void throwUnchecked(Throwable failure) {
+        if (failure instanceof RuntimeException exception) throw exception;
+        throw (Error) failure;
+    }
+}