Skip to content

fix: Pin a database error while saving a video call as a call error - EXO-91299 - #421

Merged
boubaker merged 2 commits into
feature/maintenancefrom
fix/EXO-91299-call-storage-errors
Oct 21, 2026
Merged

boubaker merged 2 commits into
feature/maintenancefrom
fix/EXO-91299-call-storage-errors

Conversation

@boubaker

@boubaker boubaker commented Oct 19, 2026 •

Copy link
Copy Markdown
Member

Symptom: when the database failed inside an EntityManager call made while saving a video call, the failure left the service as an UndeclaredThrowableException instead of the service's own call error. That error depends on the operation. Starting, stopping, removing, joining or leaving a call reports an InvalidCallException wrapping the StorageException of the failed step, whether that step is the read of the call they start with or a later read, such as the participant row of a space member. Updating a call, adding a guest and updating the invitees report a failed read of the call the same way, and a later failure as a StorageException naming the step. Adding a participant and creating a call report a failed read of a participant, and updating the participants a failed read of the call, as a StorageException naming the step. Creating a call also checks that no call already has its id: a failed read there is logged as a warning and the creation goes on, as that check intends, where it failed the creation before. A failure at commit was already reported as a call storage error.

Cause: the kernel DAOs' EntityManager proxy delivered the EntityManager's exception wrapped in an UndeclaredThrowableException, which none of the service's storage catches recognises. It is fixed in portal's EntityManagerHolder, Meeds-io/portal#1344.

Fix: WebConferencingServiceStorageErrorTest runs each of those operations through the real DAOs, the EntityManager proxy and the ExoTransactional aspect. The container, the EntityManagerService, the EntityManager and its transaction are mocked. The service is a spy whose userInfo, spaceInfo and getProvider return a user (none for an external guest), a space or a provider without reading them, and whose getCall, in the guest and invitation tests, returns a call without reading it. Each failure but the commit's is raised on an EntityManager call that reaches the database within a request: the read of the call an operation starts with, unless the request has read the call before, the check that no call has the id of the one being created, the read of a participant the request has not loaded yet (such as the row of a member who joined a space after its call started), or the persist of an invitation that flushes. The test checks that each failure is reported as the operation's error, that the creation of a call goes on after a failed check, and that a failure at commit, when adding a participant, is still reported as a storage error.

Depends on Meeds-io/portal#1344. WebConferencingServiceStorageErrorTest at a0c905d1:

  • with #1344's component/common classes on the test classpath: Tests run: 14, Failures: 0, Errors: 0, Skipped: 0;
  • with a portal.component.common 7.3.x-maintenance-SNAPSHOT built before #1344: Tests run: 14, Failures: 12, Errors: 1, Skipped: 0. Each of the 13 fails on the stubbed failure wrapped in an UndeclaredThrowableException; only the commit test passes, its failure being raised outside the proxy.

So the build of this PR passes once #1344 is published, and it merges after it.

Knowledge: Meeds-io/eng-standards#560

This change is classified N3 (at a0c905d1): no REST/DAO/schema/ACL surface touched, a test only (classification.md §3).

🤖 Generated with Claude Code

@boubaker
boubaker requested a review from ahamdi October 19, 2026 15:49
@boubaker

Copy link
Copy Markdown
Member Author

Self-review close-out — EXO-91299 (head b5e62179)

This PR is part of a delivery with Meeds-io/portal#1344, which carries the fix. The loop ran delivery-checks passes before each reviewer round, two reviewer rounds (the second one fresh), and a refute pass on the fixture finding.

# Source Sev Finding Status
1 delivery checks 🟡 Symptom claimed write-time failures, but those run at flush or commit, outside the proxy ✅ scoped to failures inside a proxied EntityManager call
2 delivery checks 🟢 The Fix line claimed every save, but join, leave and update were untested ✅ tests added for join (failing find), guest leave and update
3 delivery checks 🟡 Claimed a change in the error the user sees, but each CometD command answers the same server error either way ✅ claim removed; the tolerant read is pinned instead (getCallOfAMissingOwnerToleratesAFailedRemoval)
4 delivery checks 🟢 "at commit as well" covered only adding a participant ✅ Fix line says so
5 delivery checks 🟡 "the logged failure did not say what failed": the CometD log names the call and the request ✅ clause removed
6 delivery checks 🟢 Start/join/leave/stop/remove leave as InvalidCallException, not StorageException ✅ Symptom names both
7 round 1 🟢 Fixtures stubbed a database failure on persist/merge, where no SQL runs ✅ refute PARTLY: each failure is raised on the read that precedes the write, or on the invitation persist over a removed key, which flushes (Javadoc says why)
8 round 1 🟢 Tribe task wording claimed a user-visible change ✅ task description updated
— round 2 (fresh) — — CLEAN

Verified conform: the test runs the real CallDAO, ParticipantDAO, OriginDAO and InviteDAO through the EntityManagerHolder proxy and the woven ExoTransactional aspect. Only the EntityManagerService, the EntityManager and its transaction are mocked, the transaction's active state included. Every assertion checks the operation's own error type and that its cause is the stubbed engine failure.

Tests at b5e62179:

  • WebConferencingServiceStorageErrorTest against the published portal 7.3.x-maintenance-SNAPSHOT, which lacks the fix: Tests run: 13, Failures: 11, Errors: 1. Each wrapped cause is the stubbed failure; only the commit-time test passes.
  • The same test against #1344's classes (-Dmaven.test.dependency.excludes=io.meeds.portal:portal.component.common -Dmaven.test.additionalClasspath=<portal>/component/common/target/classes, built through process-classes): Tests run: 13, Failures: 0.
  • The services module against those classes, -Pcoverage: Tests run: 30, Failures: 0, Errors: 0, coverage checks met.
  • This PR's own build fails until #1344 is published.

Classification: N3 at b5e62179. It adds a test only and touches no REST, DAO, schema or ACL surface.

boubaker added a commit to Meeds-io/portal that referenced this pull request Oct 19, 2026
…oxy - EXO-91299 (#1344)

**Symptom**: a database error raised inside an EntityManager call of a
kernel DAO (find, persist, merge, remove, refresh...) skipped every
catch naming that exception. In video calls, such a failure while saving
a call, such as reading the call or a participant before writing it,
left the service as an UndeclaredThrowableException instead of the
service's own call error (a StorageException naming the step, wrapped in
an InvalidCallException when starting, joining, leaving, stopping or
removing a call), and the code that tolerates a storage error, such as
removing a call whose owner no longer exists while reading it, failed
the whole read instead. A failure at commit was not affected: the commit
does not go through the proxy.

**Cause**: the `EntityManagerHolder` proxy invoked the thread's
EntityManager by reflection without unwrapping the
`InvocationTargetException`, which the JDK proxy then wrapped in an
`UndeclaredThrowableException`: a catch of a JPA exception, an
`IllegalArgumentException` or an `IllegalStateException` does not
recognise it.

