Skip to content

test(search): /api/es IT coverage + docs for non-finite _score regression (#36478) - #36489

Merged
fabrizzio-dotCMS merged 2 commits into
mainfrom
issue-36478-harden-nonfinite-json
Jul 10, 2026
Merged

test(search): /api/es IT coverage + docs for non-finite _score regression (#36478)#36489
fabrizzio-dotCMS merged 2 commits into
mainfrom
issue-36478-harden-nonfinite-json

Conversation

@fabrizzio-dotCMS

@fabrizzio-dotCMS fabrizzio-dotCMS commented Jul 9, 2026

Copy link
Copy Markdown
Member

Proposed Changes

Follow-up to #36478. The base fix (non-finite _score → HTTP 500 "JSON does not allow non-finite numbers.") landed on main via #36480 with unit coverage only. This PR adds the missing end-to-end integration coverage for the actual production trigger, and documents the failure class so it does not recur.

No production code change — this is test + docs on top of the merged fix.

  • Integration testsESContentResourcePortletTest (already registered in MainSuite2a), covering both affected endpoints with the reliable NaN trigger (a query that sorts by a field without track_scores):
    • /api/es/search (search()) → field-sorted query → HTTP 200 with each hit's _score = null.
    • /api/es/raw (searchRaw(), body read from the request input stream) → same query → HTTP 200 with _score = null. /api/es/raw shares the toLegacyEsJson adapter, so it is subject to the same regression.
    • Control: track_scores: true keeps a finite _score (the guard must not alter legitimate scores).
  • Docsdocs/backend/OPENSEARCH_MIGRATION.mdKnown Gotchas → new section "Non-finite numbers (NaN/Infinity) in manual JSON serialization".

Why this matters

The existing integration tests only exercise relevance-scored bool/term queries, so the field-sorted path that produces NaN scores in production was never covered end-to-end. The unit test proves the mapping logic; these ITs prove the full REST → phase-aware SearchAPI → neutral response → legacy-wire adapter path returns valid JSON.

Documented gotcha (prevention)

Any float/double serialized from an underlying search/index/DB/compute API can be non-finite (_score is NaN on field-sorted / filter / constant_score / aggregation-only queries; also suggester scores, aggregation metrics, computed ratios). The two serializers fail differently:

Serializer Behavior on non-finite Symptom
dotCMS strict JSONObject/JSONArray testValidity() throws — eagerly in put(...) and again at serialization in numberToString HTTP 500
Jackson ObjectMapper Writes bare NaN/Infinity tokens (not valid JSON) Strict client parsers reject the response

Rule: coerce non-finite float/double to null (ES-native wire format) before it reaches the serializer.

Checklist

  • Tests — 3 new integration cases in ESContentResourcePortletTest (registered in MainSuite2a); module test-compile passes.
  • Translations — N/A (no user-facing strings).
  • Security Implications Contemplated — none; test + docs only, no schema/index/API-contract change.

Notes

  • The base fix is already on main (fix(search): guard non-finite _score in /api/es/search response (#36478) #36480); this PR does not re-touch ESContentResourcePortlet.
  • A codebase audit for other manual-serialization sinks found no additional reachable failure point (suggester score is same-sink but finite in practice; the AI vector-search distance path was verified as a false positive — model embeddings are never zero-magnitude, so pgvector cosine distance stays finite).

This PR relates to: #36478

🤖 Generated with Claude Code

This PR fixes: #36478

…gotcha (#36478)

Follow-up hardening for the #36478 regression (non-finite `_score` → HTTP 500
"JSON does not allow non-finite numbers", fixed on main by #36480). The base fix
had unit coverage only; the field-sorted trigger was never exercised end-to-end.

- Integration coverage in ESContentResourcePortletTest (already registered in
  MainSuite2a) for both affected endpoints:
    * /api/es/search  — field-sorted query (NaN score) → HTTP 200, _score: null
    * /api/es/raw     — same query via searchRaw() request body → HTTP 200, _score: null
    * control: track_scores=true keeps a finite _score (guard must not alter finite scores)
- Document the failure class in docs/backend/OPENSEARCH_MIGRATION.md (Known Gotchas):
  manual JSON serialization of values sourced from an underlying API that can return
  NaN/Infinity — the strict dotCMS JSONObject throws (500), Jackson emits invalid
  JSON tokens; coerce non-finite float/double to null (ES-native wire format).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Claude finished @fabrizzio-dotCMS's task in 3m 22s —— View job


PR Review — test(search): /api/es IT coverage + docs for non-finite _score (#36478)

  • Read the diff (ESContentResourcePortletTest.java +163, OPENSEARCH_MIGRATION.md +45)
  • Verified against production adapter (ESContentResourcePortlet.toLegacyEsJson / finiteOrNull)
  • Verified test wiring (mock request chain, searchRaw input-stream stubbing, suite registration)
  • Reviewed for bugs, test correctness, and convention adherence

This is a test + docs PR with no production code change; the base guard already merged via #36480. I checked the new tests against the real adapter and mock infrastructure.

New Issues

No blocking issues found.

Observations (non-blocking)

  • 🟡 Medium: dotcms-integration/.../ESContentResourcePortletTest.javaSuite convention. docs/backend/OPENSEARCH_MIGRATION.md:767 states "All tests related to this migration must be added to the OpenSearchUpgradeSuite." These cases exercise the regression introduced by the phase-aware SearchAPI cutover (feat(search): route /api/es/raw & /api/es/search through phase-aware SearchAPI (#36396) #36398), so they are arguably migration-related, yet they live in ESContentResourcePortletTest (registered in MainSuite2a:73). Assumption: the class is a pre-existing, general REST-endpoint test and placing the new cases alongside its sibling test_search_* methods is the intended home. What to verify: confirm this is the desired suite rather than OpenSearchUpgradeSuite. Reasonable as-is; flagging only so it's a conscious choice.

Things I verified as correct

  • NaN trigger is reliable. A field-sorted query (sort: [{moddate: desc}]) without track_scores produces non-relevance-scored hits → _score = NaN in ES/OS, which finiteOrNull (ESContentResourcePortlet.java:378) coerces to JSONObject.NULL. moddate is a core sortable date field, so the sort resolves. The assertions (isNull("_score")) match the intended null wire shape.
  • Control test is meaningful. test_search_..._withTrackScores_preservesFiniteScore asserts !isNull("_score"), correctly proving the guard doesn't null out legitimate finite scores.
  • searchRaw input-stream stubbing works. createHttpRequestWithBody calls when(request.getInputStream()) on a MockHeaderRequest wrapper chain that bottoms out in a Mockito mock (MockHttpRequestIntegrationTest); the wrapper delegates getInputStream() (not overridden), so the stub registers on the underlying mock — same delegation pattern already used for getContentType(). searchRaw reads the body via IOUtils.toString(request.getInputStream()), and MockServletInputStream over a ByteArrayInputStream satisfies it. The false overload of assertHitScoresAreNull correctly skips the esresponse wrapper (searchRaw returns the ES-wire object directly).
  • Both endpoints share the sink. search() and searchRaw() both funnel through toLegacyEsJsonhitsToLegacyJson, so covering both is the right end-to-end scope.
  • Docs are accurate. The gotcha section correctly distinguishes the strict JSONObject (throws → HTTP 500) vs Jackson (emits bare NaN tokens) failure modes, and the "coerce to null" rule matches ES-native XContent behavior. Imports (ByteArrayInputStream, ServletInputStream, ReadListener, StandardCharsets, etc.) are all present.

Verdict: LGTM — solid, well-targeted regression coverage that closes the exact end-to-end gap the unit test couldn't reach. The one item above is a convention check, not a defect.

issue-36478-harden-nonfinite-json

@mergify

mergify Bot commented Jul 10, 2026

Copy link
Copy Markdown

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@fabrizzio-dotCMS
fabrizzio-dotCMS added this pull request to the merge queue Jul 10, 2026
Merged via the queue into main with commit 6befcbf Jul 10, 2026
62 checks passed
@fabrizzio-dotCMS
fabrizzio-dotCMS deleted the issue-36478-harden-nonfinite-json branch July 10, 2026 18:21
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 Area : Documentation PR changes documentation files

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

fix(search): /api/es/search returns HTTP 500 "JSON does not allow non-finite numbers" on field-sorted and aggregation/filter queries

2 participants