IBX-12204: SiteAccessService owns a scope-change stack, SiteAccessAware deprecated by Steveb-p · Pull Request #798 · ibexa/core

Steveb-p · 2026-08-05T18:19:28Z

Warning

This branch includes a temporary commit (dependencies.json) that points the Behat CI job at
ibexa/behat#191 so the "Commands use the default
siteaccess if not specified" scenario can run (and pass) before that companion PR merges.
Must be removed before merging this PR.

🎫 Issue	IBX-12204

Description:

A ConfigScopeListener already reacts to MVCEvents::CONFIG_SCOPE_CHANGE/CONFIG_SCOPE_RESTORE and broadcasts the new SiteAccess to every VersatileScopeInterface config resolver and every SiteAccessAware view manager/view provider — but SiteAccessService itself was never one of those broadcast targets. It kept a single value, injected once via SiteAccessAware::setSiteAccess() from request matching, that never updated when a scope change was dispatched (see IBX-8074). Every scope-change call site (ContentPreviewHelper, ConsoleCommandListener) also had to hand-build a ScopeChangeEvent and dispatch it itself, with no supported API for it.

This PR makes SiteAccessService the single owner of "what is the current SiteAccess right now":

Added SiteAccessService::changeSiteAccess(SiteAccess $siteAccess): SiteAccess and restoreSiteAccess(): ?SiteAccess, which wrap the existing ScopeChangeEvent dispatch under MVCEvents::CONFIG_SCOPE_CHANGE/CONFIG_SCOPE_RESTORE — no new event types, just a supported API instead of hand-dispatching.
SiteAccessService now maintains a real LIFO stack of SiteAccess changes instead of a single injected value:
- It implements EventSubscriberInterface and subscribes to MVCEvents::SITEACCESS: on a MAIN_REQUEST match it resets the stack to [$siteAccess]; on a SUB_REQUEST match (fragments, ESI, content-preview's internal sub-request) it pushes onto the stack.
- It also subscribes to KernelEvents::FINISH_REQUEST, which pops the stack — but never below 1 remaining entry. That single "floor guard" is shared with restoreSiteAccess(), so a sub-request's own push is always safely undone by its own finish, and an unbalanced restoreSiteAccess() call (or CLI's one-time changeSiteAccess() floor) can never strip the stack down to empty.
- getCurrent() returns the top of the stack, or null if it's empty — it does not fall back to the shared, container-wide SiteAccess singleton. SiteAccessService (and ConsoleCommandListener, see below) intentionally stop relying on that singleton altogether; a SiteAccess with an "uninitialized" matcher (SiteAccess::MATCHING_TYPE_UNINITIALIZED) is treated as null defensively even if one somehow ends up on the stack.
SiteAccessAware is marked @deprecated (no runtime deprecation notice — it's still legitimately used by consumers not migrated here) in favor of calling SiteAccessService::getCurrent() directly.
Migrated the straightforward "read-only" SiteAccessAware consumers to constructor-inject SiteAccessServiceInterface and call getCurrent() instead of storing a value via setSiteAccess(): ContentPreviewHelper, ConsoleCommandListener, HttpUtils, DefaultRouter, Generator (and UrlAliasGenerator), AliasGeneratorDecorator, DecoratedFragmentRenderer/InlineFragmentRenderer.
- ConsoleCommandListener no longer mutates the shared SiteAccess singleton at all — it constructs a fresh SiteAccess for the CLI invocation and hands it to changeSiteAccess(). ConfigResolver and friends still read that singleton directly for the MATCHING_TYPE_UNINITIALIZED sentinel (see IBX-12192) and are intentionally not migrated in this PR (would require a circular DI dependency back onto SiteAccessService, plus non-trivial caching/broadcast semantics). Same reasoning excludes SiteAccess\Router, View\Manager/Provider\Configured.
- IO/repository integration tests never dispatch a request or console command, so IbexaTestKernel and LegacyTestContainerBuilder's test settings now explicitly seed SiteAccessService via changeSiteAccess() with a freshly-constructed default SiteAccess, the same mechanism CLI uses.
- Found and fixed a latent null-dereference bug in ComplexConfigProcessor while migrating it: it read $this->siteAccessService->getCurrent()->name with no null check. Added a getCurrentSiteAccessName() guard that throws InvalidArgumentException, matching the precedent already set in SiteAccessService::getSiteAccessesRelation().
- Small, intentional behavior change worth flagging: DecoratedFragmentRenderer/InlineFragmentRenderer previously only ever saw the original request's SiteAccess when rewriting fragment paths (they weren't part of ConfigScopeListener's scope-change broadcast). They now correctly reflect whatever SiteAccessService::getCurrent() says, including during an active content-preview scope change — this is a correctness fix, not a regression.
Updated DI wiring accordingly: routing.yml, services.yml, helpers.yml, image.yml, and the SecurityPass/ChainRoutingPass/FragmentPass compiler passes now pass SiteAccessServiceInterface as a constructor argument instead of the old setSiteAccess() calls; SiteAccessService itself gained an EventDispatcherInterface constructor argument and a kernel.event_subscriber tag. For HttpUtils and DefaultRouter — which extend Symfony framework base classes with their own evolving constructors — the new parameter is appended as a nullable, named-argument-bound constructor parameter ($definition->setArgument('$siteAccessService', ...)), so the existing positional arguments coming from Symfony's own service definitions are left untouched.
Pruned the now-stale phpstan-baseline.neon entries left behind by removing SiteAccessAware/setSiteAccess() from the migrated classes.

Known BC break (intentional): any custom Symfony console command that type-hints the shared Ibexa\Core\MVC\Symfony\SiteAccess service directly to read the CLI-resolved siteaccess will no longer see it updated — ConsoleCommandListener used to mutate that shared singleton in place specifically for such consumers, but no longer does (see above). Commands relying on this must switch to injecting SiteAccessServiceInterface and calling getCurrent() instead. Audited the rest of the public ibexa/* org for the same pattern and found exactly two affected spots, both now fixed:

Ibexa\Bundle\Core\Command\DebugConfigResolverCommand (ibexa:debug:config-resolver) in this repo — fixed in this PR.
ibexa/behat's TestSiteaccessCommand fixture command (used by src/bundle/Core/Features/Console/console.feature's "Commands use the default siteaccess if not specified" scenario) — fixed in companion PR ibexa/behat#191, cross-tested here via the temporary dependencies.json (see warning at the top).

Verified: phpstan analyse clean, and the unit_core/bundle_core/bundle_io PHPUnit suites pass (6567/864/27 tests respectively, no failures — only pre-existing PHPUnit-deprecation warnings unrelated to this change).

For QA:

Two scenarios worth exercising manually:

CLI scope change: run any console command with --siteaccess=<name> and confirm SiteAccessService::getCurrent() (and ConfigResolver) reflect that siteaccess for the whole command run.
Nested scope change (content preview): open content preview for a Location under a SiteAccess different from the admin one, and confirm both the previewed page's rendering and any generated fragment/ESI URLs inside it reflect the previewed SiteAccess (this exercises the sub-request push/pop path plus the fragment-renderer behavior fix above).

New unit test coverage lives in SiteAccessServiceTest for the stack semantics specifically: nested changeSiteAccess()/restoreSiteAccess() pairs behaving as LIFO, MAIN_REQUEST resetting vs. SUB_REQUEST pushing, the finish_request floor guard never popping below 1 entry, and a full sub-request-nesting scenario.

Documentation:

SiteAccessAware is now @deprecated — worth a mention in the deprecation notes for this release, pointing integrators at SiteAccessServiceInterface::getCurrent()/changeSiteAccess()/restoreSiteAccess() instead of the old setSiteAccess() + hand-dispatched ScopeChangeEvent pattern.

The BC break above should also be called out explicitly in the release/upgrade notes: custom console commands reading the shared SiteAccess service to determine the CLI-resolved siteaccess must switch to SiteAccessServiceInterface::getCurrent().

@deprecated

…re deprecated SiteAccessService now maintains a real LIFO stack of SiteAccess changes and exposes changeSiteAccess()/restoreSiteAccess(), wrapping the existing ScopeChangeEvent dispatch instead of requiring callers to hand-build it. getCurrent() reflects the active scope correctly, including through nested sub-requests (content preview, fragments, ESI). SiteAccessAware is marked @deprecated; straightforward "read current SiteAccess" consumers (ContentPreviewHelper, ConsoleCommandListener, HttpUtils, DefaultRouter, Generator, AliasGeneratorDecorator, DecoratedFragmentRenderer/InlineFragmentRenderer) are migrated to SiteAccessService::getCurrent() instead.

…etter Generator (and UrlAliasGenerator), HttpUtils, and DefaultRouter now receive SiteAccessServiceInterface as a constructor dependency instead of through a setSiteAccessService() call. For the two classes extending Symfony framework base classes (HttpUtils, DefaultRouter), the new parameter is appended as a nullable, named-argument-bound constructor parameter so the existing positional arguments coming from Symfony's own service definitions are left untouched.

…ctor arg UrlAliasGenerator::__construct() now requires SiteAccessServiceInterface as its 4th argument; the test built the generator via getMockBuilder() with only 3 constructor args, causing an ArgumentCountError on both PHP 8.3 and 8.4 CI jobs.

Removing SiteAccessAware from SiteAccessService also dropped the old setSiteAccess() call that seeded it with the shared, container-wide default SiteAccess singleton at construction time. That meant getCurrent() went from "never null once the container is built" to genuinely null until the first PostSiteAccessMatchEvent, breaking any code that reads it outside of an HTTP request cycle (integration tests, CLI warm-up, etc.) — surfaced by ComplexConfigProcessor/IOConfigResolver now throwing on IO-related integration tests. SiteAccessService now takes that shared SiteAccess singleton as a 4th constructor argument and seeds the stack with it directly (no event dispatch, matching the old setSiteAccess() semantics exactly), restoring the pre-existing guarantee.

SiteAccessService no longer takes the shared, container-wide SiteAccess as a constructor dependency, and getCurrent() goes back to being null until a real request match or an explicit changeSiteAccess() call establishes one. Its "uninitialized" matcher (SiteAccess::MATCHING_TYPE_UNINITIALIZED) is treated the same way defensively, should it ever end up on the stack. ConsoleCommandListener no longer mutates the shared SiteAccess singleton in place either: it now constructs a fresh SiteAccess for the CLI invocation and hands it to SiteAccessService::changeSiteAccess(), which it already called. IO/repository integration tests (IbexaTestKernel and LegacyTestContainerBuilder) never dispatch a request or console command, so they now explicitly seed SiteAccessService via changeSiteAccess() with a freshly-constructed default SiteAccess — the same mechanism CLI uses, not a shared/mutated singleton.

@broken

ConsoleCommandListener no longer mutates the shared SiteAccess singleton in place; it only calls SiteAccessService::changeSiteAccess() now. Any command that type-hints the shared SiteAccess service directly to read the CLI-resolved siteaccess (like the ibexa:behat:test-siteaccess fixture command used by this scenario) no longer sees it updated. This is an intentional, accepted BC break, documented in the PR description; tagging the scenario @broken until the fixture command migrates to SiteAccessServiceInterface::getCurrent().

…vice Every DI reference to SiteAccessService (YAML service arguments, a Reference() in LegacyTestContainerBuilder, and ComplexConfigProcessor's constructor type-hint) now points at SiteAccessServiceInterface instead of the concrete class, consistent with how it's already injected elsewhere (e.g. SiteAccessLimitationType). The concrete class is only still referenced where structurally required: its own service definition, the interface alias's target, and IbexaTestKernel's compiler pass (Symfony aliases have no Definition object to fetch via getDefinition()/hasDefinition()) or a call to onSiteAccessMatch(), a subscriber-only method not part of the interface.

Same bug as ibexa/behat's TestSiteaccessCommand: constructor-injected the bare, shared SiteAccess singleton to display the CLI-resolved siteaccess name, which ConsoleCommandListener no longer mutates in place. Switched to SiteAccessServiceInterface::getCurrent(), matching the pattern already used elsewhere in this PR.

The CLI SiteAccess BC break this scenario caught is now fixed on both known affected consumers: this repo's own DebugConfigResolverCommand, and ibexa/behat's TestSiteaccessCommand (fixed in a companion PR, cross-tested here via a temporary dependencies.json in the next commit).

Points Behat CI at the branch that fixes TestSiteaccessCommand, so this PR's "Commands use the default siteaccess if not specified" scenario can run and pass before that companion PR merges. Must be removed before merging this PR.

Copilot

Pull request overview

Centralizes current SiteAccess and scope-change management in SiteAccessService, migrating consumers away from SiteAccessAware.

Changes:

Adds request-aware SiteAccess stack operations and scope events.
Migrates routing, preview, console, fragment, security, and image services.
Updates dependency injection and tests.

Reviewed changes

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

Show a summary per file

File	Description
`tests/lib/MVC/Symfony/SiteAccess/SiteAccessServiceTest.php`	Tests stack and request semantics.
`tests/lib/MVC/Symfony/Security/HttpUtilsTest.php`	Tests service-based SiteAccess lookup.
`tests/lib/MVC/Symfony/Routing/UrlAliasRouterTest.php`	Supplies the new service dependency.
`tests/lib/MVC/Symfony/Routing/UrlAliasGeneratorTest.php`	Updates generator setup and mocks.
`tests/lib/MVC/Symfony/Routing/GeneratorTest.php`	Tests dynamic current SiteAccess lookup.
`tests/lib/Helper/ContentPreviewHelperTest.php`	Tests delegated scope changes.
`tests/integration/Core/Resources/settings/common.yml`	Seeds integration-test SiteAccess state.
`tests/integration/Core/LegacyTestContainerBuilder.php`	Uses the service interface.
`tests/bundle/IO/DependencyInjection/IbexaIOExtensionTest.php`	Initializes SiteAccess for IO tests.
`tests/bundle/Core/SiteAccess/Config/IOConfigResolverTest.php`	Mocks the service interface.
`tests/bundle/Core/Routing/DefaultRouterTest.php`	Tests constructor-injected service usage.
`tests/bundle/Core/Fragment/DecoratedFragmentRendererTest.php`	Tests current-scope fragment rewriting.
`tests/bundle/Core/EventListener/ConsoleCommandListenerTest.php`	Tests CLI scope changes.
`tests/bundle/Core/DependencyInjection/Compiler/SecurityPassTest.php`	Verifies HttpUtils injection.
`tests/bundle/Core/DependencyInjection/Compiler/FragmentPassTest.php`	Verifies fragment dependencies.
`tests/bundle/Core/DependencyInjection/Compiler/ChainRoutingPassTest.php`	Verifies router injection.
`src/lib/Resources/settings/roles.yml`	Injects the service interface.
`src/lib/MVC/Symfony/SiteAccess/SiteAccessServiceInterface.php`	Exposes change and restore APIs.
`src/lib/MVC/Symfony/SiteAccess/SiteAccessService.php`	Implements stack ownership and events.
`src/lib/MVC/Symfony/SiteAccess/SiteAccessAware.php`	Deprecates the legacy interface.
`src/lib/MVC/Symfony/Security/HttpUtils.php`	Reads SiteAccess dynamically.
`src/lib/MVC/Symfony/Routing/Generator/UrlAliasGenerator.php`	Injects SiteAccessService.
`src/lib/MVC/Symfony/Routing/Generator.php`	Uses the current SiteAccess for URLs.
`src/lib/MVC/Symfony/Controller/Content/PreviewController.php`	Guards missing SiteAccess state.
`src/lib/Helper/ContentPreviewHelper.php`	Delegates scope stack operations.
`src/contracts/Test/IbexaTestKernel.php`	Seeds test-kernel SiteAccess state.
`src/bundle/Core/SiteAccess/Config/ComplexConfigProcessor.php`	Handles absent current SiteAccess.
`src/bundle/Core/Routing/DefaultRouter.php`	Uses dynamic SiteAccess lookup.
`src/bundle/Core/Resources/config/services.yml`	Rewires console and fragment services.
`src/bundle/Core/Resources/config/routing.yml`	Configures generator and service dependencies.
`src/bundle/Core/Resources/config/image.yml`	Rewires image alias generation.
`src/bundle/Core/Resources/config/helpers.yml`	Rewires preview and config helpers.
`src/bundle/Core/Resources/config/commands.yml`	Updates debug-command dependency.
`src/bundle/Core/Imagine/Cache/AliasGeneratorDecorator.php`	Uses current SiteAccess in cache metadata.
`src/bundle/Core/Fragment/InlineFragmentRenderer.php`	Removes obsolete SiteAccess storage.
`src/bundle/Core/Fragment/DecoratedFragmentRenderer.php`	Rewrites paths using current SiteAccess.
`src/bundle/Core/EventListener/ConsoleCommandListener.php`	Establishes fresh CLI SiteAccess state.
`src/bundle/Core/DependencyInjection/Compiler/SecurityPass.php`	Constructor-injects HttpUtils dependency.
`src/bundle/Core/DependencyInjection/Compiler/FragmentPass.php`	Injects fragment renderer dependency.
`src/bundle/Core/DependencyInjection/Compiler/ChainRoutingPass.php`	Injects default-router dependency.
`src/bundle/Core/Command/DebugConfigResolverCommand.php`	Reads CLI SiteAccess from the service.
`phpstan-baseline.neon`	Removes obsolete suppressions.
`dependencies.json`	Temporarily pins the companion Behat branch.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

+    public function onKernelFinishRequest(FinishRequestEvent $event): void
+    {
+        $this->popSiteAccessStack();


+{
+    "recipesEndpoint": "",
+    "packages": [
+        {
+            "requirement": "dev-IBX-12204-siteaccessservice-scope-stack as 6.0.x-dev",
+            "repositoryUrl": "https://github.com/ibexa/behat.git",
+            "package": "ibexa/behat",
+            "shouldBeAddedAsVCS": false
+        }
+    ]
+}


previewContentAction() only called restoreConfigScope()/setPreviewActive(false) on the success path; both catch blocks for the forwarded sub-request returned early, skipping them. This pre-existing gap didn't matter before, since SiteAccessService never tracked scope changes at all — but now that changeConfigScope() pushes onto its stack, skipping the restore leaves SiteAccessService and ConfigResolver disagreeing on the current scope: the stack silently unwinds on request finish (no CONFIG_SCOPE_RESTORE dispatch), so SiteAccessService::getCurrent() reverts while ConfigResolver, updated by ConfigScopeListener reacting to the earlier CONFIG_SCOPE_CHANGE, stays on the preview scope. Wrapped the sub-request handling in try/finally so both calls always run, regardless of how the method exits. Calling restoreConfigScope() even when the scope was never changed is safe: restoreSiteAccess()'s floor guard never pops past the request-matched entry, so it's a no-op in that case (matching the pre-existing test expectations, which already assumed an unconditional call). Flagged by GitHub Copilot's review on PR #798.

sonarqubecloud · 2026-08-12T11:28:58Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.3% Duplication on New Code

See analysis details on SonarQube Cloud

Steveb-p added 8 commits August 5, 2026 20:19

Steveb-p mentioned this pull request Aug 7, 2026

IBX-12204: Read current SiteAccess via SiteAccessServiceInterface ibexa/behat#191

Draft

Steveb-p added 2 commits August 7, 2026 16:49

Steveb-p requested a balanced review from Copilot August 8, 2026 23:19

Copilot started reviewing on behalf of Steveb-p August 8, 2026 23:19 View session

Copilot AI reviewed Aug 8, 2026

View reviewed changes

This was referenced Aug 10, 2026

IBX-12102: Fixed SiteAccess context being lost after fragment sub-requests #802

Open

Added doc for SiteAccessStamp ibexa/documentation-developer#3340

Open

This was referenced Aug 12, 2026

IBX-8074: Made SiteAccessService react to CONFIG_SCOPE_CHANGE/RESTORE #805

Open

IBX-8074: Made SiteAccessService react to CONFIG_SCOPE_CHANGE/RESTORE #806

Open

Navigation Menu

Uh oh!

IBX-12204: SiteAccessService owns a scope-change stack, SiteAccessAware deprecated - #798

IBX-12204: SiteAccessService owns a scope-change stack, SiteAccessAware deprecated#798
Steveb-p wants to merge 11 commits into
6.0from
IBX-12204-siteaccessservice-scope-stack

Steveb-p commented Aug 5, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

sonarqubecloud Bot commented Aug 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Steveb-p commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

For QA:

Documentation:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

sonarqubecloud Bot commented Aug 12, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Steveb-p commented Aug 5, 2026 •

edited

Loading