feat: Include content block in the search embedding - EXO-88570

[mkrout]

This commit broadcasts page layout/permissions update events for reactive indexing.

[boubaker]

Review — eXIP 7.3.0.13 "Page Searching" (Layout part)

This PR is one of three implementing the specification. The full cross-repo review lives on Meeds-io/social#5974 — please read it there:

👉 Meeds-io/social#5974 (comment)

It covers social#5974, layout#562 (this PR) and notes#1747 as one changeset, ordered most-severe first.

What concerns this PR specifically

🟠 High — PR build is red. layout-service fails to compile with 4 × cannot find symbol on io.meeds.social.cms.plugin.PageUrlResolver / io.meeds.social.cms.service.PageUrlResolverService. This is merge-ordering: social#5974 has to publish its social-component-api snapshot first. Worth re-running once it lands, so the green build is on record before merge.

🟡 Medium — Only 2 of ~7 page-mutating paths broadcast. PageLayoutService.java:363 and :437 broadcast, but createPage (:220), clonePage (:256), updatePageApplicationPreferences (:296/:303) and restorePageLayout (:378) all persist page state silently. updatePageApplicationPreferences is the sharpest: the Single Note View portlet preference is what names the CMSSetting, so changing it swaps which note the page shows while the index keeps serving the old content.

🟠 High (cross-repo) — Nothing ever unindexes. No production unindex call exists anywhere across the three PRs, so a deleted page keeps a live ES document carrying its old permissions. The spec assigns this to Layout — "Existing page lifecycle events (LayoutService) trigger unindex on page deletion" — so the page-deletion event Social needs is missing from this PR. Details in the main review.

Nice touch on LayoutPageUrlResolver: routing primitives genuinely do live in webui, and resolving the nav node here rather than exposing that dependency to Social is the right call. LayoutPageUrlResolverTest covers the null-root, no-match, match and throwing paths.

Classification: N1 for the changeset as a whole (ACL/permission filtering and a trust boundary on the Social side). This PR should not merge ahead of, or independently of, social#5974.

🤖 Generated with Claude Code

[boubaker]

Review round 2 — eXIP 7.3.0.13 "Page Searching" (Layout part)

Full cross-repo re-review on Meeds-io/social#5974:

👉 Meeds-io/social#5974 (comment)

(round 1: Meeds-io/social#5974 (comment))

This PR is unchanged at 2a3df76 since round 1, while social#5974 moved forward — which changed the significance of one finding here.

Round 1 status for this PR

#	Sev	Finding	Status
1	🟠	PR build red (cannot find symbol PageUrlResolver ×4)	⏳ Merge ordering only — needs social#5974's social-component-api snapshot published, then re-run
2	🟡	Only 2 of ~7 page-mutating paths broadcast	❌ Open — and now escalated, see below
3	🟠	Nothing ever unindexes (cross-repo)	⚠️ Partial — Social added a reconcile path, but it depends on events this PR doesn't broadcast

---

🟠 High (escalated from 🟡) — the missing broadcasts now make a shipped Social feature inert

PageLayoutService.java:296-332

    layoutService.save(application.getState(), portletPreferences);
    // no broadcast — yet this is what repoints a Single Note View at another note
  }

Social's new revision added unindexDetachedBlocks, whose javadoc states it handles a block "removed from its layout, or its content-block portlet preference repointed elsewhere". The second half never happens: updatePageApplicationPreferences is the method that repoints that preference, and it broadcasts nothing. createPage (:220) and restorePageLayout (:378) are silent too — so a page created from a template already carrying a content block, or restored to its shipped layout, never enters the search index at all.

In round 1 this was a 🟡 gap in event coverage. Now that Social ships code whose documented behaviour depends on these events, it's a 🟠: the feature reads as implemented and does nothing.

Fix: broadcast PAGE_UPDATED_EVENT from updatePageApplicationPreferences, createPage and restorePageLayout, alongside the two existing broadcasts at :363 and :437.

---

🟡 Medium — page deletion (cross-repo, no change needed here after all)

Round 1 suggested Layout should add a page-deletion broadcast. Correction: it doesn't need to. The portal already broadcasts LayoutService.PAGE_REMOVED (org.exoplatform.portal.config.DataStorage.pageRemoved), covering every deletion path including the bulk removePages(siteKey) in SiteLayoutService.deleteSite. Social can listen to it directly — details in the main review. Nothing to do in this PR for that item.

---

LayoutPageUrlResolver remains a clean piece of work: resolving the nav node here rather than exposing gatein's webui routing primitives to Social is the right call, and its test covers the null-root, no-match, match and throwing paths.

