From 9eb9c93fb783643dab94199afe344bb42e055929 Mon Sep 17 00:00:00 2001 From: Sebastian Frick Date: Fri, 20 Feb 2026 16:52:11 +0100 Subject: [PATCH] fix(production): JPA-Save-Pattern, Optimistic Locking und Domain-Validierung MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Managed-Entity-Update statt detach/merge (verhindert DELETE+INSERT-Churn) - @Version für Optimistic Locking mit ConcurrentModification-Error - Null-Checks für quantityUsed/quantityUnit vor BigDecimal-Parsing - Duplicate-Check nach Consumption.create() für robustere Validierung - FetchType.EAGER→LAZY für BatchEntity.consumptions - Liquibase-Migration 020 für version-Spalte --- .../de/effigenix/domain/production/Batch.java | 23 +++++----- .../domain/production/Consumption.java | 7 +++ .../domain/production/event/BatchStarted.java | 4 ++ .../production/event/ConsumptionRecorded.java | 4 ++ .../persistence/entity/BatchEntity.java | 11 ++++- .../persistence/mapper/BatchMapper.java | 43 +++++++++++++------ .../repository/JpaBatchRepository.java | 12 +++++- .../shared/common/RepositoryError.java | 3 ++ .../changes/020-add-version-to-batches.xml | 16 +++++++ .../db/changelog/db.changelog-master.xml | 1 + .../production/FindBatchByNumberTest.java | 2 +- .../application/production/GetBatchTest.java | 2 +- .../production/ListBatchesTest.java | 2 +- .../production/RecordConsumptionTest.java | 4 +- .../production/StartBatchTest.java | 4 +- .../domain/production/BatchTest.java | 10 ++--- .../domain/production/ConsumptionTest.java | 22 ++++++++++ 17 files changed, 133 insertions(+), 37 deletions(-) create mode 100644 backend/src/main/resources/db/changelog/changes/020-add-version-to-batches.xml diff --git a/backend/src/main/java/de/effigenix/domain/production/Batch.java b/backend/src/main/java/de/effigenix/domain/production/Batch.java index 4f0f50a..8f099cd 100644 --- a/backend/src/main/java/de/effigenix/domain/production/Batch.java +++ b/backend/src/main/java/de/effigenix/domain/production/Batch.java @@ -37,6 +37,7 @@ public class Batch { private final LocalDate bestBeforeDate; private final OffsetDateTime createdAt; private OffsetDateTime updatedAt; + private final long version; private final List consumptions; private Batch( @@ -49,6 +50,7 @@ public class Batch { LocalDate bestBeforeDate, OffsetDateTime createdAt, OffsetDateTime updatedAt, + long version, List consumptions ) { this.id = id; @@ -60,6 +62,7 @@ public class Batch { this.bestBeforeDate = bestBeforeDate; this.createdAt = createdAt; this.updatedAt = updatedAt; + this.version = version; this.consumptions = consumptions; } @@ -110,6 +113,7 @@ public class Batch { draft.bestBeforeDate(), now, now, + 0L, new ArrayList<>() )); } @@ -128,21 +132,18 @@ public class Batch { return Result.failure(new BatchError.NotInProduction(id)); } - if (draft.inputBatchId() != null && !draft.inputBatchId().isBlank()) { - var inputId = BatchId.of(draft.inputBatchId()); - boolean duplicate = consumptions.stream() - .anyMatch(c -> c.inputBatchId().equals(inputId)); - if (duplicate) { - return Result.failure(new BatchError.DuplicateInputBatch(inputId)); - } - } - Consumption consumption; switch (Consumption.create(draft)) { case Result.Failure(var err) -> { return Result.failure(err); } case Result.Success(var val) -> consumption = val; } + boolean duplicate = consumptions.stream() + .anyMatch(c -> c.inputBatchId().equals(consumption.inputBatchId())); + if (duplicate) { + return Result.failure(new BatchError.DuplicateInputBatch(consumption.inputBatchId())); + } + consumptions.add(consumption); this.updatedAt = OffsetDateTime.now(ZoneOffset.UTC); return Result.success(consumption); @@ -158,10 +159,11 @@ public class Batch { LocalDate bestBeforeDate, OffsetDateTime createdAt, OffsetDateTime updatedAt, + long version, List consumptions ) { return new Batch(id, batchNumber, recipeId, status, plannedQuantity, productionDate, - bestBeforeDate, createdAt, updatedAt, new ArrayList<>(consumptions)); + bestBeforeDate, createdAt, updatedAt, version, new ArrayList<>(consumptions)); } public BatchId id() { return id; } @@ -173,5 +175,6 @@ public class Batch { public LocalDate bestBeforeDate() { return bestBeforeDate; } public OffsetDateTime createdAt() { return createdAt; } public OffsetDateTime updatedAt() { return updatedAt; } + public long version() { return version; } public List consumptions() { return Collections.unmodifiableList(consumptions); } } diff --git a/backend/src/main/java/de/effigenix/domain/production/Consumption.java b/backend/src/main/java/de/effigenix/domain/production/Consumption.java index f596022..3144bb6 100644 --- a/backend/src/main/java/de/effigenix/domain/production/Consumption.java +++ b/backend/src/main/java/de/effigenix/domain/production/Consumption.java @@ -34,6 +34,13 @@ public class Consumption { return Result.failure(new BatchError.ValidationFailure("articleId must not be blank")); } + if (draft.quantityUsed() == null || draft.quantityUsed().isBlank()) { + return Result.failure(new BatchError.InvalidConsumptionQuantity("quantityUsed must not be blank")); + } + if (draft.quantityUnit() == null || draft.quantityUnit().isBlank()) { + return Result.failure(new BatchError.InvalidConsumptionQuantity("quantityUnit must not be blank")); + } + Quantity quantity; try { var amount = new BigDecimal(draft.quantityUsed()); diff --git a/backend/src/main/java/de/effigenix/domain/production/event/BatchStarted.java b/backend/src/main/java/de/effigenix/domain/production/event/BatchStarted.java index e1381db..f7f38c3 100644 --- a/backend/src/main/java/de/effigenix/domain/production/event/BatchStarted.java +++ b/backend/src/main/java/de/effigenix/domain/production/event/BatchStarted.java @@ -4,4 +4,8 @@ import de.effigenix.domain.production.BatchId; import java.time.OffsetDateTime; +/** + * Stub – wird derzeit nicht publiziert. + * Vorgesehen für spätere Event-Infrastruktur (Audit, Inventory-Deduction, Tracing). + */ public record BatchStarted(BatchId batchId, OffsetDateTime startedAt) {} diff --git a/backend/src/main/java/de/effigenix/domain/production/event/ConsumptionRecorded.java b/backend/src/main/java/de/effigenix/domain/production/event/ConsumptionRecorded.java index 234ab12..4ed4481 100644 --- a/backend/src/main/java/de/effigenix/domain/production/event/ConsumptionRecorded.java +++ b/backend/src/main/java/de/effigenix/domain/production/event/ConsumptionRecorded.java @@ -3,4 +3,8 @@ package de.effigenix.domain.production.event; import de.effigenix.domain.production.BatchId; import de.effigenix.domain.production.ConsumptionId; +/** + * Stub – wird derzeit nicht publiziert. + * Vorgesehen für spätere Event-Infrastruktur (Chargen-Genealogie, Bestandsabzug). + */ public record ConsumptionRecorded(BatchId batchId, ConsumptionId consumptionId, BatchId inputBatchId) {} diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/entity/BatchEntity.java b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/entity/BatchEntity.java index 2424e84..6118c81 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/entity/BatchEntity.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/entity/BatchEntity.java @@ -16,6 +16,10 @@ public class BatchEntity { @Column(name = "id", nullable = false, length = 36) private String id; + @Version + @Column(name = "version", nullable = false) + private Long version; + @Column(name = "batch_number", nullable = false, unique = true, length = 20) private String batchNumber; @@ -43,7 +47,7 @@ public class BatchEntity { @Column(name = "updated_at", nullable = false) private OffsetDateTime updatedAt; - @OneToMany(mappedBy = "batch", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.EAGER) + @OneToMany(mappedBy = "batch", cascade = CascadeType.ALL, orphanRemoval = true, fetch = FetchType.LAZY) private List consumptions = new ArrayList<>(); protected BatchEntity() {} @@ -73,6 +77,7 @@ public class BatchEntity { } public String getId() { return id; } + public Long getVersion() { return version; } public String getBatchNumber() { return batchNumber; } public String getRecipeId() { return recipeId; } public String getStatus() { return status; } @@ -83,5 +88,7 @@ public class BatchEntity { public OffsetDateTime getCreatedAt() { return createdAt; } public OffsetDateTime getUpdatedAt() { return updatedAt; } public List getConsumptions() { return consumptions; } - public void setConsumptions(List consumptions) { this.consumptions = consumptions; } + + public void setStatus(String status) { this.status = status; } + public void setUpdatedAt(OffsetDateTime updatedAt) { this.updatedAt = updatedAt; } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/mapper/BatchMapper.java b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/mapper/BatchMapper.java index a8411ab..6f8ae94 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/mapper/BatchMapper.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/mapper/BatchMapper.java @@ -8,8 +8,9 @@ import de.effigenix.shared.common.Quantity; import de.effigenix.shared.common.UnitOfMeasure; import org.springframework.stereotype.Component; -import java.util.ArrayList; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; @Component public class BatchMapper { @@ -28,23 +29,28 @@ public class BatchMapper { batch.updatedAt() ); - List consumptionEntities = new ArrayList<>(); for (Consumption c : batch.consumptions()) { - consumptionEntities.add(new ConsumptionEntity( - c.id().value(), - entity, - c.inputBatchId().value(), - c.articleId().value(), - c.quantityUsed().amount(), - c.quantityUsed().uom().name(), - c.consumedAt() - )); + entity.getConsumptions().add(toConsumptionEntity(c, entity)); } - entity.setConsumptions(consumptionEntities); return entity; } + public void updateEntity(BatchEntity entity, Batch batch) { + entity.setStatus(batch.status().name()); + entity.setUpdatedAt(batch.updatedAt()); + + Set existingIds = entity.getConsumptions().stream() + .map(ConsumptionEntity::getId) + .collect(Collectors.toSet()); + + for (Consumption c : batch.consumptions()) { + if (!existingIds.contains(c.id().value())) { + entity.getConsumptions().add(toConsumptionEntity(c, entity)); + } + } + } + public Batch toDomain(BatchEntity entity) { List consumptions = entity.getConsumptions().stream() .map(ce -> Consumption.reconstitute( @@ -72,7 +78,20 @@ public class BatchMapper { entity.getBestBeforeDate(), entity.getCreatedAt(), entity.getUpdatedAt(), + entity.getVersion(), consumptions ); } + + private ConsumptionEntity toConsumptionEntity(Consumption c, BatchEntity parent) { + return new ConsumptionEntity( + c.id().value(), + parent, + c.inputBatchId().value(), + c.articleId().value(), + c.quantityUsed().amount(), + c.quantityUsed().uom().name(), + c.consumedAt() + ); + } } diff --git a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/repository/JpaBatchRepository.java b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/repository/JpaBatchRepository.java index 0048ed1..357d30f 100644 --- a/backend/src/main/java/de/effigenix/infrastructure/production/persistence/repository/JpaBatchRepository.java +++ b/backend/src/main/java/de/effigenix/infrastructure/production/persistence/repository/JpaBatchRepository.java @@ -7,6 +7,7 @@ import de.effigenix.shared.common.Result; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.context.annotation.Profile; +import org.springframework.orm.ObjectOptimisticLockingFailureException; import org.springframework.stereotype.Repository; import org.springframework.transaction.annotation.Transactional; @@ -111,8 +112,17 @@ public class JpaBatchRepository implements BatchRepository { @Transactional public Result save(Batch batch) { try { - jpaRepository.save(mapper.toEntity(batch)); + var existing = jpaRepository.findById(batch.id().value()); + if (existing.isPresent()) { + mapper.updateEntity(existing.get(), batch); + } else { + jpaRepository.save(mapper.toEntity(batch)); + } return Result.success(null); + } catch (ObjectOptimisticLockingFailureException e) { + logger.warn("Optimistic locking failure for batch {}", batch.id().value()); + return Result.failure(new RepositoryError.ConcurrentModification( + "Batch was modified by another transaction")); } catch (Exception e) { logger.trace("Database error in save", e); return Result.failure(new RepositoryError.DatabaseError(e.getMessage())); diff --git a/backend/src/main/java/de/effigenix/shared/common/RepositoryError.java b/backend/src/main/java/de/effigenix/shared/common/RepositoryError.java index 7884b45..bb6bc8e 100644 --- a/backend/src/main/java/de/effigenix/shared/common/RepositoryError.java +++ b/backend/src/main/java/de/effigenix/shared/common/RepositoryError.java @@ -21,4 +21,7 @@ public sealed interface RepositoryError { record DatabaseError(String message) implements RepositoryError { } + + record ConcurrentModification(String message) implements RepositoryError { + } } diff --git a/backend/src/main/resources/db/changelog/changes/020-add-version-to-batches.xml b/backend/src/main/resources/db/changelog/changes/020-add-version-to-batches.xml new file mode 100644 index 0000000..1694921 --- /dev/null +++ b/backend/src/main/resources/db/changelog/changes/020-add-version-to-batches.xml @@ -0,0 +1,16 @@ + + + + + + + + + + + + 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 f683703..36c9993 100644 --- a/backend/src/main/resources/db/changelog/db.changelog-master.xml +++ b/backend/src/main/resources/db/changelog/db.changelog-master.xml @@ -24,5 +24,6 @@ + diff --git a/backend/src/test/java/de/effigenix/application/production/FindBatchByNumberTest.java b/backend/src/test/java/de/effigenix/application/production/FindBatchByNumberTest.java index d223818..ffae896 100644 --- a/backend/src/test/java/de/effigenix/application/production/FindBatchByNumberTest.java +++ b/backend/src/test/java/de/effigenix/application/production/FindBatchByNumberTest.java @@ -53,7 +53,7 @@ class FindBatchByNumberTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } diff --git a/backend/src/test/java/de/effigenix/application/production/GetBatchTest.java b/backend/src/test/java/de/effigenix/application/production/GetBatchTest.java index e2208cf..72ccc8e 100644 --- a/backend/src/test/java/de/effigenix/application/production/GetBatchTest.java +++ b/backend/src/test/java/de/effigenix/application/production/GetBatchTest.java @@ -51,7 +51,7 @@ class GetBatchTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } diff --git a/backend/src/test/java/de/effigenix/application/production/ListBatchesTest.java b/backend/src/test/java/de/effigenix/application/production/ListBatchesTest.java index 4ad70dd..c439bc4 100644 --- a/backend/src/test/java/de/effigenix/application/production/ListBatchesTest.java +++ b/backend/src/test/java/de/effigenix/application/production/ListBatchesTest.java @@ -55,7 +55,7 @@ class ListBatchesTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } diff --git a/backend/src/test/java/de/effigenix/application/production/RecordConsumptionTest.java b/backend/src/test/java/de/effigenix/application/production/RecordConsumptionTest.java index 9dfbd1f..8c84995 100644 --- a/backend/src/test/java/de/effigenix/application/production/RecordConsumptionTest.java +++ b/backend/src/test/java/de/effigenix/application/production/RecordConsumptionTest.java @@ -52,7 +52,7 @@ class RecordConsumptionTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } @@ -67,7 +67,7 @@ class RecordConsumptionTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } diff --git a/backend/src/test/java/de/effigenix/application/production/StartBatchTest.java b/backend/src/test/java/de/effigenix/application/production/StartBatchTest.java index 5cffd03..d2ba366 100644 --- a/backend/src/test/java/de/effigenix/application/production/StartBatchTest.java +++ b/backend/src/test/java/de/effigenix/application/production/StartBatchTest.java @@ -52,7 +52,7 @@ class StartBatchTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } @@ -67,7 +67,7 @@ class StartBatchTest { LocalDate.of(2026, 6, 1), OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); } diff --git a/backend/src/test/java/de/effigenix/domain/production/BatchTest.java b/backend/src/test/java/de/effigenix/domain/production/BatchTest.java index 4dbf0f4..3153205 100644 --- a/backend/src/test/java/de/effigenix/domain/production/BatchTest.java +++ b/backend/src/test/java/de/effigenix/domain/production/BatchTest.java @@ -210,7 +210,7 @@ class BatchTest { Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), PRODUCTION_DATE, BEST_BEFORE_DATE, OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); var result = batch.startProduction(); @@ -231,7 +231,7 @@ class BatchTest { Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), PRODUCTION_DATE, BEST_BEFORE_DATE, OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); var result = batch.startProduction(); @@ -249,7 +249,7 @@ class BatchTest { Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), PRODUCTION_DATE, BEST_BEFORE_DATE, OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); var result = batch.startProduction(); @@ -313,7 +313,7 @@ class BatchTest { Quantity.of(new BigDecimal("100"), UnitOfMeasure.KILOGRAM).unsafeGetValue(), PRODUCTION_DATE, BEST_BEFORE_DATE, OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); var draft = new ConsumptionDraft("input-1", "article-1", "5.0", "KILOGRAM"); @@ -379,7 +379,7 @@ class BatchTest { BEST_BEFORE_DATE, OffsetDateTime.now(ZoneOffset.UTC), OffsetDateTime.now(ZoneOffset.UTC), - List.of() + 0L, List.of() ); assertThat(batch.id().value()).isEqualTo("batch-1"); diff --git a/backend/src/test/java/de/effigenix/domain/production/ConsumptionTest.java b/backend/src/test/java/de/effigenix/domain/production/ConsumptionTest.java index 013c6c9..c1a542a 100644 --- a/backend/src/test/java/de/effigenix/domain/production/ConsumptionTest.java +++ b/backend/src/test/java/de/effigenix/domain/production/ConsumptionTest.java @@ -111,6 +111,28 @@ class ConsumptionTest { assertThat(result.unsafeGetError()).isInstanceOf(BatchError.InvalidConsumptionQuantity.class); } + @Test + @DisplayName("should fail when quantity is null") + void should_Fail_When_QuantityNull() { + var draft = new ConsumptionDraft("input-batch-1", "article-1", null, "KILOGRAM"); + + var result = Consumption.create(draft); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(BatchError.InvalidConsumptionQuantity.class); + } + + @Test + @DisplayName("should fail when unit is null") + void should_Fail_When_UnitNull() { + var draft = new ConsumptionDraft("input-batch-1", "article-1", "10", null); + + var result = Consumption.create(draft); + + assertThat(result.isFailure()).isTrue(); + assertThat(result.unsafeGetError()).isInstanceOf(BatchError.InvalidConsumptionQuantity.class); + } + @Test @DisplayName("should fail when unit is invalid") void should_Fail_When_UnitInvalid() {