sebi/effigenix
1
0
Fork 0
mirror of https://github.com/s-frick/effigenix.git synced 2026-03-28 08:29:36 +01:00
Code Issues Releases Activity Actions

fix(inventory): Audit-Logging, testbare Zeitlogik und defensive Null-Prüfung für blockBatch/unblockBatch

Browse source 
- AuditEvents STOCK_BATCH_BLOCKED/UNBLOCKED hinzugefügt, reason wird als Audit-Details geloggt
- LocalDate.now() aus Stock.unblockBatch() entfernt, referenceDate als Parameter (Application Layer übergibt)
- Defensive Null-Prüfung für expiryDate in unblockBatch MHD-Check
- Ticket 003 erstellt zur Klärung ob reason im Domain-Model persistiert werden soll
This commit is contained in:
Sebastian Frick 2026-02-20 00:17:03 +01:00
parent e7c3258f07
commit d963d7fccc
10 changed files with 112 additions and 33 deletions
Download patch file Download diff file Expand all files Collapse all files

41
backend/docs/tickets/003-review-batch-block-reason-persistence.md Normal file
View file

@ -0,0 +1,41 @@
# Ticket 003 – Prüfung: Sperr-Grund (reason) im Domain Model persistieren?
**Datum:** 2026-02-20
**Commit:** e7c3258 (feat(inventory): Charge sperren/entsperren)
**Status:** Offen
---
## Kontext
Beim Code-Review von `blockBatch`/`unblockBatch` wurde festgestellt, dass der `reason`-Parameter
(Sperr-Grund) zwar über die API entgegengenommen und validiert wird (`@NotBlank`), aber aktuell
nur im Audit-Log als `details` gespeichert wird – nicht im Domain Model selbst.
Das DDD-Modell (`docs/mvp/ddd/07-inventory-bc.md`, Zeile 63) sieht die Signatur
`blockBatch(StockBatchId batchId, String reason)` vor, d.h. der `reason` war ursprünglich
als Teil der Domain-Logik geplant.
## Zu klären
1. **Soll `reason` in `StockBatch` persistiert werden?**
- Pro: Direkt abfragbar ohne Audit-Log-Suche, fachlich sichtbar im Bestandsüberblick
- Contra: Audit-Log reicht für Nachvollziehbarkeit (HACCP/GoBD), zusätzliches Feld = Schema-Migration
2. **Falls ja: als optionales Feld `blockReason` in `StockBatch`?**
- Nur gesetzt wenn `status == BLOCKED`
- Wird bei `unblockBatch` auf `null` zurückgesetzt
- Erfordert: Liquibase-Migration, JPA-Entity-Anpassung, Mapper-Update
3. **Domain-Event `StockBatchBlocked(StockId, StockBatchId, String reason)`?**
- Das DDD-Modell (Zeile 302) sieht dieses Event vor
- Aktuell keine Domain-Event-Infrastruktur implementiert → separates Thema
## Aktuelle Lösung
`reason` wird im Audit-Log via `AuditLogger.log(STOCK_BATCH_BLOCKED, batchId, "Reason: " + reason, actorId)` festgehalten. Damit ist die HACCP-Compliance gewährleistet.
## Empfehlung
Kurzfristig reicht der Audit-Log-Ansatz. Mittelfristig (wenn Chargen-Sperrgrund in der UI angezeigt
werden soll) sollte `blockReason` als optionales Feld in `StockBatch` aufgenommen werden.

10
backend/src/main/java/de/effigenix/application/inventory/BlockStockBatch.java
View file