Classification: N1 for the changeset as a whole. This PR should not merge ahead of, or independently of, social#5974.

🤖 Generated with Claude Code

[mkrout]

Fixes applied — response to round 2 review

Pushed at b1f3d5a4. Addresses finding B from social#5974's round 2 review (cross-repo review, this PR's part).

B. 🟠 Detached-block path unreachable — ✅ Fixed

PageLayoutService.createPage(), updatePageApplicationPreferences() and restorePageLayout() now broadcast PAGE_UPDATED_EVENT, same as updatePageLayout()/updatePagePermissions() already did. updatePageApplicationPreferences was the sharpest gap — it's what repoints a Single Note View's portlet preference, exactly the case the social-side listener's javadoc claimed to handle but couldn't reach.

Tests: added verify(listenerService).broadcast(...) assertions to the three corresponding PageLayoutServiceTest cases. Full layout-service suite green.

Unrelated discovery, now resolved

While verifying this locally, layout-service failed to compile against the checked-out gatein-portal — ImportMode.RESTORE_DEFAULTS and UserPortalConfigService.isDefaultPage(PageKey) (used by restorePageLayout, pre-existing code from EXO-88526/EXO-88530) don't exist on that snapshot. Root cause: the local gatein-portal checkout was 4 commits behind origin/develop. Fast-forwarded to catch up — no conflict with this PR's files, unrelated to the fixes above, just noting it in case others hit the same stale-snapshot issue.

🤖 Generated with Claude Code

[boubaker]

Review round 3 — eXIP 7.3.0.13 "Page Searching" (Layout part)

Full cross-repo review on Meeds-io/social#5974:

👉 Meeds-io/social#5974 (comment)

(earlier rounds: 1 · 2)

Re-reviewed at b1f3d5a. All findings against this PR are now resolved.

Status

#	Sev	Finding	Status
1	🟠	Only 2 of ~7 page-mutating paths broadcast — leaving Social's unindexDetachedBlocks unable to fire for the case its javadoc names	✅ Fixed — createPage (:250), updatePageApplicationPreferences (:330) and restorePageLayout (:397) now broadcast PAGE_UPDATED_EVENT, each with a verify(listenerService).broadcast(...) assertion in PageLayoutServiceTest
2	🟡	Page deletion had no broadcast	✅ Resolved elsewhere — correctly, via Meeds-io/portal#1303 rather than here. Routing it through the portal's own remove(Page) covers site deletion and node deletion too, which a Layout-side broadcast would have missed
3	🟠	PR build red	⏳ Merge ordering only — the four errors are all PageUrlResolver / PageUrlResolverService, awaiting social#5974's social-component-api snapshot

The three broadcasts landed exactly where they matter: updatePageApplicationPreferences is what repoints a Single Note View's portlet preference at a different note, and without it Social's reconcile path was documented-but-dead. Adding the assertion to the existing tests rather than writing new ones keeps the diff honest and small.

LayoutPageUrlResolver remains unchanged and is still the right shape — resolving the nav node here rather than exposing gatein's webui routing primitives to Social.

Merge order

portal#1303 → social#5974 → layout#562 / notes#1747

Nothing further needed in this PR; re-run CI once social publishes.

Classification: N1 for the changeset as a whole (ACL filtering and a trust boundary on the Social side, plus core event broadcasting in portal). This PR should not merge ahead of, or independently of, social#5974.

🤖 Generated with Claude Code

[boubaker]

Review round 4 — eXIP 7.3.0.13 "Page Searching" (Layout part)

Full cross-repo review on Meeds-io/social#5974:

👉 Meeds-io/social#5974 (comment)

(earlier rounds: 1 · 2 · 3)

Re-reviewed at 7b09d4b. This revision adds real work in NavigationLayoutService and closes the last carried-over finding.

Status

#	Sev	Finding	Status
1	🟠	Only 2 of ~7 page-mutating paths broadcast	✅ Fixed in the previous revision
2	🟢	pagePath frozen at index time — stale after a navigation rename/move (round-1 finding, long open)	✅ Fixed — updateNode and moveNode now broadcast PAGE_UPDATED_EVENT for every page in the affected subtree
3	—	Page unreachable after its last navigation node is deleted	✅ New, and well judged — see below
4	🟠	PR build red	⏳ Merge ordering only — the four errors are all PageUrlResolver/PageUrlResolverService, awaiting social's snapshot

On the new PAGE_UNREACHABLE_EVENT

