Как стать автором
Обновить

Комментарии 15

Не используйте формы для api. Это очень корявый компонент, и предназначен в первую и последнюю очередь для html форм. У нас они тоже используются, и есть понимание, что это было плохое решение.

А можно узнать почему?

За вами постою, интересен их опыт

new Length(['max' => $max]),

Просто напонмю что поддержка именованых аргументов в PHP появилась 3 года назад и была завезена в симфонийские валидаторы еще с 5.4 версии.

Слишком многословно. Часть бизнес-логики в контроллере, часть в обработчике SettingsMessage. Много трейтов и родительских классов.
Можно сделать так.

class UserController extends AbstractController
{
  #[Route("/user/settings/update", methods: ['POST'])]
  public function update(User $user, UserSettingsDto $data): PrivateUserView
  {
    $user = $this->userService->update($user, $data);
    
    return new PrivateUserView($user);
  }
}

class UserService
{
  public function update(User $user, UserSettingsDto $data): User
  {
    $this->em->beginTransaction();
    try {
      $this->em->lock($user, LockMode::PESSIMISTIC_WRITE);
      $this->em->refresh($user);

      $user->updateSettings($data);
      
      $this->em->persist($user);
      $this->em->flush();

      $this->bus->dispatch(new SettingsMessage($user));

      $this->em->commit();
      
      return $user;
    } catch (\Throwable $e) {
      $this->logger->error('Error occurred while update user settings', ['error' => $e]);
      $this->em->rollback();
      
      throw $e;
    }
  }
}

class UserSettingsDto {
  #[Assert\Length(max: 255)]
  #[Assert\Expression(
    expression: "this.firstName !== '' || this.lastName != ''",
    message: 'First and last names must not be simultaneously empty'
  )]
  public string|null $firstName;
  
  #[Assert\Length(max: 255)]
  public string|null $lastName;
    
  #[Assert\Length(max: 255)]
  #[Assert\Expression(
    expression: "this.lastName === '' || this.value != ''",
    message: 'Middle name must not be empty'
  )]
  public string|null $middleName;
}

Превращение request в UserSettingsDto и PrivateUserView в response можно сделать через рефлексию и события Symfony. Это проще, чем возиться с Symfony Forms.

Чтобы не писать везде string|null, когда null не нужен, можно сделать свой компонент валидации, который будет валидировать array по правилам из dto, и только после успешной валидации загружать данные в dto.

Вместо вызова dispatch() можно вызывать логику напрямую. Обычно она требует зависимости, и становится понятно, что тащить их все в контроллер не стоит. Поэтому и нужен сервис для бизнес-логики. Эту же логику можно потом вызвать из какой-нибудь фоновой задачи. Не пробрасывать же веб-контроллер в консольную задачу.

С LockMode::PESSIMISTIC_WRITE есть хитрый момент, что до него использовать объект user в логике все равно нельзя. Например, принимать решение в зависимости от статуса, делать ли какие-то действия. Сначала надо блокировать, чтобы поля не поменялись в другом процессе, потом делать проверки. Иначе может получиться, что удаленные пользователи создают заказы, какие-то данные отправляются в другую систему 2 раза из-за отсутствия статуса "отправлено", и т.д.

валидация/хранение правил валидации на уровне DTO врядли хорошая (точно плохая) идея

Почему? Правила валидации это требования к нетипизированным входным данным - какие поля и значения там должны быть. DTO задает список полей, поэтому там должны быть и правила для этих полей. Иначе их придется дублировать, что и происходит в form builder. После валидации нетипизированный массив превращается в типизированное DTO с известной структурой и диапазонами значениями, и можно полагаться на это в дальнейшем коде.

потому что одна и та же модель/сущность может быть создана из десятка разных dto, и иметь при этом 90% одинаковых полей, правила валидации которых вы, приняв решение возложить валидацию на DTO, будете дублировать (user.createFromRegistrationDTO, user.createFromMailSubscribeDTO, user.createFrom...).

