From 8d5478b751e52ac87e0bba94a88f3f838b3b6193 Mon Sep 17 00:00:00 2001 From: Fran Moreno Date: Mon, 29 Mar 2021 09:12:34 +0200 Subject: [PATCH] Add tag for audit readers Audit readers MUST be tagged with this tag so we can use a service locator instead of injecting the whole container in the next major version. --- UPGRADE-3.x.md | 21 +++++ .../Compiler/AddAuditReadersCompilerPass.php | 53 ++++++++++++ .../SonataAdminExtension.php | 6 ++ src/Model/AuditManager.php | 26 +++++- src/Resources/config/core.php | 2 + .../AddAuditReadersCompilerPassTest.php | 81 +++++++++++++++++++ .../SonataAdminExtensionTest.php | 17 ++++ tests/Fixtures/Model/AuditReader.php | 49 +++++++++++ tests/Model/AuditManagerTest.php | 32 ++++++-- 9 files changed, 279 insertions(+), 8 deletions(-) create mode 100644 src/DependencyInjection/Compiler/AddAuditReadersCompilerPass.php create mode 100644 tests/DependencyInjection/Compiler/AddAuditReadersCompilerPassTest.php create mode 100644 tests/Fixtures/Model/AuditReader.php diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index a840a78e98..b999f71be9 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -4,6 +4,27 @@ UPGRADE 3.x UPGRADE FROM 3.xx to 3.xx ========================= +## Deprecated not setting "sonata.admin.audit_reader" tag in audit reader services + +If you are using [autoconfiguration](https://symfony.com/doc/4.4/service_container.html#the-autoconfigure-option), +all the services implementing `Sonata\AdminBundle\Model\AuditReaderInterface` will +be automatically tagged. Otherwise, you MUST tag them explicitly. + +Before: +```xml + + + +``` + +After: +```xml + + + + +``` + ### `Sonata\AdminBundle\Filter\FilterFactory` Deprecated passing a value which is not registered as a service as argument 2 for `create()` method. diff --git a/src/DependencyInjection/Compiler/AddAuditReadersCompilerPass.php b/src/DependencyInjection/Compiler/AddAuditReadersCompilerPass.php new file mode 100644 index 0000000000..a0f857f2a8 --- /dev/null +++ b/src/DependencyInjection/Compiler/AddAuditReadersCompilerPass.php @@ -0,0 +1,53 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\DependencyInjection\Compiler; + +use Sonata\AdminBundle\Model\AuditReaderInterface; +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\LogicException; +use Symfony\Component\DependencyInjection\Reference; + +final class AddAuditReadersCompilerPass implements CompilerPassInterface +{ + public const AUDIT_READER_TAG = 'sonata.admin.audit_reader'; + + public function process(ContainerBuilder $container) + { + if (!$container->has('sonata.admin.audit.manager')) { + return; + } + + $definition = $container->getDefinition('sonata.admin.audit.manager'); + $readers = []; + + foreach ($container->findTaggedServiceIds(self::AUDIT_READER_TAG, true) as $id => $attributes) { + $serviceDefinition = $container->getDefinition($id); + + if (!is_subclass_of($serviceDefinition->getClass(), AuditReaderInterface::class)) { + throw new LogicException(sprintf( + 'Service "%s" MUST implement "%s".', + $id, + AuditReaderInterface::class + )); + } + + $readers[$id] = new Reference($id); + } + + // NEXT_MAJOR: Change index from 1 to 0. + $definition->replaceArgument(1, ServiceLocatorTagPass::register($container, $readers)); + } +} diff --git a/src/DependencyInjection/SonataAdminExtension.php b/src/DependencyInjection/SonataAdminExtension.php index 327bcc87ca..7204eca47d 100644 --- a/src/DependencyInjection/SonataAdminExtension.php +++ b/src/DependencyInjection/SonataAdminExtension.php @@ -13,7 +13,9 @@ namespace Sonata\AdminBundle\DependencyInjection; +use Sonata\AdminBundle\DependencyInjection\Compiler\AddAuditReadersCompilerPass; use Sonata\AdminBundle\DependencyInjection\Compiler\ModelManagerCompilerPass; +use Sonata\AdminBundle\Model\AuditReaderInterface; use Sonata\AdminBundle\Model\ModelManagerInterface; // NEXT_MAJOR: Uncomment this line. //use Sonata\AdminBundle\Util\AdminAclUserManagerInterface; @@ -224,6 +226,10 @@ public function load(array $configs, ContainerBuilder $container) $container ->registerForAutoconfiguration(ModelManagerInterface::class) ->addTag(ModelManagerCompilerPass::MANAGER_TAG); + + $container + ->registerForAutoconfiguration(AuditReaderInterface::class) + ->addTag(AddAuditReadersCompilerPass::AUDIT_READER_TAG); } /** diff --git a/src/Model/AuditManager.php b/src/Model/AuditManager.php index 3c0e5d2ab8..2dfa154221 100644 --- a/src/Model/AuditManager.php +++ b/src/Model/AuditManager.php @@ -13,7 +13,9 @@ namespace Sonata\AdminBundle\Model; -use Symfony\Component\DependencyInjection\ContainerInterface; +use Psr\Container\ContainerInterface; +use Sonata\AdminBundle\DependencyInjection\Compiler\AddAuditReadersCompilerPass; +use Symfony\Component\DependencyInjection\ContainerInterface as SymfonyContainerInterface; /** * @final since sonata-project/admin-bundle 3.52 @@ -33,17 +35,35 @@ class AuditManager implements AuditManagerInterface protected $readers = []; /** - * @var ContainerInterface + * @var SymfonyContainerInterface */ protected $container; - public function __construct(ContainerInterface $container) + /** + * @var ContainerInterface|null + */ + private $psrContainer; + + // NEXT_MAJOR: Remove SymfonyContainerInterface parameter and only use ContainerInterface parameter + public function __construct(SymfonyContainerInterface $container, ?ContainerInterface $psrContainer = null) { $this->container = $container; + $this->psrContainer = $psrContainer; } public function setReader($serviceId, array $classes) { + // NEXT_MAJOR: Remove this "if" block. + if (null !== $this->psrContainer && !$this->psrContainer->has($serviceId)) { + @trigger_error(sprintf( + 'Not registering the audit reader "%1$s" with tag "%2$s" is deprecated since' + .' sonata-project/admin-bundle 3.x and will not work in 4.0.' + .' You MUST add "%2$s" tag to the service "%1$s".', + $serviceId, + AddAuditReadersCompilerPass::AUDIT_READER_TAG + ), \E_USER_DEPRECATED); + } + $this->readers[$serviceId] = $classes; } diff --git a/src/Resources/config/core.php b/src/Resources/config/core.php index a08b16ef65..56d45e74c0 100644 --- a/src/Resources/config/core.php +++ b/src/Resources/config/core.php @@ -189,7 +189,9 @@ ->set('sonata.admin.audit.manager', AuditManager::class) ->public() ->args([ + // NEXT_MAJOR: Remove next line. new ReferenceConfigurator('service_container'), + null, // Service locator ]) ->alias(AuditManager::class, 'sonata.admin.audit.manager') diff --git a/tests/DependencyInjection/Compiler/AddAuditReadersCompilerPassTest.php b/tests/DependencyInjection/Compiler/AddAuditReadersCompilerPassTest.php new file mode 100644 index 0000000000..84e14d2d82 --- /dev/null +++ b/tests/DependencyInjection/Compiler/AddAuditReadersCompilerPassTest.php @@ -0,0 +1,81 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Tests\DependencyInjection\Compiler; + +use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase; +use Sonata\AdminBundle\DependencyInjection\Compiler\AddAuditReadersCompilerPass; +use Sonata\AdminBundle\Model\AuditManager; +use Sonata\AdminBundle\Tests\Fixtures\Model\AuditReader; +use Symfony\Component\DependencyInjection\Container; +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; +use Symfony\Component\DependencyInjection\Exception\LogicException; +use Symfony\Component\DependencyInjection\Reference; + +final class AddAuditReadersCompilerPassTest extends AbstractCompilerPassTestCase +{ + public function testProcess(): void + { + $auditManagerDefinition = new Definition(AuditManager::class, [ + // NEXT_MAJOR: Remove next line. + new Container(), + null, + ]); + + $this->container + ->setDefinition('sonata.admin.audit.manager', $auditManagerDefinition); + + $auditReader = new Definition(AuditReader::class); + $auditReader + ->addTag(AddAuditReadersCompilerPass::AUDIT_READER_TAG); + + $this->container + ->setDefinition('std_audit_reader', $auditReader); + + $this->compile(); + + $this->assertContainerBuilderHasServiceLocator( + // NEXT_MAJOR: Change index from 1 to 0. + (string) $this->container->getDefinition('sonata.admin.audit.manager')->getArgument(1), + [ + 'std_audit_reader' => new Reference('std_audit_reader'), + ] + ); + } + + public function testServiceTaggedMustImplementInterface(): void + { + $auditManagerDefinition = new Definition(AuditManager::class); + + $this->container + ->setDefinition('sonata.admin.audit.manager', $auditManagerDefinition); + + $auditReader = new Definition(\stdClass::class); + $auditReader + ->addTag(AddAuditReadersCompilerPass::AUDIT_READER_TAG); + + $this->container + ->setDefinition('std_audit_reader', $auditReader); + + $this->expectException(LogicException::class); + $this->expectExceptionMessage('Service "std_audit_reader" MUST implement "Sonata\AdminBundle\Model\AuditReaderInterface".'); + + $this->compile(); + } + + protected function registerCompilerPass(ContainerBuilder $container): void + { + $container->addCompilerPass(new AddAuditReadersCompilerPass()); + } +} diff --git a/tests/DependencyInjection/SonataAdminExtensionTest.php b/tests/DependencyInjection/SonataAdminExtensionTest.php index 7785ab57a9..f775cf2ae4 100644 --- a/tests/DependencyInjection/SonataAdminExtensionTest.php +++ b/tests/DependencyInjection/SonataAdminExtensionTest.php @@ -19,6 +19,8 @@ use Sonata\AdminBundle\Admin\BreadcrumbsBuilderInterface; use Sonata\AdminBundle\Admin\Pool; use Sonata\AdminBundle\Bridge\Exporter\AdminExporter; +use Sonata\AdminBundle\DependencyInjection\Compiler\AddAuditReadersCompilerPass; +use Sonata\AdminBundle\DependencyInjection\Compiler\ModelManagerCompilerPass; use Sonata\AdminBundle\DependencyInjection\Configuration; use Sonata\AdminBundle\DependencyInjection\SonataAdminExtension; use Sonata\AdminBundle\Event\AdminEventExtension; @@ -28,6 +30,8 @@ use Sonata\AdminBundle\Filter\Persister\SessionFilterPersister; use Sonata\AdminBundle\Model\AuditManager; use Sonata\AdminBundle\Model\AuditManagerInterface; +use Sonata\AdminBundle\Model\AuditReaderInterface; +use Sonata\AdminBundle\Model\ModelManagerInterface; use Sonata\AdminBundle\Route\AdminPoolLoader; use Sonata\AdminBundle\Search\SearchHandler; use Sonata\AdminBundle\Templating\MutableTemplateRegistryInterface; @@ -468,6 +472,19 @@ public function testSetInvalidSkin(): void ]); } + public function testAutoregisterAddingTagsToServices(): void + { + $this->load(); + + $autoconfiguredInstancesOf = $this->container->getAutoconfiguredInstanceof(); + + $this->assertArrayHasKey(ModelManagerInterface::class, $autoconfiguredInstancesOf); + $this->assertTrue($autoconfiguredInstancesOf[ModelManagerInterface::class]->hasTag(ModelManagerCompilerPass::MANAGER_TAG)); + + $this->assertArrayHasKey(AuditReaderInterface::class, $autoconfiguredInstancesOf); + $this->assertTrue($autoconfiguredInstancesOf[AuditReaderInterface::class]->hasTag(AddAuditReadersCompilerPass::AUDIT_READER_TAG)); + } + protected function getContainerExtensions(): array { return [new SonataAdminExtension()]; diff --git a/tests/Fixtures/Model/AuditReader.php b/tests/Fixtures/Model/AuditReader.php new file mode 100644 index 0000000000..da4f1352cb --- /dev/null +++ b/tests/Fixtures/Model/AuditReader.php @@ -0,0 +1,49 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Tests\Fixtures\Model; + +use Sonata\AdminBundle\Model\AuditReaderInterface; + +// NEXT_MAJOR: Add type declarations +final class AuditReader implements AuditReaderInterface +{ + public function find($className, $id, $revision): object + { + return new \stdClass(); + } + + public function findRevisionHistory($className, $limit = 20, $offset = 0): array + { + return [ + new \stdClass(), + ]; + } + + public function findRevision($classname, $revision): object + { + return new \stdClass(); + } + + public function findRevisions($className, $id): array + { + return [ + new \stdClass(), + ]; + } + + public function diff($className, $id, $oldRevision, $newRevision): array + { + return []; + } +} diff --git a/tests/Model/AuditManagerTest.php b/tests/Model/AuditManagerTest.php index a856c5f78e..9ad8f8718f 100644 --- a/tests/Model/AuditManagerTest.php +++ b/tests/Model/AuditManagerTest.php @@ -16,21 +16,23 @@ use PHPUnit\Framework\TestCase; use Sonata\AdminBundle\Model\AuditManager; use Sonata\AdminBundle\Model\AuditReaderInterface; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\DependencyInjection\Container; /** - * Test for AuditManager. - * * @author Andrej Hudec */ -class AuditManagerTest extends TestCase +final class AuditManagerTest extends TestCase { + // NEXT_MAJOR: Remove next line. + use ExpectDeprecationTrait; + public function testGetReader(): void { $container = new Container(); - $fooReader = $this->getMockForAbstractClass(AuditReaderInterface::class); - $barReader = $this->getMockForAbstractClass(AuditReaderInterface::class); + $fooReader = $this->createStub(AuditReaderInterface::class); + $barReader = $this->createStub(AuditReaderInterface::class); $container->set('foo_reader', $fooReader); $container->set('bar_reader', $barReader); @@ -54,4 +56,24 @@ public function testGetReaderWithException(): void $auditManager->getReader('Foo\Foo'); } + + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ + public function testReaderShouldBeTagged(): void + { + $container = new Container(); + + $fooReader = $this->createStub(AuditReaderInterface::class); + + $container->set('foo_reader', $fooReader); + + $auditManager = new AuditManager($container, new Container()); + + $this->expectDeprecation('Not registering the audit reader "foo_reader" with tag "sonata.admin.audit_reader" is deprecated since sonata-project/admin-bundle 3.x and will not work in 4.0. You MUST add "sonata.admin.audit_reader" tag to the service "foo_reader".'); + + $auditManager->setReader('foo_reader', ['Foo\Foo1', 'Foo\Foo2']); + } }