refactor(document): share skip-null date-field resolution between save and projection (#726)
Extract effectivePrecision/effectiveMetaDateEnd/effectiveMetaDateRaw, used by both applyDatePrecision (the real setters) and projectedState (the title projection), so the two can no longer drift — addresses review feedback (Markus/Felix/Sara). Writing a stored value back when the DTO omits a field is a harmless no-op, so behaviour is unchanged (185 existing DocumentServiceTest cases stay green). Also documents the file-replace "treat as manual" path inline at the reassignment site. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -431,7 +431,11 @@ public class DocumentService {
|
||||
doc.setScriptType(dto.getScriptType());
|
||||
}
|
||||
|
||||
// 4. Datei austauschen (nur wenn eine neue ausgewählt wurde)
|
||||
// 4. Datei austauschen (nur wenn eine neue ausgewählt wurde).
|
||||
// NB (#726): this reassigns originalFilename to the uploaded file's name. The title's index
|
||||
// segment is originalFilename, so after a replace the stored title no longer matches
|
||||
// build(currentState) and the row is treated as manual — neither save-time nor backfill
|
||||
// rewrites it. Accepted fail-safe (ADR-031), and autoTitleBefore was already captured above.
|
||||
boolean fileReplaced = newFile != null && !newFile.isEmpty();
|
||||
if (fileReplaced) {
|
||||
FileService.UploadResult upload = fileService.uploadFile(newFile, newFile.getOriginalFilename());
|
||||
@@ -479,44 +483,49 @@ public class DocumentService {
|
||||
}
|
||||
|
||||
/**
|
||||
* The document state the regenerated title is built from, mirroring {@code updateDocument}'s
|
||||
* setter asymmetry exactly: {@code documentDate}/{@code location} are overwritten from the DTO
|
||||
* (a null value clears the field), while precision/end/raw are taken from the DTO only when
|
||||
* non-null and otherwise kept from the stored entity (see {@link #applyDatePrecision}). The
|
||||
* index ({@code originalFilename}) is never touched by a metadata edit. A mismatch here would
|
||||
* silently produce a wrong label, so it is kept lock-step with the real setters.
|
||||
* The document state the regenerated title is built from. It is composed from the SAME
|
||||
* resolvers the real setters use — {@code documentDate}/{@code location} overwritten from the
|
||||
* DTO (a null value clears the field), precision/end/raw resolved skip-null via
|
||||
* {@link #effectivePrecision}/{@link #effectiveMetaDateEnd}/{@link #effectiveMetaDateRaw} — so
|
||||
* the projection cannot drift from {@link #updateDocument}. The index ({@code originalFilename})
|
||||
* is never touched by a metadata edit.
|
||||
*/
|
||||
private Document projectedState(Document doc, DocumentUpdateDTO dto) {
|
||||
return Document.builder()
|
||||
.originalFilename(doc.getOriginalFilename())
|
||||
.documentDate(dto.getDocumentDate())
|
||||
.location(dto.getLocation())
|
||||
.metaDatePrecision(dto.getMetaDatePrecision() != null
|
||||
? dto.getMetaDatePrecision() : doc.getMetaDatePrecision())
|
||||
.metaDateEnd(dto.getMetaDateEnd() != null
|
||||
? dto.getMetaDateEnd() : doc.getMetaDateEnd())
|
||||
.metaDateRaw(dto.getMetaDateRaw() != null
|
||||
? dto.getMetaDateRaw() : doc.getMetaDateRaw())
|
||||
.metaDatePrecision(effectivePrecision(doc, dto))
|
||||
.metaDateEnd(effectiveMetaDateEnd(doc, dto))
|
||||
.metaDateRaw(effectiveMetaDateRaw(doc, dto))
|
||||
.build();
|
||||
}
|
||||
|
||||
/**
|
||||
* Applies the three date-precision fields only when the DTO carries them.
|
||||
* A null field means "not submitted" — overwriting the stored value with null
|
||||
* would fabricate a precision the user never chose, the exact dishonesty #666
|
||||
* exists to prevent. A row with a genuinely-unknown precision must keep it when
|
||||
* an unrelated edit (e.g. a location typo) is saved.
|
||||
* Applies the three date-precision fields skip-null: a null DTO field means "not submitted",
|
||||
* so the stored value is kept rather than overwritten with null — which would fabricate a
|
||||
* precision the user never chose, the exact dishonesty #666 exists to prevent. Expressed via
|
||||
* the shared {@code effective*} resolvers so {@link #projectedState} stays lock-step (writing
|
||||
* the stored value back when the DTO omits a field is a harmless no-op).
|
||||
*/
|
||||
private void applyDatePrecision(Document doc, DocumentUpdateDTO dto) {
|
||||
if (dto.getMetaDatePrecision() != null) {
|
||||
doc.setMetaDatePrecision(dto.getMetaDatePrecision());
|
||||
doc.setMetaDatePrecision(effectivePrecision(doc, dto));
|
||||
doc.setMetaDateEnd(effectiveMetaDateEnd(doc, dto));
|
||||
doc.setMetaDateRaw(effectiveMetaDateRaw(doc, dto));
|
||||
}
|
||||
if (dto.getMetaDateEnd() != null) {
|
||||
doc.setMetaDateEnd(dto.getMetaDateEnd());
|
||||
|
||||
// Skip-null date-field resolution shared by applyDatePrecision (the real setters) and
|
||||
// projectedState (the title projection) — the single rule keeps them from diverging (#726).
|
||||
private static DatePrecision effectivePrecision(Document doc, DocumentUpdateDTO dto) {
|
||||
return dto.getMetaDatePrecision() != null ? dto.getMetaDatePrecision() : doc.getMetaDatePrecision();
|
||||
}
|
||||
if (dto.getMetaDateRaw() != null) {
|
||||
doc.setMetaDateRaw(dto.getMetaDateRaw());
|
||||
|
||||
private static LocalDate effectiveMetaDateEnd(Document doc, DocumentUpdateDTO dto) {
|
||||
return dto.getMetaDateEnd() != null ? dto.getMetaDateEnd() : doc.getMetaDateEnd();
|
||||
}
|
||||
|
||||
private static String effectiveMetaDateRaw(Document doc, DocumentUpdateDTO dto) {
|
||||
return dto.getMetaDateRaw() != null ? dto.getMetaDateRaw() : doc.getMetaDateRaw();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user