Как улучшить архитектуру и убрать дублирование?

Вкратце, есть пользователи, у них есть роли и они привязаны к какому-то воркспейсу. На основании этих двух условий нужно проводить те или иные действия. Первая версия контроллера
<?php

namespace App\Controller;

use App\Entity\User;
use App\Service\UserService;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Authorization\AuthorizationCheckerInterface;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\Security;
use Twig\Environment;

class UserController
{
    private $userService;

    /**
     * @var Environment
     */
    private $twig;

    /**
     * @var AuthorizationCheckerInterface
     */
    private $authorizationChecker;
    /**
     * @var Security
     */
    private $security;

    public function __construct(
        UserService $userService,
        Environment $twig,
        AuthorizationCheckerInterface $authorizationChecker,
        Security $security
    ) {
        $this->userService = $userService;
        $this->twig = $twig;
        $this->authorizationChecker = $authorizationChecker;
        $this->security = $security;
    }

    public function index(): Response
    {
        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_SUPER_ADMIN)
        ) {
            $users = $this->userService->getAll();

            $content = $this->twig->render('admin/users.html.twig', [
                'users' => $users
            ]);

            return new Response($content);
        }

        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_CUSTOMER_ADMIN)
        ) {
            /** @var User $user */
            $user = $this->security->getUser();

            $users = $this
                ->userService
                ->findByRoleAndWorkspaceId(
                    User::ROLE_TEAM_MEMBER,
                    $user->getWorkspace()->getId()
                );

            $content = $this->twig->render('user/users.html.twig', [
                'users' => $users
            ]);

            return new Response($content);
        }

        throw new AccessDeniedException();
    }

    public function read(string $id): Response
    {
        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_SUPER_ADMIN)
        ) {
            $user = $this->userService->getById($id);

            $content = $this->twig->render('admin/user.html.twig', [
                'user' => $user
            ]);

            return new Response($content);
        }

        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_CUSTOMER_ADMIN)
        ) {
            $user = $this->userService->getByIdAndWorkspaceId($id);

            $content = $this->twig->render('user/user.html.twig', [
                'user' => $user
            ]);

            return new Response($content);
        }

        throw new AccessDeniedException();
    }
}

Добавятся еще другие методы crud и в них будет тоже проверка похожая данной и вызовы разных методов из сервиса.
Я не уверен в правильности данного решения

Я рассматриваю вариант с переносом проверки роли пользователя в сервис и вызовом соответствующего метода, а методы контроллера станут выглядеть примерно так

public function index(): Response
    {
        $users = $this->userService->getAll();

        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_SUPER_ADMIN)
        ) {
            $content = $this->twig->render('admin/users.html.twig', [
                'users' => $users
            ]);

            return new Response($content);
        }

        if (
            $this->authorizationChecker
                ->isGranted(User::ROLE_CUSTOMER_ADMIN)
        ) {
            $content = $this->twig->render('user/users.html.twig', [
                'users' => $users
            ]);

            return new Response($content);
        }

        throw new AccessDeniedException();
    }


Какой вариант более предпочтительный и какие проблемы могут возникнуть?
  • Вопрос задан
  • 444 просмотра
Решения вопроса 1
BoShurik
@BoShurik Куратор тега Symfony
Symfony developer
Я бы сделал так. Тем более, если там добавятся еще методы, то вы утоните в if-ах при вашем подходе

<?php

namespace App\Controller;

use App\Entity\User;
use App\Service\UserService;
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Security\Core\Exception\AccessDeniedException;
use Symfony\Component\Security\Core\Security;
use Twig\Environment;

interface AdminControllerInterface
{
    public function index(): Response;

    public function read(string $id): Response;
}

class UserController implements AdminControllerInterface
{
    /**
     * Security - объект-helper, он содержит в себе AuthorizationCheckerInterface
     *
     * @var Security
     */
    private $security;

    /**
     * @var ServiceLocator
     */
    private $controllerLocators;

    public function __construct(Security $security, ServiceLocator $controllerLocators)
    {
        $this->security = $security;
        $this->controllerLocators = $controllerLocators;
    }

    public function index(): Response
    {
        $role = $this->getRole();

        if (!$this->controllerLocators->has($role)) {
            throw new AccessDeniedException();
        }

        /** @var AdminControllerInterface $controller */
        $controller = $this->controllerLocators->get($role);

        return $controller->index();
    }

    public function read(string $id): Response
    {
        $role = $this->getRole();

        if (!$this->controllerLocators->has($role)) {
            throw new AccessDeniedException();
        }

        /** @var AdminControllerInterface $controller */
        $controller = $this->controllerLocators->get($role);

        return $controller->read($id);
    }

    private function getRole(): string
    {
        // Для простоты представим, что роль у пользователя одна. Иначе искать среди ролей
        // ROLE_SUPER_ADMIN и ROLE_CUSTOMER_ADMIN и возвращаем с соответствующим приоритетом
        return current($this->security->getToken()->getRoles());
    }
}

class SuperAdminController implements AdminControllerInterface
{
    private $userService;

    /**
     * @var Environment
     */
    private $twig;

    public function __construct(
        UserService $userService,
        Environment $twig
    ) {
        $this->userService = $userService;
        $this->twig = $twig;
    }

    public function index(): Response
    {
        $users = $this->userService->getAll();

        return new Response($this->twig->render('admin/users.html.twig', [
            'users' => $users
        ]));
    }

    public function read(string $id): Response
    {
        $user = $this->userService->getById($id);

        return new Response($this->twig->render('admin/user.html.twig', [
            'user' => $user
        ]));
    }
}

class CustomerAdminController implements AdminControllerInterface
{
    private $userService;

    /**
     * @var Security
     */
    private $security;

    /**
     * @var Environment
     */
    private $twig;

    public function __construct(
        UserService $userService,
        Environment $twig
    ) {
        $this->userService = $userService;
        $this->twig = $twig;
    }

    public function index(): Response
    {
        /** @var User $user */
        $user = $this->security->getUser();

        $users = $this->userService->findByRoleAndWorkspaceId(
            User::ROLE_TEAM_MEMBER,
            $user->getWorkspace()->getId()
        );

        return new Response($this->twig->render('user/users.html.twig', [
            'users' => $users
        ]));
    }

    public function read(string $id): Response
    {
        $user = $this->userService->getByIdAndWorkspaceId($id);

        return new Response($this->twig->render('user/user.html.twig', [
            'user' => $user
        ]));
    }
}


app.admin_controller_locator:
    class: Symfony\Component\DependencyInjection\ServiceLocator
    arguments:
        -
            ROLE_SUPER_ADMIN: '@App\Controller\SuperAdminController'
            ROLE_CUSTOMER_ADMIN: '@App\Controller\CustomerAdminController'
    tags: ['container.service_locator']
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы