Deprecate using shortcut notation for user_model in favor of FQCN

UPGRADE-3.x.md

@@ -1,6 +1,11 @@
 UPGRADE 3.x
 ===========
 
+### Deprecated using shortcut notation when specifying the `user_model` option in `sonata:admin:generate-object-acl` command.
+
+The shortcut notation (`AppBundle:User`) has been deprecated in favor of the FQCN (`App\Model\User`) when passing
+`user_model` option to `sonata:admin:generate-object-acl` command.
+
 UPGRADE FROM 3.75 to 3.76
 =========================
 

src/Command/GenerateObjectAclCommand.php

@@ -21,6 +21,7 @@
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Input\InputOption;
 use Symfony\Component\Console\Output\OutputInterface;
+use Symfony\Component\Console\Question\Question;
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
 
@@ -58,6 +59,8 @@ class GenerateObjectAclCommand extends QuestionableCommand
     private $registry;
 
     /**
+     * NEXT_MAJOR: Remove $registry argument.
+     *
      * @param RegistryInterface|ManagerRegistry|null $registry
      */
     public function __construct(Pool $pool, array $aclObjectManipulators, $registry = null)
@@ -94,7 +97,7 @@ public function configure()
             ->addOption('object_owner', null, InputOption::VALUE_OPTIONAL, 'If set, the task will set the object owner for each admin.')
             // NEXT_MAJOR: Remove "user_entity" option.
             ->addOption('user_entity', null, InputOption::VALUE_OPTIONAL, '<error>DEPRECATED</error> Use <comment>user_model</comment> option instead.')
-            ->addOption('user_model', null, InputOption::VALUE_OPTIONAL, 'Shortcut notation like <comment>AcmeDemoBundle:User</comment>. If not set, it will be asked the first time an object owner is set.')
+            ->addOption('user_model', null, InputOption::VALUE_OPTIONAL, 'Fully qualified class name <comment>App\Model\User</comment>. If not set, it will be asked the first time an object owner is set.')
             ->addOption('step', null, InputOption::VALUE_NONE, 'If set, the task will ask for each admin if the ACLs need to be generated and what object owner to set, if any.')
         ;
     }
@@ -107,7 +110,7 @@ public function execute(InputInterface $input, OutputInterface $output)
             'This command helps you to generate ACL entities for the objects handled by the AdminBundle.',
             '',
             'If the step option is used, you will be asked if you want to generate the object ACL entities for each Admin.',
-            'You must use the shortcut notation like <comment>AcmeDemoBundle:User</comment> if you want to set an object owner.',
+            'You must use fully qualified class name like <comment>App\Model\User</comment> if you want to set an object owner.',
             '',
         ]);
 
