From 2c8459dfa40ba01a786befef7eb253f6674261e5 Mon Sep 17 00:00:00 2001 From: Andrew Longosz Date: Mon, 5 Feb 2024 09:40:15 +0100 Subject: [PATCH] IBX-6906: [DX] Introduced identifier-based view matchers (#322) For more details see https://issues.ibexa.co/browse/IBX-6906 and https://github.com/ibexa/core/pull/322 Key changes: * Dropped ViewMatcherRegistryPass in favor of tagged iterator Symfony DIC * Refactored ViewMatcherRegistry to rely on an injectable list of matchers * Refactored ServiceAwareMatcherFactory for identifier-based view matchers * Extracted ViewMatcherRegistryInterface * [Tests] Added coverage for ServiceAwareMatcherFactory * [PHPStan] Aligned baseline with the changes --- phpstan-baseline.neon | 20 ---- .../Compiler/ViewMatcherRegistryPass.php | 43 ------- src/bundle/Core/IbexaCoreBundle.php | 2 - .../Matcher/ServiceAwareMatcherFactory.php | 24 ++-- .../Core/Matcher/ViewMatcherRegistry.php | 20 +++- .../Core/Resources/config/templating.yml | 17 ++- .../MVC/View/ViewMatcherRegistryInterface.php | 20 ++++ .../Matcher/ClassNameMatcherFactory.php | 6 +- .../Compiler/ViewMatcherRegistryPassTest.php | 52 --------- .../ServiceAwareMatcherFactoryTest.php | 110 ++++++++++++++++++ .../Core/Matcher/ViewMatcherRegistryTest.php | 28 +++-- 11 files changed, 191 insertions(+), 151 deletions(-) delete mode 100644 src/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPass.php create mode 100644 src/contracts/MVC/View/ViewMatcherRegistryInterface.php delete mode 100644 tests/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPassTest.php create mode 100644 tests/bundle/Core/Matcher/ServiceAwareMatcherFactoryTest.php diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 1a7a08d65b..7951a805de 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -3825,16 +3825,6 @@ parameters: count: 1 path: src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php - - - message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Matcher\\\\ServiceAwareMatcherFactory\\:\\:__construct\\(\\) has parameter \\$relativeNamespace with no type specified\\.$#" - count: 1 - path: src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php - - - - message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Matcher\\\\ServiceAwareMatcherFactory\\:\\:getMatcher\\(\\) should return Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Matcher\\\\ContentBased\\\\MatcherInterface but returns Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Matcher\\\\ViewMatcherInterface\\.$#" - count: 2 - path: src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php - - message: "#^Method Ibexa\\\\Bundle\\\\Core\\\\Routing\\\\DefaultRouter\\:\\:generate\\(\\) has parameter \\$name with no type specified\\.$#" count: 1 @@ -11890,11 +11880,6 @@ parameters: count: 1 path: src/lib/MVC/Symfony/Matcher/ClassNameMatcherFactory.php - - - message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Matcher\\\\ClassNameMatcherFactory\\:\\:__construct\\(\\) has parameter \\$relativeNamespace with no type specified\\.$#" - count: 1 - path: src/lib/MVC/Symfony/Matcher/ClassNameMatcherFactory.php - - message: "#^Method Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Matcher\\\\ClassNameMatcherFactory\\:\\:getMatcher\\(\\) should return Ibexa\\\\Core\\\\MVC\\\\Symfony\\\\Matcher\\\\ViewMatcherInterface but returns object\\.$#" count: 1 @@ -26255,11 +26240,6 @@ parameters: count: 1 path: tests/bundle/Core/Imagine/VariationPurger/LegacyStorageImageFileListTest.php - - - message: "#^Parameter \\#1 \\$matchers of class Ibexa\\\\Bundle\\\\Core\\\\Matcher\\\\ViewMatcherRegistry constructor expects array\\, array\\ given\\.$#" - count: 1 - path: tests/bundle/Core/Matcher/ViewMatcherRegistryTest.php - - message: "#^Cannot access offset 'path' on array\\{scheme\\?\\: string, host\\?\\: string, port\\?\\: int\\<0, 65535\\>, user\\?\\: string, pass\\?\\: string, path\\?\\: string, query\\?\\: string, fragment\\?\\: string\\}\\|false\\.$#" count: 2 diff --git a/src/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPass.php b/src/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPass.php deleted file mode 100644 index f5bde835ba..0000000000 --- a/src/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPass.php +++ /dev/null @@ -1,43 +0,0 @@ -hasDefinition(ViewMatcherRegistry::class)) { - return; - } - - $matcherServiceRegistry = $container->getDefinition(ViewMatcherRegistry::class); - - foreach ($container->findTaggedServiceIds(self::MATCHER_TAG) as $id => $attributes) { - $matcherServiceRegistry->addMethodCall( - 'setMatcher', - [ - $id, - new Reference($id), - ] - ); - } - } -} - -class_alias(ViewMatcherRegistryPass::class, 'eZ\Bundle\EzPublishCoreBundle\DependencyInjection\Compiler\ViewMatcherRegistryPass'); diff --git a/src/bundle/Core/IbexaCoreBundle.php b/src/bundle/Core/IbexaCoreBundle.php index e52094cd08..be9b0ab962 100644 --- a/src/bundle/Core/IbexaCoreBundle.php +++ b/src/bundle/Core/IbexaCoreBundle.php @@ -31,7 +31,6 @@ use Ibexa\Bundle\Core\DependencyInjection\Compiler\StorageConnectionPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\TranslationCollectorPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\URLHandlerPass; -use Ibexa\Bundle\Core\DependencyInjection\Compiler\ViewMatcherRegistryPass; use Ibexa\Bundle\Core\DependencyInjection\Compiler\ViewProvidersPass; use Ibexa\Bundle\Core\DependencyInjection\Configuration\ComplexSettings\ComplexSettingParser; use Ibexa\Bundle\Core\DependencyInjection\Configuration\Parser as ConfigParser; @@ -78,7 +77,6 @@ public function build(ContainerBuilder $container) $container->addCompilerPass(new PlaceholderProviderPass()); $container->addCompilerPass(new NotificationRendererPass()); $container->addCompilerPass(new ConsoleCacheWarmupPass()); - $container->addCompilerPass(new ViewMatcherRegistryPass()); $container->addCompilerPass(new SiteAccessMatcherRegistryPass()); $container->addCompilerPass(new ConsoleCommandPass()); $container->addCompilerPass(new LazyDoctrineRepositoriesPass(), PassConfig::TYPE_BEFORE_REMOVING); diff --git a/src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php b/src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php index 6869994dab..8c6d749f5e 100644 --- a/src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php +++ b/src/bundle/Core/Matcher/ServiceAwareMatcherFactory.php @@ -6,24 +6,26 @@ */ namespace Ibexa\Bundle\Core\Matcher; +use Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface; use Ibexa\Contracts\Core\Repository\Repository; use Ibexa\Core\MVC\Symfony\Matcher\ClassNameMatcherFactory; +use Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface; /** * A view matcher factory that also accepts services as matchers. * * If a service id is passed as the MatcherIdentifier, this service will be used for the matching. - * Otherwise, it will fallback to the class name based matcher factory. + * If a view matcher service is registered with `identifier` attribute, that service will be used for matching. * + * Otherwise, it will fall back to the class name-based matcher factory. */ final class ServiceAwareMatcherFactory extends ClassNameMatcherFactory { - /** @var \Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry */ - private $viewMatcherRegistry; + private ViewMatcherRegistryInterface $viewMatcherRegistry; public function __construct( - ViewMatcherRegistry $viewMatcherRegistry, + ViewMatcherRegistryInterface $viewMatcherRegistry, Repository $repository, - $relativeNamespace = null, + ?string $relativeNamespace = null, array $matchConfig = [] ) { $this->viewMatcherRegistry = $viewMatcherRegistry; @@ -33,18 +35,16 @@ public function __construct( /** * @param string $matcherIdentifier - * - * @return \Ibexa\Core\MVC\Symfony\Matcher\ContentBased\MatcherInterface - * - * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException */ - protected function getMatcher($matcherIdentifier) + protected function getMatcher($matcherIdentifier): ViewMatcherInterface { if (strpos($matcherIdentifier, '@') === 0) { - return $this->viewMatcherRegistry->getMatcher(substr($matcherIdentifier, 1)); + $matcherIdentifier = substr($matcherIdentifier, 1); } - return parent::getMatcher($matcherIdentifier); + return $this->viewMatcherRegistry->hasMatcher($matcherIdentifier) + ? $this->viewMatcherRegistry->getMatcher($matcherIdentifier) + : parent::getMatcher($matcherIdentifier); } } diff --git a/src/bundle/Core/Matcher/ViewMatcherRegistry.php b/src/bundle/Core/Matcher/ViewMatcherRegistry.php index 0633c079d6..96c7275c01 100644 --- a/src/bundle/Core/Matcher/ViewMatcherRegistry.php +++ b/src/bundle/Core/Matcher/ViewMatcherRegistry.php @@ -8,20 +8,27 @@ namespace Ibexa\Bundle\Core\Matcher; +use Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface; use Ibexa\Core\Base\Exceptions\NotFoundException; use Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface; -final class ViewMatcherRegistry +/** + * @internal + */ +final class ViewMatcherRegistry implements ViewMatcherRegistryInterface { /** @var \Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface[] */ private $matchers; /** - * @param \Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface[] $matchers + * @param iterable<\Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface> $matchers */ - public function __construct(array $matchers = []) + public function __construct(iterable $matchers = []) { - $this->matchers = $matchers; + $this->matchers = []; + foreach ($matchers as $identifier => $matcher) { + $this->matchers[$identifier] = $matcher; + } } public function setMatcher(string $matcherIdentifier, ViewMatcherInterface $matcher): void @@ -44,6 +51,11 @@ public function getMatcher(string $matcherIdentifier): ViewMatcherInterface return $this->matchers[$matcherIdentifier]; } + + public function hasMatcher(string $matcherIdentifier): bool + { + return isset($this->matchers[$matcherIdentifier]); + } } class_alias(ViewMatcherRegistry::class, 'eZ\Bundle\EzPublishCoreBundle\Matcher\ViewMatcherRegistry'); diff --git a/src/bundle/Core/Resources/config/templating.yml b/src/bundle/Core/Resources/config/templating.yml index 018de1b317..ad7839456e 100644 --- a/src/bundle/Core/Resources/config/templating.yml +++ b/src/bundle/Core/Resources/config/templating.yml @@ -29,7 +29,12 @@ services: - '@Ibexa\Core\MVC\Symfony\View\Configurator\ViewProvider' - "@?logger" - Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry: ~ + Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface: + alias: Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry + + Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry: + arguments: + $matchers: !tagged_iterator { tag: 'ibexa.view.matcher', index_by: identifier } ibexa.content_view_provider.configured: class: Ibexa\Bundle\Core\View\Provider\Configured @@ -40,7 +45,7 @@ services: ibexa.content_view.matcher_factory: class: Ibexa\Bundle\Core\Matcher\ServiceAwareMatcherFactory arguments: - - '@Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry' + - '@Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface' - '@ibexa.api.repository' - 'Ibexa\Core\MVC\Symfony\Matcher\ContentBased' @@ -61,7 +66,7 @@ services: ibexa.content_view.default_matcher_factory: class: Ibexa\Bundle\Core\Matcher\ServiceAwareMatcherFactory arguments: - - '@Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry' + - '@Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface' - '@ibexa.api.repository' - 'Ibexa\Core\MVC\Symfony\Matcher\ContentBased' @@ -82,9 +87,9 @@ services: ibexa.location_view.matcher_factory: class: Ibexa\Bundle\Core\Matcher\ServiceAwareMatcherFactory arguments: - - '@Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry' - - '@ibexa.api.repository' - - 'Ibexa\Core\MVC\Symfony\Matcher\ContentBased' + $viewMatcherRegistry: '@Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface' + $repository: '@ibexa.api.repository' + $relativeNamespace: 'Ibexa\Core\MVC\Symfony\Matcher\ContentBased' ibexa.location_view.matcher_factory.dynamically_configured: class: Ibexa\Core\MVC\Symfony\Matcher\DynamicallyConfiguredMatcherFactoryDecorator diff --git a/src/contracts/MVC/View/ViewMatcherRegistryInterface.php b/src/contracts/MVC/View/ViewMatcherRegistryInterface.php new file mode 100644 index 0000000000..32f5248e57 --- /dev/null +++ b/src/contracts/MVC/View/ViewMatcherRegistryInterface.php @@ -0,0 +1,20 @@ +repository = $repository; $this->matcherRelativeNamespace = $relativeNamespace; diff --git a/tests/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPassTest.php b/tests/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPassTest.php deleted file mode 100644 index fbcc131792..0000000000 --- a/tests/bundle/Core/DependencyInjection/Compiler/ViewMatcherRegistryPassTest.php +++ /dev/null @@ -1,52 +0,0 @@ -setDefinition(ViewMatcherRegistry::class, new Definition()); - } - - protected function registerCompilerPass(ContainerBuilder $container): void - { - $container->addCompilerPass(new ViewMatcherRegistryPass()); - } - - public function testSetMatcher(): void - { - $def = new Definition(); - $def->addTag(ViewMatcherRegistryPass::MATCHER_TAG); - $serviceId = 'service_id'; - $this->setDefinition($serviceId, $def); - - $this->compile(); - - $this->assertContainerBuilderHasServiceDefinitionWithMethodCall( - ViewMatcherRegistry::class, - 'setMatcher', - [ - $serviceId, - new Reference($serviceId), - ] - ); - } -} - -class_alias(ViewMatcherRegistryPassTest::class, 'eZ\Bundle\EzPublishCoreBundle\Tests\DependencyInjection\Compiler\ViewMatcherRegistryPassTest'); diff --git a/tests/bundle/Core/Matcher/ServiceAwareMatcherFactoryTest.php b/tests/bundle/Core/Matcher/ServiceAwareMatcherFactoryTest.php new file mode 100644 index 0000000000..faf21dd3e1 --- /dev/null +++ b/tests/bundle/Core/Matcher/ServiceAwareMatcherFactoryTest.php @@ -0,0 +1,110 @@ +}>> + */ +final class ServiceAwareMatcherFactoryTest extends TestCase +{ + /** @var \Ibexa\Contracts\Core\MVC\View\ViewMatcherRegistryInterface&\PHPUnit\Framework\MockObject\MockObject */ + private ViewMatcherRegistryInterface $viewMatcherRegistryMock; + + /** @var \Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface&\PHPUnit\Framework\MockObject\MockObject */ + private ViewMatcherInterface $matcherMock; + + protected function setUp(): void + { + $this->viewMatcherRegistryMock = $this->createMock(ViewMatcherRegistryInterface::class); + $this->viewMatcherRegistryMock->method('hasMatcher')->willReturnMap( + [ + ['App\Matcher', true], + ['IdentifierBasedMatcher', true], + ] + ); + + $this->matcherMock = $this->createMock(ViewMatcherInterface::class); + + $this->viewMatcherRegistryMock->method('getMatcher')->willReturnMap( + [ + ['App\Matcher', $this->matcherMock], + ['IdentifierBasedMatcher', $this->matcherMock], + ] + ); + } + + /** + * @phpstan-return iterable + */ + public function getDataForTestMatch(): iterable + { + yield 'full view service-based matcher' => [ + new ContentView(), + [ + 'full' => [ + 'my_view' => [ + 'match' => [ + '@App\Matcher' => 'service-based config', + ], + ], + ], + ], + 'service-based config', + ]; + + yield 'full view identifier-based matcher' => [ + new ContentView(), + [ + 'full' => [ + 'my_view' => [ + 'match' => [ + 'IdentifierBasedMatcher' => 'identifier-based config', + ], + ], + ], + ], + 'identifier-based config', + ]; + } + + /** + * @dataProvider getDataForTestMatch + * + * @phpstan-param TMatchConfigArray $matchConfig + */ + public function testMatch(View $view, array $matchConfig, string $matchedConfigValue): void + { + $serviceMatcherFactory = new ServiceAwareMatcherFactory( + $this->viewMatcherRegistryMock, + $this->createMock(Repository::class), + null, + $matchConfig + ); + $this->matcherMock->method('setMatchingConfig')->with(true); + $this->matcherMock->method('match')->with($view)->willReturn($matchedConfigValue); + + self::assertSame( + [ + 'match' => $matchConfig['full']['my_view']['match'], + 'matcher' => $this->matcherMock, + ], + $serviceMatcherFactory->match($view) + ); + } +} diff --git a/tests/bundle/Core/Matcher/ViewMatcherRegistryTest.php b/tests/bundle/Core/Matcher/ViewMatcherRegistryTest.php index c9ba364173..93bb44aba6 100644 --- a/tests/bundle/Core/Matcher/ViewMatcherRegistryTest.php +++ b/tests/bundle/Core/Matcher/ViewMatcherRegistryTest.php @@ -8,21 +8,30 @@ use Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry; use Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException; -use Ibexa\Core\MVC\Symfony\Matcher\ContentBased\MatcherInterface; +use Ibexa\Core\MVC\Symfony\Matcher\ViewMatcherInterface; use PHPUnit\Framework\TestCase; -class ViewMatcherRegistryTest extends TestCase +/** + * @covers \Ibexa\Bundle\Core\Matcher\ViewMatcherRegistry + */ +final class ViewMatcherRegistryTest extends TestCase { private const MATCHER_NAME = 'test_matcher'; + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + */ public function testGetMatcher(): void { $matcher = $this->getMatcherMock(); $registry = new ViewMatcherRegistry([self::MATCHER_NAME => $matcher]); - $this->assertSame($matcher, $registry->getMatcher(self::MATCHER_NAME)); + self::assertSame($matcher, $registry->getMatcher(self::MATCHER_NAME)); } + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + */ public function testSetMatcher(): void { $matcher = $this->getMatcherMock(); @@ -30,18 +39,21 @@ public function testSetMatcher(): void $registry->setMatcher(self::MATCHER_NAME, $matcher); - $this->assertSame($matcher, $registry->getMatcher(self::MATCHER_NAME)); + self::assertSame($matcher, $registry->getMatcher(self::MATCHER_NAME)); } + /** + * @throws \Ibexa\Contracts\Core\Repository\Exceptions\NotFoundException + */ public function testSetMatcherOverride(): void { $matcher = $this->getMatcherMock(); $newMatcher = $this->getMatcherMock(); - $registry = new ViewMatcherRegistry([self::MATCHER_NAME, $matcher]); + $registry = new ViewMatcherRegistry([self::MATCHER_NAME => $matcher]); $registry->setMatcher(self::MATCHER_NAME, $newMatcher); - $this->assertSame($newMatcher, $registry->getMatcher(self::MATCHER_NAME)); + self::assertSame($newMatcher, $registry->getMatcher(self::MATCHER_NAME)); } public function testGetMatcherNotFound(): void @@ -52,9 +64,9 @@ public function testGetMatcherNotFound(): void $registry->getMatcher(self::MATCHER_NAME); } - protected function getMatcherMock(): MatcherInterface + protected function getMatcherMock(): ViewMatcherInterface { - return $this->createMock(MatcherInterface::class); + return $this->createMock(ViewMatcherInterface::class); } }