**Fix**: the proxy rethrows the exception the EntityManager threw.
`EntityManagerHolderTest` pins it on a real EntityManager: a find with a
null id, a persist of a detached entity and a refresh of a removed row
each reach the DAO's caller as the engine's own exception, and the
EntityManager the proxy opened outside a request is closed after a
failed call.

**Impact**: every caller of a kernel DAO (a `GenericDAOJPAImpl`
subclass, in every addon) now receives the exception an EntityManager
method called on the proxy throws (find, persist, merge, remove,
refresh, flush, getReference...); the execution of a query never went
through the proxy and is unchanged. Only a catch narrower than
`Exception` or `RuntimeException` behaves differently for an exception:
- a catch of the EntityManager's exception, or of
`IllegalArgumentException` or `IllegalStateException`, now fires.
Hibernate has by then marked the transaction rollback-only for most of
them (not for `NoResultException`, `NonUniqueResultException` and the
lock and query timeouts): inside an outer `@ExoTransactional` method,
such a catch that does not rethrow lets the aspect roll the transaction
back without an error, where the failure used to reach the caller;
- a catch of `UndeclaredThrowableException` around a kernel DAO call no
longer matches, and an `Error` the EntityManager throws now passes a
catch of `Exception` or `RuntimeException`, as an `Error` does anywhere
else.

A sweep of the direct callers in the org's repositories found the
catches that now fire only in web-conferencing, none of them inside an
outer transaction, and no catch of `UndeclaredThrowableException` around
a kernel DAO call. exoplatform/web-conferencing#421 pins the video-call
errors on this fix; its build passes once this fix is published.

Knowledge: Meeds-io/eng-standards#560

This change is classified N2 (at `ffe62c50f6`) — its approver must be an
Archi/Dev who knows it is N2, not an approval on AI review alone.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

@ahamdi ahamdi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AI review — Round #1

Scope: the single test file at b5e62179. The fix itself is in Meeds-io/portal#1344, which is merged. 2 inline findings: 🟡 1, 🟢 1.

Verified conform

  • The test drives the real CallDAO, ParticipantDAO, OriginDAO and InviteDAO through GenericDAOJPAImpl's EntityManager proxy and the woven ExoTransactional aspect, with a mocked container. It boots no Kernel, and it resets ExoContainerContext in @AfterEach.
  • The mocked transaction keeps its active state across begin/commit/rollback, so the aspect's own branching runs as in production.
  • Every assertion checks both the operation's own exception type and that its cause is the stubbed failure (assertSame), not just that something was thrown.
  • addParticipant, addGuest, updateInvites, updateParticipants and createCall pin failures that are reachable in a request: a find of a key not yet loaded, or the invite persist over a removed key, which flushes. The commit test pins that a commit failure is still reported as a storage error.
  • Knowledge: Meeds-io/eng-standards#560 is an open draft that carries this delivery's web-conferencing and portal pitfalls. That is the expected state until the code reaches develop.
  • The new file's header year (2026) is correct.

What it does well: it pins the portal fix through the real wiring, not through a mocked DAO, so the UndeclaredThrowableException regression is visible from the consumer that suffered it. The Javadoc on the flush and commit cases says why each failure is raised where it is.

Classification: N3. The PR adds a test only and touches no REST, DAO, schema or ACL surface. It must still be validated by an Architect/Developer and not merged on AI review alone.

🤖 Generated with Claude Code

exo-swf pushed a commit to Meeds-io/portal that referenced this pull request Oct 19, 2026
…oxy - EXO-91299 (#1344)

**Symptom**: a database error raised inside an EntityManager call of a
kernel DAO (find, persist, merge, remove, refresh...) skipped every
catch naming that exception. In video calls, such a failure while saving
a call, such as reading the call or a participant before writing it,
left the service as an UndeclaredThrowableException instead of the
service's own call error (a StorageException naming the step, wrapped in
an InvalidCallException when starting, joining, leaving, stopping or
removing a call), and the code that tolerates a storage error, such as
removing a call whose owner no longer exists while reading it, failed
the whole read instead. A failure at commit was not affected: the commit
does not go through the proxy.

**Cause**: the `EntityManagerHolder` proxy invoked the thread's
EntityManager by reflection without unwrapping the
`InvocationTargetException`, which the JDK proxy then wrapped in an
`UndeclaredThrowableException`: a catch of a JPA exception, an
`IllegalArgumentException` or an `IllegalStateException` does not
recognise it.

**Fix**: the proxy rethrows the exception the EntityManager threw.
`EntityManagerHolderTest` pins it on a real EntityManager: a find with a
null id, a persist of a detached entity and a refresh of a removed row
each reach the DAO's caller as the engine's own exception, and the
EntityManager the proxy opened outside a request is closed after a
failed call.

**Impact**: every caller of a kernel DAO (a `GenericDAOJPAImpl`
subclass, in every addon) now receives the exception an EntityManager
method called on the proxy throws (find, persist, merge, remove,
refresh, flush, getReference...); the execution of a query never went
through the proxy and is unchanged. Only a catch narrower than
`Exception` or `RuntimeException` behaves differently for an exception:
- a catch of the EntityManager's exception, or of
`IllegalArgumentException` or `IllegalStateException`, now fires.
Hibernate has by then marked the transaction rollback-only for most of
them (not for `NoResultException`, `NonUniqueResultException` and the
lock and query timeouts): inside an outer `@ExoTransactional` method,
such a catch that does not rethrow lets the aspect roll the transaction
back without an error, where the failure used to reach the caller;
- a catch of `UndeclaredThrowableException` around a kernel DAO call no
longer matches, and an `Error` the EntityManager throws now passes a
catch of `Exception` or `RuntimeException`, as an `Error` does anywhere
else.

A sweep of the direct callers in the org's repositories found the
catches that now fire only in web-conferencing, none of them inside an
outer transaction, and no catch of `UndeclaredThrowableException` around
a kernel DAO call. exoplatform/web-conferencing#421 pins the video-call
errors on this fix; its build passes once this fix is published.

Knowledge: Meeds-io/eng-standards#560

This change is classified N2 (at `ffe62c50f6`) — its approver must be an
Archi/Dev who knows it is N2, not an approval on AI review alone.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
@exo-swf
exo-swf force-pushed the feature/maintenance branch from 2ab6d8c to a4eb11d Compare October 19, 2026 23:18
…XO-91299

Symptom: when the database failed inside an EntityManager call made while
saving a video call, such as reading the call or a participant before
writing it, the failure left the service as an UndeclaredThrowableException
instead of the service's own call error (a StorageException naming the
step, wrapped in an InvalidCallException when starting, joining, leaving,
stopping or removing a call), and the code that tolerates a storage error,
such as removing a call whose owner no longer exists while reading it,
failed the whole read instead. A failure at commit was already reported as
a call storage error.

