From 46001bbf9d4e1a4ac4517fe8c212c179d081ff85 Mon Sep 17 00:00:00 2001 From: Marcel Date: Sat, 25 Apr 2026 16:52:38 +0200 Subject: [PATCH] refactor(documents): extract buildSearchSpec and resolveTags helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Markus #3 / Felix B2 — kill the duplicated spec-chain across findIdsForFilter and searchDocuments, and centralise the "name string → Tag (find or create)" loop that updateDocumentTags and applyBulkEditToDocument were each carrying their own copy of. `buildSearchSpec` is the single source of truth for the seven-spec chain (text + date range + sender + receiver + tags + tag-prefix + status). Both callers do their own FTS short-circuit, then delegate. `resolveTags` is the single source of truth for trimming, blank-skipping, and find-or-create through TagService. Both updateDocumentTags (replace semantics) and applyBulkEditToDocument (additive merge) consume it. No behaviour change. All 231 backend tests still green. Refs #225, PR #331 Co-Authored-By: Claude Sonnet 4.6 --- .../service/DocumentService.java | 77 ++++++++++--------- 1 file changed, 42 insertions(+), 35 deletions(-) diff --git a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java index 12ea8f4a..3fd9e43a 100644 --- a/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java +++ b/backend/src/main/java/org/raddatz/familienarchiv/service/DocumentService.java @@ -336,22 +336,29 @@ public class DocumentService { public Document updateDocumentTags(UUID docId, List tagNames) { Document doc = documentRepository.findById(docId) .orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + docId)); - - Set newTags = new HashSet<>(); - - for (String name : tagNames) { - // Clean the string - String cleanName = name.trim(); - if (cleanName.isEmpty()) - continue; - - newTags.add(tagService.findOrCreate(cleanName)); - } - - doc.setTags(newTags); + doc.setTags(resolveTags(tagNames)); return documentRepository.save(doc); } + /** + * Resolves a list of tag-name strings to {@link Tag} entities, trimming + * whitespace and skipping blank entries. Single source of truth for + * "name string → Tag" so the find-or-create policy stays consistent + * across single-doc updates ({@link #updateDocumentTags}), bulk edits + * ({@link #applyBulkEditToDocument}), and the upload-batch path + * ({@code applyBatchMetadata}). + */ + private Set resolveTags(List tagNames) { + if (tagNames == null || tagNames.isEmpty()) return new HashSet<>(); + Set resolved = new HashSet<>(); + for (String name : tagNames) { + String cleanName = name.trim(); + if (cleanName.isEmpty()) continue; + resolved.add(tagService.findOrCreate(cleanName)); + } + return resolved; + } + /** * Returns all document IDs matching the given filter parameters, ignoring * pagination. Used by the bulk-edit "Alle X editieren" fast path so the @@ -367,19 +374,33 @@ public class DocumentService { rankedIds = documentRepository.findRankedIdsByFts(text); if (rankedIds.isEmpty()) return List.of(); } + + Specification spec = buildSearchSpec( + hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator); + return documentRepository.findAll(spec).stream().map(Document::getId).toList(); + } + + /** + * Single source of truth for the search Specification chain. Shared by + * {@link #searchDocuments} (paged + sorted) and {@link #findIdsForFilter} + * (uncapped, ID-only). Caller does its own FTS short-circuit when the + * full-text query returned no rows. + */ + private Specification buildSearchSpec(boolean hasText, List ftsIds, + LocalDate from, LocalDate to, + UUID sender, UUID receiver, + List tags, String tagQ, + DocumentStatus status, TagOperator tagOperator) { boolean useOrLogic = tagOperator == TagOperator.OR; List> expandedTagSets = tagService.expandTagNamesToDescendantIdSets(tags); - - Specification textSpec = hasText ? hasIds(rankedIds) : (root, query, cb) -> null; - Specification spec = Specification.where(textSpec) + Specification textSpec = hasText ? hasIds(ftsIds) : (root, query, cb) -> null; + return Specification.where(textSpec) .and(isBetween(from, to)) .and(hasSender(sender)) .and(hasReceiver(receiver)) .and(hasTags(expandedTagSets, useOrLogic)) .and(hasTagPartial(tagQ)) .and(hasStatus(status)); - - return documentRepository.findAll(spec).stream().map(Document::getId).toList(); } /** @@ -423,12 +444,7 @@ public class DocumentService { if (dto.getTagNames() != null && !dto.getTagNames().isEmpty()) { Set merged = new HashSet<>(doc.getTags()); - for (String name : dto.getTagNames()) { - String clean = name.trim(); - if (!clean.isEmpty()) { - merged.add(tagService.findOrCreate(clean)); - } - } + merged.addAll(resolveTags(dto.getTagNames())); doc.setTags(merged); } @@ -522,17 +538,8 @@ public class DocumentService { if (rankedIds.isEmpty()) return DocumentSearchResult.of(List.of()); } - boolean useOrLogic = tagOperator == TagOperator.OR; - List> expandedTagSets = tagService.expandTagNamesToDescendantIdSets(tags); - - Specification textSpec = hasText ? hasIds(rankedIds) : (root, query, cb) -> null; - Specification spec = Specification.where(textSpec) - .and(isBetween(from, to)) - .and(hasSender(sender)) - .and(hasReceiver(receiver)) - .and(hasTags(expandedTagSets, useOrLogic)) - .and(hasTagPartial(tagQ)) - .and(hasStatus(status)); + Specification spec = buildSearchSpec( + hasText, rankedIds, from, to, sender, receiver, tags, tagQ, status, tagOperator); // SENDER, RECEIVER and RELEVANCE sorts load the full match set and slice in memory. // JPA's Sort.by("sender.lastName") generates an INNER JOIN that silently drops