refactor(timeline): resolve each letter's primary tag once
mapDocument re-ran the alphabetical min() scan over the letter's tag set to look up its already-resolved root, duplicating the work resolveLetterRootTags had just done and leaving two independent definitions of "primary tag" that could silently diverge. Key the resolved-root map by document id and compute the primary tag exactly once per letter; drop the redundant resolvePrimaryRoot helper. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -19,6 +19,7 @@ import org.springframework.transaction.annotation.Transactional;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.Comparator;
|
||||
import java.util.HashMap;
|
||||
import java.util.LinkedHashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
@@ -105,9 +106,9 @@ public class TimelineService {
|
||||
if (!passesYearFilter(doc.getDocumentDate(), doc.getMetaDatePrecision(), filter)) continue;
|
||||
letters.add(doc);
|
||||
}
|
||||
Map<UUID, RootTag> rootByPrimaryTagId = resolveLetterRootTags(letters);
|
||||
Map<UUID, RootTag> rootByDocId = resolveLetterRootTags(letters);
|
||||
for (Document doc : letters) {
|
||||
entries.add(mapDocument(doc, rootByPrimaryTagId));
|
||||
entries.add(mapDocument(doc, rootByDocId));
|
||||
}
|
||||
|
||||
return bucket(entries);
|
||||
@@ -232,8 +233,8 @@ public class TimelineService {
|
||||
);
|
||||
}
|
||||
|
||||
private TimelineEntryDTO mapDocument(Document doc, Map<UUID, RootTag> rootByPrimaryTagId) {
|
||||
RootTag root = resolvePrimaryRoot(doc, rootByPrimaryTagId);
|
||||
private TimelineEntryDTO mapDocument(Document doc, Map<UUID, RootTag> rootByDocId) {
|
||||
RootTag root = rootByDocId.get(doc.getId());
|
||||
return new TimelineEntryDTO(
|
||||
Kind.LETTER,
|
||||
doc.getMetaDatePrecision(),
|
||||
@@ -255,22 +256,27 @@ public class TimelineService {
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the root tags for the letters' primary tags in one batched pass — no per-letter
|
||||
* N+1: each letter contributes only its alphabetically-first assigned tag (#835), and
|
||||
* {@link TagService#resolveRootTags} memoizes the ancestry walk per distinct tag.
|
||||
* Resolves each letter's primary root tag in one batched pass, keyed by document id — no
|
||||
* per-letter N+1: each letter contributes only its alphabetically-first assigned tag (#835),
|
||||
* so the {@code min()} scan over a letter's tag set runs exactly once here (not again at map
|
||||
* time), and {@link TagService#resolveRootTags} memoizes the ancestry walk per distinct tag.
|
||||
*/
|
||||
private Map<UUID, RootTag> resolveLetterRootTags(List<Document> letters) {
|
||||
List<Tag> primaryTags = letters.stream()
|
||||
.map(TimelineService::primaryTag)
|
||||
.filter(t -> t != null)
|
||||
.toList();
|
||||
if (primaryTags.isEmpty()) return Map.of();
|
||||
return tagService.resolveRootTags(primaryTags);
|
||||
}
|
||||
Map<UUID, Tag> primaryByDocId = new LinkedHashMap<>();
|
||||
for (Document doc : letters) {
|
||||
Tag primary = primaryTag(doc);
|
||||
if (primary != null) primaryByDocId.put(doc.getId(), primary);
|
||||
}
|
||||
if (primaryByDocId.isEmpty()) return Map.of();
|
||||
|
||||
private RootTag resolvePrimaryRoot(Document doc, Map<UUID, RootTag> rootByPrimaryTagId) {
|
||||
Tag primary = primaryTag(doc);
|
||||
return primary == null ? null : rootByPrimaryTagId.get(primary.getId());
|
||||
Map<UUID, RootTag> rootByTagId =
|
||||
tagService.resolveRootTags(new ArrayList<>(primaryByDocId.values()));
|
||||
Map<UUID, RootTag> rootByDocId = new HashMap<>();
|
||||
primaryByDocId.forEach((docId, primary) -> {
|
||||
RootTag root = rootByTagId.get(primary.getId());
|
||||
if (root != null) rootByDocId.put(docId, root);
|
||||
});
|
||||
return rootByDocId;
|
||||
}
|
||||
|
||||
/** A letter's primary tag: the alphabetically-first of its assigned tags by name (#835). */
|
||||
|
||||
Reference in New Issue
Block a user