diff --git a/build.gradle.kts b/build.gradle.kts index be08d5547..5c470ab48 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -88,6 +88,8 @@ dependencies { testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:${Versions.JUNIT}") testRuntimeOnly("org.junit.platform:junit-platform-launcher") + testImplementation("org.mockito:mockito-core:${Versions.MOCKITO}") + testImplementation("org.testcontainers:junit-jupiter:${Versions.TESTCONTAINERS}") testImplementation("org.testcontainers:mysql:${Versions.TESTCONTAINERS}") testImplementation("mysql:mysql-connector-java:${Versions.MYSQL_CONNECTOR}") diff --git a/buildSrc/src/main/kotlin/Versions.kt b/buildSrc/src/main/kotlin/Versions.kt index d0dc40f10..8e318c21c 100644 --- a/buildSrc/src/main/kotlin/Versions.kt +++ b/buildSrc/src/main/kotlin/Versions.kt @@ -32,4 +32,5 @@ object Versions { const val JUNIT = "6.1.0" const val TESTCONTAINERS = "1.21.4" const val MYSQL_CONNECTOR = "8.0.33" + const val MOCKITO = "5.14.2" } diff --git a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java index 0a10c3a7e..3e04bfb0b 100644 --- a/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java +++ b/src/main/java/com/eternalcode/parcellockers/content/ParcelContent.java @@ -7,4 +7,14 @@ public record ParcelContent(UUID uniqueId, List items) { + public ParcelContent { + // Guard against null/empty stacks leaking into the content (issue #221): empty/air slots + // can slip past the GUI write filters and round-trip through the persister as nulls, which + // would later NPE when the collection GUI reads itemStack.getType(). + items = items == null || items.isEmpty() + ? List.of() + : items.stream() + .filter(item -> item != null && !item.isEmpty()) + .toList(); + } } diff --git a/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java b/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java index 63d87811c..301b12c7f 100644 --- a/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java +++ b/src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java @@ -17,11 +17,12 @@ public class ItemStackPersister extends BaseDataType { private static final ItemStackPersister instance = new ItemStackPersister(); + // Paper plugins must NOT use legacy (Spigot map) ItemStack serialization: it drops empty/air + // stacks to null, which caused the NPE in issue #221. The default Paper serializer uses an NBT + // byte array (ItemStack#serializeAsBytes) that round-trips empties safely. The deserializer + // auto-detects and still reads any data previously written in the legacy format. private static final ObjectMapper JSON = JsonMapper.builder() - .addModule(JacksonPaper.builder() - .useLegacyItemStackSerialization() - .build() - ) + .addModule(JacksonPaper.builder().build()) .build(); private ItemStackPersister() { diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java index bea26810a..60fa2f476 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGui.java @@ -137,11 +137,17 @@ public void show(Player player) { private GuiItem button(ConfigItem template, String placeholder, String value, dev.triumphteam.gui.components.GuiAction action) { ConfigItem item = template.clone(); - return item.name(item.name().replace(placeholder, value)) - .lore(item.lore().stream().map(line -> line.replace(placeholder, value)).toList()) + String replacement = nullToEmpty(value); + return item.name(item.name().replace(placeholder, replacement)) + .lore(item.lore().stream().map(line -> line.replace(placeholder, replacement)).toList()) .toGuiItem(action); } + /** Coerces a nullable placeholder value to empty so {@link String#replace} never sees a null replacement. */ + static String nullToEmpty(String value) { + return value == null ? "" : value; + } + private void apply(Player player, CompletableFuture future) { future.thenAccept(result -> { this.notifyResult(player, result); diff --git a/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java new file mode 100644 index 000000000..2983c662f --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/content/ParcelContentTest.java @@ -0,0 +1,45 @@ +package com.eternalcode.parcellockers.content; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.List; +import java.util.UUID; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; + +class ParcelContentTest { + + @Test + void dropsNullItems() { + // Reproduces issue #221: a null element in the content list makes CollectionGui + // NPE on itemStack.getType(). The model must never expose null items. + ItemStack stone = mock(ItemStack.class); + + ParcelContent content = new ParcelContent(UUID.randomUUID(), Arrays.asList(stone, null)); + + assertEquals(List.of(stone), content.items()); + } + + @Test + void dropsEmptyItems() { + // Empty/air slots can slip past the GUI write filters; they must not be exposed + // as content, otherwise they round-trip through the persister as nulls. + ItemStack stone = mock(ItemStack.class); + ItemStack air = mock(ItemStack.class); + when(air.isEmpty()).thenReturn(true); + + ParcelContent content = new ParcelContent(UUID.randomUUID(), List.of(stone, air)); + + assertEquals(List.of(stone), content.items()); + } + + @Test + void handlesNullItemsList() { + ParcelContent content = new ParcelContent(UUID.randomUUID(), null); + + assertEquals(List.of(), content.items()); + } +} diff --git a/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java b/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java new file mode 100644 index 000000000..32b466e5b --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/gui/implementation/admin/AdminParcelEditGuiTest.java @@ -0,0 +1,30 @@ +package com.eternalcode.parcellockers.gui.implementation.admin; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +class AdminParcelEditGuiTest { + + @Test + @DisplayName("Should return empty string for a null placeholder value") + void nullToEmptyWhenValueIsNull() { + assertEquals("", AdminParcelEditGui.nullToEmpty(null)); + } + + @Test + @DisplayName("Should return the original value when not null") + void nullToEmptyWhenValueIsNotNull() { + assertEquals("desc", AdminParcelEditGui.nullToEmpty("desc")); + } + + @Test + @DisplayName("Should not throw when substituting a null parcel description into a template") + void replaceWithNullDescriptionDoesNotThrow() { + String template = "Description: {DESCRIPTION}"; + assertDoesNotThrow(() -> template.replace("{DESCRIPTION}", AdminParcelEditGui.nullToEmpty(null))); + assertEquals("Description: ", template.replace("{DESCRIPTION}", AdminParcelEditGui.nullToEmpty(null))); + } +}