feat(import): warn on generation monotonicity violations (#689)
Inject RelationshipService into CanonicalImportOrchestrator and walk PARENT_OF edges in the family network after both person loaders finish (before documents). For every edge where child.generation is set and not strictly deeper than parent.generation, log a WARN — soft check, never fails the batch. Reads through getFamilyNetwork() per the layering rule (orchestrator never touches PersonRelationshipRepository directly). Curators see the warning in the import log; the rest of the pipeline is unaffected so data with curatorial gaps still loads cleanly. Refs #689 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
@@ -4,13 +4,21 @@ import lombok.RequiredArgsConstructor;
|
|||||||
import lombok.extern.slf4j.Slf4j;
|
import lombok.extern.slf4j.Slf4j;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
import org.raddatz.familienarchiv.exception.ErrorCode;
|
import org.raddatz.familienarchiv.exception.ErrorCode;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.RelationType;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.RelationshipService;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.NetworkDTO;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.PersonNodeDTO;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.RelationshipDTO;
|
||||||
import org.springframework.beans.factory.annotation.Value;
|
import org.springframework.beans.factory.annotation.Value;
|
||||||
import org.springframework.scheduling.annotation.Async;
|
import org.springframework.scheduling.annotation.Async;
|
||||||
import org.springframework.stereotype.Service;
|
import org.springframework.stereotype.Service;
|
||||||
|
|
||||||
import java.io.File;
|
import java.io.File;
|
||||||
import java.time.LocalDateTime;
|
import java.time.LocalDateTime;
|
||||||
|
import java.util.HashMap;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.Map;
|
||||||
|
import java.util.UUID;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Runs the four canonical loaders in their real dependency order — encoded explicitly
|
* Runs the four canonical loaders in their real dependency order — encoded explicitly
|
||||||
@@ -34,6 +42,7 @@ public class CanonicalImportOrchestrator {
|
|||||||
private final PersonRegisterImporter personRegisterImporter;
|
private final PersonRegisterImporter personRegisterImporter;
|
||||||
private final PersonTreeImporter personTreeImporter;
|
private final PersonTreeImporter personTreeImporter;
|
||||||
private final DocumentImporter documentImporter;
|
private final DocumentImporter documentImporter;
|
||||||
|
private final RelationshipService relationshipService;
|
||||||
|
|
||||||
@Value("${app.import.dir:/import}")
|
@Value("${app.import.dir:/import}")
|
||||||
private String canonicalDir;
|
private String canonicalDir;
|
||||||
@@ -67,6 +76,7 @@ public class CanonicalImportOrchestrator {
|
|||||||
tagTreeImporter.load(tagTree);
|
tagTreeImporter.load(tagTree);
|
||||||
personRegisterImporter.load(persons);
|
personRegisterImporter.load(persons);
|
||||||
personTreeImporter.load(personsTree);
|
personTreeImporter.load(personsTree);
|
||||||
|
warnOnGenerationMonotonicityViolations();
|
||||||
DocumentImporter.LoadResult result = documentImporter.load(documents);
|
DocumentImporter.LoadResult result = documentImporter.load(documents);
|
||||||
|
|
||||||
currentStatus = new ImportStatus(ImportStatus.State.DONE, "IMPORT_DONE",
|
currentStatus = new ImportStatus(ImportStatus.State.DONE, "IMPORT_DONE",
|
||||||
@@ -91,4 +101,31 @@ public class CanonicalImportOrchestrator {
|
|||||||
}
|
}
|
||||||
return artifact;
|
return artifact;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Walks every PARENT_OF edge in the family graph and logs a WARN whenever a child's
|
||||||
|
* generation is not strictly deeper than its parent's. Soft check only — the import
|
||||||
|
* is never aborted; the warning is a forensic signal for the curator. Reads through
|
||||||
|
* {@link RelationshipService} so the orchestrator stays within the layering rule
|
||||||
|
* (no direct repository access).
|
||||||
|
*/
|
||||||
|
private void warnOnGenerationMonotonicityViolations() {
|
||||||
|
NetworkDTO network = relationshipService.getFamilyNetwork();
|
||||||
|
Map<UUID, PersonNodeDTO> byId = new HashMap<>(network.nodes().size());
|
||||||
|
for (PersonNodeDTO node : network.nodes()) {
|
||||||
|
byId.put(node.id(), node);
|
||||||
|
}
|
||||||
|
for (RelationshipDTO edge : network.edges()) {
|
||||||
|
if (edge.relationType() != RelationType.PARENT_OF) continue;
|
||||||
|
PersonNodeDTO parent = byId.get(edge.personId());
|
||||||
|
PersonNodeDTO child = byId.get(edge.relatedPersonId());
|
||||||
|
if (parent == null || child == null) continue;
|
||||||
|
Integer pg = parent.generation();
|
||||||
|
Integer cg = child.generation();
|
||||||
|
if (pg != null && cg != null && cg <= pg) {
|
||||||
|
log.warn("Generation monotonicity violation: parent {} (G{}) -> child {} (G{})",
|
||||||
|
parent.displayName(), pg, child.displayName(), cg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -7,12 +7,18 @@ import org.mockito.InOrder;
|
|||||||
import org.mockito.Mock;
|
import org.mockito.Mock;
|
||||||
import org.mockito.junit.jupiter.MockitoExtension;
|
import org.mockito.junit.jupiter.MockitoExtension;
|
||||||
import org.raddatz.familienarchiv.exception.DomainException;
|
import org.raddatz.familienarchiv.exception.DomainException;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.RelationType;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.RelationshipService;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.NetworkDTO;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.PersonNodeDTO;
|
||||||
|
import org.raddatz.familienarchiv.person.relationship.dto.RelationshipDTO;
|
||||||
import org.springframework.test.util.ReflectionTestUtils;
|
import org.springframework.test.util.ReflectionTestUtils;
|
||||||
|
|
||||||
import java.io.File;
|
import java.io.File;
|
||||||
import java.nio.file.Files;
|
import java.nio.file.Files;
|
||||||
import java.nio.file.Path;
|
import java.nio.file.Path;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
|
import java.util.UUID;
|
||||||
|
|
||||||
import static org.assertj.core.api.Assertions.assertThat;
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
import static org.assertj.core.api.Assertions.assertThatThrownBy;
|
||||||
@@ -29,10 +35,12 @@ class CanonicalImportOrchestratorTest {
|
|||||||
@Mock PersonRegisterImporter personRegisterImporter;
|
@Mock PersonRegisterImporter personRegisterImporter;
|
||||||
@Mock PersonTreeImporter personTreeImporter;
|
@Mock PersonTreeImporter personTreeImporter;
|
||||||
@Mock DocumentImporter documentImporter;
|
@Mock DocumentImporter documentImporter;
|
||||||
|
@Mock RelationshipService relationshipService;
|
||||||
|
|
||||||
private CanonicalImportOrchestrator orchestrator(Path dir) {
|
private CanonicalImportOrchestrator orchestrator(Path dir) {
|
||||||
CanonicalImportOrchestrator o = new CanonicalImportOrchestrator(
|
CanonicalImportOrchestrator o = new CanonicalImportOrchestrator(
|
||||||
tagTreeImporter, personRegisterImporter, personTreeImporter, documentImporter);
|
tagTreeImporter, personRegisterImporter, personTreeImporter, documentImporter,
|
||||||
|
relationshipService);
|
||||||
ReflectionTestUtils.setField(o, "canonicalDir", dir.toString());
|
ReflectionTestUtils.setField(o, "canonicalDir", dir.toString());
|
||||||
return o;
|
return o;
|
||||||
}
|
}
|
||||||
@@ -53,6 +61,7 @@ class CanonicalImportOrchestratorTest {
|
|||||||
void runImport_loadsTagsAndPersonsBeforeDocuments(@TempDir Path dir) throws Exception {
|
void runImport_loadsTagsAndPersonsBeforeDocuments(@TempDir Path dir) throws Exception {
|
||||||
writeAllArtifacts(dir);
|
writeAllArtifacts(dir);
|
||||||
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(0, List.of()));
|
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(0, List.of()));
|
||||||
|
when(relationshipService.getFamilyNetwork()).thenReturn(new NetworkDTO(List.of(), List.of()));
|
||||||
CanonicalImportOrchestrator o = orchestrator(dir);
|
CanonicalImportOrchestrator o = orchestrator(dir);
|
||||||
|
|
||||||
o.runImport();
|
o.runImport();
|
||||||
@@ -68,6 +77,7 @@ class CanonicalImportOrchestratorTest {
|
|||||||
void runImport_setsStatusDone_onSuccess(@TempDir Path dir) throws Exception {
|
void runImport_setsStatusDone_onSuccess(@TempDir Path dir) throws Exception {
|
||||||
writeAllArtifacts(dir);
|
writeAllArtifacts(dir);
|
||||||
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(3, List.of()));
|
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(3, List.of()));
|
||||||
|
when(relationshipService.getFamilyNetwork()).thenReturn(new NetworkDTO(List.of(), List.of()));
|
||||||
CanonicalImportOrchestrator o = orchestrator(dir);
|
CanonicalImportOrchestrator o = orchestrator(dir);
|
||||||
|
|
||||||
o.runImport();
|
o.runImport();
|
||||||
@@ -118,6 +128,7 @@ class CanonicalImportOrchestratorTest {
|
|||||||
writeAllArtifacts(dir);
|
writeAllArtifacts(dir);
|
||||||
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(1,
|
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(1,
|
||||||
List.of(new ImportStatus.SkippedFile("fake.pdf", ImportStatus.SkipReason.INVALID_PDF_SIGNATURE))));
|
List.of(new ImportStatus.SkippedFile("fake.pdf", ImportStatus.SkipReason.INVALID_PDF_SIGNATURE))));
|
||||||
|
when(relationshipService.getFamilyNetwork()).thenReturn(new NetworkDTO(List.of(), List.of()));
|
||||||
CanonicalImportOrchestrator o = orchestrator(dir);
|
CanonicalImportOrchestrator o = orchestrator(dir);
|
||||||
|
|
||||||
o.runImport();
|
o.runImport();
|
||||||
@@ -127,4 +138,46 @@ class CanonicalImportOrchestratorTest {
|
|||||||
.extracting(ImportStatus.SkippedFile::filename)
|
.extracting(ImportStatus.SkippedFile::filename)
|
||||||
.containsExactly("fake.pdf");
|
.containsExactly("fake.pdf");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ─── generation monotonicity soft-check (#689) ─────────────────────────────
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void runImport_invokesGetFamilyNetwork_afterPersonLoaders_beforeDocuments(@TempDir Path dir) throws Exception {
|
||||||
|
writeAllArtifacts(dir);
|
||||||
|
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(0, List.of()));
|
||||||
|
when(relationshipService.getFamilyNetwork()).thenReturn(new NetworkDTO(List.of(), List.of()));
|
||||||
|
CanonicalImportOrchestrator o = orchestrator(dir);
|
||||||
|
|
||||||
|
o.runImport();
|
||||||
|
|
||||||
|
InOrder order = inOrder(personRegisterImporter, personTreeImporter, relationshipService, documentImporter);
|
||||||
|
order.verify(personRegisterImporter).load(any());
|
||||||
|
order.verify(personTreeImporter).load(any());
|
||||||
|
order.verify(relationshipService).getFamilyNetwork();
|
||||||
|
order.verify(documentImporter).load(any());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void runImport_completes_evenWhenMonotonicityViolatingEdgePresent(@TempDir Path dir) throws Exception {
|
||||||
|
// child.generation (2) <= parent.generation (3) — monotonicity violation.
|
||||||
|
// The orchestrator must WARN and continue; it must not abort or fail-closed.
|
||||||
|
writeAllArtifacts(dir);
|
||||||
|
UUID parentId = UUID.randomUUID();
|
||||||
|
UUID childId = UUID.randomUUID();
|
||||||
|
PersonNodeDTO parent = new PersonNodeDTO(parentId, "Parent", null, null, 3, true);
|
||||||
|
PersonNodeDTO child = new PersonNodeDTO(childId, "Child", null, null, 2, true);
|
||||||
|
RelationshipDTO edge = new RelationshipDTO(
|
||||||
|
UUID.randomUUID(), parentId, childId,
|
||||||
|
"Parent", null, null, "Child", null, null,
|
||||||
|
RelationType.PARENT_OF, null, null, null);
|
||||||
|
when(relationshipService.getFamilyNetwork())
|
||||||
|
.thenReturn(new NetworkDTO(List.of(parent, child), List.of(edge)));
|
||||||
|
when(documentImporter.load(any())).thenReturn(new DocumentImporter.LoadResult(0, List.of()));
|
||||||
|
CanonicalImportOrchestrator o = orchestrator(dir);
|
||||||
|
|
||||||
|
o.runImport();
|
||||||
|
|
||||||
|
assertThat(o.getStatus().state()).isEqualTo(ImportStatus.State.DONE);
|
||||||
|
verify(documentImporter).load(any());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user