Implement UIViewRoot.restoreViewScopeState() properly - #2212
Merged
Conversation
restoreViewScopeState() restored the component tree via super.restoreState() and never touched the view map id, i.e. the opposite of what it promises, and had no callers in Mojarra. The view scope restore was instead driven out-of-band: FaceletViewHandlingStrategy set a RESTORE_VIEW_SCOPE_ONLY context attribute and called the full restoreState(), which checked that attribute to skip super.restoreState() and leave only the view map wiring. Extract that wiring into restoreViewMap() and let restoreViewScopeState() invoke it, so FaceletViewHandlingStrategy can call the spec method directly, as prescribed by the StateManagementStrategy.restoreView() javadoc. The saved state shape is unchanged. This removes the attribute branching along with the duplicated UIViewRoot.RESTORE_VIEW_SCOPE_ONLY_KEY and ViewScopeManager.RESTORE_VIEW_SCOPE_ONLY literals. Guard the wiring with a viewScopeStateRestored flag so restoreState() skips it once restoreViewScopeState() has run. It previously ran on both, which is destructive when the saved view map is no longer among the active view maps -- legitimately LRU-evicted, or gone with an expired session: building the view mints a replacement via ViewScopeManager.registerViewMap() and puts its id in the transient state, and the second run restored the stale id over it and nulled the view map, orphaning the replacement. Beans created during buildView() were discarded, @PostConstruct ran again, and the replacement lingered in the active view maps without ever receiving PreDestroyViewMapEvent. Ref eclipse-ee4j/mojarra#5787 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pizzi80
pushed a commit
to pizzi80/mojarra
that referenced
this pull request
Jul 16, 2026
Call UIViewRoot.restoreViewScopeState() from the createMetadataView restore path, as prescribed by the StateManagementStrategy.restoreView() javadoc, now that jakartaee/faces#2212 implements it properly. This replaces setting a RESTORE_VIEW_SCOPE_ONLY context attribute around a full restoreState() call, and lets the ViewScopeManager constant behind it go. Add unit tests for both halves of the API change: that restoreViewScopeState() restores the view map and not the component state, and that a subsequent restoreState() does not rewire the view map over a replacement minted while the view was being built. Both fail without jakartaee/faces#2212. Bump the faces submodule to 321e6fae1. Closes eclipse-ee4j#5787 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
UIViewRoot.restoreViewScopeState(FacesContext, Object), a stub since Faces 2.2 (#787). Full rationale in eclipse-ee4j/mojarra#5787; the Mojarra side follows in a separate PR.The method restored the component tree rather than the view scope and had no callers in Mojarra, so the real behaviour was driven out-of-band through a
RESTORE_VIEW_SCOPE_ONLYcontext attribute. This extracts the view map wiring into a privaterestoreViewMap(), invokes it fromrestoreViewScopeState()asStateManagementStrategy.restoreView()'s javadoc prescribes, and drops the attribute branching. The saved state shape is unchanged.A
viewScopeStateRestoredflag stopsrestoreState()from rewiring the view map a second time. That was destructive when the saved view map is no longer among the active view maps (LRU-evicted, or gone with an expired session): building the view mints a replacement, and the second run restored the stale id over it, orphaning it.Cleaner API/impl split.
UIViewRootis the only class in this repo carryingorg.glassfish.mojarra.*string contracts, and it had three. This drops the only one that steered behaviour —restoreState()branched on impl runtime state via a key duplicated as two independent literals with no import between them. The remainingVIEW_MAP_ID_KEYandACTIVE_VIEW_MAPS_KEYmerely name where the view scope data lives; both are tracked by #2185, now the last item for this class.Compatibility. Public since 2.2 but unused in Mojarra. This makes it load-bearing and corrects its semantics, so any external caller relying on the current behaviour sees a change. The signature is unchanged.
Testing. Full Faces TCK green. Unit tests are added on the Mojarra side, as this repo has no
UIViewRoottest; both were verified to fail without this change, and removing only the guard fails only the guard test. Note the TCK pins just the signature — javadoc assertionJSF:JAVADOC:3009describes this behaviour, is markedtestable="true", and has no test.🤖 Generated with Claude Code (Opus 4.8)