dto отвечает за передачу данных и не имеет права заниматься проверкой их корректности.

модель/сущность

Ага, вот дело как раз в том, что валидировать надо не модель, а параметры действия.
Вот есть у нас фильтр по сущностям и действие "Поиск", там есть 2 поля "Дата создания: От" и "До", API должно проверять, что значение это дата. А в сущности таких полей нет, там только 1 поле "Дата создания". Или галочка "Показать только с изображениями". Или галочка "Уведомить пользователя" при редактировании статьи модератором. Или когда в форме создания сущности делают только 1 поле "Название", а в форме редактирования штук 30 полей.

user.createFromRegistrationDTO

Сущность не может создавать сама себя, и тем более содержать логику валидации входных параметров для всех возможных сценариев. Это именованный конструктор, аналогичный вызову new в вызывающем коде, он может только копировать данные из DTO. Поэтому DTO, которое передается в конструктор, уже должно быть провалидировано на момент этого вызова.

dto отвечает за передачу данных и не имеет права заниматься проверкой их корректности

Проверкой корректности занимается вызывающий код, который содержит вызов валидатора. Валидатор возвращает результат валидации, вызывающий код его проверяет. DTO содержит лишь список правил и само ничего не делает.

Ага, вот дело как раз в том, что валидировать надо не модель, а параметры действия.

именно этим путём вы будете дублировать логику/правила валидации во все места, откуда данные попадают в модель.

Это нарушение DRY, со всеми присущими последствиями.

сегодня модель создаётся из 3 мест в веб интерфейсе и в 1 консольной команде. завтра таких мест 13, послезавтра - 33. а потом приходит от бизнеса новая вводная, и вот вы уже побежали менять атрибуты в dto'шках/регулярки во всех всех 33 контроллерах/дтошках/гдевытамхранитеконстрэйнты.

альтернативы этому:
а) сделать так, чтобы модель не давала записать в себя невалидные данные в принципе (кидать исключение из setter'а)
б) кидать исключение при первой же попытке использования с невалидными данными (где-то в нужный вам момент вызовете $validator->validate($entity))
в) вводить value object'ы, и валидацию (по крайней мере, техническую, про бизнесовую давайте не будем даже тут говорить) проводить на уровне создания value object'а, не доводя до модели/entity в принципе.

в таком случае, если у вас будет 33 места откуда данные попадают в модель, и появится изменение требований валидации этих данных, вы логику валидации измените в одном месте, а не в 33.

это - безусловное приемущество описываемых мной подходов.

именно этим путём вы будете дублировать логику/правила валидации во все места, откуда данные попадают в модель.

Вот я как раз и говорю, что во всех местах могут быть разные правила, и бизнес может менять их независимо. При создании товара поле "Описание" необязательное, при первом редактировании обязательное. Или при редактировании тоже необязательное, но должно быть заполнено при изменении статуса на "Опубликован". Как вы это сделаете через правило в поле сущности?

сегодня модель создаётся из 3 мест в веб интерфейсе и в 1 консольной команде

Обычно большинство полей меняется только в 1-2 местах.
Если их больше, то во всех 3 местах в интерфейсе разный набор полей и разные правила их валидации, а в консольной команде часть полей берется из базы и валидировать их не надо.
Поля в DTO должны соответствовать полям в интерфейсе пользователя, и валидация часто бывает зависимая от других полей. Например, может быть требование возвращать ошибку, если в фильтре значение поля "Дата от" больше "Дата до". Как вы сделаете эту валидацию через сущность?

завтра таких мест 13, послезавтра - 33. а потом приходит от бизнеса новая вводная
вы логику валидации измените в одном месте, а не в 33

Да-да, завтра от бизнеса приходит новая вводная, что в месте номер 17 надо разрешить не указывать поле, а во всех остальных местах оставить обязательным.

Это нарушение DRY

Нет, это разные сценарии, а не повторение одного, они могут меняться независимо. Во всех сущностях есть поле id, но это не значит, что это повторение.

