This is the operation to use for a purchase. Unlike calling {@link #getBalance} and then + * {@link #withdraw}, nothing can slip in between the check and the debit.
+ * + * @param playerId The UUID of the player to debit. + * @param amount The amount to debit, must be strictly positive. + * @param reason The reason of the withdrawal. + * @return The outcome. Nothing is debited unless the status is + * {@link TransactionResult.Status#SUCCESS}. + */ + public TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, String reason) { + return this.withdrawIfSufficient(playerId, amount, DEFAULT_CURRENCY_NAME, reason); + } + + /** + * Remove some money from a player, but only if the player can actually afford it. + * + * @param playerId The UUID of the player to debit. + * @param amount The amount to debit, must be strictly positive. + * @param currencyName The name of the currency. + * @param reason The reason of the withdrawal. + * @return The outcome. Nothing is debited unless the status is + * {@link TransactionResult.Status#SUCCESS}. + */ + public TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, String currencyName, String reason) { + this.canBeUse(currencyName); + return this.providers.get(currencyName).withdrawIfSufficient(playerId, amount, reason); + } + + /** + * Asynchronous variant of {@link #withdrawIfSufficient(UUID, BigDecimal, String, String)}. + * + * @param playerId The UUID of the player to debit. + * @param amount The amount to debit, must be strictly positive. + * @param currencyName The name of the currency. + * @param reason The reason of the withdrawal. + * @return A future completed with the outcome. + */ + public CompletableFutureWorth checking on a network where several servers share one economy: an emulated + * operation is only protected against races inside this server.
+ * + * @param currencyName The name of the currency. + * @return True when the backend itself guarantees the operation. + */ + public boolean hasNativeConditionalWithdraw(String currencyName) { + this.canBeUse(currencyName); + return this.providers.get(currencyName).hasNativeConditionalWithdraw(); + } + private void canBeUse(String currencyName) { if (this.isDisable()) { throw new IllegalStateException("The plugin " + this.name + " is not enable."); } - if (autoCreate) { + if (this.autoCreate) { - if (currencySpecific) { - registerProvider(currencyName, currencyName); + if (this.currencySpecific) { + this.registerProvider(currencyName, currencyName); } else { - registerProvider(currencyName); + this.registerProvider(currencyName); } } else if (!this.providers.containsKey(currencyName)) { - String currency = name.equalsIgnoreCase("default") ? "" : " and for the currency " + name; + String currency = this.name.equalsIgnoreCase(DEFAULT_CURRENCY_NAME) ? "" : " and for the currency " + name; throw new IllegalStateException("You must create the provider for the plugin " + this.name + currency + " before using it."); } } diff --git a/src/main/java/fr/traqueur/currencies/CurrenciesAPI.java b/src/main/java/fr/traqueur/currencies/CurrenciesAPI.java new file mode 100644 index 0000000..02958c3 --- /dev/null +++ b/src/main/java/fr/traqueur/currencies/CurrenciesAPI.java @@ -0,0 +1,47 @@ +package fr.traqueur.currencies; + +import org.bukkit.Bukkit; +import org.bukkit.plugin.Plugin; + +public final class CurrenciesAPI { + + private static volatile Plugin plugin; + + private CurrenciesAPI() { + } + + /** + * Register the plugin instance used to schedule work on the main server thread. + * + * @param owningPlugin The plugin instance, must not be null. + */ + public static void init(Plugin owningPlugin) { + if (owningPlugin == null) { + throw new IllegalArgumentException("The plugin instance cannot be null."); + } + if (plugin != null) { + throw new IllegalStateException("The plugin instance has already been set by " + plugin.getName() + "."); + } + plugin = owningPlugin; + } + + /** + * @return The registered plugin instance, or null when {@link #init(Plugin)} was never called. + */ + public static Plugin getPlugin() { + return plugin; + } + + /** + * Whether the caller is on the main server thread. + * + * @return True when the caller is running on the main server thread. + */ + static boolean isMainThread() { + try { + return Bukkit.isPrimaryThread(); + } catch (Throwable throwable) { + return false; + } + } +} diff --git a/src/main/java/fr/traqueur/currencies/CurrencyArgumentChecks.java b/src/main/java/fr/traqueur/currencies/CurrencyArgumentChecks.java new file mode 100644 index 0000000..340cf18 --- /dev/null +++ b/src/main/java/fr/traqueur/currencies/CurrencyArgumentChecks.java @@ -0,0 +1,38 @@ +package fr.traqueur.currencies; + +import java.math.BigDecimal; +import java.util.UUID; + +/** + * Argument checks shared by every conditional currency operation. + * + *Custom providers that override + * {@link CurrencyProvider#withdrawIfSufficient(UUID, BigDecimal, String)} should call + * {@link #findProblem(UUID, BigDecimal)} first, so that every implementation rejects the same + * nonsensical inputs in the same way.
+ */ +public final class CurrencyArgumentChecks { + + private CurrencyArgumentChecks() { + } + + /** + * Looks for a problem with the arguments to a conditional currency operation. + * + * @param playerId The player. + * @param amount The requested amount. + * @return A result describing the problem, or null when the arguments are usable. + */ + public static TransactionResult findProblem(UUID playerId, BigDecimal amount) { + if (playerId == null) { + return TransactionResult.failed(amount, "The player UUID cannot be null."); + } + if (amount == null) { + return TransactionResult.failed(BigDecimal.ZERO, "The amount cannot be null."); + } + if (amount.signum() <= 0) { + return TransactionResult.failed(amount, "The amount must be strictly positive, was " + amount + "."); + } + return null; + } +} diff --git a/src/main/java/fr/traqueur/currencies/CurrencyLocks.java b/src/main/java/fr/traqueur/currencies/CurrencyLocks.java new file mode 100644 index 0000000..d62bf73 --- /dev/null +++ b/src/main/java/fr/traqueur/currencies/CurrencyLocks.java @@ -0,0 +1,49 @@ +package fr.traqueur.currencies; + +import java.util.UUID; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; + +final class CurrencyLocks { + + private static final int STRIPES = 64; + private static final long LOCK_TIMEOUT_MILLIS = 250L; + private static final ReentrantLock[] LOCKS = new ReentrantLock[STRIPES]; + + static { + for (int i = 0; i < STRIPES; i++) { + LOCKS[i] = new ReentrantLock(); + } + } + + private CurrencyLocks() { + } + + /** + * Resolve the lock guarding a given provider and player pair. + * + * @param provider The provider performing the operation. + * @param playerId The player being debited. + * @return The lock to use. + */ + static ReentrantLock lockFor(CurrencyProvider provider, UUID playerId) { + int hash = System.identityHashCode(provider) * 31 + (playerId == null ? 0 : playerId.hashCode()); + hash ^= (hash >>> 16); + return LOCKS[hash & (STRIPES - 1)]; + } + + /** + * Try to acquire a lock within the configured timeout. + * + * @param lock The lock to acquire. + * @return True when the lock was acquired and must be released by the caller. + */ + static boolean tryLock(ReentrantLock lock) { + try { + return lock.tryLock(LOCK_TIMEOUT_MILLIS, TimeUnit.MILLISECONDS); + } catch (InterruptedException exception) { + Thread.currentThread().interrupt(); + return false; + } + } +} diff --git a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java index 49873f8..7afb949 100644 --- a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java +++ b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java @@ -1,7 +1,11 @@ package fr.traqueur.currencies; +import org.bukkit.Bukkit; + import java.math.BigDecimal; import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.locks.ReentrantLock; /** * Interface used to interact with a currency provider. @@ -36,4 +40,127 @@ public interface CurrencyProvider { */ BigDecimal getBalance(UUID playerId); + /** + * Whether this provider can check the balance and apply the debit as one indivisible + * operation. + * + *When this returns false, {@link #withdrawIfSufficient} still works but is emulated by + * the library: the sequence is serialized inside this JVM, which stops one server racing + * itself, but it cannot stop a second server acting on the same shared economy.
+ * + * @return True when the backend itself guarantees the operation. + */ + default boolean hasNativeConditionalWithdraw() { + return false; + } + + /** + * Whether this provider must be used from the main server thread. + * + *This defaults to true on purpose. Most Bukkit plugin APIs are not thread safe, + * and a provider that reads or writes live player state, such as inventory contents or + * experience levels, will corrupt that state if it is touched from another thread. Assuming + * the unsafe case by default means an unknown third party provider is never called off the + * main thread by accident.
+ * + *Override this to return false only when the backend is documented as safe for + * concurrent access, for example one that talks to Redis or to its own thread safe storage. + * Doing so lets {@link #withdrawIfSufficientAsync} keep the work off the main thread.
+ * + * @return True when every call has to happen on the main server thread. + */ + default boolean requiresMainThread() { + return true; + } + + /** + * Debit a player, but only if the funds are actually available. + * + *This is the operation to use for a purchase. Unlike a balance check followed by a + * separate {@link #withdraw}, nothing can slip between the two steps.
+ * + *The default implementation emulates the operation by reading the balance and then + * withdrawing, with the whole sequence held under a lock so that two threads cannot both + * observe the same balance and both debit. The returned result reports + * {@link TransactionResult#isBackendGuaranteed()} as false to make that limitation visible.
+ * + * @param playerId The UUID of the player to debit. + * @param amount The amount to debit, must be strictly positive. + * @param reason The reason of the withdrawal. + * @return The outcome. Nothing is debited unless the status is + * {@link TransactionResult.Status#SUCCESS}. + */ + default TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, String reason) { + TransactionResult invalid = CurrencyArgumentChecks.findProblem(playerId, amount); + if (invalid != null) { + return invalid; + } + + ReentrantLock lock = CurrencyLocks.lockFor(this, playerId); + if (!CurrencyLocks.tryLock(lock)) { + return TransactionResult.failed(amount, "Timed out waiting for a concurrent operation on the same balance."); + } + + try { + BigDecimal balance = this.getBalance(playerId); + if (balance == null) { + balance = BigDecimal.ZERO; + } + + if (balance.compareTo(amount) < 0) { + return TransactionResult.emulatedInsufficientFunds(amount, balance); + } + + this.withdraw(playerId, amount, reason); + return TransactionResult.emulatedSuccess(amount, balance.subtract(amount)); + } catch (Exception exception) { + return TransactionResult.failed(amount, "The backend threw while withdrawing: " + exception.getMessage()); + } finally { + lock.unlock(); + } + } + + /** + * Asynchronous variant of {@link #withdrawIfSufficient}. + * + *For a provider that must run on the main server thread, the work is scheduled back onto + * it, which requires {@link CurrenciesAPI#init(org.bukkit.plugin.Plugin)} to have been called. + * Without it the returned result is a failure rather than an unsafe off thread call.
+ * + * @param playerId The UUID of the player to debit. + * @param amount The amount to debit, must be strictly positive. + * @param reason The reason of the withdrawal. + * @return A future completed with the outcome. The future itself never completes + * exceptionally, failures are reported through the result. + */ + default CompletableFuture{@code
+ * CurrencyRegistry.register("my_gems", new MyGemsProvider());
+ *
+ * TransactionResult result = CurrencyRegistry.require("my_gems")
+ * .withdrawIfSufficient(playerId, BigDecimal.TEN, "Shop purchase");
+ * if (result.isSuccess()) {
+ * // give the goods
+ * }
+ * }
+ *
+ * A custom provider only has to implement {@code deposit}, {@code withdraw} and + * {@code getBalance}. It inherits an emulated {@link CurrencyProvider#withdrawIfSufficient} which + * is serialized inside this JVM. If the backend can refuse a withdrawal itself, override + * {@code withdrawIfSufficient} and {@code hasNativeConditionalWithdraw} to get a real guarantee. If the + * backend is safe to use off the main server thread, also override + * {@link CurrencyProvider#requiresMainThread()} to return false.
+ */ +public final class CurrencyRegistry { + + private static final MapUse this from a provider that overrides + * {@link CurrencyProvider#withdrawIfSufficient(java.util.UUID, BigDecimal, String)} because its + * backend can refuse a withdrawal on its own.
+ * + * @param amount The amount that was debited. + * @param balance The resulting balance, or null when the backend does not report it. + * @return The result. + */ + public static TransactionResult nativeSuccess(BigDecimal amount, BigDecimal balance) { + return new TransactionResult(Status.SUCCESS, amount, balance, null, true); + } + + /** + * Build a successful result for an operation the library emulated with a balance read followed + * by a withdraw. + * + * @param amount The amount that was debited. + * @param balance The resulting balance, or null when it is not known. + * @return The result. + */ + public static TransactionResult emulatedSuccess(BigDecimal amount, BigDecimal balance) { + return new TransactionResult(Status.SUCCESS, amount, balance, null, false); + } + + /** + * Build an insufficient funds result for a backend that made the decision itself. Nothing was + * debited. + * + * @param amount The amount that was requested. + * @param balance The balance that was observed, or null when it is not known. + * @return The result. + */ + public static TransactionResult nativeInsufficientFunds(BigDecimal amount, BigDecimal balance) { + return new TransactionResult(Status.INSUFFICIENT_FUNDS, amount, balance, null, true); + } + + /** + * Build an insufficient funds result for a check the library performed itself. Nothing was + * debited. + * + * @param amount The amount that was requested. + * @param balance The balance that was observed, or null when it is not known. + * @return The result. + */ + public static TransactionResult emulatedInsufficientFunds(BigDecimal amount, BigDecimal balance) { + return new TransactionResult(Status.INSUFFICIENT_FUNDS, amount, balance, null, false); + } + + /** + * Build a result for a backend that cannot perform this operation. Nothing was debited. + * + * @param amount The amount that was requested. + * @param errorMessage A human readable explanation. + * @return The result. + */ + public static TransactionResult unsupported(BigDecimal amount, String errorMessage) { + return new TransactionResult(Status.UNSUPPORTED, amount, null, errorMessage, false); + } + + /** + * Build a failed result. Nothing was debited. + * + * @param amount The amount that was requested. + * @param errorMessage A human readable explanation. + * @return The result. + */ + public static TransactionResult failed(BigDecimal amount, String errorMessage) { + return new TransactionResult(Status.FAILED, amount, null, errorMessage, false); + } + + /** + * @return The outcome of the operation. + */ + public Status getStatus() { + return this.status; + } + + /** + * @return True only when the funds were actually debited. + */ + public boolean isSuccess() { + return this.status == Status.SUCCESS; + } + + /** + * @return True when the backend guaranteed that the check and the debit were indivisible. + * False means the library emulated the operation and it is only safe against concurrent + * access from inside this server. + */ + public boolean isBackendGuaranteed() { + return this.backendGuaranteed; + } + + /** + * @return The amount that was requested. + */ + public BigDecimal getAmount() { + return this.amount; + } + + /** + * @return The resulting balance, or null when the backend does not report one. + */ + public BigDecimal getBalance() { + return this.balance; + } + + /** + * @return A human-readable explanation for a failure, or null. + */ + public String getErrorMessage() { + return this.errorMessage; + } + + @Override + public String toString() { + return "TransactionResult{status=" + this.status + + ", amount=" + this.amount + + ", balance=" + this.balance + + ", backendGuaranteed=" + this.backendGuaranteed + + (this.errorMessage == null ? "" : ", error='" + this.errorMessage + "'") + + '}'; + } +} diff --git a/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java b/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java index 3b26755..4116f7e 100644 --- a/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java +++ b/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java @@ -1,13 +1,18 @@ package fr.traqueur.currencies.providers; +import fr.traqueur.currencies.CurrencyArgumentChecks; import fr.traqueur.currencies.CurrencyProvider; +import fr.traqueur.currencies.TransactionResult; import org.bukkit.Bukkit; import org.bukkit.entity.Player; import org.bukkit.plugin.RegisteredServiceProvider; import su.nightexpress.excellenteconomy.api.ExcellentEconomyAPI; import su.nightexpress.excellenteconomy.api.currency.operation.OperationContext; +import su.nightexpress.excellenteconomy.api.currency.operation.OperationResult; import java.math.BigDecimal; +import java.util.concurrent.CompletableFuture; +import java.util.function.BiFunction; import java.util.UUID; public class ExcellentEconomyProvider implements CurrencyProvider { @@ -53,4 +58,67 @@ public BigDecimal getBalance(UUID playerId) { : this.api.getBalanceAsync(playerId, this.currencyName).join(); return BigDecimal.valueOf(raw); } + + @Override + public boolean hasNativeConditionalWithdraw() { + return true; + } + + @Override + public TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, String reason) { + TransactionResult invalid = CurrencyArgumentChecks.findProblem(playerId, amount); + if (invalid != null) { + return invalid; + } + + try { + OperationContext ctx = OperationContext.custom(reason); + Player player = Bukkit.getPlayer(playerId); + + boolean success; + if (player != null) { + success = this.api.withdraw(player, this.currencyName, amount.doubleValue(), ctx); + } else { + OperationResult result = this.api.withdrawAsync(playerId, this.currencyName, amount.doubleValue(), ctx).join(); + success = result != null && result.success(); + } + + if (success) { + return TransactionResult.nativeSuccess(amount, this.getBalance(playerId)); + } + return TransactionResult.nativeInsufficientFunds(amount, this.getBalance(playerId)); + } catch (Exception exception) { + return TransactionResult.failed(amount, "ExcellentEconomy threw while withdrawing: " + exception.getMessage()); + } + } + + @Override + public CompletableFutureWorth checking on a network where several servers share one economy: an emulated - * operation is only protected against races inside this server.
+ *Useful for inspecting a provider's capabilities, and the point + * {@link CurrencyRegistry#resolve(String, String)} bridges to so that a built-in currency and a + * custom one can be looked up the same way.
* * @param currencyName The name of the currency. - * @return True when the backend itself guarantees the operation. + * @return The provider. */ - public boolean hasNativeConditionalWithdraw(String currencyName) { + @NotNull + public CurrencyProvider getProvider(String currencyName) { this.canBeUse(currencyName); - return this.providers.get(currencyName).hasNativeConditionalWithdraw(); + return this.providers.get(currencyName); + } + + public Guarantee getWithdrawGuarantee(String currencyName) { + this.canBeUse(currencyName); + return this.providers.get(currencyName).getWithdrawGuarantee(); } private void canBeUse(String currencyName) { diff --git a/src/main/java/fr/traqueur/currencies/CurrencyExecutor.java b/src/main/java/fr/traqueur/currencies/CurrencyExecutor.java new file mode 100644 index 0000000..2bdca2e --- /dev/null +++ b/src/main/java/fr/traqueur/currencies/CurrencyExecutor.java @@ -0,0 +1,44 @@ +package fr.traqueur.currencies; + +import java.util.concurrent.Executor; +import java.util.concurrent.Executors; +import java.util.concurrent.ThreadFactory; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * The thread pool used by the asynchronous currency operations. + * + *Deliberately not the common {@link java.util.concurrent.ForkJoinPool}, which is what + * {@code CompletableFuture.supplyAsync} uses when no executor is given. That pool is shared with + * everything else in the JVM and sized for CPU bound work, so on a machine with few cores its + * parallelism can be as low as one. A withdrawal waiting on a SQL or Redis round trip would then + * block the whole queue, including work that has nothing to do with currencies.
+ * + *The threads are daemons, so they never hold the server open on shutdown, and they are named so + * that a thread dump from a user reporting lag is actually readable.
+ */ +final class CurrencyExecutor { + private static final int THREADS = Math.max(2, Math.min(8, Runtime.getRuntime().availableProcessors())); + + private static final Executor EXECUTOR = Executors.newFixedThreadPool(THREADS, new ThreadFactory() { + + private final AtomicInteger counter = new AtomicInteger(); + + @Override + public Thread newThread(Runnable runnable) { + Thread thread = new Thread(runnable, "CurrenciesAPI-async-" + this.counter.incrementAndGet()); + thread.setDaemon(true); + return thread; + } + }); + + private CurrencyExecutor() { + } + + /** + * @return The executor for asynchronous currency operations. + */ + static Executor get() { + return EXECUTOR; + } +} diff --git a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java index 7afb949..7b01a81 100644 --- a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java +++ b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java @@ -41,17 +41,21 @@ public interface CurrencyProvider { BigDecimal getBalance(UUID playerId); /** - * Whether this provider can check the balance and apply the debit as one indivisible - * operation. + * How strong a promise this provider can make about {@link #withdrawIfSufficient}. * - *When this returns false, {@link #withdrawIfSufficient} still works but is emulated by - * the library: the sequence is serialized inside this JVM, which stops one server racing - * itself, but it cannot stop a second server acting on the same shared economy.
+ *Every provider supports the operation, this only says who guarantee it. The default is + * {@link Guarantee#EMULATED}, meaning the library performs the check and the debit itself under + * a lock, which stops one server racing itself but cannot stop a second server acting on the + * same shared economy.
* - * @return True when the backend itself guarantees the operation. + *Override with {@link Guarantee#DELEGATED} when the backend reports the outcome of the + * withdrawal but does not promise the check and the debit were indivisible, and with + * {@link Guarantee#NATIVE} only when it validates inside storage that every server shares.
+ * + * @return The level of guarantee behind a conditional withdrawal. */ - default boolean hasNativeConditionalWithdraw() { - return false; + default Guarantee getWithdrawGuarantee() { + return Guarantee.EMULATED; } /** @@ -82,7 +86,8 @@ default boolean requiresMainThread() { *The default implementation emulates the operation by reading the balance and then * withdrawing, with the whole sequence held under a lock so that two threads cannot both * observe the same balance and both debit. The returned result reports - * {@link TransactionResult#isBackendGuaranteed()} as false to make that limitation visible.
+ * {@link TransactionResult#getGuarantee()} as {@link Guarantee#EMULATED} to make that limitation + * visible. * * @param playerId The UUID of the player to debit. * @param amount The amount to debit, must be strictly positive. @@ -98,7 +103,7 @@ default TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, ReentrantLock lock = CurrencyLocks.lockFor(this, playerId); if (!CurrencyLocks.tryLock(lock)) { - return TransactionResult.failed(amount, "Timed out waiting for a concurrent operation on the same balance."); + return TransactionResult.failed(amount, "Timed out waiting for the currency lock, nothing was taken."); } try { @@ -108,11 +113,11 @@ default TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, } if (balance.compareTo(amount) < 0) { - return TransactionResult.emulatedInsufficientFunds(amount, balance); + return TransactionResult.insufficientFunds(amount, balance, Guarantee.EMULATED); } this.withdraw(playerId, amount, reason); - return TransactionResult.emulatedSuccess(amount, balance.subtract(amount)); + return TransactionResult.success(amount, balance.subtract(amount), Guarantee.EMULATED); } catch (Exception exception) { return TransactionResult.failed(amount, "The backend threw while withdrawing: " + exception.getMessage()); } finally { @@ -140,7 +145,7 @@ default CompletableFutureA custom provider only has to implement {@code deposit}, {@code withdraw} and * {@code getBalance}. It inherits an emulated {@link CurrencyProvider#withdrawIfSufficient} which * is serialized inside this JVM. If the backend can refuse a withdrawal itself, override - * {@code withdrawIfSufficient} and {@code hasNativeConditionalWithdraw} to get a real guarantee. If the + * {@code withdrawIfSufficient} and {@code getWithdrawGuarantee} to report a real guarantee. If the * backend is safe to use off the main server thread, also override * {@link CurrencyProvider#requiresMainThread()} to return false.
*/ @@ -47,10 +47,12 @@ private CurrencyRegistry() { * @throws IllegalStateException if a different provider is already registered under this name. */ public static void register(String name, CurrencyProvider provider) { - String key = normalize(name); + // Provider checked before the name, so register(null, null) reports the provider rather + // than blaming the name. if (provider == null) { throw new IllegalArgumentException("The provider cannot be null."); } + String key = normalize(name); CurrencyProvider existing = PROVIDERS.putIfAbsent(key, provider); if (existing != null && existing != provider) { @@ -66,11 +68,10 @@ public static void register(String name, CurrencyProvider provider) { * @return The provider that was previously registered, or null. */ public static CurrencyProvider registerOrReplace(String name, CurrencyProvider provider) { - String key = normalize(name); if (provider == null) { throw new IllegalArgumentException("The provider cannot be null."); } - return PROVIDERS.put(key, provider); + return PROVIDERS.put(normalize(name), provider); } /** @@ -120,7 +121,12 @@ public static boolean isRegistered(String name) { } /** - * @return The names of every registered custom currency. The returned set is a snapshot. + * The names of every registered custom currency. + * + *The names come back normalized, trimmed and lower cased, because that is the form + * lookups use. Registering {@code "MyGems"} and reading this back gives {@code "mygems"}.
+ * + * @return A snapshot of the registered names, in their normalized form. */ @NotNull public static SetThis is the bridge between the two. Without it a caller holding a currency name from a + * configuration file would have to know in advance which of the two mechanisms it belongs to, + * and there would be two unrelated ways to do the same thing.
+ * + *The built in constants win when a name matches both, so a custom registration cannot + * silently shadow {@code VAULT}. Note that a built in currency also needs a currency name for + * the economies that support several, which is why {@code currencyName} is separate: it is + * ignored for a custom provider, which is registered per currency already.
+ * + * @param name The currency name, either a {@link Currencies} constant or a registered custom name. + * @param currencyName The sub currency for a built in provider, or null for the default one. + * @return The provider backing that name. + * @throws IllegalStateException if the name matches neither. + */ + @NotNull + public static CurrencyProvider resolve(String name, String currencyName) { + if (name != null) { + try { + Currencies currency = Currencies.fromName(name.trim().toUpperCase(java.util.Locale.ROOT)); + return currency.getProvider(currencyName == null ? Currencies.DEFAULT_CURRENCY_NAME : currencyName); + } catch (IllegalArgumentException ignored) { + } + } + + CurrencyProvider provider = find(name); + if (provider == null) { + throw new IllegalStateException("No currency named " + name + " is known, either as a built in " + + "Currencies constant or as a provider registered with CurrencyRegistry.register(name, provider)."); + } + return provider; + } + private static String normalize(String name) { if (name == null || name.trim().isEmpty()) { throw new IllegalArgumentException("The currency name cannot be null or blank."); diff --git a/src/main/java/fr/traqueur/currencies/Guarantee.java b/src/main/java/fr/traqueur/currencies/Guarantee.java new file mode 100644 index 0000000..1222ed5 --- /dev/null +++ b/src/main/java/fr/traqueur/currencies/Guarantee.java @@ -0,0 +1,49 @@ +package fr.traqueur.currencies; + +/** + * How strong the promise behind a conditional currency operation actually is. + * + *A two state boolean was not enough to describe the backends honestly. Some of them validate + * the funds inside their own shared storage, some merely report an outcome without promising the + * check and the debit were indivisible, and for the rest this library has to emulate the operation. + * Collapsing the middle case into "guaranteed" told callers they were safe against a cross server + * double spend when they were not.
+ */ +public enum Guarantee { + + /** + * The backend applied the check and the debit as one indivisible operation, inside storage that + * every server shares. This is the only level that is safe against a second server acting on + * the same balance at the same time. + */ + NATIVE, + + /** + * The backend reported whether the withdrawal succeeded, but does not promise that the check + * and the debit were indivisible. + * + *Vault is the clearest example: {@code withdrawPlayer} returns a response, but Vault is an + * abstraction over whichever economy plugin is installed, and most of them do a plain read, + * modify and write. The result is trustworthy for a single request, and it is better than an + * emulated check because the decision was made by the thing that owns the money, but it is not + * a cross server guarantee.
+ */ + DELEGATED, + + /** + * This library performed the check and the debit itself, serialized under a lock. + * + *Protects one server against racing itself. A second server sharing the same economy does + * not see that lock.
+ */ + EMULATED; + + /** + * Whether this level is safe when several servers share one economy. + * + * @return True only for {@link #NATIVE}. + */ + public boolean isCrossServerSafe() { + return this == NATIVE; + } +} diff --git a/src/main/java/fr/traqueur/currencies/TransactionResult.java b/src/main/java/fr/traqueur/currencies/TransactionResult.java index 99adf53..c2a5033 100644 --- a/src/main/java/fr/traqueur/currencies/TransactionResult.java +++ b/src/main/java/fr/traqueur/currencies/TransactionResult.java @@ -1,5 +1,8 @@ package fr.traqueur.currencies; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + import java.math.BigDecimal; public final class TransactionResult { @@ -24,9 +27,15 @@ public enum Status { UNSUPPORTED, /** - * The operation could not be completed for any other reason, for example the - * economy plugin returned an error or the player data could not be loaded. - * Nothing was debited. + * The operation could not be completed for any other reason, for example the economy + * plugin returned an error or the player data could not be loaded. + * + *Nothing was debited in the ordinary case. The one exception worth knowing about is a + * backend that throws after it has already applied the withdrawal, for instance a + * committed transaction followed by an error on the way back. The library cannot tell that + * apart from a clean failure, so a caller handing out something valuable should treat a + * FAILED result as "no goods, and worth logging" rather than as proof the money is + * untouched.
*/ FAILED } @@ -35,65 +44,38 @@ public enum Status { private final BigDecimal amount; private final BigDecimal balance; private final String errorMessage; - private final boolean backendGuaranteed; + private final Guarantee guarantee; - private TransactionResult(Status status, BigDecimal amount, BigDecimal balance, String errorMessage, boolean backendGuaranteed) { + private TransactionResult(Status status, BigDecimal amount, BigDecimal balance, String errorMessage, Guarantee guarantee) { this.status = status; this.amount = amount == null ? BigDecimal.ZERO : amount; this.balance = balance; this.errorMessage = errorMessage; - this.backendGuaranteed = backendGuaranteed; - } - - /** - * Build a successful result for a backend that applied the check and the debit itself. - * - *Use this from a provider that overrides - * {@link CurrencyProvider#withdrawIfSufficient(java.util.UUID, BigDecimal, String)} because its - * backend can refuse a withdrawal on its own.
- * - * @param amount The amount that was debited. - * @param balance The resulting balance, or null when the backend does not report it. - * @return The result. - */ - public static TransactionResult nativeSuccess(BigDecimal amount, BigDecimal balance) { - return new TransactionResult(Status.SUCCESS, amount, balance, null, true); - } - - /** - * Build a successful result for an operation the library emulated with a balance read followed - * by a withdraw. - * - * @param amount The amount that was debited. - * @param balance The resulting balance, or null when it is not known. - * @return The result. - */ - public static TransactionResult emulatedSuccess(BigDecimal amount, BigDecimal balance) { - return new TransactionResult(Status.SUCCESS, amount, balance, null, false); + this.guarantee = guarantee; } /** - * Build an insufficient funds result for a backend that made the decision itself. Nothing was - * debited. + * Build a successful result. * - * @param amount The amount that was requested. - * @param balance The balance that was observed, or null when it is not known. + * @param amount The amount that was debited. + * @param balance The resulting balance, or null when it is not known. + * @param guarantee How strong the promise behind the operation is. * @return The result. */ - public static TransactionResult nativeInsufficientFunds(BigDecimal amount, BigDecimal balance) { - return new TransactionResult(Status.INSUFFICIENT_FUNDS, amount, balance, null, true); + public static TransactionResult success(BigDecimal amount, BigDecimal balance, Guarantee guarantee) { + return new TransactionResult(Status.SUCCESS, amount, balance, null, guarantee); } /** - * Build an insufficient funds result for a check the library performed itself. Nothing was - * debited. + * Build a result for a player who could not afford the amount. Nothing was debited. * - * @param amount The amount that was requested. - * @param balance The balance that was observed, or null when it is not known. + * @param amount The amount that was requested. + * @param balance The balance that was observed, or null when it is not known. + * @param guarantee How strong the promise behind the check is. * @return The result. */ - public static TransactionResult emulatedInsufficientFunds(BigDecimal amount, BigDecimal balance) { - return new TransactionResult(Status.INSUFFICIENT_FUNDS, amount, balance, null, false); + public static TransactionResult insufficientFunds(BigDecimal amount, BigDecimal balance, Guarantee guarantee) { + return new TransactionResult(Status.INSUFFICIENT_FUNDS, amount, balance, null, guarantee); } /** @@ -104,7 +86,7 @@ public static TransactionResult emulatedInsufficientFunds(BigDecimal amount, Big * @return The result. */ public static TransactionResult unsupported(BigDecimal amount, String errorMessage) { - return new TransactionResult(Status.UNSUPPORTED, amount, null, errorMessage, false); + return new TransactionResult(Status.UNSUPPORTED, amount, null, errorMessage, Guarantee.EMULATED); } /** @@ -115,12 +97,13 @@ public static TransactionResult unsupported(BigDecimal amount, String errorMessa * @return The result. */ public static TransactionResult failed(BigDecimal amount, String errorMessage) { - return new TransactionResult(Status.FAILED, amount, null, errorMessage, false); + return new TransactionResult(Status.FAILED, amount, null, errorMessage, Guarantee.EMULATED); } /** * @return The outcome of the operation. */ + @NotNull public Status getStatus() { return this.status; } @@ -133,17 +116,19 @@ public boolean isSuccess() { } /** - * @return True when the backend guaranteed that the check and the debit were indivisible. - * False means the library emulated the operation and it is only safe against concurrent - * access from inside this server. + * @return How strong the promise behind this operation was. Use + * {@link Guarantee#isCrossServerSafe()} to decide whether it holds on a network where several + * servers share one economy. */ - public boolean isBackendGuaranteed() { - return this.backendGuaranteed; + @NotNull + public Guarantee getGuarantee() { + return this.guarantee; } /** * @return The amount that was requested. */ + @NotNull public BigDecimal getAmount() { return this.amount; } @@ -151,6 +136,7 @@ public BigDecimal getAmount() { /** * @return The resulting balance, or null when the backend does not report one. */ + @Nullable public BigDecimal getBalance() { return this.balance; } @@ -158,6 +144,7 @@ public BigDecimal getBalance() { /** * @return A human-readable explanation for a failure, or null. */ + @Nullable public String getErrorMessage() { return this.errorMessage; } @@ -167,7 +154,7 @@ public String toString() { return "TransactionResult{status=" + this.status + ", amount=" + this.amount + ", balance=" + this.balance - + ", backendGuaranteed=" + this.backendGuaranteed + + ", guarantee=" + this.guarantee + (this.errorMessage == null ? "" : ", error='" + this.errorMessage + "'") + '}'; } diff --git a/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java b/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java index 4116f7e..95faece 100644 --- a/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java +++ b/src/main/java/fr/traqueur/currencies/providers/ExcellentEconomyProvider.java @@ -2,6 +2,7 @@ import fr.traqueur.currencies.CurrencyArgumentChecks; import fr.traqueur.currencies.CurrencyProvider; +import fr.traqueur.currencies.Guarantee; import fr.traqueur.currencies.TransactionResult; import org.bukkit.Bukkit; import org.bukkit.entity.Player; @@ -59,9 +60,18 @@ public BigDecimal getBalance(UUID playerId) { return BigDecimal.valueOf(raw); } + private BigDecimal getBalanceWithoutBukkit(UUID playerId) { + try { + Double raw = this.api.getBalanceAsync(playerId, this.currencyName).join(); + return raw == null ? null : BigDecimal.valueOf(raw); + } catch (Exception exception) { + return null; + } + } + @Override - public boolean hasNativeConditionalWithdraw() { - return true; + public Guarantee getWithdrawGuarantee() { + return Guarantee.NATIVE; } @Override @@ -73,20 +83,20 @@ public TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, try { OperationContext ctx = OperationContext.custom(reason); - Player player = Bukkit.getPlayer(playerId); - - boolean success; - if (player != null) { - success = this.api.withdraw(player, this.currencyName, amount.doubleValue(), ctx); - } else { - OperationResult result = this.api.withdrawAsync(playerId, this.currencyName, amount.doubleValue(), ctx).join(); - success = result != null && result.success(); + + OperationResult result = this.api.withdrawAsync(playerId, this.currencyName, amount.doubleValue(), ctx).join(); + + if (result != null && result.success()) { + return TransactionResult.success(amount, null, Guarantee.NATIVE); } - if (success) { - return TransactionResult.nativeSuccess(amount, this.getBalance(playerId)); + BigDecimal balance = this.getBalanceWithoutBukkit(playerId); + if (balance != null && balance.compareTo(amount) < 0) { + return TransactionResult.insufficientFunds(amount, balance, Guarantee.NATIVE); } - return TransactionResult.nativeInsufficientFunds(amount, this.getBalance(playerId)); + + return TransactionResult.failed(amount, "ExcellentEconomy refused the withdrawal and the player could afford it, " + + "so the currency " + this.currencyName + " may be unknown or the player data may not be loaded."); } catch (Exception exception) { return TransactionResult.failed(amount, "ExcellentEconomy threw while withdrawing: " + exception.getMessage()); } @@ -107,9 +117,14 @@ public CompletableFuture{@link #withdraw} deliberately reads the balance, pauses, then writes the result back. That is + * how a real backend behaves when the write goes over a network, and it is what makes an + * unsynchronized check-then-withdraw lose updates. Without the pause the race almost never shows up + * and the test would pass for the wrong reason.
+ */ +final class FakeProvider implements CurrencyProvider { + + private final MapBackends that cannot perform an atomic "debit only if the funds are there" have to be emulated + * as a balance read followed by a withdraw. Two threads running that sequence at the same time for + * the same player can both observe the same balance and both debit, which is the classic double + * spend. Holding a lock for the duration of the sequence removes that window.
+ * + *The lock is keyed on the exact pair being operated on, so two unrelated players never wait on + * each other and a timeout genuinely means there was a competing operation on that same balance. + * An earlier version used a fixed set of striped locks, which was cheap but let independent players + * collide, producing a failed purchase whose message claimed a conflict that had not happened.
+ * + *The map does not grow without bound despite having no fixed size: each entry is reference + * counted and removed as soon as the last holder or waiter is done with it, so its size tracks the + * number of operations currently in flight rather than the number of players ever seen.
+ * + *This only protects against concurrency inside this JVM. When several servers share one + * economy database, a lock held here is invisible to the other servers. That limitation is why an + * emulated result reports {@link Guarantee#EMULATED}.
+ */ final class CurrencyLocks { - private static final int STRIPES = 1024; private static final long LOCK_TIMEOUT_MILLIS = 250L; private static final long MAIN_THREAD_LOCK_TIMEOUT_MILLIS = 25L; - private static final ReentrantLock[] LOCKS = new ReentrantLock[STRIPES]; - static { - for (int i = 0; i < STRIPES; i++) { - LOCKS[i] = new ReentrantLock(); - } - } + private static final MapThe provider is identified by its identity hash rather than by equality, because two + * distinct provider instances for the same economy are genuinely separate paths to the same + * money only by coincidence, and providers do not define equals.
+ */ + private static final class Key { + + private final int providerIdentity; + private final UUID playerId; + + private Key(int providerIdentity, UUID playerId) { + this.providerIdentity = providerIdentity; + this.playerId = playerId; + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } + if (!(other instanceof Key)) { + return false; + } + Key key = (Key) other; + return this.providerIdentity == key.providerIdentity + && (Objects.equals(this.playerId, key.playerId)); + } + + @Override + public int hashCode() { + return this.providerIdentity * 31 + (this.playerId == null ? 0 : this.playerId.hashCode()); } } } diff --git a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java index 7b01a81..dcf98f9 100644 --- a/src/main/java/fr/traqueur/currencies/CurrencyProvider.java +++ b/src/main/java/fr/traqueur/currencies/CurrencyProvider.java @@ -5,7 +5,6 @@ import java.math.BigDecimal; import java.util.UUID; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.locks.ReentrantLock; /** * Interface used to interact with a currency provider. @@ -100,10 +99,11 @@ default TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, if (invalid != null) { return invalid; } - - ReentrantLock lock = CurrencyLocks.lockFor(this, playerId); - if (!CurrencyLocks.tryLock(lock)) { - return TransactionResult.failed(amount, "Timed out waiting for the currency lock, nothing was taken."); + + CurrencyLocks.Handle handle = CurrencyLocks.tryAcquire(this, playerId); + if (handle == null) { + return TransactionResult.failed(amount, + "Timed out waiting for another operation on the same balance, nothing was taken."); } try { @@ -121,7 +121,7 @@ default TransactionResult withdrawIfSufficient(UUID playerId, BigDecimal amount, } catch (Exception exception) { return TransactionResult.failed(amount, "The backend threw while withdrawing: " + exception.getMessage()); } finally { - lock.unlock(); + handle.release(); } } diff --git a/src/test/java/fr/traqueur/currencies/CurrencyLocksTest.java b/src/test/java/fr/traqueur/currencies/CurrencyLocksTest.java new file mode 100644 index 0000000..35fe3cd --- /dev/null +++ b/src/test/java/fr/traqueur/currencies/CurrencyLocksTest.java @@ -0,0 +1,144 @@ +package fr.traqueur.currencies; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.math.BigDecimal; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * The lock is keyed per provider and player, so it has two properties worth pinning down: unrelated + * players must never wait on each other, and the entries must not pile up over time. + */ +class CurrencyLocksTest { + + private static final String REASON = "test"; + + @Test + @DisplayName("two different players do not block each other") + void differentPlayersDoNotBlockEachOther() throws Exception { + FakeProvider provider = new FakeProvider(0); + UUID first = UUID.randomUUID(); + + CurrencyLocks.Handle held = CurrencyLocks.tryAcquire(provider, first); + assertNotNull(held, "the first acquisition should succeed"); + + try { + UUID second = UUID.randomUUID(); + CurrencyLocks.Handle other = CurrencyLocks.tryAcquire(provider, second); + assertNotNull(other, "an unrelated player must not be blocked"); + other.release(); + } finally { + held.release(); + } + } + + @Test + @DisplayName("the same player is serialized, and a timeout means a real conflict") + void samePlayerIsSerialized() throws Exception { + FakeProvider provider = new FakeProvider(0); + UUID player = UUID.randomUUID(); + + CurrencyLocks.Handle held = CurrencyLocks.tryAcquire(provider, player); + assertNotNull(held); + + try { + ExecutorService pool = Executors.newSingleThreadExecutor(); + try { + Future