diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7fd7a5c..0b46cec 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -35,6 +35,13 @@ jobs: - name: Run unit tests run: mvn -B -Dpaper.version=${{ matrix.paper-version }} -Dmockbukkit.artifactId=${{ matrix.mockbukkit-artifactId }} -Dmockbukkit.version=${{ matrix.mockbukkit-version }} test + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v5 + with: + files: target/site/jacoco/jacoco.xml + flags: unit-tests + fail_ci_if_error: false + feature-tests: runs-on: ubuntu-latest needs: unit-tests @@ -65,3 +72,10 @@ jobs: - name: Run feature tests run: mvn -B -Dpaper.version=${{ matrix.paper-version }} -Dmockbukkit.artifactId=${{ matrix.mockbukkit-artifactId }} -Dmockbukkit.version=${{ matrix.mockbukkit-version }} -Pfeature-tests -Dtest=*FeatureTest test + + - name: Upload coverage to Codecov + uses: codecov/codecov-action@v5 + with: + files: target/site/jacoco/jacoco.xml + flags: feature-tests + fail_ci_if_error: false diff --git a/CHANGELOG.md b/CHANGELOG.md index b8a170d..c8ecc20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.5.5] - 2026-05-18 + +### Fixed +- **`on-sell` commands not executing for `item-type: COMMAND` items** — `sell()` no longer checks or removes physical items from the player's inventory when the item's delivery type is `COMMAND`. Previously the transaction exited early with "insufficient items" because the player had no material to hand over, preventing sell commands from running at all. +- **`on-sell execute-as` overridden by `on-buy execute-as`** — `ShopPricingManager` now tracks `execute-as` independently for the `on-buy` and `on-sell` blocks. Previously a single shared flag meant that setting `on-buy: execute-as: player` would silently override `on-sell: execute-as: console`, causing sell commands to run as the player instead of the console. + +### Added +- **Code coverage reporting** — JaCoCo is now configured in the Maven build (`jacoco-maven-plugin 0.8.12`). Coverage reports (`jacoco.xml`) are generated on every `mvn test` run and uploaded to Codecov by the CI workflow for both unit-test and feature-test jobs. + ## [2.5.4] - 2026-05-14 ### Fixed diff --git a/pom.xml b/pom.xml index 3ce12da..d04aa0b 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ com.skyblockexp ezshops - 2.5.4 + 2.5.5 EzShops Plugin Standalone plugin providing the Skyblock shop command and sign shops. jar @@ -219,9 +219,29 @@ 3.0.0-M8 false - -Dnet.bytebuddy.experimental=true + @{argLine} -Dnet.bytebuddy.experimental=true + + org.jacoco + jacoco-maven-plugin + 0.8.12 + + + jacoco-prepare-agent + + prepare-agent + + + + jacoco-report + test + + report + + + + diff --git a/src/main/java/com/skyblockexp/ezshops/shop/ShopMenuLayout.java b/src/main/java/com/skyblockexp/ezshops/shop/ShopMenuLayout.java index c506d10..444f3ca 100644 --- a/src/main/java/com/skyblockexp/ezshops/shop/ShopMenuLayout.java +++ b/src/main/java/com/skyblockexp/ezshops/shop/ShopMenuLayout.java @@ -176,6 +176,7 @@ public static final class Item { private final java.util.List buyCommands; private final java.util.List sellCommands; private final Boolean commandsRunAsConsole; + private final Boolean sellCommandsRunAsConsole; private final int requiredIslandLevel; private final ShopPriceType priceType; private final String priceId; @@ -198,6 +199,14 @@ public Item(String id, Material material, ItemDecoration display, int slot, int ShopPrice price, ItemType type, EntityType spawnerEntity, Map enchantments, int requiredIslandLevel, ShopPriceType priceType, java.util.List buyCommands, java.util.List sellCommands, Boolean commandsRunAsConsole, String priceId, DeliveryType delivery) { + this(id, material, display, slot, page, amount, bulkAmount, price, type, spawnerEntity, enchantments, requiredIslandLevel, priceType, buyCommands, sellCommands, commandsRunAsConsole, null, priceId, delivery); + } + + public Item(String id, Material material, ItemDecoration display, int slot, int page, int amount, int bulkAmount, + ShopPrice price, ItemType type, EntityType spawnerEntity, + Map enchantments, int requiredIslandLevel, ShopPriceType priceType, + java.util.List buyCommands, java.util.List sellCommands, Boolean commandsRunAsConsole, + Boolean sellCommandsRunAsConsole, String priceId, DeliveryType delivery) { this.id = Objects.requireNonNull(id, "id"); this.page = Math.max(0, page); this.material = Objects.requireNonNull(material, "material"); @@ -212,6 +221,7 @@ public Item(String id, Material material, ItemDecoration display, int slot, int this.buyCommands = buyCommands == null ? List.of() : List.copyOf(buyCommands); this.sellCommands = sellCommands == null ? List.of() : List.copyOf(sellCommands); this.commandsRunAsConsole = commandsRunAsConsole == null ? Boolean.TRUE : commandsRunAsConsole; + this.sellCommandsRunAsConsole = sellCommandsRunAsConsole; this.requiredIslandLevel = Math.max(0, requiredIslandLevel); this.priceType = priceType == null ? ShopPriceType.STATIC : priceType; this.priceId = priceId == null ? material.name() : priceId; @@ -286,6 +296,10 @@ public Boolean commandsRunAsConsole() { return commandsRunAsConsole; } + public Boolean sellCommandsRunAsConsole() { + return sellCommandsRunAsConsole != null ? sellCommandsRunAsConsole : commandsRunAsConsole; + } + public DeliveryType delivery() { return delivery; } diff --git a/src/main/java/com/skyblockexp/ezshops/shop/ShopPricingManager.java b/src/main/java/com/skyblockexp/ezshops/shop/ShopPricingManager.java index 5dc37ba..9149b69 100644 --- a/src/main/java/com/skyblockexp/ezshops/shop/ShopPricingManager.java +++ b/src/main/java/com/skyblockexp/ezshops/shop/ShopPricingManager.java @@ -742,7 +742,8 @@ private ShopMenuLayout.Item parseItem(String contextPrefix, String itemId, Confi // parse optional command hooks java.util.List buyCommands = section.getStringList("buy-commands"); java.util.List sellCommands = section.getStringList("sell-commands"); - Boolean commandsRunAsConsole = null; + Boolean buyCommandsRunAsConsole = null; + Boolean sellCommandsRunAsConsole = null; // support 'on-buy'/'on-sell' blocks with execute-as and commands if (section.isConfigurationSection("on-buy")) { org.bukkit.configuration.ConfigurationSection onBuy = section.getConfigurationSection("on-buy"); @@ -751,18 +752,20 @@ private ShopMenuLayout.Item parseItem(String contextPrefix, String itemId, Confi buyCommands = onBuy.getStringList("commands"); } String exec = onBuy.getString("execute-as", null); - if (exec != null && exec.equalsIgnoreCase("player")) { - commandsRunAsConsole = Boolean.FALSE; + if (exec != null) { + buyCommandsRunAsConsole = !exec.equalsIgnoreCase("player"); } } } if (section.isConfigurationSection("on-sell")) { org.bukkit.configuration.ConfigurationSection onSell = section.getConfigurationSection("on-sell"); - if (onSell != null && onSell.isSet("commands")) { - sellCommands = onSell.getStringList("commands"); + if (onSell != null) { + if (onSell.isSet("commands")) { + sellCommands = onSell.getStringList("commands"); + } String exec = onSell.getString("execute-as", null); - if (exec != null && exec.equalsIgnoreCase("player")) { - commandsRunAsConsole = Boolean.FALSE; + if (exec != null) { + sellCommandsRunAsConsole = !exec.equalsIgnoreCase("player"); } } } @@ -770,7 +773,7 @@ private ShopMenuLayout.Item parseItem(String contextPrefix, String itemId, Confi DeliveryType delivery = DeliveryType.fromConfig(section.getString("item-type")); return new ShopMenuLayout.Item(itemId, material, decoration, slot, page, amount, bulkAmount, price, type, spawnerEntity, enchantments, requiredIslandLevel, priceType, buyCommands, sellCommands, - commandsRunAsConsole, configuredPriceId, delivery); + buyCommandsRunAsConsole, sellCommandsRunAsConsole, configuredPriceId, delivery); } private Map> readItemData(ConfigurationSection section) { diff --git a/src/main/java/com/skyblockexp/ezshops/shop/ShopTransactionService.java b/src/main/java/com/skyblockexp/ezshops/shop/ShopTransactionService.java index c708b05..dff4c9d 100644 --- a/src/main/java/com/skyblockexp/ezshops/shop/ShopTransactionService.java +++ b/src/main/java/com/skyblockexp/ezshops/shop/ShopTransactionService.java @@ -386,16 +386,20 @@ public ShopTransactionResult sell(Player player, com.skyblockexp.ezshops.shop.Sh return ShopTransactionResult.failure(errorMessages.invalidSellPrice()); } - int sellableAmount = countMaterial(player, item.material()); - if (sellableAmount < amount) { - return ShopTransactionResult.failure(errorMessages.insufficientItems()); + if (item.delivery() != DeliveryType.COMMAND) { + int sellableAmount = countMaterial(player, item.material()); + if (sellableAmount < amount) { + return ShopTransactionResult.failure(errorMessages.insufficientItems()); + } + removeItems(player, item.material(), amount); } - removeItems(player, item.material(), amount); EconomyResponse response = economy.depositPlayer(player, totalGain); if (!response.transactionSuccess()) { - List leftovers = giveItems(player, item.material(), amount); - handleLeftoverItems(player, leftovers); + if (item.delivery() != DeliveryType.COMMAND) { + List leftovers = giveItems(player, item.material(), amount); + handleLeftoverItems(player, leftovers); + } return ShopTransactionResult.failure(errorMessages.transactionFailed(response.errorMessage)); } @@ -411,7 +415,7 @@ public ShopTransactionResult sell(Player player, com.skyblockexp.ezshops.shop.Sh tokens.put("display", item.display() != null ? item.display().displayName() : ""); tokens.put("price", item.price() != null ? formatCurrency(item.price().sellPrice()) : ""); tokens.put("total", formatCurrency(totalGain)); - hookService.executeHooks(player, item.sellCommands(), item.commandsRunAsConsole() == null ? true : item.commandsRunAsConsole(), tokens); + hookService.executeHooks(player, item.sellCommands(), item.sellCommandsRunAsConsole() == null ? true : item.sellCommandsRunAsConsole(), tokens); org.bukkit.Bukkit.getPluginManager().callEvent(new com.skyblockexp.ezshops.event.ShopSaleEvent(player, new ItemStack(item.material(), Math.max(1, amount)), amount, totalGain)); } return result; diff --git a/src/test/java/com/skyblockexp/ezshops/shop/ShopItemTypeFeatureTest.java b/src/test/java/com/skyblockexp/ezshops/shop/ShopItemTypeFeatureTest.java index 26e6fb4..4b84c64 100644 --- a/src/test/java/com/skyblockexp/ezshops/shop/ShopItemTypeFeatureTest.java +++ b/src/test/java/com/skyblockexp/ezshops/shop/ShopItemTypeFeatureTest.java @@ -104,6 +104,56 @@ void buy_item_type_command_runs_hooks_but_no_item_given() { verify(hook).executeHooks(eq(player), eq(buyCommands), eq(false), tokensCaptor.capture()); } + @Test + void sell_command_delivery_succeeds_without_physical_items_and_runs_hooks() { + loadProviderPlugin(Mockito.mock(Economy.class)); + var plugin = loadPlugin(com.skyblockexp.ezshops.EzShopsPlugin.class); + + ShopPricingManager pricingManager = Mockito.mock(ShopPricingManager.class); + Economy econ = Mockito.mock(Economy.class); + + ShopPrice price = new ShopPrice(10.0, 5.0); + when(pricingManager.getPrice(eq("DIAMOND"))).thenReturn(Optional.of(price)); + when(pricingManager.estimateBulkTotal(eq("DIAMOND"), eq(1), any())).thenReturn(5.0); + + when(econ.depositPlayer((org.bukkit.OfflinePlayer) any(), anyDouble())) + .thenReturn(new EconomyResponse(0.0, 5.0, EconomyResponse.ResponseType.SUCCESS, "ok")); + + ShopTransactionService svc = new ShopTransactionService(pricingManager, econ, + com.skyblockexp.ezshops.config.ShopMessageConfiguration.load(plugin).transactions()); + + TransactionHookService hook = Mockito.mock(TransactionHookService.class); + svc.setTransactionHookService(hook); + + // Player has NO DIAMOND in their inventory — a COMMAND sell should not require it + Player player = server.addPlayer("seller_cmd"); + player.addAttachment(plugin, ShopTransactionService.PERMISSION_SELL, true); + + ShopMenuLayout.ItemDecoration decoration = + new ShopMenuLayout.ItemDecoration(Material.DIAMOND, 1, "", List.of()); + List sellCommands = List.of("give {player} diamond 1"); + ShopMenuLayout.Item item = new ShopMenuLayout.Item("diamond_command_sell", Material.DIAMOND, decoration, + 0, 0, 1, 1, price, ShopMenuLayout.ItemType.MATERIAL, null, Map.of(), 0, + ShopPriceType.STATIC, List.of(), sellCommands, Boolean.TRUE, null, DeliveryType.COMMAND); + + ShopTransactionResult result = svc.sell(player, item, 1); + + assertTrue(result.success(), "COMMAND-delivery sell should succeed even without physical items: " + result.message()); + + // Economy should have deposited the sell price + verify(econ).depositPlayer((org.bukkit.OfflinePlayer) any(), eq(5.0)); + + // Inventory must be untouched + int remaining = player.getInventory().all(Material.DIAMOND).values().stream() + .mapToInt(ItemStack::getAmount).sum(); + assertEquals(0, remaining, "COMMAND-delivery sell must not remove items from the player's inventory"); + + // Sell hooks must still run + ArgumentCaptor tokensCaptor = ArgumentCaptor.forClass(Map.class); + verify(hook).executeHooks(eq(player), eq(sellCommands), eq(Boolean.TRUE), tokensCaptor.capture()); + assertEquals("1", tokensCaptor.getValue().get("amount")); + } + @Test void buy_item_type_none_charges_but_no_item_and_no_hooks() { loadProviderPlugin(Mockito.mock(Economy.class)); diff --git a/src/test/java/com/skyblockexp/ezshops/shop/ShopTransactionHookFeatureTest.java b/src/test/java/com/skyblockexp/ezshops/shop/ShopTransactionHookFeatureTest.java index 6a8b88c..e60ec59 100644 --- a/src/test/java/com/skyblockexp/ezshops/shop/ShopTransactionHookFeatureTest.java +++ b/src/test/java/com/skyblockexp/ezshops/shop/ShopTransactionHookFeatureTest.java @@ -20,6 +20,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyDouble; import static org.mockito.ArgumentMatchers.eq; @@ -112,6 +113,106 @@ void sell_item_triggers_hookService_with_expected_tokens() { assertEquals("DIAMOND", tokens.get("material")); } + @Test + void sell_command_delivery_item_runs_hooks_and_deposits_economy_without_physical_item() { + loadProviderPlugin(Mockito.mock(Economy.class)); + var plugin = loadPlugin(com.skyblockexp.ezshops.EzShopsPlugin.class); + + ShopPricingManager pricingManager = Mockito.mock(ShopPricingManager.class); + Economy econ = Mockito.mock(Economy.class); + + ShopPrice price = new ShopPrice(10.0, 3.8); + when(pricingManager.getPrice(eq("AMETHYST_SHARD"))).thenReturn(Optional.of(price)); + when(pricingManager.estimateBulkTotal(eq("AMETHYST_SHARD"), eq(16), any())).thenReturn(60.8); + when(econ.depositPlayer((org.bukkit.OfflinePlayer) any(), anyDouble())) + .thenReturn(new EconomyResponse(0.0, 60.8, EconomyResponse.ResponseType.SUCCESS, "ok")); + + ShopTransactionService svc = new ShopTransactionService(pricingManager, econ, + com.skyblockexp.ezshops.config.ShopMessageConfiguration.load(plugin).transactions()); + + TransactionHookService hook = Mockito.mock(TransactionHookService.class); + svc.setTransactionHookService(hook); + + // Player has no AMETHYST_SHARD — COMMAND delivery should not require physical items + Player player = server.addPlayer("seller_shard"); + player.addAttachment(plugin, ShopTransactionService.PERMISSION_SELL, true); + + ShopMenuLayout.ItemDecoration decoration = + new ShopMenuLayout.ItemDecoration(org.bukkit.Material.AMETHYST_SHARD, 1, "", List.of()); + List sellCommands = List.of("say hi"); + ShopMenuLayout.Item item = new ShopMenuLayout.Item("AMETHYST_SHARD", org.bukkit.Material.AMETHYST_SHARD, + decoration, 10, 0, 16, 64, price, ShopMenuLayout.ItemType.MATERIAL, null, Map.of(), 0, + com.skyblockexp.ezshops.shop.ShopPriceType.STATIC, + List.of(), sellCommands, Boolean.TRUE, null, + com.skyblockexp.ezshops.shop.DeliveryType.COMMAND); + + ShopTransactionResult result = svc.sell(player, item, 16); + + assertTrue(result.success(), + "Sell with COMMAND delivery should succeed even when player has no physical items: " + result.message()); + + // Economy deposit must occur + verify(econ).depositPlayer((org.bukkit.OfflinePlayer) any(), eq(60.8)); + + // Sell hook must fire + verify(hook).executeHooks(eq(player), eq(sellCommands), eq(Boolean.TRUE), any()); + } + + @Test + void sell_execute_as_is_independent_from_buy_execute_as() { + // Tests that on-sell execute-as:console does not inherit on-buy execute-as:player. + // sellCommandsRunAsConsole() must reflect the sell-specific setting. + loadProviderPlugin(Mockito.mock(Economy.class)); + var plugin = loadPlugin(com.skyblockexp.ezshops.EzShopsPlugin.class); + + ShopPricingManager pricingManager = Mockito.mock(ShopPricingManager.class); + Economy econ = Mockito.mock(Economy.class); + + ShopPrice price = new ShopPrice(10.0, 5.0); + when(pricingManager.getPrice(eq("DIAMOND"))).thenReturn(Optional.of(price)); + when(pricingManager.estimateBulkTotal(eq("DIAMOND"), eq(1), any())).thenReturn(5.0); + when(econ.depositPlayer((org.bukkit.OfflinePlayer) any(), anyDouble())) + .thenReturn(new EconomyResponse(0.0, 5.0, EconomyResponse.ResponseType.SUCCESS, "ok")); + + ShopTransactionService svc = new ShopTransactionService(pricingManager, econ, + com.skyblockexp.ezshops.config.ShopMessageConfiguration.load(plugin).transactions()); + + TransactionHookService hook = Mockito.mock(TransactionHookService.class); + svc.setTransactionHookService(hook); + + Player player = server.addPlayer("seller_exec"); + player.getInventory().addItem(new org.bukkit.inventory.ItemStack(org.bukkit.Material.DIAMOND, 1)); + player.addAttachment(plugin, ShopTransactionService.PERMISSION_SELL, true); + svc.setIgnoreItemsWithNBT(false); + + ShopMenuLayout.ItemDecoration decoration = + new ShopMenuLayout.ItemDecoration(org.bukkit.Material.DIAMOND, 1, "", List.of()); + List buyCommands = List.of("buycmd"); + List sellCommands = List.of("sellcmd"); + + // Simulate: on-buy execute-as: player (commandsRunAsConsole = FALSE) + // on-sell execute-as: console (sellCommandsRunAsConsole = TRUE) + ShopMenuLayout.Item item = new ShopMenuLayout.Item("diamond_exec", org.bukkit.Material.DIAMOND, + decoration, 0, 0, 1, 1, price, ShopMenuLayout.ItemType.MATERIAL, null, Map.of(), 0, + com.skyblockexp.ezshops.shop.ShopPriceType.STATIC, + buyCommands, sellCommands, + Boolean.FALSE, // on-buy: execute-as: player + Boolean.TRUE, // on-sell: execute-as: console (independent) + null, + com.skyblockexp.ezshops.shop.DeliveryType.ITEM); + + // Verify the accessor honours sell-specific setting + assertEquals(Boolean.FALSE, item.commandsRunAsConsole(), + "buy commandsRunAsConsole should be FALSE (player)"); + assertEquals(Boolean.TRUE, item.sellCommandsRunAsConsole(), + "sell commandsRunAsConsole should be TRUE (console), independent from buy"); + + svc.sell(player, item, 1); + + // Sell hook must run as console (TRUE), NOT as player (FALSE) + verify(hook).executeHooks(eq(player), eq(sellCommands), eq(Boolean.TRUE), any()); + } + @Test void sell_item_with_nbt_is_ignored_when_configured() { loadProviderPlugin(Mockito.mock(Economy.class));