diff --git a/buildSrc/src/main/kotlin/realty-conventions.gradle.kts b/buildSrc/src/main/kotlin/realty-conventions.gradle.kts index b6d3b13..cb123a1 100644 --- a/buildSrc/src/main/kotlin/realty-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/realty-conventions.gradle.kts @@ -3,7 +3,7 @@ plugins { } group = "io.github.md5sha256" -version = "1.4.1" +version = "1.4.2" val targetJavaVersion = 25 diff --git a/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java b/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java index 28fa680..b9b4e3e 100644 --- a/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java +++ b/realty-paper/src/main/java/io/github/md5sha256/realty/economy/TreasuryEconomyProvider.java @@ -9,6 +9,7 @@ import java.math.BigDecimal; import java.math.RoundingMode; import java.util.List; +import java.util.Optional; import java.util.UUID; /** @@ -16,13 +17,14 @@ * each transfer is recorded with a human-readable message that appears * in the player's Treasury transaction history. *

- * Account resolution: the payer is always resolved as a personal account - * (created with starting balance if missing). The recipient is resolved by - * preferring its GOVERNMENT account, then PERSONAL, then BUSINESS — so - * government landlords (legacy DCGovernment-style real UUIDs that own both a - * personal and a government account) route income to their government - * treasury, while ordinary landlords still get their personal balance rather - * than a firm BUSINESS account they happen to own. + * Account resolution is the same on both sides of a transfer: prefer the + * party's GOVERNMENT account, then PERSONAL, then BUSINESS. Government + * entities (legacy DCGovernment-style real UUIDs that own both a personal and + * a government account) therefore both receive income into and pay refunds out + * of their government treasury, while ordinary players resolve to their + * personal balance rather than a firm BUSINESS account they happen to own. + * Balance reads follow the same preference, so an affordability check always + * inspects the account the subsequent transfer would actually touch. */ public final class TreasuryEconomyProvider implements EconomyProvider { @@ -36,10 +38,16 @@ public TreasuryEconomyProvider(@NotNull TreasuryApi treasuryApi) { @Override public double getBalance(@NotNull UUID playerId) { - if (!treasuryApi.hasAccountByOwnerUuid(playerId)) { + // Read the same account transfer() would debit, not whichever one + // getBalanceByOwnerUuid happens to pick -- otherwise a government + // entity is checked for affordability against its personal balance. + // A read must not open an account, so there is no create-if-missing + // fallback here: no accounts means no funds. + Account account = preferredAccount(treasuryApi.getAccountsByOwner(playerId)).orElse(null); + if (account == null) { return 0.0; } - BigDecimal balance = treasuryApi.getBalanceByOwnerUuid(playerId); + BigDecimal balance = treasuryApi.getBalanceByAccountId(account.getAccountId()); return balance != null ? balance.doubleValue() : 0.0; } @@ -47,8 +55,10 @@ public double getBalance(@NotNull UUID playerId) { public @NotNull PaymentResult transfer(@NotNull UUID fromId, @NotNull UUID toId, double amount, @NotNull String ledgerMessage) { try { - Account payer = treasuryApi.resolveOrCreatePersonal(fromId); - Account recipient = resolveRecipientAccount(toId); + // Both sides resolve identically: a refund from a government landlord + // must leave the same account the rent was paid into. + Account payer = resolveAccount(fromId); + Account recipient = resolveAccount(toId); // Treasury rejects amounts with more than 2 decimal places. Amounts // derived from arithmetic (e.g. pro-rata refunds: price * remaining / // total) can carry extra precision, so normalise to 2 decimals here. @@ -80,37 +90,50 @@ public boolean hasLedgerSupport() { } /** - * Resolves the recipient's Treasury account, preferring + * Resolves a party's Treasury account, preferring * GOVERNMENT > PERSONAL > BUSINESS > first-available. *

* GOVERNMENT wins first because legacy government entities (e.g. * DCGovernment) are real Minecraft accounts whose UUID owns both a * personal and a government account; their leasehold income must land in - * the government treasury, not the player's personal balance. + * the government treasury, not the player's personal balance — and, on the + * paying side, a lease-termination refund must be debited from that same + * treasury rather than the entity's personal balance. *

- * Ordinary landlords have no government account, so PERSONAL is chosen next: + * Ordinary players have no government account, so PERSONAL is chosen next: * rental/sale income belongs to them personally, never a firm BUSINESS * account they happen to own (firm accounts are owned by the proprietor's * own UUID, which is how such funds previously leaked into business * accounts). *

- * When the recipient has no account at all, resolve-or-create their personal + * When the party has no account at all, resolve-or-create their personal * account. */ - private @NotNull Account resolveRecipientAccount(@NotNull UUID ownerUuid) { - List accounts = treasuryApi.getAccountsByOwner(ownerUuid); - if (!accounts.isEmpty()) { - return accounts.stream() - .filter(a -> a.getAccountType() == AccountType.GOVERNMENT) - .findFirst() - .or(() -> accounts.stream() - .filter(a -> a.getAccountType() == AccountType.PERSONAL) - .findFirst()) - .or(() -> accounts.stream() - .filter(a -> a.getAccountType() == AccountType.BUSINESS) - .findFirst()) - .orElse(accounts.get(0)); + private @NotNull Account resolveAccount(@NotNull UUID ownerUuid) { + return preferredAccount(treasuryApi.getAccountsByOwner(ownerUuid)) + .orElseGet(() -> treasuryApi.resolveOrCreatePersonal(ownerUuid)); + } + + /** + * Applies the GOVERNMENT > PERSONAL > BUSINESS > first-available + * preference to an already-fetched account list, or empty when the party + * holds no accounts at all. Shared by {@link #resolveAccount(UUID)} and + * {@link #getBalance(UUID)} so a balance check and the transfer it gates + * can never disagree about which account is in play. + */ + private @NotNull Optional preferredAccount(@NotNull List accounts) { + if (accounts.isEmpty()) { + return Optional.empty(); } - return treasuryApi.resolveOrCreatePersonal(ownerUuid); + return Optional.of(accounts.stream() + .filter(a -> a.getAccountType() == AccountType.GOVERNMENT) + .findFirst() + .or(() -> accounts.stream() + .filter(a -> a.getAccountType() == AccountType.PERSONAL) + .findFirst()) + .or(() -> accounts.stream() + .filter(a -> a.getAccountType() == AccountType.BUSINESS) + .findFirst()) + .orElse(accounts.get(0))); } } diff --git a/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java b/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java index aaf98c5..03b56f7 100644 --- a/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java +++ b/realty-paper/src/test/java/io/github/md5sha256/realty/economy/TreasuryEconomyProviderTest.java @@ -18,6 +18,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -30,6 +31,7 @@ class TreasuryEconomyProviderTest { private TreasuryEconomyProvider provider; private final UUID payer = UUID.randomUUID(); + private final UUID recipient = UUID.randomUUID(); @BeforeEach void setUp() { @@ -46,7 +48,7 @@ private Account account(int id, AccountType type, UUID owner) { private int capturedDestination(UUID recipient) { Account payerPersonal = account(1, AccountType.PERSONAL, payer); - when(treasuryApi.resolveOrCreatePersonal(payer)).thenReturn(payerPersonal); + when(treasuryApi.getAccountsByOwner(payer)).thenReturn(List.of(payerPersonal)); when(treasuryApi.transfer(any())).thenReturn(99L); PaymentResult result = provider.transfer(payer, recipient, 50.0, "Rental Payment: REGION"); @@ -107,4 +109,96 @@ void recipientWithNoAccounts_resolvesOrCreatesPersonal() { assertEquals(88, capturedDestination(newOwner)); } + + private int capturedSource(UUID payerUuid) { + Account recipientPersonal = account(2, AccountType.PERSONAL, recipient); + when(treasuryApi.getAccountsByOwner(recipient)).thenReturn(List.of(recipientPersonal)); + when(treasuryApi.transfer(any())).thenReturn(99L); + + PaymentResult result = provider.transfer(payerUuid, recipient, 50.0, "Lease Termination Refund: REGION"); + assertInstanceOf(PaymentResult.Success.class, result); + + ArgumentCaptor req = ArgumentCaptor.forClass(TransferRequest.class); + verify(treasuryApi).transfer(req.capture()); + assertEquals(recipientPersonal.getAccountId(), req.getValue().toAccountId()); + return req.getValue().fromAccountId(); + } + + @Test + void governmentPayer_refundIsDebitedFromGovernmentNotPersonal() { + UUID government = UUID.randomUUID(); + // The mirror of legacyGovernment_withPersonalAndGovernmentAccount_routesToGovernment: + // a refund from a government landlord must leave the same account the rent + // was paid into, not the entity's personal balance. + when(treasuryApi.getAccountsByOwner(government)).thenReturn(List.of( + account(13, AccountType.PERSONAL, government), + account(9, AccountType.GOVERNMENT, government))); + + assertEquals(9, capturedSource(government), + "a government landlord's refund must be debited from the government account"); + } + + @Test + void firmProprietorPayer_paysFromPersonalNotBusiness() { + UUID proprietor = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(proprietor)).thenReturn(List.of( + account(500, AccountType.BUSINESS, proprietor), + account(42, AccountType.PERSONAL, proprietor))); + + assertEquals(42, capturedSource(proprietor), + "an ordinary payer must pay from their personal account, not a firm they own"); + } + + @Test + void payerWithNoAccounts_resolvesOrCreatesPersonal() { + UUID newPayer = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(newPayer)).thenReturn(List.of()); + when(treasuryApi.resolveOrCreatePersonal(newPayer)) + .thenReturn(account(88, AccountType.PERSONAL, newPayer)); + + assertEquals(88, capturedSource(newPayer)); + } + + @Test + void governmentBalance_readsTheGovernmentAccountNotPersonal() { + UUID government = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(government)).thenReturn(List.of( + account(13, AccountType.PERSONAL, government), + account(9, AccountType.GOVERNMENT, government))); + when(treasuryApi.getBalanceByAccountId(9)).thenReturn(new BigDecimal("250.00")); + + assertEquals(250.0, provider.getBalance(government), + "a government entity's balance must be read from the account it transacts with"); + } + + @Test + void firmProprietorBalance_readsPersonalNotBusiness() { + UUID proprietor = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(proprietor)).thenReturn(List.of( + account(500, AccountType.BUSINESS, proprietor), + account(42, AccountType.PERSONAL, proprietor))); + when(treasuryApi.getBalanceByAccountId(42)).thenReturn(new BigDecimal("10.50")); + + assertEquals(10.50, provider.getBalance(proprietor)); + } + + @Test + void balanceWithNoAccounts_isZeroAndCreatesNothing() { + UUID stranger = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(stranger)).thenReturn(List.of()); + + assertEquals(0.0, provider.getBalance(stranger)); + // A balance read must never have the side effect of opening an account. + verify(treasuryApi, never()).resolveOrCreatePersonal(stranger); + } + + @Test + void balanceOfNull_isZero() { + UUID owner = UUID.randomUUID(); + when(treasuryApi.getAccountsByOwner(owner)).thenReturn(List.of( + account(42, AccountType.PERSONAL, owner))); + when(treasuryApi.getBalanceByAccountId(42)).thenReturn(null); + + assertEquals(0.0, provider.getBalance(owner)); + } }