GH-221 NPE in CollectionGui from null parcel content items

[Jakubk15]

Problem

Closes #221. Opening the parcel collection GUI threw a NullPointerException:

Cannot invoke "org.bukkit.inventory.ItemStack.getType()" because "itemStack" is null
  at ...CollectionGui.lambda$createParcelItemAsync$11(CollectionGui.java:133)

CollectionGui.createParcelItemAsync iterates a parcel's stored content list and calls itemStack.getType() with no null guard. The list contained a literal null element.

Root cause

The content list survives a serialize → DB → deserialize round-trip through ItemStackPersister, which was configured with legacy (Spigot map) ItemStack serialization. Legacy serialization drops empty/air stacks to null. Empty/air inventory slots can slip past the GUI write filters (if (item == null) continue; does not catch isEmpty()/air), get persisted, and come back as null list elements — which then NPE on read.

Fix

Two layers:

1. Defensive guard at the model boundary (ParcelContent): the record's compact constructor now strips null and isEmpty() stacks and exposes an immutable list. Every read and write path funnels through ParcelContent, so this heals already-corrupted rows on read and prevents bad writes.
2. Upstream fix (ItemStackPersister): stop using legacy serialization. The jackson-bukkit Paper module defaults to an NBT byte-array format (ItemStack#serializeAsBytes) that round-trips empties safely. Its deserializer auto-detects the format, so previously stored legacy-format rows still read correctly and are rewritten in the new format on next save. This is the configuration the library docs explicitly recommend for paper plugins ("make sure that you are not using legacy serialization").

Tests

• New ParcelContentTest covers null-stripping and empty-stripping (written test-first, watched fail, then fixed).
• Added Mockito as a test dependency: paper-api ItemStacks cannot be constructed in plain unit tests (they need a running server's RegistryAccess), so ItemStack is mocked.

Verification notes

./gradlew compileJava test passes. The serialization round-trip itself is not covered by automated tests: paper-api ItemStacks require a running server and the existing integration tests are Docker-gated, so the format change relies on the library's documented backward-compatible auto-detection. Worth a manual smoke test (send a parcel, collect it) on a real server before release.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request addresses issue #221 by adding a guard in the ParcelContent constructor to filter out null and empty item stacks, and by updating ItemStackPersister to use default Paper serialization instead of legacy serialization. Additionally, Mockito was added to the test dependencies and new unit tests were introduced. The reviewer's feedback suggests optimizing the ParcelContent constructor to avoid stream overhead when the list is empty, and adding a unit test to verify that a null items list is handled correctly.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

It would be beneficial to add a unit test to verify that the compact constructor correctly handles a null items list by defaulting to an empty list, ensuring full coverage of the defensive guard.

        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());
    }

[Jakubk15]

@claude review

[claude]

Claude finished @Jakubk15's task in 2s —— View job

---

I'll analyze this and get back to you.