1
0
Fork 0
mirror of https://github.com/s-frick/effigenix.git synced 2026-03-28 10:29:35 +01:00

fix(production): JPA-Save-Pattern, Optimistic Locking und Domain-Validierung

- 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
This commit is contained in:
Sebastian Frick 2026-02-20 16:52:11 +01:00
parent a9f5956812
commit 9eb9c93fb7
17 changed files with 133 additions and 37 deletions

View file

@ -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<Consumption> consumptions;
private Batch(
@ -49,6 +50,7 @@ public class Batch {
LocalDate bestBeforeDate,
OffsetDateTime createdAt,
OffsetDateTime updatedAt,
long version,
List<Consumption> 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<Consumption> 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<Consumption> consumptions() { return Collections.unmodifiableList(consumptions); }
}

View file

@ -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());

View file

@ -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) {}

View file

@ -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) {}

View file

@ -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<ConsumptionEntity> 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<ConsumptionEntity> getConsumptions() { return consumptions; }
public void setConsumptions(List<ConsumptionEntity> consumptions) { this.consumptions = consumptions; }
public void setStatus(String status) { this.status = status; }
public void setUpdatedAt(OffsetDateTime updatedAt) { this.updatedAt = updatedAt; }
}

View file

@ -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<ConsumptionEntity> 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<String> 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<Consumption> 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()
);
}
}

View file

@ -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<RepositoryError, Void> save(Batch batch) {
try {
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()));

View file

@ -21,4 +21,7 @@ public sealed interface RepositoryError {
record DatabaseError(String message) implements RepositoryError {
}
record ConcurrentModification(String message) implements RepositoryError {
}
}

View file

@ -0,0 +1,16 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog
xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog
http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
<changeSet id="020-add-version-to-batches" author="effigenix">
<addColumn tableName="batches">
<column name="version" type="bigint" defaultValueNumeric="0">
<constraints nullable="false"/>
</column>
</addColumn>
</changeSet>
</databaseChangeLog>

View file

@ -24,5 +24,6 @@
<include file="db/changelog/changes/017-timestamps-to-timestamptz.xml"/>
<include file="db/changelog/changes/018-add-article-id-to-recipes.xml"/>
<include file="db/changelog/changes/019-create-batch-consumptions-table.xml"/>
<include file="db/changelog/changes/020-add-version-to-batches.xml"/>
</databaseChangeLog>

View file

@ -53,7 +53,7 @@ class FindBatchByNumberTest {
LocalDate.of(2026, 6, 1),
OffsetDateTime.now(ZoneOffset.UTC),
OffsetDateTime.now(ZoneOffset.UTC),
List.of()
0L, List.of()
);
}

View file

@ -51,7 +51,7 @@ class GetBatchTest {
LocalDate.of(2026, 6, 1),
OffsetDateTime.now(ZoneOffset.UTC),
OffsetDateTime.now(ZoneOffset.UTC),
List.of()
0L, List.of()
);
}

View file

@ -55,7 +55,7 @@ class ListBatchesTest {
LocalDate.of(2026, 6, 1),
OffsetDateTime.now(ZoneOffset.UTC),
OffsetDateTime.now(ZoneOffset.UTC),
List.of()
0L, List.of()
);
}

View file

@ -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()
);
}

View file

@ -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()
);
}

View file

@ -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");

View file

@ -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() {