refactor(person): single source of truth for generation bounds (#689)

Markus flagged the 0/10 range was duplicated across five sites (DB
CHECK, both importers, DTO @Min/@Max, dropdown range). New
PersonGeneration.MIN_GENERATION / MAX_GENERATION constants are now
the canonical Java source; the DTO annotations and both importer
guards reference them. The V70 SQL CHECK comment now points at the
Java constants so future widening updates one Java class plus one
SQL literal (Flyway forbids rewriting the migration in place).

Refs #689

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Marcel
2026-05-28 16:16:26 +02:00
parent 39276b179d
commit 516a0a3814
5 changed files with 37 additions and 22 deletions

View File

@@ -2,6 +2,7 @@ package org.raddatz.familienarchiv.importing;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j; import lombok.extern.slf4j.Slf4j;
import org.raddatz.familienarchiv.person.PersonGeneration;
import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.person.PersonService;
import org.raddatz.familienarchiv.person.PersonType; import org.raddatz.familienarchiv.person.PersonType;
import org.raddatz.familienarchiv.person.PersonUpsertCommand; import org.raddatz.familienarchiv.person.PersonUpsertCommand;
@@ -33,8 +34,6 @@ public class PersonRegisterImporter {
// carry an inline note. Out-of-range values are caught by the post-parse // carry an inline note. Out-of-range values are caught by the post-parse
// range guard, not by the regex. // range guard, not by the regex.
private static final Pattern GENERATION_PATTERN = Pattern.compile("^\\s*G?\\s*(-?\\d+)"); private static final Pattern GENERATION_PATTERN = Pattern.compile("^\\s*G?\\s*(-?\\d+)");
private static final int GENERATION_MIN = 0;
private static final int GENERATION_MAX = 10;
private final PersonService personService; private final PersonService personService;
@@ -68,16 +67,16 @@ public class PersonRegisterImporter {
/** /**
* Parses an optional {@code G n} generation cell. Returns null for blanks, * Parses an optional {@code G n} generation cell. Returns null for blanks,
* non-matching strings, and any value outside {@value #GENERATION_MIN}..{@value #GENERATION_MAX} * non-matching strings, and any value outside the {@link PersonGeneration}
* (mirroring the V70 CHECK). Out-of-range values log a WARN but never abort * bounds (mirroring the V70 CHECK). Out-of-range values log a WARN but
* the batch — REQ-IMP-001. * never abort the batch — REQ-IMP-001.
*/ */
static Integer parseGeneration(String raw, String personId) { static Integer parseGeneration(String raw, String personId) {
if (raw == null || raw.isBlank()) return null; if (raw == null || raw.isBlank()) return null;
Matcher m = GENERATION_PATTERN.matcher(raw); Matcher m = GENERATION_PATTERN.matcher(raw);
if (!m.find()) return null; if (!m.find()) return null;
int parsed = Integer.parseInt(m.group(1)); int parsed = Integer.parseInt(m.group(1));
if (parsed < GENERATION_MIN || parsed > GENERATION_MAX) { if (parsed < PersonGeneration.MIN_GENERATION || parsed > PersonGeneration.MAX_GENERATION) {
log.warn("Skipping out-of-range generation '{}' for row {}", raw, personId); log.warn("Skipping out-of-range generation '{}' for row {}", raw, personId);
return null; return null;
} }

View File

@@ -7,6 +7,7 @@ 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.Person; import org.raddatz.familienarchiv.person.Person;
import org.raddatz.familienarchiv.person.PersonGeneration;
import org.raddatz.familienarchiv.person.PersonService; import org.raddatz.familienarchiv.person.PersonService;
import org.raddatz.familienarchiv.person.PersonType; import org.raddatz.familienarchiv.person.PersonType;
import org.raddatz.familienarchiv.person.PersonUpsertCommand; import org.raddatz.familienarchiv.person.PersonUpsertCommand;
@@ -87,24 +88,21 @@ public class PersonTreeImporter {
} }
/** /**
* Returns the JSON {@code generation} value if present and in * Returns the JSON {@code generation} value if present and within the
* {@value #GENERATION_MIN}..{@value #GENERATION_MAX}; null otherwise. Out-of-range * {@link PersonGeneration} bounds; null otherwise. Out-of-range values
* values log a WARN but never abort the batch — mirrors the register-importer * log a WARN but never abort the batch — mirrors the register-importer
* skip-and-warn policy. * skip-and-warn policy.
*/ */
private static Integer generationOrNull(JsonNode node, String personId) { private static Integer generationOrNull(JsonNode node, String personId) {
Integer raw = intOrNull(node, "generation"); Integer raw = intOrNull(node, "generation");
if (raw == null) return null; if (raw == null) return null;
if (raw < GENERATION_MIN || raw > GENERATION_MAX) { if (raw < PersonGeneration.MIN_GENERATION || raw > PersonGeneration.MAX_GENERATION) {
log.warn("Skipping out-of-range generation '{}' for person {}", raw, personId); log.warn("Skipping out-of-range generation '{}' for person {}", raw, personId);
return null; return null;
} }
return raw; return raw;
} }
private static final int GENERATION_MIN = 0;
private static final int GENERATION_MAX = 10;
private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) { private int createRelationships(JsonNode relationships, Map<String, UUID> idByRowId) {
int created = 0; int created = 0;
for (JsonNode node : relationships) { for (JsonNode node : relationships) {

View File

@@ -0,0 +1,16 @@
package org.raddatz.familienarchiv.person;
/**
* Single source of truth for the {@code persons.generation} value range.
* The DB CHECK in V70, the {@code PersonUpdateDTO} Bean Validation annotations,
* and the canonical importers all reference these constants so a future widening
* (e.g. accepting {@code G 1} ancestors) happens in one place. Mirror this file
* by hand in the V70 migration comment when adjusting bounds.
*/
public final class PersonGeneration {
public static final int MIN_GENERATION = 0;
public static final int MAX_GENERATION = 10;
private PersonGeneration() {}
}

View File

@@ -23,9 +23,9 @@ public class PersonUpdateDTO {
private String notes; private String notes;
private Integer birthYear; private Integer birthYear;
private Integer deathYear; private Integer deathYear;
// Mirror of the persons.generation CHECK constraint (V70). // Mirror of the persons.generation CHECK constraint (V70). Bounds live in
// null clears the field; 0..10 is the allowlist of valid indices. // PersonGeneration so DB, DTO, and importer all read from one place.
@Min(0) @Min(PersonGeneration.MIN_GENERATION)
@Max(10) @Max(PersonGeneration.MAX_GENERATION)
private Integer generation; private Integer generation;
} }

View File

@@ -10,11 +10,13 @@
ALTER TABLE persons ADD COLUMN generation SMALLINT; ALTER TABLE persons ADD COLUMN generation SMALLINT;
-- Allowlist of valid generation indices. The upper bound is intentionally -- Allowlist of valid generation indices. The 0..10 bounds mirror
-- loose — current data tops out at G 5, but a future G 6 → G 10 widening -- PersonGeneration.MIN_GENERATION / MAX_GENERATION in Java — keep the
-- needs no migration. A G 1 ancestor would require a separate one-shot -- two in sync (the DTO @Min/@Max and both importer range guards read from
-- shift migration (out of scope here; the layout's normalise step already -- those Java constants). Current data tops out at G 5, but a future G 6 →
-- handles negative seeds at render time). -- G 10 widening needs no migration. A G 1 ancestor would require a
-- separate one-shot shift migration (out of scope here; the layout's
-- normalise step already handles negative seeds at render time).
ALTER TABLE persons ADD CONSTRAINT chk_generation_range ALTER TABLE persons ADD CONSTRAINT chk_generation_range
CHECK (generation IS NULL OR generation BETWEEN 0 AND 10); CHECK (generation IS NULL OR generation BETWEEN 0 AND 10);