@@ -226,25 +229,44 @@ protected function getUserEntityClass(InputInterface $input, OutputInterface $ou
 
         if ('' === $this->userEntityClass) {
             if ($input->getOption('user_model')) {
-                [$userBundle, $userModel] = Validators::validateEntityName($input->getOption('user_model'));
+                $userModelFromInput = $input->getOption('user_model');
             } else {
-                [$userBundle, $userModel] = $this->askAndValidate(
+                $userModelFromInput = $this->getQuestionHelper()->ask(
                     $input,
                     $output,
-                    'Please enter the User Entity shortcut name: ',
-                    '',
-                    'Sonata\AdminBundle\Command\Validators::validateEntityName'
+                    new Question('Please enter the User Model fully qualified class name: ', '')
                 );
             }
 
-            // Entity exists?
-            if ($this->registry instanceof RegistryInterface) {
-                $namespace = $this->registry->getEntityNamespace($userBundle);
+            if (!class_exists($userModelFromInput)) {
+                // NEXT_MAJOR: Remove the trigger, uncomment the exception and remove the code below the exception
+                // until "else".
+                @trigger_error(sprintf(
+                    'Passing a model shortcut name ("%s" given) as "user_model" option is deprecated since'
+                    .' sonata-project/admin-bundle 3.x and will throw an exception in 4.x.'
+                    .' Pass a fully qualified class name instead (e.g. App\Model\User).',
+                    $userModelFromInput
+                ), E_USER_DEPRECATED);
+
+//                throw new \InvalidArgumentException(sprintf(
+//                    'The "user_model" name be a fully qualified class name'
+//                    .' ("%s" given, expecting something like App\Model\User)',
+//                    $userModelFromInput
+//                ));
+
+                [$userBundle, $userModel] = Validators::validateEntityName($userModelFromInput);
+
+                // Model exists?
+                if ($this->registry instanceof RegistryInterface) {
+                    $namespace = $this->registry->getEntityNamespace($userBundle);
+                } else {
+                    $namespace = $this->registry->getAliasNamespace($userBundle);
+                }
+
+                $this->userEntityClass = sprintf('%s\%s', $namespace, $userModel);
             } else {
-                $namespace = $this->registry->getAliasNamespace($userBundle);
+                $this->userEntityClass = $userModelFromInput;
             }
-
-            $this->userEntityClass = sprintf('%s\%s', $namespace, $userModel);
         }
 
         return $this->userEntityClass;

src/Resources/config/commands.php

@@ -48,6 +48,7 @@
                 new ReferenceConfigurator('validator'),
             ])
 
+        // NEXT_MAJOR: Remove "doctrine" argument.
         ->set(GenerateObjectAclCommand::class, GenerateObjectAclCommand::class)
             ->public()
             ->tag('console.command')

tests/Command/GenerateObjectAclCommandTest.php

@@ -15,23 +15,27 @@
 
 use Doctrine\Persistence\ManagerRegistry;
 use PHPUnit\Framework\TestCase;
-use Prophecy\Argument;
 use Sonata\AdminBundle\Admin\AbstractAdmin;
 use Sonata\AdminBundle\Admin\Pool;
 use Sonata\AdminBundle\Command\GenerateObjectAclCommand;
+use Sonata\AdminBundle\Tests\Fixtures\Entity\Foo;
 use Sonata\AdminBundle\Util\ObjectAclManipulatorInterface;
 use Symfony\Bridge\Doctrine\RegistryInterface;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Output\StreamOutput;
 use Symfony\Component\Console\Tester\CommandTester;
 use Symfony\Component\DependencyInjection\Container;
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
+use Symfony\Component\Security\Acl\Domain\UserSecurityIdentity;
 
 /**
  * @author Javier Spagnoletti <[email protected]>
  */
 class GenerateObjectAclCommandTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     /**
      * @var Container
      */
@@ -66,7 +70,7 @@ public function testExecuteWithDeprecatedDoctrineService(): void
     {
         $pool = new Pool($this->container, '', '');
 
-        $registry = $this->prophesize(RegistryInterface::class)->reveal();
+        $registry = $this->createStub(RegistryInterface::class);
         $command = new GenerateObjectAclCommand($pool, [], $registry);
 
         $application = new Application();
@@ -83,7 +87,7 @@ public function testExecuteWithEmptyManipulators(): void
     {
         $pool = new Pool($this->container, '', '');
 
-        $registry = $this->prophesize(ManagerRegistry::class)->reveal();
+        $registry = $this->createStub(ManagerRegistry::class);
         $command = new GenerateObjectAclCommand($pool, [], $registry);
 
         $application = new Application();
@@ -98,21 +102,27 @@ public function testExecuteWithEmptyManipulators(): void
 
     public function testExecuteWithManipulatorNotFound(): void
     {
-        $admin = $this->prophesize(AbstractAdmin::class);
-        $registry = $this->prophesize(ManagerRegistry::class);
-        $pool = $this->prophesize(Pool::class);
+        $admin = $this->createStub(AbstractAdmin::class);
+        $registry = $this->createStub(ManagerRegistry::class);
+        $pool = $this->createStub(Pool::class);
 
-        $admin->getManagerType(Argument::any())->willReturn('bar');
+        $admin
+            ->method('getManagerType')
+            ->willReturn('bar');
 
-        $pool->getAdminServiceIds()->willReturn(['acme.admin.foo']);
+        $pool
+            ->method('getAdminServiceIds')
+            ->willReturn(['acme.admin.foo']);
 
-        $pool->getInstance(Argument::any())->willReturn($admin->reveal());
+        $pool
+            ->method('getInstance')
+            ->willReturn($admin);
 
         $aclObjectManipulators = [
             'bar' => new \stdClass(),
         ];
 
-        $command = new GenerateObjectAclCommand($pool->reveal(), $aclObjectManipulators, $registry->reveal());
+        $command = new GenerateObjectAclCommand($pool, $aclObjectManipulators, $registry);
 
         $application = new Application();
         $application->add($command);
@@ -126,20 +136,27 @@ public function testExecuteWithManipulatorNotFound(): void
 
     public function testExecuteWithManipulatorNotObjectAclManipulatorInterface(): void
     {
-        $admin = $this->prophesize(AbstractAdmin::class);
-        $registry = $this->prophesize(ManagerRegistry::class);
-        $pool = $this->prophesize(Pool::class);
+        $admin = $this->createStub(AbstractAdmin::class);
+        $registry = $this->createStub(ManagerRegistry::class);
+        $pool = $this->createStub(Pool::class);
+
+        $admin
+            ->method('getManagerType')
+            ->willReturn('bar');
 
-        $admin->getManagerType(Argument::any())->willReturn('bar');
+        $pool
+            ->method('getAdminServiceIds')
+            ->willReturn(['acme.admin.foo']);
 
-        $pool->getAdminServiceIds()->willReturn(['acme.admin.foo']);
-        $pool->getInstance(Argument::any())->willReturn($admin->reveal());
+        $pool
+            ->method('getInstance')
+            ->willReturn($admin);
 
         $aclObjectManipulators = [
             'sonata.admin.manipulator.acl.object.bar' => new \stdClass(),
         ];
 
-        $command = new GenerateObjectAclCommand($pool->reveal(), $aclObjectManipulators, $registry->reveal());
+        $command = new GenerateObjectAclCommand($pool, $aclObjectManipulators, $registry);
 
         $application = new Application();
         $application->add($command);
@@ -153,26 +170,33 @@ public function testExecuteWithManipulatorNotObjectAclManipulatorInterface(): vo
 
     public function testExecuteWithManipulator(): void
     {
-        $admin = $this->prophesize(AbstractAdmin::class);
-        $registry = $this->prophesize(ManagerRegistry::class);
-        $pool = $this->prophesize(Pool::class);
+        $admin = $this->createStub(AbstractAdmin::class);
+        $registry = $this->createStub(ManagerRegistry::class);
+        $pool = $this->createStub(Pool::class);
+
+        $admin
+            ->method('getManagerType')
+            ->willReturn('bar');
 
-        $admin->getManagerType(Argument::any())->willReturn('bar');
-        $admin = $admin->reveal();
+        $pool
+            ->method('getAdminServiceIds')
+            ->willReturn(['acme.admin.foo']);
 
-        $pool->getAdminServiceIds()->willReturn(['acme.admin.foo']);
-        $pool->getInstance(Argument::any())->willReturn($admin);
+        $pool
+            ->method('getInstance')
+            ->willReturn($admin);
 
-        $manipulator = $this->prophesize(ObjectAclManipulatorInterface::class);
+        $manipulator = $this->createMock(ObjectAclManipulatorInterface::class);
         $manipulator
-            ->batchConfigureAcls(Argument::type(StreamOutput::class), Argument::is($admin), null)
-            ->shouldBeCalledTimes(1);
+            ->expects($this->once())
+            ->method('batchConfigureAcls')
+            ->with($this->isInstanceOf(StreamOutput::class), $admin, null);
 
         $aclObjectManipulators = [
-            'sonata.admin.manipulator.acl.object.bar' => $manipulator->reveal(),
+            'sonata.admin.manipulator.acl.object.bar' => $manipulator,
         ];
 
-        $command = new GenerateObjectAclCommand($pool->reveal(), $aclObjectManipulators, $registry->reveal());
+        $command = new GenerateObjectAclCommand($pool, $aclObjectManipulators, $registry);
 
         $application = new Application();
         $application->add($command);
@@ -181,4 +205,81 @@ public function testExecuteWithManipulator(): void
         $commandTester = new CommandTester($command);
         $commandTester->execute(['command' => $command->getName()]);
     }
+
+    /**
+     * NEXT_MAJOR: Remove this test.
+     *
+     * @group legacy
+     */
+    public function testExecuteWithDeprecatedUserModelNotation(): void
+    {
+        $pool = new Pool($this->container, '', '');
+
+        $registry = $this->createStub(ManagerRegistry::class);
+        $command = new GenerateObjectAclCommand($pool, [], $registry);
+
+        $application = new Application();
+        $application->add($command);
+
+        $command = $application->find(GenerateObjectAclCommand::getDefaultName());
+        $commandTester = new CommandTester($command);
+
+        $this->expectDeprecation(
+            'Passing a model shortcut name ("AppBundle:User" given) as "user_model" option is deprecated'
+            .' since sonata-project/admin-bundle 3.x and will throw an exception in 4.x.'
+            .' Pass a fully qualified class name instead (e.g. App\Model\User).'
+        );
+        $commandTester->execute([
+            'command' => $command->getName(),
+            '--user_model' => 'AppBundle:User',
+        ]);
+    }
+
+    public function testExecuteWithUserModel(): void
+    {
+        $admin = $this->createStub(AbstractAdmin::class);
+        $registry = $this->createStub(ManagerRegistry::class);
+        $pool = $this->createStub(Pool::class);
+
+        $admin
+            ->method('getManagerType')
+            ->willReturn('bar');
+
+        $pool
+            ->method('getAdminServiceIds')
+            ->willReturn(['acme.admin.foo']);
+
+        $pool
+            ->method('getInstance')
+            ->willReturn($admin);
+
+        $manipulator = $this->createMock(ObjectAclManipulatorInterface::class);
+        $manipulator
+            ->expects($this->once())
+            ->method('batchConfigureAcls')
+            ->with(
+                $this->isInstanceOf(StreamOutput::class),
+                $admin,
+                $this->callback(static function (UserSecurityIdentity $userSecurityIdentity): bool {
+                    return Foo::class === $userSecurityIdentity->getClass();
+                })
+            );
+
+        $aclObjectManipulators = [
+            'sonata.admin.manipulator.acl.object.bar' => $manipulator,
+        ];
+
+        $command = new GenerateObjectAclCommand($pool, $aclObjectManipulators, $registry);
+
+        $application = new Application();
+        $application->add($command);
+
+        $command = $application->find(GenerateObjectAclCommand::getDefaultName());
+        $commandTester = new CommandTester($command);
+        $commandTester->execute([
+            'command' => $command->getName(),
+            '--user_model' => Foo::class,
+            '--object_owner' => true,
+        ]);
+    }
 }