feat(navigation): support concurrent multi-tab/iframe ZK navigation (ScopedValue + cross-request hand-off) - #112
Merged
Conversation
…f session-scoped PageNavigationController/PageEmbedController render their view via a servlet forward, so setPageLater()/runLater() and the ZK desktop that consumes them (ZKNavigationManager.init(), doAfterCompose()) always run on the same thread within the same request. A session-scoped bean was therefore both unnecessary and unsafe: concurrent tabs/iframes in the same HTTP session would race on the single shared page/params slot. Replaces the @scope("session") bean with a ThreadLocal holder (same public API) and adds a request-completion filter to clear it, so pooled threads don't retain a stale instance across requests. Refs #107, #108 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…o NavigationManager Adds NavigationManager#getId(), a framework-agnostic identifier generated once per instance (BaseNavigationManager), so a manager can be correlated to "which tab/iframe/desktop" it belongs to when several instances are active in the same session at once. Adds NavigationManagerRegistry (@scope("session")), tracking every active instance keyed by id. BaseNavigationManager self-registers on construction. ZKNavigationManager wires best-effort unregistration via a ZK DesktopCleanup listener when a desktop (tab/iframe) is destroyed; correctness doesn't depend on this firing, since the registry itself is session-scoped and is discarded along with the whole session. Enables a future host shell embedding multiple ZK iframes to enumerate or target a specific open tab/iframe's navigation instead of only reaching "the current one" implicitly resolved from the executing request/desktop. Refs #107, #109, #110 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e id, and registry - NavigationManagerSessionTest: same-thread get/set roundtrip, updateNavManager consuming/clearing pending state, runLater/executeQueue ordering, clear() behavior, and the key property motivating the redesign: a page set on one thread must not be visible from another (concurrent tab/iframe isolation). - BaseNavigationManagerIdTest: getId() is non-null, stable, and unique per instance. - NavigationManagerRegistryTest: register/unregister/find/getActiveInstances, including BaseNavigationManager's self-registration when a registry is reachable via Containers, and that construction doesn't fail without one. - NavigationManagerSessionCleanupFilterTest: clears the thread-local after a successful chain and after the chain throws; the pending page stays visible to the chain itself. TestNavigationManager is a minimal concrete BaseNavigationManager used only by these tests (BaseNavigationManager itself is abstract; the only real implementation, ZKNavigationManager, requires a ZK desktop). Verified with `mvn test` on platform/core/navigation and platform/core/web (-am): 21 new tests, all green, no regressions in the reactor. Refs #107, #108, #109, #110 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tionManagerSession Java 25 (this project's target) finalized ScopedValue (JEP 506), the structured-concurrency-friendly replacement for ThreadLocal in exactly this kind of "bind a value for the dynamic extent of a request" use case, with better behavior under virtual threads and no manual cleanup step (the binding is torn down automatically when the bound run/call returns, even on exception). NavigationManagerSession.SCOPE is now a ScopedValue<NavigationManagerSession>; getInstance() throws NoSuchElementException if called outside a bound scope, instead of the old ThreadLocal's implicit lazy-create. Renamed NavigationManagerSessionCleanupFilter -> NavigationManagerSessionScopeFilter, since its role changed from "clear afterward" to "bind the scope around the whole request" via ScopedValue.where(...).call(...) - the filter was only just introduced on this branch, so this is not an API break. Updated tests accordingly, including a concurrency test proving two threads bootstrapping concurrently (simulating two iframes/tabs) never see each other's pending page - the actual property this whole change exists to guarantee. Verified with `mvn clean test` on platform/core/navigation and platform/core/web (-am): all green, no regressions. Refs #107, #108 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
With NavigationManagerSession bound per request (ScopedValue), an intent recorded right before an HTTP redirect (e.g. a LoginListener calling setPageLater/runLater, or a ZK command doing setPageLater + sendRedirect) was lost when the scope ended. NavigationManagerSessionScopeFilter now parks any still-pending intent in the HTTP session at the end of the request and restores it into the next request's instance, where the first desktop bootstrap consumes it. The parked state is transient (queued callbacks are not serializable) and never shared across sessions. - Filter is ordered before the Spring Security chain (@order HIGHEST_PRECEDENCE + 10) so the scope is already bound when login listeners run on AuthenticationSuccessEvent. - NavigationManagerSession: hasPendingState(), absorb(other) and an onPending hook. The hook makes the filter create the HTTP session when the intent is recorded, before a redirect commits the response (found with the demo app: a first request with no session lost the intent). - Javadoc of NavigationManager.setPageLater no longer forbids redirects. Refs #107, #108 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXVz2G6v1fqFdQjRSh9XZF
Real-world cases for verifying NavigationManagerSession and its filter: a filter at Spring Security's order that records setPageLater/runLater and redirects (simulated login), a controller doing the same, a bare desktop that only shows what was handed off, and a callbacks counter. Endpoints live under /demo/handoff/*. Refs #107, #108 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GXVz2G6v1fqFdQjRSh9XZF
This was referenced Sep 30, 2026
1 task done
marioserrano09
added a commit
that referenced
this pull request
Oct 1, 2026
* chore: bump version to 26.10.0 Includes navigation multi-desktop support (#112), inline ZK embed (#114) and localized navigation labels (#116). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> * chore(cli): bump cli.properties versions to 26.10.0 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5.5 <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.
Refs #107, #108 (Epic: concurrent multi-tab / multi-iframe ZK navigation)
What
NavigationManagerSessionis no longer a session-scoped bean with a single shared slot. Each request gets its own instance bound through aScopedValuebyNavigationManagerSessionScopeFilter, so several ZK desktops (iframes / tabs) bootstrapping at once in the same HTTP session no longer overwrite each other's pending page orrunLaterqueue.Also in this branch: stable
NavigationManager.getId()and a session-levelNavigationManagerRegistry(unregistered on ZK desktop cleanup).Last commits: cross-request hand-off
Per-request scope alone breaks callers that record an intent and then end the request without forwarding into a desktop — e.g. a
LoginListenercallingsetPageLater/runLaterfollowed by a redirect, or a ZK command doingsetPageLater+sendRedirect. These are used by downstream apps (found while reviewing dynamia-erp).transient(queued callbacks are not serializable) and never shared across sessions.@Order(HIGHEST_PRECEDENCE + 10): the scope must be bound before Spring Security firesAuthenticationSuccessEventlogin listeners.NavigationManagerSession:hasPendingState(),absorb(), and anonPendinghook so the session exists before a redirect commits the response (bug found with the demo app: first request without a session lost the intent).Behavior notes
NavigationManagerSession.getInstance()throwsNoSuchElementExceptionoutside a bound scope (fail fast; background jobs must not callsetPageLater/runLater).SerializableonNavigationManagerSessionis now vestigial; left untouched to keep this PR focused.Verification
NavigationManagerSessionTest(10),NavigationManagerSessionScopeFilterTest(9, incl. concurrency, cross-request hand-off, no-session-yet redirect, order < Spring Security).examples/demo-zk-books(new/demo/handoff/*endpoints), run from the packaged jar:setPageLater+ redirect → opens the handed-off page (not the default one);/page-embed/*requests on one session → no cross-contamination; noNoSuchElementException/ dropped-state warnings in logs.Not verified
-100simulates the position). Needs a run against dynamia-erp.Follow-ups
EmbedSessionLockFilterbecomes unnecessary once this ships.docs/backend/.🤖 Generated with Claude Code
https://claude.ai/code/session_01GXVz2G6v1fqFdQjRSh9XZF