Cause: the kernel DAOs' EntityManager proxy delivered the EntityManager's
exception wrapped in an UndeclaredThrowableException, which none of the
service's storage catches recognises. It is fixed in portal's
EntityManagerHolder.

Fix: a test runs adding a participant or a guest, updating the invitees or
the participants, creating a started group call, starting, joining,
leaving, updating, stopping and removing a call through the real DAOs, the
EntityManager proxy and the ExoTransactional aspect, with only the
persistence engine behind them mocked. It raises each failure where a
database failure leaves an EntityManager call (the read that precedes the
write, or the persist of an invitation that flushes) and checks it is
reported as the operation's error. It also checks that reading a call
whose owner no longer exists tolerates a failed removal, and that a failure
at commit, when adding a participant, is still reported as a storage error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…r tests - EXO-91299

Starting, stopping, removing, joining, leaving and updating a call read
the call through getCall before anything else, so within a request that
read is the one that reaches the database. Their tests fail it and check
the InvalidCallException wrapping its StorageException. A member who
joined a space after its call started has no participant row, and joining
the call reads that row for the first time in the request: its test checks
joinCall's own InvalidCallException. Creating a call checks that no call
has its id: its test checks that the call is still created after a failure
there. The test describes what it mocks: the container, the
EntityManagerService, the EntityManager and its transaction, and the
methods stubbed on the service's spy.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@boubaker
boubaker requested a review from ahamdi October 20, 2026 16:25

@ahamdi ahamdi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AI review — Round #2 (follow-up)

Scope: the changes from b0caed30 to a0c905d1 (the rebase from b5e62179 to b0caed30 changed nothing), plus the two Round #1 findings.

# Finding Status
1 🟡 7 tests stubbed a database failure on a find that runs no SQL in production ✅ Fixed in a0c905d1, verified
2 🟢 The body and the self-review described less mocking than the test does ✅ Fixed in a0c905d1, verified

🟡 Medium — the first commit's message no longer matches the test, and the repo's default squash keeps it

b0caed30

…and the code that tolerates a storage error,
such as removing a call whose owner no longer exists while reading it,
failed the whole read instead.
…
EntityManager proxy and the ExoTransactional aspect, with only the
persistence engine behind them mocked.
…
It also checks that reading a call
whose owner no longer exists tolerates a failed removal, …

The repo squashes with squash_merge_commit_message: COMMIT_MESSAGES, and it also allows rebase merges. Either way, both commit messages land on feature/maintenance unless the merger edits them by hand. Three claims in b0caed30 are false at a0c905d1:

  • getCallOfAMissingOwnerToleratesAFailedRemoval is removed, and the reply in its thread explains why: no failure of that removal goes through the proxy in a request.
  • "with only the persistence engine behind them mocked" is the claim that Round #1's 🟢 corrected.
  • The InvalidCallException list leaves out updating a call. updateCallReportsAFailedReadOfTheCallAsAnInvalidCall now pins it, raised from getCall (WebConferencingService.java:1001).

Your thread reply asks the merger to use the PR body when squashing. That depends on someone remembering it at merge time.

Fix: squash the two commits into one whose message is the PR's current Symptom / Cause / Fix, then push with --force-with-lease. Alternatively, reword b0caed30 so it makes none of the three claims.


🟢 Nit — the test-run evidence names no SHA

The Depends on paragraph and the Round #1 thread reply say: "With #1344, 14 tests pass. Without it, 13 of the 14 fail." Neither says which commit the run used, and neither gives the result line. The count is consistent with the code at head. Only addParticipantReportsAFailedCommitAsAStorageError can pass without #1344, because invalidateCall catches PersistenceException only (WebConferencingService.java:3942). Still, a run on another commit is not evidence for this one.

