mirror of
https://github.com/s-frick/effigenix.git
synced 2026-03-28 10:39:35 +01:00
fix(inventory): confirmReservation – Layering, Error-Semantik und FK-Bug
- 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)
This commit is contained in:
parent
0b6028b967
commit
74dc9a6981
9 changed files with 43 additions and 44 deletions
|
|
@ -44,10 +44,7 @@ public class ConfirmReservation {
|
||||||
}
|
}
|
||||||
|
|
||||||
// 3. MovementType aus ReferenceType ableiten
|
// 3. MovementType aus ReferenceType ableiten
|
||||||
String movementType = switch (confirmed.referenceType()) {
|
String movementType = confirmed.referenceType().toMovementType().name();
|
||||||
case PRODUCTION_ORDER -> "PRODUCTION_CONSUMPTION";
|
|
||||||
case SALE_ORDER -> "SALE";
|
|
||||||
};
|
|
||||||
|
|
||||||
// 4. StockMovements erzeugen
|
// 4. StockMovements erzeugen
|
||||||
List<StockMovement> movements = new ArrayList<>();
|
List<StockMovement> movements = new ArrayList<>();
|
||||||
|
|
@ -68,7 +65,7 @@ public class ConfirmReservation {
|
||||||
);
|
);
|
||||||
switch (StockMovement.record(draft)) {
|
switch (StockMovement.record(draft)) {
|
||||||
case Result.Failure(var err) ->
|
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);
|
case Result.Success(var val) -> movements.add(val);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -2,5 +2,12 @@ package de.effigenix.domain.inventory;
|
||||||
|
|
||||||
public enum ReferenceType {
|
public enum ReferenceType {
|
||||||
PRODUCTION_ORDER,
|
PRODUCTION_ORDER,
|
||||||
SALE_ORDER
|
SALE_ORDER;
|
||||||
|
|
||||||
|
public MovementType toMovementType() {
|
||||||
|
return switch (this) {
|
||||||
|
case PRODUCTION_ORDER -> MovementType.PRODUCTION_CONSUMPTION;
|
||||||
|
case SALE_ORDER -> MovementType.SALE;
|
||||||
|
};
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -35,7 +35,7 @@ import java.util.stream.Collectors;
|
||||||
* - reserve: FEFO allocation across AVAILABLE/EXPIRING_SOON batches sorted by expiryDate ASC
|
* - reserve: FEFO allocation across AVAILABLE/EXPIRING_SOON batches sorted by expiryDate ASC
|
||||||
* - releaseReservation: removes reservation by ID, implicitly freeing allocated quantities
|
* - releaseReservation: removes reservation by ID, implicitly freeing allocated quantities
|
||||||
* - confirmReservation: deducts allocated quantities from batches, removes reservation, returns ConfirmedReservation
|
* - 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
|
* - reservations track allocated quantities per batch; no over-reservation possible
|
||||||
*/
|
*/
|
||||||
public class Stock {
|
public class Stock {
|
||||||
|
|
@ -366,7 +366,7 @@ public class Stock {
|
||||||
return Result.failure(new StockError.ReservationNotFound(reservationId.value()));
|
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<ConfirmedAllocation> confirmedAllocations = new ArrayList<>();
|
List<ConfirmedAllocation> confirmedAllocations = new ArrayList<>();
|
||||||
for (StockBatchAllocation alloc : reservation.allocations()) {
|
for (StockBatchAllocation alloc : reservation.allocations()) {
|
||||||
StockBatch batch = batches.stream()
|
StockBatch batch = batches.stream()
|
||||||
|
|
@ -376,35 +376,21 @@ public class Stock {
|
||||||
if (batch == null) {
|
if (batch == null) {
|
||||||
return Result.failure(new StockError.BatchNotFound(alloc.stockBatchId().value()));
|
return Result.failure(new StockError.BatchNotFound(alloc.stockBatchId().value()));
|
||||||
}
|
}
|
||||||
|
|
||||||
confirmedAllocations.add(new ConfirmedAllocation(
|
confirmedAllocations.add(new ConfirmedAllocation(
|
||||||
batch.id(), batch.batchReference(), alloc.allocatedQuantity()));
|
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;
|
Quantity remaining;
|
||||||
switch (batch.removeQuantity(confirmed.allocatedQuantity())) {
|
switch (batch.removeQuantity(alloc.allocatedQuantity())) {
|
||||||
case Result.Failure(var err) -> { return Result.failure(err); }
|
case Result.Failure(var err) -> { return Result.failure(err); }
|
||||||
case Result.Success(var val) -> remaining = val;
|
case Result.Success(var val) -> remaining = val;
|
||||||
}
|
}
|
||||||
|
|
||||||
if (remaining.amount().signum() == 0) {
|
int index = this.batches.indexOf(batch);
|
||||||
this.batches.remove(batch);
|
this.batches.set(index, batch.withQuantity(remaining));
|
||||||
} else {
|
|
||||||
int index = this.batches.indexOf(batch);
|
|
||||||
this.batches.set(index, batch.withQuantity(remaining));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// 4. Reservation entfernen
|
// 3. Reservation entfernen
|
||||||
this.reservations.remove(reservation);
|
this.reservations.remove(reservation);
|
||||||
|
|
||||||
return Result.success(new ConfirmedReservation(
|
return Result.success(new ConfirmedReservation(
|
||||||
|
|
|
||||||
|
|
@ -113,6 +113,10 @@ public sealed interface StockError {
|
||||||
@Override public String code() { return "UNAUTHORIZED"; }
|
@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 {
|
record RepositoryFailure(String message) implements StockError {
|
||||||
@Override public String code() { return "REPOSITORY_ERROR"; }
|
@Override public String code() { return "REPOSITORY_ERROR"; }
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -332,7 +332,7 @@ public class StockController {
|
||||||
}
|
}
|
||||||
|
|
||||||
logger.info("Reservation {} of stock {} confirmed", reservationId, stockId);
|
logger.info("Reservation {} of stock {} confirmed", reservationId, stockId);
|
||||||
return ResponseEntity.ok().build();
|
return ResponseEntity.noContent().build();
|
||||||
}
|
}
|
||||||
|
|
||||||
public static class StockDomainErrorException extends RuntimeException {
|
public static class StockDomainErrorException extends RuntimeException {
|
||||||
|
|
|
||||||
|
|
@ -47,6 +47,7 @@ public final class InventoryErrorHttpStatusMapper {
|
||||||
case StockError.InvalidReferenceId e -> 400;
|
case StockError.InvalidReferenceId e -> 400;
|
||||||
case StockError.InvalidReservationPriority e -> 400;
|
case StockError.InvalidReservationPriority e -> 400;
|
||||||
case StockError.Unauthorized e -> 403;
|
case StockError.Unauthorized e -> 403;
|
||||||
|
case StockError.MovementCreationFailed e -> 500;
|
||||||
case StockError.RepositoryFailure e -> 500;
|
case StockError.RepositoryFailure e -> 500;
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1853,15 +1853,17 @@ class StockTest {
|
||||||
assertThat(confirmed.referenceType()).isEqualTo(ReferenceType.SALE_ORDER);
|
assertThat(confirmed.referenceType()).isEqualTo(ReferenceType.SALE_ORDER);
|
||||||
assertThat(confirmed.allocations()).hasSize(2);
|
assertThat(confirmed.allocations()).hasSize(2);
|
||||||
assertThat(stock.reservations()).isEmpty();
|
assertThat(stock.reservations()).isEmpty();
|
||||||
// batch1 fully consumed → removed, batch2 has 4kg remaining
|
// batch1 fully consumed (0 kg), batch2 has 4kg remaining
|
||||||
assertThat(stock.batches()).hasSize(1);
|
assertThat(stock.batches()).hasSize(2);
|
||||||
assertThat(stock.batches().getFirst().quantity().amount())
|
assertThat(stock.batches().get(0).quantity().amount())
|
||||||
|
.isEqualByComparingTo(BigDecimal.ZERO);
|
||||||
|
assertThat(stock.batches().get(1).quantity().amount())
|
||||||
.isEqualByComparingTo(new BigDecimal("4"));
|
.isEqualByComparingTo(new BigDecimal("4"));
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("should remove batch when quantity reaches zero")
|
@DisplayName("should keep batch with zero quantity after full deduction")
|
||||||
void shouldRemoveBatchWhenQuantityZero() {
|
void shouldKeepBatchWithZeroQuantity() {
|
||||||
var stock = createStockWithBatchAndExpiry("10", UnitOfMeasure.KILOGRAM,
|
var stock = createStockWithBatchAndExpiry("10", UnitOfMeasure.KILOGRAM,
|
||||||
StockBatchStatus.AVAILABLE, LocalDate.of(2026, 12, 31));
|
StockBatchStatus.AVAILABLE, LocalDate.of(2026, 12, 31));
|
||||||
|
|
||||||
|
|
@ -1871,7 +1873,8 @@ class StockTest {
|
||||||
var result = stock.confirmReservation(reserveResult.unsafeGetValue().id());
|
var result = stock.confirmReservation(reserveResult.unsafeGetValue().id());
|
||||||
|
|
||||||
assertThat(result.isSuccess()).isTrue();
|
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();
|
assertThat(stock.reservations()).isEmpty();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1506,7 +1506,7 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest {
|
||||||
// Bestätigen
|
// Bestätigen
|
||||||
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
||||||
.header("Authorization", "Bearer " + adminToken))
|
.header("Authorization", "Bearer " + adminToken))
|
||||||
.andExpect(status().isOk());
|
.andExpect(status().isNoContent());
|
||||||
|
|
||||||
// Prüfen: Reservation entfernt, Menge physisch abgezogen
|
// Prüfen: Reservation entfernt, Menge physisch abgezogen
|
||||||
mockMvc.perform(get("/api/inventory/stocks/{id}", stockId)
|
mockMvc.perform(get("/api/inventory/stocks/{id}", stockId)
|
||||||
|
|
@ -1517,8 +1517,8 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest {
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@DisplayName("Bestätigung der gesamten Charge → Batch wird entfernt")
|
@DisplayName("Bestätigung der gesamten Charge → Batch bleibt mit Menge 0")
|
||||||
void confirmReservation_fullBatch_removesBatch() throws Exception {
|
void confirmReservation_fullBatch_keepsBatchWithZeroQuantity() throws Exception {
|
||||||
String stockId = createStock();
|
String stockId = createStock();
|
||||||
addBatchToStock(stockId); // 10 KILOGRAM
|
addBatchToStock(stockId); // 10 KILOGRAM
|
||||||
|
|
||||||
|
|
@ -1535,13 +1535,14 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest {
|
||||||
// Bestätigen
|
// Bestätigen
|
||||||
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
||||||
.header("Authorization", "Bearer " + adminToken))
|
.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)
|
mockMvc.perform(get("/api/inventory/stocks/{id}", stockId)
|
||||||
.header("Authorization", "Bearer " + adminToken))
|
.header("Authorization", "Bearer " + adminToken))
|
||||||
.andExpect(jsonPath("$.reservations.length()").value(0))
|
.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));
|
.andExpect(jsonPath("$.availableQuantity").value(0));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1599,10 +1600,10 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest {
|
||||||
.andReturn();
|
.andReturn();
|
||||||
String reservationId = objectMapper.readTree(reserveResult.getResponse().getContentAsString()).get("id").asText();
|
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)
|
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
||||||
.header("Authorization", "Bearer " + adminToken))
|
.header("Authorization", "Bearer " + adminToken))
|
||||||
.andExpect(status().isOk());
|
.andExpect(status().isNoContent());
|
||||||
|
|
||||||
// Zweites Confirm → 404
|
// Zweites Confirm → 404
|
||||||
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId)
|
||||||
|
|
@ -1638,7 +1639,7 @@ class StockControllerIntegrationTest extends AbstractIntegrationTest {
|
||||||
// Erste Reservierung bestätigen
|
// Erste Reservierung bestätigen
|
||||||
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId1)
|
mockMvc.perform(post("/api/inventory/stocks/{stockId}/reservations/{reservationId}/confirm", stockId, reservationId1)
|
||||||
.header("Authorization", "Bearer " + adminToken))
|
.header("Authorization", "Bearer " + adminToken))
|
||||||
.andExpect(status().isOk());
|
.andExpect(status().isNoContent());
|
||||||
|
|
||||||
// Batch: 10 - 4 = 6 kg, SO-001 bleibt
|
// Batch: 10 - 4 = 6 kg, SO-001 bleibt
|
||||||
mockMvc.perform(get("/api/inventory/stocks/{id}", stockId)
|
mockMvc.perform(get("/api/inventory/stocks/{id}", stockId)
|
||||||
|
|
|
||||||
|
|
@ -147,7 +147,7 @@ public final class InventoryScenario {
|
||||||
http("Reservierung bestätigen")
|
http("Reservierung bestätigen")
|
||||||
.post("/api/inventory/stocks/#{reserveStockId}/reservations/#{reservationId}/confirm")
|
.post("/api/inventory/stocks/#{reserveStockId}/reservations/#{reservationId}/confirm")
|
||||||
.header("Authorization", "Bearer #{accessToken}")
|
.header("Authorization", "Bearer #{accessToken}")
|
||||||
.check(status().in(200, 404))
|
.check(status().in(204, 404))
|
||||||
).exec(session -> session.remove("reservationId"))
|
).exec(session -> session.remove("reservationId"))
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue