Merge pull request #2108 from tarlepp/feat/remove-param-converter

Feat - Remove use of `ParamConverter`
tarlepp · Dec 29, 2022 · 5be7373 · 5be7373
2 parents c36f91a + 8f40fcf
commit 5be7373
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 62 deletions.
diff --git a/src/Controller/v1/Role/InheritedRolesController.php b/src/Controller/v1/Role/InheritedRolesController.php
@@ -9,11 +9,9 @@
 namespace App\Controller\v1\Role;
 
 use App\Entity\Role;
-use App\Resource\RoleResource;
 use App\Security\Interfaces\RolesServiceInterface;
 use App\Security\RolesService;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Annotation\Route;
@@ -80,10 +78,6 @@ public function __construct(
         methods: [Request::METHOD_GET],
     )]
     #[IsGranted(RolesServiceInterface::ROLE_ADMIN)]
-    #[ParamConverter(
-        data: 'role',
-        class: RoleResource::class,
-    )]
     public function __invoke(Role $role): JsonResponse
     {
         return new JsonResponse($this->rolesService->getInheritedRoles([$role->getId()]));

diff --git a/src/Controller/v1/User/AttachUserGroupController.php b/src/Controller/v1/User/AttachUserGroupController.php
@@ -15,7 +15,6 @@
 use App\Security\RolesService;
 use Nelmio\ApiDocBundle\Annotation\Model;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -136,14 +135,7 @@ public function __construct(
         methods: [Request::METHOD_POST],
     )]
     #[IsGranted(RolesService::ROLE_ROOT)]
-    #[ParamConverter(
-        data: 'user',
-        class: UserResource::class,
-    )]
-    #[ParamConverter(
-        data: 'userGroup',
-        class: UserGroupResource::class,
-    )]
+
     public function __invoke(User $user, UserGroup $userGroup): JsonResponse
     {
         $status = $user->getUserGroups()->contains($userGroup) ? Response::HTTP_OK : Response::HTTP_CREATED;

diff --git a/src/Controller/v1/User/DeleteUserController.php b/src/Controller/v1/User/DeleteUserController.php
@@ -14,7 +14,6 @@
 use App\Rest\Traits\Methods;
 use App\Security\RolesService;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Exception\HttpException;
@@ -44,23 +43,19 @@ public function __construct(
      * @throws Throwable
      */
     #[Route(
-        path: '/v1/user/{requestUser}',
+        path: '/v1/user/{user}',
         requirements: [
             'requestUser' => '%app.uuid_v1_regex%',
         ],
         methods: [Request::METHOD_DELETE],
     )]
     #[IsGranted(RolesService::ROLE_ROOT)]
-    #[ParamConverter(
-        data: 'requestUser',
-        class: UserResource::class,
-    )]
-    public function __invoke(Request $request, User $requestUser, User $loggedInUser): Response
+    public function __invoke(Request $request, User $user, User $loggedInUser): Response
     {
-        if ($loggedInUser === $requestUser) {
+        if ($loggedInUser === $user) {
             throw new HttpException(Response::HTTP_BAD_REQUEST, 'You cannot remove yourself...');
         }
 
-        return $this->deleteMethod($request, $requestUser->getId());
+        return $this->deleteMethod($request, $user->getId());
     }
 }
diff --git a/src/Controller/v1/User/DetachUserGroupController.php b/src/Controller/v1/User/DetachUserGroupController.php
@@ -15,7 +15,6 @@
 use App\Security\RolesService;
 use Nelmio\ApiDocBundle\Annotation\Model;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Annotation\Route;
@@ -122,14 +121,6 @@ public function __construct(
         methods: [Request::METHOD_DELETE],
     )]
     #[IsGranted(RolesService::ROLE_ROOT)]
