From 0f0fb6f263564275f32c76fc40afb48102cc39c9 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:01:40 +0200 Subject: [PATCH 01/17] feat: add COLLECTED parcel status and guard ParcelSendTask against re-delivery Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../eternalcode/parcellockers/parcel/ParcelStatus.java | 3 ++- .../parcellockers/parcel/task/ParcelSendTask.java | 2 +- .../parcellockers/parcel/task/ParcelSendTaskTest.java | 10 ++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/ParcelStatus.java b/src/main/java/com/eternalcode/parcellockers/parcel/ParcelStatus.java index 8744073be..6f8fb1e3f 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/ParcelStatus.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/ParcelStatus.java @@ -3,5 +3,6 @@ public enum ParcelStatus { SENT, - DELIVERED + DELIVERED, + COLLECTED } diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java b/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java index 49c4eaee0..8c93901ce 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java @@ -35,7 +35,7 @@ public ParcelSendTask(Parcel parcel, ParcelService parcelService, DeliveryManage /** Pure decision: what to do given the latest parcel + delivery state at fire time. */ public static Decision decide(Optional currentParcel, Optional currentDelivery, Instant now) { - if (currentParcel.isEmpty() || currentParcel.get().status() == ParcelStatus.DELIVERED) { + if (currentParcel.isEmpty() || currentParcel.get().status() != ParcelStatus.SENT) { return Decision.ABORT; } if (currentDelivery.isPresent() && currentDelivery.get().deliveryTimestamp().isAfter(now)) { diff --git a/src/test/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTaskTest.java b/src/test/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTaskTest.java index 80dace4d4..29014a276 100644 --- a/src/test/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTaskTest.java +++ b/src/test/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTaskTest.java @@ -54,4 +54,14 @@ void deliversWhenSentWithNoDeliveryRow() { Parcel sent = parcel(ParcelStatus.SENT); assertEquals(Decision.DELIVER, ParcelSendTask.decide(Optional.of(sent), Optional.empty(), now)); } + + @Test + void abortsWhenAlreadyCollected() { + // A COLLECTED parcel sits in the receiver's return window; a stale delivery row + // must not re-deliver it, or the items could be collected twice. + Parcel collected = parcel(ParcelStatus.COLLECTED); + Delivery due = new Delivery(collected.uuid(), Instant.parse("2026-06-21T11:00:00Z")); + assertEquals(Decision.ABORT, ParcelSendTask.decide( + Optional.of(collected), Optional.of(due), Instant.parse("2026-06-21T12:00:00Z"))); + } } From 2638dec534215e3f6c35a55920c970089ae86d34 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:07:13 +0200 Subject: [PATCH 02/17] feat: add CollectedParcel domain and collected_parcels repository Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../returns/CollectedParcel.java | 7 ++ .../repository/CollectedParcelRepository.java | 20 ++++ .../CollectedParcelRepositoryOrmLite.java | 52 +++++++++ .../repository/CollectedParcelTable.java | 36 +++++++ ...lectedParcelRepositoryIntegrationTest.java | 101 ++++++++++++++++++ 5 files changed, 216 insertions(+) create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/CollectedParcel.java create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepository.java create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelTable.java create mode 100644 src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/returns/CollectedParcel.java b/src/main/java/com/eternalcode/parcellockers/returns/CollectedParcel.java new file mode 100644 index 000000000..23fe45674 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/CollectedParcel.java @@ -0,0 +1,7 @@ +package com.eternalcode.parcellockers.returns; + +import java.time.Instant; +import java.util.UUID; + +public record CollectedParcel(UUID parcel, Instant collectedAt) { +} diff --git a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepository.java b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepository.java new file mode 100644 index 000000000..fd2064f40 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepository.java @@ -0,0 +1,20 @@ +package com.eternalcode.parcellockers.returns.repository; + +import com.eternalcode.parcellockers.returns.CollectedParcel; +import java.time.Instant; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; + +public interface CollectedParcelRepository { + + CompletableFuture save(CollectedParcel collectedParcel); + + CompletableFuture> find(UUID parcel); + + /** Returns rows collected at or before the given cutoff (i.e. whose return window expired). */ + CompletableFuture> findExpired(Instant cutoff); + + CompletableFuture delete(UUID parcel); +} diff --git a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java new file mode 100644 index 000000000..6de2a0a33 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java @@ -0,0 +1,52 @@ +package com.eternalcode.parcellockers.returns.repository; + +import com.eternalcode.commons.scheduler.Scheduler; +import com.eternalcode.parcellockers.database.DatabaseManager; +import com.eternalcode.parcellockers.database.wrapper.AbstractRepositoryOrmLite; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import java.time.Instant; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; + +public class CollectedParcelRepositoryOrmLite extends AbstractRepositoryOrmLite implements CollectedParcelRepository { + + public CollectedParcelRepositoryOrmLite(DatabaseManager databaseManager, Scheduler scheduler) { + super(databaseManager, scheduler); + this.createTable(CollectedParcelTable.class); + } + + @Override + public CompletableFuture save(CollectedParcel collectedParcel) { + Objects.requireNonNull(collectedParcel, "CollectedParcel cannot be null"); + return this.insertIfAbsent(CollectedParcelTable.class, CollectedParcelTable.from(collectedParcel)) + .thenApply(dao -> null); + } + + @Override + public CompletableFuture> find(UUID parcel) { + Objects.requireNonNull(parcel, "Parcel UUID cannot be null"); + return this.selectSafe(CollectedParcelTable.class, parcel) + .thenApply(optional -> optional.map(CollectedParcelTable::toCollectedParcel)); + } + + @Override + public CompletableFuture> findExpired(Instant cutoff) { + Objects.requireNonNull(cutoff, "Cutoff cannot be null"); + return this.action(CollectedParcelTable.class, dao -> dao.queryBuilder() + .where() + .le(CollectedParcelTable.COLLECTED_AT_COLUMN, cutoff) + .query() + .stream() + .map(CollectedParcelTable::toCollectedParcel) + .toList()); + } + + @Override + public CompletableFuture delete(UUID parcel) { + Objects.requireNonNull(parcel, "Parcel UUID cannot be null"); + return this.deleteById(CollectedParcelTable.class, parcel).thenApply(rows -> rows > 0); + } +} diff --git a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelTable.java b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelTable.java new file mode 100644 index 000000000..88c167fe2 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelTable.java @@ -0,0 +1,36 @@ +package com.eternalcode.parcellockers.returns.repository; + +import com.eternalcode.parcellockers.database.persister.InstantPersister; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import com.j256.ormlite.field.DatabaseField; +import com.j256.ormlite.table.DatabaseTable; +import java.time.Instant; +import java.util.UUID; + +@DatabaseTable(tableName = "collected_parcels") +class CollectedParcelTable { + + static final String COLLECTED_AT_COLUMN = "collected_at"; + + @DatabaseField(id = true) + private UUID parcel; + + @DatabaseField(columnName = COLLECTED_AT_COLUMN, persisterClass = InstantPersister.class) + private Instant collectedAt; + + CollectedParcelTable() { + } + + CollectedParcelTable(UUID parcel, Instant collectedAt) { + this.parcel = parcel; + this.collectedAt = collectedAt; + } + + static CollectedParcelTable from(CollectedParcel collectedParcel) { + return new CollectedParcelTable(collectedParcel.parcel(), collectedParcel.collectedAt()); + } + + CollectedParcel toCollectedParcel() { + return new CollectedParcel(this.parcel, this.collectedAt); + } +} diff --git a/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java b/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java new file mode 100644 index 000000000..fd11a4cc5 --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java @@ -0,0 +1,101 @@ +package com.eternalcode.parcellockers.database; + +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 com.eternalcode.parcellockers.TestScheduler; +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepository; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepositoryOrmLite; +import java.nio.file.Path; +import java.sql.SQLException; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.Optional; +import java.util.UUID; +import java.util.logging.Logger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.testcontainers.containers.MySQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; + +@Testcontainers(disabledWithoutDocker = true) +class CollectedParcelRepositoryIntegrationTest extends IntegrationTestSpec { + + @Container + private static final MySQLContainer mySQLContainer = new MySQLContainer<>(DockerImageName.parse("mysql:latest")); + + @TempDir + private Path tempDir; + + private DatabaseManager databaseManager; + + private CollectedParcelRepository repository() throws SQLException { + PluginConfig config = new PluginConfig(); + config.settings.databaseType = DatabaseType.MYSQL; + config.settings.host = mySQLContainer.getHost(); + config.settings.port = String.valueOf(mySQLContainer.getFirstMappedPort()); + config.settings.databaseName = mySQLContainer.getDatabaseName(); + config.settings.user = mySQLContainer.getUsername(); + config.settings.password = mySQLContainer.getPassword(); + + DatabaseManager databaseManager = new DatabaseManager(config, Logger.getLogger("ParcelLockers"), this.tempDir.toFile()); + databaseManager.connect(); + this.databaseManager = databaseManager; + + return new CollectedParcelRepositoryOrmLite(databaseManager, new TestScheduler()); + } + + @Test + void savesAndFindsCollectedParcel() throws SQLException { + CollectedParcelRepository repository = this.repository(); + UUID parcel = UUID.randomUUID(); + // MySQL TIMESTAMP columns don't keep nanos; truncate so the round-trip compares equal. + Instant collectedAt = Instant.now().truncatedTo(ChronoUnit.SECONDS); + + this.await(repository.save(new CollectedParcel(parcel, collectedAt))); + + Optional found = this.await(repository.find(parcel)); + assertTrue(found.isPresent()); + assertEquals(collectedAt, found.get().collectedAt()); + } + + @Test + void findExpiredReturnsOnlyRowsAtOrBeforeCutoff() throws SQLException { + CollectedParcelRepository repository = this.repository(); + Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS); + + UUID expired = UUID.randomUUID(); + UUID fresh = UUID.randomUUID(); + this.await(repository.save(new CollectedParcel(expired, now.minusSeconds(3600)))); + this.await(repository.save(new CollectedParcel(fresh, now))); + + List result = this.await(repository.findExpired(now.minusSeconds(60))); + assertEquals(1, result.size()); + assertEquals(expired, result.get(0).parcel()); + } + + @Test + void deleteRemovesRow() throws SQLException { + CollectedParcelRepository repository = this.repository(); + UUID parcel = UUID.randomUUID(); + this.await(repository.save(new CollectedParcel(parcel, Instant.now().truncatedTo(ChronoUnit.SECONDS)))); + + assertTrue(this.await(repository.delete(parcel))); + assertFalse(this.await(repository.find(parcel)).isPresent()); + assertFalse(this.await(repository.delete(parcel))); + } + + @AfterEach + void tearDown() { + if (this.databaseManager != null) { + this.databaseManager.disconnect(); + } + } +} From f8340692d2bc9894832f7755e3f3068d03140e71 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:14:44 +0200 Subject: [PATCH 03/17] fix: compare return-window expiry temporally, not lexicographically The collected_at column is a string-persisted ISO-8601 Instant; SQL range operators compare it lexicographically and misorder same-second values. Filter in Java and cover the cutoff boundary in the integration test. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../CollectedParcelRepositoryOrmLite.java | 16 +++++++++------- ...CollectedParcelRepositoryIntegrationTest.java | 15 ++++++++++++--- 2 files changed, 21 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java index 6de2a0a33..e27986fad 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java @@ -35,13 +35,15 @@ public CompletableFuture> find(UUID parcel) { @Override public CompletableFuture> findExpired(Instant cutoff) { Objects.requireNonNull(cutoff, "Cutoff cannot be null"); - return this.action(CollectedParcelTable.class, dao -> dao.queryBuilder() - .where() - .le(CollectedParcelTable.COLLECTED_AT_COLUMN, cutoff) - .query() - .stream() - .map(CollectedParcelTable::toCollectedParcel) - .toList()); + // collected_at is persisted as an ISO-8601 string (InstantPersister); a SQL range operator + // would compare it lexicographically, which misorders same-second values with different + // fractional renderings. Compare temporally in Java instead — the table only holds parcels + // inside the return window, so a full scan per purge run is cheap. + return this.selectAll(CollectedParcelTable.class) + .thenApply(rows -> rows.stream() + .map(CollectedParcelTable::toCollectedParcel) + .filter(collected -> !collected.collectedAt().isAfter(cutoff)) + .toList()); } @Override diff --git a/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java b/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java index fd11a4cc5..ef28c000b 100644 --- a/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java +++ b/src/test/java/com/eternalcode/parcellockers/database/CollectedParcelRepositoryIntegrationTest.java @@ -70,15 +70,24 @@ void savesAndFindsCollectedParcel() throws SQLException { void findExpiredReturnsOnlyRowsAtOrBeforeCutoff() throws SQLException { CollectedParcelRepository repository = this.repository(); Instant now = Instant.now().truncatedTo(ChronoUnit.SECONDS); + Instant cutoff = now.minusSeconds(60); UUID expired = UUID.randomUUID(); + UUID atCutoff = UUID.randomUUID(); + UUID justAfterCutoff = UUID.randomUUID(); UUID fresh = UUID.randomUUID(); this.await(repository.save(new CollectedParcel(expired, now.minusSeconds(3600)))); + this.await(repository.save(new CollectedParcel(atCutoff, cutoff))); + // Sub-second offset: guards the temporal (not lexicographic) comparison of the + // string-persisted column. + this.await(repository.save(new CollectedParcel(justAfterCutoff, cutoff.plusMillis(500)))); this.await(repository.save(new CollectedParcel(fresh, now))); - List result = this.await(repository.findExpired(now.minusSeconds(60))); - assertEquals(1, result.size()); - assertEquals(expired, result.get(0).parcel()); + List result = this.await(repository.findExpired(cutoff)); + List resultParcels = result.stream().map(CollectedParcel::parcel).toList(); + assertEquals(2, result.size()); + assertTrue(resultParcels.contains(expired)); + assertTrue(resultParcels.contains(atCutoff)); } @Test From ef1af91bb962b5c52bc24495e95337d2069017b6 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 14:22:24 +0200 Subject: [PATCH 04/17] feat: add conditional collect/return status flips and returnable query to ParcelRepository Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../parcel/repository/ParcelRepository.java | 16 +++ .../repository/ParcelRepositoryOrmLite.java | 50 +++++++ ...ParcelReturnRepositoryIntegrationTest.java | 134 ++++++++++++++++++ 3 files changed, 200 insertions(+) create mode 100644 src/test/java/com/eternalcode/parcellockers/database/ParcelReturnRepositoryIntegrationTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java index 43f5b49f0..1719c7688 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java @@ -43,6 +43,22 @@ public interface ParcelRepository { */ CompletableFuture> findCollectible(UUID receiver, UUID destinationLocker, Page page); + /** + * Atomically flips a DELIVERED parcel to COLLECTED. Returns false when the parcel is missing + * or not DELIVERED — the caller must treat that as "someone else already collected it". + */ + CompletableFuture markCollected(UUID uuid); + + /** + * Atomically turns a COLLECTED parcel into its reverse SENT shipment (parties and lockers + * swapped as prepared by the caller). Returns false when the parcel is missing or not + * COLLECTED — the caller must treat that as "already returned or purged". + */ + CompletableFuture markReturned(Parcel returned); + + /** Returns the COLLECTED parcels of the given receiver (candidates for a return). */ + CompletableFuture> findReturnable(UUID receiver, Page page); + /** * Counts the parcels currently occupying a destination locker. Collected parcels are removed * from storage, so every parcel addressed to the locker (in-transit or delivered) occupies a diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java index 43c560084..adec6b8c4 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java @@ -7,6 +7,7 @@ import com.eternalcode.parcellockers.parcel.ParcelStatus; import com.eternalcode.parcellockers.shared.Page; import com.eternalcode.parcellockers.shared.PageResult; +import com.j256.ormlite.stmt.UpdateBuilder; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -15,9 +16,11 @@ public class ParcelRepositoryOrmLite extends AbstractRepositoryOrmLite implements ParcelRepository { + private static final String UUID_COLUMN = "uuid"; private static final String RECEIVER_COLUMN = "receiver"; private static final String SENDER_COLUMN = "sender"; private static final String DESTINATION_LOCKER_COLUMN = "destination_locker"; + private static final String ENTRY_LOCKER_COLUMN = "entry_locker"; private static final String STATUS_COLUMN = "status"; public ParcelRepositoryOrmLite(DatabaseManager databaseManager, Scheduler scheduler) { @@ -96,6 +99,51 @@ public CompletableFuture> findCollectible(UUID receiver, UUID }, ParcelTable::toParcel); } + @Override + public CompletableFuture markCollected(UUID uuid) { + Objects.requireNonNull(uuid, "UUID cannot be null"); + return this.action(ParcelTable.class, dao -> { + UpdateBuilder builder = dao.updateBuilder(); + builder.updateColumnValue(STATUS_COLUMN, ParcelStatus.COLLECTED); + builder.where() + .eq(UUID_COLUMN, uuid) + .and() + .eq(STATUS_COLUMN, ParcelStatus.DELIVERED); + return builder.update() > 0; + }); + } + + @Override + public CompletableFuture markReturned(Parcel returned) { + Objects.requireNonNull(returned, "Returned parcel cannot be null"); + return this.action(ParcelTable.class, dao -> { + UpdateBuilder builder = dao.updateBuilder(); + builder.updateColumnValue(SENDER_COLUMN, returned.sender()); + builder.updateColumnValue(RECEIVER_COLUMN, returned.receiver()); + builder.updateColumnValue(ENTRY_LOCKER_COLUMN, returned.entryLocker()); + builder.updateColumnValue(DESTINATION_LOCKER_COLUMN, returned.destinationLocker()); + builder.updateColumnValue(STATUS_COLUMN, ParcelStatus.SENT); + builder.where() + .eq(UUID_COLUMN, returned.uuid()) + .and() + .eq(STATUS_COLUMN, ParcelStatus.COLLECTED); + return builder.update() > 0; + }); + } + + @Override + public CompletableFuture> findReturnable(UUID receiver, Page page) { + Objects.requireNonNull(receiver, "Receiver UUID cannot be null"); + Objects.requireNonNull(page, "Page cannot be null"); + return this.queryPage(ParcelTable.class, page, builder -> { + builder.where() + .eq(RECEIVER_COLUMN, receiver) + .and() + .eq(STATUS_COLUMN, ParcelStatus.COLLECTED); + return builder; + }, ParcelTable::toParcel); + } + @Override public CompletableFuture countParcelsByDestinationLocker(UUID destinationLocker) { Objects.requireNonNull(destinationLocker, "Destination locker UUID cannot be null"); @@ -103,6 +151,8 @@ public CompletableFuture countParcelsByDestinationLocker(UUID destinati long count = dao.queryBuilder() .where() .eq(DESTINATION_LOCKER_COLUMN, destinationLocker) + .and() + .ne(STATUS_COLUMN, ParcelStatus.COLLECTED) .countOf(); return (int) count; }); diff --git a/src/test/java/com/eternalcode/parcellockers/database/ParcelReturnRepositoryIntegrationTest.java b/src/test/java/com/eternalcode/parcellockers/database/ParcelReturnRepositoryIntegrationTest.java new file mode 100644 index 000000000..debf5b1a6 --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/database/ParcelReturnRepositoryIntegrationTest.java @@ -0,0 +1,134 @@ +package com.eternalcode.parcellockers.database; + +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 com.eternalcode.parcellockers.TestScheduler; +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.parcel.ParcelSize; +import com.eternalcode.parcellockers.parcel.ParcelStatus; +import com.eternalcode.parcellockers.parcel.repository.ParcelRepository; +import com.eternalcode.parcellockers.parcel.repository.ParcelRepositoryOrmLite; +import com.eternalcode.parcellockers.shared.Page; +import com.eternalcode.parcellockers.shared.PageResult; +import java.nio.file.Path; +import java.sql.SQLException; +import java.util.UUID; +import java.util.logging.Logger; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.testcontainers.containers.MySQLContainer; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.utility.DockerImageName; + +@Testcontainers(disabledWithoutDocker = true) +class ParcelReturnRepositoryIntegrationTest extends IntegrationTestSpec { + + @Container + private static final MySQLContainer mySQLContainer = new MySQLContainer<>(DockerImageName.parse("mysql:latest")); + + @TempDir + private Path tempDir; + + private DatabaseManager databaseManager; + + private ParcelRepository repository() throws SQLException { + PluginConfig config = new PluginConfig(); + config.settings.databaseType = DatabaseType.MYSQL; + config.settings.host = mySQLContainer.getHost(); + config.settings.port = String.valueOf(mySQLContainer.getFirstMappedPort()); + config.settings.databaseName = mySQLContainer.getDatabaseName(); + config.settings.user = mySQLContainer.getUsername(); + config.settings.password = mySQLContainer.getPassword(); + + DatabaseManager databaseManager = new DatabaseManager(config, Logger.getLogger("ParcelLockers"), this.tempDir.toFile()); + databaseManager.connect(); + this.databaseManager = databaseManager; + + return new ParcelRepositoryOrmLite(databaseManager, new TestScheduler()); + } + + private static Parcel parcel(UUID receiver, UUID destinationLocker, ParcelStatus status) { + return new Parcel(UUID.randomUUID(), UUID.randomUUID(), "p", "d", false, + receiver, ParcelSize.SMALL, UUID.randomUUID(), destinationLocker, status); + } + + @Test + void markCollectedFlipsOnlyDeliveredParcels() throws SQLException { + ParcelRepository repository = this.repository(); + Parcel delivered = parcel(UUID.randomUUID(), UUID.randomUUID(), ParcelStatus.DELIVERED); + this.await(repository.save(delivered)); + + assertTrue(this.await(repository.markCollected(delivered.uuid()))); + assertEquals(ParcelStatus.COLLECTED, this.await(repository.findById(delivered.uuid())).orElseThrow().status()); + + // Second collect attempt must not report success — this is the double-collect guard. + assertFalse(this.await(repository.markCollected(delivered.uuid()))); + + Parcel sent = parcel(UUID.randomUUID(), UUID.randomUUID(), ParcelStatus.SENT); + this.await(repository.save(sent)); + assertFalse(this.await(repository.markCollected(sent.uuid()))); + } + + @Test + void markReturnedSwapsPartiesAndLockersOnlyWhenCollected() throws SQLException { + ParcelRepository repository = this.repository(); + Parcel collected = parcel(UUID.randomUUID(), UUID.randomUUID(), ParcelStatus.COLLECTED); + this.await(repository.save(collected)); + + Parcel returned = new Parcel(collected.uuid(), collected.receiver(), collected.name(), + collected.description(), collected.priority(), collected.sender(), collected.size(), + collected.destinationLocker(), collected.entryLocker(), ParcelStatus.SENT); + + assertTrue(this.await(repository.markReturned(returned))); + + Parcel stored = this.await(repository.findById(collected.uuid())).orElseThrow(); + assertEquals(collected.receiver(), stored.sender()); + assertEquals(collected.sender(), stored.receiver()); + assertEquals(collected.destinationLocker(), stored.entryLocker()); + assertEquals(collected.entryLocker(), stored.destinationLocker()); + assertEquals(ParcelStatus.SENT, stored.status()); + + // A second return of the same parcel must fail (status is no longer COLLECTED). + assertFalse(this.await(repository.markReturned(returned))); + } + + @Test + void findReturnableReturnsOnlyCollectedParcelsOfReceiver() throws SQLException { + ParcelRepository repository = this.repository(); + UUID receiver = UUID.randomUUID(); + + this.await(repository.save(parcel(receiver, UUID.randomUUID(), ParcelStatus.COLLECTED))); + this.await(repository.save(parcel(receiver, UUID.randomUUID(), ParcelStatus.COLLECTED))); + this.await(repository.save(parcel(receiver, UUID.randomUUID(), ParcelStatus.DELIVERED))); + this.await(repository.save(parcel(UUID.randomUUID(), UUID.randomUUID(), ParcelStatus.COLLECTED))); + + PageResult page = this.await(repository.findReturnable(receiver, new Page(0, 10))); + assertEquals(2, page.items().size()); + assertTrue(page.items().stream().allMatch(item -> item.status() == ParcelStatus.COLLECTED)); + assertTrue(page.items().stream().allMatch(item -> item.receiver().equals(receiver))); + } + + @Test + void collectedParcelsDoNotCountTowardsLockerFullness() throws SQLException { + ParcelRepository repository = this.repository(); + UUID locker = UUID.randomUUID(); + + this.await(repository.save(parcel(UUID.randomUUID(), locker, ParcelStatus.SENT))); + this.await(repository.save(parcel(UUID.randomUUID(), locker, ParcelStatus.DELIVERED))); + this.await(repository.save(parcel(UUID.randomUUID(), locker, ParcelStatus.COLLECTED))); + + assertEquals(2, this.await(repository.countParcelsByDestinationLocker(locker))); + } + + @AfterEach + void tearDown() { + if (this.databaseManager != null) { + this.databaseManager.disconnect(); + } + } +} From 6ddf92e4fba42494f2f003f188b542791e5860eb Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:24:35 +0200 Subject: [PATCH 05/17] feat: add return window, fees, attribute-check flags and return notices to config Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../implementation/MessageConfig.java | 20 +++++ .../implementation/PluginConfig.java | 79 +++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java index a7f007137..4bcd1ea84 100644 --- a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java +++ b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java @@ -125,6 +125,26 @@ public static class ParcelMessages extends OkaeriConfig { .chat("&2✔ &a${AMOUNT} has been withdrawn from your account to cover the parcel sending fee.") .sound(SoundEventKeys.ENTITY_EXPERIENCE_ORB_PICKUP) .build(); + public Notice returned = Notice.builder() + .chat("&2✔ &aParcel returned. It is on its way back to the sender.") + .sound(SoundEventKeys.ENTITY_PLAYER_LEVELUP) + .build(); + public Notice cannotReturn = Notice.builder() + .chat("&4✘ &cThis parcel cannot be returned right now. Your items were given back.") + .sound(SoundEventKeys.ENTITY_VILLAGER_NO) + .build(); + public Notice returnItemsMismatch = Notice.builder() + .chat("&4✘ &cThe deposited items do not match the original parcel contents!") + .sound(SoundEventKeys.ENTITY_VILLAGER_NO) + .build(); + public Notice returnWindowExpired = Notice.builder() + .chat("&4✘ &cThe return window for this parcel has expired!") + .sound(SoundEventKeys.ENTITY_VILLAGER_NO) + .build(); + public Notice returnFeeWithdrawn = Notice.builder() + .chat("&2✔ &a${AMOUNT} has been withdrawn from your account to cover the parcel return fee.") + .sound(SoundEventKeys.ENTITY_EXPERIENCE_ORB_PICKUP) + .build(); @Comment({"", "# The parcel info message." }) public Notice parcelInfoMessages = Notice.builder() diff --git a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfig.java b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfig.java index 1ae9d486c..083e217a5 100644 --- a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfig.java +++ b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfig.java @@ -94,6 +94,25 @@ public static class Settings extends OkaeriConfig { @Comment({"", "# Large parcel fee in in-game currency"}) public double largeParcelFee = 50.0; + + @Comment({"", "# How long after collection a parcel can still be returned.", "# Expired collected parcels are purged periodically."}) + public Duration parcelReturnWindow = Duration.ofDays(7); + + @Comment({"", "# Small parcel return fee in in-game currency"}) + public double smallParcelReturnFee = 5.0; + + @Comment({"", "# Medium parcel return fee in in-game currency"}) + public double mediumParcelReturnFee = 12.5; + + @Comment({"", "# Large parcel return fee in in-game currency"}) + public double largeParcelReturnFee = 25.0; + + @Comment({ + "", + "# Which item attributes must match the original parcel content when a player returns a parcel.", + "# Material types and total amounts must always match; each flag below relaxes one attribute when set to false." + }) + public ReturnChecks returnChecks = new ReturnChecks(); } public static class GuiSettings extends OkaeriConfig { @@ -490,6 +509,66 @@ public static class GuiSettings extends OkaeriConfig { @Comment({ "", "# The lore line showing when the parcel has arrived. Placeholders: {DATE} - arrival date" }) public String parcelArrivedLine = "&aArrived on: &2{DATE}"; + + @Comment({ "", "# The title of the parcel return GUI" }) + public String parcelReturnGuiTitle = "&5Return parcels"; + + @Comment({ "", "# The title of the return deposit GUI" }) + public String parcelReturnDepositGuiTitle = "&5Deposit the parcel items"; + + @Comment({ "", "# The item of the parcel locker return button" }) + public ConfigItem parcelLockerReturnItem = new ConfigItem() + .name("&5↩ Return parcels") + .lore(List.of("&5» &dClick to return a collected parcel.")) + .type(Material.HOPPER) + .glow(true); + + @Comment({ "", "# The item of the parcel in the return GUI" }) + public ConfigItem parcelReturnRowItem = new ConfigItem() + .name("&d{NAME}") + .lore(List.of( + "&6Sender: &e{SENDER}", + "&6Size: &e{SIZE}", + "&6Description: &e{DESCRIPTION}" + ) + ) + .type(Material.CHEST_MINECART); + + @Comment({ "", "# The item displayed in the return GUI when there is nothing to return" }) + public ConfigItem noReturnableParcelsItem = new ConfigItem() + .name("&4✘ &cNo returnable parcels") + .lore(List.of("&cYou don't have any parcels to return.")) + .type(Material.STRUCTURE_VOID); + + @Comment({ "", "# The item of the confirm return button" }) + public ConfigItem confirmReturnItem = new ConfigItem() + .name("&2✔ &aConfirm return") + .lore(List.of("&2» &aDeposit the original items above, then click to return the parcel.")) + .type(Material.GREEN_DYE); + + @Comment({ "", "# The lore line showing how long the parcel can still be returned. Placeholder: {DURATION}" }) + public String returnWindowRemainingLine = "&5Return window: &d{DURATION} left"; + + @Comment({ "", "# The lore line shown when the return window has expired." }) + public String returnWindowExpiredLine = "&cReturn window expired"; + } + + public static class ReturnChecks extends OkaeriConfig { + + @Comment("# Whether durability (damage) must match the original items.") + public boolean checkDurability = true; + + @Comment("# Whether custom display names must match the original items.") + public boolean checkItemName = true; + + @Comment("# Whether enchantments must match the original items.") + public boolean checkEnchantments = true; + + @Comment("# Whether lore must match the original items.") + public boolean checkLore = true; + + @Comment({"# Whether all remaining item data (NBT) must match the original items.", "# When false, only the attributes enabled above are compared."}) + public boolean checkNbt = true; } public static class DiscordSettings extends OkaeriConfig { From 827c54770d14d9bf616c06592025c5573a709e6b Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:23:01 +0200 Subject: [PATCH 06/17] feat: add config-driven item equivalence for parcel returns Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../returns/ReturnItemEquivalence.java | 103 ++++++++++++++++ .../returns/ReturnItemEquivalenceTest.java | 113 ++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalence.java create mode 100644 src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalence.java b/src/main/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalence.java new file mode 100644 index 000000000..70d89157b --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalence.java @@ -0,0 +1,103 @@ +package com.eternalcode.parcellockers.returns; + +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.BiPredicate; +import net.kyori.adventure.text.Component; +import org.bukkit.enchantments.Enchantment; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.Damageable; +import org.bukkit.inventory.meta.ItemMeta; + +/** + * Config-driven equivalence between a deposited item and an original parcel item. + * Material must always match; amounts are compared by {@link ParcelReturnValidator}, + * not here. The relation is symmetric. + */ +public class ReturnItemEquivalence implements BiPredicate { + + private final PluginConfig.ReturnChecks checks; + + public ReturnItemEquivalence(PluginConfig.ReturnChecks checks) { + this.checks = checks; + } + + @Override + public boolean test(ItemStack expected, ItemStack actual) { + if (expected.getType() != actual.getType()) { + return false; + } + if (this.checks.checkNbt) { + if (this.allAttributeChecksEnabled()) { + return expected.isSimilar(actual); + } + return this.normalize(expected).isSimilar(this.normalize(actual)); + } + return this.attributesMatch(expected.getItemMeta(), actual.getItemMeta()); + } + + private boolean allAttributeChecksEnabled() { + return this.checks.checkDurability + && this.checks.checkItemName + && this.checks.checkEnchantments + && this.checks.checkLore; + } + + /** Strips the attributes whose check is disabled so isSimilar ignores them. */ + private ItemStack normalize(ItemStack item) { + ItemStack copy = item.clone(); + ItemMeta meta = copy.getItemMeta(); + if (meta == null) { + return copy; + } + if (!this.checks.checkDurability && meta instanceof Damageable damageable) { + damageable.setDamage(0); + } + if (!this.checks.checkItemName) { + meta.displayName(null); + } + if (!this.checks.checkEnchantments) { + meta.getEnchants().keySet().forEach(meta::removeEnchant); + } + if (!this.checks.checkLore) { + meta.lore(null); + } + copy.setItemMeta(meta); + return copy; + } + + /** checkNbt = false: compare only the enabled attributes. */ + private boolean attributesMatch(ItemMeta expected, ItemMeta actual) { + if (this.checks.checkDurability && damage(expected) != damage(actual)) { + return false; + } + if (this.checks.checkItemName && !Objects.equals(displayName(expected), displayName(actual))) { + return false; + } + if (this.checks.checkEnchantments && !enchants(expected).equals(enchants(actual))) { + return false; + } + if (this.checks.checkLore && !Objects.equals(lore(expected), lore(actual))) { + return false; + } + return true; + } + + private static int damage(ItemMeta meta) { + return meta instanceof Damageable damageable ? damageable.getDamage() : 0; + } + + private static Component displayName(ItemMeta meta) { + return meta == null ? null : meta.displayName(); + } + + private static Map enchants(ItemMeta meta) { + return meta == null ? Map.of() : meta.getEnchants(); + } + + private static List lore(ItemMeta meta) { + return meta == null ? null : meta.lore(); + } +} diff --git a/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java b/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java new file mode 100644 index 000000000..fcb2529fd --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java @@ -0,0 +1,113 @@ +package com.eternalcode.parcellockers.returns; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import java.util.Map; +import org.bukkit.Material; +import org.bukkit.enchantments.Enchantment; +import org.bukkit.inventory.ItemStack; +import org.bukkit.inventory.meta.Damageable; +import org.bukkit.inventory.meta.ItemMeta; +import org.junit.jupiter.api.Test; + +class ReturnItemEquivalenceTest { + + private static PluginConfig.ReturnChecks checks(boolean durability, boolean name, boolean enchants, boolean lore, boolean nbt) { + PluginConfig.ReturnChecks checks = new PluginConfig.ReturnChecks(); + checks.checkDurability = durability; + checks.checkItemName = name; + checks.checkEnchantments = enchants; + checks.checkLore = lore; + checks.checkNbt = nbt; + return checks; + } + + private static ItemStack item(Material type) { + ItemStack item = mock(ItemStack.class); + when(item.getType()).thenReturn(type); + return item; + } + + private static ItemMeta damagedMeta(int damage) { + ItemMeta meta = mock(ItemMeta.class, withSettings().extraInterfaces(Damageable.class)); + when(((Damageable) meta).getDamage()).thenReturn(damage); + when(meta.getEnchants()).thenReturn(Map.of()); + return meta; + } + + @Test + void fullyStrictChecksDelegateToIsSimilar() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.DIAMOND_SWORD); + when(expected.isSimilar(actual)).thenReturn(true); + + ReturnItemEquivalence equivalence = new ReturnItemEquivalence(checks(true, true, true, true, true)); + assertTrue(equivalence.test(expected, actual)); + + when(expected.isSimilar(actual)).thenReturn(false); + assertFalse(equivalence.test(expected, actual)); + } + + @Test + void differentMaterialNeverMatches() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.IRON_SWORD); + + ReturnItemEquivalence equivalence = new ReturnItemEquivalence(checks(false, false, false, false, false)); + assertFalse(equivalence.test(expected, actual)); + } + + @Test + void durabilityMismatchFailsWhenChecked() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.DIAMOND_SWORD); + ItemMeta expectedMeta = damagedMeta(0); + ItemMeta actualMeta = damagedMeta(100); + when(expected.getItemMeta()).thenReturn(expectedMeta); + when(actual.getItemMeta()).thenReturn(actualMeta); + + assertFalse(new ReturnItemEquivalence(checks(true, false, false, false, false)).test(expected, actual)); + assertTrue(new ReturnItemEquivalence(checks(false, false, false, false, false)).test(expected, actual)); + } + + @Test + void enchantmentMismatchFailsWhenChecked() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.DIAMOND_SWORD); + + ItemMeta expectedMeta = mock(ItemMeta.class); + ItemMeta actualMeta = mock(ItemMeta.class); + when(expectedMeta.getEnchants()).thenReturn(enchantsOf("sharpness", 3)); + when(actualMeta.getEnchants()).thenReturn(Map.of()); + when(expected.getItemMeta()).thenReturn(expectedMeta); + when(actual.getItemMeta()).thenReturn(actualMeta); + + assertFalse(new ReturnItemEquivalence(checks(false, false, true, false, false)).test(expected, actual)); + assertTrue(new ReturnItemEquivalence(checks(false, false, false, false, false)).test(expected, actual)); + } + + /** + * Mocking Enchantment triggers its static registry lookup (needs a running server), so the + * key is a plain string smuggled through type erasure — getEnchants() map equality is all + * the equivalence compares. + */ + @SuppressWarnings({"unchecked", "rawtypes"}) + private static Map enchantsOf(String key, int level) { + return (Map) Map.of(key, level); + } + + @Test + void missingMetaOnBothSidesMatches() { + ItemStack expected = item(Material.COBBLESTONE); + ItemStack actual = item(Material.COBBLESTONE); + when(expected.getItemMeta()).thenReturn(null); + when(actual.getItemMeta()).thenReturn(null); + + assertTrue(new ReturnItemEquivalence(checks(true, true, true, true, false)).test(expected, actual)); + } +} From bee53f5e1ad6e5d5d7051cdf4d5576f87fa8ae95 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:26:59 +0200 Subject: [PATCH 07/17] feat: add multiset return-content validator Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../returns/ParcelReturnValidator.java | 56 +++++++++++++ .../returns/ParcelReturnValidatorTest.java | 82 +++++++++++++++++++ 2 files changed, 138 insertions(+) create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java create mode 100644 src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnValidatorTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java new file mode 100644 index 000000000..89a63abe1 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java @@ -0,0 +1,56 @@ +package com.eternalcode.parcellockers.returns; + +import java.util.ArrayList; +import java.util.List; +import java.util.function.BiPredicate; +import org.bukkit.inventory.ItemStack; + +/** + * Validates that deposited items are, as a multiset, exactly the original parcel content: + * every deposited stack must be equivalent to some original item, and the total amount per + * equivalence group must match. Stack splitting or merging is irrelevant. + */ +public class ParcelReturnValidator { + + private final BiPredicate equivalence; + + public ParcelReturnValidator(BiPredicate equivalence) { + this.equivalence = equivalence; + } + + public boolean matches(List deposited, List expected) { + List samples = new ArrayList<>(); + List expectedTotals = new ArrayList<>(); + List depositedTotals = new ArrayList<>(); + + for (ItemStack item : expected) { + int index = this.indexOf(samples, item); + if (index < 0) { + samples.add(item); + expectedTotals.add(item.getAmount()); + depositedTotals.add(0); + continue; + } + expectedTotals.set(index, expectedTotals.get(index) + item.getAmount()); + } + + for (ItemStack item : deposited) { + int index = this.indexOf(samples, item); + if (index < 0) { + return false; + } + depositedTotals.set(index, depositedTotals.get(index) + item.getAmount()); + } + + return expectedTotals.equals(depositedTotals); + } + + private int indexOf(List samples, ItemStack item) { + for (int i = 0; i < samples.size(); i++) { + if (this.equivalence.test(samples.get(i), item)) { + return i; + } + } + return -1; + } +} diff --git a/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnValidatorTest.java b/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnValidatorTest.java new file mode 100644 index 000000000..5c47b0a2b --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnValidatorTest.java @@ -0,0 +1,82 @@ +package com.eternalcode.parcellockers.returns; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.List; +import java.util.function.BiPredicate; +import org.bukkit.Material; +import org.bukkit.inventory.ItemStack; +import org.junit.jupiter.api.Test; + +class ParcelReturnValidatorTest { + + /** Equivalence by material only — the real attribute logic is tested in ReturnItemEquivalenceTest. */ + private static final BiPredicate BY_MATERIAL = (a, b) -> a.getType() == b.getType(); + + private final ParcelReturnValidator validator = new ParcelReturnValidator(BY_MATERIAL); + + private static ItemStack item(Material type, int amount) { + ItemStack item = mock(ItemStack.class); + when(item.getType()).thenReturn(type); + when(item.getAmount()).thenReturn(amount); + return item; + } + + @Test + void exactSameStacksMatch() { + List expected = List.of(item(Material.DIAMOND, 5), item(Material.OAK_LOG, 64)); + List deposited = List.of(item(Material.DIAMOND, 5), item(Material.OAK_LOG, 64)); + assertTrue(this.validator.matches(deposited, expected)); + } + + @Test + void splitStacksStillMatch() { + List expected = List.of(item(Material.OAK_LOG, 64)); + List deposited = List.of(item(Material.OAK_LOG, 30), item(Material.OAK_LOG, 34)); + assertTrue(this.validator.matches(deposited, expected)); + } + + @Test + void mergedStacksStillMatch() { + List expected = List.of(item(Material.OAK_LOG, 30), item(Material.OAK_LOG, 34)); + List deposited = List.of(item(Material.OAK_LOG, 64)); + assertTrue(this.validator.matches(deposited, expected)); + } + + @Test + void missingAmountFails() { + List expected = List.of(item(Material.DIAMOND, 5)); + List deposited = List.of(item(Material.DIAMOND, 4)); + assertFalse(this.validator.matches(deposited, expected)); + } + + @Test + void extraAmountFails() { + List expected = List.of(item(Material.DIAMOND, 5)); + List deposited = List.of(item(Material.DIAMOND, 6)); + assertFalse(this.validator.matches(deposited, expected)); + } + + @Test + void wrongTypeFails() { + List expected = List.of(item(Material.DIAMOND, 5)); + List deposited = List.of(item(Material.EMERALD, 5)); + assertFalse(this.validator.matches(deposited, expected)); + } + + @Test + void extraForeignItemFails() { + List expected = List.of(item(Material.DIAMOND, 5)); + List deposited = List.of(item(Material.DIAMOND, 5), item(Material.DIRT, 1)); + assertFalse(this.validator.matches(deposited, expected)); + } + + @Test + void emptyDepositAgainstNonEmptyContentFails() { + List expected = List.of(item(Material.DIAMOND, 5)); + assertFalse(this.validator.matches(List.of(), expected)); + } +} From 264d5c3260e210ef6aaaebba9aafa749f34682e8 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:32:02 +0200 Subject: [PATCH 08/17] feat: keep collected parcels for the return window instead of deleting them Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../parcellockers/ParcelLockers.java | 3 + .../parcel/service/ParcelService.java | 9 +++ .../parcel/service/ParcelServiceImpl.java | 72 +++++++++++++------ 3 files changed, 61 insertions(+), 23 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java index c7342143b..0623f647e 100644 --- a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java +++ b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java @@ -44,6 +44,7 @@ import com.eternalcode.parcellockers.parcel.service.ParcelService; import com.eternalcode.parcellockers.parcel.service.ParcelServiceImpl; import com.eternalcode.parcellockers.parcel.task.ParcelSendTask; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepositoryOrmLite; import com.eternalcode.parcellockers.updater.UpdaterService; import com.eternalcode.parcellockers.user.UserManager; import com.eternalcode.parcellockers.user.UserManagerImpl; @@ -122,12 +123,14 @@ public void onEnable() { DeliveryRepositoryOrmLite deliveryRepository = new DeliveryRepositoryOrmLite(databaseManager, scheduler); ItemStorageRepository itemStorageRepository = new ItemStorageRepositoryOrmLite(databaseManager, scheduler); UserRepository userRepository = new UserRepositoryOrmLite(databaseManager, scheduler); + CollectedParcelRepositoryOrmLite collectedParcelRepository = new CollectedParcelRepositoryOrmLite(databaseManager, scheduler); // service and managers ParcelService parcelService = new ParcelServiceImpl( noticeService, parcelRepository, parcelContentRepository, + collectedParcelRepository, scheduler, config, this.economy, diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java index abaa9cb55..364afc21f 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java @@ -44,6 +44,15 @@ public interface ParcelService { */ CompletableFuture> getCollectible(UUID receiver, UUID destinationLocker, Page page); + /** Returns the COLLECTED parcels of the given receiver (candidates for a return). */ + CompletableFuture> getReturnable(UUID receiver, Page page); + + /** + * Atomically turns a COLLECTED parcel into its reverse SENT shipment. Returns false when + * the parcel was already returned or purged in the meantime. + */ + CompletableFuture markReturned(Parcel returned); + CompletableFuture> getAll(Page page); CompletableFuture delete(UUID uuid); diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java index 27a687f88..4259aea10 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java @@ -10,15 +10,19 @@ import com.eternalcode.parcellockers.notification.NoticeService; import com.eternalcode.parcellockers.parcel.Parcel; import com.eternalcode.parcellockers.parcel.ParcelSize; +import com.eternalcode.parcellockers.parcel.ParcelStatus; import com.eternalcode.parcellockers.parcel.event.ParcelCollectEvent; import com.eternalcode.parcellockers.parcel.event.ParcelSendEvent; import com.eternalcode.parcellockers.parcel.repository.ParcelRepository; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepository; import com.eternalcode.parcellockers.shared.Page; import com.eternalcode.parcellockers.shared.PageResult; import com.eternalcode.parcellockers.shared.exception.ParcelOperationException; import com.github.benmanes.caffeine.cache.Cache; import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.base.Preconditions; +import java.time.Instant; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -43,6 +47,7 @@ public class ParcelServiceImpl implements ParcelService { private final NoticeService noticeService; private final ParcelRepository parcelRepository; private final ParcelContentRepository parcelContentRepository; + private final CollectedParcelRepository collectedParcelRepository; private final Scheduler scheduler; private final PluginConfig config; private final Economy economy; @@ -54,6 +59,7 @@ public ParcelServiceImpl( NoticeService noticeService, ParcelRepository parcelRepository, ParcelContentRepository parcelContentRepository, + CollectedParcelRepository collectedParcelRepository, Scheduler scheduler, PluginConfig config, Economy economy, @@ -62,6 +68,7 @@ public ParcelServiceImpl( this.noticeService = noticeService; this.parcelRepository = parcelRepository; this.parcelContentRepository = parcelContentRepository; + this.collectedParcelRepository = collectedParcelRepository; this.scheduler = scheduler; this.config = config; this.economy = economy; @@ -220,8 +227,11 @@ public CompletableFuture collect(Player player, Parcel parcel) { CompletableFuture result = new CompletableFuture<>(); // Re-check inventory space on the main thread (the previous async check was a TOCTOU), - // then delete the parcel BEFORE handing the items back so it cannot be collected twice. - // Items are only given once the delete is confirmed, so a failed delete never destroys them. + // then flip the status BEFORE handing the items back so the parcel cannot be collected + // twice. The parcel and content rows are kept: they are the snapshot a later return is + // validated against. The collected_parcels row is written first so that a successful + // flip always has a collection timestamp; a stray row from a failed flip is ignored by + // the purge task (it only purges parcels that are actually COLLECTED). this.scheduler.run(() -> { if (!canHold(player, items)) { this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.noInventorySpace); @@ -229,31 +239,17 @@ public CompletableFuture collect(Player player, Parcel parcel) { return; } - this.parcelRepository.delete(parcel) - .thenCompose(deleted -> { - if (!deleted) { - return CompletableFuture.completedFuture(false); - } - // The parcel is gone, so it can never be collected again; deleting its content - // is best-effort cleanup and must not block handing the items back. Otherwise a - // failed content delete would lose the items permanently. - return this.parcelContentRepository.delete(parcel.uuid()) - .handle((contentDeleted, throwable) -> { - if (throwable != null) { - this.server.getLogger().warning("Failed to delete content for collected parcel " - + parcel.uuid() + ": " + throwable.getMessage()); - } - return true; - }); - }) - .thenAccept(removed -> { - if (!removed) { - this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.databaseError); + this.collectedParcelRepository.save(new CollectedParcel(parcel.uuid(), Instant.now())) + .thenCompose(saved -> this.parcelRepository.markCollected(parcel.uuid())) + .thenAccept(marked -> { + if (!Boolean.TRUE.equals(marked)) { + // Someone else collected it first (or the status changed under us). + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.cannotCollect); result.complete(null); return; } - this.parcelsByUuid.invalidate(parcel.uuid()); + this.parcelsByUuid.put(parcel.uuid(), withStatus(parcel, ParcelStatus.COLLECTED)); this.scheduler.run(() -> { items.forEach(item -> ItemUtil.giveItem(player, item)); this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.collected); @@ -271,6 +267,12 @@ public CompletableFuture collect(Player player, Parcel parcel) { }); } + private static Parcel withStatus(Parcel parcel, ParcelStatus status) { + return new Parcel(parcel.uuid(), parcel.sender(), parcel.name(), parcel.description(), + parcel.priority(), parcel.receiver(), parcel.size(), parcel.entryLocker(), + parcel.destinationLocker(), status); + } + @Override public CompletableFuture> get(UUID uuid) { Objects.requireNonNull(uuid, "UUID cannot be null"); @@ -321,6 +323,30 @@ public CompletableFuture> getCollectible(UUID receiver, UUID }); } + @Override + public CompletableFuture> getReturnable(UUID receiver, Page page) { + Objects.requireNonNull(receiver, "Receiver UUID cannot be null"); + Objects.requireNonNull(page, "Page cannot be null"); + + return this.parcelRepository.findReturnable(receiver, page) + .thenApply(result -> { + result.items().forEach(parcel -> this.parcelsByUuid.put(parcel.uuid(), parcel)); + return result; + }); + } + + @Override + public CompletableFuture markReturned(Parcel returned) { + Objects.requireNonNull(returned, "Returned parcel cannot be null"); + + return this.parcelRepository.markReturned(returned).thenApply(updated -> { + if (updated) { + this.parcelsByUuid.put(returned.uuid(), returned); + } + return updated; + }); + } + @Override public CompletableFuture> getAll(Page page) { Objects.requireNonNull(page, "Page cannot be null"); From 54596bd9c8495ad521a9ffdf72051926fc112f99 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:40:47 +0200 Subject: [PATCH 09/17] feat: add parcel return service and ParcelReturnEvent Orchestrates the return of a COLLECTED parcel: re-verifies status/receiver, checks the return window, validates deposited items against the stored content snapshot, checks entry-locker capacity, charges the return fee (bypassable), flips the parcel into a reverse SENT shipment via ParcelService.markReturned, and schedules the normal delivery task. Every abort path hands the deposited items back and sends a specific notice; the fee is refunded on any failure after it was charged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../parcellockers/ParcelLockers.java | 18 ++ .../parcel/event/ParcelReturnEvent.java | 30 +++ .../returns/ParcelReturnService.java | 234 ++++++++++++++++++ .../returns/ParcelReturnServiceTest.java | 33 +++ 4 files changed, 315 insertions(+) create mode 100644 src/main/java/com/eternalcode/parcellockers/parcel/event/ParcelReturnEvent.java create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java create mode 100644 src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnServiceTest.java diff --git a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java index 0623f647e..16ae00ae3 100644 --- a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java +++ b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java @@ -44,6 +44,9 @@ import com.eternalcode.parcellockers.parcel.service.ParcelService; import com.eternalcode.parcellockers.parcel.service.ParcelServiceImpl; import com.eternalcode.parcellockers.parcel.task.ParcelSendTask; +import com.eternalcode.parcellockers.returns.ParcelReturnService; +import com.eternalcode.parcellockers.returns.ParcelReturnValidator; +import com.eternalcode.parcellockers.returns.ReturnItemEquivalence; import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepositoryOrmLite; import com.eternalcode.parcellockers.updater.UpdaterService; import com.eternalcode.parcellockers.user.UserManager; @@ -155,6 +158,21 @@ public void onEnable() { noticeService ); + ParcelReturnValidator returnValidator = new ParcelReturnValidator(new ReturnItemEquivalence(config.settings.returnChecks)); + ParcelReturnService parcelReturnService = new ParcelReturnService( + parcelService, + parcelContentManager, + collectedParcelRepository, + deliveryManager, + lockerManager, + returnValidator, + scheduler, + config, + noticeService, + this.economy, + server + ); + // guis TriumphGui.init(this); GuiManager guiManager = new GuiManager( diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/event/ParcelReturnEvent.java b/src/main/java/com/eternalcode/parcellockers/parcel/event/ParcelReturnEvent.java new file mode 100644 index 000000000..c388b0d4d --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/parcel/event/ParcelReturnEvent.java @@ -0,0 +1,30 @@ +package com.eternalcode.parcellockers.parcel.event; + +import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.shared.event.CancellableEvent; +import org.bukkit.event.HandlerList; +import org.jetbrains.annotations.NotNull; + +public class ParcelReturnEvent extends CancellableEvent { + + private static final HandlerList HANDLER_LIST = new HandlerList(); + + private final Parcel parcel; + + public ParcelReturnEvent(Parcel parcel) { + this.parcel = parcel; + } + + public static HandlerList getHandlerList() { + return HANDLER_LIST; + } + + public Parcel getParcel() { + return this.parcel; + } + + @Override + public @NotNull HandlerList getHandlers() { + return HANDLER_LIST; + } +} diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java new file mode 100644 index 000000000..078bb21cf --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java @@ -0,0 +1,234 @@ +package com.eternalcode.parcellockers.returns; + +import com.eternalcode.commons.bukkit.ItemUtil; +import com.eternalcode.commons.scheduler.Scheduler; +import com.eternalcode.multification.notice.provider.NoticeProvider; +import com.eternalcode.parcellockers.configuration.implementation.MessageConfig; +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import com.eternalcode.parcellockers.content.ParcelContentManager; +import com.eternalcode.parcellockers.delivery.DeliveryManager; +import com.eternalcode.parcellockers.locker.LockerManager; +import com.eternalcode.parcellockers.notification.NoticeService; +import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.parcel.ParcelSize; +import com.eternalcode.parcellockers.parcel.ParcelStatus; +import com.eternalcode.parcellockers.parcel.event.ParcelReturnEvent; +import com.eternalcode.parcellockers.parcel.service.ParcelService; +import com.eternalcode.parcellockers.parcel.task.ParcelSendTask; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepository; +import java.time.Duration; +import java.time.Instant; +import java.util.List; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.concurrent.CompletableFuture; +import java.util.logging.Logger; +import net.milkbowl.vault.economy.Economy; +import org.bukkit.Server; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; + +/** + * Orchestrates returning a collected parcel: validates the deposited items against the stored + * content snapshot, charges the return fee, flips the parcel into a reverse SENT shipment and + * schedules the normal delivery task. Every abort path hands the deposited items back. + */ +public class ParcelReturnService { + + private static final Logger LOGGER = Logger.getLogger(ParcelReturnService.class.getName()); + private static final String PARCEL_FEE_BYPASS_PERMISSION = "parcellockers.fee.bypass"; + private static final String PLACEHOLDER_AMOUNT = "{AMOUNT}"; + + private final ParcelService parcelService; + private final ParcelContentManager parcelContentManager; + private final CollectedParcelRepository collectedParcelRepository; + private final DeliveryManager deliveryManager; + private final LockerManager lockerManager; + private final ParcelReturnValidator validator; + private final Scheduler scheduler; + private final PluginConfig config; + private final NoticeService noticeService; + private final Economy economy; + private final Server server; + + public ParcelReturnService( + ParcelService parcelService, + ParcelContentManager parcelContentManager, + CollectedParcelRepository collectedParcelRepository, + DeliveryManager deliveryManager, + LockerManager lockerManager, + ParcelReturnValidator validator, + Scheduler scheduler, + PluginConfig config, + NoticeService noticeService, + Economy economy, + Server server + ) { + this.parcelService = parcelService; + this.parcelContentManager = parcelContentManager; + this.collectedParcelRepository = collectedParcelRepository; + this.deliveryManager = deliveryManager; + this.lockerManager = lockerManager; + this.validator = validator; + this.scheduler = scheduler; + this.config = config; + this.noticeService = noticeService; + this.economy = economy; + this.server = server; + } + + public static boolean isWithinReturnWindow(CollectedParcel collected, Duration window, Instant now) { + return collected.collectedAt().plus(window).isAfter(now); + } + + public CompletableFuture> getCollectedInfo(UUID parcelId) { + Objects.requireNonNull(parcelId, "Parcel UUID cannot be null"); + return this.collectedParcelRepository.find(parcelId); + } + + public CompletableFuture returnParcel(Player player, Parcel parcel, List deposited) { + Objects.requireNonNull(player, "Player cannot be null"); + Objects.requireNonNull(parcel, "Parcel cannot be null"); + Objects.requireNonNull(deposited, "Deposited items cannot be null"); + + ParcelReturnEvent event = new ParcelReturnEvent(parcel); + this.server.getPluginManager().callEvent(event); + if (event.isCancelled()) { + return this.abort(player, deposited, messages -> messages.parcel.cannotReturn); + } + + return this.parcelService.get(parcel.uuid()).thenCompose(optionalParcel -> { + if (optionalParcel.isEmpty() + || optionalParcel.get().status() != ParcelStatus.COLLECTED + || !player.getUniqueId().equals(optionalParcel.get().receiver())) { + return this.abort(player, deposited, messages -> messages.parcel.cannotReturn); + } + Parcel current = optionalParcel.get(); + + return this.collectedParcelRepository.find(current.uuid()).thenCompose(optionalCollected -> { + if (optionalCollected.isEmpty() + || !isWithinReturnWindow(optionalCollected.get(), this.config.settings.parcelReturnWindow, Instant.now())) { + return this.abort(player, deposited, messages -> messages.parcel.returnWindowExpired); + } + + return this.parcelContentManager.get(current.uuid()).thenCompose(optionalContent -> { + if (optionalContent.isEmpty()) { + return this.abort(player, deposited, messages -> messages.parcel.cannotReturn); + } + if (!this.validator.matches(deposited, optionalContent.get().items())) { + return this.abort(player, deposited, messages -> messages.parcel.returnItemsMismatch); + } + + // The return ships to the original entry locker. + return this.lockerManager.isLockerFull(current.entryLocker()).thenCompose(isFull -> { + if (Boolean.TRUE.equals(isFull)) { + return this.abort(player, deposited, messages -> messages.parcel.lockerFull); + } + return this.execute(player, current, deposited); + }); + }); + }); + }).exceptionally(throwable -> { + LOGGER.severe("Failed to return parcel " + parcel.uuid() + " for " + player.getName() + ": " + throwable.getMessage()); + this.giveBack(player, deposited); + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.cannotReturn); + return null; + }); + } + + private CompletableFuture execute(Player player, Parcel current, List deposited) { + double chargedFee = 0; + if (!player.hasPermission(PARCEL_FEE_BYPASS_PERMISSION)) { + double fee = this.returnFeeFor(current.size()); + if (fee > 0) { + boolean success = this.economy.withdrawPlayer(player, fee).transactionSuccess(); + String formattedFee = String.format("%.2f", fee); + if (!success) { + this.noticeService.create() + .notice(messages -> messages.parcel.insufficientFunds) + .player(player.getUniqueId()) + .placeholder(PLACEHOLDER_AMOUNT, formattedFee) + .send(); + this.giveBack(player, deposited); + return CompletableFuture.completedFuture(null); + } + chargedFee = fee; + this.noticeService.create() + .notice(messages -> messages.parcel.returnFeeWithdrawn) + .player(player.getUniqueId()) + .placeholder(PLACEHOLDER_AMOUNT, formattedFee) + .send(); + } + } + double refundableFee = chargedFee; + + Parcel returned = new Parcel(current.uuid(), current.receiver(), current.name(), + current.description(), current.priority(), current.sender(), current.size(), + current.destinationLocker(), current.entryLocker(), ParcelStatus.SENT); + + List depositedCopy = deposited.stream().map(ItemStack::clone).toList(); + + // Content is overwritten with the actually-deposited items first (they may legitimately + // differ from the snapshot when check flags are relaxed); only then the status flip makes + // the parcel a live shipment. markReturned failing means a concurrent return/purge won. + return this.parcelContentManager.update(current.uuid(), depositedCopy) + .thenCompose(updated -> this.parcelService.markReturned(returned)) + .thenCompose(marked -> { + if (!Boolean.TRUE.equals(marked)) { + this.refund(player, refundableFee); + return this.abort(player, deposited, messages -> messages.parcel.cannotReturn); + } + + // Best-effort cleanup: a leftover row is ignored by the purge task because the + // parcel is no longer COLLECTED. + this.collectedParcelRepository.delete(current.uuid()).exceptionally(throwable -> { + LOGGER.warning("Failed to delete collected_parcels row for returned parcel " + + current.uuid() + ": " + throwable.getMessage()); + return false; + }); + + Duration delay = returned.priority() + ? this.config.settings.priorityParcelSendDuration + : this.config.settings.parcelSendDuration; + this.deliveryManager.create(returned.uuid(), Instant.now().plus(delay)); + this.scheduler.runLaterAsync( + new ParcelSendTask(returned, this.parcelService, this.deliveryManager, this.scheduler), + delay); + + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returned); + return CompletableFuture.completedFuture(null); + }) + .exceptionally(throwable -> { + this.refund(player, refundableFee); + this.giveBack(player, deposited); + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.cannotReturn); + LOGGER.severe("Failed to execute return of parcel " + current.uuid() + ": " + throwable.getMessage()); + return null; + }); + } + + private double returnFeeFor(ParcelSize size) { + return switch (size) { + case SMALL -> this.config.settings.smallParcelReturnFee; + case MEDIUM -> this.config.settings.mediumParcelReturnFee; + case LARGE -> this.config.settings.largeParcelReturnFee; + }; + } + + private void refund(Player player, double fee) { + if (fee > 0) { + this.economy.depositPlayer(player, fee); + } + } + + private CompletableFuture abort(Player player, List deposited, NoticeProvider notice) { + this.giveBack(player, deposited); + this.noticeService.player(player.getUniqueId(), notice); + return CompletableFuture.completedFuture(null); + } + + private void giveBack(Player player, List items) { + this.scheduler.run(() -> items.forEach(item -> ItemUtil.giveItem(player, item))); + } +} diff --git a/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnServiceTest.java b/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnServiceTest.java new file mode 100644 index 000000000..ab1a59b9f --- /dev/null +++ b/src/test/java/com/eternalcode/parcellockers/returns/ParcelReturnServiceTest.java @@ -0,0 +1,33 @@ +package com.eternalcode.parcellockers.returns; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.time.Duration; +import java.time.Instant; +import java.util.UUID; +import org.junit.jupiter.api.Test; + +class ParcelReturnServiceTest { + + private static final Instant NOW = Instant.parse("2026-07-02T12:00:00Z"); + private static final Duration WINDOW = Duration.ofDays(7); + + @Test + void withinWindowJustAfterCollection() { + CollectedParcel collected = new CollectedParcel(UUID.randomUUID(), NOW.minus(Duration.ofHours(1))); + assertTrue(ParcelReturnService.isWithinReturnWindow(collected, WINDOW, NOW)); + } + + @Test + void outsideWindowAfterExpiry() { + CollectedParcel collected = new CollectedParcel(UUID.randomUUID(), NOW.minus(Duration.ofDays(8))); + assertFalse(ParcelReturnService.isWithinReturnWindow(collected, WINDOW, NOW)); + } + + @Test + void exactExpiryInstantIsOutsideWindow() { + CollectedParcel collected = new CollectedParcel(UUID.randomUUID(), NOW.minus(WINDOW)); + assertFalse(ParcelReturnService.isWithinReturnWindow(collected, WINDOW, NOW)); + } +} From f47e28a098cfbb378dd947a6a8e59953cfa38e80 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:48:50 +0200 Subject: [PATCH 10/17] fix: keep post-commit return failures away from the refund path After markReturned succeeds the parcel is a live shipment; a throwing DeliveryManager.create (stale cached delivery from the original trip) must not trigger the refund/give-back recovery, which would duplicate the deposited items. Schedule the send task first and upsert the delivery timestamp instead. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../returns/ParcelReturnService.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java index 078bb21cf..cb9e82b4d 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java @@ -180,6 +180,10 @@ private CompletableFuture execute(Player player, Parcel current, List messages.parcel.cannotReturn); } + // Past the commit point: the parcel is now a live SENT shipment and its content IS the + // deposited items. Nothing below may route to the refund/give-back recovery — undoing + // here would duplicate the items. + // Best-effort cleanup: a leftover row is ignored by the purge task because the // parcel is no longer COLLECTED. this.collectedParcelRepository.delete(current.uuid()).exceptionally(throwable -> { @@ -191,11 +195,26 @@ private CompletableFuture execute(Player player, Parcel current, List { + LOGGER.severe("Failed to persist delivery for returned parcel " + returned.uuid() + + " (send task scheduled in-memory; a restart before it fires may strand the parcel): " + + throwable.getMessage()); + return null; + }); + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returned); return CompletableFuture.completedFuture(null); }) From ad20fff4a5a41c20298f43581b2dc71692305b40 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 19:52:19 +0200 Subject: [PATCH 11/17] feat: purge collected parcels after the return window expires Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../parcellockers/ParcelLockers.java | 7 ++ .../returns/task/ReturnWindowPurgeTask.java | 70 +++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java diff --git a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java index 16ae00ae3..d09349c5e 100644 --- a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java +++ b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java @@ -48,6 +48,7 @@ import com.eternalcode.parcellockers.returns.ParcelReturnValidator; import com.eternalcode.parcellockers.returns.ReturnItemEquivalence; import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepositoryOrmLite; +import com.eternalcode.parcellockers.returns.task.ReturnWindowPurgeTask; import com.eternalcode.parcellockers.updater.UpdaterService; import com.eternalcode.parcellockers.user.UserManager; import com.eternalcode.parcellockers.user.UserManagerImpl; @@ -173,6 +174,12 @@ public void onEnable() { server ); + scheduler.timerAsync( + new ReturnWindowPurgeTask(parcelService, collectedParcelRepository, config), + Duration.ofSeconds(30), + Duration.ofMinutes(30) + ); + // guis TriumphGui.init(this); GuiManager guiManager = new GuiManager( diff --git a/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java b/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java new file mode 100644 index 000000000..56281fe7c --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java @@ -0,0 +1,70 @@ +package com.eternalcode.parcellockers.returns.task; + +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import com.eternalcode.parcellockers.parcel.ParcelStatus; +import com.eternalcode.parcellockers.parcel.service.ParcelService; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import com.eternalcode.parcellockers.returns.repository.CollectedParcelRepository; +import java.time.Duration; +import java.time.Instant; +import java.util.concurrent.CompletableFuture; +import java.util.logging.Logger; + +/** + * Deletes collected parcels whose return window expired: the parcel row, its content row and the + * collected_parcels row. Runs periodically; failures are logged and retried on the next run. + */ +public class ReturnWindowPurgeTask implements Runnable { + + private static final Logger LOGGER = Logger.getLogger(ReturnWindowPurgeTask.class.getName()); + + /** Extra slack past the window so an in-flight return at the expiry boundary cannot race the purge. */ + private static final Duration GRACE = Duration.ofMinutes(5); + + private final ParcelService parcelService; + private final CollectedParcelRepository collectedParcelRepository; + private final PluginConfig config; + + public ReturnWindowPurgeTask( + ParcelService parcelService, + CollectedParcelRepository collectedParcelRepository, + PluginConfig config + ) { + this.parcelService = parcelService; + this.collectedParcelRepository = collectedParcelRepository; + this.config = config; + } + + @Override + public void run() { + Instant cutoff = Instant.now().minus(this.config.settings.parcelReturnWindow).minus(GRACE); + + this.collectedParcelRepository.findExpired(cutoff) + .thenAccept(expired -> expired.forEach(this::purge)) + .exceptionally(throwable -> { + LOGGER.severe("Failed to query expired collected parcels: " + throwable.getMessage()); + return null; + }); + } + + private void purge(CollectedParcel collected) { + this.parcelService.get(collected.parcel()) + .thenCompose(optionalParcel -> { + if (optionalParcel.isPresent() && optionalParcel.get().status() == ParcelStatus.COLLECTED) { + // Delete the parcel (and content) first; the row is only removed once that + // succeeded so a failed delete is retried on the next run. + return this.parcelService.delete(collected.parcel()) + .thenCompose(deleted -> Boolean.TRUE.equals(deleted) + ? this.collectedParcelRepository.delete(collected.parcel()) + : CompletableFuture.completedFuture(false)); + } + // Stray row: the parcel is gone or is a live shipment again — drop only the row. + return this.collectedParcelRepository.delete(collected.parcel()); + }) + .exceptionally(throwable -> { + LOGGER.warning("Failed to purge expired collected parcel " + collected.parcel() + + " (will retry next run): " + throwable.getMessage()); + return false; + }); + } +} From 3e452311eddd9e2fb66182f71c2a59435b624275 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Fri, 3 Jul 2026 20:01:31 +0200 Subject: [PATCH 12/17] feat: add return button, return list GUI and deposit GUI (GH-69) Wires the last piece of the parcel return flow into the GUI layer: GuiManager gains getReturnableParcels/getCollectedInfo/returnParcel passthroughs to ParcelService/ParcelReturnService, ReturnGui lists a player's collected parcels with a remaining-return-window lore line, ReturnDepositGui lets them deposit the original items and confirms the return (or gives everything back on close), and LockerGui gets a "Return parcels" button between collect and send. Threads the return window Duration through SendingGui/ItemStorageGui as well, since they rebuild LockerGui/each other and would otherwise fail to compile after LockerGui's constructor changed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../parcellockers/ParcelLockers.java | 6 +- .../parcellockers/gui/GuiManager.java | 19 +- .../implementation/locker/ItemStorageGui.java | 9 +- .../gui/implementation/locker/LockerGui.java | 18 +- .../locker/ReturnDepositGui.java | 109 +++++++++++ .../gui/implementation/locker/ReturnGui.java | 173 ++++++++++++++++++ .../gui/implementation/locker/SendingGui.java | 12 +- 7 files changed, 336 insertions(+), 10 deletions(-) create mode 100644 src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnDepositGui.java create mode 100644 src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java diff --git a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java index d09349c5e..5b32696fc 100644 --- a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java +++ b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java @@ -190,7 +190,8 @@ public void onEnable() { parcelDispatchService, parcelContentManager, deliveryManager, - config.settings.allowCollectingFromAnyLocker + config.settings.allowCollectingFromAnyLocker, + parcelReturnService ); MainGui mainGUI = new MainGui( @@ -205,7 +206,8 @@ public void onEnable() { scheduler, config.guiSettings, guiManager, - noticeService + noticeService, + config.settings.parcelReturnWindow ); AdminParcelService adminParcelService = new AdminParcelService( diff --git a/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java b/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java index 5ed683ef6..0c560a8e2 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java @@ -12,6 +12,8 @@ import com.eternalcode.parcellockers.parcel.Parcel; import com.eternalcode.parcellockers.parcel.service.ParcelDispatchService; import com.eternalcode.parcellockers.parcel.service.ParcelService; +import com.eternalcode.parcellockers.returns.CollectedParcel; +import com.eternalcode.parcellockers.returns.ParcelReturnService; import com.eternalcode.parcellockers.shared.Page; import com.eternalcode.parcellockers.shared.PageResult; import com.eternalcode.parcellockers.user.User; @@ -35,6 +37,7 @@ public class GuiManager { private final ParcelContentManager parcelContentManager; private final DeliveryManager deliveryManager; private final boolean allowCollectingFromAnyLocker; + private final ParcelReturnService parcelReturnService; public GuiManager( ParcelService parcelService, @@ -44,7 +47,8 @@ public GuiManager( ParcelDispatchService parcelDispatchService, ParcelContentManager parcelContentManager, DeliveryManager deliveryManager, - boolean allowCollectingFromAnyLocker + boolean allowCollectingFromAnyLocker, + ParcelReturnService parcelReturnService ) { this.parcelService = parcelService; this.lockerManager = lockerManager; @@ -54,6 +58,7 @@ public GuiManager( this.parcelContentManager = parcelContentManager; this.deliveryManager = deliveryManager; this.allowCollectingFromAnyLocker = allowCollectingFromAnyLocker; + this.parcelReturnService = parcelReturnService; } /** @@ -157,4 +162,16 @@ public CompletableFuture deleteAllParcels(CommandSender sender, NoticeServ public CompletableFuture deleteAllLockers(CommandSender sender, NoticeService noticeService) { return this.lockerManager.deleteAll(sender, noticeService); } + + public CompletableFuture> getReturnableParcels(UUID receiver, Page page) { + return this.parcelService.getReturnable(receiver, page); + } + + public CompletableFuture> getCollectedInfo(UUID parcelId) { + return this.parcelReturnService.getCollectedInfo(parcelId); + } + + public void returnParcel(Player player, Parcel parcel, List deposited) { + this.parcelReturnService.returnParcel(player, parcel, deposited); + } } diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java index 0dc1d6eab..a7aef9406 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java @@ -11,6 +11,7 @@ import dev.triumphteam.gui.guis.Gui; import dev.triumphteam.gui.guis.GuiItem; import dev.triumphteam.gui.guis.StorageGui; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.stream.IntStream; @@ -26,6 +27,7 @@ public class ItemStorageGui { private final GuiManager guiManager; private final NoticeService noticeService; private final SendingGuiState state; + private final Duration returnWindow; public ItemStorageGui( Scheduler scheduler, @@ -33,7 +35,8 @@ public ItemStorageGui( MiniMessage miniMessage, GuiManager guiManager, NoticeService noticeService, - SendingGuiState state + SendingGuiState state, + Duration returnWindow ) { this.scheduler = scheduler; this.guiSettings = guiSettings; @@ -41,6 +44,7 @@ public ItemStorageGui( this.guiManager = guiManager; this.noticeService = noticeService; this.state = state; + this.returnWindow = returnWindow; } void show(Player player, ParcelSize size) { @@ -111,7 +115,8 @@ void show(Player player, ParcelSize size) { this.miniMessage, this.noticeService, this.guiManager, - this.state + this.state, + this.returnWindow ).show(player)) .exceptionally(throwable -> { // Persisting the staged items failed; hand them back so they are not lost. diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java index 3ee62762a..ff90417e3 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java @@ -9,6 +9,7 @@ import com.eternalcode.parcellockers.notification.NoticeService; import dev.triumphteam.gui.guis.Gui; import dev.triumphteam.gui.guis.GuiItem; +import java.time.Duration; import java.util.UUID; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.minimessage.MiniMessage; @@ -21,17 +22,19 @@ public class LockerGui implements GuiView { private final GuiSettings guiSettings; private final GuiManager guiManager; private final NoticeService noticeService; + private final Duration returnWindow; public LockerGui( MiniMessage miniMessage, Scheduler scheduler, GuiSettings guiSettings, GuiManager guiManager, - NoticeService noticeService + NoticeService noticeService, Duration returnWindow ) { this.miniMessage = miniMessage; this.scheduler = scheduler; this.guiSettings = guiSettings; this.guiManager = guiManager; this.noticeService = noticeService; + this.returnWindow = returnWindow; } public void show(Player player, UUID entryLocker) { @@ -53,14 +56,25 @@ public void show(Player player, UUID entryLocker) { entryLocker ); + ReturnGui returnGui = new ReturnGui( + this.guiSettings, + this.scheduler, + this.guiManager, + this.miniMessage, + this.noticeService, + this.returnWindow + ); + gui.setItem(21, this.guiSettings.parcelLockerCollectItem.toGuiItem(event -> collectionGui.show(player))); + gui.setItem(22, this.guiSettings.parcelLockerReturnItem.toGuiItem(event -> returnGui.show(player))); gui.setItem(23, this.guiSettings.parcelLockerSendItem.toGuiItem(event -> new SendingGui( this.scheduler, this.guiSettings, this.miniMessage, this.noticeService, this.guiManager, - new SendingGuiState().entryLocker(entryLocker) + new SendingGuiState().entryLocker(entryLocker), + this.returnWindow ).show(player, entryLocker))); gui.open(player); diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnDepositGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnDepositGui.java new file mode 100644 index 000000000..85e0e8610 --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnDepositGui.java @@ -0,0 +1,109 @@ +package com.eternalcode.parcellockers.gui.implementation.locker; + +import com.eternalcode.commons.bukkit.ItemUtil; +import com.eternalcode.commons.scheduler.Scheduler; +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig.GuiSettings; +import com.eternalcode.parcellockers.gui.GuiManager; +import com.eternalcode.parcellockers.notification.NoticeService; +import com.eternalcode.parcellockers.parcel.Parcel; +import dev.triumphteam.gui.guis.Gui; +import dev.triumphteam.gui.guis.GuiItem; +import dev.triumphteam.gui.guis.StorageGui; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.IntStream; +import net.kyori.adventure.text.minimessage.MiniMessage; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; + +/** + * Deposit GUI for a parcel return: the player places the original items, then confirms. + * Confirm hands the stacks to the return service (which gives them back on any failure); + * closing without confirming gives everything back immediately. + */ +public class ReturnDepositGui { + + private final Scheduler scheduler; + private final GuiSettings guiSettings; + private final MiniMessage miniMessage; + private final GuiManager guiManager; + private final NoticeService noticeService; + private final Parcel parcel; + + public ReturnDepositGui( + Scheduler scheduler, + GuiSettings guiSettings, + MiniMessage miniMessage, + GuiManager guiManager, + NoticeService noticeService, + Parcel parcel + ) { + this.scheduler = scheduler; + this.guiSettings = guiSettings; + this.miniMessage = miniMessage; + this.guiManager = guiManager; + this.noticeService = noticeService; + this.parcel = parcel; + } + + void show(Player player) { + int rows = switch (this.parcel.size()) { + case SMALL -> 2; + case MEDIUM -> 3; + case LARGE -> 4; + }; + + StorageGui gui = Gui.storage() + .title(this.miniMessage.deserialize(this.guiSettings.parcelReturnDepositGuiTitle)) + .rows(rows) + .create(); + + GuiItem backgroundItem = this.guiSettings.mainGuiBackgroundItem.toGuiItem(event -> event.setCancelled(true)); + IntStream.rangeClosed(1, 9).forEach(i -> gui.setItem(gui.getRows(), i, backgroundItem)); + + AtomicBoolean confirmed = new AtomicBoolean(false); + + GuiItem confirmItem = this.guiSettings.confirmReturnItem.toGuiItem(event -> { + event.setCancelled(true); + + List deposited = this.takeDepositedItems(gui); + if (deposited.isEmpty()) { + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returnItemsMismatch); + return; + } + + confirmed.set(true); + gui.close(player); + this.guiManager.returnParcel(player, this.parcel, deposited); + }); + gui.setItem(gui.getRows(), 5, confirmItem); + + gui.setCloseGuiAction(event -> { + if (confirmed.get()) { + return; + } + // Closed without confirming: give every deposited stack back. + List leftovers = this.takeDepositedItems(gui); + this.scheduler.run(() -> leftovers.forEach(item -> ItemUtil.giveItem(player, item))); + }); + + this.scheduler.run(() -> gui.open(player)); + } + + /** Snapshots and clears the deposit slots (everything above the bottom control row). */ + private List takeDepositedItems(StorageGui gui) { + ItemStack[] contents = gui.getInventory().getContents(); + List items = new ArrayList<>(); + + for (int i = 0; i < contents.length - 9; i++) { + ItemStack item = contents[i]; + if (item == null || item.isEmpty()) { + continue; + } + items.add(item.clone()); + gui.getInventory().setItem(i, null); + } + return items; + } +} diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java new file mode 100644 index 000000000..cf250a5be --- /dev/null +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java @@ -0,0 +1,173 @@ +package com.eternalcode.parcellockers.gui.implementation.locker; + +import com.eternalcode.commons.concurrent.FutureHandler; +import com.eternalcode.commons.scheduler.Scheduler; +import com.eternalcode.parcellockers.configuration.implementation.PluginConfig.GuiSettings; +import com.eternalcode.parcellockers.configuration.serializable.ConfigItem; +import com.eternalcode.parcellockers.gui.GuiManager; +import com.eternalcode.parcellockers.gui.GuiView; +import com.eternalcode.parcellockers.gui.PaginatedGuiRefresher; +import com.eternalcode.parcellockers.notification.NoticeService; +import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.parcel.util.PlaceholderUtil; +import com.eternalcode.parcellockers.shared.Page; +import com.eternalcode.parcellockers.util.DurationUtil; +import com.eternalcode.parcellockers.util.MaterialUtil; +import com.spotify.futures.CompletableFutures; +import dev.triumphteam.gui.guis.Gui; +import dev.triumphteam.gui.guis.GuiItem; +import dev.triumphteam.gui.guis.PaginatedGui; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.function.Supplier; +import net.kyori.adventure.text.Component; +import net.kyori.adventure.text.minimessage.MiniMessage; +import org.bukkit.entity.Player; +import org.bukkit.inventory.ItemStack; + +public class ReturnGui implements GuiView { + + private static final int WIDTH = 7; + private static final int HEIGHT = 4; + private static final Page FIRST_PAGE = new Page(0, WIDTH * HEIGHT); + + private final GuiSettings guiSettings; + private final Scheduler scheduler; + private final GuiManager guiManager; + private final MiniMessage miniMessage; + private final NoticeService noticeService; + private final Duration returnWindow; + + public ReturnGui( + GuiSettings guiSettings, + Scheduler scheduler, + GuiManager guiManager, + MiniMessage miniMessage, + NoticeService noticeService, + Duration returnWindow + ) { + this.guiSettings = guiSettings; + this.scheduler = scheduler; + this.guiManager = guiManager; + this.miniMessage = miniMessage; + this.noticeService = noticeService; + this.returnWindow = returnWindow; + } + + @Override + public void show(Player player) { + this.show(player, FIRST_PAGE); + } + + @Override + public void show(Player player, Page page) { + Component guiTitle = this.miniMessage.deserialize(this.guiSettings.parcelReturnGuiTitle); + ConfigItem rowItem = this.guiSettings.parcelReturnRowItem; + + PaginatedGui gui = Gui.paginated() + .rows(6) + .disableAllInteractions() + .title(guiTitle) + .create(); + + this.setupStaticItems(player, gui); + + this.guiManager.getReturnableParcels(player.getUniqueId(), page).thenAccept(result -> { + if (result == null || result.items().isEmpty()) { + gui.setItem(22, this.guiSettings.noReturnableParcelsItem.toGuiItem()); + this.scheduler.run(() -> gui.open(player)); + return; + } + + PaginatedGuiRefresher refresher = new PaginatedGuiRefresher(gui); + + this.setupNavigation(gui, page, result, player, this.guiSettings); + + result.items().stream() + .map(parcel -> this.createParcelItemAsync(parcel, rowItem, player)) + .collect(CompletableFutures.joinList()) + .thenAccept(suppliers -> { + if (suppliers.isEmpty()) { + gui.setItem(22, this.guiSettings.noReturnableParcelsItem.toGuiItem()); + this.scheduler.run(() -> gui.open(player)); + return; + } + for (Supplier supplier : suppliers) { + refresher.addItem(supplier); + } + this.scheduler.run(() -> gui.open(player)); + }).exceptionally(FutureHandler::handleException); + }).exceptionally(FutureHandler::handleException); + } + + private void setupStaticItems(Player player, PaginatedGui gui) { + GuiItem closeItem = this.guiSettings.closeItem.toGuiItem(event -> gui.close(player)); + GuiItem cornerItem = this.guiSettings.cornerItem.toGuiItem(); + GuiItem backgroundItem = this.guiSettings.mainGuiBackgroundItem.toGuiItem(); + + for (int cornerSlot : CORNER_SLOTS) { + gui.setItem(cornerSlot, cornerItem); + } + + for (int borderSlot : BORDER_SLOTS) { + gui.setItem(borderSlot, backgroundItem); + } + + gui.setItem(49, closeItem); + } + + private CompletableFuture> createParcelItemAsync( + Parcel parcel, + ConfigItem rowItem, + Player player + ) { + CompletableFuture> loreFuture = + PlaceholderUtil.replaceParcelPlaceholdersAsync(parcel, rowItem.lore(), this.guiManager); + CompletableFuture> contentFuture = this.guiManager.getParcelContent(parcel.uuid()) + .thenApply(optional -> optional.map(content -> content.items()).orElse(List.of())); + CompletableFuture windowLineFuture = this.guiManager.getCollectedInfo(parcel.uuid()) + .thenApply(optional -> optional + .map(collected -> { + Duration remaining = Duration.between(Instant.now(), collected.collectedAt().plus(this.returnWindow)); + return remaining.isNegative() || remaining.isZero() + ? this.guiSettings.returnWindowExpiredLine + : this.guiSettings.returnWindowRemainingLine.replace("{DURATION}", DurationUtil.format(remaining)); + }) + .orElse(this.guiSettings.returnWindowExpiredLine)); + + return CompletableFutures.combine(loreFuture, contentFuture, windowLineFuture, (processedLore, items, windowLine) -> { + Supplier supplier = () -> { + ConfigItem item = rowItem.clone(); + item.name(item.name().replace("{NAME}", parcel.name())); + + List lore = new ArrayList<>(processedLore); + lore.add(windowLine); + if (!items.isEmpty()) { + lore.add(this.guiSettings.parcelItemsCollectionGui); + for (ItemStack itemStack : items) { + lore.add(this.guiSettings.parcelItemCollectionFormat + .replace("{ITEM}", MaterialUtil.format(itemStack.getType())) + .replace("{AMOUNT}", Integer.toString(itemStack.getAmount())) + ); + } + } + + item.lore(lore); + item.glow(true); + + return item.toGuiItem(event -> new ReturnDepositGui( + this.scheduler, + this.guiSettings, + this.miniMessage, + this.guiManager, + this.noticeService, + parcel + ).show(player)); + }; + return supplier; + }).toCompletableFuture(); + } +} diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java index 31b3d9f05..dbb88960f 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java @@ -21,6 +21,7 @@ import io.papermc.paper.registry.data.dialog.action.DialogAction; import io.papermc.paper.registry.data.dialog.input.DialogInput; import io.papermc.paper.registry.data.dialog.type.DialogType; +import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -53,6 +54,7 @@ public class SendingGui implements GuiView { private final GuiManager guiManager; private final SendingGuiState state; + private final Duration returnWindow; private Gui gui; @@ -62,7 +64,8 @@ public SendingGui( MiniMessage miniMessage, NoticeService noticeService, GuiManager guiManager, - SendingGuiState state + SendingGuiState state, + Duration returnWindow ) { this.scheduler = scheduler; this.guiSettings = guiSettings; @@ -70,6 +73,7 @@ public SendingGui( this.noticeService = noticeService; this.guiManager = guiManager; this.state = state; + this.returnWindow = returnWindow; } public void show(Player player, UUID entryLocker) { @@ -192,7 +196,8 @@ public void show(Player player) { this.miniMessage, this.guiManager, this.noticeService, - this.state + this.state, + this.returnWindow ); this.guiManager.getItemStorage(player.getUniqueId()).thenAccept(result -> { int slotsSize = result.items().size(); @@ -251,7 +256,8 @@ public void show(Player player) { this.scheduler, this.guiSettings, this.guiManager, - this.noticeService + this.noticeService, + this.returnWindow ).show(player, this.state.entryLocker())); ConfigItem smallButton = this.guiSettings.smallParcelSizeItem; From 2a9f44f104ff858d7c3173be6793c6ef321af754 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Sat, 4 Jul 2026 10:03:06 +0200 Subject: [PATCH 13/17] fix: refuse status changes on COLLECTED parcels (GH-69) Flipping a COLLECTED parcel's status back to SENT re-armed a delivery even though the content row still held the collection snapshot, letting an admin one-click the receiver into collecting the same items twice. AdminParcelService.changeStatus now rejects any status change while a parcel is COLLECTED, and AdminParcelEditGui no longer offers the toggle for one, so the click surfaces the new admin.statusLocked notice instead of a silent no-op. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../implementation/MessageConfig.java | 1 + .../admin/AdminParcelEditGui.java | 6 ++++++ .../parcel/service/AdminParcelService.java | 9 ++++++++ .../parcel/service/EditResult.java | 2 +- .../service/AdminParcelServiceTest.java | 21 +++++++++++++++++++ 5 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java index 4bcd1ea84..06f9ed6bc 100644 --- a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java +++ b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java @@ -209,6 +209,7 @@ public static class AdminMessages extends OkaeriConfig { public Notice teleportWorldMissing = Notice.chat("&4✘ &cThat locker's world is not loaded."); public Notice sizeTooSmall = Notice.chat("&4✘ &cThe parcel's contents do not fit in that size."); public Notice destinationFull = Notice.chat("&4✘ &cThat destination locker is full."); + public Notice statusLocked = Notice.chat("&4✘ &cA collected parcel's status cannot be changed; it can only be returned."); public Notice contentsUpdated = Notice.chat("&2✔ &aParcel contents updated."); public Notice priorityUpdated = Notice.chat("&2✔ &aPriority updated and delivery time adjusted."); public Notice noPermission = Notice.chat("&4✘ &cYou do not have permission to do that."); 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 60fa2f476..38fe917ad 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 @@ -188,6 +188,7 @@ private void notifyResult(Player player, EditResult result) { case OK -> this.noticeService.create().notice(m -> m.admin.parcelUpdated).player(player.getUniqueId()).send(); case SIZE_TOO_SMALL -> this.noticeService.create().notice(m -> m.admin.sizeTooSmall).player(player.getUniqueId()).send(); case DESTINATION_FULL -> this.noticeService.create().notice(m -> m.admin.destinationFull).player(player.getUniqueId()).send(); + case PARCEL_COLLECTED -> this.noticeService.create().notice(m -> m.admin.statusLocked).player(player.getUniqueId()).send(); } } @@ -227,6 +228,11 @@ private static ParcelSize nextSize(ParcelSize size) { } private static ParcelStatus nextStatus(ParcelStatus status) { + if (status == ParcelStatus.COLLECTED) { + // A COLLECTED parcel cannot be toggled back to SENT/DELIVERED; AdminParcelService + // rejects the change too, so this only avoids offering the flip in the first place. + return ParcelStatus.COLLECTED; + } return status == ParcelStatus.SENT ? ParcelStatus.DELIVERED : ParcelStatus.SENT; } } diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java index 36b6e7418..fdb3d1539 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java @@ -62,7 +62,16 @@ public CompletableFuture changeDescription(Parcel parcel, String des return this.persist(withDescription(parcel, description)); } + /** + * A COLLECTED parcel's content row is the collection snapshot, not a live shipment; flipping + * its status (e.g. back to SENT) would arm a delivery and hand the receiver the same items + * a second time. Refuse the change outright — a return is the only supported transition out + * of COLLECTED. + */ public CompletableFuture changeStatus(Parcel parcel, ParcelStatus status) { + if (parcel.status() == ParcelStatus.COLLECTED) { + return CompletableFuture.completedFuture(EditResult.of(EditResult.Status.PARCEL_COLLECTED)); + } Parcel updated = withStatus(parcel, status); return this.parcelService.update(updated).thenCompose(ignored -> { if (status == parcel.status()) { diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/EditResult.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/EditResult.java index c245412e6..d00ef3add 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/EditResult.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/EditResult.java @@ -2,7 +2,7 @@ public final class EditResult { - public enum Status { OK, SIZE_TOO_SMALL, DESTINATION_FULL } + public enum Status { OK, SIZE_TOO_SMALL, DESTINATION_FULL, PARCEL_COLLECTED } private final Status status; diff --git a/src/test/java/com/eternalcode/parcellockers/parcel/service/AdminParcelServiceTest.java b/src/test/java/com/eternalcode/parcellockers/parcel/service/AdminParcelServiceTest.java index 586e18c27..aa33f5a2f 100644 --- a/src/test/java/com/eternalcode/parcellockers/parcel/service/AdminParcelServiceTest.java +++ b/src/test/java/com/eternalcode/parcellockers/parcel/service/AdminParcelServiceTest.java @@ -2,13 +2,34 @@ import static org.junit.jupiter.api.Assertions.assertEquals; +import com.eternalcode.parcellockers.parcel.Parcel; import com.eternalcode.parcellockers.parcel.ParcelSize; +import com.eternalcode.parcellockers.parcel.ParcelStatus; import java.time.Duration; import java.time.Instant; +import java.util.UUID; import org.junit.jupiter.api.Test; class AdminParcelServiceTest { + private static Parcel collectedParcel() { + return new Parcel(UUID.randomUUID(), UUID.randomUUID(), "name", "description", false, + UUID.randomUUID(), ParcelSize.SMALL, UUID.randomUUID(), UUID.randomUUID(), ParcelStatus.COLLECTED); + } + + @Test + void changeStatusRefusesCollectedParcelWithoutTouchingAnyCollaborator() { + // No collaborator is a usable mock/instance here on purpose: the COLLECTED guard must + // short-circuit before dereferencing any of them, so a regression that removes the guard + // and reaches a collaborator call would fail this test with an NPE instead of passing. + AdminParcelService service = new AdminParcelService(null, null, null, null, null, null); + + Parcel collected = collectedParcel(); + EditResult result = service.changeStatus(collected, ParcelStatus.SENT).join(); + + assertEquals(EditResult.Status.PARCEL_COLLECTED, result.status()); + } + @Test void capacityMatchesContentGuiUsableSlots() { assertEquals(9, AdminParcelService.capacity(ParcelSize.SMALL)); From 9f97f09ba5c0d91c7d49fd1971cdbb6e105bd502 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Sat, 4 Jul 2026 10:03:22 +0200 Subject: [PATCH 14/17] fix: contain post-commit failures in ParcelReturnService.execute (GH-69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The post-commit block after markReturned succeeded relied on each async step's own .exceptionally for safety, but a synchronous throw inside it (e.g. the scheduler rejecting work during plugin disable) would propagate to the outer exceptionally, which refunds the fee and gives the deposited items back even though the parcel is already a live SENT shipment whose content row holds those same items — duplicating them. Wrap the post-commit statements in a try/catch that only logs, so no synchronous exception past the commit point can ever reach the refund/give-back recovery path. Pre-commit behavior (refund + give-back + cannotReturn on a failed markReturned) is unchanged. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01UWGThsFyex5LiEyN63L3Wu --- .../returns/ParcelReturnService.java | 71 ++++++++++--------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java index cb9e82b4d..1b16ab9cd 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java @@ -182,40 +182,47 @@ private CompletableFuture execute(Player player, Parcel current, List { - LOGGER.warning("Failed to delete collected_parcels row for returned parcel " - + current.uuid() + ": " + throwable.getMessage()); - return false; - }); - - Duration delay = returned.priority() - ? this.config.settings.priorityParcelSendDuration - : this.config.settings.parcelSendDuration; - - // Schedule the send task before persisting the delivery: ParcelSendTask.decide treats - // a missing delivery row as "deliver when due", so the task is safe even if the - // delivery upsert below fails. - this.scheduler.runLaterAsync( - new ParcelSendTask(returned, this.parcelService, this.deliveryManager, this.scheduler), - delay); - - // Use update (upsert), not create: a stale delivery entry can still be cached for - // this UUID from the parcel's original trip (its cleanup delete is best-effort and - // re-cached on restart via cacheAll), and create() throws IllegalStateException on an - // existing cache entry. Throwing here must not happen post-commit. - this.deliveryManager.update(returned.uuid(), Instant.now().plus(delay)) - .exceptionally(throwable -> { - LOGGER.severe("Failed to persist delivery for returned parcel " + returned.uuid() - + " (send task scheduled in-memory; a restart before it fires may strand the parcel): " - + throwable.getMessage()); - return null; + // here would duplicate the items. The try/catch below makes that structural: even a + // synchronous throw (e.g. the scheduler rejecting work during plugin disable) is only + // logged, never allowed to reach the outer exceptionally. + try { + // Best-effort cleanup: a leftover row is ignored by the purge task because the + // parcel is no longer COLLECTED. + this.collectedParcelRepository.delete(current.uuid()).exceptionally(throwable -> { + LOGGER.warning("Failed to delete collected_parcels row for returned parcel " + + current.uuid() + ": " + throwable.getMessage()); + return false; }); - this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returned); + Duration delay = returned.priority() + ? this.config.settings.priorityParcelSendDuration + : this.config.settings.parcelSendDuration; + + // Schedule the send task before persisting the delivery: ParcelSendTask.decide treats + // a missing delivery row as "deliver when due", so the task is safe even if the + // delivery upsert below fails. + this.scheduler.runLaterAsync( + new ParcelSendTask(returned, this.parcelService, this.deliveryManager, this.scheduler), + delay); + + // Use update (upsert), not create: a stale delivery entry can still be cached for + // this UUID from the parcel's original trip (its cleanup delete is best-effort and + // re-cached on restart via cacheAll), and create() throws IllegalStateException on an + // existing cache entry. Throwing here must not happen post-commit. + this.deliveryManager.update(returned.uuid(), Instant.now().plus(delay)) + .exceptionally(throwable -> { + LOGGER.severe("Failed to persist delivery for returned parcel " + returned.uuid() + + " (send task scheduled in-memory; a restart before it fires may strand the parcel): " + + throwable.getMessage()); + return null; + }); + + this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returned); + } catch (Exception exception) { + LOGGER.severe("Post-commit step threw for returned parcel " + current.uuid() + + ": the return is already committed (parcel is SENT, content holds the deposited " + + "items) so no refund/give-back recovery was attempted: " + exception.getMessage()); + } return CompletableFuture.completedFuture(null); }) .exceptionally(throwable -> { From 6d86d468d89af8e895097ba6646e460d0cc60a9a Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:11:10 +0200 Subject: [PATCH 15/17] Add Graphify to CLAUDE.md --- CLAUDE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 0a7b1a572..071edc2d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,3 +59,13 @@ Each domain (`locker`, `parcel`, `content`, `delivery`, `itemstorage`, `user`, ` ### Testing Tests live in `src/test/java/`. Integration tests (e.g. `LockerRepositoryIntegrationTest`) extend `IntegrationTestSpec` and use Testcontainers (MySQL) to test repository implementations against a real database. `ParcelPageTest` is a unit test with no container dependency. + +## graphify + +This project has a knowledge graph at graphify-out/ with god nodes, community structure, and cross-file relationships. + +Rules: +- For codebase questions, first run `graphify query ""` when graphify-out/graph.json exists. Use `graphify path "" ""` for relationships and `graphify explain ""` for focused concepts. These return a scoped subgraph, usually much smaller than GRAPH_REPORT.md or raw grep output. +- If graphify-out/wiki/index.md exists, use it for broad navigation instead of raw source browsing. +- Read graphify-out/GRAPH_REPORT.md only for broad architecture review or when query/path/explain do not surface enough context. +- After modifying code, run `graphify update .` to keep the graph current (AST-only, no API cost). From fb79943cda9e00d23ae45e00ff8f7e50732e8512 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Sat, 4 Jul 2026 15:59:00 +0200 Subject: [PATCH 16/17] fix: address GH-233 follow-up cleanups from GH-69 review Correctness hardening: - Make AdminParcelService.changeStatus a conditional UPDATE (ParcelRepository.updateIfStatus, WHERE status = expected) so a stale admin-GUI snapshot can no longer resurrect a parcel that concurrently became COLLECTED. - Upsert (not insert-if-absent) the collected_parcels row on collect, so a re-collect after a failed best-effort delete overwrites the stale collectedAt instead of shortening the new return window. Cleanup / UX: - ReturnWindowPurgeTask now also deletes the delivery row for a purged parcel, closing the leak left by a failed delete at ParcelSendTask.deliver time. - Move the returnFeeWithdrawn notice to after markReturned commits, so a lost race (fee refunded) no longer leaves the player thinking the fee was kept. - Reword insufficientFunds to be fee-neutral; it fires for both send and return fees. - Drop the dead result == null check in ReturnGui.show (getReturnableParcels never completes with null). Tests / docs / plumbing: - Add name/lore relaxed-flag coverage to ReturnItemEquivalenceTest. - Document the transitive-equivalence assumption ParcelReturnValidator's grouping relies on. - Switch new-code exception logging to LOGGER.log(Level, msg, throwable) so stack traces aren't dropped. - Move returnWindow onto GuiManager (alongside the existing allowCollectingFromAnyLocker pattern) instead of threading it through LockerGui/SendingGui/ItemStorageGui as a pass-through constructor param. Co-Authored-By: Claude Sonnet 5 --- .../parcellockers/ParcelLockers.java | 8 ++-- .../implementation/MessageConfig.java | 2 +- .../parcellockers/gui/GuiManager.java | 11 +++++- .../implementation/locker/ItemStorageGui.java | 9 +---- .../gui/implementation/locker/LockerGui.java | 11 ++---- .../gui/implementation/locker/ReturnGui.java | 9 ++--- .../gui/implementation/locker/SendingGui.java | 12 ++---- .../parcel/repository/ParcelRepository.java | 9 +++++ .../repository/ParcelRepositoryOrmLite.java | 27 +++++++++++++ .../parcel/service/AdminParcelService.java | 7 +++- .../parcel/service/ParcelService.java | 7 ++++ .../parcel/service/ParcelServiceImpl.java | 14 +++++++ .../parcel/task/ParcelSendTask.java | 9 +++-- .../returns/ParcelReturnService.java | 35 ++++++++++------- .../returns/ParcelReturnValidator.java | 7 ++++ .../CollectedParcelRepositoryOrmLite.java | 7 +++- .../returns/task/ReturnWindowPurgeTask.java | 23 +++++++++-- .../ParcelRepositoryIntegrationTest.java | 16 ++++++++ .../returns/ReturnItemEquivalenceTest.java | 38 +++++++++++++++++++ 19 files changed, 200 insertions(+), 61 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java index 5b32696fc..184d3e1e2 100644 --- a/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java +++ b/src/main/java/com/eternalcode/parcellockers/ParcelLockers.java @@ -175,7 +175,7 @@ public void onEnable() { ); scheduler.timerAsync( - new ReturnWindowPurgeTask(parcelService, collectedParcelRepository, config), + new ReturnWindowPurgeTask(parcelService, collectedParcelRepository, deliveryManager, config), Duration.ofSeconds(30), Duration.ofMinutes(30) ); @@ -191,7 +191,8 @@ public void onEnable() { parcelContentManager, deliveryManager, config.settings.allowCollectingFromAnyLocker, - parcelReturnService + parcelReturnService, + config.settings.parcelReturnWindow ); MainGui mainGUI = new MainGui( @@ -206,8 +207,7 @@ public void onEnable() { scheduler, config.guiSettings, guiManager, - noticeService, - config.settings.parcelReturnWindow + noticeService ); AdminParcelService adminParcelService = new AdminParcelService( diff --git a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java index 06f9ed6bc..87cda1f19 100644 --- a/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java +++ b/src/main/java/com/eternalcode/parcellockers/configuration/implementation/MessageConfig.java @@ -118,7 +118,7 @@ public static class ParcelMessages extends OkaeriConfig { .sound(SoundEventKeys.ENTITY_ITEM_BREAK) .build(); public Notice insufficientFunds = Notice.builder() - .chat("&4✘ &cYou do not have enough funds to send this parcel! Required: &6${AMOUNT}&c.") + .chat("&4✘ &cYou do not have enough funds to cover this fee! Required: &6${AMOUNT}&c.") .sound(SoundEventKeys.ENTITY_VILLAGER_NO) .build(); public Notice feeWithdrawn = Notice.builder() diff --git a/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java b/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java index 0c560a8e2..54666dc98 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/GuiManager.java @@ -18,6 +18,7 @@ import com.eternalcode.parcellockers.shared.PageResult; import com.eternalcode.parcellockers.user.User; import com.eternalcode.parcellockers.user.UserManager; +import java.time.Duration; import java.util.List; import java.util.Objects; import java.util.Optional; @@ -38,6 +39,7 @@ public class GuiManager { private final DeliveryManager deliveryManager; private final boolean allowCollectingFromAnyLocker; private final ParcelReturnService parcelReturnService; + private final Duration returnWindow; public GuiManager( ParcelService parcelService, @@ -48,7 +50,8 @@ public GuiManager( ParcelContentManager parcelContentManager, DeliveryManager deliveryManager, boolean allowCollectingFromAnyLocker, - ParcelReturnService parcelReturnService + ParcelReturnService parcelReturnService, + Duration returnWindow ) { this.parcelService = parcelService; this.lockerManager = lockerManager; @@ -59,6 +62,12 @@ public GuiManager( this.deliveryManager = deliveryManager; this.allowCollectingFromAnyLocker = allowCollectingFromAnyLocker; this.parcelReturnService = parcelReturnService; + this.returnWindow = returnWindow; + } + + /** The configured return window, exposed so GUIs don't each need their own copy threaded in. */ + public Duration returnWindow() { + return this.returnWindow; } /** diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java index a7aef9406..0dc1d6eab 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ItemStorageGui.java @@ -11,7 +11,6 @@ import dev.triumphteam.gui.guis.Gui; import dev.triumphteam.gui.guis.GuiItem; import dev.triumphteam.gui.guis.StorageGui; -import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.stream.IntStream; @@ -27,7 +26,6 @@ public class ItemStorageGui { private final GuiManager guiManager; private final NoticeService noticeService; private final SendingGuiState state; - private final Duration returnWindow; public ItemStorageGui( Scheduler scheduler, @@ -35,8 +33,7 @@ public ItemStorageGui( MiniMessage miniMessage, GuiManager guiManager, NoticeService noticeService, - SendingGuiState state, - Duration returnWindow + SendingGuiState state ) { this.scheduler = scheduler; this.guiSettings = guiSettings; @@ -44,7 +41,6 @@ public ItemStorageGui( this.guiManager = guiManager; this.noticeService = noticeService; this.state = state; - this.returnWindow = returnWindow; } void show(Player player, ParcelSize size) { @@ -115,8 +111,7 @@ void show(Player player, ParcelSize size) { this.miniMessage, this.noticeService, this.guiManager, - this.state, - this.returnWindow + this.state ).show(player)) .exceptionally(throwable -> { // Persisting the staged items failed; hand them back so they are not lost. diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java index ff90417e3..c5338bc6f 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerGui.java @@ -9,7 +9,6 @@ import com.eternalcode.parcellockers.notification.NoticeService; import dev.triumphteam.gui.guis.Gui; import dev.triumphteam.gui.guis.GuiItem; -import java.time.Duration; import java.util.UUID; import net.kyori.adventure.text.Component; import net.kyori.adventure.text.minimessage.MiniMessage; @@ -22,19 +21,17 @@ public class LockerGui implements GuiView { private final GuiSettings guiSettings; private final GuiManager guiManager; private final NoticeService noticeService; - private final Duration returnWindow; public LockerGui( MiniMessage miniMessage, Scheduler scheduler, GuiSettings guiSettings, GuiManager guiManager, - NoticeService noticeService, Duration returnWindow + NoticeService noticeService ) { this.miniMessage = miniMessage; this.scheduler = scheduler; this.guiSettings = guiSettings; this.guiManager = guiManager; this.noticeService = noticeService; - this.returnWindow = returnWindow; } public void show(Player player, UUID entryLocker) { @@ -61,8 +58,7 @@ public void show(Player player, UUID entryLocker) { this.scheduler, this.guiManager, this.miniMessage, - this.noticeService, - this.returnWindow + this.noticeService ); gui.setItem(21, this.guiSettings.parcelLockerCollectItem.toGuiItem(event -> collectionGui.show(player))); @@ -73,8 +69,7 @@ public void show(Player player, UUID entryLocker) { this.miniMessage, this.noticeService, this.guiManager, - new SendingGuiState().entryLocker(entryLocker), - this.returnWindow + new SendingGuiState().entryLocker(entryLocker) ).show(player, entryLocker))); gui.open(player); diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java index cf250a5be..365bbac28 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReturnGui.java @@ -39,22 +39,19 @@ public class ReturnGui implements GuiView { private final GuiManager guiManager; private final MiniMessage miniMessage; private final NoticeService noticeService; - private final Duration returnWindow; public ReturnGui( GuiSettings guiSettings, Scheduler scheduler, GuiManager guiManager, MiniMessage miniMessage, - NoticeService noticeService, - Duration returnWindow + NoticeService noticeService ) { this.guiSettings = guiSettings; this.scheduler = scheduler; this.guiManager = guiManager; this.miniMessage = miniMessage; this.noticeService = noticeService; - this.returnWindow = returnWindow; } @Override @@ -76,7 +73,7 @@ public void show(Player player, Page page) { this.setupStaticItems(player, gui); this.guiManager.getReturnableParcels(player.getUniqueId(), page).thenAccept(result -> { - if (result == null || result.items().isEmpty()) { + if (result.items().isEmpty()) { gui.setItem(22, this.guiSettings.noReturnableParcelsItem.toGuiItem()); this.scheduler.run(() -> gui.open(player)); return; @@ -131,7 +128,7 @@ private CompletableFuture> createParcelItemAsync( CompletableFuture windowLineFuture = this.guiManager.getCollectedInfo(parcel.uuid()) .thenApply(optional -> optional .map(collected -> { - Duration remaining = Duration.between(Instant.now(), collected.collectedAt().plus(this.returnWindow)); + Duration remaining = Duration.between(Instant.now(), collected.collectedAt().plus(this.guiManager.returnWindow())); return remaining.isNegative() || remaining.isZero() ? this.guiSettings.returnWindowExpiredLine : this.guiSettings.returnWindowRemainingLine.replace("{DURATION}", DurationUtil.format(remaining)); diff --git a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java index dbb88960f..31b3d9f05 100644 --- a/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java +++ b/src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/SendingGui.java @@ -21,7 +21,6 @@ import io.papermc.paper.registry.data.dialog.action.DialogAction; import io.papermc.paper.registry.data.dialog.input.DialogInput; import io.papermc.paper.registry.data.dialog.type.DialogType; -import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -54,7 +53,6 @@ public class SendingGui implements GuiView { private final GuiManager guiManager; private final SendingGuiState state; - private final Duration returnWindow; private Gui gui; @@ -64,8 +62,7 @@ public SendingGui( MiniMessage miniMessage, NoticeService noticeService, GuiManager guiManager, - SendingGuiState state, - Duration returnWindow + SendingGuiState state ) { this.scheduler = scheduler; this.guiSettings = guiSettings; @@ -73,7 +70,6 @@ public SendingGui( this.noticeService = noticeService; this.guiManager = guiManager; this.state = state; - this.returnWindow = returnWindow; } public void show(Player player, UUID entryLocker) { @@ -196,8 +192,7 @@ public void show(Player player) { this.miniMessage, this.guiManager, this.noticeService, - this.state, - this.returnWindow + this.state ); this.guiManager.getItemStorage(player.getUniqueId()).thenAccept(result -> { int slotsSize = result.items().size(); @@ -256,8 +251,7 @@ public void show(Player player) { this.scheduler, this.guiSettings, this.guiManager, - this.noticeService, - this.returnWindow + this.noticeService ).show(player, this.state.entryLocker())); ConfigItem smallButton = this.guiSettings.smallParcelSizeItem; diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java index 1719c7688..f453a981b 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java @@ -1,6 +1,7 @@ package com.eternalcode.parcellockers.parcel.repository; import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.parcel.ParcelStatus; import com.eternalcode.parcellockers.shared.Page; import com.eternalcode.parcellockers.shared.PageResult; import java.util.List; @@ -15,6 +16,14 @@ public interface ParcelRepository { CompletableFuture update(Parcel parcel); + /** + * Conditionally persists every mutable field of {@code updated}, but only if the row's + * current status still equals {@code expectedStatus}. Returns false when the row moved out + * from under the caller (e.g. a concurrent collect), so a stale in-memory snapshot cannot + * clobber it. + */ + CompletableFuture updateIfStatus(Parcel updated, ParcelStatus expectedStatus); + CompletableFuture> findAll(); CompletableFuture> findPage(Page page); diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java index adec6b8c4..eb4b2352b 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java @@ -22,6 +22,10 @@ public class ParcelRepositoryOrmLite extends AbstractRepositoryOrmLite implement private static final String DESTINATION_LOCKER_COLUMN = "destination_locker"; private static final String ENTRY_LOCKER_COLUMN = "entry_locker"; private static final String STATUS_COLUMN = "status"; + private static final String NAME_COLUMN = "name"; + private static final String DESCRIPTION_COLUMN = "description"; + private static final String PRIORITY_COLUMN = "priority"; + private static final String SIZE_COLUMN = "size"; public ParcelRepositoryOrmLite(DatabaseManager databaseManager, Scheduler scheduler) { super(databaseManager, scheduler); @@ -40,6 +44,29 @@ public CompletableFuture update(Parcel parcel) { return this.upsert(ParcelTable.class, ParcelTable.from(parcel)).thenApply(dao -> null); } + @Override + public CompletableFuture updateIfStatus(Parcel updated, ParcelStatus expectedStatus) { + Objects.requireNonNull(updated, "Parcel cannot be null"); + Objects.requireNonNull(expectedStatus, "Expected status cannot be null"); + return this.action(ParcelTable.class, dao -> { + UpdateBuilder builder = dao.updateBuilder(); + builder.updateColumnValue(SENDER_COLUMN, updated.sender()); + builder.updateColumnValue(NAME_COLUMN, updated.name()); + builder.updateColumnValue(DESCRIPTION_COLUMN, updated.description()); + builder.updateColumnValue(PRIORITY_COLUMN, updated.priority()); + builder.updateColumnValue(RECEIVER_COLUMN, updated.receiver()); + builder.updateColumnValue(SIZE_COLUMN, updated.size()); + builder.updateColumnValue(ENTRY_LOCKER_COLUMN, updated.entryLocker()); + builder.updateColumnValue(DESTINATION_LOCKER_COLUMN, updated.destinationLocker()); + builder.updateColumnValue(STATUS_COLUMN, updated.status()); + builder.where() + .eq(UUID_COLUMN, updated.uuid()) + .and() + .eq(STATUS_COLUMN, expectedStatus); + return builder.update() > 0; + }); + } + @Override public CompletableFuture> findById(UUID uuid) { Objects.requireNonNull(uuid, "UUID cannot be null"); diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java index fdb3d1539..01547f50b 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/AdminParcelService.java @@ -73,7 +73,12 @@ public CompletableFuture changeStatus(Parcel parcel, ParcelStatus st return CompletableFuture.completedFuture(EditResult.of(EditResult.Status.PARCEL_COLLECTED)); } Parcel updated = withStatus(parcel, status); - return this.parcelService.update(updated).thenCompose(ignored -> { + // Conditional on the snapshot's status: a concurrent collect between the GUI opening and + // this edit landing must not resurrect a COLLECTED parcel back to SENT/DELIVERED. + return this.parcelService.updateIfStatus(updated, parcel.status()).thenCompose(applied -> { + if (!Boolean.TRUE.equals(applied)) { + return CompletableFuture.completedFuture(EditResult.of(EditResult.Status.PARCEL_COLLECTED)); + } if (status == parcel.status()) { return CompletableFuture.completedFuture(EditResult.ok()); } diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java index 364afc21f..581f2cdc4 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelService.java @@ -2,6 +2,7 @@ import com.eternalcode.parcellockers.notification.NoticeService; import com.eternalcode.parcellockers.parcel.Parcel; +import com.eternalcode.parcellockers.parcel.ParcelStatus; import com.eternalcode.parcellockers.shared.Page; import com.eternalcode.parcellockers.shared.PageResult; import java.util.List; @@ -24,6 +25,12 @@ public interface ParcelService { CompletableFuture update(Parcel parcel); + /** + * Conditionally applies {@code updated}, refusing when the parcel's current status no + * longer matches {@code expectedStatus} (e.g. a concurrent collect raced an admin edit). + */ + CompletableFuture updateIfStatus(Parcel updated, ParcelStatus expectedStatus); + CompletableFuture collect(Player player, Parcel parcel); CompletableFuture delete(CommandSender sender, Parcel parcel); diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java index 4259aea10..a3bbbe472 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/service/ParcelServiceImpl.java @@ -181,6 +181,20 @@ public CompletableFuture update(Parcel updated) { return this.parcelRepository.update(updated); } + @Override + public CompletableFuture updateIfStatus(Parcel updated, ParcelStatus expectedStatus) { + Objects.requireNonNull(updated, "Updated parcel cannot be null"); + Objects.requireNonNull(expectedStatus, "Expected status cannot be null"); + return this.parcelRepository.updateIfStatus(updated, expectedStatus).thenApply(applied -> { + if (applied) { + this.parcelsByUuid.put(updated.uuid(), updated); + } else { + this.parcelsByUuid.invalidate(updated.uuid()); + } + return applied; + }); + } + @Override public CompletableFuture delete(CommandSender sender, Parcel parcel) { Objects.requireNonNull(sender, "Sender cannot be null"); diff --git a/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java b/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java index 8c93901ce..d06f9d757 100644 --- a/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java +++ b/src/main/java/com/eternalcode/parcellockers/parcel/task/ParcelSendTask.java @@ -11,6 +11,7 @@ import java.time.Instant; import java.util.Optional; import java.util.UUID; +import java.util.logging.Level; import java.util.logging.Logger; import org.bukkit.Bukkit; import org.bukkit.scheduler.BukkitRunnable; @@ -54,7 +55,7 @@ public void run() { // Parcel gone or already delivered: clean up any stray delivery row. optionalDelivery.ifPresent(delivery -> this.deliveryManager.delete(this.parcelId).exceptionally(throwable -> { - LOGGER.severe("Failed to delete stray delivery for " + this.parcelId + ": " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to delete stray delivery for " + this.parcelId, throwable); return false; })); case RESCHEDULE -> { @@ -67,7 +68,7 @@ public void run() { case DELIVER -> this.deliver(optionalParcel.get()); } })).exceptionally(throwable -> { - LOGGER.severe("ParcelSendTask failed for " + this.parcelId + ": " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "ParcelSendTask failed for " + this.parcelId, throwable); return null; }); } @@ -87,8 +88,8 @@ private void deliver(Parcel current) { this.parcelService.update(delivered) .thenCompose(ignored -> this.deliveryManager.delete(delivered.uuid())) .exceptionally(throwable -> { - LOGGER.severe("Failed to deliver parcel " + delivered.uuid() - + " (delivery left for retry): " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to deliver parcel " + delivered.uuid() + + " (delivery left for retry)", throwable); return null; }); } diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java index 1b16ab9cd..052af49e6 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java @@ -23,6 +23,7 @@ import java.util.Optional; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.logging.Level; import java.util.logging.Logger; import net.milkbowl.vault.economy.Economy; import org.bukkit.Server; @@ -130,7 +131,7 @@ public CompletableFuture returnParcel(Player player, Parcel parcel, List { - LOGGER.severe("Failed to return parcel " + parcel.uuid() + " for " + player.getName() + ": " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to return parcel " + parcel.uuid() + " for " + player.getName(), throwable); this.giveBack(player, deposited); this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.cannotReturn); return null; @@ -154,11 +155,9 @@ private CompletableFuture execute(Player player, Parcel current, List messages.parcel.returnFeeWithdrawn) - .player(player.getUniqueId()) - .placeholder(PLACEHOLDER_AMOUNT, formattedFee) - .send(); + // The fee-withdrawn notice is sent only after the status flip commits (below): if + // markReturned loses the race and the fee is refunded, the player must not have + // already been told the fee was withdrawn. } } double refundableFee = chargedFee; @@ -186,11 +185,19 @@ private CompletableFuture execute(Player player, Parcel current, List 0) { + this.noticeService.create() + .notice(messages -> messages.parcel.returnFeeWithdrawn) + .player(player.getUniqueId()) + .placeholder(PLACEHOLDER_AMOUNT, String.format("%.2f", refundableFee)) + .send(); + } + // Best-effort cleanup: a leftover row is ignored by the purge task because the // parcel is no longer COLLECTED. this.collectedParcelRepository.delete(current.uuid()).exceptionally(throwable -> { - LOGGER.warning("Failed to delete collected_parcels row for returned parcel " - + current.uuid() + ": " + throwable.getMessage()); + LOGGER.log(Level.WARNING, "Failed to delete collected_parcels row for returned parcel " + + current.uuid(), throwable); return false; }); @@ -211,17 +218,17 @@ private CompletableFuture execute(Player player, Parcel current, List { - LOGGER.severe("Failed to persist delivery for returned parcel " + returned.uuid() - + " (send task scheduled in-memory; a restart before it fires may strand the parcel): " - + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to persist delivery for returned parcel " + returned.uuid() + + " (send task scheduled in-memory; a restart before it fires may strand the parcel)", + throwable); return null; }); this.noticeService.player(player.getUniqueId(), messages -> messages.parcel.returned); } catch (Exception exception) { - LOGGER.severe("Post-commit step threw for returned parcel " + current.uuid() + LOGGER.log(Level.SEVERE, "Post-commit step threw for returned parcel " + current.uuid() + ": the return is already committed (parcel is SENT, content holds the deposited " - + "items) so no refund/give-back recovery was attempted: " + exception.getMessage()); + + "items) so no refund/give-back recovery was attempted", exception); } return CompletableFuture.completedFuture(null); }) @@ -229,7 +236,7 @@ private CompletableFuture execute(Player player, Parcel current, List messages.parcel.cannotReturn); - LOGGER.severe("Failed to execute return of parcel " + current.uuid() + ": " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to execute return of parcel " + current.uuid(), throwable); return null; }); } diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java index 89a63abe1..60bec55af 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnValidator.java @@ -9,6 +9,13 @@ * Validates that deposited items are, as a multiset, exactly the original parcel content: * every deposited stack must be equivalent to some original item, and the total amount per * equivalence group must match. Stack splitting or merging is irrelevant. + * + *

Grouping ({@link #indexOf}) assumes {@code equivalence} is a true equivalence relation + * (reflexive, symmetric, transitive): each expected item is bucketed under the first + * sample it matches, and later items are folded into that same bucket without being re-compared + * to each other. If the predicate is not transitive (e.g. a "close enough" relaxed-attribute + * check where A~B and B~C but not A~C), distinct original items can collapse into one bucket + * and the amount check may pass for a return that should not have matched. */ public class ParcelReturnValidator { diff --git a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java index e27986fad..3e9ffe6c5 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/repository/CollectedParcelRepositoryOrmLite.java @@ -21,8 +21,11 @@ public CollectedParcelRepositoryOrmLite(DatabaseManager databaseManager, Schedul @Override public CompletableFuture save(CollectedParcel collectedParcel) { Objects.requireNonNull(collectedParcel, "CollectedParcel cannot be null"); - return this.insertIfAbsent(CollectedParcelTable.class, CollectedParcelTable.from(collectedParcel)) - .thenApply(dao -> null); + // Upsert, not insert-if-absent: a re-collect after a failed best-effort delete of the + // previous row must overwrite the stale collectedAt, not silently keep it (which would + // shorten the new return window by however long the parcel spent on its second trip). + return this.upsert(CollectedParcelTable.class, CollectedParcelTable.from(collectedParcel)) + .thenApply(status -> null); } @Override diff --git a/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java b/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java index 56281fe7c..46d98e3f4 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/task/ReturnWindowPurgeTask.java @@ -1,6 +1,7 @@ package com.eternalcode.parcellockers.returns.task; import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import com.eternalcode.parcellockers.delivery.DeliveryManager; import com.eternalcode.parcellockers.parcel.ParcelStatus; import com.eternalcode.parcellockers.parcel.service.ParcelService; import com.eternalcode.parcellockers.returns.CollectedParcel; @@ -8,6 +9,7 @@ import java.time.Duration; import java.time.Instant; import java.util.concurrent.CompletableFuture; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -23,15 +25,18 @@ public class ReturnWindowPurgeTask implements Runnable { private final ParcelService parcelService; private final CollectedParcelRepository collectedParcelRepository; + private final DeliveryManager deliveryManager; private final PluginConfig config; public ReturnWindowPurgeTask( ParcelService parcelService, CollectedParcelRepository collectedParcelRepository, + DeliveryManager deliveryManager, PluginConfig config ) { this.parcelService = parcelService; this.collectedParcelRepository = collectedParcelRepository; + this.deliveryManager = deliveryManager; this.config = config; } @@ -42,7 +47,7 @@ public void run() { this.collectedParcelRepository.findExpired(cutoff) .thenAccept(expired -> expired.forEach(this::purge)) .exceptionally(throwable -> { - LOGGER.severe("Failed to query expired collected parcels: " + throwable.getMessage()); + LOGGER.log(Level.SEVERE, "Failed to query expired collected parcels", throwable); return null; }); } @@ -56,14 +61,24 @@ private void purge(CollectedParcel collected) { return this.parcelService.delete(collected.parcel()) .thenCompose(deleted -> Boolean.TRUE.equals(deleted) ? this.collectedParcelRepository.delete(collected.parcel()) - : CompletableFuture.completedFuture(false)); + : CompletableFuture.completedFuture(false)) + .thenCompose(unused -> { + // Best-effort: clears a stray delivery row left behind by a failed + // delete at ParcelSendTask.deliver time; inert but leaks otherwise. + return this.deliveryManager.delete(collected.parcel()) + .exceptionally(throwable -> { + LOGGER.log(Level.WARNING, "Failed to delete stray delivery for purged parcel " + + collected.parcel(), throwable); + return false; + }); + }); } // Stray row: the parcel is gone or is a live shipment again — drop only the row. return this.collectedParcelRepository.delete(collected.parcel()); }) .exceptionally(throwable -> { - LOGGER.warning("Failed to purge expired collected parcel " + collected.parcel() - + " (will retry next run): " + throwable.getMessage()); + LOGGER.log(Level.WARNING, "Failed to purge expired collected parcel " + collected.parcel() + + " (will retry next run)", throwable); return false; }); } diff --git a/src/test/java/com/eternalcode/parcellockers/database/ParcelRepositoryIntegrationTest.java b/src/test/java/com/eternalcode/parcellockers/database/ParcelRepositoryIntegrationTest.java index 4ee9ea80b..4892af966 100644 --- a/src/test/java/com/eternalcode/parcellockers/database/ParcelRepositoryIntegrationTest.java +++ b/src/test/java/com/eternalcode/parcellockers/database/ParcelRepositoryIntegrationTest.java @@ -1,6 +1,7 @@ package com.eternalcode.parcellockers.database; 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 com.eternalcode.parcellockers.TestScheduler; @@ -64,6 +65,21 @@ void test() throws SQLException { assertTrue(parcel.isPresent()); assertEquals(uuid, parcel.get().uuid()); + // updateIfStatus refuses when the expected status no longer matches the row (e.g. a + // concurrent status change won the race) and leaves the row untouched. + Parcel staleEdit = new Parcel(uuid, sender, "renamed", "description", true, receiver, + ParcelSize.SMALL, entryLocker, destinationLocker, ParcelStatus.DELIVERED); + boolean staleApplied = this.await(parcelRepository.updateIfStatus(staleEdit, ParcelStatus.DELIVERED)); + assertFalse(staleApplied); + assertEquals("name", this.await(parcelRepository.findById(uuid)).orElseThrow().name()); + + // updateIfStatus applies when the expected status still matches. + Parcel freshEdit = new Parcel(uuid, sender, "renamed", "description", true, receiver, + ParcelSize.SMALL, entryLocker, destinationLocker, ParcelStatus.SENT); + boolean freshApplied = this.await(parcelRepository.updateIfStatus(freshEdit, ParcelStatus.SENT)); + assertTrue(freshApplied); + assertEquals("renamed", this.await(parcelRepository.findById(uuid)).orElseThrow().name()); + List byReceiver = this.await(parcelRepository.findByReceiver(receiver)); assertEquals(1, byReceiver.size()); assertEquals(uuid, byReceiver.getFirst().uuid()); diff --git a/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java b/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java index fcb2529fd..401d810e9 100644 --- a/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java +++ b/src/test/java/com/eternalcode/parcellockers/returns/ReturnItemEquivalenceTest.java @@ -7,7 +7,9 @@ import static org.mockito.Mockito.withSettings; import com.eternalcode.parcellockers.configuration.implementation.PluginConfig; +import java.util.List; import java.util.Map; +import net.kyori.adventure.text.Component; import org.bukkit.Material; import org.bukkit.enchantments.Enchantment; import org.bukkit.inventory.ItemStack; @@ -101,6 +103,42 @@ private static Map enchantsOf(String key, int level) { return (Map) Map.of(key, level); } + @Test + void nameMismatchFailsWhenChecked() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.DIAMOND_SWORD); + + ItemMeta expectedMeta = mock(ItemMeta.class); + ItemMeta actualMeta = mock(ItemMeta.class); + when(expectedMeta.displayName()).thenReturn(Component.text("Old Sword")); + when(actualMeta.displayName()).thenReturn(Component.text("New Sword")); + when(expectedMeta.getEnchants()).thenReturn(Map.of()); + when(actualMeta.getEnchants()).thenReturn(Map.of()); + when(expected.getItemMeta()).thenReturn(expectedMeta); + when(actual.getItemMeta()).thenReturn(actualMeta); + + assertFalse(new ReturnItemEquivalence(checks(false, true, false, false, false)).test(expected, actual)); + assertTrue(new ReturnItemEquivalence(checks(false, false, false, false, false)).test(expected, actual)); + } + + @Test + void loreMismatchFailsWhenChecked() { + ItemStack expected = item(Material.DIAMOND_SWORD); + ItemStack actual = item(Material.DIAMOND_SWORD); + + ItemMeta expectedMeta = mock(ItemMeta.class); + ItemMeta actualMeta = mock(ItemMeta.class); + when(expectedMeta.lore()).thenReturn(List.of(Component.text("Enchanted"))); + when(actualMeta.lore()).thenReturn(List.of()); + when(expectedMeta.getEnchants()).thenReturn(Map.of()); + when(actualMeta.getEnchants()).thenReturn(Map.of()); + when(expected.getItemMeta()).thenReturn(expectedMeta); + when(actual.getItemMeta()).thenReturn(actualMeta); + + assertFalse(new ReturnItemEquivalence(checks(false, false, false, true, false)).test(expected, actual)); + assertTrue(new ReturnItemEquivalence(checks(false, false, false, false, false)).test(expected, actual)); + } + @Test void missingMetaOnBothSidesMatches() { ItemStack expected = item(Material.COBBLESTONE); From 01316805c7cb60a83b766a7d16d0f130ad2cc3f7 Mon Sep 17 00:00:00 2001 From: Jakubk15 <77227023+Jakubk15@users.noreply.github.com> Date: Sat, 4 Jul 2026 16:23:39 +0200 Subject: [PATCH 17/17] fix: run Vault economy calls on the primary thread in ParcelReturnService withdrawPlayer/depositPlayer were called from inside async CompletableFuture continuations chained off repository lookups, so they executed on a DB-executor thread rather than the main thread Vault providers expect. Dispatch both through Scheduler.complete/run to hop back onto the primary thread before touching Economy. Co-Authored-By: Claude Sonnet 5 --- .../returns/ParcelReturnService.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java index 052af49e6..d22bad108 100644 --- a/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java +++ b/src/main/java/com/eternalcode/parcellockers/returns/ParcelReturnService.java @@ -139,29 +139,33 @@ public CompletableFuture returnParcel(Player player, Parcel parcel, List execute(Player player, Parcel current, List deposited) { - double chargedFee = 0; - if (!player.hasPermission(PARCEL_FEE_BYPASS_PERMISSION)) { - double fee = this.returnFeeFor(current.size()); - if (fee > 0) { - boolean success = this.economy.withdrawPlayer(player, fee).transactionSuccess(); - String formattedFee = String.format("%.2f", fee); - if (!success) { + double fee = player.hasPermission(PARCEL_FEE_BYPASS_PERMISSION) ? 0 : this.returnFeeFor(current.size()); + if (fee <= 0) { + return this.proceedWithReturn(player, current, deposited, 0); + } + + // Vault economy providers expect withdrawPlayer to run on the primary thread; by this + // point the call chain has already hopped onto a repository/DB executor thread, so the + // withdrawal must be scheduled back onto the main thread rather than called in place. + return this.scheduler.complete(() -> this.economy.withdrawPlayer(player, fee).transactionSuccess()) + .thenCompose(success -> { + if (!Boolean.TRUE.equals(success)) { this.noticeService.create() .notice(messages -> messages.parcel.insufficientFunds) .player(player.getUniqueId()) - .placeholder(PLACEHOLDER_AMOUNT, formattedFee) + .placeholder(PLACEHOLDER_AMOUNT, String.format("%.2f", fee)) .send(); this.giveBack(player, deposited); - return CompletableFuture.completedFuture(null); + return CompletableFuture.completedFuture(null); } - chargedFee = fee; // The fee-withdrawn notice is sent only after the status flip commits (below): if // markReturned loses the race and the fee is refunded, the player must not have // already been told the fee was withdrawn. - } - } - double refundableFee = chargedFee; + return this.proceedWithReturn(player, current, deposited, fee); + }); + } + private CompletableFuture proceedWithReturn(Player player, Parcel current, List deposited, double refundableFee) { Parcel returned = new Parcel(current.uuid(), current.receiver(), current.name(), current.description(), current.priority(), current.sender(), current.size(), current.destinationLocker(), current.entryLocker(), ParcelStatus.SENT); @@ -251,7 +255,9 @@ private double returnFeeFor(ParcelSize size) { private void refund(Player player, double fee) { if (fee > 0) { - this.economy.depositPlayer(player, fee); + // Same primary-thread requirement as the withdrawal in execute(): this runs from + // async continuations, so the deposit must be dispatched back onto the main thread. + this.scheduler.run(() -> this.economy.depositPlayer(player, fee)); } }