From 22716b37712fc3fe3e51bb8465cbcfe358c7871f Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Wed, 9 Sep 2026 11:13:12 +0200 Subject: [PATCH 1/9] IBX-12046: Removed Symfony 8 deprecated code usage Replaced DefaultRouter's inheritance from the @final FrameworkBundle Router with a decorator of router.default, dropped symfony/templating from the base Controller, and aligned Security, Validator, HttpFoundation and OptionsResolver usages with the Symfony 7.4 contracts so the unit and integration suites report no direct Symfony deprecations. --- composer.json | 2 +- phpstan-baseline.neon | 12 - .../Compiler/ChainRoutingPass.php | 15 +- .../Compiler/RouterPass.php | 26 -- src/bundle/Core/IbexaCoreBundle.php | 2 - .../Core/Imagine/Filter/UnsupportedFilter.php | 2 +- src/bundle/Core/Resources/config/routing.yml | 14 + src/bundle/Core/Routing/DefaultRouter.php | 115 ++++--- .../Debug/Collector/IbexaCoreCollector.php | 4 +- .../Repository/Values/ValueObject.php | 2 +- .../Constraint/UniqueIdentifier.php | 49 ++- .../Controller/Content/QueryController.php | 5 +- src/lib/MVC/Symfony/Controller/Controller.php | 18 +- .../Controller/QueryRenderController.php | 10 +- .../Authorization/Voter/CoreVoter.php | 3 +- .../Authorization/Voter/ValueObjectVoter.php | 3 +- src/lib/MVC/Symfony/Security/User.php | 16 +- src/lib/MVC/Symfony/Security/UserChecker.php | 3 +- src/lib/MVC/Symfony/Security/UserWrapped.php | 4 + .../Extension/FieldRenderingExtension.php | 2 +- .../QueryType/BuiltIn/AbstractQueryType.php | 23 +- .../LocationIsContainerContentType.php | 6 +- .../Compiler/ChainRoutingPassTest.php | 26 +- ...ustedHeaderClientIpEventSubscriberTest.php | 13 +- .../bundle/Core/Routing/DefaultRouterTest.php | 280 +++++++----------- .../MVC/Symfony/Controller/ControllerTest.php | 6 +- ...sitoryUserAuthenticationSubscriberTest.php | 3 +- tests/lib/MVC/Symfony/Security/UserTest.php | 1 - .../RemoteIdentifierMapperTest.php | 2 +- 29 files changed, 303 insertions(+), 364 deletions(-) delete mode 100644 src/bundle/Core/DependencyInjection/Compiler/RouterPass.php diff --git a/composer.json b/composer.json index 46f22641f1..529f143c68 100644 --- a/composer.json +++ b/composer.json @@ -47,13 +47,13 @@ "symfony/http-foundation": "^7.4", "symfony/http-kernel": "^7.4", "symfony/mime": "^7.4", + "symfony/options-resolver": "^7.4", "symfony/polyfill-php80": "^1.27", "symfony/process": "^7.4", "symfony/security-bundle": "^7.4", "symfony/security-core": "^7.4", "symfony/security-http": "^7.4", "symfony/serializer": "^7.4", - "symfony/templating": "^6.4.0", "symfony/translation": "^7.4", "symfony/validator": "^7.4", "symfony/var-dumper": "^7.4", diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 25372ad367..91af3b7be3 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -2742,12 +2742,6 @@ parameters: count: 1 path: src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php - - - message: '#^Class Ibexa\\Bundle\\Core\\Routing\\DefaultRouter extends @final class Symfony\\Bundle\\FrameworkBundle\\Routing\\Router\.$#' - identifier: class.extendsFinalByPhpDoc - count: 1 - path: src/bundle/Core/Routing/DefaultRouter.php - - message: '#^Cannot access property \$name on Ibexa\\Core\\MVC\\Symfony\\SiteAccess\|null\.$#' identifier: property.nonObject @@ -7086,12 +7080,6 @@ parameters: count: 1 path: src/lib/MVC/Symfony/Controller/Controller.php - - - message: '#^Method Ibexa\\Core\\MVC\\Symfony\\Controller\\Controller\:\:getTemplateEngine\(\) should return Symfony\\Component\\Templating\\EngineInterface but returns object\.$#' - identifier: return.type - count: 1 - path: src/lib/MVC/Symfony/Controller/Controller.php - - message: '#^Method Ibexa\\Core\\MVC\\Symfony\\Controller\\Controller\:\:render\(\) has parameter \$parameters with no value type specified in iterable type array\.$#' identifier: missingType.iterableValue diff --git a/src/bundle/Core/DependencyInjection/Compiler/ChainRoutingPass.php b/src/bundle/Core/DependencyInjection/Compiler/ChainRoutingPass.php index 76d9769a08..b9ef758fde 100644 --- a/src/bundle/Core/DependencyInjection/Compiler/ChainRoutingPass.php +++ b/src/bundle/Core/DependencyInjection/Compiler/ChainRoutingPass.php @@ -8,8 +8,6 @@ namespace Ibexa\Bundle\Core\DependencyInjection\Compiler; use Ibexa\Core\MVC\Symfony\Routing\ChainRouter; -use Ibexa\Core\MVC\Symfony\SiteAccess; -use Ibexa\Core\MVC\Symfony\SiteAccess\Router; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Reference; @@ -25,19 +23,10 @@ public function process(ContainerBuilder $container): void $chainRouter = $container->getDefinition(ChainRouter::class); // Enforce default router to be part of the routing chain - // The default router will be given the highest priority so that it will be used by default + // The default router will be given the highest priority so that it will be used by default. + // The SiteAccess-aware behavior is provided by \Ibexa\Bundle\Core\Routing\DefaultRouter decorating router.default. if ($container->hasDefinition('router.default')) { $defaultRouter = $container->getDefinition('router.default'); - $defaultRouter->addMethodCall('setSiteAccess', [new Reference(SiteAccess::class)]); - $defaultRouter->addMethodCall('setConfigResolver', [new Reference('ibexa.config.resolver')]); - $defaultRouter->addMethodCall( - 'setNonSiteAccessAwareRoutes', - ['%ibexa.default_router.non_site_access_aware_routes%'] - ); - $defaultRouter->addMethodCall( - 'setSiteAccessRouter', - [new Reference(Router::class)] - ); if (!$defaultRouter->hasTag('router')) { $defaultRouter->addTag( 'router', diff --git a/src/bundle/Core/DependencyInjection/Compiler/RouterPass.php b/src/bundle/Core/DependencyInjection/Compiler/RouterPass.php deleted file mode 100644 index 27874182ec..0000000000 --- a/src/bundle/Core/DependencyInjection/Compiler/RouterPass.php +++ /dev/null @@ -1,26 +0,0 @@ -hasDefinition('router.default')) { - return; - } - - $container - ->findDefinition('router.default') - ->setClass(DefaultRouter::class); - } -} diff --git a/src/bundle/Core/IbexaCoreBundle.php b/src/bundle/Core/IbexaCoreBundle.php index 2904692d85..9049d661bb 100644 --- a/src/bundle/Core/IbexaCoreBundle.php +++ b/src/bundle/Core/IbexaCoreBundle.php @@ -24,7 +24,6 @@ use Ibexa\Bundle\Core\DependencyInjection\Compiler\RegisterSearchEngineIndexerPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\RegisterSearchEnginePass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\RegisterStorageEnginePass; -use Ibexa\Bundle\Core\DependencyInjection\Compiler\RouterPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\SecurityPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\SessionConfigurationPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\SiteAccessMatcherRegistryPass; @@ -69,7 +68,6 @@ public function build(ContainerBuilder $container): void $container->addCompilerPass(new RegisterSearchEngineIndexerPass()); $container->addCompilerPass(new AggregateFieldValueMapperPass()); $container->addCompilerPass(new FieldRegistryPass()); - $container->addCompilerPass(new RouterPass()); $container->addCompilerPass(new SecurityPass()); $container->addCompilerPass(new FragmentPass()); $container->addCompilerPass(new StorageConnectionPass()); diff --git a/src/bundle/Core/Imagine/Filter/UnsupportedFilter.php b/src/bundle/Core/Imagine/Filter/UnsupportedFilter.php index 2b6cfd1f48..8ce5359216 100644 --- a/src/bundle/Core/Imagine/Filter/UnsupportedFilter.php +++ b/src/bundle/Core/Imagine/Filter/UnsupportedFilter.php @@ -15,7 +15,7 @@ class UnsupportedFilter extends AbstractFilter /** * @throws \Imagine\Exception\NotSupportedException */ - public function apply(ImageInterface $image) + public function apply(ImageInterface $image): ImageInterface { throw new NotSupportedException('The filter is not supported by your current configuration.'); } diff --git a/src/bundle/Core/Resources/config/routing.yml b/src/bundle/Core/Resources/config/routing.yml index 59f463f70e..1e0fbe170d 100644 --- a/src/bundle/Core/Resources/config/routing.yml +++ b/src/bundle/Core/Resources/config/routing.yml @@ -11,6 +11,20 @@ parameters: ">" : "%3E" services: + ibexa.routing.default_router: + class: Ibexa\Bundle\Core\Routing\DefaultRouter + decorates: router.default + decoration_on_invalid: ignore + arguments: + $innerRouter: '@.inner' + $siteAccessRouter: '@Ibexa\Core\MVC\Symfony\SiteAccess\Router' + $nonSiteAccessAwareRoutes: '%ibexa.default_router.non_site_access_aware_routes%' + $logger: '@?logger' + calls: + - [setSiteAccess, ['@?Ibexa\Core\MVC\Symfony\SiteAccess']] + tags: + - { name: monolog.logger, channel: router } + Ibexa\Core\MVC\Symfony\Routing\ChainRouter: class: Ibexa\Core\MVC\Symfony\Routing\ChainRouter arguments: ["@?logger"] diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php index b8999d2460..a9bf267d32 100644 --- a/src/bundle/Core/Routing/DefaultRouter.php +++ b/src/bundle/Core/Routing/DefaultRouter.php @@ -4,63 +4,82 @@ * @copyright Copyright (C) Ibexa AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ +declare(strict_types=1); namespace Ibexa\Bundle\Core\Routing; -use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; use Ibexa\Core\MVC\Symfony\Routing\RequestContextFactory; use Ibexa\Core\MVC\Symfony\Routing\SimplifiedRequest; use Ibexa\Core\MVC\Symfony\SiteAccess; use Ibexa\Core\MVC\Symfony\SiteAccess\SiteAccessAware; use Ibexa\Core\MVC\Symfony\SiteAccess\SiteAccessRouterInterface; use Ibexa\Core\MVC\Symfony\SiteAccess\URILexer; -use Symfony\Bundle\FrameworkBundle\Routing\Router; +use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; use Symfony\Component\Routing\Exception\RouteNotFoundException; +use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext; +use Symfony\Component\Routing\RouteCollection; +use Symfony\Component\Routing\RouterInterface; /** - * Extension of Symfony default router implementing RequestMatcherInterface. + * SiteAccess-aware decorator of the Symfony router. + * + * Matching honours the `semanticPathinfo` request attribute set by the SiteAccess matcher, and link generation + * prepends the SiteAccess URI part (for URI-based SiteAccess matchers) and supports the `siteaccess` route parameter. */ -class DefaultRouter extends Router implements SiteAccessAware +final class DefaultRouter implements RouterInterface, RequestMatcherInterface, WarmableInterface, SiteAccessAware { - protected ?SiteAccess $siteAccess = null; + private ?SiteAccess $siteAccess = null; - /** @var string[] */ - protected array $nonSiteAccessAwareRoutes = []; + /** + * @param string[] $nonSiteAccessAwareRoutes route name prefixes that are not supposed to be SiteAccess aware, + * i.e. routes pointing to asset generation + */ + public function __construct( + private readonly RouterInterface&RequestMatcherInterface $innerRouter, + private readonly SiteAccessRouterInterface $siteAccessRouter, + private readonly array $nonSiteAccessAwareRoutes = [], + private readonly ?LoggerInterface $logger = null + ) { + } - protected ConfigResolverInterface $configResolver; + public function setSiteAccess(?SiteAccess $siteAccess = null): void + { + $this->siteAccess = $siteAccess; + } - protected SiteAccessRouterInterface $siteAccessRouter; + public function getInnerRouter(): RouterInterface&RequestMatcherInterface + { + return $this->innerRouter; + } - public function setConfigResolver(ConfigResolverInterface $configResolver): void + public function setContext(RequestContext $context): void { - $this->configResolver = $configResolver; + $this->innerRouter->setContext($context); } - public function setSiteAccess(?SiteAccess $siteAccess = null): void + public function getContext(): RequestContext { - $this->siteAccess = $siteAccess; + return $this->innerRouter->getContext(); } - /** - * Injects route names that are not supposed to be SiteAccess aware. - * i.e. Routes pointing to asset generation (like assetic). - * - * @param string[] $routes - */ - public function setNonSiteAccessAwareRoutes(array $routes): void + public function getRouteCollection(): RouteCollection { - $this->nonSiteAccessAwareRoutes = $routes; + return $this->innerRouter->getRouteCollection(); } - public function setSiteAccessRouter(SiteAccessRouterInterface $siteAccessRouter): void + /** + * @return array + */ + public function match(string $pathinfo): array { - $this->siteAccessRouter = $siteAccessRouter; + return $this->innerRouter->match($pathinfo); } /** - * @return array An array of parameters + * @return array */ public function matchRequest(Request $request): array { @@ -72,7 +91,7 @@ public function matchRequest(Request $request): array ); } - return parent::matchRequest($request); + return $this->innerRouter->matchRequest($request); } /** @@ -91,16 +110,16 @@ public function generate(string $name, array $parameters = [], int $referenceTyp // Switch request context for link generation. $context = $this->getContextBySimplifiedRequest($siteAccess->matcher->getRequest()); $this->setContext($context); - } elseif ($this->logger) { + } else { $siteAccess = $this->siteAccess; - $this->logger->notice("Could not generate a link using provided 'siteaccess' parameter: {$parameters['siteaccess']}. Generating using current context."); + $this->logger?->notice("Could not generate a link using provided 'siteaccess' parameter: {$parameters['siteaccess']}. Generating using current context."); } unset($parameters['siteaccess']); } try { - $url = parent::generate($name, $parameters, $referenceType); + $url = $this->innerRouter->generate($name, $parameters, $referenceType); } catch (RouteNotFoundException $e) { // Switch back to original context, for next links generation. $this->setContext($originalContext); @@ -108,14 +127,14 @@ public function generate(string $name, array $parameters = [], int $referenceTyp } // Now putting back SiteAccess URI if needed. - if ($isSiteAccessAware && $siteAccess && $siteAccess->matcher instanceof URILexer) { + if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) { if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { $scheme = $context->getScheme(); $port = ''; - if ($scheme === 'http' && $this->context->getHttpPort() !== 80) { - $port = ':' . $this->context->getHttpPort(); - } elseif ($scheme === 'https' && $this->context->getHttpsPort() !== 443) { - $port = ':' . $this->context->getHttpsPort(); + if ($scheme === 'http' && $context->getHttpPort() !== 80) { + $port = ':' . $context->getHttpPort(); + } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { + $port = ':' . $context->getHttpsPort(); } $base = $context->getHost() . $port . $context->getBaseUrl(); @@ -134,18 +153,15 @@ public function generate(string $name, array $parameters = [], int $referenceTyp } /** - * Checks if $routeName is a siteAccess aware route, and thus needs to have siteAccess URI prepended. - * Will be used for link generation, only in the case of URI SiteAccess matching. + * @return string[] */ - protected function isSiteAccessAwareRoute(string $routeName): bool + public function warmUp(string $cacheDir, ?string $buildDir = null): array { - foreach ($this->nonSiteAccessAwareRoutes as $ignoredPrefix) { - if (str_starts_with($routeName, $ignoredPrefix)) { - return false; - } + if ($this->innerRouter instanceof WarmableInterface) { + return $this->innerRouter->warmUp($cacheDir, $buildDir); } - return true; + return []; } /** @@ -154,6 +170,21 @@ protected function isSiteAccessAwareRoute(string $routeName): bool public function getContextBySimplifiedRequest(SimplifiedRequest $simplifiedRequest): RequestContext { // inline-instantiated on purpose as it's lightweight and injecting it here through DI can be complicated - return (new RequestContextFactory($this->context))->getContextBySimplifiedRequest($simplifiedRequest); + return (new RequestContextFactory($this->getContext()))->getContextBySimplifiedRequest($simplifiedRequest); + } + + /** + * Checks if $routeName is a siteAccess aware route, and thus needs to have siteAccess URI prepended. + * Will be used for link generation, only in the case of URI SiteAccess matching. + */ + private function isSiteAccessAwareRoute(string $routeName): bool + { + foreach ($this->nonSiteAccessAwareRoutes as $ignoredPrefix) { + if (str_starts_with($routeName, $ignoredPrefix)) { + return false; + } + } + + return true; } } diff --git a/src/bundle/Debug/Collector/IbexaCoreCollector.php b/src/bundle/Debug/Collector/IbexaCoreCollector.php index e3cf10d475..9a5c277499 100644 --- a/src/bundle/Debug/Collector/IbexaCoreCollector.php +++ b/src/bundle/Debug/Collector/IbexaCoreCollector.php @@ -21,7 +21,7 @@ public function __construct() $this->reset(); } - public function collect(Request $request, Response $response, ?Throwable $exception = null) + public function collect(Request $request, Response $response, ?Throwable $exception = null): void { /** @var \Symfony\Component\HttpKernel\DataCollector\DataCollectorInterface $innerCollector */ foreach ($this->data['collectors'] as $innerCollector) { @@ -104,7 +104,7 @@ public function getPanelTemplate($collectorName) /** * {@inheritdoc} */ - public function reset() + public function reset(): void { $this->data = [ 'collectors' => [], diff --git a/src/contracts/Repository/Values/ValueObject.php b/src/contracts/Repository/Values/ValueObject.php index 8ea2c030c8..125b586c1d 100644 --- a/src/contracts/Repository/Values/ValueObject.php +++ b/src/contracts/Repository/Values/ValueObject.php @@ -9,7 +9,7 @@ use Ibexa\Contracts\Core\Repository\Exceptions\PropertyNotFoundException; use Ibexa\Contracts\Core\Repository\Exceptions\PropertyReadOnlyException; -use Symfony\Component\Serializer\Annotation\Ignore as SerializerIgnore; +use Symfony\Component\Serializer\Attribute\Ignore as SerializerIgnore; /** * The base class for all value objects and structs. diff --git a/src/contracts/Validation/Constraint/UniqueIdentifier.php b/src/contracts/Validation/Constraint/UniqueIdentifier.php index 39d535bdaf..82ea033db0 100644 --- a/src/contracts/Validation/Constraint/UniqueIdentifier.php +++ b/src/contracts/Validation/Constraint/UniqueIdentifier.php @@ -10,6 +10,7 @@ use JMS\TranslationBundle\Model\Message; use JMS\TranslationBundle\Translation\TranslationContainerInterface; +use Symfony\Component\Validator\Attribute\HasNamedArguments; use Symfony\Component\Validator\Constraint; abstract class UniqueIdentifier extends Constraint implements TranslationContainerInterface @@ -24,9 +25,46 @@ abstract class UniqueIdentifier extends Constraint implements TranslationContain public ?string $reportErrorPath = null; - public function getDefaultOption(): string - { - return 'identifierPath'; + /** + * @param string|array $identifierPath Property path of the identifier, or (deprecated) an options array + * @param array|null $groups + */ + #[HasNamedArguments] + public function __construct( + string|array $identifierPath, + ?string $existingIdPath = null, + ?string $reportErrorPath = null, + ?string $message = null, + ?array $groups = null, + mixed $payload = null + ) { + if (is_array($identifierPath)) { + trigger_deprecation( + 'ibexa/core', + '6.0', + 'Passing an options array to "%s" is deprecated, use named arguments instead.', + static::class + ); + + $options = $identifierPath; + $identifierPath = $options['identifierPath'] ?? $options['value'] ?? null; + $existingIdPath ??= $options['existingIdPath'] ?? null; + $reportErrorPath ??= $options['reportErrorPath'] ?? null; + $message ??= $options['message'] ?? null; + $groups ??= $options['groups'] ?? null; + $payload ??= $options['payload'] ?? null; + + if (!is_string($identifierPath)) { + throw new \InvalidArgumentException(sprintf('The "identifierPath" option of "%s" is required.', static::class)); + } + } + + parent::__construct(null, $groups, $payload); + + $this->identifierPath = $identifierPath; + $this->existingIdPath = $existingIdPath; + $this->reportErrorPath = $reportErrorPath; + $this->message = $message ?? static::MESSAGE; } /** @@ -37,11 +75,6 @@ public function getTargets(): array return [self::CLASS_CONSTRAINT]; } - public function getRequiredOptions(): array - { - return ['identifierPath']; - } - public static function getTranslationMessages(): array { return [ diff --git a/src/lib/MVC/Symfony/Controller/Content/QueryController.php b/src/lib/MVC/Symfony/Controller/Content/QueryController.php index 35fdafd6bb..74efcbe9cb 100644 --- a/src/lib/MVC/Symfony/Controller/Content/QueryController.php +++ b/src/lib/MVC/Symfony/Controller/Content/QueryController.php @@ -119,7 +119,10 @@ private function runPagingQuery(ContentView $view, Request $request) $limit = $queryParameters['limit'] ?? 10; $pageParam = $queryParameters['page_param'] ?? 'page'; - $page = $request->get($pageParam, 1); + $page = $request->attributes->get($pageParam) + ?? $request->query->get($pageParam) + ?? $request->request->get($pageParam) + ?? 1; $pager = new Pagerfanta( $this->getAdapter($this->contentViewQueryTypeMapper->map($view)) diff --git a/src/lib/MVC/Symfony/Controller/Controller.php b/src/lib/MVC/Symfony/Controller/Controller.php index 2e26e45a37..b2fda5a457 100644 --- a/src/lib/MVC/Symfony/Controller/Controller.php +++ b/src/lib/MVC/Symfony/Controller/Controller.php @@ -17,8 +17,8 @@ use Symfony\Component\HttpFoundation\RequestStack; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface; -use Symfony\Component\Templating\EngineInterface; use Symfony\Contracts\Service\ServiceSubscriberInterface; +use Twig\Environment; abstract class Controller implements ServiceSubscriberInterface { @@ -81,17 +81,19 @@ public function render($view, array $parameters = [], ?Response $response = null $response = new Response(); } - $response->setContent($this->getTemplateEngine()->render($view, $parameters)); + $response->setContent($this->getTwig()->render($view, $parameters)); return $response; } - /** - * @return \Symfony\Component\Templating\EngineInterface - */ - public function getTemplateEngine() + public function getTwig(): Environment { - return $this->container->get('templating'); + $twig = $this->container->get('twig'); + if (!$twig instanceof Environment) { + throw new \LogicException(sprintf('The "twig" service must be an instance of %s, %s given.', Environment::class, get_debug_type($twig))); + } + + return $twig; } /** @@ -142,7 +144,7 @@ public static function getSubscribedServices(): array { return [ 'logger' => '?' . LoggerInterface::class, - 'templating' => EngineInterface::class, + 'twig' => Environment::class, 'ibexa.config.resolver' => ConfigResolverInterface::class, 'ibexa.api.repository' => Repository::class, 'request_stack' => RequestStack::class, diff --git a/src/lib/MVC/Symfony/Controller/QueryRenderController.php b/src/lib/MVC/Symfony/Controller/QueryRenderController.php index 062190b6fa..e2cb9be634 100644 --- a/src/lib/MVC/Symfony/Controller/QueryRenderController.php +++ b/src/lib/MVC/Symfony/Controller/QueryRenderController.php @@ -48,7 +48,11 @@ public function renderQuery(Request $request, array $options): QueryView $results = new Pagerfanta($this->getAdapter($options)); if ($options['pagination']['enabled']) { - $currentPage = $request->get($options['pagination']['page_param'], 1); + $pageParam = $options['pagination']['page_param']; + $currentPage = $request->attributes->get($pageParam) + ?? $request->query->get($pageParam) + ?? $request->request->get($pageParam) + ?? 1; $results->setAllowOutOfRangePages(true); $results->setMaxPerPage($options['pagination']['limit']); @@ -71,7 +75,7 @@ private function resolveOptions(array $options): array { $resolver = new OptionsResolver(); - $resolver->setDefault('query', static function (OptionsResolver $resolver): void { + $resolver->setOptions('query', static function (OptionsResolver $resolver): void { $resolver->setDefaults([ 'parameters' => [], 'assign_results_to' => 'items', @@ -83,7 +87,7 @@ private function resolveOptions(array $options): array $resolver->setAllowedTypes('assign_results_to', 'string'); }); - $resolver->setDefault('pagination', static function (OptionsResolver $resolver): void { + $resolver->setOptions('pagination', static function (OptionsResolver $resolver): void { $resolver->setDefaults([ 'enabled' => true, 'limit' => 10, diff --git a/src/lib/MVC/Symfony/Security/Authorization/Voter/CoreVoter.php b/src/lib/MVC/Symfony/Security/Authorization/Voter/CoreVoter.php index 8a58d7ef21..afb9263ddb 100644 --- a/src/lib/MVC/Symfony/Security/Authorization/Voter/CoreVoter.php +++ b/src/lib/MVC/Symfony/Security/Authorization/Voter/CoreVoter.php @@ -10,6 +10,7 @@ use Ibexa\Contracts\Core\Repository\PermissionResolver; use Ibexa\Core\MVC\Symfony\Security\Authorization\Attribute as AuthorizationAttribute; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; class CoreVoter implements VoterInterface @@ -58,7 +59,7 @@ public function supportsClass($class): bool * * @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED */ - public function vote(TokenInterface $token, $object, array $attributes): int + public function vote(TokenInterface $token, $object, array $attributes, ?Vote $vote = null): int { foreach ($attributes as $attribute) { if ($this->supportsAttribute($attribute)) { diff --git a/src/lib/MVC/Symfony/Security/Authorization/Voter/ValueObjectVoter.php b/src/lib/MVC/Symfony/Security/Authorization/Voter/ValueObjectVoter.php index 422308faff..8f9bede9a5 100644 --- a/src/lib/MVC/Symfony/Security/Authorization/Voter/ValueObjectVoter.php +++ b/src/lib/MVC/Symfony/Security/Authorization/Voter/ValueObjectVoter.php @@ -10,6 +10,7 @@ use Ibexa\Contracts\Core\Repository\PermissionResolver; use Ibexa\Core\MVC\Symfony\Security\Authorization\Attribute as AuthorizationAttribute; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; +use Symfony\Component\Security\Core\Authorization\Voter\Vote; use Symfony\Component\Security\Core\Authorization\Voter\VoterInterface; /** @@ -54,7 +55,7 @@ public function supportsClass($class): bool * * @return int either ACCESS_GRANTED, ACCESS_ABSTAIN, or ACCESS_DENIED */ - public function vote(TokenInterface $token, $object, array $attributes): int + public function vote(TokenInterface $token, $object, array $attributes, ?Vote $vote = null): int { foreach ($attributes as $attribute) { if ($this->supportsAttribute($attribute)) { diff --git a/src/lib/MVC/Symfony/Security/User.php b/src/lib/MVC/Symfony/Security/User.php index db17bbc6d4..9a1e557d28 100644 --- a/src/lib/MVC/Symfony/Security/User.php +++ b/src/lib/MVC/Symfony/Security/User.php @@ -66,16 +66,6 @@ public function getPassword(): string return $this->getAPIUser()->getPasswordHash(); } - /** - * Returns the salt that was originally used to encode the password. - * - * This can return null if the password was not encoded using a salt. - */ - public function getSalt(): ?string - { - return null; - } - /** * Returns the username used to authenticate the user. */ @@ -90,11 +80,11 @@ public function getUserIdentifier(): string } /** - * Removes sensitive data from the user. + * Nothing to erase: the API user is never serialized (see {@see self::__sleep()}). * - * This is important if, at any given point, sensitive information like - * the plain-text password is stored on this object. + * @deprecated since Symfony 7.3, {@see \Symfony\Component\Security\Core\User\UserInterface::eraseCredentials()} is removed in Symfony 8.0 */ + #[\Deprecated] public function eraseCredentials(): void { } diff --git a/src/lib/MVC/Symfony/Security/UserChecker.php b/src/lib/MVC/Symfony/Security/UserChecker.php index 40703b4b8b..da7f48dcbc 100644 --- a/src/lib/MVC/Symfony/Security/UserChecker.php +++ b/src/lib/MVC/Symfony/Security/UserChecker.php @@ -11,6 +11,7 @@ use Ibexa\Contracts\Core\Repository\UserService; use Ibexa\Core\MVC\Symfony\Security\Exception\PasswordExpiredException; use Ibexa\Core\MVC\Symfony\Security\UserInterface as IbexaUserInterface; +use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; use Symfony\Component\Security\Core\Exception\DisabledException; use Symfony\Component\Security\Core\User\UserCheckerInterface; use Symfony\Component\Security\Core\User\UserInterface; @@ -39,7 +40,7 @@ public function checkPreAuth(UserInterface $user): void } } - public function checkPostAuth(UserInterface $user): void + public function checkPostAuth(UserInterface $user, ?TokenInterface $token = null): void { if (!$user instanceof IbexaUserInterface) { return; diff --git a/src/lib/MVC/Symfony/Security/UserWrapped.php b/src/lib/MVC/Symfony/Security/UserWrapped.php index 941461a7e3..973a6a58de 100644 --- a/src/lib/MVC/Symfony/Security/UserWrapped.php +++ b/src/lib/MVC/Symfony/Security/UserWrapped.php @@ -93,6 +93,10 @@ public function getRoles(): array return $this->wrappedUser->getRoles(); } + /** + * @deprecated since Symfony 7.3, {@see \Symfony\Component\Security\Core\User\UserInterface::eraseCredentials()} is removed in Symfony 8.0 + */ + #[\Deprecated] public function eraseCredentials(): void { $this->wrappedUser->eraseCredentials(); diff --git a/src/lib/MVC/Symfony/Templating/Twig/Extension/FieldRenderingExtension.php b/src/lib/MVC/Symfony/Templating/Twig/Extension/FieldRenderingExtension.php index bbf234881d..91eab082ed 100644 --- a/src/lib/MVC/Symfony/Templating/Twig/Extension/FieldRenderingExtension.php +++ b/src/lib/MVC/Symfony/Templating/Twig/Extension/FieldRenderingExtension.php @@ -50,7 +50,7 @@ public function __construct( $this->translationHelper = $translationHelper; } - public function getFunctions() + public function getFunctions(): array { $renderFieldCallable = function (Environment $environment, Content|ContentAwareInterface $data, $fieldIdentifier, array $params = []) { $this->fieldBlockRenderer->setTwig($environment); diff --git a/src/lib/QueryType/BuiltIn/AbstractQueryType.php b/src/lib/QueryType/BuiltIn/AbstractQueryType.php index 28cf18b62d..2bed8e63fb 100644 --- a/src/lib/QueryType/BuiltIn/AbstractQueryType.php +++ b/src/lib/QueryType/BuiltIn/AbstractQueryType.php @@ -46,18 +46,19 @@ public function __construct( protected function configureOptions(OptionsResolver $resolver): void { + $resolver->setOptions('filter', static function (OptionsResolver $resolver): void { + $resolver->setDefaults([ + 'content_type' => [], + 'visible_only' => true, + 'siteaccess_aware' => true, + ]); + + $resolver->setAllowedTypes('content_type', 'array'); + $resolver->setAllowedTypes('visible_only', 'bool'); + $resolver->setAllowedTypes('siteaccess_aware', 'bool'); + }); + $resolver->setDefaults([ - 'filter' => static function (OptionsResolver $resolver): void { - $resolver->setDefaults([ - 'content_type' => [], - 'visible_only' => true, - 'siteaccess_aware' => true, - ]); - - $resolver->setAllowedTypes('content_type', 'array'); - $resolver->setAllowedTypes('visible_only', 'bool'); - $resolver->setAllowedTypes('siteaccess_aware', 'bool'); - }, 'offset' => 0, 'limit' => self::DEFAULT_LIMIT, 'sort' => [], diff --git a/src/lib/Repository/Validator/Constraint/LocationIsContainerContentType.php b/src/lib/Repository/Validator/Constraint/LocationIsContainerContentType.php index 5914d107b0..aeda683dad 100644 --- a/src/lib/Repository/Validator/Constraint/LocationIsContainerContentType.php +++ b/src/lib/Repository/Validator/Constraint/LocationIsContainerContentType.php @@ -8,6 +8,7 @@ namespace Ibexa\Core\Repository\Validator\Constraint; +use Symfony\Component\Validator\Attribute\HasNamedArguments; use Symfony\Component\Validator\Constraint; final class LocationIsContainerContentType extends Constraint @@ -21,16 +22,15 @@ final class LocationIsContainerContentType extends Constraint public string $message = 'Location with {{ contentTypeName }} is not a container content type.'; /** - * @param array|null $options * @param array|null $groups */ + #[HasNamedArguments] public function __construct( - ?array $options = null, ?string $message = null, ?array $groups = null, mixed $payload = null ) { - parent::__construct($options ?? [], $groups, $payload); + parent::__construct(null, $groups, $payload); $this->message = $message ?? $this->message; } diff --git a/tests/bundle/Core/DependencyInjection/Compiler/ChainRoutingPassTest.php b/tests/bundle/Core/DependencyInjection/Compiler/ChainRoutingPassTest.php index a97122ffd3..c557e62eb7 100644 --- a/tests/bundle/Core/DependencyInjection/Compiler/ChainRoutingPassTest.php +++ b/tests/bundle/Core/DependencyInjection/Compiler/ChainRoutingPassTest.php @@ -9,8 +9,6 @@ use Ibexa\Bundle\Core\DependencyInjection\Compiler\ChainRoutingPass; use Ibexa\Core\MVC\Symfony\Routing\ChainRouter; -use Ibexa\Core\MVC\Symfony\SiteAccess; -use Ibexa\Core\MVC\Symfony\SiteAccess\Router; use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; @@ -74,9 +72,6 @@ public function testAddRouterWithDefaultRouter($declaredPriority, $expectedPrior { $defaultRouter = new Definition(); $this->setDefinition('router.default', $defaultRouter); - $this->setDefinition(SiteAccess::class, new Definition()); - $this->setDefinition('ibexa.config.resolver', new Definition()); - $this->setDefinition(Router::class, new Definition()); $resolverDef = new Definition(); $serviceId = 'some_service_id'; @@ -90,26 +85,7 @@ public function testAddRouterWithDefaultRouter($declaredPriority, $expectedPrior $this->compile(); // Assertion for default router - $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'router.default', - 'setSiteAccess', - [new Reference(SiteAccess::class)] - ); - $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'router.default', - 'setConfigResolver', - [new Reference('ibexa.config.resolver')] - ); - $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'router.default', - 'setNonSiteAccessAwareRoutes', - ['%ibexa.default_router.non_site_access_aware_routes%'] - ); - $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - 'router.default', - 'setSiteAccessRouter', - [new Reference(Router::class)] - ); + $this->assertContainerBuilderHasServiceDefinitionWithTag('router.default', 'router', ['priority' => 255]); $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( ChainRouter::class, 'add', diff --git a/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php index 29de81a1b6..d30e172a87 100644 --- a/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php +++ b/tests/bundle/Core/EventSubscriber/TrustedHeaderClientIpEventSubscriberTest.php @@ -19,24 +19,15 @@ final class TrustedHeaderClientIpEventSubscriberTest extends TestCase { - private ?string $originalRemoteAddr; + private ?string $originalRemoteAddr = null; private const string PROXY_IP = '127.100.100.1'; private const string REAL_CLIENT_IP = '98.76.123.234'; - /** - * @param array $data - */ - public function __construct(?string $name = null, array $data = [], string $dataName = '') - { - parent::__construct($name, $data, $dataName); - - $this->originalRemoteAddr = $_SERVER['REMOTE_ADDR'] ?? null; - } - protected function setUp(): void { + $this->originalRemoteAddr = $_SERVER['REMOTE_ADDR'] ?? null; $_SERVER['REMOTE_ADDR'] = null; Request::setTrustedProxies([], -1); } diff --git a/tests/bundle/Core/Routing/DefaultRouterTest.php b/tests/bundle/Core/Routing/DefaultRouterTest.php index cd3a45a3a3..c06892e057 100644 --- a/tests/bundle/Core/Routing/DefaultRouterTest.php +++ b/tests/bundle/Core/Routing/DefaultRouterTest.php @@ -4,148 +4,106 @@ * @copyright Copyright (C) Ibexa AS. All rights reserved. * @license For full copyright and license information view LICENSE file distributed with this source code. */ +declare(strict_types=1); namespace Ibexa\Tests\Bundle\Core\Routing; use Ibexa\Bundle\Core\Routing\DefaultRouter; use Ibexa\Bundle\Core\SiteAccess\Matcher; -use Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface; use Ibexa\Core\MVC\Symfony\Routing\SimplifiedRequest; use Ibexa\Core\MVC\Symfony\SiteAccess; +use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; -use ReflectionObject; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; -use Symfony\Component\Routing\Matcher\UrlMatcherInterface; use Symfony\Component\Routing\RequestContext; +use Symfony\Component\Routing\Router; -class DefaultRouterTest extends TestCase +/** + * @covers \Ibexa\Bundle\Core\Routing\DefaultRouter + */ +final class DefaultRouterTest extends TestCase { - /** @var \PHPUnit\Framework\MockObject\MockObject|\Symfony\Component\DependencyInjection\ContainerInterface */ - protected $container; + private const array NON_SITEACCESS_AWARE_ROUTES = ['_dontwantsiteaccess']; + + private Router&MockObject $innerRouter; - /** @var \PHPUnit\Framework\MockObject\MockObject|\Ibexa\Contracts\Core\SiteAccess\ConfigResolverInterface */ - protected $configResolver; + private SiteAccess\SiteAccessRouterInterface&MockObject $siteAccessRouter; - /** @var \Symfony\Component\Routing\RequestContext */ - protected $requestContext; + private RequestContext $requestContext; protected function setUp(): void { parent::setUp(); - $this->container = $this->createMock(ContainerInterface::class); - $this->configResolver = $this->createMock(ConfigResolverInterface::class); + $this->innerRouter = $this->createMock(Router::class); + $this->siteAccessRouter = $this->createMock(SiteAccess\SiteAccessRouterInterface::class); $this->requestContext = new RequestContext(); + $this->innerRouter->method('getContext')->willReturnCallback(fn (): RequestContext => $this->requestContext); } - /** - * @return class-string<\Ibexa\Bundle\Core\Routing\DefaultRouter> - */ - protected function getRouterClass(): string - { - return DefaultRouter::class; - } - - /** - * @param array $mockedMethods - * - * @return \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter - */ - protected function generateRouter(array $mockedMethods = []) + private function createRouter(): DefaultRouter { - /** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */ - $router = $this - ->getMockBuilder($this->getRouterClass()) - ->setConstructorArgs([$this->container, 'foo', [], $this->requestContext]) - ->setMethods(array_merge($mockedMethods)) - ->getMock(); - $router->setConfigResolver($this->configResolver); - - return $router; + return new DefaultRouter($this->innerRouter, $this->siteAccessRouter, self::NON_SITEACCESS_AWARE_ROUTES); } - public function testMatchRequestWithSemanticPathinfo() + public function testMatchRequestWithSemanticPathinfo(): void { $pathinfo = '/siteaccess/foo/bar'; $semanticPathinfo = '/foo/bar'; $request = Request::create($pathinfo); $request->attributes->set('semanticPathinfo', $semanticPathinfo); - - /** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */ - $router = $this->generateRouter(['getMatcher']); $matchedParameters = ['_controller' => 'AcmeBundle:myAction']; - $matcher = $this->createMock(UrlMatcherInterface::class); - $matcher->expects(self::once()) - ->method('match') - ->with($semanticPathinfo) - ->willReturn($matchedParameters); - - $router + $this->innerRouter ->expects(self::once()) - ->method('getMatcher') - ->willReturn($matcher); + ->method('matchRequest') + ->with(self::callback( + static fn (Request $matchedRequest): bool => $matchedRequest->getPathInfo() === $semanticPathinfo + )) + ->willReturn($matchedParameters); - self::assertSame($matchedParameters, $router->matchRequest($request)); + self::assertSame($matchedParameters, $this->createRouter()->matchRequest($request)); + // The original request must not be altered + self::assertSame($pathinfo, $request->getPathInfo()); } - public function testMatchRequestRegularPathinfo() + public function testMatchRequestRegularPathinfo(): void { $matchedParameters = ['_controller' => 'AcmeBundle:myAction']; - $pathinfo = '/siteaccess/foo/bar'; - - $request = Request::create($pathinfo); - - $this->configResolver->expects(self::never())->method('getParameter'); - - /** @var \PHPUnit\Framework\MockObject\MockObject&\Ibexa\Bundle\Core\Routing\DefaultRouter $router */ - $router = $this->generateRouter(['getMatcher']); - - $matcher = $this->createMock(UrlMatcherInterface::class); - $matcher->expects(self::once()) - ->method('match') - ->with($pathinfo) - ->willReturn($matchedParameters); + $request = Request::create('/siteaccess/foo/bar'); - $router + $this->innerRouter ->expects(self::once()) - ->method('getMatcher') - ->willReturn($matcher); + ->method('matchRequest') + ->with(self::identicalTo($request)) + ->willReturn($matchedParameters); - self::assertSame($matchedParameters, $router->matchRequest($request)); + self::assertSame($matchedParameters, $this->createRouter()->matchRequest($request)); } /** * @dataProvider providerGenerateNoSiteAccess */ - public function testGenerateNoSiteAccess($url) + public function testGenerateNoSiteAccess(string $url): void { - $generator = $this->createMock(UrlGeneratorInterface::class); - $generator + $this->innerRouter ->expects(self::once()) ->method('generate') ->with(__METHOD__) ->willReturn($url); - /** @var \Ibexa\Bundle\Core\Routing\DefaultRouter&\PHPUnit\Framework\MockObject\MockObject $router */ - $router = $this->generateRouter(['getGenerator']); - $router - ->expects(self::any()) - ->method('getGenerator') - ->willReturn($generator); - - self::assertSame($url, $router->generate(__METHOD__)); + self::assertSame($url, $this->createRouter()->generate(__METHOD__)); } - public function providerGenerateNoSiteAccess() + /** + * @return iterable + */ + public function providerGenerateNoSiteAccess(): iterable { - return [ - ['/foo/bar'], - ['/foo/bar/baz?truc=muche&tata=toto'], - ['http://ibexa.co/Products/Ibexa-CMS'], - ['http://www.metalfrance.net/decouvertes/edge-caress-inverse-ep'], - ]; + yield ['/foo/bar']; + yield ['/foo/bar/baz?truc=muche&tata=toto']; + yield ['http://ibexa.co/Products/Ibexa-CMS']; + yield ['http://www.metalfrance.net/decouvertes/edge-caress-inverse-ep']; } /** @@ -157,31 +115,28 @@ public function providerGenerateNoSiteAccess() * @param string $saName The SiteAccess name * @param bool $isMatcherLexer True if the siteaccess matcher is URILexer * @param int $referenceType The type of reference to be generated (one of the constants) - * @param string $routeName */ - public function testGenerateWithSiteAccess($urlGenerated, $relevantUri, $expectedUrl, $saName, $isMatcherLexer, $referenceType, $routeName) - { + public function testGenerateWithSiteAccess( + string $urlGenerated, + string $relevantUri, + string $expectedUrl, + string $saName, + bool $isMatcherLexer, + int $referenceType, + ?string $routeName + ): void { $routeName = $routeName ?: __METHOD__; - $nonSiteAccessAwareRoutes = ['_dontwantsiteaccess']; - $generator = $this->createMock(UrlGeneratorInterface::class); - $generator + $this->innerRouter ->expects(self::once()) ->method('generate') ->with($routeName) ->willReturn($urlGenerated); - /** @var \Ibexa\Bundle\Core\Routing\DefaultRouter&\PHPUnit\Framework\MockObject\MockObject $router */ - $router = $this->generateRouter(['getGenerator']); - $router - ->expects(self::any()) - ->method('getGenerator') - ->willReturn($generator); - // If matcher is URILexer, we make it act as it's supposed to, prepending the siteaccess. if ($isMatcherLexer) { $matcher = $this->createMock(SiteAccess\URILexer::class); // Route is siteaccess aware, we're expecting analyseLink() to be called - if (!in_array($routeName, $nonSiteAccessAwareRoutes)) { + if (!in_array($routeName, self::NON_SITEACCESS_AWARE_ROUTES, true)) { $matcher ->expects(self::once()) ->method('analyseLink') @@ -197,133 +152,116 @@ public function testGenerateWithSiteAccess($urlGenerated, $relevantUri, $expecte $matcher = $this->createMock(Matcher::class); } - $sa = new SiteAccess($saName, 'test', $matcher); - $router->setSiteAccess($sa); - - $requestContext = new RequestContext(); $urlComponents = parse_url($urlGenerated); if (isset($urlComponents['host'])) { - $requestContext->setHost($urlComponents['host']); - $requestContext->setScheme($urlComponents['scheme']); + $this->requestContext->setHost($urlComponents['host']); + $this->requestContext->setScheme($urlComponents['scheme']); if (isset($urlComponents['port']) && $urlComponents['scheme'] === 'http') { - $requestContext->setHttpPort($urlComponents['port']); + $this->requestContext->setHttpPort($urlComponents['port']); } elseif (isset($urlComponents['port']) && $urlComponents['scheme'] === 'https') { - $requestContext->setHttpsPort($urlComponents['port']); + $this->requestContext->setHttpsPort($urlComponents['port']); } } - $requestContext->setBaseUrl( + $this->requestContext->setBaseUrl( substr($urlComponents['path'], 0, strpos($urlComponents['path'], $relevantUri)) ); - $router->setContext($requestContext); - $router->setNonSiteAccessAwareRoutes($nonSiteAccessAwareRoutes); + + $router = $this->createRouter(); + $router->setSiteAccess(new SiteAccess($saName, 'test', $matcher)); self::assertSame($expectedUrl, $router->generate($routeName, [], $referenceType)); } - public function providerGenerateWithSiteAccess() + /** + * @return iterable + */ + public function providerGenerateWithSiteAccess(): iterable { - return [ - ['/foo/bar', '/foo/bar', '/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_PATH, null], - ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess'], - ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess'], - ['https://ezpublish.dev/secured', '/secured', 'https://ezpublish.dev/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['https://ezpublish.dev:445/secured', '/secured', 'https://ezpublish.dev:445/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['http://ezpublish.dev:8080/foo/root_folder/bar/baz', '/bar/baz', 'http://ezpublish.dev:8080/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], - ['/foo/bar/baz', '/foo/bar/baz', '/test_siteaccess/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null], - ['/foo/root_folder/bar/baz', '/bar/baz', '/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null], - ['/foo/bar/baz', '/foo/bar/baz', '/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, '_dontwantsiteaccess'], - ]; + yield ['/foo/bar', '/foo/bar', '/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_PATH, null]; + yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess']; + yield ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess']; + yield ['https://ezpublish.dev/secured', '/secured', 'https://ezpublish.dev/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['https://ezpublish.dev:445/secured', '/secured', 'https://ezpublish.dev:445/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['http://ezpublish.dev:8080/foo/root_folder/bar/baz', '/bar/baz', 'http://ezpublish.dev:8080/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; + yield ['/foo/bar/baz', '/foo/bar/baz', '/test_siteaccess/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null]; + yield ['/foo/root_folder/bar/baz', '/bar/baz', '/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null]; + yield ['/foo/bar/baz', '/foo/bar/baz', '/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, '_dontwantsiteaccess']; } - public function testGenerateReverseSiteAccessMatch() + public function testGenerateReverseSiteAccessMatch(): void { $routeName = 'some_route_name'; $urlGenerated = 'http://phoenix-rises.fm/foo/bar'; $siteAccessName = 'foo_test'; - $siteAccessRouter = $this->createMock(SiteAccess\SiteAccessRouterInterface::class); $versatileMatcher = $this->createMock(SiteAccess\VersatileMatcher::class); $simplifiedRequest = new SimplifiedRequest('http', 'phoenix-rises.fm'); $versatileMatcher ->expects(self::once()) ->method('getRequest') ->willReturn($simplifiedRequest); - $siteAccessRouter + $this->siteAccessRouter ->expects(self::once()) ->method('matchByName') ->with($siteAccessName) ->willReturn(new SiteAccess($siteAccessName, 'foo', $versatileMatcher)); - $generator = $this->createMock(UrlGeneratorInterface::class); - $generator - ->expects(self::at(0)) + $contexts = []; + $this->innerRouter + ->expects(self::exactly(2)) ->method('setContext') - ->with(self::isInstanceOf(RequestContext::class)); - $generator - ->expects(self::at(1)) + ->willReturnCallback(static function (RequestContext $context) use (&$contexts): void { + $contexts[] = $context; + }); + $this->innerRouter + ->expects(self::once()) ->method('generate') - ->with($routeName) + ->with($routeName, []) ->willReturn($urlGenerated); - $generator - ->expects(self::at(2)) - ->method('setContext') - ->with($this->requestContext); - $router = new DefaultRouter($this->container, 'foo', [], $this->requestContext); - $router->setConfigResolver($this->configResolver); + $router = $this->createRouter(); $router->setSiteAccess(new SiteAccess('test', 'test', $this->createMock(Matcher::class))); - $router->setSiteAccessRouter($siteAccessRouter); - $refRouter = new ReflectionObject($router); - $refGenerator = $refRouter->getProperty('generator'); - $refGenerator->setAccessible(true); - $refGenerator->setValue($router, $generator); self::assertSame( $urlGenerated, $router->generate($routeName, ['siteaccess' => $siteAccessName], DefaultRouter::ABSOLUTE_PATH) ); + + // Context is switched to the target SiteAccess for generation, then restored + self::assertCount(2, $contexts); + self::assertSame('phoenix-rises.fm', $contexts[0]->getHost()); + self::assertNotSame($this->requestContext, $contexts[0]); + self::assertSame($this->requestContext, $contexts[1]); } /** * @dataProvider providerGetContextBySimplifiedRequest - * - * @param string $uri */ - public function testGetContextBySimplifiedRequest($uri) + public function testGetContextBySimplifiedRequest(string $uri): void { - $this->getExpectedRequestContext($uri); - - $router = new DefaultRouter($this->container, 'foo', [], $this->requestContext); - self::assertEquals( $this->getExpectedRequestContext($uri), - $router->getContextBySimplifiedRequest(SimplifiedRequest::fromUrl($uri)) + $this->createRouter()->getContextBySimplifiedRequest(SimplifiedRequest::fromUrl($uri)) ); } /** - * Data provider for testGetContextBySimplifiedRequest. - * - * @see testGetContextBySimplifiedRequest - * - * @phpstan-return array + * @return iterable */ - public function providerGetContextBySimplifiedRequest() + public function providerGetContextBySimplifiedRequest(): iterable { - return [ - ['/foo/bar'], - ['http://ezpublish.dev/foo/bar'], - ['http://ezpublish.dev:8080/foo/bar'], - ['https://ezpublish.dev/secured'], - ['https://ezpublish.dev:445/secured'], - ['http://ezpublish.dev:8080/foo/root_folder/bar/baz'], - ]; + yield ['/foo/bar']; + yield ['http://ezpublish.dev/foo/bar']; + yield ['http://ezpublish.dev:8080/foo/bar']; + yield ['https://ezpublish.dev/secured']; + yield ['https://ezpublish.dev:445/secured']; + yield ['http://ezpublish.dev:8080/foo/root_folder/bar/baz']; } - private function getExpectedRequestContext($uri) + private function getExpectedRequestContext(string $uri): RequestContext { $requestContext = new RequestContext(); $uriComponents = parse_url($uri); diff --git a/tests/lib/MVC/Symfony/Controller/ControllerTest.php b/tests/lib/MVC/Symfony/Controller/ControllerTest.php index 3bf3bc6b31..35b001520c 100644 --- a/tests/lib/MVC/Symfony/Controller/ControllerTest.php +++ b/tests/lib/MVC/Symfony/Controller/ControllerTest.php @@ -11,7 +11,7 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Response; -use Symfony\Component\Templating\EngineInterface; +use Twig\Environment; /** * @covers \Ibexa\Core\MVC\Symfony\Controller\Controller::render @@ -31,13 +31,13 @@ class ControllerTest extends TestCase protected function setUp(): void { - $this->templateEngineMock = $this->createMock(EngineInterface::class); + $this->templateEngineMock = $this->createMock(Environment::class); $this->containerMock = $this->createMock(ContainerInterface::class); $this->controller = $this->getMockForAbstractClass(Controller::class, [$this->containerMock]); $this->containerMock ->expects(self::any()) ->method('get') - ->with('templating') + ->with('twig') ->will(self::returnValue($this->templateEngineMock)); } diff --git a/tests/lib/MVC/Symfony/Security/Authentication/EventSubscriber/RepositoryUserAuthenticationSubscriberTest.php b/tests/lib/MVC/Symfony/Security/Authentication/EventSubscriber/RepositoryUserAuthenticationSubscriberTest.php index c305df6cbb..8802033e18 100644 --- a/tests/lib/MVC/Symfony/Security/Authentication/EventSubscriber/RepositoryUserAuthenticationSubscriberTest.php +++ b/tests/lib/MVC/Symfony/Security/Authentication/EventSubscriber/RepositoryUserAuthenticationSubscriberTest.php @@ -164,7 +164,8 @@ private function getCheckPassportEvent( $passport = new Passport( new UserBadge( - $user->getUserIdentifier(), + // mocked users return an empty identifier, which Symfony 8 rejects + $user->getUserIdentifier() ?: 'user', static fn (string $userIdentifier): IbexaUserInterface => $userProvider->loadUserByIdentifier($userIdentifier) ), new PasswordCredentials($user->getPassword()) diff --git a/tests/lib/MVC/Symfony/Security/UserTest.php b/tests/lib/MVC/Symfony/Security/UserTest.php index 5561078ba1..94a7efb783 100644 --- a/tests/lib/MVC/Symfony/Security/UserTest.php +++ b/tests/lib/MVC/Symfony/Security/UserTest.php @@ -45,7 +45,6 @@ public function testConstruct() self::assertSame($login, $user->getUsername()); self::assertSame($passwordHash, $user->getPassword()); self::assertSame($roles, $user->getRoles()); - self::assertNull($user->getSalt()); } public function testIsEqualTo() diff --git a/tests/lib/Search/Common/FieldValueMapper/RemoteIdentifierMapperTest.php b/tests/lib/Search/Common/FieldValueMapper/RemoteIdentifierMapperTest.php index b005a84866..b90fe383e5 100644 --- a/tests/lib/Search/Common/FieldValueMapper/RemoteIdentifierMapperTest.php +++ b/tests/lib/Search/Common/FieldValueMapper/RemoteIdentifierMapperTest.php @@ -126,7 +126,7 @@ public function getDataForTestMap(): iterable yield 'identifier with non-printable characters' => [ new Field( 'identifier', - utf8_decode("Non\x09Printable\x0EIdentifier"), + mb_convert_encoding("Non\x09Printable\x0EIdentifier", 'ISO-8859-1', 'UTF-8'), new IdentifierField() ), 'Non PrintableIdentifier', From bf3502e4b33f3b63f5a11f7efc3e21fe6e632e92 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Wed, 9 Sep 2026 11:27:23 +0200 Subject: [PATCH 2/9] IBX-12046: Remove stale phpstan ignore rules for DefaultRouterTest Missing-type ignores no longer match now typed methods, tripping reportUnmatchedIgnoredErrors. --- phpstan-baseline.neon | 66 ------------------------------------------- 1 file changed, 66 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 91af3b7be3..f48f2c1936 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -21408,72 +21408,6 @@ parameters: count: 2 path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:getExpectedRequestContext\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:getExpectedRequestContext\(\) has parameter \$uri with no type specified\.$#' - identifier: missingType.parameter - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:providerGenerateNoSiteAccess\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:providerGenerateWithSiteAccess\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testGenerateNoSiteAccess\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testGenerateNoSiteAccess\(\) has parameter \$url with no type specified\.$#' - identifier: missingType.parameter - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testGenerateReverseSiteAccessMatch\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testGenerateWithSiteAccess\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testGetContextBySimplifiedRequest\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testMatchRequestRegularPathinfo\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - - - message: '#^Method Ibexa\\Tests\\Bundle\\Core\\Routing\\DefaultRouterTest\:\:testMatchRequestWithSemanticPathinfo\(\) has no return type specified\.$#' - identifier: missingType.return - count: 1 - path: tests/bundle/Core/Routing/DefaultRouterTest.php - - message: '#^Offset ''scheme'' might not exist on array\{scheme\?\: string, host\: string, port\: int\<0, 65535\>, user\?\: string, pass\?\: string, path\?\: string, query\?\: string, fragment\?\: string\}\.$#' identifier: offsetAccess.notFound From c77886b63f334c1ef59026577132cac009febe47 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Wed, 9 Sep 2026 13:35:29 +0200 Subject: [PATCH 3/9] IBX-12046: Harden Symfony compatibility changes --- src/bundle/Core/Routing/DefaultRouter.php | 45 +++--- .../Constraint/UniqueIdentifier.php | 85 +++++++++-- .../Controller/Content/QueryController.php | 10 +- .../Controller/QueryRenderController.php | 10 +- src/lib/MVC/Symfony/Security/UserWrapped.php | 8 +- .../bundle/Core/Routing/DefaultRouterTest.php | 144 +++++++++++++----- .../DownloadControllerRequestFlowTest.php | 20 +++ .../MVC/Symfony/InternalRoutingTestKernel.php | 2 + .../Controller/QueryRenderControllerTest.php | 18 +++ .../Constraint/UniqueIdentifierTest.php | 90 +++++++++++ 10 files changed, 348 insertions(+), 84 deletions(-) create mode 100644 tests/lib/Validation/Constraint/UniqueIdentifierTest.php diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php index a9bf267d32..f1074d99bb 100644 --- a/src/bundle/Core/Routing/DefaultRouter.php +++ b/src/bundle/Core/Routing/DefaultRouter.php @@ -17,7 +17,6 @@ use Psr\Log\LoggerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\CacheWarmer\WarmableInterface; -use Symfony\Component\Routing\Exception\RouteNotFoundException; use Symfony\Component\Routing\Matcher\RequestMatcherInterface; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\RouteCollection; @@ -120,36 +119,32 @@ public function generate(string $name, array $parameters = [], int $referenceTyp try { $url = $this->innerRouter->generate($name, $parameters, $referenceType); - } catch (RouteNotFoundException $e) { - // Switch back to original context, for next links generation. - $this->setContext($originalContext); - throw $e; - } - // Now putting back SiteAccess URI if needed. - if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) { - if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { - $scheme = $context->getScheme(); - $port = ''; - if ($scheme === 'http' && $context->getHttpPort() !== 80) { - $port = ':' . $context->getHttpPort(); - } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { - $port = ':' . $context->getHttpsPort(); + // Now putting back SiteAccess URI if needed. + if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) { + if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { + $scheme = $context->getScheme(); + $port = ''; + if ($scheme === 'http' && $context->getHttpPort() !== 80) { + $port = ':' . $context->getHttpPort(); + } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { + $port = ':' . $context->getHttpsPort(); + } + + $base = $context->getHost() . $port . $context->getBaseUrl(); + } else { + $base = $context->getBaseUrl(); } - $base = $context->getHost() . $port . $context->getBaseUrl(); - } else { - $base = $context->getBaseUrl(); + $linkUri = $base ? substr($url, strpos($url, $base) + strlen($base)) : $url; + $url = str_replace($linkUri, $siteAccess->matcher->analyseLink($linkUri), $url); } - $linkUri = $base ? substr($url, strpos($url, $base) + strlen($base)) : $url; - $url = str_replace($linkUri, $siteAccess->matcher->analyseLink($linkUri), $url); + return $url; + } finally { + // Switch back to original context, for next links generation, including when generation fails. + $this->setContext($originalContext); } - - // Switch back to original context, for next links generation. - $this->setContext($originalContext); - - return $url; } /** diff --git a/src/contracts/Validation/Constraint/UniqueIdentifier.php b/src/contracts/Validation/Constraint/UniqueIdentifier.php index 82ea033db0..57cbc91282 100644 --- a/src/contracts/Validation/Constraint/UniqueIdentifier.php +++ b/src/contracts/Validation/Constraint/UniqueIdentifier.php @@ -12,9 +12,21 @@ use JMS\TranslationBundle\Translation\TranslationContainerInterface; use Symfony\Component\Validator\Attribute\HasNamedArguments; use Symfony\Component\Validator\Constraint; +use Symfony\Component\Validator\Exception\InvalidOptionsException; +use Symfony\Component\Validator\Exception\MissingOptionsException; abstract class UniqueIdentifier extends Constraint implements TranslationContainerInterface { + private const array OPTION_NAMES = [ + 'identifierPath', + 'value', + 'existingIdPath', + 'reportErrorPath', + 'message', + 'groups', + 'payload', + ]; + protected const string MESSAGE = 'ibexa.identifier_already_exists'; public string $message = self::MESSAGE; @@ -26,19 +38,36 @@ abstract class UniqueIdentifier extends Constraint implements TranslationContain public ?string $reportErrorPath = null; /** - * @param string|array $identifierPath Property path of the identifier, or (deprecated) an options array + * @param string|array|null $identifierPath Property path of the identifier, or (deprecated) an options array + * @param string|array|null $existingIdPath Property path of the existing identifier, or validation groups when using the legacy positional signature + * @param mixed $reportErrorPath Property path to report validation errors on, or the payload when using the legacy positional signature * @param array|null $groups + * @param array|null $options Deprecated options array */ #[HasNamedArguments] public function __construct( - string|array $identifierPath, - ?string $existingIdPath = null, - ?string $reportErrorPath = null, + string|array|null $identifierPath = null, + string|array|null $existingIdPath = null, + mixed $reportErrorPath = null, ?string $message = null, ?array $groups = null, - mixed $payload = null + mixed $payload = null, + ?array $options = null ) { + // Preserve Constraint's former ($options, $groups, $payload) positional signature. + if (is_array($existingIdPath)) { + $payload ??= $reportErrorPath; + $groups ??= $existingIdPath; + $existingIdPath = null; + $reportErrorPath = null; + } + if (is_array($identifierPath)) { + $options = array_merge($identifierPath, $options ?? []); + $identifierPath = null; + } + + if ($options !== null) { trigger_deprecation( 'ibexa/core', '6.0', @@ -46,17 +75,33 @@ public function __construct( static::class ); - $options = $identifierPath; - $identifierPath = $options['identifierPath'] ?? $options['value'] ?? null; + $this->validateOptionNames($options); + if (array_key_exists('value', $options)) { + $options['identifierPath'] = $options['value']; + } + + $identifierPath ??= $options['identifierPath'] ?? null; $existingIdPath ??= $options['existingIdPath'] ?? null; $reportErrorPath ??= $options['reportErrorPath'] ?? null; $message ??= $options['message'] ?? null; $groups ??= $options['groups'] ?? null; $payload ??= $options['payload'] ?? null; + } - if (!is_string($identifierPath)) { - throw new \InvalidArgumentException(sprintf('The "identifierPath" option of "%s" is required.', static::class)); - } + if ($identifierPath === null) { + throw new MissingOptionsException( + sprintf('The option "identifierPath" must be set for constraint "%s".', static::class), + ['identifierPath'] + ); + } + if (!is_string($identifierPath)) { + throw new \TypeError(sprintf('The "identifierPath" option of "%s" must be a string.', static::class)); + } + if (!is_string($existingIdPath) && $existingIdPath !== null) { + throw new \TypeError(sprintf('The "existingIdPath" option of "%s" must be a string or null.', static::class)); + } + if (!is_string($reportErrorPath) && $reportErrorPath !== null) { + throw new \TypeError(sprintf('The "reportErrorPath" option of "%s" must be a string or null.', static::class)); } parent::__construct(null, $groups, $payload); @@ -67,6 +112,26 @@ public function __construct( $this->message = $message ?? static::MESSAGE; } + /** + * @param array $options + */ + private function validateOptionNames(array $options): void + { + $invalidOptions = array_diff(array_keys($options), self::OPTION_NAMES); + if ($invalidOptions === []) { + return; + } + + throw new InvalidOptionsException( + sprintf( + 'The options "%s" do not exist in constraint "%s".', + implode('", "', $invalidOptions), + static::class + ), + $invalidOptions + ); + } + /** * @return array */ diff --git a/src/lib/MVC/Symfony/Controller/Content/QueryController.php b/src/lib/MVC/Symfony/Controller/Content/QueryController.php index 74efcbe9cb..8fa36f009b 100644 --- a/src/lib/MVC/Symfony/Controller/Content/QueryController.php +++ b/src/lib/MVC/Symfony/Controller/Content/QueryController.php @@ -119,10 +119,12 @@ private function runPagingQuery(ContentView $view, Request $request) $limit = $queryParameters['limit'] ?? 10; $pageParam = $queryParameters['page_param'] ?? 'page'; - $page = $request->attributes->get($pageParam) - ?? $request->query->get($pageParam) - ?? $request->request->get($pageParam) - ?? 1; + $page = match (true) { + $request->attributes->has($pageParam) => $request->attributes->get($pageParam), + $request->query->has($pageParam) => $request->query->all()[$pageParam], + $request->request->has($pageParam) => $request->request->all()[$pageParam], + default => 1, + }; $pager = new Pagerfanta( $this->getAdapter($this->contentViewQueryTypeMapper->map($view)) diff --git a/src/lib/MVC/Symfony/Controller/QueryRenderController.php b/src/lib/MVC/Symfony/Controller/QueryRenderController.php index e2cb9be634..1a79063d55 100644 --- a/src/lib/MVC/Symfony/Controller/QueryRenderController.php +++ b/src/lib/MVC/Symfony/Controller/QueryRenderController.php @@ -49,10 +49,12 @@ public function renderQuery(Request $request, array $options): QueryView $results = new Pagerfanta($this->getAdapter($options)); if ($options['pagination']['enabled']) { $pageParam = $options['pagination']['page_param']; - $currentPage = $request->attributes->get($pageParam) - ?? $request->query->get($pageParam) - ?? $request->request->get($pageParam) - ?? 1; + $currentPage = match (true) { + $request->attributes->has($pageParam) => $request->attributes->get($pageParam), + $request->query->has($pageParam) => $request->query->all()[$pageParam], + $request->request->has($pageParam) => $request->request->all()[$pageParam], + default => 1, + }; $results->setAllowOutOfRangePages(true); $results->setMaxPerPage($options['pagination']['limit']); diff --git a/src/lib/MVC/Symfony/Security/UserWrapped.php b/src/lib/MVC/Symfony/Security/UserWrapped.php index 973a6a58de..af762988ab 100644 --- a/src/lib/MVC/Symfony/Security/UserWrapped.php +++ b/src/lib/MVC/Symfony/Security/UserWrapped.php @@ -99,7 +99,13 @@ public function getRoles(): array #[\Deprecated] public function eraseCredentials(): void { - $this->wrappedUser->eraseCredentials(); + $wrappedUserReflection = new \ReflectionObject($this->wrappedUser); + if ($wrappedUserReflection->hasMethod('eraseCredentials')) { + $eraseCredentials = $wrappedUserReflection->getMethod('eraseCredentials'); + if ($eraseCredentials->isPublic()) { + $eraseCredentials->invoke($this->wrappedUser); + } + } } public function isEqualTo(UserInterface $user): bool diff --git a/tests/bundle/Core/Routing/DefaultRouterTest.php b/tests/bundle/Core/Routing/DefaultRouterTest.php index c06892e057..95df91a08e 100644 --- a/tests/bundle/Core/Routing/DefaultRouterTest.php +++ b/tests/bundle/Core/Routing/DefaultRouterTest.php @@ -14,7 +14,9 @@ use Ibexa\Core\MVC\Symfony\SiteAccess; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; +use Symfony\Bundle\FrameworkBundle\Routing\Router as FrameworkRouter; use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\Routing\Exception\InvalidParameterException; use Symfony\Component\Routing\Generator\UrlGeneratorInterface; use Symfony\Component\Routing\RequestContext; use Symfony\Component\Routing\Router; @@ -100,10 +102,12 @@ public function testGenerateNoSiteAccess(string $url): void */ public function providerGenerateNoSiteAccess(): iterable { - yield ['/foo/bar']; - yield ['/foo/bar/baz?truc=muche&tata=toto']; - yield ['http://ibexa.co/Products/Ibexa-CMS']; - yield ['http://www.metalfrance.net/decouvertes/edge-caress-inverse-ep']; + return [ + ['/foo/bar'], + ['/foo/bar/baz?truc=muche&tata=toto'], + ['http://ibexa.co/Products/Ibexa-CMS'], + ['http://www.metalfrance.net/decouvertes/edge-caress-inverse-ep'], + ]; } /** @@ -177,18 +181,20 @@ public function testGenerateWithSiteAccess( */ public function providerGenerateWithSiteAccess(): iterable { - yield ['/foo/bar', '/foo/bar', '/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_PATH, null]; - yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess']; - yield ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess']; - yield ['https://ezpublish.dev/secured', '/secured', 'https://ezpublish.dev/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['https://ezpublish.dev:445/secured', '/secured', 'https://ezpublish.dev:445/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['http://ezpublish.dev:8080/foo/root_folder/bar/baz', '/bar/baz', 'http://ezpublish.dev:8080/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null]; - yield ['/foo/bar/baz', '/foo/bar/baz', '/test_siteaccess/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null]; - yield ['/foo/root_folder/bar/baz', '/bar/baz', '/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null]; - yield ['/foo/bar/baz', '/foo/bar/baz', '/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, '_dontwantsiteaccess']; + return [ + ['/foo/bar', '/foo/bar', '/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_PATH, null], + ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', false, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['http://ezpublish.dev/foo/bar', '/foo/bar', 'http://ezpublish.dev/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess'], + ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/test_siteaccess/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['http://ezpublish.dev:8080/foo/bar', '/foo/bar', 'http://ezpublish.dev:8080/foo/bar', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, '_dontwantsiteaccess'], + ['https://ezpublish.dev/secured', '/secured', 'https://ezpublish.dev/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['https://ezpublish.dev:445/secured', '/secured', 'https://ezpublish.dev:445/test_siteaccess/secured', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['http://ezpublish.dev:8080/foo/root_folder/bar/baz', '/bar/baz', 'http://ezpublish.dev:8080/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_URL, null], + ['/foo/bar/baz', '/foo/bar/baz', '/test_siteaccess/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null], + ['/foo/root_folder/bar/baz', '/bar/baz', '/foo/root_folder/test_siteaccess/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, null], + ['/foo/bar/baz', '/foo/bar/baz', '/foo/bar/baz', 'test_siteaccess', true, UrlGeneratorInterface::ABSOLUTE_PATH, '_dontwantsiteaccess'], + ]; } public function testGenerateReverseSiteAccessMatch(): void @@ -197,25 +203,12 @@ public function testGenerateReverseSiteAccessMatch(): void $urlGenerated = 'http://phoenix-rises.fm/foo/bar'; $siteAccessName = 'foo_test'; - $versatileMatcher = $this->createMock(SiteAccess\VersatileMatcher::class); - $simplifiedRequest = new SimplifiedRequest('http', 'phoenix-rises.fm'); - $versatileMatcher - ->expects(self::once()) - ->method('getRequest') - ->willReturn($simplifiedRequest); - $this->siteAccessRouter - ->expects(self::once()) - ->method('matchByName') - ->with($siteAccessName) - ->willReturn(new SiteAccess($siteAccessName, 'foo', $versatileMatcher)); - $contexts = []; - $this->innerRouter - ->expects(self::exactly(2)) - ->method('setContext') - ->willReturnCallback(static function (RequestContext $context) use (&$contexts): void { - $contexts[] = $context; - }); + $this->expectReverseSiteAccessMatch( + $siteAccessName, + new SimplifiedRequest('http', 'phoenix-rises.fm'), + $contexts + ); $this->innerRouter ->expects(self::once()) ->method('generate') @@ -237,6 +230,49 @@ public function testGenerateReverseSiteAccessMatch(): void self::assertSame($this->requestContext, $contexts[1]); } + public function testGenerateRestoresContextWhenInnerRouterThrows(): void + { + $siteAccessName = 'foo_test'; + $contexts = []; + $this->expectReverseSiteAccessMatch( + $siteAccessName, + new SimplifiedRequest('https', 'example.com'), + $contexts + ); + $this->innerRouter + ->expects(self::once()) + ->method('generate') + ->willThrowException(new InvalidParameterException()); + + $this->expectException(InvalidParameterException::class); + try { + $this->createRouter()->generate('route', ['siteaccess' => $siteAccessName]); + } finally { + self::assertCount(2, $contexts); + self::assertNotSame($this->requestContext, $contexts[0]); + self::assertSame($this->requestContext, $contexts[1]); + } + } + + public function testWarmUpDelegatesToInnerRouter(): void + { + $innerRouter = $this->createMock(FrameworkRouter::class); + $innerRouter + ->expects(self::once()) + ->method('warmUp') + ->with('/cache', '/build') + ->willReturn(['/cache/routes.php']); + + $router = new DefaultRouter($innerRouter, $this->siteAccessRouter); + + self::assertSame(['/cache/routes.php'], $router->warmUp('/cache', '/build')); + } + + public function testWarmUpDoesNothingWhenInnerRouterIsNotWarmable(): void + { + self::assertSame([], $this->createRouter()->warmUp('/cache', '/build')); + } + /** * @dataProvider providerGetContextBySimplifiedRequest */ @@ -253,12 +289,40 @@ public function testGetContextBySimplifiedRequest(string $uri): void */ public function providerGetContextBySimplifiedRequest(): iterable { - yield ['/foo/bar']; - yield ['http://ezpublish.dev/foo/bar']; - yield ['http://ezpublish.dev:8080/foo/bar']; - yield ['https://ezpublish.dev/secured']; - yield ['https://ezpublish.dev:445/secured']; - yield ['http://ezpublish.dev:8080/foo/root_folder/bar/baz']; + return [ + ['/foo/bar'], + ['http://ezpublish.dev/foo/bar'], + ['http://ezpublish.dev:8080/foo/bar'], + ['https://ezpublish.dev/secured'], + ['https://ezpublish.dev:445/secured'], + ['http://ezpublish.dev:8080/foo/root_folder/bar/baz'], + ]; + } + + /** + * @param \Symfony\Component\Routing\RequestContext[] $contexts + */ + private function expectReverseSiteAccessMatch( + string $siteAccessName, + SimplifiedRequest $simplifiedRequest, + array &$contexts + ): void { + $versatileMatcher = $this->createMock(SiteAccess\VersatileMatcher::class); + $versatileMatcher + ->expects(self::once()) + ->method('getRequest') + ->willReturn($simplifiedRequest); + $this->siteAccessRouter + ->expects(self::once()) + ->method('matchByName') + ->with($siteAccessName) + ->willReturn(new SiteAccess($siteAccessName, 'foo', $versatileMatcher)); + $this->innerRouter + ->expects(self::exactly(2)) + ->method('setContext') + ->willReturnCallback(static function (RequestContext $context) use (&$contexts): void { + $contexts[] = $context; + }); } private function getExpectedRequestContext(string $uri): RequestContext diff --git a/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php b/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php index 1030dfd57b..a33f56ba0b 100644 --- a/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php +++ b/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php @@ -8,11 +8,14 @@ namespace Ibexa\Tests\Integration\Core\MVC\Symfony\Controller\Content; +use Ibexa\Bundle\Core\Routing\DefaultRouter; use Ibexa\Bundle\IO\BinaryStreamResponse; use Ibexa\Contracts\Core\Repository\Values\Content\Field; use Ibexa\Contracts\Core\Test\IbexaKernelTestCase; +use Ibexa\Core\MVC\Symfony\Routing\ChainRouter; use Ibexa\Tests\Core\MVC\Symfony\Controller\Controller\Content\DownloadControllerTestTrait; use Ibexa\Tests\Integration\Core\MVC\Symfony\InternalRoutingTestKernel; +use Symfony\Bundle\FrameworkBundle\Routing\Router as FrameworkRouter; use Symfony\Component\EventDispatcher\EventDispatcher; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpFoundation\RequestStack; @@ -91,6 +94,23 @@ public function testDownloadsFileWithUrlEncodedFilename(): void self::assertInstanceOf(BinaryStreamResponse::class, $response); } + public function testDefaultRouterDecoratesFrameworkRouterAndOccursOnceInChain(): void + { + $defaultRouter = self::getContainer()->get('router.default'); + self::assertInstanceOf(DefaultRouter::class, $defaultRouter); + self::assertInstanceOf(FrameworkRouter::class, $defaultRouter->getInnerRouter()); + + $chainRouter = self::getContainer()->get('test.ibexa.chain_router'); + self::assertInstanceOf(ChainRouter::class, $chainRouter); + self::assertCount( + 1, + array_filter( + $chainRouter->all(), + static fn (object $router): bool => $router === $defaultRouter + ) + ); + } + private function configureDownloadController(RouteCollection $routes): void { $route = $routes->get('ibexa.content.download'); diff --git a/tests/integration/Core/MVC/Symfony/InternalRoutingTestKernel.php b/tests/integration/Core/MVC/Symfony/InternalRoutingTestKernel.php index bf4a4c28fb..9f233f0810 100644 --- a/tests/integration/Core/MVC/Symfony/InternalRoutingTestKernel.php +++ b/tests/integration/Core/MVC/Symfony/InternalRoutingTestKernel.php @@ -9,6 +9,7 @@ namespace Ibexa\Tests\Integration\Core\MVC\Symfony; use Ibexa\Contracts\Core\Test\IbexaTestKernel; +use Ibexa\Core\MVC\Symfony\Routing\ChainRouter; use Symfony\Component\Config\Loader\LoaderInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -20,6 +21,7 @@ public function registerContainerConfiguration(LoaderInterface $loader): void $loader->load(static function (ContainerBuilder $container): void { self::loadRouting($container); + $container->setAlias('test.ibexa.chain_router', ChainRouter::class)->setPublic(true); }); } diff --git a/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php b/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php index eb3ee41bbf..cdb49bba87 100644 --- a/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php +++ b/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php @@ -111,6 +111,24 @@ public function testRenderQueryWithAllOptions(): void ); } + public function testPaginationUsesRequestAttributeBeforeQueryAndRequestParameters(): void + { + $adapter = $this->configureMocks(self::ALL_OPTIONS); + + $items = new Pagerfanta($adapter); + $items->setAllowOutOfRangePages(true); + $items->setCurrentPage(4); + $items->setMaxPerPage(self::EXAMPLE_MAX_PER_PAGE); + + $this->assertRenderQueryResult( + new QueryView('example.html.twig', [ + 'results' => $items, + ]), + self::ALL_OPTIONS, + new Request(['p' => 2], ['p' => 3], ['p' => 4]) + ); + } + /** * @phpstan-param TOptionsArray $options * diff --git a/tests/lib/Validation/Constraint/UniqueIdentifierTest.php b/tests/lib/Validation/Constraint/UniqueIdentifierTest.php new file mode 100644 index 0000000000..2db4db96e2 --- /dev/null +++ b/tests/lib/Validation/Constraint/UniqueIdentifierTest.php @@ -0,0 +1,90 @@ +identifierPath); + self::assertSame('id', $constraint->existingIdPath); + self::assertSame('identifier', $constraint->reportErrorPath); + self::assertSame('Already exists', $constraint->message); + self::assertSame(['custom'], $constraint->groups); + self::assertSame($payload, $constraint->payload); + } + + public function testLegacyNamedOptions(): void + { + $constraint = new class(options: [ + 'value' => 'identifier', + 'existingIdPath' => 'id', + 'reportErrorPath' => 'identifier', + ]) extends UniqueIdentifier { + }; + + self::assertSame('identifier', $constraint->identifierPath); + self::assertSame('id', $constraint->existingIdPath); + self::assertSame('identifier', $constraint->reportErrorPath); + } + + public function testLegacyPositionalSignature(): void + { + $payload = new \stdClass(); + $constraint = new class( + ['identifierPath' => 'identifier'], + ['custom'], + $payload + ) extends UniqueIdentifier { + }; + + self::assertSame('identifier', $constraint->identifierPath); + self::assertSame(['custom'], $constraint->groups); + self::assertSame($payload, $constraint->payload); + } + + public function testRejectsUnknownLegacyOption(): void + { + $this->expectException(InvalidOptionsException::class); + + $constraint = new class(['identifierPath' => 'identifier', 'identiferPath' => 'typo']) extends UniqueIdentifier { + }; + + self::fail(sprintf('Expected exception was not thrown while constructing %s.', $constraint::class)); + } + + public function testRequiresIdentifierPath(): void + { + $this->expectException(MissingOptionsException::class); + + $constraint = new class() extends UniqueIdentifier { + }; + + self::fail(sprintf('Expected exception was not thrown while constructing %s.', $constraint::class)); + } +} From 33792bf20ed6a0677a7b84eab06856530bed28bf Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 09:18:08 +0200 Subject: [PATCH 4/9] IBX-12046: Dropped the options-array constructor path from UniqueIdentifier 6.0 is a major and Symfony 8 removes options-array support from Constraint itself, so the contract now only accepts named/positional arguments. The three subclasses in the org (discounts, segmentation) are final and their production usages go through mapping loaders or attributes, which already pass named arguments; only test call sites changed (ibexa/discounts#361, ibexa/segmentation#225). --- .../Constraint/UniqueIdentifier.php | 98 ++----------------- .../Constraint/UniqueIdentifierTest.php | 56 +++-------- 2 files changed, 22 insertions(+), 132 deletions(-) diff --git a/src/contracts/Validation/Constraint/UniqueIdentifier.php b/src/contracts/Validation/Constraint/UniqueIdentifier.php index 57cbc91282..7cf88a0c7b 100644 --- a/src/contracts/Validation/Constraint/UniqueIdentifier.php +++ b/src/contracts/Validation/Constraint/UniqueIdentifier.php @@ -12,21 +12,9 @@ use JMS\TranslationBundle\Translation\TranslationContainerInterface; use Symfony\Component\Validator\Attribute\HasNamedArguments; use Symfony\Component\Validator\Constraint; -use Symfony\Component\Validator\Exception\InvalidOptionsException; -use Symfony\Component\Validator\Exception\MissingOptionsException; abstract class UniqueIdentifier extends Constraint implements TranslationContainerInterface { - private const array OPTION_NAMES = [ - 'identifierPath', - 'value', - 'existingIdPath', - 'reportErrorPath', - 'message', - 'groups', - 'payload', - ]; - protected const string MESSAGE = 'ibexa.identifier_already_exists'; public string $message = self::MESSAGE; @@ -38,72 +26,20 @@ abstract class UniqueIdentifier extends Constraint implements TranslationContain public ?string $reportErrorPath = null; /** - * @param string|array|null $identifierPath Property path of the identifier, or (deprecated) an options array - * @param string|array|null $existingIdPath Property path of the existing identifier, or validation groups when using the legacy positional signature - * @param mixed $reportErrorPath Property path to report validation errors on, or the payload when using the legacy positional signature + * @param string $identifierPath Property path of the identifier to check for uniqueness + * @param string|null $existingIdPath Property path of the ID of the object being updated, so it does not collide with itself + * @param string|null $reportErrorPath Property path to report the violation on (defaults to $identifierPath) * @param array|null $groups - * @param array|null $options Deprecated options array */ #[HasNamedArguments] public function __construct( - string|array|null $identifierPath = null, - string|array|null $existingIdPath = null, - mixed $reportErrorPath = null, + string $identifierPath, + ?string $existingIdPath = null, + ?string $reportErrorPath = null, ?string $message = null, ?array $groups = null, - mixed $payload = null, - ?array $options = null + mixed $payload = null ) { - // Preserve Constraint's former ($options, $groups, $payload) positional signature. - if (is_array($existingIdPath)) { - $payload ??= $reportErrorPath; - $groups ??= $existingIdPath; - $existingIdPath = null; - $reportErrorPath = null; - } - - if (is_array($identifierPath)) { - $options = array_merge($identifierPath, $options ?? []); - $identifierPath = null; - } - - if ($options !== null) { - trigger_deprecation( - 'ibexa/core', - '6.0', - 'Passing an options array to "%s" is deprecated, use named arguments instead.', - static::class - ); - - $this->validateOptionNames($options); - if (array_key_exists('value', $options)) { - $options['identifierPath'] = $options['value']; - } - - $identifierPath ??= $options['identifierPath'] ?? null; - $existingIdPath ??= $options['existingIdPath'] ?? null; - $reportErrorPath ??= $options['reportErrorPath'] ?? null; - $message ??= $options['message'] ?? null; - $groups ??= $options['groups'] ?? null; - $payload ??= $options['payload'] ?? null; - } - - if ($identifierPath === null) { - throw new MissingOptionsException( - sprintf('The option "identifierPath" must be set for constraint "%s".', static::class), - ['identifierPath'] - ); - } - if (!is_string($identifierPath)) { - throw new \TypeError(sprintf('The "identifierPath" option of "%s" must be a string.', static::class)); - } - if (!is_string($existingIdPath) && $existingIdPath !== null) { - throw new \TypeError(sprintf('The "existingIdPath" option of "%s" must be a string or null.', static::class)); - } - if (!is_string($reportErrorPath) && $reportErrorPath !== null) { - throw new \TypeError(sprintf('The "reportErrorPath" option of "%s" must be a string or null.', static::class)); - } - parent::__construct(null, $groups, $payload); $this->identifierPath = $identifierPath; @@ -112,26 +48,6 @@ public function __construct( $this->message = $message ?? static::MESSAGE; } - /** - * @param array $options - */ - private function validateOptionNames(array $options): void - { - $invalidOptions = array_diff(array_keys($options), self::OPTION_NAMES); - if ($invalidOptions === []) { - return; - } - - throw new InvalidOptionsException( - sprintf( - 'The options "%s" do not exist in constraint "%s".', - implode('", "', $invalidOptions), - static::class - ), - $invalidOptions - ); - } - /** * @return array */ diff --git a/tests/lib/Validation/Constraint/UniqueIdentifierTest.php b/tests/lib/Validation/Constraint/UniqueIdentifierTest.php index 2db4db96e2..5346c60bfe 100644 --- a/tests/lib/Validation/Constraint/UniqueIdentifierTest.php +++ b/tests/lib/Validation/Constraint/UniqueIdentifierTest.php @@ -10,8 +10,7 @@ use Ibexa\Contracts\Core\Validation\Constraint\UniqueIdentifier; use PHPUnit\Framework\TestCase; -use Symfony\Component\Validator\Exception\InvalidOptionsException; -use Symfony\Component\Validator\Exception\MissingOptionsException; +use Symfony\Component\Validator\Attribute\HasNamedArguments; /** * @covers \Ibexa\Contracts\Core\Validation\Constraint\UniqueIdentifier @@ -39,52 +38,27 @@ public function testNamedArguments(): void self::assertSame($payload, $constraint->payload); } - public function testLegacyNamedOptions(): void + public function testIdentifierPathIsTheOnlyRequiredArgument(): void { - $constraint = new class(options: [ - 'value' => 'identifier', - 'existingIdPath' => 'id', - 'reportErrorPath' => 'identifier', - ]) extends UniqueIdentifier { + $constraint = new class('identifier') extends UniqueIdentifier { }; self::assertSame('identifier', $constraint->identifierPath); - self::assertSame('id', $constraint->existingIdPath); - self::assertSame('identifier', $constraint->reportErrorPath); + self::assertNull($constraint->existingIdPath); + self::assertNull($constraint->reportErrorPath); + self::assertSame('ibexa.identifier_already_exists', $constraint->message); + self::assertSame([UniqueIdentifier::DEFAULT_GROUP], $constraint->groups); + self::assertNull($constraint->payload); + self::assertSame([UniqueIdentifier::CLASS_CONSTRAINT], $constraint->getTargets()); } - public function testLegacyPositionalSignature(): void + public function testConstructorSupportsNamedArgumentsForMappingLoaders(): void { - $payload = new \stdClass(); - $constraint = new class( - ['identifierPath' => 'identifier'], - ['custom'], - $payload - ) extends UniqueIdentifier { - }; - - self::assertSame('identifier', $constraint->identifierPath); - self::assertSame(['custom'], $constraint->groups); - self::assertSame($payload, $constraint->payload); - } - - public function testRejectsUnknownLegacyOption(): void - { - $this->expectException(InvalidOptionsException::class); - - $constraint = new class(['identifierPath' => 'identifier', 'identiferPath' => 'typo']) extends UniqueIdentifier { - }; - - self::fail(sprintf('Expected exception was not thrown while constructing %s.', $constraint::class)); - } - - public function testRequiresIdentifierPath(): void - { - $this->expectException(MissingOptionsException::class); - - $constraint = new class() extends UniqueIdentifier { - }; + $constructor = new \ReflectionMethod(UniqueIdentifier::class, '__construct'); - self::fail(sprintf('Expected exception was not thrown while constructing %s.', $constraint::class)); + self::assertNotEmpty( + $constructor->getAttributes(HasNamedArguments::class), + 'Symfony mapping loaders (YAML/XML/attributes) rely on #[HasNamedArguments] to pass options as named arguments.' + ); } } From 67bb274a18d1b0d24c4b2296cf71386c119e0f26 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 11:01:08 +0200 Subject: [PATCH 5/9] IBX-12046: Made DefaultRouter::getContextBySimplifiedRequest() private It is an implementation detail of generate(); the reusable logic lives in the internal RequestContextFactory, which has its own test, and nothing in the organization calls the router method. --- phpstan-baseline.neon | 4 +- src/bundle/Core/Routing/DefaultRouter.php | 5 +- .../bundle/Core/Routing/DefaultRouterTest.php | 46 ------------------- 3 files changed, 5 insertions(+), 50 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index f48f2c1936..4e72c2e967 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -21411,13 +21411,13 @@ parameters: - message: '#^Offset ''scheme'' might not exist on array\{scheme\?\: string, host\: string, port\: int\<0, 65535\>, user\?\: string, pass\?\: string, path\?\: string, query\?\: string, fragment\?\: string\}\.$#' identifier: offsetAccess.notFound - count: 2 + count: 1 path: tests/bundle/Core/Routing/DefaultRouterTest.php - message: '#^Offset ''scheme'' might not exist on array\{scheme\?\: string, host\: string, port\?\: int\<0, 65535\>, user\?\: string, pass\?\: string, path\?\: string, query\?\: string, fragment\?\: string\}\.$#' identifier: offsetAccess.notFound - count: 2 + count: 1 path: tests/bundle/Core/Routing/DefaultRouterTest.php - diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php index f1074d99bb..d2661e91c7 100644 --- a/src/bundle/Core/Routing/DefaultRouter.php +++ b/src/bundle/Core/Routing/DefaultRouter.php @@ -162,9 +162,10 @@ public function warmUp(string $cacheDir, ?string $buildDir = null): array /** * Merges context from $simplifiedRequest into a clone of the current context. */ - public function getContextBySimplifiedRequest(SimplifiedRequest $simplifiedRequest): RequestContext + private function getContextBySimplifiedRequest(SimplifiedRequest $simplifiedRequest): RequestContext { - // inline-instantiated on purpose as it's lightweight and injecting it here through DI can be complicated + // Instantiated per call on purpose: the factory clones the current context and mutates that clone, + // so it is per-call state and cannot be a shared service. return (new RequestContextFactory($this->getContext()))->getContextBySimplifiedRequest($simplifiedRequest); } diff --git a/tests/bundle/Core/Routing/DefaultRouterTest.php b/tests/bundle/Core/Routing/DefaultRouterTest.php index 95df91a08e..d39a98301a 100644 --- a/tests/bundle/Core/Routing/DefaultRouterTest.php +++ b/tests/bundle/Core/Routing/DefaultRouterTest.php @@ -273,32 +273,6 @@ public function testWarmUpDoesNothingWhenInnerRouterIsNotWarmable(): void self::assertSame([], $this->createRouter()->warmUp('/cache', '/build')); } - /** - * @dataProvider providerGetContextBySimplifiedRequest - */ - public function testGetContextBySimplifiedRequest(string $uri): void - { - self::assertEquals( - $this->getExpectedRequestContext($uri), - $this->createRouter()->getContextBySimplifiedRequest(SimplifiedRequest::fromUrl($uri)) - ); - } - - /** - * @return iterable - */ - public function providerGetContextBySimplifiedRequest(): iterable - { - return [ - ['/foo/bar'], - ['http://ezpublish.dev/foo/bar'], - ['http://ezpublish.dev:8080/foo/bar'], - ['https://ezpublish.dev/secured'], - ['https://ezpublish.dev:445/secured'], - ['http://ezpublish.dev:8080/foo/root_folder/bar/baz'], - ]; - } - /** * @param \Symfony\Component\Routing\RequestContext[] $contexts */ @@ -324,24 +298,4 @@ private function expectReverseSiteAccessMatch( $contexts[] = $context; }); } - - private function getExpectedRequestContext(string $uri): RequestContext - { - $requestContext = new RequestContext(); - $uriComponents = parse_url($uri); - if (isset($uriComponents['host'])) { - $requestContext->setHost($uriComponents['host']); - $requestContext->setScheme($uriComponents['scheme']); - if (isset($uriComponents['port']) && $uriComponents['scheme'] === 'http') { - $requestContext->setHttpPort($uriComponents['port']); - } elseif (isset($uriComponents['port']) && $uriComponents['scheme'] === 'https') { - $requestContext->setHttpsPort($uriComponents['port']); - } - } - if (isset($uriComponents['path'])) { - $requestContext->setPathInfo($uriComponents['path']); - } - - return $requestContext; - } } From 407edbc58010d88edeacadc11d2d0c193c67e528 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 11:21:06 +0200 Subject: [PATCH 6/9] IBX-12046: Read the pagination page from the query string only Both query controllers receive the page number as a query-string parameter (main-request content view, or the fragment's own URI); request attributes and the POST body were only consulted because Request::get() did so generically. InputBag::getInt() also turns garbage values into a 400 instead of a Pagerfanta exception. --- src/lib/MVC/Symfony/Controller/Content/QueryController.php | 7 +------ src/lib/MVC/Symfony/Controller/QueryRenderController.php | 7 +------ .../MVC/Symfony/Controller/QueryRenderControllerTest.php | 5 +++-- 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/src/lib/MVC/Symfony/Controller/Content/QueryController.php b/src/lib/MVC/Symfony/Controller/Content/QueryController.php index 8fa36f009b..d800be5aa9 100644 --- a/src/lib/MVC/Symfony/Controller/Content/QueryController.php +++ b/src/lib/MVC/Symfony/Controller/Content/QueryController.php @@ -119,12 +119,7 @@ private function runPagingQuery(ContentView $view, Request $request) $limit = $queryParameters['limit'] ?? 10; $pageParam = $queryParameters['page_param'] ?? 'page'; - $page = match (true) { - $request->attributes->has($pageParam) => $request->attributes->get($pageParam), - $request->query->has($pageParam) => $request->query->all()[$pageParam], - $request->request->has($pageParam) => $request->request->all()[$pageParam], - default => 1, - }; + $page = $request->query->getInt($pageParam, 1); $pager = new Pagerfanta( $this->getAdapter($this->contentViewQueryTypeMapper->map($view)) diff --git a/src/lib/MVC/Symfony/Controller/QueryRenderController.php b/src/lib/MVC/Symfony/Controller/QueryRenderController.php index 1a79063d55..e8e888ab98 100644 --- a/src/lib/MVC/Symfony/Controller/QueryRenderController.php +++ b/src/lib/MVC/Symfony/Controller/QueryRenderController.php @@ -49,12 +49,7 @@ public function renderQuery(Request $request, array $options): QueryView $results = new Pagerfanta($this->getAdapter($options)); if ($options['pagination']['enabled']) { $pageParam = $options['pagination']['page_param']; - $currentPage = match (true) { - $request->attributes->has($pageParam) => $request->attributes->get($pageParam), - $request->query->has($pageParam) => $request->query->all()[$pageParam], - $request->request->has($pageParam) => $request->request->all()[$pageParam], - default => 1, - }; + $currentPage = $request->query->getInt($pageParam, 1); $results->setAllowOutOfRangePages(true); $results->setMaxPerPage($options['pagination']['limit']); diff --git a/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php b/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php index cdb49bba87..1343336d1c 100644 --- a/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php +++ b/tests/lib/MVC/Symfony/Controller/QueryRenderControllerTest.php @@ -111,15 +111,16 @@ public function testRenderQueryWithAllOptions(): void ); } - public function testPaginationUsesRequestAttributeBeforeQueryAndRequestParameters(): void + public function testPaginationReadsPageFromQueryStringOnly(): void { $adapter = $this->configureMocks(self::ALL_OPTIONS); $items = new Pagerfanta($adapter); $items->setAllowOutOfRangePages(true); - $items->setCurrentPage(4); + $items->setCurrentPage(2); $items->setMaxPerPage(self::EXAMPLE_MAX_PER_PAGE); + // request body and attributes carrying the same parameter must not influence pagination $this->assertRenderQueryResult( new QueryView('example.html.twig', [ 'results' => $items, From 1377c769c9cfaaeac79c9b68c20ec1ac879812b7 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 13:38:12 +0200 Subject: [PATCH 7/9] IBX-12046: Dropped redundant isPublic() check in UserWrapped::eraseCredentials() --- src/lib/MVC/Symfony/Security/UserWrapped.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/lib/MVC/Symfony/Security/UserWrapped.php b/src/lib/MVC/Symfony/Security/UserWrapped.php index af762988ab..86a96c6971 100644 --- a/src/lib/MVC/Symfony/Security/UserWrapped.php +++ b/src/lib/MVC/Symfony/Security/UserWrapped.php @@ -101,10 +101,7 @@ public function eraseCredentials(): void { $wrappedUserReflection = new \ReflectionObject($this->wrappedUser); if ($wrappedUserReflection->hasMethod('eraseCredentials')) { - $eraseCredentials = $wrappedUserReflection->getMethod('eraseCredentials'); - if ($eraseCredentials->isPublic()) { - $eraseCredentials->invoke($this->wrappedUser); - } + $wrappedUserReflection->getMethod('eraseCredentials')->invoke($this->wrappedUser); } } From 23b2fc8f4ef95a2f0adb5ba638bd42866423f6bd Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 13:38:25 +0200 Subject: [PATCH 8/9] IBX-12046: Extracted SiteAccess URI prepending in DefaultRouter::generate() into a helper method --- src/bundle/Core/Routing/DefaultRouter.php | 38 +++++++++++++---------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php index d2661e91c7..d38a3a718e 100644 --- a/src/bundle/Core/Routing/DefaultRouter.php +++ b/src/bundle/Core/Routing/DefaultRouter.php @@ -122,22 +122,7 @@ public function generate(string $name, array $parameters = [], int $referenceTyp // Now putting back SiteAccess URI if needed. if ($isSiteAccessAware && $siteAccess !== null && $siteAccess->matcher instanceof URILexer) { - if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { - $scheme = $context->getScheme(); - $port = ''; - if ($scheme === 'http' && $context->getHttpPort() !== 80) { - $port = ':' . $context->getHttpPort(); - } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { - $port = ':' . $context->getHttpsPort(); - } - - $base = $context->getHost() . $port . $context->getBaseUrl(); - } else { - $base = $context->getBaseUrl(); - } - - $linkUri = $base ? substr($url, strpos($url, $base) + strlen($base)) : $url; - $url = str_replace($linkUri, $siteAccess->matcher->analyseLink($linkUri), $url); + $url = $this->prependSiteAccessUri($url, $context, $referenceType, $siteAccess->matcher); } return $url; @@ -147,6 +132,27 @@ public function generate(string $name, array $parameters = [], int $referenceTyp } } + private function prependSiteAccessUri(string $url, RequestContext $context, int $referenceType, URILexer $matcher): string + { + if ($referenceType === self::ABSOLUTE_URL || $referenceType === self::NETWORK_PATH) { + $scheme = $context->getScheme(); + $port = ''; + if ($scheme === 'http' && $context->getHttpPort() !== 80) { + $port = ':' . $context->getHttpPort(); + } elseif ($scheme === 'https' && $context->getHttpsPort() !== 443) { + $port = ':' . $context->getHttpsPort(); + } + + $base = $context->getHost() . $port . $context->getBaseUrl(); + } else { + $base = $context->getBaseUrl(); + } + + $linkUri = $base ? substr($url, strpos($url, $base) + strlen($base)) : $url; + + return str_replace($linkUri, $matcher->analyseLink($linkUri), $url); + } + /** * @return string[] */ From cc41e4022b5dd43500136b4d54504c3ad27703b6 Mon Sep 17 00:00:00 2001 From: Dawid Parafinski Date: Thu, 10 Sep 2026 13:38:32 +0200 Subject: [PATCH 9/9] IBX-12046: Dropped DefaultRouter::getInnerRouter(), test fetches inner router from container --- src/bundle/Core/Routing/DefaultRouter.php | 5 ----- .../Controller/Content/DownloadControllerRequestFlowTest.php | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/bundle/Core/Routing/DefaultRouter.php b/src/bundle/Core/Routing/DefaultRouter.php index d38a3a718e..da3000e2e3 100644 --- a/src/bundle/Core/Routing/DefaultRouter.php +++ b/src/bundle/Core/Routing/DefaultRouter.php @@ -49,11 +49,6 @@ public function setSiteAccess(?SiteAccess $siteAccess = null): void $this->siteAccess = $siteAccess; } - public function getInnerRouter(): RouterInterface&RequestMatcherInterface - { - return $this->innerRouter; - } - public function setContext(RequestContext $context): void { $this->innerRouter->setContext($context); diff --git a/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php b/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php index a33f56ba0b..f7acd3ee21 100644 --- a/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php +++ b/tests/integration/Core/MVC/Symfony/Controller/Content/DownloadControllerRequestFlowTest.php @@ -98,7 +98,7 @@ public function testDefaultRouterDecoratesFrameworkRouterAndOccursOnceInChain(): { $defaultRouter = self::getContainer()->get('router.default'); self::assertInstanceOf(DefaultRouter::class, $defaultRouter); - self::assertInstanceOf(FrameworkRouter::class, $defaultRouter->getInnerRouter()); + self::assertInstanceOf(FrameworkRouter::class, self::getContainer()->get('ibexa.routing.default_router.inner')); $chainRouter = self::getContainer()->get('test.ibexa.chain_router'); self::assertInstanceOf(ChainRouter::class, $chainRouter);