-    #[ParamConverter(
-        data: 'user',
-        class: UserResource::class,
-    )]
-    #[ParamConverter(
-        data: 'userGroup',
-        class: UserGroupResource::class,
-    )]
     public function __invoke(User $user, UserGroup $userGroup): JsonResponse
     {
         $this->userResource->save($user->removeUserGroup($userGroup), false);

diff --git a/src/Controller/v1/UserGroup/AttachUserController.php b/src/Controller/v1/UserGroup/AttachUserController.php
@@ -15,7 +15,6 @@
 use App\Security\RolesService;
 use Nelmio\ApiDocBundle\Annotation\Model;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Annotation\Route;
@@ -124,14 +123,6 @@ public function __construct(
         methods: [Request::METHOD_POST],
     )]
     #[IsGranted(RolesService::ROLE_ROOT)]
-    #[ParamConverter(
-        data: 'userGroup',
-        class: UserGroupResource::class,
-    )]
-    #[ParamConverter(
-        data: 'user',
-        class: UserResource::class,
-    )]
     public function __invoke(UserGroup $userGroup, User $user): JsonResponse
     {
         $status = $userGroup->getUsers()->contains($user) ? 200 : 201;

diff --git a/src/Controller/v1/UserGroup/DetachUserController.php b/src/Controller/v1/UserGroup/DetachUserController.php
@@ -15,7 +15,6 @@
 use App\Security\RolesService;
 use Nelmio\ApiDocBundle\Annotation\Model;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Annotation\Route;
@@ -111,14 +110,6 @@ public function __construct(
         methods: [Request::METHOD_DELETE],
     )]
     #[IsGranted(RolesService::ROLE_ROOT)]
-    #[ParamConverter(
-        data: 'userGroup',
-        class: UserGroupResource::class,
-    )]
-    #[ParamConverter(
-        data: 'user',
-        class: UserResource::class,
-    )]
     public function __invoke(UserGroup $userGroup, User $user): JsonResponse
     {
         $this->userGroupResource->save($userGroup->removeUser($user), false);

diff --git a/src/Controller/v1/UserGroup/UsersController.php b/src/Controller/v1/UserGroup/UsersController.php
@@ -10,13 +10,11 @@
 
 use App\Entity\User;
 use App\Entity\UserGroup;
-use App\Resource\UserGroupResource;
 use App\Resource\UserResource;
 use App\Rest\ResponseHandler;
 use App\Security\RolesService;
 use Nelmio\ApiDocBundle\Annotation\Model;
 use OpenApi\Annotations as OA;
-use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\Routing\Annotation\Route;
@@ -86,10 +84,6 @@ public function __construct(
         methods: [Request::METHOD_GET],
     )]
     #[IsGranted(RolesService::ROLE_ADMIN)]
