refactor(documents): extract buildSearchSpec and resolveTags helpers
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 <noreply@anthropic.com>
This commit is contained in:
@@ -336,22 +336,29 @@ public class DocumentService {
|
||||
public Document updateDocumentTags(UUID docId, List<String> tagNames) {
|
||||
Document doc = documentRepository.findById(docId)
|
||||
.orElseThrow(() -> DomainException.notFound(ErrorCode.DOCUMENT_NOT_FOUND, "Document not found: " + docId));
|
||||
|
||||
Set<Tag> 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<Tag> resolveTags(List<String> tagNames) {
|
||||
if (tagNames == null || tagNames.isEmpty()) return new HashSet<>();
|
||||
Set<Tag> 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<Document> 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<Document> buildSearchSpec(boolean hasText, List<UUID> ftsIds,
|
||||
LocalDate from, LocalDate to,
|
||||
UUID sender, UUID receiver,
|
||||
List<String> tags, String tagQ,
|
||||
DocumentStatus status, TagOperator tagOperator) {
|
||||
boolean useOrLogic = tagOperator == TagOperator.OR;
|
||||
List<Set<UUID>> expandedTagSets = tagService.expandTagNamesToDescendantIdSets(tags);
|
||||
|
||||
Specification<Document> textSpec = hasText ? hasIds(rankedIds) : (root, query, cb) -> null;
|
||||
Specification<Document> spec = Specification.where(textSpec)
|
||||
Specification<Document> 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<Tag> 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<Set<UUID>> expandedTagSets = tagService.expandTagNamesToDescendantIdSets(tags);
|
||||
|
||||
Specification<Document> textSpec = hasText ? hasIds(rankedIds) : (root, query, cb) -> null;
|
||||
Specification<Document> 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<Document> 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
|
||||
|
||||
Reference in New Issue
Block a user