From 74dc9a69810b8b30217fcffc299812f7a1bfa90e Mon Sep 17 00:00:00 2001 From: Sebastian Frick Date: Wed, 25 Feb 2026 23:13:04 +0100 Subject: [PATCH] =?UTF-8?q?fix(inventory):=20confirmReservation=20?= =?UTF-8?q?=E2=80=93=20Layering,=20Error-Semantik=20und=20FK-Bug?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - MovementType-Ableitung via ReferenceType.toMovementType() in Domain Layer - Doppelten Batch-Lookup in Stock.confirmReservation() eliminiert - StockError.MovementCreationFailed statt RepositoryFailure für Domain-Fehler - 204 No Content statt 200 OK (konsistent mit releaseReservation) - Batches mit Menge 0 nicht mehr entfernen (FK stock_movements → stock_batches) --- .../inventory/ConfirmReservation.java | 7 ++--- .../domain/inventory/ReferenceType.java | 9 +++++- .../de/effigenix/domain/inventory/Stock.java | 28 +++++-------------- .../domain/inventory/StockError.java | 4 +++ .../web/controller/StockController.java | 2 +- .../InventoryErrorHttpStatusMapper.java | 1 + .../effigenix/domain/inventory/StockTest.java | 15 ++++++---- .../web/StockControllerIntegrationTest.java | 19 +++++++------ .../loadtest/scenario/InventoryScenario.java | 2 +- 9 files changed, 43 insertions(+), 44 deletions(-) diff --git a/backend/src/main/java/de/effigenix/application/inventory/ConfirmReservation.java b/backend/src/main/java/de/effigenix/application/inventory/ConfirmReservation.java index d6f0214..e50aec3 100644 --- a/backend/src/main/java/de/effigenix/application/inventory/ConfirmReservation.java +++ b/backend/src/main/java/de/effigenix/application/inventory/ConfirmReservation.java @@ -44,10 +44,7 @@ public class ConfirmReservation { } // 3. MovementType aus ReferenceType ableiten - String movementType = switch (confirmed.referenceType()) { - case PRODUCTION_ORDER -> "PRODUCTION_CONSUMPTION"; - case SALE_ORDER -> "SALE"; - }; + String movementType = confirmed.referenceType().toMovementType().name(); // 4. StockMovements erzeugen List movements = new ArrayList<>(); @@ -68,7 +65,7 @@ public class ConfirmReservation { ); switch (StockMovement.record(draft)) { case Result.Failure(var err) -> - { return Result.failure(new StockError.RepositoryFailure(err.message())); } + { return Result.failure(new StockError.MovementCreationFailed(err.message())); } case Result.Success(var val) -> movements.add(val); } } diff --git a/backend/src/main/java/de/effigenix/domain/inventory/ReferenceType.java b/backend/src/main/java/de/effigenix/domain/inventory/ReferenceType.java index c16d2aa..fa73a57 100644 --- a/backend/src/main/java/de/effigenix/domain/inventory/ReferenceType.java +++ b/backend/src/main/java/de/effigenix/domain/inventory/ReferenceType.java @@ -2,5 +2,12 @@ package de.effigenix.domain.inventory; public enum ReferenceType { PRODUCTION_ORDER, - SALE_ORDER + SALE_ORDER; + + public MovementType toMovementType() { + return switch (this) { + case PRODUCTION_ORDER -> MovementType.PRODUCTION_CONSUMPTION; + case SALE_ORDER -> MovementType.SALE; + }; + } } diff --git a/backend/src/main/java/de/effigenix/domain/inventory/Stock.java b/backend/src/main/java/de/effigenix/domain/inventory/Stock.java index b069f59..b1bc94c 100644 --- a/backend/src/main/java/de/effigenix/domain/inventory/Stock.java +++ b/backend/src/main/java/de/effigenix/domain/inventory/Stock.java @@ -35,7 +35,7 @@ import java.util.stream.Collectors; * - reserve: FEFO allocation across AVAILABLE/EXPIRING_SOON batches sorted by expiryDate ASC * - releaseReservation: removes reservation by ID, implicitly freeing allocated quantities * - confirmReservation: deducts allocated quantities from batches, removes reservation, returns ConfirmedReservation - * - batches with quantity 0 after deduction are removed + * - batches with quantity 0 after deduction are kept (FK from stock_movements requires batch persistence) * - reservations track allocated quantities per batch; no over-reservation possible */ public class Stock { @@ -366,7 +366,7 @@ public class Stock { return Result.failure(new StockError.ReservationNotFound(reservationId.value())); } - // 2. Für jede Allocation: Batch finden und BatchReference merken + // 2. Für jede Allocation: Batch finden, Menge abziehen, ConfirmedAllocation erzeugen List confirmedAllocations = new ArrayList<>(); for (StockBatchAllocation alloc : reservation.allocations()) { StockBatch batch = batches.stream() @@ -376,35 +376,21 @@ public class Stock { if (batch == null) { return Result.failure(new StockError.BatchNotFound(alloc.stockBatchId().value())); } + confirmedAllocations.add(new ConfirmedAllocation( batch.id(), batch.batchReference(), alloc.allocatedQuantity())); - } - - // 3. Für jede Allocation: Menge abziehen, bei 0 Batch entfernen - for (ConfirmedAllocation confirmed : confirmedAllocations) { - StockBatch batch = batches.stream() - .filter(b -> b.id().equals(confirmed.stockBatchId())) - .findFirst() - .orElse(null); - if (batch == null) { - return Result.failure(new StockError.BatchNotFound(confirmed.stockBatchId().value())); - } Quantity remaining; - switch (batch.removeQuantity(confirmed.allocatedQuantity())) { + switch (batch.removeQuantity(alloc.allocatedQuantity())) { case Result.Failure(var err) -> { return Result.failure(err); } case Result.Success(var val) -> remaining = val; } - if (remaining.amount().signum() == 0) { - this.batches.remove(batch); - } else { - int index = this.batches.indexOf(batch); - this.batches.set(index, batch.withQuantity(remaining)); - } + int index = this.batches.indexOf(batch); + this.batches.set(index, batch.withQuantity(remaining)); } - // 4. Reservation entfernen + // 3. Reservation entfernen this.reservations.remove(reservation); return Result.success(new ConfirmedReservation( diff --git a/backend/src/main/java/de/effigenix/domain/inventory/StockError.java b/backend/src/main/java/de/effigenix/domain/inventory/StockError.java index 853f3b7..448006b 100644 --- a/backend/src/main/java/de/effigenix/domain/inventory/StockError.java +++ b/backend/src/main/java/de/effigenix/domain/inventory/StockError.java @@ -113,6 +113,10 @@ public sealed interface StockError { @Override public String code() { return "UNAUTHORIZED"; } } + record MovementCreationFailed(String message) implements StockError { + @Override public String code() { return "MOVEMENT_CREATION_FAILED"; } + } + record RepositoryFailure(String message) implements StockError { @Override public String code() { return "REPOSITORY_ERROR"; } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java index 45fa8f6..e71019a 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java +++ b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java @@ -332,7 +332,7 @@ public class StockController { } logger.info("Reservation {} of stock {} confirmed", reservationId, stockId); - return ResponseEntity.ok().build(); + return ResponseEntity.noContent().build(); } public static class StockDomainErrorException extends RuntimeException { diff --git a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/exception/InventoryErrorHttpStatusMapper.java b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/exception/InventoryErrorHttpStatusMapper.java index f6df127..4cca45a 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/inventory/web/exception/InventoryErrorHttpStatusMapper.java +++ b/backend/src/main/java/de/effigenix/infrastructure/inventory/web/exception/InventoryErrorHttpStatusMapper.java @@ -47,6 +47,7 @@ public final class InventoryErrorHttpStatusMapper { case StockError.InvalidReferenceId e -> 400; case StockError.InvalidReservationPriority e -> 400; case StockError.Unauthorized e -> 403; + case StockError.MovementCreationFailed e -> 500; case StockError.RepositoryFailure e -> 500; }; } diff --git a/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java b/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java index 117debb..0656d9a 100644 --- a/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java +++ b/backend/src/test/java/de/effigenix/domain/inventory/StockTest.java @@ -1853,15 +1853,17 @@ class StockTest { assertThat(confirmed.referenceType()).isEqualTo(ReferenceType.SALE_ORDER); assertThat(confirmed.allocations()).hasSize(2); assertThat(stock.reservations()).isEmpty(); - // batch1 fully consumed → removed, batch2 has 4kg remaining - assertThat(stock.batches()).hasSize(1); - assertThat(stock.batches().getFirst().quantity().amount()) + // batch1 fully consumed (0 kg), batch2 has 4kg remaining + assertThat(stock.batches()).hasSize(2); + assertThat(stock.batches().get(0).quantity().amount()) + .isEqualByComparingTo(BigDecimal.ZERO); + assertThat(stock.batches().get(1).quantity().amount()) .isEqualByComparingTo(new BigDecimal("4")); } @Test - @DisplayName("should remove batch when quantity reaches zero") - void shouldRemoveBatchWhenQuantityZero() { + @DisplayName("should keep batch with zero quantity after full deduction") + void shouldKeepBatchWithZeroQuantity() { var stock = createStockWithBatchAndExpiry("10", UnitOfMeasure.KILOGRAM, StockBatchStatus.AVAILABLE, LocalDate.of(2026, 12, 31)); @@ -1871,7 +1873,8 @@ class StockTest { var result = stock.confirmReservation(reserveResult.unsafeGetValue().id()); assertThat(result.isSuccess()).isTrue(); - assertThat(stock.batches()).isEmpty(); + assertThat(stock.batches()).hasSize(1); + assertThat(stock.batches().getFirst().quantity().amount()).isEqualByComparingTo(BigDecimal.ZERO); assertThat(stock.reservations()).isEmpty(); } diff --git a/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockControllerIntegrationTest.java b/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockControllerIntegrationTest.java index 28f99be..a53efd8 100644 --- a/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockControllerIntegrationTest.java +++ b/backend/src/test/java/de/effigenix/infrastructure/inventory/web/StockControllerIntegrationTest.java @@ -1506,7 +1506,7 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest { // Bestätigen mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); // Prüfen: Reservation entfernt, Menge physisch abgezogen mockMvc.perform(get("/api/inventory/stocks/{id}", stockId) @@ -1517,8 +1517,8 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest { } @Test - @DisplayName("Bestätigung der gesamten Charge → Batch wird entfernt") - void confirmReservation_fullBatch_removesBatch() throws Exception { + @DisplayName("Bestätigung der gesamten Charge → Batch bleibt mit Menge 0") + void confirmReservation_fullBatch_keepsBatchWithZeroQuantity() throws Exception { String stockId = createStock(); addBatchToStock(stockId); // 10 KILOGRAM @@ -1535,13 +1535,14 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest { // Bestätigen mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); - // Batch entfernt + // Batch bleibt mit Menge 0 (FK von stock_movements erfordert Persistenz) mockMvc.perform(get("/api/inventory/stocks/{id}", stockId) .header("Authorization", "Bearer " + adminToken)) .andExpect(jsonPath("$.reservations.length()").value(0)) - .andExpect(jsonPath("$.batches.length()").value(0)) + .andExpect(jsonPath("$.batches.length()").value(1)) + .andExpect(jsonPath("$.batches[0].quantityAmount").value(0)) .andExpect(jsonPath("$.availableQuantity").value(0)); } @@ -1599,10 +1600,10 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest { .andReturn(); String reservationId = objectMapper.readTree(reserveResult.getResponse().getContentAsString()).get("id").asText(); - // Erstes Confirm → 200 + // Erstes Confirm → 204 mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); // Zweites Confirm → 404 mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId) @@ -1638,7 +1639,7 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest { // Erste Reservierung bestätigen mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId1) .header("Authorization", "Bearer " + adminToken)) - .andExpect(status().isOk()); + .andExpect(status().isNoContent()); // Batch: 10 - 4 = 6 kg, SO-001 bleibt mockMvc.perform(get("/api/inventory/stocks/{id}", stockId) 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 7b4844d..42ecb77 100644 --- a/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java +++ b/loadtest/src/test/java/de/effigenix/loadtest/scenario/InventoryScenario.java @@ -147,7 +147,7 @@ public final class InventoryScenario { http("Reservierung bestätigen") .post("/api/inventory/stocks/#{reserveStockId}/reservations/#{reservationId}/confirm") .header("Authorization", "Bearer #{accessToken}") - .check(status().in(200, 404)) + .check(status().in(204, 404)) ).exec(session -> session.remove("reservationId")) ) );