Как должен выглядеть идеальный контроллер?

Question

[Danbka]

Привет.

Почти во всех мануалах рассказывают, что контроллер должен быть маленьким и заниматься 3 вещами:
- принять request
- передать данные классу-обработчику (сервисному классу, как угодно назовите)
- вернуть response

Я предлагаю абстрагироваться от любых фреймворков и подумать над тем, как лучше всего передавать данные из контроллера в класс-обработчик. Для старта предлагаю написать идеальный контроллер для простой и довольно типичной задачи: обновление name и location пользователя. Вот 2 варианта для критики и исправления:

public function update(Request $request, Response $response, array $args): Response {
    $payload = $request->getParsedBody() ?? [];

    // validate $payload HERE

    // find user, method return DTO object
    $user = $this->userManager->getUserById((int)$args['id']);

    if (isset($payload['name'])) {
        $user->setName($payload['name']);
    }

    if (isset($payload['location'])) {
        $user->setLocation($payload['location']);
    }

    $this->userManager->updateUser($user);

    return $response->withStatus(204);
}

public function update(Request $request, Response $response, array $args): Response {
    $payload = $request->getParsedBody() ?? [];

    // validate $payload HERE

    // find user, method return DTO object or throw exception
    $user = $this->userManager->getUserById((int)$args['id']);

    $this->userManager->updateUser($user, $payload['name'], $payload['location']);

    return $response->withStatus(204);
}

Второй вариант выглядит, на первый взгляд, лучше. Но что если у пользователя будет 10 полей, которые надо обновить? Тогда метод updateUser() будет принимать 11 параметров? - Нехорошо. Да и нужно ли тут использовать DTO в таком случае? Можно просто передать id.

Как бы вы реализовали это?

Comments

[Adamos]

$this->userManager->updateUser((int)$args['id'], $request->getParsedBody() ?? []);

Потому что для контроллера знания о том, какие поля могут быть у юзера и как менеджер получает юзера по айдишнику - это совершенно лишние знания. А вот проверку насчет того, что залогиненный сейчас вообще может править этого юзера, можно и в контроллере прописать. Максимально абстрактную, конечно.

[Danbka]

Adamos, спасибо за ответ. Что вы имеет ввиду под "максимально абстрактной" проверкой прав доступа?

[Adamos]

Danbka, метод типа isEditAllowed($currentUser, $editedUser)- без какой-либо информации о реализации.

[Дмитрий]

Идеальный контроллер, должен просто пробросить реквест нужной моделе, и вернуть нужный ответ. Дополнительная логика приводит к толстому контроллеру

Answer 1

[Виктор Кожухарь]

Если хотите идеал, то он должен соответствовать следующим пунктам:

1. Сериализация/десериализация - это дорогостоящее мероприятие, поэтому оно должно делаться только в двух местах: прямо на входе и прямо на выходе. Вход - это ваш контроллер, Выход - это другой сервис, куда вы передаёте данные, или база данных (тут тоже происходит сериализация, либо явно, либо в ORM). Во всех остальных слоях инфообмен должен совершаться уже при помощи объектов PHP либо нативных типов. Это экономит ресурсы. При передаче между слоями приложения объектов вместо значений либо ассоциативных массивов вы сразу будете видеть очепятки, IDE вам прекрасно поможет при помощи автодополнения, объекты могут иметь какие-то полезные методы.

2. Очень желательно в каждом из слоёв иметь собственный класс, отвечающий за данные. Например, нам в слой API приходит JSON-чик с новым пользователем.
- Сериализуем JSON в DTO UserInAPI, сразу валидируем всё то, что мы можем валидировать без слоя бизнес-логики, и либо отдаём клиенту ошибку, либо передаём сам объект UserInAPI в следующий слой: слой бизнес-логики
- В слое бизнес логики, получаем DTO UserInAPI на входе, преобразуем его в свой бизнес-объект UserInBusiness, валидируем его уже с точки зрения бизнеса, и либо возвращаем ошибку в слой API, либо совершаем над ним действия, и передаём объект класса UserInBusiness в слой работы с базой
- В слое работы с базой данных получаем на входе объект UserInBusiness, преобразуем его уже в сущность базы данных UserInDB, валидируем всё на предмет корректности данных для базы, и либо возвращаем ошибку в бизнес, либо сохраняем сущность класса UserInDB в базу.

Зачем такие сложности, вы спросите? А просто обратите внимание на то, что скорость изменения кода в разных слоях разная.
- API вообще должен меняться раз в сто лет, чтобы не злить клиентов. Поэтому DTO класс UserInAPI будет стабильным и редко будет меняться.
- Бизнес-логика меняется очень часто. У класса UserInBusiness постоянно будут добавляться поля и методы, тут жизнь будет кипеть.
- Слой базы данных будет меняться реже, чем слой бизнеса, но чаще, чем слой API, потому что нам нужны будут новые поля в базе, новые таблицы и связанные таблицы.
- И если мы один тип сущности протащим во все слои, то эта сущность обрастёт таким количеством различной хрени, что нам плохо станет, когда будем на неё смотреть. Либо она обрастёт кучей декораторов в каждом из слоёв. Поэтому лучше всё разделить.

3. Теперь внимание, казалось бы, что мы слишком сильно связываем наши слои, и нижестоящие слои знают что-то о вышестоящих, а это неправильно. Ведь мы передаём объект UserInAPI в слой бизнеса, т.е. слой бизнеса должен уметь работать с этим объектом. И так же слой базы должен уметь работать с объектом бизнеса UserInBusiness. Как же быть? А очень просто. На входе слоёв использовать интерфейсы. Т.е. в слое бизнеса мы будем принимать не сам класс UserInAPI, а объект, имплементирующий интерфейс UserIncoming, который объявим в бизнес слое, и заставим слой API сделать так, чтобы его класс UserInAPI имплементировал этот интерфейс. Таким образом слой бизнеса ничего не будет знать о слое API, а будет ждать на входе данные по контракту, описанному в интерфейсе. Бизнесу плевать на конкретную реализацию, ему нужны только методы getUsername, getEmail из интерфейса. А какой класс ему их предоставит - пофигу. Таким образом мы практически полностью разделяем слои и в два счёта сможем поменять слой API, где у нас HTTP контроллеры, на слой RabbitMQ, SOAP, Grpc и т.д.

Comments

[Дмитрий]

Думаю товарищ ещё больше запутается

[Danbka]

Спасибо за такой детальный ответ, но для моих нужд это слишком сложно :)

[Виктор Кожухарь]

Danbka, Вы просили идеал, я вам дал "идеал")
Задайте себе вопрос: будет ли ваш сервис развиваться?

Если не будет, то пишите максимально просто и сосредоточьтесь только на одном: все важные части кода должны быть покрыты тестами и тесты должно быть легко писать. Уже только одна эта установка заставит вас писать код лучше и подскажет, как сделать контроллер "идеальным" для вашей конкретной цели.

Если же это только начало очень большого проекта, то такое разделение на слои лучше реализовать сразу. Это как мыть руки перед едой. Можно этого и не делать, но есть шанс получить проблемы, которые потом очень долго лечить. А всего-то надо было вымыть руки...

[Danbka]

Виктор Кожухарь, В любом случае спасибо. Я отмечу ваш ответ решением для будущих искателей "приключений". Скажите, пожалуйста, а где-то можно посмотреть на реализацию всего, что вы описали? Хотелось бы посмотреть, как это все в коде выглядит.