Fix: add the Tests run: result line and the SHA it ran on, at the head after the squash above, for both runs (with #1344 and without it).


Verified conform

  • The start, stop, remove, join, leave and update tests no longer stub getCall or currentUserId. Each one fails the first find(CallEntity) that getCall makes, and getCall wraps that StorageException in an InvalidCallException (l.1229-1235). Each test asserts both the StorageException and the stubbed cause.
  • In joinCallOfANewSpaceMemberReportsAFailedReadOfItsParticipantAsAnInvalidCall, Mary comes from the space's members and not from findCallParts, so she is added as a LEAVED participant (l.2815-2820). updateParticipant then finds her ParticipantId for the first time in the request, which runs a SELECT, and its StorageException reaches joinCall's own catch. This failure can happen in production.
  • createCallToleratesAFailedCheckOfAnExistingCall pins the tolerant invalidateCall. Without #1344, the UndeclaredThrowableException escaped its PersistenceException catch and failed the creation.
  • The class Javadoc and the PR body now list everything mocked, the spy's stubs included.
  • Meeds-io/portal#1344 is merged. Knowledge: Meeds-io/eng-standards#560 is an open draft. It carries this delivery's portal and web-conferencing pitfalls, which is the expected state until the code reaches develop.

What it does well: the fix goes past what Round #1 asked. Instead of narrowing the claims, it made the tests run the real getCall, so each one now pins a failure a request can hit. It also found a second reachable read, the participant row of a space member who joined after the call started. The removed missing-owner test comes with a precise reason why that path can never fail through the proxy.

Classification: N3. The PR adds a test only and touches no REST, DAO, schema or ACL surface. It must still be validated by an Architect/Developer and not merged on AI review alone.

🤖 Generated with Claude Code

@boubaker

Copy link
Copy Markdown
Member Author

Round #2:

  • 🟡 First commit's message: kept as is, by the Architects Lead's ruling. When squashing into feature/maintenance and develop, the PR title and body are the commit message to use. This repo does not follow the Meeds-io repos' default squash setting yet, and its default is to be aligned with theirs.
  • 🟢 Evidence SHA: the Depends on paragraph now gives both runs at a0c905d1 with their result lines. With #1344: Tests run: 14, Failures: 0, Errors: 0, Skipped: 0. Without it: Tests run: 14, Failures: 12, Errors: 1, Skipped: 0.

@boubaker
boubaker requested a review from ahamdi October 21, 2026 06:25
@boubaker
boubaker enabled auto-merge (squash) October 21, 2026 06:26

@ahamdi ahamdi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

AI review — Round #3 (final)

Scope: the head is still a0c905d1, so no code changed since Round #2. This round checks the author's replies and the live PR body.

# Round Finding Status
1 #1 🟡 7 tests stubbed a database failure on a find that runs no SQL in production ✅ Fixed in a0c905d1, verified in Round #2
2 #1 🟢 The body and the self-review described less mocking than the test does ✅ Fixed in a0c905d1, verified in Round #2
3 #2 🟡 The first commit's message no longer matches the test, and the repo's default squash keeps it ➖ Not taken. The author reports an Architects Lead ruling: the commit stays as is, and the merger squashes with the PR title and body as the commit message. Whoever merges this into feature/maintenance and develop: replace the default squash message with the PR title and body.
4 #2 🟢 The test-run evidence named no SHA ✅ Fixed. The Depends on paragraph now gives both runs at a0c905d1 with their result lines. The numbers match the code at head: 14 tests, 13 of them failing without #1344 (12 failures and 1 error), and only the commit test passing, because that failure is raised outside the proxy.

All findings from previous rounds are resolved, and nothing is outstanding from the AI review side.

Classification: N3. The PR adds a test only and touches no REST, DAO, schema or ACL surface. It must still be validated by an Architect/Developer and not merged on AI review alone. It merges after Meeds-io/portal#1344 is published.

🤖 Generated with Claude Code

@boubaker
boubaker merged commit 5866c3e into feature/maintenance Oct 21, 2026
22 of 23 checks passed
@boubaker
boubaker deleted the fix/EXO-91299-call-storage-errors branch October 21, 2026 06:41
boubaker added a commit to Meeds-io/portal that referenced this pull request Oct 21, 2026
… pages EXO-91183 (#1338)

**Symptom 1**: after the built-in sites (`global`, `dw`, `myworkspace`)
are restored with their layout, a displayed site's pages can render
empty: the meta site was left with an empty layout.

**Symptom 2**: an overwrite restore of a site defined by several
configurations (`global` has six: multifactor-authentication, agenda,
documents, processes, sites, digital-workplace) logs, for every second
configuration (reproduced on a local server):

```
NewPortalConfig error: Error while committing the transaction [org.hibernate.TransientPropertyValueException: Persistent instance of 'org.exoplatform.portal.jdbc.entity.NodeEntity' references an unsaved transient instance of 'org.exoplatform.portal.jdbc.entity.PageEntity' (persist the transient instance before flushing) [org.exoplatform.portal.jdbc.entity.NodeEntity.page -> org.exoplatform.portal.jdbc.entity.PageEntity]]
Can't find Window with ids: ...
```

and leaves pages missing, pages whose layout names deleted windows, and
the `dw` site's People and Spaces nodes without a page.

**Cause 1**: `SiteStorageImpl.save` replaced a site's layout one kernel
DAO call at a time, each `@ExoTransactional` call committing on its own.
A concurrent reader could read the site row while its root container was
already deleted and get, and cache, an empty layout.
`UserPortalConfigService.restoreSite` then read the site again after the
import and saved that empty layout back over the restored one.

**Cause 2**: a restore imported each configuration of the site in turn
with the requested mode. In overwrite mode, each configuration deleted
every page of the site, the pages the previous configurations had just
imported included, and re-created its own pages with new identifiers:
the nodes of other sites that reference them lost their page
(`FK_PORTAL_NODE_PAGE` is `ON DELETE SET NULL`). Its page deletion was
refused at commit while the page windows were already deleted in their
own transactions. In overwrite and restore-defaults mode, each
configuration also deleted the nodes the previous ones had imported
under the nodes it defines. The startup import did the same for a
configuration in overwrite mode, at every start.

**Fix**:
- `@ContainerTransactional(atomic = true)` runs the method in one JPA
transaction, committed when it returns and rolled back when it throws;
when a failed call marked it rollback-only, a method that returned
throws `RollbackException`. Kernel EntityManager writes join it. When
the method begins it, Spring transactional methods and Spring Data
repositories join it too, and their callbacks run with its outcome.
`SiteStorageImpl` `create(PortalData)`, `save(PortalData)` and
`remove(SiteKey)` use it.
- The site, layout, page, navigation and description caches evict again
once the transaction completes (`TransactionCallbacks.afterCompletion`),
after invalidating the loads in flight; `FutureCache.clearFutures`
invalidates every load it removes.
- A site read whose root container is missing is read again once
(`SiteDAO#refresh`), then flagged `layoutUnresolved` with an empty
layout; a save of it keeps the stored layout.
- `restoreSite` applies the previous site's settings to the imported
config before its save, instead of saving the site a second time.
- A site restore (`UserPortalConfigService#restoreSite`), and the
startup import of a site that one of its configurations overwrites,
import the site from all its configurations
(`NewPortalConfigListener#importSitePages`, `#importSiteNavigation`): an
overwrite deletes the pages that no configuration defines, then the
pages of every configuration are imported in place; the navigation of
every configuration is imported without deleting any node, then
`NavigationOrphanRemover` deletes the nodes that none of them defines,
as the mode does (every such node in overwrite mode, the nodes nested in
a default node in restore-defaults mode, a node only in the path of a
fragment attachment point keeping its children). Nothing is deleted when
the pages or the navigation file of a configuration can't be parsed; a
file a configuration can't open counts as defining none. All pages are
imported before any navigation.
- A restore imports the pages of every configuration of the site,
whatever was imported before: the check of
`NewPortalConfigListener#initPageDB` (`createdOwners`, filled by the
startup import and by a site-layout restore) no longer applies to it.
- Each orphan page is removed on its own: a page that can't be removed,
as one removed since the orphans were selected, is logged (WARN) and the
next ones are removed.
- `PageStorageImpl` deletes a page, and replaces a page layout, in one
transaction, and broadcasts the page event once that write returns.
- A page delete, a page layout save, a page clone, a site save and a
site removal read their row again (`PageDAO#refresh`,
`SiteDAO#refresh`), and work from the layout that row holds. Before,
they worked from the page or site as this request first read it, for
instance through a node load, which reads the pages its nodes reference;
with Meeds-io/layout#581, a site restore reads the site's navigation
before it imports the pages. A layout another request saved since is now
removed with the page or site, replaced by a layout save, and copied by
a clone.
- `PageEntity` is `@DynamicUpdate`. A page row has two writers, its
properties save and its layout save, and no version: an update now
writes only the columns it changed, so a properties save no longer
writes back the page body it read. It reads its row again too, so each
property it saves is compared with the row as stored.
- A failing atomic call that joined an active kernel transaction marks
it rollback-only and logs a WARN naming the method.
- `ExoTransactionalAspect` rolls back and throws `RollbackException`
when the method it began a transaction for returns over a rollback-only
transaction, as `ContainerTransactionalAspect` already did for its own
transactions. A method that throws keeps its error, unless ending the
transaction fails, as before. The EntityManager it started is closed
whatever the method's outcome.
- At startup, a configuration whose site type or import mode is unknown
is left out of the overwritten sites; on a site no other configuration
overwrites, its own import logs the error as before.
- Pinned by `ContainerTransactionalAspectTest`,
`ExoTransactionalAspectTest`, `ContainerTransactionalRepositoryTest` (a
real Spring Data repository), `TransactionCallbacksTest`,
`FutureCacheTest`, `TestDataStorage` (a layout write and a page delete
failing midway through the woven advice, a node update evicting the node
cache after the commit), `SiteDAOTest`, `TestUserPortalConfigService`,
`TestRestoreSiteConfigurations` in the new `InitContainer10TestSuite` (a
restore and a startup import of sites defined by several configurations,
a node of another site keeping its page, a configuration of an unknown
site type, a pages restore of a site this startup didn't import, an
orphan page the request has read and another request saves a layout on
during the restore), `TestDataStorage` (a page layout save, a page
properties save, a page clone, a site save and a site removal after
another request saved a layout on the page or site the request had read,
an update of the page properties that leaves the layout column another
request wrote, and a properties save that writes back a property another
request changed since the request read the page),
`NewPortalConfigListenerTest` (an orphan page that can't be removed),
`TestNavigationFragmentImporterRestoreDefaults` (now run, in
`InitContainer1TestSuite`), `TestSiteDataImportOverwrite` and
`TestImportNavigationOverwrite`.

**Upgrade impact**:
- A page delete, a page layout or properties save, a page clone, and a
site save or removal read their row once more (a SELECT by id).
- A site whose stored body names a missing root container is kept, read
with an empty layout flagged unresolved (WARN) and cached so; a save of
it keeps the stored layout (WARN). A restore with layout, or a save that
sets a layout, repairs it.
- `restoreSite` makes one save per configuration that declares the site:
`PORTAL_CONFIG_UPDATED`, or `PORTAL_CONFIG_CREATED` for a site the
import creates, and none when the import skips the site.
- A write failure now rolls back the whole site create, save or remove.
Inside an enclosing kernel transaction, a failed site create, save or
remove, page delete or page layout save marks that transaction
rollback-only and logs a WARN naming the method.
- **An `@ExoTransactional` method that began the transaction and returns
over it marked rollback-only now throws
`jakarta.persistence.RollbackException`** after the rollback, instead of
returning with none of its writes committed. A method that joined an
active transaction doesn't throw: the method that began it does. The
transaction is marked by an atomic call above, or by a failing JPA
operation (every converted `PersistenceException` but
`NoResultException`, `NonUniqueResultException`, `LockTimeoutException`
and `QueryTimeoutException`). So any such method that catches the
failure and returns now fails visibly. The callers checked:
- `JPASettingServiceImpl#set` catches every exception of its body: a
`SettingService#set` call made outside a transaction now throws where it
logged an ERROR.
- social `LayoutUpgradePlugin#updateLayout`: the exception leaves
`processUpgrade`, the upgrades after it don't run, and
`UpgradeProductService` doesn't record the plugin as done. The whole
plugin runs again at every start, re-applying the upgrades before the
failing one, until that one passes. Before, the failing upgrade's writes
were rolled back and the plugin was recorded as done.
- social `MetadataServiceImpl#start`: a failing metadata init plugin
would fail the start of the portal container; Meeds-io/social#6245
creates each plugin's metadata in its own transaction.
- social `SocialUserEventListenerImpl#postDelete` throws, `UserDAOImpl`
logs it (WARN), and the user is deleted as before.
- gamification `ProgramSpaceListener` and commons
`JPAQueueMessageImpl#send` keep their outcome: the exception is dropped
by an unread future, or logged by the job (ERROR), and the writes are
rolled back as before.
- A kernel component whose `start()` makes such a call outside a
transaction fails its start (`ContainerException`), and the portal
container is left partly started.
- An overwrite restore with pages keeps the pages and nodes of every
configuration of the site; the pages it imports keep their identifier,
so the nodes of other sites keep their page. The pages and nodes that no
configuration defines are deleted once, after every import for the
nodes.
- A restore imports the site layout of every configuration, then their
pages, then their navigation: a node that references a page of a later
configuration of the site is linked to it.
- A restore-defaults restore with navigation keeps the nodes of every
configuration, whatever their order; a node nested in one configuration
that another one attaches a fragment to keeps its custom children.
- The search index of a restored site's pages is refreshed by
Meeds-io/layout#581. With this PR alone, an overwrite restore updates
the pages in place with the portal's page update event only: their
indexed blocks keep their content and permissions until the page is next
saved through Layout, or the next full reindex, as the restore-defaults
restore already did.
- When the navigation, or the pages, file of one configuration can't be
parsed, a restore deletes no node, or no page, of the site. A file a
configuration can't open counts as defining none: the pages and nodes
only it defines are deleted as orphans, as the previous whole-site
removal deleted every page.
- A pages restore without the site layout ("Restore pages") restores the
pages of a site that neither this start nor a site-layout restore since
then imported (after a restart, a site no configuration overrides);
before, it restored only its navigation. A REST restore with layout and
pages whose site layout import fails now still imports the pages.
- **Startup contract change**: when one configuration of a site is in
overwrite mode (an extension, or a property such as
`exo.agenda.portalConfig.metadata.importmode=overwrite`), every start
that runs that configuration (its `override` true, or a first start, or
a reimport) imports the pages and the navigation of all the
configurations of the site, whatever their own mode and `override`, then
deletes the pages and nodes that none of them defines. The pages created
in the UI in that site are among them, as before. So are the nodes
created in the UI under a node that one of its configurations defines or
attaches a fragment to, the root included when one attaches there, as
one configuration of every shipped site does; before, a start deleted
only those under the attachment points of the overwriting
configuration's own fragments. Each configuration's pages are saved from
its files in place (layout, state and permissions replaced), and its
navigation merged. An extension in overwrite mode no longer deletes the
pages and nodes the other configurations of the site define: they are
kept and the extension's own are merged over them. On a first start, or
a reimport, with an overwrite configuration, the result is the union of
the configurations, where the last overwrite used to win.
- The shipped configurations (meeds sites, digital-workplace, agenda,
documents, processes, multifactor-authentication) default to insert or
conserve: the change applies to a deployment that sets an overwrite
mode.
- A configuration of that site whose `pages.xml` or `navigation.xml`
can't be parsed: no page, or no node, is deleted.
- A configuration of that site with an unknown import mode: its pages
and navigation are imported in merge mode; its site layout import still
logs the mode.
  - The site layout import, and the shipped defaults, are unchanged.
- A page delete or a page layout save that fails midway rolls back its
windows and permissions; before, the page kept a layout naming deleted
windows (`Can't find Window with ids`).

**Known limits**:
- `PAGE_DESTROYED` is broadcast before the commit when the page removal
joins a site removal (`SiteStorageImpl#remove`); no production listener.
- A node write drops every navigation and node load in flight, for all
sites, before evicting; the after-completion repeat is pinned, the
in-flight invalidation is not.
- A page delete, layout save or clone, and a site save or removal read
their row without a lock: a layout another request commits between that
read and the write is missed.
- `PAGE_REMOVED` carries the page as `LayoutService#remove(PageKey)`
gets it, from the page cache or from the request's first read of the
page: it may lack a layout saved since, which the delete removes.
- A user site's pages are never deleted by the orphan removal, as the
page import never did (not reachable today: no user-type configuration
ships).
- The template clone (`savePortalFromTemplate`) keeps the unresolved
flag, and `create` ignores it.
- The in-flight load pin covers the site cache; the `clearFutures` race
fix is not pinned.
- The banner write of the `PortalConfig` overloads runs before, outside
the transaction.
- A reader that straddles the commit still misses containers: a missing
root is re-read once, then flagged; a missing nested container is
skipped.
- Spring: a Spring write inside an atomic method joined to an outer
non-atomic kernel transaction keeps its own transaction. With a Spring
synchronization already active and no EntityManager of its own, the
atomic transaction collects none of the Spring callbacks. A Spring
`@CacheEvict` evicts at once, before the commit. A Spring call made
while the commit flushes (an entity listener) does not join.

**Follow-ups**: the per-session `UIPortal` cache is not refreshed on a
meta-site layout change; `global` is displayed on community-dev but not
in the packaged configuration; nested container misses; an optional
guard in the QA restore hook (meeds-qa-ui#339); a Spring join inside a
non-atomic kernel transaction; the web-conferencing pins of task 91299
(exoplatform/web-conferencing#421).

**Release order**: Meeds-io/social#6245 before this PR, or with it:
without it, a failing metadata init plugin fails the start. This PR
before the `Meeds-io/layout` PR of EXO-91183, which refuses to hand on
an unresolved site's layout and compiles against
`PortalConfig#isLayoutUnresolved`.

**Tests**: at the head `65f8e9a240`, `mvn -B -ntp -o
-Dcheckstyle.skip=true -pl component/portal
-Dtest='TestDataStorage,TestModelStorage,TestPageService,TestUserPortalConfigService,TestRestoreSiteConfigurations,TestSiteDataImport*,TestImportNavigation*,TestImportFragment*,TestNavigationFragmentImporterRestoreDefaults,NewPortalConfigListenerTest'
-Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=true test`: 18
classes, 148 tests, 0 failures, 0 errors. The full suite (`mvn -B -ntp
clean verify javadoc:javadoc -Pcoverage`) wasn't run at this head.

Knowledge: Meeds-io/eng-standards#409, #560, #586, #587

This change is classified N1 — Kernel ↔ Spring bridge (the atomic
`@ContainerTransactional` transaction that Spring transactional methods
and Spring Data repositories join), the outcome of every
`@ExoTransactional` method, the JPA transaction boundaries over the site
and page storage writes, the access and edit permissions a restored site
keeps, and the irreversible deletion of pages and navigation nodes by a
restore and by the startup import. Its approver must be an Archi/Dev who
knows it is N1, not an approval on AI review alone; author ≠ approver.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
boubaker added a commit to Meeds-io/portal that referenced this pull request Oct 27, 2026
…oxy - EXO-91299 (#1355)

**Symptom**: a database error raised inside an EntityManager call of a
kernel DAO (find, persist, merge, remove, refresh...) skipped every
catch naming that exception. In video calls, such a failure while saving
a call, such as reading the call or a participant before writing it,
left the service as an UndeclaredThrowableException instead of the
service's own call error (a StorageException naming the step, wrapped in
an InvalidCallException when starting, joining, leaving, stopping or
removing a call), and the code that tolerates a storage error, such as
removing a call whose owner no longer exists while reading it, failed
the whole read instead. A failure at commit was not affected: the commit
does not go through the proxy.

**Cause**: the `EntityManagerHolder` proxy invoked the thread's
EntityManager by reflection without unwrapping the
`InvocationTargetException`, which the JDK proxy then wrapped in an
`UndeclaredThrowableException`: a catch of a JPA exception, an
`IllegalArgumentException` or an `IllegalStateException` does not
recognise it.

**Fix**: the proxy rethrows the exception the EntityManager threw.
`EntityManagerHolderTest` pins it on a real EntityManager: a find with a
null id, a persist of a detached entity and a refresh of a removed row
each reach the DAO's caller as the engine's own exception, and the
EntityManager the proxy opened outside a request is closed after a
failed call.

**Impact**: every caller of a kernel DAO (a `GenericDAOJPAImpl`
subclass, in every addon) now receives the exception an EntityManager
method called on the proxy throws (find, persist, merge, remove,
refresh, flush, getReference...); the execution of a query never went
through the proxy and is unchanged. Only a catch narrower than
`Exception` or `RuntimeException` behaves differently for an exception:
- a catch of the EntityManager's exception, or of
`IllegalArgumentException` or `IllegalStateException`, now fires.
Hibernate has by then marked the transaction rollback-only for most of
them (not for `NoResultException`, `NonUniqueResultException` and the
lock and query timeouts): inside an outer `@ExoTransactional` method,
such a catch that does not rethrow lets the aspect roll the transaction
back without an error, where the failure used to reach the caller;
- a catch of `UndeclaredThrowableException` around a kernel DAO call no
longer matches, and an `Error` the EntityManager throws now passes a
catch of `Exception` or `RuntimeException`, as an `Error` does anywhere
else.

A sweep of the direct callers in the org's repositories found the
catches that now fire only in web-conferencing, none of them inside an
outer transaction, and no catch of `UndeclaredThrowableException` around
a kernel DAO call. exoplatform/web-conferencing#421 pins the video-call
errors on this fix; its build passes once this fix is published.

Knowledge: Meeds-io/eng-standards#560

This change is classified N2 (at `ffe62c50f6`) — its approver must be an
Archi/Dev who knows it is N2, not an approval on AI review alone.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
boubaker added a commit to Meeds-io/portal that referenced this pull request Oct 27, 2026
… pages EXO-91183 (#1357)

**Symptom 1**: after the built-in sites (`global`, `dw`, `myworkspace`)
are restored with their layout, a displayed site's pages can render
empty: the meta site was left with an empty layout.

**Symptom 2**: an overwrite restore of a site defined by several
configurations (`global` has six: multifactor-authentication, agenda,
documents, processes, sites, digital-workplace) logs, for every second
configuration (reproduced on a local server):

```
NewPortalConfig error: Error while committing the transaction [org.hibernate.TransientPropertyValueException: Persistent instance of 'org.exoplatform.portal.jdbc.entity.NodeEntity' references an unsaved transient instance of 'org.exoplatform.portal.jdbc.entity.PageEntity' (persist the transient instance before flushing) [org.exoplatform.portal.jdbc.entity.NodeEntity.page -> org.exoplatform.portal.jdbc.entity.PageEntity]]
Can't find Window with ids: ...
```

and leaves pages missing, pages whose layout names deleted windows, and
the `dw` site's People and Spaces nodes without a page.

**Cause 1**: `SiteStorageImpl.save` replaced a site's layout one kernel
DAO call at a time, each `@ExoTransactional` call committing on its own.
A concurrent reader could read the site row while its root container was
already deleted and get, and cache, an empty layout.
`UserPortalConfigService.restoreSite` then read the site again after the
import and saved that empty layout back over the restored one.

**Cause 2**: a restore imported each configuration of the site in turn
with the requested mode. In overwrite mode, each configuration deleted
every page of the site, the pages the previous configurations had just
imported included, and re-created its own pages with new identifiers:
the nodes of other sites that reference them lost their page
(`FK_PORTAL_NODE_PAGE` is `ON DELETE SET NULL`). Its page deletion was
refused at commit while the page windows were already deleted in their
own transactions. In overwrite and restore-defaults mode, each
configuration also deleted the nodes the previous ones had imported
under the nodes it defines. The startup import did the same for a
configuration in overwrite mode, at every start.

**Fix**:
- `@ContainerTransactional(atomic = true)` runs the method in one JPA
transaction, committed when it returns and rolled back when it throws;
when a failed call marked it rollback-only, a method that returned
throws `RollbackException`. Kernel EntityManager writes join it. When
the method begins it, Spring transactional methods and Spring Data
repositories join it too, and their callbacks run with its outcome.
`SiteStorageImpl` `create(PortalData)`, `save(PortalData)` and
`remove(SiteKey)` use it.
- The site, layout, page, navigation and description caches evict again
once the transaction completes (`TransactionCallbacks.afterCompletion`),
after invalidating the loads in flight; `FutureCache.clearFutures`
invalidates every load it removes.
- A site read whose root container is missing is read again once
(`SiteDAO#refresh`), then flagged `layoutUnresolved` with an empty
layout; a save of it keeps the stored layout.
- `restoreSite` applies the previous site's settings to the imported
config before its save, instead of saving the site a second time.
- A site restore (`UserPortalConfigService#restoreSite`), and the
startup import of a site that one of its configurations overwrites,
import the site from all its configurations
(`NewPortalConfigListener#importSitePages`, `#importSiteNavigation`): an
overwrite deletes the pages that no configuration defines, then the
pages of every configuration are imported in place; the navigation of
every configuration is imported without deleting any node, then
`NavigationOrphanRemover` deletes the nodes that none of them defines,
as the mode does (every such node in overwrite mode, the nodes nested in
a default node in restore-defaults mode, a node only in the path of a
fragment attachment point keeping its children). Nothing is deleted when
the pages or the navigation file of a configuration can't be parsed; a
file a configuration can't open counts as defining none. All pages are
imported before any navigation.
- A restore imports the pages of every configuration of the site,
whatever was imported before: the check of
`NewPortalConfigListener#initPageDB` (`createdOwners`, filled by the
startup import and by a site-layout restore) no longer applies to it.
- Each orphan page is removed on its own: a page that can't be removed,
as one removed since the orphans were selected, is logged (WARN) and the
next ones are removed.
- `PageStorageImpl` deletes a page, and replaces a page layout, in one
transaction, and broadcasts the page event once that write returns.
- A page delete, a page layout save, a page clone, a site save and a
site removal read their row again (`PageDAO#refresh`,
`SiteDAO#refresh`), and work from the layout that row holds. Before,
they worked from the page or site as this request first read it, for
instance through a node load, which reads the pages its nodes reference;
with Meeds-io/layout#581, a site restore reads the site's navigation
before it imports the pages. A layout another request saved since is now
removed with the page or site, replaced by a layout save, and copied by
a clone.
- `PageEntity` is `@DynamicUpdate`. A page row has two writers, its
properties save and its layout save, and no version: an update now
writes only the columns it changed, so a properties save no longer
writes back the page body it read. It reads its row again too, so each
property it saves is compared with the row as stored.
- A failing atomic call that joined an active kernel transaction marks
it rollback-only and logs a WARN naming the method.
- `ExoTransactionalAspect` rolls back and throws `RollbackException`
when the method it began a transaction for returns over a rollback-only
transaction, as `ContainerTransactionalAspect` already did for its own
transactions. A method that throws keeps its error, unless ending the
transaction fails, as before. The EntityManager it started is closed
whatever the method's outcome.
- At startup, a configuration whose site type or import mode is unknown
is left out of the overwritten sites; on a site no other configuration
overwrites, its own import logs the error as before.
- Pinned by `ContainerTransactionalAspectTest`,
`ExoTransactionalAspectTest`, `ContainerTransactionalRepositoryTest` (a
real Spring Data repository), `TransactionCallbacksTest`,
`FutureCacheTest`, `TestDataStorage` (a layout write and a page delete
failing midway through the woven advice, a node update evicting the node
cache after the commit), `SiteDAOTest`, `TestUserPortalConfigService`,
`TestRestoreSiteConfigurations` in the new `InitContainer10TestSuite` (a
restore and a startup import of sites defined by several configurations,
a node of another site keeping its page, a configuration of an unknown
site type, a pages restore of a site this startup didn't import, an
orphan page the request has read and another request saves a layout on
during the restore), `TestDataStorage` (a page layout save, a page
properties save, a page clone, a site save and a site removal after
another request saved a layout on the page or site the request had read,
an update of the page properties that leaves the layout column another
request wrote, and a properties save that writes back a property another
request changed since the request read the page),
`NewPortalConfigListenerTest` (an orphan page that can't be removed),
`TestNavigationFragmentImporterRestoreDefaults` (now run, in
`InitContainer1TestSuite`), `TestSiteDataImportOverwrite` and
`TestImportNavigationOverwrite`.

**Upgrade impact**:
- A page delete, a page layout or properties save, a page clone, and a
site save or removal read their row once more (a SELECT by id).
- A site whose stored body names a missing root container is kept, read
with an empty layout flagged unresolved (WARN) and cached so; a save of
it keeps the stored layout (WARN). A restore with layout, or a save that
sets a layout, repairs it.
- `restoreSite` makes one save per configuration that declares the site:
`PORTAL_CONFIG_UPDATED`, or `PORTAL_CONFIG_CREATED` for a site the
import creates, and none when the import skips the site.
- A write failure now rolls back the whole site create, save or remove.
Inside an enclosing kernel transaction, a failed site create, save or
remove, page delete or page layout save marks that transaction
rollback-only and logs a WARN naming the method.
- **An `@ExoTransactional` method that began the transaction and returns
over it marked rollback-only now throws
`jakarta.persistence.RollbackException`** after the rollback, instead of
returning with none of its writes committed. A method that joined an
active transaction doesn't throw: the method that began it does. The
transaction is marked by an atomic call above, or by a failing JPA
operation (every converted `PersistenceException` but
`NoResultException`, `NonUniqueResultException`, `LockTimeoutException`
and `QueryTimeoutException`). So any such method that catches the
failure and returns now fails visibly. The callers checked:
- `JPASettingServiceImpl#set` catches every exception of its body: a
`SettingService#set` call made outside a transaction now throws where it
logged an ERROR.
- social `LayoutUpgradePlugin#updateLayout`: the exception leaves
`processUpgrade`, the upgrades after it don't run, and
`UpgradeProductService` doesn't record the plugin as done. The whole
plugin runs again at every start, re-applying the upgrades before the
failing one, until that one passes. Before, the failing upgrade's writes
were rolled back and the plugin was recorded as done.
- social `MetadataServiceImpl#start`: a failing metadata init plugin
would fail the start of the portal container; Meeds-io/social#6245
creates each plugin's metadata in its own transaction.
- social `SocialUserEventListenerImpl#postDelete` throws, `UserDAOImpl`
logs it (WARN), and the user is deleted as before.
- gamification `ProgramSpaceListener` and commons
`JPAQueueMessageImpl#send` keep their outcome: the exception is dropped
by an unread future, or logged by the job (ERROR), and the writes are
rolled back as before.
- A kernel component whose `start()` makes such a call outside a
transaction fails its start (`ContainerException`), and the portal
container is left partly started.
- An overwrite restore with pages keeps the pages and nodes of every
configuration of the site; the pages it imports keep their identifier,
so the nodes of other sites keep their page. The pages and nodes that no
configuration defines are deleted once, after every import for the
nodes.
- A restore imports the site layout of every configuration, then their
pages, then their navigation: a node that references a page of a later
configuration of the site is linked to it.
- A restore-defaults restore with navigation keeps the nodes of every
configuration, whatever their order; a node nested in one configuration
that another one attaches a fragment to keeps its custom children.
- The search index of a restored site's pages is refreshed by
Meeds-io/layout#581. With this PR alone, an overwrite restore updates
the pages in place with the portal's page update event only: their
indexed blocks keep their content and permissions until the page is next
saved through Layout, or the next full reindex, as the restore-defaults
restore already did.
- When the navigation, or the pages, file of one configuration can't be
parsed, a restore deletes no node, or no page, of the site. A file a
configuration can't open counts as defining none: the pages and nodes
only it defines are deleted as orphans, as the previous whole-site
removal deleted every page.
- A pages restore without the site layout ("Restore pages") restores the
pages of a site that neither this start nor a site-layout restore since
then imported (after a restart, a site no configuration overrides);
before, it restored only its navigation. A REST restore with layout and
pages whose site layout import fails now still imports the pages.
- **Startup contract change**: when one configuration of a site is in
overwrite mode (an extension, or a property such as
`exo.agenda.portalConfig.metadata.importmode=overwrite`), every start
that runs that configuration (its `override` true, or a first start, or
a reimport) imports the pages and the navigation of all the
configurations of the site, whatever their own mode and `override`, then
deletes the pages and nodes that none of them defines. The pages created
in the UI in that site are among them, as before. So are the nodes
created in the UI under a node that one of its configurations defines or
attaches a fragment to, the root included when one attaches there, as
one configuration of every shipped site does; before, a start deleted
only those under the attachment points of the overwriting
configuration's own fragments. Each configuration's pages are saved from
its files in place (layout, state and permissions replaced), and its
navigation merged. An extension in overwrite mode no longer deletes the
pages and nodes the other configurations of the site define: they are
kept and the extension's own are merged over them. On a first start, or
a reimport, with an overwrite configuration, the result is the union of
the configurations, where the last overwrite used to win.
- The shipped configurations (meeds sites, digital-workplace, agenda,
documents, processes, multifactor-authentication) default to insert or
conserve: the change applies to a deployment that sets an overwrite
mode.
- A configuration of that site whose `pages.xml` or `navigation.xml`
can't be parsed: no page, or no node, is deleted.
- A configuration of that site with an unknown import mode: its pages
and navigation are imported in merge mode; its site layout import still
logs the mode.
  - The site layout import, and the shipped defaults, are unchanged.
- A page delete or a page layout save that fails midway rolls back its
windows and permissions; before, the page kept a layout naming deleted
windows (`Can't find Window with ids`).

**Known limits**:
- `PAGE_DESTROYED` is broadcast before the commit when the page removal
joins a site removal (`SiteStorageImpl#remove`); no production listener.
- A node write drops every navigation and node load in flight, for all
sites, before evicting; the after-completion repeat is pinned, the
in-flight invalidation is not.
- A page delete, layout save or clone, and a site save or removal read
their row without a lock: a layout another request commits between that
read and the write is missed.
- `PAGE_REMOVED` carries the page as `LayoutService#remove(PageKey)`
gets it, from the page cache or from the request's first read of the
page: it may lack a layout saved since, which the delete removes.
- A user site's pages are never deleted by the orphan removal, as the
page import never did (not reachable today: no user-type configuration
ships).
- The template clone (`savePortalFromTemplate`) keeps the unresolved
flag, and `create` ignores it.
- The in-flight load pin covers the site cache; the `clearFutures` race
fix is not pinned.
- The banner write of the `PortalConfig` overloads runs before, outside
the transaction.
- A reader that straddles the commit still misses containers: a missing
root is re-read once, then flagged; a missing nested container is
skipped.
- Spring: a Spring write inside an atomic method joined to an outer
non-atomic kernel transaction keeps its own transaction. With a Spring
synchronization already active and no EntityManager of its own, the
atomic transaction collects none of the Spring callbacks. A Spring
`@CacheEvict` evicts at once, before the commit. A Spring call made
while the commit flushes (an entity listener) does not join.

**Follow-ups**: the per-session `UIPortal` cache is not refreshed on a
meta-site layout change; `global` is displayed on community-dev but not
in the packaged configuration; nested container misses; an optional
guard in the QA restore hook (meeds-qa-ui#339); a Spring join inside a
non-atomic kernel transaction; the web-conferencing pins of task 91299
(exoplatform/web-conferencing#421).

**Release order**: Meeds-io/social#6245 before this PR, or with it:
without it, a failing metadata init plugin fails the start. This PR
before the `Meeds-io/layout` PR of EXO-91183, which refuses to hand on
an unresolved site's layout and compiles against
`PortalConfig#isLayoutUnresolved`.

**Tests**: at the head `65f8e9a240`, `mvn -B -ntp -o
-Dcheckstyle.skip=true -pl component/portal
-Dtest='TestDataStorage,TestModelStorage,TestPageService,TestUserPortalConfigService,TestRestoreSiteConfigurations,TestSiteDataImport*,TestImportNavigation*,TestImportFragment*,TestNavigationFragmentImporterRestoreDefaults,NewPortalConfigListenerTest'
-Dsurefire.failIfNoSpecifiedTests=false -Djacoco.skip=true test`: 18
classes, 148 tests, 0 failures, 0 errors. The full suite (`mvn -B -ntp
clean verify javadoc:javadoc -Pcoverage`) wasn't run at this head.

Knowledge: Meeds-io/eng-standards#409, #560, #586, #587

This change is classified N1 — Kernel ↔ Spring bridge (the atomic
`@ContainerTransactional` transaction that Spring transactional methods
and Spring Data repositories join), the outcome of every
`@ExoTransactional` method, the JPA transaction boundaries over the site
and page storage writes, the access and edit permissions a restored site
keeps, and the irreversible deletion of pages and navigation nodes by a
restore and by the startup import. Its approver must be an Archi/Dev who
knows it is N1, not an approval on AI review alone; author ≠ approver.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants