mirror of
https://github.com/s-frick/effigenix.git
synced 2026-03-28 08:29:36 +01:00
fix(production): N+1-Query in traceForward durch Level-by-Level BFS ersetzen
traceForward aus BatchTraceabilityService in BatchRepository verschoben. Statt einer Query pro Knoten (N+1) wird jetzt eine IN(:parentIds)-Query pro Tiefenebene ausgeführt (max. maxDepth Queries statt N).
This commit is contained in:
Sebastian Frick
parent
ddb674d618
commit
8948103957
6 changed files with 130 additions and 174 deletions
|
|
@ -31,5 +31,7 @@ public interface BatchRepository {
|
|||
|
||||
Result<RepositoryError, List<Batch>> findByInputBatchId(BatchId inputBatchId);
|
||||
|
||||
Result<RepositoryError, List<TracedBatch>> traceForward(BatchId startBatchId, int maxDepth);
|
||||
|
||||
Result<RepositoryError, Void> save(Batch batch);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -2,11 +2,7 @@ package de.effigenix.domain.production;
|
|||
|
||||
import de.effigenix.shared.common.Result;
|
||||
|
||||
import java.util.ArrayDeque;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Set;
|
||||
|
||||
public class BatchTraceabilityService {
|
||||
|
||||
|
|
@ -33,35 +29,15 @@ public class BatchTraceabilityService {
|
|||
}
|
||||
}
|
||||
|
||||
List<TracedBatch> result = new ArrayList<>();
|
||||
Set<String> visited = new HashSet<>();
|
||||
visited.add(startBatchId.value());
|
||||
|
||||
record BfsEntry(BatchId batchId, int depth) {}
|
||||
var queue = new ArrayDeque<BfsEntry>();
|
||||
queue.add(new BfsEntry(startBatchId, 0));
|
||||
|
||||
while (!queue.isEmpty()) {
|
||||
var entry = queue.poll();
|
||||
if (entry.depth() >= maxDepth) {
|
||||
continue;
|
||||
if (maxDepth <= 0) {
|
||||
return Result.success(List.of());
|
||||
}
|
||||
|
||||
switch (batchRepository.findByInputBatchId(entry.batchId())) {
|
||||
return switch (batchRepository.traceForward(startBatchId, maxDepth)) {
|
||||
case Result.Failure(var err) ->
|
||||
{ return Result.failure(new BatchError.RepositoryFailure(err.message())); }
|
||||
case Result.Success(var children) -> {
|
||||
int childDepth = entry.depth() + 1;
|
||||
for (Batch child : children) {
|
||||
if (visited.add(child.id().value())) {
|
||||
result.add(new TracedBatch(child, childDepth));
|
||||
queue.add(new BfsEntry(child.id(), childDepth));
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return Result.success(result);
|
||||
Result.failure(new BatchError.RepositoryFailure(err.message()));
|
||||
case Result.Success(var traced) ->
|
||||
Result.success(traced);
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -17,6 +17,8 @@ import java.sql.ResultSet;
|
|||
import java.sql.SQLException;
|
||||
import java.time.LocalDate;
|
||||
import java.time.OffsetDateTime;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashSet;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
|
||||
|
|
@ -210,6 +212,44 @@ public class JdbcBatchRepository implements BatchRepository {
|
|||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Result<RepositoryError, List<TracedBatch>> traceForward(BatchId startBatchId, int maxDepth) {
|
||||
try {
|
||||
String startId = startBatchId.value();
|
||||
var results = new ArrayList<TracedBatch>();
|
||||
var visited = new HashSet<String>();
|
||||
visited.add(startId);
|
||||
|
||||
var currentLevel = List.of(startId);
|
||||
for (int depth = 1; depth <= maxDepth && !currentLevel.isEmpty(); depth++) {
|
||||
int currentDepth = depth;
|
||||
var children = jdbc.sql("""
|
||||
SELECT DISTINCT b.*
|
||||
FROM batches b
|
||||
JOIN batch_consumptions bc ON b.id = bc.batch_id
|
||||
WHERE bc.input_batch_id IN (:parentIds)
|
||||
""")
|
||||
.param("parentIds", currentLevel)
|
||||
.query(this::mapBatchRow)
|
||||
.list();
|
||||
|
||||
var nextLevel = new ArrayList<String>();
|
||||
for (Batch child : children) {
|
||||
if (visited.add(child.id().value())) {
|
||||
results.add(new TracedBatch(child, currentDepth));
|
||||
nextLevel.add(child.id().value());
|
||||
}
|
||||
}
|
||||
currentLevel = nextLevel;
|
||||
}
|
||||
|
||||
return Result.success(results);
|
||||
} catch (Exception e) {
|
||||
logger.trace("Database error in traceForward", e);
|
||||
return Result.failure(new RepositoryError.DatabaseError(e.getMessage()));
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
public Result<RepositoryError, Void> save(Batch batch) {
|
||||
try {
|
||||
|
|
@ -294,7 +334,7 @@ public class JdbcBatchRepository implements BatchRepository {
|
|||
.query((rs, rowNum) -> rs.getString("id"))
|
||||
.list();
|
||||
|
||||
var existingIdSet = new java.util.HashSet<>(existingIds);
|
||||
var existingIdSet = new HashSet<>(existingIds);
|
||||
|
||||
for (Consumption c : batch.consumptions()) {
|
||||
if (!existingIdSet.contains(c.id().value())) {
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import de.effigenix.domain.production.BatchNumber;
|
|||
import de.effigenix.domain.production.BatchRepository;
|
||||
import de.effigenix.domain.production.BatchStatus;
|
||||
import de.effigenix.domain.production.RecipeId;
|
||||
import de.effigenix.domain.production.TracedBatch;
|
||||
import de.effigenix.shared.common.RepositoryError;
|
||||
import de.effigenix.shared.common.Result;
|
||||
import org.springframework.context.annotation.Profile;
|
||||
|
|
@ -77,6 +78,11 @@ public class StubBatchRepository implements BatchRepository {
|
|||
return Result.failure(STUB_ERROR);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Result<RepositoryError, List<TracedBatch>> traceForward(BatchId startBatchId, int maxDepth) {
|
||||
return Result.failure(STUB_ERROR);
|
||||
}
|
||||
|
||||
@Override
|
||||
public Result<RepositoryError, Void> save(Batch batch) {
|
||||
return Result.failure(STUB_ERROR);
|
||||
|
|
|
|||
|
|
@ -96,7 +96,7 @@ class BatchTraceabilityServiceFuzzTest {
|
|||
|
||||
/**
|
||||
* In-memory BatchRepository that serves a pre-built graph.
|
||||
* Only findById and findByInputBatchId are implemented (the rest throw).
|
||||
* Implements traceForward with BFS (mirrors the contract of the recursive CTE).
|
||||
*/
|
||||
private static class InMemoryGraphRepository implements BatchRepository {
|
||||
|
||||
|
|
@ -122,6 +122,33 @@ class BatchTraceabilityServiceFuzzTest {
|
|||
return Result.success(children.stream().map(BatchTraceabilityServiceFuzzTest::makeBatch).toList());
|
||||
}
|
||||
|
||||
@Override
|
||||
public Result<RepositoryError, List<TracedBatch>> traceForward(BatchId startBatchId, int maxDepth) {
|
||||
List<TracedBatch> result = new ArrayList<>();
|
||||
Set<String> visited = new HashSet<>();
|
||||
visited.add(startBatchId.value());
|
||||
|
||||
record BfsEntry(String batchId, int depth) {}
|
||||
var queue = new ArrayDeque<BfsEntry>();
|
||||
queue.add(new BfsEntry(startBatchId.value(), 0));
|
||||
|
||||
while (!queue.isEmpty()) {
|
||||
var entry = queue.poll();
|
||||
if (entry.depth() >= maxDepth) continue;
|
||||
|
||||
var children = adjacency.getOrDefault(entry.batchId(), List.of());
|
||||
int childDepth = entry.depth() + 1;
|
||||
for (String childId : children) {
|
||||
if (visited.add(childId)) {
|
||||
result.add(new TracedBatch(makeBatch(childId), childDepth));
|
||||
queue.add(new BfsEntry(childId, childDepth));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return Result.success(result);
|
||||
}
|
||||
|
||||
// --- Remaining methods are not used by BatchTraceabilityService ---
|
||||
|
||||
@Override public Result<RepositoryError, List<Batch>> findAll() { throw new UnsupportedOperationException(); }
|
||||
|
|
|
|||
|
|
@ -56,88 +56,6 @@ class BatchTraceabilityServiceTest {
|
|||
@DisplayName("traceForward")
|
||||
class TraceForward {
|
||||
|
||||
@Test
|
||||
@DisplayName("should return empty list when no downstream batches exist")
|
||||
void should_ReturnEmptyList_When_NoDownstreamBatches() {
|
||||
var startId = BatchId.of("start-batch");
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start-batch"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of()));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
assertThat(result.unsafeGetValue()).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should return direct downstream batches at depth 1")
|
||||
void should_ReturnDirectDownstream_AtDepth1() {
|
||||
var startId = BatchId.of("start-batch");
|
||||
var child1 = sampleBatch("child-1");
|
||||
var child2 = sampleBatch("child-2");
|
||||
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start-batch"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(child1, child2)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("child-1"))).thenReturn(Result.success(List.of()));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("child-2"))).thenReturn(Result.success(List.of()));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
var traced = result.unsafeGetValue();
|
||||
assertThat(traced).hasSize(2);
|
||||
assertThat(traced).allMatch(t -> t.depth() == 1);
|
||||
assertThat(traced).extracting(t -> t.batch().id().value())
|
||||
.containsExactlyInAnyOrder("child-1", "child-2");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should traverse multi-level chain with correct depths")
|
||||
void should_TraverseMultiLevel_WithCorrectDepths() {
|
||||
var startId = BatchId.of("start");
|
||||
var level1 = sampleBatch("level-1");
|
||||
var level2 = sampleBatch("level-2");
|
||||
var level3 = sampleBatch("level-3");
|
||||
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(level1)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("level-1"))).thenReturn(Result.success(List.of(level2)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("level-2"))).thenReturn(Result.success(List.of(level3)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("level-3"))).thenReturn(Result.success(List.of()));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
var traced = result.unsafeGetValue();
|
||||
assertThat(traced).hasSize(3);
|
||||
assertThat(traced.get(0).batch().id().value()).isEqualTo("level-1");
|
||||
assertThat(traced.get(0).depth()).isEqualTo(1);
|
||||
assertThat(traced.get(1).batch().id().value()).isEqualTo("level-2");
|
||||
assertThat(traced.get(1).depth()).isEqualTo(2);
|
||||
assertThat(traced.get(2).batch().id().value()).isEqualTo("level-3");
|
||||
assertThat(traced.get(2).depth()).isEqualTo(3);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should detect cycles and terminate without endless loop")
|
||||
void should_DetectCycles_AndTerminate() {
|
||||
var startId = BatchId.of("start");
|
||||
var child = sampleBatch("child");
|
||||
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(child)));
|
||||
// child references back to start — cycle
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("child")))
|
||||
.thenReturn(Result.success(List.of(sampleBatch("start"))));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
var traced = result.unsafeGetValue();
|
||||
assertThat(traced).hasSize(1);
|
||||
assertThat(traced.get(0).batch().id().value()).isEqualTo("child");
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should fail with BatchNotFound when start batch does not exist")
|
||||
void should_FailWithBatchNotFound_When_StartBatchDoesNotExist() {
|
||||
|
|
@ -148,26 +66,7 @@ class BatchTraceabilityServiceTest {
|
|||
|
||||
assertThat(result.isFailure()).isTrue();
|
||||
assertThat(result.unsafeGetError()).isInstanceOf(BatchError.BatchNotFound.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should stop at max depth limit")
|
||||
void should_StopAtMaxDepth() {
|
||||
var startId = BatchId.of("start");
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
|
||||
var level1 = sampleBatch("level-1");
|
||||
var level2 = sampleBatch("level-2");
|
||||
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(level1)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("level-1"))).thenReturn(Result.success(List.of(level2)));
|
||||
|
||||
var result = service.traceForward(startId, 2);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
var traced = result.unsafeGetValue();
|
||||
assertThat(traced).hasSize(2);
|
||||
assertThat(traced).extracting(TracedBatch::depth).containsExactly(1, 2);
|
||||
verify(batchRepository, never()).traceForward(any(), anyInt());
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -181,46 +80,40 @@ class BatchTraceabilityServiceTest {
|
|||
|
||||
assertThat(result.isFailure()).isTrue();
|
||||
assertThat(result.unsafeGetError()).isInstanceOf(BatchError.RepositoryFailure.class);
|
||||
verify(batchRepository, never()).traceForward(any(), anyInt());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should fail with RepositoryFailure when findByInputBatchId returns error")
|
||||
void should_FailWithRepositoryFailure_When_FindByInputBatchIdReturnsError() {
|
||||
@DisplayName("should delegate to repository traceForward when start batch exists")
|
||||
void should_DelegateToRepository_When_StartBatchExists() {
|
||||
var startId = BatchId.of("start");
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
when(batchRepository.findByInputBatchId(startId))
|
||||
.thenReturn(Result.failure(new RepositoryError.DatabaseError("timeout")));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isFailure()).isTrue();
|
||||
assertThat(result.unsafeGetError()).isInstanceOf(BatchError.RepositoryFailure.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should deduplicate shared child in diamond graph")
|
||||
void should_DeduplicateSharedChild_InDiamondGraph() {
|
||||
// A → B, A → C, B → D, C → D ⇒ D appears only once
|
||||
var startId = BatchId.of("A");
|
||||
var batchB = sampleBatch("B");
|
||||
var batchC = sampleBatch("C");
|
||||
var batchD = sampleBatch("D");
|
||||
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("A"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(batchB, batchC)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("B"))).thenReturn(Result.success(List.of(batchD)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("C"))).thenReturn(Result.success(List.of(sampleBatch("D"))));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("D"))).thenReturn(Result.success(List.of()));
|
||||
var traced = List.of(
|
||||
new TracedBatch(sampleBatch("child-1"), 1),
|
||||
new TracedBatch(sampleBatch("child-2"), 1)
|
||||
);
|
||||
when(batchRepository.traceForward(startId, BatchTraceabilityService.DEFAULT_MAX_DEPTH))
|
||||
.thenReturn(Result.success(traced));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
var traced = result.unsafeGetValue();
|
||||
assertThat(traced).extracting(t -> t.batch().id().value())
|
||||
.containsExactlyInAnyOrder("B", "C", "D");
|
||||
// D must appear only once despite being reachable from B and C
|
||||
assertThat(traced.stream().filter(t -> t.batch().id().value().equals("D")).count())
|
||||
.isEqualTo(1);
|
||||
assertThat(result.unsafeGetValue()).hasSize(2);
|
||||
verify(batchRepository).traceForward(startId, BatchTraceabilityService.DEFAULT_MAX_DEPTH);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should pass custom maxDepth to repository")
|
||||
void should_PassCustomMaxDepth_ToRepository() {
|
||||
var startId = BatchId.of("start");
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
when(batchRepository.traceForward(startId, 5)).thenReturn(Result.success(List.of()));
|
||||
|
||||
var result = service.traceForward(startId, 5);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
verify(batchRepository).traceForward(startId, 5);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
@ -233,23 +126,35 @@ class BatchTraceabilityServiceTest {
|
|||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
assertThat(result.unsafeGetValue()).isEmpty();
|
||||
verify(batchRepository, never()).traceForward(any(), anyInt());
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should not include start batch in result")
|
||||
void should_NotIncludeStartBatch_InResult() {
|
||||
@DisplayName("should fail with RepositoryFailure when traceForward returns error")
|
||||
void should_FailWithRepositoryFailure_When_TraceForwardReturnsError() {
|
||||
var startId = BatchId.of("start");
|
||||
var child = sampleBatch("child");
|
||||
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start"))));
|
||||
when(batchRepository.findByInputBatchId(startId)).thenReturn(Result.success(List.of(child)));
|
||||
when(batchRepository.findByInputBatchId(BatchId.of("child"))).thenReturn(Result.success(List.of()));
|
||||
when(batchRepository.traceForward(startId, BatchTraceabilityService.DEFAULT_MAX_DEPTH))
|
||||
.thenReturn(Result.failure(new RepositoryError.DatabaseError("timeout")));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isFailure()).isTrue();
|
||||
assertThat(result.unsafeGetError()).isInstanceOf(BatchError.RepositoryFailure.class);
|
||||
}
|
||||
|
||||
@Test
|
||||
@DisplayName("should return empty list when no downstream batches exist")
|
||||
void should_ReturnEmptyList_When_NoDownstreamBatches() {
|
||||
var startId = BatchId.of("start-batch");
|
||||
when(batchRepository.findById(startId)).thenReturn(Result.success(Optional.of(sampleBatch("start-batch"))));
|
||||
when(batchRepository.traceForward(startId, BatchTraceabilityService.DEFAULT_MAX_DEPTH))
|
||||
.thenReturn(Result.success(List.of()));
|
||||
|
||||
var result = service.traceForward(startId);
|
||||
|
||||
assertThat(result.isSuccess()).isTrue();
|
||||
assertThat(result.unsafeGetValue()).extracting(t -> t.batch().id().value())
|
||||
.doesNotContain("start");
|
||||
assertThat(result.unsafeGetValue()).isEmpty();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue