diff --git a/UPGRADE-3.x.md b/UPGRADE-3.x.md index 5b8db04a59..a840a78e98 100644 --- a/UPGRADE-3.x.md +++ b/UPGRADE-3.x.md @@ -4,6 +4,10 @@ UPGRADE 3.x UPGRADE FROM 3.xx to 3.xx ========================= +### `Sonata\AdminBundle\Filter\FilterFactory` + +Deprecated passing a value which is not registered as a service as argument 2 for `create()` method. + ### Deprecated MenuBuilderInterface Deprecated `AbstractAdmin::getSideMenu()` diff --git a/src/DependencyInjection/Compiler/AddFilterTypeCompilerPass.php b/src/DependencyInjection/Compiler/AddFilterTypeCompilerPass.php index 359583584e..4c70566bdb 100644 --- a/src/DependencyInjection/Compiler/AddFilterTypeCompilerPass.php +++ b/src/DependencyInjection/Compiler/AddFilterTypeCompilerPass.php @@ -13,9 +13,13 @@ namespace Sonata\AdminBundle\DependencyInjection\Compiler; +use Sonata\AdminBundle\Filter\FilterInterface; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\ContainerBuilder; +// NEXT_MAJOR: Uncomment next line. +// use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; + /** * @final since sonata-project/admin-bundle 3.52 * @@ -25,6 +29,10 @@ class AddFilterTypeCompilerPass implements CompilerPassInterface { public function process(ContainerBuilder $container) { + if (!$container->has('sonata.admin.builder.filter.factory')) { + return; + } + $definition = $container->getDefinition('sonata.admin.builder.filter.factory'); $types = []; @@ -34,6 +42,32 @@ public function process(ContainerBuilder $container) $serviceDefinition->setShared(false); $serviceDefinition->setPublic(true); // Temporary fix until we can support service locators + $class = $serviceDefinition->getClass(); + $reflectionClass = $container->getReflectionClass($class); + + if (null === $reflectionClass) { + // NEXT_MAJOR: Remove "trigger_error" and uncomment the exception below. + @trigger_error(sprintf( + 'Not declaring a filter with an existing class name is deprecated since' + .' sonata-project/admin-bundle 3.x and will not work in 4.0.' + .' You MUST register a service with an existing class name for service "%s".', + $id, + ), \E_USER_DEPRECATED); + + //throw new InvalidArgumentException(sprintf('Class "%s" used for service "%s" cannot be found.', $class, $id)); + } + + if (null !== $reflectionClass && !$reflectionClass->isSubclassOf(FilterInterface::class)) { + // NEXT_MAJOR: Remove "trigger_error" and uncomment the exception below. + @trigger_error(sprintf( + 'Registering service "%s" without implementing interface "%s" is deprecated since' + .' sonata-project/admin-bundle 3.x and will be mandatory in 4.0.', + $id, + FilterInterface::class, + ), \E_USER_DEPRECATED); + // throw new InvalidArgumentException(sprintf('Service "%s" must implement interface "%s".', $id, FilterInterface::class)); + } + $types[$serviceDefinition->getClass()] = $id; // NEXT_MAJOR: Remove this loop, only FQCN will be supported diff --git a/src/Filter/FilterFactory.php b/src/Filter/FilterFactory.php index 46eebc8eac..7657a2f357 100644 --- a/src/Filter/FilterFactory.php +++ b/src/Filter/FilterFactory.php @@ -55,12 +55,19 @@ public function create($name, $type, array $options = []) if ($filter && !class_exists($type)) { @trigger_error(sprintf( 'Referencing a filter by name (%s) is deprecated since version 3.57 and will be removed in 4.0.' - .' Use the fully-qualified type class name instead (%s)', + .' Use the fully-qualified type class name instead (%s).', $type, \get_class($filter) ), \E_USER_DEPRECATED); } } elseif (class_exists($type)) { + // NEXT_MAJOR: Remove this "elseif" block + @trigger_error(sprintf( + 'Not declaring a filter as service is deprecated since sonata-project/admin-bundle 3.x' + .' and will not work in 4.0.' + .' You MUST register a service with class name (%s) instead.', + $type, + ), \E_USER_DEPRECATED); $filter = new $type(); } else { throw new \RuntimeException(sprintf('No attached service to type named `%s`', $type)); diff --git a/tests/DependencyInjection/Compiler/AddFilterTypeCompilerPassTest.php b/tests/DependencyInjection/Compiler/AddFilterTypeCompilerPassTest.php index 2ca89da4da..fbb3536d26 100644 --- a/tests/DependencyInjection/Compiler/AddFilterTypeCompilerPassTest.php +++ b/tests/DependencyInjection/Compiler/AddFilterTypeCompilerPassTest.php @@ -13,83 +13,112 @@ namespace Sonata\AdminBundle\Tests\DependencyInjection\Compiler; -use PHPUnit\Framework\TestCase; +use Matthias\SymfonyDependencyInjectionTest\PhpUnit\AbstractCompilerPassTestCase; use Sonata\AdminBundle\DependencyInjection\Compiler\AddFilterTypeCompilerPass; +use Sonata\AdminBundle\Filter\FilterFactoryInterface; +use Sonata\AdminBundle\Tests\Fixtures\Filter\BarFilter; +use Sonata\AdminBundle\Tests\Fixtures\Filter\FooFilter; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; -class AddFilterTypeCompilerPassTest extends TestCase -{ - private $filterFactory; - - private $fooFilter; - - private $barFilter; +// NEXT_MAJOR: Uncomment next line. +//use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; - private $bazFilter; +final class AddFilterTypeCompilerPassTest extends AbstractCompilerPassTestCase +{ + use ExpectDeprecationTrait; protected function setUp(): void { - $this->filterFactory = $this->createMock(Definition::class); - $this->fooFilter = $this->createMock(Definition::class); - $this->barFilter = $this->createMock(Definition::class); - $this->bazFilter = $this->createMock(Definition::class); + parent::setUp(); + + $filterFactoryDefinition = new Definition(FilterFactoryInterface::class, [ + null, + [], + ]); + + $this->container + ->setDefinition('sonata.admin.builder.filter.factory', $filterFactoryDefinition); } public function testProcess(): void { - $containerBuilderMock = $this->createMock(ContainerBuilder::class); - - $containerBuilderMock - ->method('getDefinition') - ->with($this->anything()) - ->willReturnMap([ - ['sonata.admin.builder.filter.factory', $this->filterFactory], - ['acme.demo.foo_filter', $this->fooFilter], - ['acme.demo.bar_filter', $this->barFilter], - ['acme.demo.baz_filter', $this->bazFilter], + $fooFilter = new Definition(FooFilter::class); + $fooFilter + ->addTag('sonata.admin.filter.type', [ + 'alias' => 'foo_filter_alias', ]); - $containerBuilderMock->expects($this->once()) - ->method('findTaggedServiceIds') - ->with($this->equalTo('sonata.admin.filter.type')) - ->willReturn([ - 'acme.demo.foo_filter' => [ - 'tag1' => [ - 'alias' => 'foo_filter_alias', - ], - ], - 'acme.demo.bar_filter' => [ - 'tag1' => [ - 'alias' => 'bar_filter_alias', - ], - ], - 'acme.demo.baz_filter' => [ - 'tag1' => [ - ], - ], - ]); + $this->container + ->setDefinition('acme.demo.foo_filter', $fooFilter); - $this->fooFilter->method('getClass') - ->willReturn('Acme\Filter\FooFilter'); + $barFilter = new Definition(BarFilter::class); + $barFilter + ->addTag('sonata.admin.filter.type'); - $this->barFilter->method('getClass') - ->willReturn('Acme\Filter\BarFilter'); + $this->container + ->setDefinition('acme.demo.bar_filter', $barFilter); - $this->bazFilter->method('getClass') - ->willReturn('Acme\Filter\BazFilter'); + $this->compile(); - $this->filterFactory->expects($this->once()) - ->method('replaceArgument') - ->with(1, $this->equalTo([ + $this->assertContainerBuilderHasServiceDefinitionWithArgument( + 'sonata.admin.builder.filter.factory', + 1, + [ 'foo_filter_alias' => 'acme.demo.foo_filter', - 'Acme\Filter\FooFilter' => 'acme.demo.foo_filter', - 'bar_filter_alias' => 'acme.demo.bar_filter', - 'Acme\Filter\BarFilter' => 'acme.demo.bar_filter', - 'Acme\Filter\BazFilter' => 'acme.demo.baz_filter', - ])); - - $extensionsPass = new AddFilterTypeCompilerPass(); - $extensionsPass->process($containerBuilderMock); + FooFilter::class => 'acme.demo.foo_filter', + BarFilter::class => 'acme.demo.bar_filter', + ] + ); + } + + /** + * NEXT_MAJOR: Remove legacy group. + * + * @group legacy + */ + public function testServicesMustHaveAClassName(): void + { + $filter = new Definition('not_existing_class'); + $filter + ->addTag('sonata.admin.filter.type'); + + $this->container + ->setDefinition('acme.demo.foo_filter', $filter); + + // NEXT_MAJOR: Remove deprecation and uncomment exception. + $this->expectDeprecation('Not declaring a filter with an existing class name is deprecated since sonata-project/admin-bundle 3.x and will not work in 4.0. You MUST register a service with an existing class name for service "acme.demo.foo_filter".'); +// $this->expectException(InvalidArgumentException::class); +// $this->expectExceptionMessage('Class "not_existing_class" used for service "acme.demo.foo_filter" cannot be found.'); + + $this->compile(); + } + + /** + * NEXT_MAJOR: Remove legacy group. + * + * @group legacy + */ + public function testServicesMustImplementFilterInterface(): void + { + $filter = new Definition(\stdClass::class); + $filter + ->addTag('sonata.admin.filter.type'); + + $this->container + ->setDefinition('acme.demo.foo_filter', $filter); + + // NEXT_MAJOR: Remove deprecation and uncomment exception. + $this->expectDeprecation('Registering service "acme.demo.foo_filter" without implementing interface "Sonata\AdminBundle\Filter\FilterInterface" is deprecated since sonata-project/admin-bundle 3.x and will be mandatory in 4.0.'); +// $this->expectException(InvalidArgumentException::class); +// $this->expectExceptionMessage('Service "acme.demo.foo_filter" MUST implement interface "Sonata\AdminBundle\Filter\FilterInterface".'); + + $this->compile(); + } + + protected function registerCompilerPass(ContainerBuilder $container): void + { + $container->addCompilerPass(new AddFilterTypeCompilerPass()); } } diff --git a/tests/Filter/FilterFactoryTest.php b/tests/Filter/FilterFactoryTest.php index 8bf092df8e..f8f336cecf 100644 --- a/tests/Filter/FilterFactoryTest.php +++ b/tests/Filter/FilterFactoryTest.php @@ -17,16 +17,20 @@ use Sonata\AdminBundle\Filter\FilterFactory; use Sonata\AdminBundle\Filter\FilterInterface; use Sonata\AdminBundle\Form\Type\Filter\DefaultType; +use Sonata\AdminBundle\Tests\Fixtures\Filter\FooFilter; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; use Symfony\Component\DependencyInjection\Container; class FilterFactoryTest extends TestCase { + use ExpectDeprecationTrait; + public function testEmptyType(): void { $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('The type must be defined'); - $filter = new FilterFactory(new Container(), []); + $filter = new FilterFactory(new Container()); $filter->create('test', null); } @@ -35,7 +39,7 @@ public function testUnknownType(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('No attached service to type named `mytype`'); - $filter = new FilterFactory(new Container(), []); + $filter = new FilterFactory(new Container()); $filter->create('test', 'mytype'); } @@ -44,13 +48,18 @@ public function testUnknownClassType(): void $this->expectException(\RuntimeException::class); $this->expectExceptionMessage('No attached service to type named `Sonata\AdminBundle\Form\Type\Filter\FooType`'); - $filter = new FilterFactory(new Container(), []); + $filter = new FilterFactory(new Container()); $filter->create('test', 'Sonata\AdminBundle\Form\Type\Filter\FooType'); } public function testClassType(): void { - $filter = new FilterFactory(new Container(), []); + $container = new Container(); + $container + ->set(DefaultType::class, new DefaultType()); + $filter = new FilterFactory($container, [ + DefaultType::class => DefaultType::class, + ]); $this->expectException(\RuntimeException::class); $this->expectExceptionMessage( @@ -60,6 +69,20 @@ public function testClassType(): void $filter->create('test', DefaultType::class); } + /** + * NEXT_MAJOR: Remove this test. + * + * @group legacy + */ + public function testItTriggersDeprecationWithoutTheService(): void + { + $filter = new FilterFactory(new Container()); + + $this->expectDeprecation('Not declaring a filter as service is deprecated since sonata-project/admin-bundle 3.x and will not work in 4.0. You MUST register a service with class name (Sonata\AdminBundle\Tests\Fixtures\Filter\FooFilter) instead.'); + + $filter->create('test', FooFilter::class); + } + /** * NEXT_MAJOR: Remove this test. * diff --git a/tests/Fixtures/Filter/BarFilter.php b/tests/Fixtures/Filter/BarFilter.php new file mode 100644 index 0000000000..073bc9e890 --- /dev/null +++ b/tests/Fixtures/Filter/BarFilter.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Sonata\AdminBundle\Tests\Fixtures\Filter; + +use Sonata\AdminBundle\Datagrid\ProxyQueryInterface; +use Sonata\AdminBundle\Filter\Filter; +use Sonata\AdminBundle\Form\Type\Filter\DefaultType; + +final class BarFilter extends Filter +{ + /** + * NEXT_MAJOR: Remove this method. + */ + public function filter(ProxyQueryInterface $query, $alias, $field, $value): void + { + } + + // NEXT_MAJOR: Add type declarations + public function apply($query, $value): void + { + } + + public function getDefaultOptions(): array + { + return ['bar' => 'bar']; + } + + public function getRenderSettings(): array + { + return [DefaultType::class, [ + 'label' => 'label', + ]]; + } +}