docs(search-api): fix the VTL examples and document JSON output - #37544
Merged
Merged
Conversation
The Velocity section of the search API migration guide could not be followed as written: - The viewtool is registered as `estool`, not `ESContent`, so every snippet started with a variable that resolves to nothing. - The examples used Java call syntax (`$raw.hits().hits()`, `$hit.id()`). Velocity resolves properties through bean getters, so these are `$raw.hits.hits` and `$hit.id` in a template. - The Java example was wrong too: `SearchHits` declares its record components as `getHits`/`getTotalHits` (so Velocity can see them as properties), which makes the Java calls `hits().getHits()` and `hits().getTotalHits()`, not `hits().hits()`. - `aggregations()` was recommended without noting it returns the flattened first-level-terms view, silently dropping nested aggregations and top_hits. `$raw.aggregations` is the full tree. Adds a "JSON output" section answering what replaces `.toString()`: JSON is still available through `$json.generate(...)` in the neutral shape — a path deliberately kept working (issue #36435) — while Elasticsearch's own wire format is gone for good, so only consumers that parse that specific envelope have to be restructured. Also documents that the query is lowercased before it runs, so aggregation names come back lowercased. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
11 tasks
Contributor
|
Claude finished @fabrizzio-dotCMS's task in 3m 8s —— View job Review — docs(search-api): VTL examples & JSON outputDocs-only PR. I verified every accessor and behavioral claim in the changed text against source, and rechecked the two review comments.
Verification resultsEvery claim in the diff holds against the code:
The reduced-payload example ( Resolved (reviewer comments)
New IssuesNo issues found. The rewrites are accurate and internally consistent, and both requested changes are addressed. · |
Four fixes to the migration runbook, each verified against the source: - Neutral response accessors. `SearchHits` declares its record components as `getHits`/`getTotalHits` — deliberately, so Velocity resolves them as bean properties — so `hits().hits()` and `hits().totalHits()` do not exist. The Java calls are `hits().getHits()` and `hits().getTotalHits().value()`. Also swaps `aggregations()` for `getAggregations()`: the former returns the flattened first-level-terms view and drops nested aggregations and top_hits. - When esSearch / esRaw break. Both delegate to APILocator.getEsSearchAPI(), the legacy Elasticsearch client, in every phase — they never reach the phase router. So they do not fail in Phase 2: they keep answering from Elasticsearch while the rest of the site reads OpenSearch, which is a silent divergence rather than a crash. They fail at Phase 3. Corrected in the $estool table, both plugin-grading tables, and the Timing section, whose conclusion now reads that neither a clean Phase 1 nor a clean Phase 2 says anything about plugin readiness. - OS_INDEX_REPLICAS. It is declared with a fallback to ES_INDEX_REPLICAS (OSIndexProperty), so the claim that OpenSearch "does not inherit dotCMS's implicit default" was wrong. The advice to set it stands; the reason is that it otherwise inherits the old cluster's value, or pins nothing. - Site Search crawl. Stage 3.9 warned that an incremental crawl can leave a dynamically-mapped copy, while R12 already documented the gate that prevents exactly that. The warning now points at R12 and keeps the instruction: run the full crawl yourself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Pushed a second commit (
One known issue deliberately left alone: the runbook describes the Phase 2 read fallback as covering every read, which is not true in |
Review feedback on #37544: - The header said the guide was for "plugin and integration developers", but section 3 is written for template authors — more visible now that the section has grown. It now names both audiences and says which sections belong to each. - Section 4's "before" example cast the result of `esSearch(...)` to `ESSearchResults`, which is already its return type. Removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Member
Author
|
Both review notes applied in
On the lowercasing note: the fact is right and worth recording. |
jdcmsd
requested changes
Sep 14, 2026
Follow-up to the previous commit, which corrected the plugin tables and R7's timing section but left four other places still saying customisations "break at Phase 2" — so the runbook contradicted itself. The same reasoning applies to templates as to plugins: `$estool.esSearch` and `esRaw` reach `APILocator.getEsSearchAPI()` in every phase and never touch the phase router. They do not break in Phase 2. While the two indices agree they render exactly as before, which means walking the site in Stage 4 proves nothing about them; they fail in Stage 5, when Elasticsearch is gone. That changes what Stage 4 is actually risky for. Its real exposure is the supported path — content pulls, the admin search, URL maps, REST and GraphQL — which does switch engine there, so any difference between the indices surfaces: results in a different order where the query gave no explicit sort, or short results where the copy is incomplete. The stage heading and the Phase 2 description now say that, with the legacy methods called out separately as the deferred, symptomless risk they are. Also moves the template/plugin deadline from "before Stage 4" to "before Stage 5", with the reason: fix them while Elasticsearch is still live and old and new output can be compared side by side. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes the Velocity section of
docs/backend/SEARCH_API_MIGRATION.md, whose examples couldnot be followed as written, and adds a section answering the question the guide raises but
never resolves: if
.toString()no longer returns JSON, what does a template use instead?Why
Every snippet in section 3 fails:
$ESContent.raw($query)estool(toolbox.xml).$ESContentresolves to nothing$estool.raw($query)$raw.hits().hits()$raw.hits.hits$hit.id()$hit.idhits().totalHits().value()$raw.hits.totalHits.valueThe Java example in section 1 was also wrong:
SearchHitsdeclares its record componentsas
getHits/getTotalHits— deliberately, so Velocity can resolve them as properties —which means there is no
hits()ortotalHits()method on it.raw.hits().hits()does notcompile; it is
raw.hits().getHits().And
aggregations()was recommended with no caveat. Its own javadoc says it returns"first-level terms aggregations… only aggregations that have buckets are included" — it
silently drops nested aggregations and
top_hits. The full tree isgetAggregations(),reached from VTL as
$raw.aggregations.Changes
$estool, with every snippet rewritten in real Velocity.a Velocity heading. Both flavours of
aggregationsare listed, with the flattened onemarked lossy.
SearchHitslooks the way it does, so the naming stops surprising peoplein both languages.
$json.generate(...)still produces JSON in the neutralshape — a path kept working on purpose (
ContentSearchResponse.getAggregations()isdeliberately not
@JsonIgnoreso the reflective JSON builder still sees it, issue $json.generate() reflection-based navigation of aggregation results silently breaks after #36026 records migration #36435)— while Elasticsearch's wire format is gone by design. Only consumers that parse that
specific envelope need restructuring. Includes a reduced-payload example and a
three-case summary table.
(
StringUtils.lowercaseStringExceptMatchingTokens), so an aggregation declared"tagAgg"comes back keyed
tagagg— a#foreachover the wrong key renders nothing rather thanfailing.
Verification
Every accessor in the changed text was checked against the source:
ESContentTool,ContentSearchResponse,SearchHits,TotalHits,SearchHit,AggregationBucket,JSONTool, and the viewtool keys intoolbox.xml.Docs only — no code changes.
Related: #37520 (the audit/rewrite tool, which uses these rules as its spec).
🤖 Generated with Claude Code
This PR fixes: #37520
This PR fixes: #37520