@ -1,20 +1,25 @@
package de.effigenix.application.inventory;
import de.effigenix.application.inventory.command.BlockStockBatchCommand;
import de.effigenix.application.usermanagement.AuditEvent;
import de.effigenix.application.usermanagement.AuditLogger;
import de.effigenix.domain.inventory.*;
import de.effigenix.shared.common.Result;
import de.effigenix.shared.security.ActorId;
import org.springframework.transaction.annotation.Transactional;
@Transactional
public class BlockStockBatch {
private final StockRepository stockRepository;
private final AuditLogger auditLogger;
public BlockStockBatch(StockRepository stockRepository) {
public BlockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) {
this.stockRepository = stockRepository;
this.auditLogger = auditLogger;
}
public Result<StockError, Void> execute(BlockStockBatchCommand cmd) {
public Result<StockError, Void> execute(BlockStockBatchCommand cmd, ActorId performedBy) {
// 1. Stock laden
Stock stock;
switch (stockRepository.findById(StockId.of(cmd.stockId()))) {
@ -41,6 +46,7 @@ public class BlockStockBatch {
case Result.Success(var ignored) -> { }
}
auditLogger.log(AuditEvent.STOCK_BATCH_BLOCKED, cmd.batchId(), "Reason: " + cmd.reason(), performedBy);
return Result.success(null);
}
}

14
backend/src/main/java/de/effigenix/application/inventory/UnblockStockBatch.java
View file

@ -1,20 +1,27 @@
package de.effigenix.application.inventory;
import de.effigenix.application.inventory.command.UnblockStockBatchCommand;
import de.effigenix.application.usermanagement.AuditEvent;
import de.effigenix.application.usermanagement.AuditLogger;
import de.effigenix.domain.inventory.*;
import de.effigenix.shared.common.Result;
import de.effigenix.shared.security.ActorId;
import org.springframework.transaction.annotation.Transactional;
import java.time.LocalDate;
@Transactional
public class UnblockStockBatch {
private final StockRepository stockRepository;
private final AuditLogger auditLogger;
public UnblockStockBatch(StockRepository stockRepository) {
public UnblockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) {
this.stockRepository = stockRepository;
this.auditLogger = auditLogger;
}
public Result<StockError, Void> execute(UnblockStockBatchCommand cmd) {
public Result<StockError, Void> execute(UnblockStockBatchCommand cmd, ActorId performedBy) {
// 1. Stock laden
Stock stock;
switch (stockRepository.findById(StockId.of(cmd.stockId()))) {
@ -29,7 +36,7 @@ public class UnblockStockBatch {
}
// 2. Batch entsperren (Domain validiert)
switch (stock.unblockBatch(StockBatchId.of(cmd.batchId()))) {
switch (stock.unblockBatch(StockBatchId.of(cmd.batchId()), LocalDate.now())) {
case Result.Failure(var err) -> { return Result.failure(err); }
case Result.Success(var ignored) -> { }
}
@ -41,6 +48,7 @@ public class UnblockStockBatch {
case Result.Success(var ignored) -> { }
}
auditLogger.log(AuditEvent.STOCK_BATCH_UNBLOCKED, cmd.batchId(), performedBy);
return Result.success(null);
}
}

2
backend/src/main/java/de/effigenix/application/usermanagement/AuditEvent.java
View file

@ -46,6 +46,8 @@ public enum AuditEvent {
STOCK_ADJUSTED,
STOCK_MOVEMENT_RECORDED,
INVENTORY_COUNT_PERFORMED,
STOCK_BATCH_BLOCKED,
STOCK_BATCH_UNBLOCKED,
// ==================== Procurement BC ====================
PURCHASE_ORDER_CREATED,

6
backend/src/main/java/de/effigenix/domain/inventory/Stock.java
View file

@ -173,7 +173,7 @@ public class Stock {
return Result.success(null);
}
public Result<StockError, Void> unblockBatch(StockBatchId batchId) {
public Result<StockError, Void> unblockBatch(StockBatchId batchId, LocalDate referenceDate) {
StockBatch batch = batches.stream()
.filter(b -> b.id().equals(batchId))
.findFirst()
@ -186,8 +186,8 @@ public class Stock {
}
StockBatchStatus newStatus = StockBatchStatus.AVAILABLE;
if (minimumShelfLife != null) {
LocalDate threshold = LocalDate.now().plusDays(minimumShelfLife.days());
if (minimumShelfLife != null && batch.expiryDate() != null) {
LocalDate threshold = referenceDate.plusDays(minimumShelfLife.days());
if (batch.expiryDate().isBefore(threshold)) {
newStatus = StockBatchStatus.EXPIRING_SOON;
}

9
backend/src/main/java/de/effigenix/infrastructure/config/InventoryUseCaseConfiguration.java
View file

@ -10,6 +10,7 @@ import de.effigenix.application.inventory.CreateStorageLocation;
import de.effigenix.application.inventory.DeactivateStorageLocation;
import de.effigenix.application.inventory.ListStorageLocations;
import de.effigenix.application.inventory.UpdateStorageLocation;
import de.effigenix.application.usermanagement.AuditLogger;
import de.effigenix.domain.inventory.StockRepository;
import de.effigenix.domain.inventory.StorageLocationRepository;
import org.springframework.context.annotation.Bean;
@ -63,12 +64,12 @@ public class InventoryUseCaseConfiguration {
}
@Bean
public BlockStockBatch blockStockBatch(StockRepository stockRepository) {
return new BlockStockBatch(stockRepository);
public BlockStockBatch blockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) {
return new BlockStockBatch(stockRepository, auditLogger);
}
@Bean
public UnblockStockBatch unblockStockBatch(StockRepository stockRepository) {
return new UnblockStockBatch(stockRepository);
public UnblockStockBatch unblockStockBatch(StockRepository stockRepository, AuditLogger auditLogger) {
return new UnblockStockBatch(stockRepository, auditLogger);
}
}

5
backend/src/main/java/de/effigenix/infrastructure/inventory/web/controller/StockController.java
View file

@ -11,6 +11,7 @@ import de.effigenix.application.inventory.command.CreateStockCommand;
import de.effigenix.application.inventory.command.RemoveStockBatchCommand;
import de.effigenix.application.inventory.command.UnblockStockBatchCommand;
import de.effigenix.domain.inventory.StockError;
import de.effigenix.shared.security.ActorId;
import de.effigenix.infrastructure.inventory.web.dto.AddStockBatchRequest;
import de.effigenix.infrastructure.inventory.web.dto.BlockStockBatchRequest;
import de.effigenix.infrastructure.inventory.web.dto.CreateStockRequest;
@ -136,7 +137,7 @@ public class StockController {
logger.info("Blocking batch {} of stock {} by actor: {}", batchId, stockId, authentication.getName());
var cmd = new BlockStockBatchCommand(stockId, batchId, request.reason());
var result = blockStockBatch.execute(cmd);
var result = blockStockBatch.execute(cmd, ActorId.of(authentication.getName()));
if (result.isFailure()) {
throw new StockDomainErrorException(result.unsafeGetError());
@ -156,7 +157,7 @@ public class StockController {
logger.info("Unblocking batch {} of stock {} by actor: {}", batchId, stockId, authentication.getName());
var cmd = new UnblockStockBatchCommand(stockId, batchId);
var result = unblockStockBatch.execute(cmd);
var result = unblockStockBatch.execute(cmd, ActorId.of(authentication.getName()));
if (result.isFailure()) {
throw new StockDomainErrorException(result.unsafeGetError());

24
backend/src/test/java/de/effigenix/application/inventory/BlockStockBatchTest.java
View file

@ -1,11 +1,13 @@
package de.effigenix.application.inventory;
import de.effigenix.application.inventory.command.BlockStockBatchCommand;
import de.effigenix.application.usermanagement.AuditLogger;
import de.effigenix.domain.inventory.*;
import de.effigenix.shared.common.Quantity;
import de.effigenix.shared.common.RepositoryError;
import de.effigenix.shared.common.Result;
import de.effigenix.shared.common.UnitOfMeasure;
import de.effigenix.shared.security.ActorId;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
@ -29,15 +31,17 @@ import static org.mockito.Mockito.*;
class BlockStockBatchTest {
@Mock private StockRepository stockRepository;
@Mock private AuditLogger auditLogger;
private BlockStockBatch blockStockBatch;
private StockBatchId batchId;
private Stock existingStock;
private BlockStockBatchCommand validCommand;
private final ActorId actor = ActorId.of("test-user");
@BeforeEach
void setUp() {
blockStockBatch = new BlockStockBatch(stockRepository);
blockStockBatch = new BlockStockBatch(stockRepository, auditLogger);
batchId = StockBatchId.of("batch-1");
var batch = StockBatch.reconstitute(
@ -67,11 +71,12 @@ class BlockStockBatchTest {
.thenReturn(Result.success(Optional.of(existingStock)));
when(stockRepository.save(any())).thenReturn(Result.success(null));
var result = blockStockBatch.execute(validCommand);
var result = blockStockBatch.execute(validCommand, actor);
assertThat(result.isSuccess()).isTrue();
verify(stockRepository).save(existingStock);
assertThat(existingStock.batches().getFirst().status()).isEqualTo(StockBatchStatus.BLOCKED);
verify(auditLogger).log(any(), eq("batch-1"), eq("Reason: Quality issue"), eq(actor));
}
@Test
@ -80,11 +85,12 @@ class BlockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.success(Optional.empty()));
var result = blockStockBatch.execute(validCommand);
var result = blockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.StockNotFound.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -93,11 +99,12 @@ class BlockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost")));
var result = blockStockBatch.execute(validCommand);
var result = blockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -108,10 +115,11 @@ class BlockStockBatchTest {
when(stockRepository.save(any()))
.thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost")));
var result = blockStockBatch.execute(validCommand);
var result = blockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class);
verifyNoInteractions(auditLogger);
}
@Test
@ -121,11 +129,12 @@ class BlockStockBatchTest {
.thenReturn(Result.success(Optional.of(existingStock)));
var cmd = new BlockStockBatchCommand("stock-1", "nonexistent", "Quality issue");
var result = blockStockBatch.execute(cmd);
var result = blockStockBatch.execute(cmd, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -149,10 +158,11 @@ class BlockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.success(Optional.of(stock)));
var result = blockStockBatch.execute(validCommand);
var result = blockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchAlreadyBlocked.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
}

24
backend/src/test/java/de/effigenix/application/inventory/UnblockStockBatchTest.java
View file

@ -1,11 +1,13 @@
package de.effigenix.application.inventory;
import de.effigenix.application.inventory.command.UnblockStockBatchCommand;
import de.effigenix.application.usermanagement.AuditLogger;
import de.effigenix.domain.inventory.*;
import de.effigenix.shared.common.Quantity;
import de.effigenix.shared.common.RepositoryError;
import de.effigenix.shared.common.Result;
import de.effigenix.shared.common.UnitOfMeasure;
import de.effigenix.shared.security.ActorId;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
@ -29,15 +31,17 @@ import static org.mockito.Mockito.*;
class UnblockStockBatchTest {
@Mock private StockRepository stockRepository;
@Mock private AuditLogger auditLogger;
private UnblockStockBatch unblockStockBatch;
private StockBatchId batchId;
private Stock existingStock;
private UnblockStockBatchCommand validCommand;
private final ActorId actor = ActorId.of("test-user");
@BeforeEach
void setUp() {
unblockStockBatch = new UnblockStockBatch(stockRepository);
unblockStockBatch = new UnblockStockBatch(stockRepository, auditLogger);
batchId = StockBatchId.of("batch-1");
var batch = StockBatch.reconstitute(
@ -67,11 +71,12 @@ class UnblockStockBatchTest {
.thenReturn(Result.success(Optional.of(existingStock)));
when(stockRepository.save(any())).thenReturn(Result.success(null));
var result = unblockStockBatch.execute(validCommand);
var result = unblockStockBatch.execute(validCommand, actor);
assertThat(result.isSuccess()).isTrue();
verify(stockRepository).save(existingStock);
assertThat(existingStock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE);
verify(auditLogger).log(any(), eq("batch-1"), eq(actor));
}
@Test
@ -80,11 +85,12 @@ class UnblockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.success(Optional.empty()));
var result = unblockStockBatch.execute(validCommand);
var result = unblockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.StockNotFound.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -93,11 +99,12 @@ class UnblockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost")));
var result = unblockStockBatch.execute(validCommand);
var result = unblockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -108,10 +115,11 @@ class UnblockStockBatchTest {
when(stockRepository.save(any()))
.thenReturn(Result.failure(new RepositoryError.DatabaseError("connection lost")));
var result = unblockStockBatch.execute(validCommand);
var result = unblockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.RepositoryFailure.class);
verifyNoInteractions(auditLogger);
}
@Test
@ -121,11 +129,12 @@ class UnblockStockBatchTest {
.thenReturn(Result.success(Optional.of(existingStock)));
var cmd = new UnblockStockBatchCommand("stock-1", "nonexistent");
var result = unblockStockBatch.execute(cmd);
var result = unblockStockBatch.execute(cmd, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
@Test
@ -149,10 +158,11 @@ class UnblockStockBatchTest {
when(stockRepository.findById(StockId.of("stock-1")))
.thenReturn(Result.success(Optional.of(stock)));
var result = unblockStockBatch.execute(validCommand);
var result = unblockStockBatch.execute(validCommand, actor);
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotBlocked.class);
verify(stockRepository, never()).save(any());
verifyNoInteractions(auditLogger);
}
}

10
backend/src/test/java/de/effigenix/domain/inventory/StockTest.java
View file

@ -513,7 +513,7 @@ class StockTest {
var stock = createStockWithBatch("10", UnitOfMeasure.KILOGRAM, StockBatchStatus.BLOCKED);
var batchId = stock.batches().getFirst().id();
var result = stock.unblockBatch(batchId);
var result = stock.unblockBatch(batchId, LocalDate.now());
assertThat(result.isSuccess()).isTrue();
assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE);
@ -539,7 +539,7 @@ class StockTest {
);
var batchId = stock.batches().getFirst().id();
var result = stock.unblockBatch(batchId);
var result = stock.unblockBatch(batchId, LocalDate.now());
assertThat(result.isSuccess()).isTrue();
assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.EXPIRING_SOON);
@ -565,7 +565,7 @@ class StockTest {
);
var batchId = stock.batches().getFirst().id();
var result = stock.unblockBatch(batchId);
var result = stock.unblockBatch(batchId, LocalDate.now());
assertThat(result.isSuccess()).isTrue();
assertThat(stock.batches().getFirst().status()).isEqualTo(StockBatchStatus.AVAILABLE);
@ -577,7 +577,7 @@ class StockTest {
var stock = createStockWithBatch("10", UnitOfMeasure.KILOGRAM, StockBatchStatus.AVAILABLE);
var batchId = stock.batches().getFirst().id();
var result = stock.unblockBatch(batchId);
var result = stock.unblockBatch(batchId, LocalDate.now());
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotBlocked.class);
@ -588,7 +588,7 @@ class StockTest {
void shouldFailWhenBatchNotFound() {
var stock = createValidStock();
var result = stock.unblockBatch(StockBatchId.of("nonexistent"));
var result = stock.unblockBatch(StockBatchId.of("nonexistent"), LocalDate.now());
assertThat(result.isFailure()).isTrue();
assertThat(result.unsafeGetError()).isInstanceOf(StockError.BatchNotFound.class);