Как обойти дублирование кода в контроллере Symfony2?

Question

[Александр Евгеньевич]

Здравствуйте.
Вопрос следующий. Есть ссылки для обращения к ресурсам. Ресурсы имеют некую иерархию, например, ссылка будет иметь следующий вид:

/cart/{cartId}/product/{productId}

И соответственно, у нас есть методы GET, POST, PUT, PATCH, DELETE. При каждом запросе к Action-ам, нам необходимо проверять есть ли корзина с таким ID, есть ли продукт с таким ID, имею ли я право работать с продуктом и т.д., а при несоответствии условию кидается HTTPException.
Вот и назрел вопрос, чтобы не дублировать код по Action-методам контроллеров, каким образом лучше сделать такую предпроверку типичных условий ?
Были мысли вынести все в обычный менеджер, но не думаю, что бросание HTTPException-ов менеджером хорошая идея. Также думал вынести такую проверку в RequestListener, но пока смутно представляю, как такое будет работать. На данный момент типовые проверки вынес в трейты, а контроллер имплементировал от интерфейса с методами, как в трейте.
Есть ли какое-то наиболее подходящее решение данной проблемы ?

Answer 1

[Сергей Протько]

Были мысли вынести все в обычный менеджер, но не думаю, что бросание HTTPException-ов менеджером хорошая идея.

Да, потому есть такая штука у FosRestBundle как ErrorController который умеет конвертить обычные исключения в корректный респонс со своими статус кодами.

Вынести все эти штуки в ваш сервис-менеджер, что бы вся бизнес логика была рядом, это здравая мысль.

Comments

[Александр Евгеньевич]

Значит типовые задачи выношу в менеджер, эти типовые задачи обеспечиваю своими исключениями, которые перехватываются в RequestSubscriber-e, который в свою очередь их преобразует в корректный Response. Я так понял ?

[Сергей Протько]

Александр Евгеньевич: да, как-то так да. У Symfony есть событие - kernel.exception, там это дело можно сконвертить в HTTP ответ нужный.

[voronkovich]

Александр Евгеньевич: Будет лучше если методы вашего сервиса будут возвращать true/false, а не бросать исключения. Эти методы вызывать в контроллерах, и уже там бросать соответствующие http-исключения.
Также можно создать слушатель события kernel.request. Узнать текущий маршрут и его параметры можно через аттрибуты _route и _route_params экземпляра запроса: $request->attributes->get ('_route_params'). Далее осуществить все проверки, используя методы указанного выше сервиса и бросить соответствующее http-исключение или установить Response.
Конвертировать исключения при помощи kernel.exception, в данном случае мне кажется излишним.

[Сергей Протько]

voronkovich: чем это лучше? Как раз таки лучше что бы методы бросали исключения там где происходит исключительная ситуация, а уж по kernel.exception конвертить исключения приложения в HTTP ответы.

И это не лишнее, зато код становится проще и более логичен.

[Александр Евгеньевич]

voronkovich: проблема в том, что при true\false я не смогу получить точной причины отказа. То ли я не смог создать запись из-за прав, то ли из-за того, что что-то было не найдено и т.д. Если делать через проверку маршрута - это тоже проблема, потому что, вы же сами понимаете, маршрут может поменяться, а дублировать все роуты в сервисе, не кажется мне хорошим делом.
Еще мне кажется вместо аннотаций (вы писали ниже), можно было бы добавить параметром в роуте, путь к классу, который будет проверять значения, перед передачей их далее, в action. Хз, в общем, пока буду делать через сервисы.

[voronkovich]

Александр Евгеньевич: Вместо роутов можно использовать аттрибут _controller, чтобы узнать какой контроллер будет обрабатывать запрос.

[voronkovich]

Сергей Протько: я имел ввиду что исходя из вопроса, человеку нужно просто сделать три проверки: есть ли корзина с таким id, есть ли товар с таким id, может ли пользователь его менять. Это делается легко через аннотации. Исполльзование событий (что в моем случае с kernel.request, что в вашем с kernel.exception) - это overkill/overheadи/излишне

[Сергей Протько]

voronkovich: то что вы предлагаете это какой-то неявный и неочевидный треш, уж извините. И да, я считаю именно это оверхэдом, оверкилом и т.д. поддерживать такой код я не хочу.

[Сергей Протько]

voronkovich: Бизнес логика (а эти ограничения являются частью бизнес логики) должна быть в сервисах и только там. И кидаться исключениями. В контексте задачи не зная нюансов это самый удобный вариант. А если еще FosRestBundle поставить то и листенер kernel.exception писать не надо (если я правильно понимаю то у автора апишка).

[Александр Евгеньевич]

Сергей Протько: апишка, но FosRestBundle ставиться не будет )

[Сергей Протько]

Александр Евгеньевич: да как бы листенер написать даже быстрее наверное будет, чем поставить через composer + почитать доку.

[voronkovich]

Сергей Протько: и все таки я не могу согласиться что авторизация является частью бизнес-логики, как и проверка наличия в БД объекта с определенным идентификатором. Вероятно мы по-разному понимаем этот термин.

[Сергей Протько]

voronkovich: наличие в базе данных заменяем на "наличие в репозитории", никакой информации о инфраструктуре, просто "объект существует" в терминах юз кейсов.

А авторизацию заменяем на проверку прав. Авторизацию и так хэндлит фронт контроллер симфони.

[voronkovich]

Сергей Протько: с таким подходом согласен :). В таком виде ваше решение вполне уместно.

Answer 2

[shagguboy]

https://knpuniversity.com/screencast/symfony-journ...

Comments

[Сергей Протько]

Ну вы бы хоть пояснили мысль. Я согласен с вами если вы предлагали таким образом конвертить исключения в нормальные респонсы.

[Александр Евгеньевич]

не понял, это вы к чему ?

[shagguboy]

Сергей Протько: это как делать листенеры

[Александр Евгеньевич]

shagguboy: насчет Listenera-a я имел ввиду, что не представляю, как организовать в нем логику работы с типовыми запросами по определенным урлам. Вы целиком вопрос читали ?

Answer 3

[voronkovich]

Ваши проверки можно вынести в аннотации.
Для проверки, существует ли сущность с нужным id, можно использовать аннотацию @ParamConverter
Для проверки прав доступа можно использовать @Security
Все исключения будут бросаться автоматически.