Block Editor: propagate nested content updates through relationship-chain hydration

[Copilot]

nested-block-editor.mp4

• Diagnose root cause from CI failure logs
• Fix refreshContentlet to not throw JsonParseException on non-JSON _raw default values
• Parallel validation (CodeQL clean; code review false-positive addressed in comments)

[Copilot]

Pull request overview

Updates Story Block hydration so that nested Story Block references embedded inside hydrated relationship payloads are also refreshed, enabling relationship-chain updates to propagate correctly.

Changes:

• Add recursive traversal to refresh Story Block values found inside nested relationship payload maps/lists (with a bounded traversal depth).
• Reduce the nested refresh traversal ceiling and document why it’s safe given the existing relationship depth cap (0–3).
• Add an integration test covering nested publish propagation through a Story Block → relationship → Story Block chain.

Reviewed changes

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

File	Description
dotCMS/src/main/java/com/dotcms/contenttype/business/StoryBlockAPIImpl.java	Adds bounded recursive refresh of nested Story Block values within relationship-hydrated payload structures.
dotcms-integration/src/test/java/com/dotcms/contenttype/business/StoryBlockAPITest.java	Adds coverage asserting nested Story Block content refreshes after publishing a deeply nested referenced contentlet.

[fmontes]

@copilot fix conflicts

[fmontes]

@copilot resolve the merge conflicts in this pull request

[claude]

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

---

Review

• Read changed files and understand the approach
• Analyze for bugs, edge cases, and design issues

---

Overall: The StackOverflow fix (ThreadLocal cycle detection) and the nested Story Block refresh are both correct in their core logic. Three things worth looking at:

---

1. refreshNestedStoryBlockValues is called for every non-StoryBlock field, not just relationship fields

StoryBlockAPIImpl.java:756–758

} else {
    dataMap.putIfAbsent(field.variable(),
            this.refreshNestedStoryBlockValues(value, contentlet.getIdentifier(),
                    MAX_NESTED_STORY_BLOCK_REFRESH_DEPTH));
}

This runs for text fields, binary fields, date fields, tag fields — everything that isn't a StoryBlockField. For each String-typed field value, the code falls through to the String branch at line 841 and calls refreshStoryBlockValueReferences, which calls JsonUtil.isValidJSON(value.toString()) on every string — including large WYSIWYG bodies. Only RelationshipField values can hold nested contentlet maps that might embed a Story Block. The remaining field types can never match isStoryBlockMap, so they're pure wasted work.

Consider restricting the call:

} else if (field instanceof RelationshipField) {
    dataMap.putIfAbsent(field.variable(),
            this.refreshNestedStoryBlockValues(value, contentlet.getIdentifier(),
                    MAX_NESTED_STORY_BLOCK_REFRESH_DEPTH));
} else {
    dataMap.putIfAbsent(field.variable(), value);
}

Fix this →

---

2. Map copy-on-write rebuilds from a second entrySet() iteration — assumes stable order

StoryBlockAPIImpl.java:804–811

refreshedMap = new LinkedHashMap<>(valueMap.size());
for (final Map.Entry<String, Object> existingEntry : valueMap.entrySet()) {
    if (existingEntry.getKey().equals(entry.getKey())) {
        break;
    }
    refreshedMap.put(existingEntry.getKey(), existingEntry.getValue());
}

The inner loop iterates valueMap.entrySet() a second time to copy entries that precede the changed one. HashMap iteration order is stable within a single JVM session (no structural modifications), so this is safe in practice — and in practice these maps come from Jackson deserialization (which uses LinkedHashMap). But the method signature accepts Map<String, Object> with no ordering contract, so a caller passing a HashMap would see entries silently reordered. A safer alternative is to collect entrySet() to a List once before the outer loop, then index into it for the rebuild.

---

3. isCurrentDepthEmpty computed outside the try block — NPE skips finally

StoryBlockAPIImpl.java:596–599

final HttpServletRequest request = HttpServletRequestThreadLocal.INSTANCE.getRequest();
final boolean isCurrentDepthEmpty = request.getAttribute(CURRENT_DEPTH_ATTR) == null;
try {
    ...
} finally {
    if (isCurrentDepthEmpty) {
        ...
        REFRESH_PROCESSING_IDENTIFIERS.remove();
    }
}

If getRequest() returns null (programmatic/background invocation with no servlet request on the thread), the NPE on line 599 is thrown before the try block, so the finally does not execute. In the original code both lines were inside the try. Since no ThreadLocal entries are added before this point, there's no actual leak — but the finally placement is now load-bearing for the new ThreadLocal cleanup, and "NPE before try skips finally" is a silent contract that future maintainers may not notice. At minimum, add a null check for request and return early, or move both lines back inside the try.

---

Minor

• Javadoc on refreshNestedStoryBlockValues (line 770) says parentContentletIdentifier is "used to prevent self-refresh loops." It's passed through to refreshStoryBlockValueReferences only for logging/error messages — the actual cycle prevention is the ThreadLocal in refreshBlockEditorDataMap. The comment is misleading.
• No test covers the case where nested is an unpublished draft (only live version). Worth adding, but not blocking.