From a8bbe3a951c109561b57e9c8d7bc3be0815f19c4 Mon Sep 17 00:00:00 2001 From: Sebastian Frick Date: Wed, 25 Feb 2026 08:51:25 +0100 Subject: [PATCH] =?UTF-8?q?fix(inventory):=20Instant.MIN/MAX,=20performed?= =?UTF-8?q?=5Fat-Index,=20batchReference-Naming=20und=20-Validierung,=20La?= =?UTF-8?q?sttest-Zeitr=C3=A4ume?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Instant.MIN/MAX durch separate Repository-Methoden ersetzt (findAllByPerformedAtAfter/Before) - DB-Index auf performed_at für Zeitraum-Abfragen - Naming-Konsistenz: findAllByBatchId → findAllByBatchReference im Domain-Repository - batchReference-Validierung (Blank-Check) mit InvalidBatchReference-Error - Lasttest: variierende Zeiträume (7–90 Tage) statt statischem 10-Jahres-Fenster --- .../inventory/ListStockMovements.java | 16 ++++++-- .../inventory/StockMovementRepository.java | 6 ++- .../JpaStockMovementRepository.java | 32 +++++++++++++-- .../StockMovementJpaRepository.java | 4 ++ ...-performed-at-index-to-stock-movements.xml | 14 +++++++ .../db/changelog/db.changelog-master.xml | 1 + .../inventory/ListStockMovementsTest.java | 41 ++++++++++++------- ...tockMovementControllerIntegrationTest.java | 10 +++++ .../loadtest/scenario/InventoryScenario.java | 15 ++++++- 9 files changed, 114 insertions(+), 25 deletions(-) create mode 100644 backend/src/main/resources/db/changelog/changes/031-add-performed-at-index-to-stock-movements.xml diff --git a/backend/src/main/java/de/effigenix/application/inventory/ListStockMovements.java b/backend/src/main/java/de/effigenix/application/inventory/ListStockMovements.java index b441a65..acf5d88 100644 --- a/backend/src/main/java/de/effigenix/application/inventory/ListStockMovements.java +++ b/backend/src/main/java/de/effigenix/application/inventory/ListStockMovements.java @@ -62,7 +62,11 @@ public class ListStockMovements { } if (batchReference != null) { - return mapResult(stockMovementRepository.findAllByBatchId(batchReference)); + if (batchReference.isBlank()) { + return Result.failure(new StockMovementError.InvalidBatchReference( + "Batch reference must not be blank")); + } + return mapResult(stockMovementRepository.findAllByBatchReference(batchReference)); } if (movementType != null) { @@ -81,9 +85,13 @@ public class ListStockMovements { return Result.failure(new StockMovementError.InvalidDateRange( "'from' must not be after 'to'")); } - Instant effectiveFrom = from != null ? from : Instant.MIN; - Instant effectiveTo = to != null ? to : Instant.MAX; - return mapResult(stockMovementRepository.findAllByPerformedAtBetween(effectiveFrom, effectiveTo)); + if (from != null && to != null) { + return mapResult(stockMovementRepository.findAllByPerformedAtBetween(from, to)); + } + if (from != null) { + return mapResult(stockMovementRepository.findAllByPerformedAtAfter(from)); + } + return mapResult(stockMovementRepository.findAllByPerformedAtBefore(to)); } return mapResult(stockMovementRepository.findAll()); diff --git a/backend/src/main/java/de/effigenix/domain/inventory/StockMovementRepository.java b/backend/src/main/java/de/effigenix/domain/inventory/StockMovementRepository.java index 8f5f48f..df67b70 100644 --- a/backend/src/main/java/de/effigenix/domain/inventory/StockMovementRepository.java +++ b/backend/src/main/java/de/effigenix/domain/inventory/StockMovementRepository.java @@ -20,9 +20,13 @@ public interface StockMovementRepository { Result> findAllByMovementType(MovementType movementType); - Result> findAllByBatchId(String batchId); + Result> findAllByBatchReference(String batchReference); Result> findAllByPerformedAtBetween(Instant from, Instant to); + Result> findAllByPerformedAtAfter(Instant from); + + Result> findAllByPerformedAtBefore(Instant to); + Result save(StockMovement stockMovement); } diff --git a/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/JpaStockMovementRepository.java b/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/JpaStockMovementRepository.java index aaa04ea..0865a63 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/JpaStockMovementRepository.java +++ b/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/JpaStockMovementRepository.java @@ -95,14 +95,14 @@ public class JpaStockMovementRepository implements StockMovementRepository { } @Override - public Result> findAllByBatchId(String batchId) { + public Result> findAllByBatchReference(String batchReference) { try { - List result = jpaRepository.findAllByBatchId(batchId).stream() + List result = jpaRepository.findAllByBatchId(batchReference).stream() .map(mapper::toDomain) .toList(); return Result.success(result); } catch (Exception e) { - logger.warn("Database error in findAllByBatchId", e); + logger.warn("Database error in findAllByBatchReference", e); return Result.failure(new RepositoryError.DatabaseError(e.getMessage())); } } @@ -120,6 +120,32 @@ public class JpaStockMovementRepository implements StockMovementRepository { } } + @Override + public Result> findAllByPerformedAtAfter(Instant from) { + try { + List result = jpaRepository.findAllByPerformedAtGreaterThanEqual(from).stream() + .map(mapper::toDomain) + .toList(); + return Result.success(result); + } catch (Exception e) { + logger.warn("Database error in findAllByPerformedAtAfter", e); + return Result.failure(new RepositoryError.DatabaseError(e.getMessage())); + } + } + + @Override + public Result> findAllByPerformedAtBefore(Instant to) { + try { + List result = jpaRepository.findAllByPerformedAtLessThanEqual(to).stream() + .map(mapper::toDomain) + .toList(); + return Result.success(result); + } catch (Exception e) { + logger.warn("Database error in findAllByPerformedAtBefore", e); + return Result.failure(new RepositoryError.DatabaseError(e.getMessage())); + } + } + @Override @Transactional public Result save(StockMovement stockMovement) { diff --git a/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/StockMovementJpaRepository.java b/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/StockMovementJpaRepository.java index 1293544..757fe51 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/StockMovementJpaRepository.java +++ b/backend/src/main/java/de/effigenix/infrastructure/inventory/persistence/repository/StockMovementJpaRepository.java @@ -17,4 +17,8 @@ public interface StockMovementJpaRepository extends JpaRepository findAllByBatchId(String batchId); List findAllByPerformedAtBetween(Instant from, Instant to); + + List findAllByPerformedAtGreaterThanEqual(Instant from); + + List findAllByPerformedAtLessThanEqual(Instant to); } diff --git a/backend/src/main/resources/db/changelog/changes/031-add-performed-at-index-to-stock-movements.xml b/backend/src/main/resources/db/changelog/changes/031-add-performed-at-index-to-stock-movements.xml new file mode 100644 index 0000000..873b799 --- /dev/null +++ b/backend/src/main/resources/db/changelog/changes/031-add-performed-at-index-to-stock-movements.xml @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/backend/src/main/resources/db/changelog/db.changelog-master.xml b/backend/src/main/resources/db/changelog/db.changelog-master.xml index ad9fc29..06ae537 100644 --- a/backend/src/main/resources/db/changelog/db.changelog-master.xml +++ b/backend/src/main/resources/db/changelog/db.changelog-master.xml @@ -35,5 +35,6 @@ + diff --git a/backend/src/test/java/de/effigenix/application/inventory/ListStockMovementsTest.java b/backend/src/test/java/de/effigenix/application/inventory/ListStockMovementsTest.java index 9a0f85c..9e2d19b 100644 --- a/backend/src/test/java/de/effigenix/application/inventory/ListStockMovementsTest.java +++ b/backend/src/test/java/de/effigenix/application/inventory/ListStockMovementsTest.java @@ -22,7 +22,9 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @DisplayName("ListStockMovements Use Case") @@ -155,20 +157,20 @@ class ListStockMovementsTest { @Test @DisplayName("should filter by batchReference") void shouldFilterByBatchReference() { - when(repository.findAllByBatchId("CHARGE-001")) + when(repository.findAllByBatchReference("CHARGE-001")) .thenReturn(Result.success(List.of(sampleMovement))); var result = useCase.execute(null, null, null, "CHARGE-001", null, null, actor); assertThat(result.isSuccess()).isTrue(); assertThat(result.unsafeGetValue()).hasSize(1); - verify(repository).findAllByBatchId("CHARGE-001"); + verify(repository).findAllByBatchReference("CHARGE-001"); } @Test @DisplayName("should return empty list when no movements for batch") void shouldReturnEmptyForUnknownBatch() { - when(repository.findAllByBatchId("UNKNOWN")) + when(repository.findAllByBatchReference("UNKNOWN")) .thenReturn(Result.success(List.of())); var result = useCase.execute(null, null, null, "UNKNOWN", null, null, actor); @@ -177,10 +179,19 @@ class ListStockMovementsTest { assertThat(result.unsafeGetValue()).isEmpty(); } + @Test + @DisplayName("should fail with InvalidBatchReference when blank") + void shouldFailWhenBatchReferenceBlank() { + var result = useCase.execute(null, null, null, " ", null, null, actor); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(StockMovementError.InvalidBatchReference.class); + } + @Test @DisplayName("should fail when repository fails for batchReference") void shouldFailWhenRepositoryFailsForBatch() { - when(repository.findAllByBatchId("CHARGE-001")) + when(repository.findAllByBatchReference("CHARGE-001")) .thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost"))); var result = useCase.execute(null, null, null, "CHARGE-001", null, null, actor); @@ -240,25 +251,25 @@ class ListStockMovementsTest { @Test @DisplayName("should filter with only from (open-ended)") void shouldFilterByFromOnly() { - when(repository.findAllByPerformedAtBetween(eq(from), any())) + when(repository.findAllByPerformedAtAfter(from)) .thenReturn(Result.success(List.of(sampleMovement))); var result = useCase.execute(null, null, null, null, from, null, actor); assertThat(result.isSuccess()).isTrue(); - verify(repository).findAllByPerformedAtBetween(eq(from), eq(Instant.MAX)); + verify(repository).findAllByPerformedAtAfter(from); } @Test @DisplayName("should filter with only to (open-ended)") void shouldFilterByToOnly() { - when(repository.findAllByPerformedAtBetween(any(), eq(to))) + when(repository.findAllByPerformedAtBefore(to)) .thenReturn(Result.success(List.of(sampleMovement))); var result = useCase.execute(null, null, null, null, null, to, actor); assertThat(result.isSuccess()).isTrue(); - verify(repository).findAllByPerformedAtBetween(eq(Instant.MIN), eq(to)); + verify(repository).findAllByPerformedAtBefore(to); } @Test @@ -311,7 +322,7 @@ class ListStockMovementsTest { assertThat(result.isSuccess()).isTrue(); verify(repository).findAllByStockId(StockId.of("stock-1")); verify(repository, never()).findAllByArticleId(any()); - verify(repository, never()).findAllByBatchId(any()); + verify(repository, never()).findAllByBatchReference(any()); verify(repository, never()).findAllByMovementType(any()); } @@ -325,20 +336,20 @@ class ListStockMovementsTest { assertThat(result.isSuccess()).isTrue(); verify(repository).findAllByArticleId(ArticleId.of("article-1")); - verify(repository, never()).findAllByBatchId(any()); + verify(repository, never()).findAllByBatchReference(any()); verify(repository, never()).findAllByMovementType(any()); } @Test @DisplayName("batchReference takes priority over movementType") void batchReferenceTakesPriorityOverMovementType() { - when(repository.findAllByBatchId("CHARGE-001")) + when(repository.findAllByBatchReference("CHARGE-001")) .thenReturn(Result.success(List.of())); var result = useCase.execute(null, null, "GOODS_RECEIPT", "CHARGE-001", null, null, actor); assertThat(result.isSuccess()).isTrue(); - verify(repository).findAllByBatchId("CHARGE-001"); + verify(repository).findAllByBatchReference("CHARGE-001"); verify(repository, never()).findAllByMovementType(any()); } @@ -375,7 +386,7 @@ class ListStockMovementsTest { @Test @DisplayName("batchReference takes priority over from/to") void batchReferenceTakesPriorityOverDateRange() { - when(repository.findAllByBatchId("CHARGE-001")) + when(repository.findAllByBatchReference("CHARGE-001")) .thenReturn(Result.success(List.of())); Instant from = Instant.parse("2026-01-01T00:00:00Z"); @@ -383,7 +394,7 @@ class ListStockMovementsTest { var result = useCase.execute(null, null, null, "CHARGE-001", from, to, actor); assertThat(result.isSuccess()).isTrue(); - verify(repository).findAllByBatchId("CHARGE-001"); + verify(repository).findAllByBatchReference("CHARGE-001"); verify(repository, never()).findAllByPerformedAtBetween(any(), any()); } } diff --git a/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockMovementControllerIntegrationTest.java b/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockMovementControllerIntegrationTest.java index 50fe492..82c6c6d 100644 --- a/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockMovementControllerIntegrationTest.java +++ b/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockMovementControllerIntegrationTest.java @@ -555,6 +555,16 @@ class StockMovementControllerIntegrationTest extends AbstractIntegrationTest { .andExpect(jsonPath("$").isArray()); } + @Test + @DisplayName("Leere batchReference → 400") + void filterByBlankBatchReference_returns400() throws Exception { + mockMvc.perform(get("/api/inventory/stock-movements") + .param("batchReference", " ") + .header("Authorization", "Bearer " + adminToken)) + .andExpect(status().isBadRequest()) + .andExpect(jsonPath("$.code").value("INVALID_BATCH_REFERENCE")); + } + @Test @DisplayName("Unbekannte batchReference → 200 mit []") void filterByUnknownBatchReference_returns200Empty() throws Exception { diff --git a/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java b/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java index 873a276..9e54a91 100644 --- a/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java +++ b/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java @@ -106,9 +106,20 @@ public final class InventoryScenario { } public static ChainBuilder listStockMovementsByDateRange() { - return exec( + return exec(session -> { + var rnd = ThreadLocalRandom.current(); + int startDay = rnd.nextInt(1, 28); + int startMonth = rnd.nextInt(1, 13); + int durationDays = rnd.nextInt(7, 90); + var from = java.time.LocalDate.of(2026, startMonth, startDay) + .atStartOfDay(java.time.ZoneOffset.UTC).toInstant(); + var to = from.plus(java.time.Duration.ofDays(durationDays)); + return session + .set("dateFrom", from.toString()) + .set("dateTo", to.toString()); + }).exec( http("Bestandsbewegungen nach Zeitraum") - .get("/api/inventory/stock-movements?from=2020-01-01T00:00:00Z&to=2030-12-31T23:59:59Z") + .get("/api/inventory/stock-movements?from=#{dateFrom}&to=#{dateTo}") .header("Authorization", "Bearer #{accessToken}") .check(status().is(200)) );