Deleting the last navigation node pointing to a page doesn't delete the page, so a content block on it would have stayed indexed forever behind a search result leading nowhere. Three things about this implementation are worth calling out:

• It is deliberately not LayoutService#PAGE_REMOVED, and the javadoc says why: that event means the page's data is gone, and a listener acting on it is entitled to clean up everything attached to the page — which would destroy live data here. Conflating the two would have been the easy mistake.
• The subtree is collected before the deletion, because NodeEntity#children cascades — so descendants' pages are caught too, not just the deleted node's own page.
• Reachability is checked against a single tree load for the whole set rather than one load per page, and there's a test asserting exactly that (verify(navigationService, times(2)).loadNode(SITE_KEY)).

I verified NavigationService.loadNode(SiteKey) does use Scope.ALL, so the subtree walk genuinely sees descendants rather than just the root's immediate children.

---

🟡 Medium — a full navigation-tree load on every node property change

NavigationLayoutService.java:207 · :463

    navigationService.updateNode(nodeId, nodeState);
    broadcastPagesUpdated(nodeData.getSiteKey(), nodeId, username);   // unconditional

broadcastPagesUpdated walks loadNode(siteKey) — the whole site tree, Scope.ALL — and then re-indexes every page in the node's subtree. But updateNode fires for any node property: label, visibility, start/end date. Only a rename changes the page URI, so toggling a section's visibility now costs a full navigation-tree load plus a reindex of every descendant page, for nothing.

Fix: compare the node's name against the incoming model (and the parent, for moveNode) and skip the broadcast when the URI can't have changed.

---

🟢 Nit — cross-repo string contracts

layout.page.updated, layout.page.permissions.updated and layout.page.unreachable are each defined twice — once here, once in Social — with a comment explaining the duplication. Layout already depends on social-component-api; a small constants holder there would turn these into compile-time contracts. Same applies to "name" (CMSPortlet.NAME ↔ PageContentBlockUtils.WIDGET_SETTING_NAME_PREFERENCE).

---

Nothing blocking here. Merge order: portal#1303 → social#5974 → layout#562 / notes#1747; re-run CI once social publishes.

Classification: N1 for the changeset as a whole.

🤖 Generated with Claude Code

[boubaker]

Review round 5 — eXIP 7.3.0.13 "Page Searching" (Layout part)

Full cross-repo review on Meeds-io/social#5974:

👉 Meeds-io/social#5974 (comment)

(earlier rounds: 1 · 2 · 3 · 4)

Re-reviewed at fd265f4. No findings. My round-4 item is closed, and two real bugs were found here that I had missed.

Status

#	Sev	Finding	Status
1	🟡	Full navigation-tree load on every node property change	✅ Fixed — better than proposed, see below
2	🟢	Cross-repo string contracts duplicated	❌ Open (cosmetic)
3	🟠	PR build red	⏳ Merge ordering only — the four errors are still PageUrlResolver/PageUrlResolverService, awaiting social's snapshot

On the updateNode fix

I suggested guarding the broadcast by comparing the node's name. The better answer was found instead: NavigationUpdateModel carries no name at all — only label, pageRef, target, visibility, schedule, icon and labels — so updateNode can never move a URI, and the subtree reindex wasn't merely wasteful, it was unnecessary in principle. I verified the model; the reasoning in the comment is accurate.

What replaced it covers a case I hadn't considered: re-pointing a node at a different page. The new page becomes reachable at this URI, and the old one may have just lost its last node — in which case its indexed block would keep a pagePath now serving different content. Both directions are handled, and updateNodeDoesNotReindexAnythingWhenThePageReferenceIsUnchanged asserts the tree isn't loaded at all for a label edit.

Two bugs found here, not by the review

• moveNode looked the subtree up in the wrong site. A cross-site move is permitted — the destination parent's site is what the ACL check runs against — so after the move the node exists only in the destination tree. Using the source site key would have found nothing and silently left every page in the moved subtree indexed with its pre-move URI. moveNodeAcrossSitesLooksTheSubtreeUpInTheDestinationSite pins it, including verify(navigationService, never()).loadNode(SITE_KEY).
• createNode didn't reindex the page it points at. This was a one-way door: once a page's blocks were unindexed because their last node was deleted, no later event would have brought them back. Draft nodes are correctly excluded, with a test.

Nothing to change here. Merge order: portal#1303 → social#5974 → layout#562 / notes#1747; re-run CI once social publishes.

Classification: N1 for the changeset as a whole.

🤖 Generated with Claude Code