Skip to content

fix(search): recover exact-name and mid-token matches in the legacy relationship dialog (#37052) - #37056

Open
ihoffmann-dot wants to merge 1 commit into
mainfrom
issue-37052-legacy-relationship-search-mid-token
Open

fix(search): recover exact-name and mid-token matches in the legacy relationship dialog (#37052)#37056
ihoffmann-dot wants to merge 1 commit into
mainfrom
issue-37052-legacy-relationship-search-mid-token

Conversation

@ihoffmann-dot

Copy link
Copy Markdown
Member

Problem

#36791 / PR #36793 fixed the exact-name and mid-token matching gap in GlobalSearchAttributeStrategy — the shared strategy behind the Content Search portlet, Content Drive's keyword search, and the new (Angular) edit content Relationships dialog.

The legacy (Dojo/JSP) edit content screen never goes through that code. Its "Relate" dialog is dotcms.dijit.form.ContentSelector, which dispatches over DWR to ContentletAjax.searchContentletsByUser — a separate, drifted copy of the same logic that still gates on a catchall-only prefix:

luceneQuery.append("+" + fieldName + ":" + fieldValueStr + "* ");   // +catchall:img_0004.jpeg*
...
if ("catchall".equals(fieldName)) {
    ...
    luceneQuery.append(" title_dotraw:*" + fieldValueStr + "*^5 "); // optional BOOST only
}

The title_dotraw wildcard was already there, but only as an optional boost clause — the mandatory +catchall:<term>* had already excluded the document, so it never rescued anything. Surfaced during QA of #36791.

Fix

Make the catchall gate a disjunction, mirroring GlobalSearchAttributeStrategy:

+(catchall:<term>*^10 OR title_dotraw:*<term>*^2)
  • title_dotraw is scoped to a single field, so this recovers mid-token and exact-full-value matches without reintroducing the broad, whole-document catchall:*term* removed in [BUG] Content Drive: keyword/title search returns inconsistent or incoherent results #36688.
  • Asymmetric boosts are deliberate and match the reference implementation: a genuine token-prefix hit outranks a raw-substring hit that can land anywhere.
  • Query-construction only. No mapping/analyzer change, no reindex.

Two deliberate secondary changes worth a reviewer's eye:

  1. The catchall branch is now evaluated before the contains("-") branch. Previously a term containing a dash (e.g. nordic-skiing) fell into the no-wildcard branch and behaved differently. GlobalSearchAttributeStrategy has no such special case, so this aligns the two. languageId keeps its existing handling.
  2. The standalone title_dotraw:*value*^5 boost was removed, since the same clause is now part of the mandatory gate at ^2. Keeping both would duplicate the clause.

Validation

New integration test ContentletAjaxTest#test_searchContentletsByUser_globalSearch_matchesMidTokenAndExactFullName, seeding a File Asset named IMG_<uniqueToken>_0004.jpeg (tokenizes to img_<uniqueToken>_0004 + jpeg):

Search term Before After Matches via
IMG_<uniqueToken> (genuine token prefix) catchall prefix
<uniqueToken> (mid-token) title_dotraw substring
IMG_<uniqueToken>_0004.jpeg (exact full name, spans the .) title_dotraw substring
zz<uniqueToken> (in no field) 0 results 0 results negative control — no broad catchall

The term is unique per run so the assertions hold against the shared test index.

Red/Green verified. With the production change reverted the test fails on the mid-token assertion (expected:<1> but was:<0>) while the preceding token-prefix assertion passes — so it isolates this defect rather than failing for an unrelated reason. With the fix, ContentletAjaxTest is 10/10 green (9 pre-existing + 1 new). The class is already registered in MainSuite2a, so no suite changes were needed.

Scope

Closes #37052. Follow-up to #36791 / PR #36793 — same defect, second code path.

Out of scope / follow-ups

  • Collapsing the duplicated query-building logic by having ContentletAjax call GlobalSearchAttributeStrategy directly (as BrowserAPIImpl already does). Removes the drift at the root, but touches more legacy surface than this fix warrants.

@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ihoffmann-dot's task in 2m 15s —— View job


Code Review

I reviewed the diff against origin/main, the surrounding query-construction logic in ContentletAjax.java, and cross-checked against the reference GlobalSearchAttributeStrategy this PR mirrors.

  • Read PR diff and changed files
  • Compare against reference GlobalSearchAttributeStrategy
  • Verify result-list indexing used by the new test
  • Check for introduced bugs / regressions

Findings

The fix is correct and faithfully mirrors GlobalSearchAttributeStrategy (+(catchall:value*^10 OR title_dotraw:*value*^2)). Making the title_dotraw substring a member of the mandatory +(...) gate — rather than an optional boost sitting behind an already-exclusionary +catchall:term* — is exactly what recovers the mid-token / exact-full-name cases without reintroducing the broad catchall:*value*. Removing the now-redundant standalone title_dotraw:*value*^5 boost (line ~982) is right, since the clause lives in the gate at ^2. The negative-control assertion correctly proves the broad wildcard is not back. Result indexing in the test (get(0).total, get(3).identifier) matches how searchContentletsByUser assembles the list (ContentletAjax.java:1084-1114).

One item worth a look before merge:

New Issues

  • 🟡 Medium: ContentletAjax.java:966Multi-word catchall terms may parse ambiguously inside the new required group. For a search like foo bar, the term reaches this branch as foo bar (spaces collapsed but not split), producing +(catchall:foo bar*^10 OR title_dotraw:*foo bar*^2). In the Lucene/ES query-string parser the bare bar* token inside the group binds to the default field rather than to catchall/title_dotraw, so a multi-word global search behaves differently from a single-token one.
    Assumption: the ContentSelector "Relate" dialog can submit multi-word global-search terms.
    What to verify: run a two-word catchall search through this path and confirm the parsed query behaves as intended. Note this space-splitting quirk is pre-existing (the old +catchall:foo bar* had the same leak) and the reference GlobalSearchAttributeStrategy:37-38 builds the identical construction — so this is not a regression introduced here, just the one edge the new test doesn't cover. A single-token multi-word case in the test would document the expected behavior. Fix this →

No security, transaction, or convention issues (query-construction only, values escaped via the existing specialCharsToEscape at ContentletAjax.java:951, no mapping/reindex change). Test cleanup (destroy + deltree) is handled in finally. The re-ordering of the catchall branch ahead of the contains("-") branch is intentional, documented, and aligns the two code paths.

Overall this is a clean, well-scoped, well-tested fix — the Medium item is non-blocking.
issue-37052-legacy-relationship-search-mid-token

// including mid-word). Mirrors GlobalSearchAttributeStrategy, which is
// the equivalent gate for the new Content Search / Content Drive path.
luceneQuery.append("+(catchall:" + fieldValueStr + "*^10 OR title_dotraw:*"
+ fieldValueStr + "*^2) ");

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how did you choose the ^10 and ^2?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Safe To Rollback Area : Backend PR changes Java/Maven backend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Legacy edit content: Relationships search dialog still misses exact-name and mid-token terms (ContentletAjax catchall-only gate)

3 participants