сделать так, чтобы модель не давала записать в себя невалидные данные в принципе (кидать исключение из setter'а)

Тогда модель превращается в God-object, в котором есть всё.
Сеттер не может знать, из какого сценария его вызвали. В одних сценариях поле может быть необязательным, а в других обязательным. Например, некоторые поля могут заполняться только топ-менеджером.

вводить value object'ы, и валидацию проводить на уровне создания value object'а

Вот DTO для входных данных это и есть такой value object. DTO не имеет идентификатора, значит он является value object.

Вот я как раз и говорю, что во всех местах могут быть разные правила, и бизнес может менять их независимо. При создании товара поле "Описание" необязательное, при первом редактировании обязательное. Или при редактировании тоже необязательное, но должно быть заполнено при изменении статуса на "Опубликован". Как вы это сделаете через правило в поле сущности?

Стоит разобраться, и преобразовать набор требований в бизнес модель, и управлять ими на уровне модели, а не на уровне DTO и контроллеров.

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

Как вы это сделаете через правило в поле сущности?

Если их действительно невозможно рассмотреть как отдельные сущности (то есть мыимеем дело с инвариантами), то поступлю следующим образом.

Увижу, что мы имеем дело с ситуацией, когда у нас в зависимости от контекста меняются правила валидации объекта.
Введу понятие контекста валидации.

Сделаю сущность, в которой все поля не обязательны, повешу constraint'ы, которые будут содержать правила для каждого сценария (какие поля должны быть null, какие нет), и буду по необходимости руками дергать валидатор, передавая ему нужный контекст.

Набором ограничений для каждого инварианта буду управлять централированно в одном месте (из конфига в идеальном варианте), а DTO буду использовать по назначению, для передачи данных.

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

Отсутствие нестандартных решений ("а вот у нас в проекте в объектах для передачи данных... валидация!") упростит поддержку и ввод новых сотрудников в проект.

Возможно, даже помогу бизнесу понять че они делают, если требования хаотичны, а они сами формализовать не в состоянии.

Тогда модель превращается в God-object, в котором есть всё.
Вот DTO для входных данных это и есть такой value object.

А ваша DTO не превращается? :)

В этом и ошибка, потому что это уже и не DTO (потому что наделен доп. функцональностью), и ещё не valueObject (потому что не самостоятелен), при этом собирает все негативные черты дублирования кода (копируем правила валидации из одного в другой).

Стоит разобраться, и преобразовать набор требований в бизнес модель

Ну вот из-за этого и начинаются проблемы в поддержке, когда бизнес-требования выражены в коде не явно, а преобразованы неизвестно во что.
Бизнес-требования я уже описал, на вопрос вы не ответили.

Иначе вы увязаете в наборе сомнительного качества скриптов

Нет, я проверял, работает нормально.

А ваша DTO не превращается? :)

Конечно нет, я не делаю 1 DTO на модель, у меня разные DTO на разные сценарии. Отдельная форма в интерфейсе - отдельное DTO с правилами ее валидации. Бизнес решил убрать какой-то сценарий - удаляем DTO и код, который с ним работает.

копируем правила валидации из одного в другой

Я уже несколько раз написал, что ничего не копируется. Если у вас большинство DTO пересекаются на 90%, это ошибка проектирования. Бизнесу куча похожих форм в интерфейсе тоже не нужна, он сам в них будет путаться. Если же он требует, значит так и надо сделать. Указание #[Assert\Email] в 2 местах это не копирование, а аналог вызова функции.

Тут кстати непонятно, то есть вы предлагаете сначала в бизнес-сущность загрузить некорректные данные, то есть перевести ее в невалидное состояние, а потом ее валидировать? С моей точки зрения такое допускать вообще нельзя. Валидация делается не для полей модели, а для входных данных. Они могут вообще не сохраняться ни в какую модель, а влиять только на выполнение запроса (галочка "Отправить уведомление").

Логика в контроллере звучит очень гибко а главное тестируемо

Зарегистрируйтесь на Хабре, чтобы оставить комментарий

Публикации

Истории