Skip to content

fix(block-editor): guard StoryBlock JSON parsing against non-object scalars#35469

Merged
wezell merged 3 commits intomainfrom
fmontes/content-search-fix
Apr 27, 2026
Merged

fix(block-editor): guard StoryBlock JSON parsing against non-object scalars#35469
wezell merged 3 commits intomainfrom
fmontes/content-search-fix

Conversation

@fmontes
Copy link
Copy Markdown
Member

@fmontes fmontes commented Apr 27, 2026

Fixes regression from #35412: /api/content/_search throws Cannot deserialize value of type LinkedHashMap from Integer when a Story Block references a contentlet with scalar field values.

Fix

1. JSON guard. JsonUtil.isValidJSON() returns true for scalars (42, "foo", [1,2]). Replaced with isJsonObject() (valid JSON + starts with {) at the 4 call sites feeding toMap(). HTML strings correctly skip refresh (no {).

2. Failure isolation. One bad contentlet was killing the whole search response. Now:

  • ContentletTransformer.refreshStoryBlockReferences catches/logs — bad contentlet returns un-refreshed.
  • StoryBlockAPIImpl.isRefreshed isolates each child block.
  • StoryBlockAPIImpl.refreshContentlet isolates each field (falls back to raw value).

QA

Re-run QA from #35408 (PR #35412) and #35403 (PR #35413).

Plus: via Content REST API, create a contentlet with raw HTML as the Block Editor field value (e.g. <p>hello</p>). Confirm create + update + _search + edit-UI all work without MismatchedInputException in logs.

Tests

  • test_refreshStoryBlockValueReferences_with_non_object_json_scalars — numeric/string/boolean/array/HTML inputs
  • test_refreshStoryBlockValueReferences_isolates_bad_child_block — inner isolation
  • test_refreshReferences_does_not_throw_on_malformed_nested_block — API-level resilience

…calars

JsonUtil.isValidJSON() returns true for any valid JSON token, including
bare scalars like 42, "foo", or [1,2,3]. After #35412 added recursive
hydration of nested contentlet field values via refreshNestedStoryBlockValues,
scalar field values from related contentlets started reaching toMap() and
failing with "Cannot deserialize value of type LinkedHashMap from Integer",
breaking /api/content/_search whenever a Story Block field referenced a
contentlet with numeric/boolean fields.

Add an isJsonObject() guard that requires the value to be a JSON object
(starts with '{'), and apply it at the four entry points that feed toMap.
HTML body content is correctly skipped (fails the '{' check).

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@fmontes fmontes requested a review from Copilot April 27, 2026 18:35
@github-actions github-actions Bot added the Area : Backend PR changes Java/Maven backend code label Apr 27, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 27, 2026

Claude finished @fmontes's task in 2m 36s —— View job


Review

  • Gather context
  • Review StoryBlockAPIImpl.java
  • Review ContentletTransformer.java
  • Review StoryBlockAPITest.java
  • Post findings

Issues

1. ESContentFactoryImpl.processCachedContentlet is unprotected — the resilience fix is incomplete

refreshStoryBlockValueReferences still re-throws at line 217 of StoryBlockAPIImpl:

} catch (final Exception e) {
    ...
    throw new DotRuntimeException(errorMsg, e);   // ← still throws
}

refreshReferences propagates that exception. ContentletTransformer.refreshStoryBlockReferences now catches it, but there is a second, unguarded call site in ESContentFactoryImpl.processCachedContentlet (line 858):

final StoryBlockReferenceResult storyBlockRefreshedResult =
        APILocator.getStoryBlockAPI().refreshReferences(cachedContentlet);   // no try-catch

The isJsonObject guard eliminates the specific scalar bug, but any other toMap() failure on a valid-but-malformed JSON object will still reach this unguarded call site and can break cache lookups. Either wrap processCachedContentlet's call site or remove the rethrow and return the original contentlet instead, consistent with how isRefreshed handles per-block failures. Fix this →


2. Double Javadoc block in StoryBlockAPITest.java

There are two consecutive /** ... */ blocks immediately before test_refreshReferences_does_not_throw_on_malformed_nested_block. The first one (referencing refreshStoryBlockValueReferences) is a copy-paste leftover and should be removed. Java silently drops it but it misleads readers. Fix this →


3. Missing getValue() assertions for string, boolean, and array inputs

(Copilot also flagged this.) test_refreshStoryBlockValueReferences_with_non_object_json_scalars asserts isRefreshed() == false for "hello", true, and [1,2,3] but does not call assertEquals on the returned value. The Javadoc says the original value must be returned unchanged — the test should prove that. Fix this →


4. test_refreshStoryBlockValueReferences_isolates_bad_child_block only asserts non-null

The only assertion is assertNotNull(result). An isRefreshed() check and/or getValue() check would make this a meaningful regression guard rather than a trivially-passing test. Fix this →


Minor

  • isJsonObject calls startsWith("{") before isValidJSON — correct order; no issue.
  • The catch (Exception e) blocks in isRefreshed and refreshContentlet are broad but intentional and logged. Acceptable for a resilience boundary.
  • refreshNestedStoryBlockValues is not guarded at the isRefreshed call site (line 835), but the round-trip through toJson before refreshStoryBlockValueReferences means the isJsonObject guard will always pass there. Not a bug.

Item 1 is the real risk; 2–4 are test quality issues.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Guards Story Block JSON parsing in StoryBlockAPIImpl so the transformer pipeline won’t attempt to deserialize non-object JSON tokens (scalars/arrays) into a LinkedHashMap, preventing /api/content/_search failures when related contentlets contain scalar defaults that look like valid JSON.

Changes:

  • Introduced isJsonObject(String) and applied it to all call sites that would otherwise feed non-object JSON into toMap().
  • Updated Story Block refresh/dependency extraction paths to skip parsing when the root JSON token isn’t an object.
  • Added an integration test covering scalar/array/HTML inputs to ensure no exception is thrown.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
dotCMS/src/main/java/com/dotcms/contenttype/business/StoryBlockAPIImpl.java Adds isJsonObject() and uses it to guard toMap() entry points against non-object JSON values.
dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockAPITest.java Adds regression test exercising numeric/string/boolean/array/HTML inputs for refreshStoryBlockValueReferences.

fmontes and others added 2 commits April 27, 2026 12:40
…ntentlet and per field

A single bad contentlet was able to abort the entire ContentletTransformer
pipeline, taking down /api/content/_search and any other code path that
materializes contentlets in bulk. Add two layers of failure isolation:

1. Outer (ContentletTransformer.refreshStoryBlockReferences): if the
   StoryBlock API throws for one contentlet, log and return that contentlet
   with un-refreshed (but valid) Story Block data instead of aborting the
   whole transform.

2. Inner (StoryBlockAPIImpl): wrap each child block in isRefreshed() and
   each field in refreshContentlet() so a single malformed nested reference
   or a single broken field on a related contentlet does not prevent the
   rest of the parent's hydration. Bad fields fall back to their raw value.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Inner: refreshStoryBlockValueReferences must not throw when one nested
  child block is malformed (missing 'type' key). Asserts the per-child
  isolation in StoryBlockAPIImpl.isRefreshed.
- Outer: refreshReferences(Contentlet) must not propagate exceptions when
  the stored Story Block JSON contains a malformed nested child. This is
  the boundary ContentletTransformer relies on to keep a single bad
  contentlet from aborting an entire /api/content/_search response.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@wezell wezell left a comment

Choose a reason for hiding this comment

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

Not going to be worse

@wezell wezell enabled auto-merge April 27, 2026 19:37
@wezell wezell added this pull request to the merge queue Apr 27, 2026
Merged via the queue into main with commit a8ce11c Apr 27, 2026
61 checks passed
@wezell wezell deleted the fmontes/content-search-fix branch April 27, 2026 20:36
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.

Block Editor: Nested contentlet updates don't propagate through relationship chains

3 participants