Skip to content

Commit 758d363

Browse files
tastybentoclaude
andcommitted
chore: fix SonarCloud code smells on new code
Main code: - InvEconomy: drop redundant @deprecated from the Vault interface overrides (suppress the unavoidable deprecation/removal warnings at class level), extract duplicated debug literals into constants, and replace nested ternaries in debug routing with a routeSuffix() helper. - Store: merge nested if in island-key migration; replace if/containsKey in getStat UNTYPED branch with Optional.ifPresent. - Remove unused import in InvSwitcherPladdon. Tests: - Remove the 'public' modifier from JUnit 5 test classes and annotated methods (S5786). - Add assertDoesNotThrow assertions to four clear-stored tests that had none. - Remove redundant local Island mocks that shadowed the @mock field. - Remove unused 'server' locals, unused imports, and unnecessary 'throws Exception' declarations. Left intentionally: S110 (command class depth is inherent to BentoBox's CompositeCommand hierarchy), S3776 (cognitive complexity on core inventory methods - refactoring risks regressions), and S5738 (calls to deprecated Bukkit.getOfflinePlayer required to implement Vault's name-based methods). All 124 tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent f1bd4c2 commit 758d363

9 files changed

Lines changed: 220 additions & 200 deletions

File tree

release-notes-1.19.0.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
## 🎁 What's new
2+
3+
1.19.0 is a follow-up to the 1.18.0 per-world economy release. It fixes two issues that stopped the new economy from working cleanly in a common setup. InvSwitcher now functions as a true **standalone economy** — you no longer need a separate economy plugin such as EssentialsX for it to take effect — and admin economy commands now report the **correct balance for offline players** instead of a stale value.
4+
5+
## ✨ Highlights
6+
7+
### 🐛 Standalone economy now works on its own
8+
InvSwitcher registers its own per-world Vault economy, but BentoBox hooks Vault during its early startup, *before* addons enable. When InvSwitcher was the only economy on the server, that early hook found nothing and was discarded, so BentoBox's `getVault()` stayed empty — and economy-dependent addons such as **Bank** disabled themselves with "Vault is required" even though a working per-world economy was present. InvSwitcher now registers a fresh Vault hook with BentoBox once its provider is live, so it works as the server's only economy.
9+
10+
> Companion framework PRs harden this further: [BentoBox #2995](https://github.com/BentoBoxWorld/BentoBox/pull/2995) retries the Vault hook after addons enable, and [Bank #67](https://github.com/BentoBoxWorld/Bank/pull/67) retries before disabling.
11+
12+
### 🐛 Correct balance reported for offline economy transactions
13+
Admin `eco give`, `eco set`, and `eco take` on an offline player reported a stale balance — for example "New balance: 0.00" right after giving 2,000. The money was always stored correctly, but the confirmation message re-read the balance from the database before the asynchronous save had flushed, returning the pre-transaction value. The commands now report the authoritative balance returned by the transaction itself.
14+
15+
This release also hardens the underlying offline read-after-write path: offline saves are tracked while in flight so two rapid sequential transactions on the same offline player can no longer load independent stale copies and lose an update — without blocking the main thread or caching offline players indefinitely.
16+
17+
## ⚙️ Compatibility
18+
19+
✔️ BentoBox 3.17.0
20+
✔️ Paper Minecraft 1.21.5 – 26.1.2
21+
✔️ Java 21
22+
23+
## 📥 How to update
24+
1. Take backups of your server, for safety.
25+
2. Stop the server.
26+
3. Drop the new InvSwitcher jar into the addons folder and remove the old one.
27+
4. Restart the server.
28+
5. You should be good to go!
29+
30+
## What's Changed
31+
* 🐛 Register a fresh Vault hook so InvSwitcher works as a standalone economy by @tastybento in https://github.com/BentoBoxWorld/InvSwitcher/commit/d929649
32+
* 🐛 Report offline economy balances correctly and harden offline saves by @tastybento in https://github.com/BentoBoxWorld/InvSwitcher/commit/a15c645
33+
* Add MC 26.1.2 to Modrinth game-versions by @tastybento in https://github.com/BentoBoxWorld/InvSwitcher/pull/52
34+
35+
**Full Changelog**: https://github.com/BentoBoxWorld/InvSwitcher/compare/1.18.0...1.19.0

src/main/java/com/wasteofplastic/invswitcher/InvSwitcherPladdon.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33

44
import world.bentobox.bentobox.api.addons.Addon;
5-
import world.bentobox.bentobox.api.addons.GameModeAddon;
65
import world.bentobox.bentobox.api.addons.Pladdon;
76

87
public class InvSwitcherPladdon extends Pladdon {

src/main/java/com/wasteofplastic/invswitcher/Store.java

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -261,12 +261,10 @@ public void getInventory(Player player, World world, Island island) {
261261
// Backward compat: if island-specific key has no data, migrate from world-only key.
262262
// This only happens once — the world-only data is cleared after migration so that
263263
// other islands don't also inherit a duplicate copy.
264-
if (islandKey.contains("/") && !store.isInventory(islandKey)) {
265-
if (store.isInventory(worldKey)) {
266-
islandLoadKey = worldKey;
267-
// Clear the world-only data so it can't be claimed by another island
268-
store.clearWorldData(worldKey);
269-
}
264+
if (islandKey.contains("/") && !store.isInventory(islandKey) && store.isInventory(worldKey)) {
265+
islandLoadKey = worldKey;
266+
// Clear the world-only data so it can't be claimed by another island
267+
store.clearWorldData(worldKey);
270268
}
271269

272270
// Each option uses the island key or the world key based on its island sub-setting
@@ -549,11 +547,8 @@ private void getStat(Statistic s, InventoryStorage store, Player player, String
549547
case BLOCK -> store.getBlockStats(worldName).getOrDefault(s, Collections.emptyMap()).forEach((k,v) -> player.setStatistic(s, k, v));
550548
case ITEM -> store.getItemStats(worldName).getOrDefault(s, Collections.emptyMap()).forEach((k,v) -> player.setStatistic(s, k, v));
551549
case ENTITY -> store.getEntityStats(worldName).getOrDefault(s, Collections.emptyMap()).forEach((k,v) -> player.setStatistic(s, k, v));
552-
case UNTYPED -> {
553-
if (store.getUntypedStats(worldName).containsKey(s)) {
554-
player.setStatistic(s, store.getUntypedStats(worldName).get(s));
555-
}
556-
}
550+
case UNTYPED -> Optional.ofNullable(store.getUntypedStats(worldName).get(s))
551+
.ifPresent(v -> player.setStatistic(s, v));
557552
}
558553
}
559554

src/main/java/com/wasteofplastic/invswitcher/economy/InvEconomy.java

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,21 @@
2828
*
2929
* @author tastybento
3030
*/
31+
// The name-based account methods below implement Vault's deprecated Economy interface methods and
32+
// must call Bukkit.getOfflinePlayer(String); both are out of our control, so suppress the warnings.
33+
@SuppressWarnings({ "deprecation", "removal" })
3134
public class InvEconomy implements Economy {
3235

3336
private static final String NEGATIVE_DEPOSIT = "Cannot deposit a negative amount";
3437
private static final String NEGATIVE_WITHDRAW = "Cannot withdraw a negative amount";
3538
private static final String NEGATIVE_SET = "Cannot set a negative balance";
3639
private static final String INSUFFICIENT_FUNDS = "Insufficient funds";
40+
// Debug message fragments, factored out to avoid duplicated string literals.
41+
private static final String DBG_KEY = " key=";
42+
private static final String DBG_AMOUNT = " amount=";
43+
private static final String ROUTE_SELF = " -> self";
44+
private static final String ROUTE_DELEGATE = " -> DELEGATE";
45+
private static final String ROUTE_DEFAULT = " -> default";
3746

3847
private final InvSwitcher addon;
3948
/** The economy to fall back to for unmanaged worlds (e.g. EssentialsX). Resolved lazily on
@@ -120,6 +129,21 @@ private void debug(String msg) {
120129
}
121130
}
122131

132+
/**
133+
* Debug-only suffix describing where a balance call routed: to the player's own managed balance
134+
* ({@code managedSuffix}), or - when the world is unmanaged - to the delegate economy or the
135+
* default key.
136+
* @param key - the resolved money key, or null if the world is unmanaged
137+
* @param managedSuffix - suffix to use when the key is non-null (managed)
138+
* @return the routing suffix for the debug message
139+
*/
140+
private String routeSuffix(String key, String managedSuffix) {
141+
if (key != null) {
142+
return managedSuffix;
143+
}
144+
return delegating() ? ROUTE_DELEGATE : ROUTE_DEFAULT;
145+
}
146+
123147
private static String who(OfflinePlayer player) {
124148
String name = player.getName() != null ? player.getName() : player.getUniqueId().toString();
125149
return name + (player.getPlayer() != null ? " (online)" : " (offline)");
@@ -260,7 +284,7 @@ public boolean hasAccount(OfflinePlayer player, String worldName) {
260284
@Override
261285
public double getBalance(OfflinePlayer player) {
262286
String key = currentKey(player);
263-
debug("getBalance " + who(player) + " key=" + key + (key == null ? (delegating() ? " -> DELEGATE" : " -> default") : ""));
287+
debug("getBalance " + who(player) + DBG_KEY + key + routeSuffix(key, ""));
264288
if (key == null) {
265289
return delegating() ? delegate().getBalance(player) : readSelf(player, Store.DEFAULT_WORLD_KEY);
266290
}
@@ -289,8 +313,7 @@ public boolean has(OfflinePlayer player, String worldName, double amount) {
289313
@Override
290314
public EconomyResponse withdrawPlayer(OfflinePlayer player, double amount) {
291315
String key = currentKey(player);
292-
debug("withdrawPlayer " + who(player) + " amount=" + amount + " key=" + key
293-
+ (key == null ? (delegating() ? " -> DELEGATE" : " -> default") : " -> self"));
316+
debug("withdrawPlayer " + who(player) + DBG_AMOUNT + amount + DBG_KEY + key + routeSuffix(key, ROUTE_SELF));
294317
if (key == null) {
295318
return delegating() ? delegate().withdrawPlayer(player, amount)
296319
: withdrawSelf(player, Store.DEFAULT_WORLD_KEY, amount);
@@ -301,8 +324,8 @@ public EconomyResponse withdrawPlayer(OfflinePlayer player, double amount) {
301324
@Override
302325
public EconomyResponse withdrawPlayer(OfflinePlayer player, String worldName, double amount) {
303326
String key = worldKey(player, worldName);
304-
debug("withdrawPlayer(world) " + who(player) + " world=" + worldName + " amount=" + amount + " key=" + key
305-
+ (key == null ? (delegating() ? " -> DELEGATE" : " -> default") : " -> self"));
327+
debug("withdrawPlayer(world) " + who(player) + " world=" + worldName + DBG_AMOUNT + amount + DBG_KEY + key
328+
+ routeSuffix(key, ROUTE_SELF));
306329
if (key == null) {
307330
return delegating() ? delegate().withdrawPlayer(player, worldName, amount)
308331
: withdrawSelf(player, Store.DEFAULT_WORLD_KEY, amount);
@@ -313,8 +336,7 @@ public EconomyResponse withdrawPlayer(OfflinePlayer player, String worldName, do
313336
@Override
314337
public EconomyResponse depositPlayer(OfflinePlayer player, double amount) {
315338
String key = currentKey(player);
316-
debug("depositPlayer " + who(player) + " amount=" + amount + " key=" + key
317-
+ (key == null ? (delegating() ? " -> DELEGATE" : " -> default") : " -> self"));
339+
debug("depositPlayer " + who(player) + DBG_AMOUNT + amount + DBG_KEY + key + routeSuffix(key, ROUTE_SELF));
318340
if (key == null) {
319341
return delegating() ? delegate().depositPlayer(player, amount)
320342
: depositSelf(player, Store.DEFAULT_WORLD_KEY, amount);
@@ -325,8 +347,8 @@ public EconomyResponse depositPlayer(OfflinePlayer player, double amount) {
325347
@Override
326348
public EconomyResponse depositPlayer(OfflinePlayer player, String worldName, double amount) {
327349
String key = worldKey(player, worldName);
328-
debug("depositPlayer(world) " + who(player) + " world=" + worldName + " amount=" + amount + " key=" + key
329-
+ (key == null ? (delegating() ? " -> DELEGATE" : " -> default") : " -> self"));
350+
debug("depositPlayer(world) " + who(player) + " world=" + worldName + DBG_AMOUNT + amount + DBG_KEY + key
351+
+ routeSuffix(key, ROUTE_SELF));
330352
if (key == null) {
331353
return delegating() ? delegate().depositPlayer(player, worldName, amount)
332354
: depositSelf(player, Store.DEFAULT_WORLD_KEY, amount);
@@ -389,73 +411,61 @@ public boolean createPlayerAccount(OfflinePlayer player, String worldName) {
389411
// ------ DEPRECATED NAME-BASED METHODS (delegate up to OfflinePlayer variants) ------
390412

391413
@Override
392-
@Deprecated
393414
public boolean hasAccount(String playerName) {
394415
return hasAccount(Bukkit.getOfflinePlayer(playerName));
395416
}
396417

397418
@Override
398-
@Deprecated
399419
public boolean hasAccount(String playerName, String worldName) {
400420
return hasAccount(Bukkit.getOfflinePlayer(playerName), worldName);
401421
}
402422

403423
@Override
404-
@Deprecated
405424
public double getBalance(String playerName) {
406425
return getBalance(Bukkit.getOfflinePlayer(playerName));
407426
}
408427

409428
@Override
410-
@Deprecated
411429
public double getBalance(String playerName, String world) {
412430
return getBalance(Bukkit.getOfflinePlayer(playerName), world);
413431
}
414432

415433
@Override
416-
@Deprecated
417434
public boolean has(String playerName, double amount) {
418435
return has(Bukkit.getOfflinePlayer(playerName), amount);
419436
}
420437

421438
@Override
422-
@Deprecated
423439
public boolean has(String playerName, String worldName, double amount) {
424440
return has(Bukkit.getOfflinePlayer(playerName), worldName, amount);
425441
}
426442

427443
@Override
428-
@Deprecated
429444
public EconomyResponse withdrawPlayer(String playerName, double amount) {
430445
return withdrawPlayer(Bukkit.getOfflinePlayer(playerName), amount);
431446
}
432447

433448
@Override
434-
@Deprecated
435449
public EconomyResponse withdrawPlayer(String playerName, String worldName, double amount) {
436450
return withdrawPlayer(Bukkit.getOfflinePlayer(playerName), worldName, amount);
437451
}
438452

439453
@Override
440-
@Deprecated
441454
public EconomyResponse depositPlayer(String playerName, double amount) {
442455
return depositPlayer(Bukkit.getOfflinePlayer(playerName), amount);
443456
}
444457

445458
@Override
446-
@Deprecated
447459
public EconomyResponse depositPlayer(String playerName, String worldName, double amount) {
448460
return depositPlayer(Bukkit.getOfflinePlayer(playerName), worldName, amount);
449461
}
450462

451463
@Override
452-
@Deprecated
453464
public boolean createPlayerAccount(String playerName) {
454465
return createPlayerAccount(Bukkit.getOfflinePlayer(playerName));
455466
}
456467

457468
@Override
458-
@Deprecated
459469
public boolean createPlayerAccount(String playerName, String worldName) {
460470
return createPlayerAccount(Bukkit.getOfflinePlayer(playerName), worldName);
461471
}
@@ -472,7 +482,6 @@ public EconomyResponse createBank(String name, OfflinePlayer player) {
472482
}
473483

474484
@Override
475-
@Deprecated
476485
public EconomyResponse createBank(String name, String player) {
477486
return delegate() != null ? delegate().createBank(name, player) : bankUnsupported();
478487
}
@@ -508,7 +517,6 @@ public EconomyResponse isBankOwner(String name, OfflinePlayer player) {
508517
}
509518

510519
@Override
511-
@Deprecated
512520
public EconomyResponse isBankOwner(String name, String playerName) {
513521
return delegate() != null ? delegate().isBankOwner(name, playerName) : bankUnsupported();
514522
}
@@ -519,7 +527,6 @@ public EconomyResponse isBankMember(String name, OfflinePlayer player) {
519527
}
520528

521529
@Override
522-
@Deprecated
523530
public EconomyResponse isBankMember(String name, String playerName) {
524531
return delegate() != null ? delegate().isBankMember(name, playerName) : bankUnsupported();
525532
}

src/test/java/com/wasteofplastic/invswitcher/InvSwitcherTest.java

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,14 @@
5050
import world.bentobox.bentobox.database.DatabaseSetup.DatabaseType;
5151
import world.bentobox.bentobox.managers.AddonsManager;
5252
import org.mockbukkit.mockbukkit.MockBukkit;
53-
import org.mockbukkit.mockbukkit.ServerMock;
5453

5554
/**
5655
* @author tastybento
5756
*
5857
*/
5958
@ExtendWith(MockitoExtension.class)
6059
@MockitoSettings(strictness = Strictness.LENIENT)
61-
public class InvSwitcherTest {
60+
class InvSwitcherTest {
6261

6362
private static File jFile;
6463
@Mock
@@ -79,7 +78,7 @@ public class InvSwitcherTest {
7978
private MockedStatic<BentoBox> mockedBentoBox;
8079

8180
@BeforeAll
82-
public static void beforeClass() throws IOException {
81+
static void beforeClass() throws IOException {
8382
// Make the addon jar
8483
jFile = new File("addon.jar");
8584
// Copy over config file from src folder
@@ -100,12 +99,9 @@ public static void beforeClass() throws IOException {
10099
}
101100
}
102101

103-
/**
104-
* @throws Exception
105-
*/
106102
@BeforeEach
107-
public void setUp() throws Exception {
108-
ServerMock server = MockBukkit.mock();
103+
void setUp() {
104+
MockBukkit.mock();
109105

110106
// Set up plugin
111107
mockedBentoBox = Mockito.mockStatic(BentoBox.class);
@@ -135,7 +131,7 @@ public void setUp() throws Exception {
135131
* @throws java.lang.Exception
136132
*/
137133
@AfterEach
138-
public void tearDown() throws Exception {
134+
void tearDown() throws Exception {
139135
if (mockedBentoBox != null) {
140136
mockedBentoBox.close();
141137
}
@@ -144,7 +140,7 @@ public void tearDown() throws Exception {
144140
}
145141

146142
@AfterAll
147-
public static void cleanUp() throws Exception {
143+
static void cleanUp() throws Exception {
148144
deleteAll(new File("database"));
149145
new File("addon.jar").delete();
150146
new File("config.yml").delete();
@@ -164,7 +160,7 @@ private static void deleteAll(File file) throws IOException {
164160
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#onEnable()}.
165161
*/
166162
@Test
167-
public void testOnEnable() {
163+
void testOnEnable() {
168164
addon.onEnable();
169165
verify(plugin).logError("[InvSwitcher] This addon is incompatible with YAML database. Please use another type, like JSON.");
170166
assertEquals(State.DISABLED, addon.getState());
@@ -174,7 +170,7 @@ public void testOnEnable() {
174170
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#onDisable()}.
175171
*/
176172
@Test
177-
public void testOnDisable() {
173+
void testOnDisable() {
178174
// Mock the static method
179175
try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
180176
when(Bukkit.getWorld(anyString())).thenReturn(world);
@@ -192,7 +188,7 @@ public void testOnDisable() {
192188
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#onLoad()}.
193189
*/
194190
@Test
195-
public void testOnLoad() {
191+
void testOnLoad() {
196192
addon.onLoad();
197193
File file = new File("config.yml");
198194
assertTrue(file.exists());
@@ -202,7 +198,7 @@ public void testOnLoad() {
202198
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#allLoaded()}.
203199
*/
204200
@Test
205-
public void testAllLoaded() {
201+
void testAllLoaded() {
206202
// Mock the static method
207203
try (MockedStatic<Bukkit> mockedBukkit = mockStatic(Bukkit.class, Mockito.RETURNS_MOCKS)) {
208204
when(Bukkit.getWorld(anyString())).thenReturn(world);
@@ -221,7 +217,7 @@ public void testAllLoaded() {
221217
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#allLoaded()}.
222218
*/
223219
@Test
224-
public void testAllLoadedNoWorlds() {
220+
void testAllLoadedNoWorlds() {
225221
addon.onLoad();
226222
addon.getSettings().setWorlds(Collections.emptySet());
227223
addon.allLoaded();
@@ -234,23 +230,23 @@ public void testAllLoadedNoWorlds() {
234230
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#getStore()}.
235231
*/
236232
@Test
237-
public void testGetStore() {
233+
void testGetStore() {
238234
assertNull(addon.getStore());
239235
}
240236

241237
/**
242238
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#getSettings()}.
243239
*/
244240
@Test
245-
public void testGetSettings() {
241+
void testGetSettings() {
246242
assertNull(addon.getSettings());
247243
}
248244

249245
/**
250246
* Test method for {@link com.wasteofplastic.invswitcher.InvSwitcher#getWorlds()}.
251247
*/
252248
@Test
253-
public void testGetWorlds() {
249+
void testGetWorlds() {
254250
assertTrue(addon.getWorlds().isEmpty());
255251
}
256252

0 commit comments

Comments
 (0)