-    #[ParamConverter(
-        data: 'userGroup',
-        class: UserGroupResource::class,
-    )]
     public function __invoke(Request $request, UserGroup $userGroup): Response
     {
         return $this->responseHandler

diff --git a/src/ValueResolver/EntityValueResolver.php b/src/ValueResolver/EntityValueResolver.php
@@ -15,8 +15,11 @@
 use Symfony\Component\HttpKernel\Controller\ValueResolverInterface;
 use Symfony\Component\HttpKernel\ControllerMetadata\ArgumentMetadata;
 use Throwable;
+use function end;
+use function explode;
 use function is_string;
 use function is_subclass_of;
+use function Symfony\Component\String\u;
 
 /**
  * Class EntityValueResolver
@@ -46,10 +49,28 @@ public function __construct(
     ) {
     }
 
+    /**
+     * With this we check following cases:
+     *  1. Request parameter is a string (query, request, attributes)
+     *  2. Argument type is subclass of EntityInterface
+     *  3. Argument name is same as entity name (argument type) as in camel case format
+     *  4. Our REST resource collection has resource for this entity
+     *
+     * Examples:
+     *  public function __invoke(UserGroup $userGroup): JsonResponse    => Works
+     *  public function __invoke(User $user): JsonResponse              => Works
+     *  public function __invoke(UserGroup $UserGroup): JsonResponse    => Doesn't work
+     *  public function __invoke(UserGroup $group): JsonResponse        => Doesn't work
+     *  public function __invoke(User $requestUser): JsonResponse       => Doesn't work (another resolver does this)
+     */
     public function supports(Request $request, ArgumentMetadata $argument): bool
     {
+        $bits = explode('\\', (string)$argument->getType());
+        $entity = end($bits);
+
         return is_string($this->getUuid($argument, $request))
             && is_subclass_of((string)$argument->getType(), EntityInterface::class, true)
+            && $argument->getName() === u($entity)->camel()->toString()
             && $this->resourceCollection->hasEntityResource($argument->getType());
     }
 

diff --git a/tests/Integration/ValueResolver/EntityValueResolverTest.php b/tests/Integration/ValueResolver/EntityValueResolverTest.php
@@ -31,6 +31,10 @@ public function testThatSupportsReturnFalseWithNotSupportedRequestParameterType(
     {
         $resourceCollection = $this->getMockBuilder(ResourceCollection::class)->disableOriginalConstructor()->getMock();
 
+        $resourceCollection
+            ->expects(static::never())
+            ->method('hasEntityResource');
+
         $resolver = new EntityValueResolver($resourceCollection);
         $metadata = new ArgumentMetadata('foo', null, false, false, null);
 
@@ -50,8 +54,37 @@ public function testThatSupportsReturnFalseWithWrongArgumentParameterType(): voi
     {
         $resourceCollection = $this->getMockBuilder(ResourceCollection::class)->disableOriginalConstructor()->getMock();
 
+        $resourceCollection
+            ->expects(static::never())
+            ->method('hasEntityResource');
+
+        $resolver = new EntityValueResolver($resourceCollection);
+        $metadata = new ArgumentMetadata('user', stdClass::class, false, false, null);
+
+        self::assertFalse($resolver->supports(Request::create('/', 'GET', [
+            'foo' => 'bar',
+        ]), $metadata));
+    }
+
+    /**
+     * @throws Throwable
+     *
+     * @testdox Test that `resolve()` method returns empty array when argument name is wrong
+     */
+    public function testThatResolveReturnEmptyArrayWhenArgumentNameIsWrong(): void
+    {
+        $resourceCollection = $this->getMockBuilder(ResourceCollection::class)->disableOriginalConstructor()->getMock();
+
+        $resourceCollection
+            ->expects(static::never())
+            ->method('hasEntityResource');
+
+        $resourceCollection
+            ->expects(static::never())
+            ->method('hasEntityResource');
+
         $resolver = new EntityValueResolver($resourceCollection);
-        $metadata = new ArgumentMetadata('foo', stdClass::class, false, false, null);
+        $metadata = new ArgumentMetadata('foo', User::class, false, false, null);
 
         self::assertFalse($resolver->supports(Request::create('/', 'GET', [
             'foo' => 'bar',
@@ -69,14 +102,14 @@ public function testThatSupportsMethodCallsExpectedResourceCollectionMethods():
             ->willReturn(false);
 
         $resolver = new EntityValueResolver($resourceCollection);
-        $metadata = new ArgumentMetadata('foo', User::class, false, false, null);
+        $metadata = new ArgumentMetadata('user', User::class, false, false, null);
 
         $resolver->supports(
             Request::create(
                 '/',
                 'GET',
                 [
-                    'foo' => 'bar',
+                    'user' => 'bar',
                 ]
             ),
             $metadata
@@ -94,14 +127,14 @@ public function testThatSupportsMethodReturnFalseWithNotSupportedEntityResource(
             ->willReturn(false);
 
         $resolver = new EntityValueResolver($resourceCollection);
-        $metadata = new ArgumentMetadata('foo', User::class, false, false, null);
+        $metadata = new ArgumentMetadata('user', User::class, false, false, null);
 
         self::assertFalse($resolver->supports(
             Request::create(
                 '/',
                 'GET',
                 [
-                    'foo' => 'bar',
+                    'user' => 'bar',
                 ]
             ),
             $metadata