Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Рефакторинг сервиса валидации #10

Closed
foobar1643 opened this issue Apr 30, 2017 · 8 comments
Closed

Рефакторинг сервиса валидации #10

foobar1643 opened this issue Apr 30, 2017 · 8 comments

Comments

@foobar1643
Copy link
Collaborator

Сейчас вся валидация происходит прямо в сервисах, сначала идут проверки на существование элемента в теле POST запроса, а затем статическим методом происходит валидация типов. Это неправильно и сложно тестировать. Нужно сделать отдельный сервис валидации:

  • Сервис валидации будет принимать массив, значения в котором будут обязательными параметрами, которые нужно валидировать.
  • У сервиса валидации будет метод addValidationRule(string $field, string $ruleName), который будет добавлять указанную функцию (правило валидации) к указанному полю в входящем массиве.
  • У сервиса валидации будет метод validateFields(array $fields), который будет принимать аргументом массив полей, которые нужно валидировать. Массив всегда имеет формат [имя_поля => значение], при чем это необязательно может быть массив сформированный из объекта PSR-7 Request.
  • Сама валидация происходит в два этапа. В validateFields() мы циклом проходимся по сохраненным ранее обязательным параметрам, и проверяем есть ли такой ключ в массиве $fields, если ключ есть, проверяем не пустой ли он (обрезав пробелы через trim()).
  • Затем, если ключ в массиве есть, и он не пустой, мы смотрим в массив правил валидации для этого правила, если там есть какие-нибудь функции - вызываем их, если какая-то возвращает false - добавляем ошибку в массив ошибок.

Когда цикл заканчивается, возвращаем массив вида [имя_поля => сообщение_об_ошибке], или пустой массив если все хорошо.

@someApprentice отпиши что думаешь, не слишком ли переусложненно?

@someApprentice
Copy link
Collaborator

@foobar1643 Я думаю, что понятие "переусложненно" не должно препятствовать улучшению структуры. Как ты сказал, что это неправильно и сложно тестировать, значит нужно сделать отдельный сервис валидации. К тому же, что-то подобное я реализовывал когда писал студентов, и если бы я не писал 'casual' код, я бы выбрал именно такой способ.

@foobar1643 foobar1643 added this to the Версия 1.0.0 milestone Jun 29, 2017
@izac1
Copy link
Collaborator

izac1 commented Aug 8, 2017

Можно примерно показать пример вызова функций.

@kubk
Copy link
Contributor

kubk commented Aug 8, 2017

Есть же готовые валидаторы, например symfony/validator умеет валидировать массивы, объекты, вложенные структуры и так далее.
Пример использования этого валидатора вне symfony: http://symfony.com/doc/current/components/validator.html

@kubk
Copy link
Contributor

kubk commented Aug 8, 2017

Или вот попроще: https://github.com/alexgarrett/violin

@izac1
Copy link
Collaborator

izac1 commented Aug 8, 2017

@kubk С одной стороны ты прав зачем плодить в 100 раз велосипед с другой хочется написать самому чтоб получить опыт @foobar1643 что скажешь ?

@foobar1643
Copy link
Collaborator Author

foobar1643 commented Aug 9, 2017

Тут все зависит сколько времени мы готовы на это потратить. Если делать такой валидатор как я описал в тут - при полном понимании задачи это не должно занять больше чем пол дня. С другой стороны, тогда нужно будет тратить время на ревью и правки по коду, и отладку сервиса под наши задачи. Ревью результатов работы желательно чтобы проводили все люди, которые работают над проектом, для того чтобы все имели понимание о том как работает данный класс и как его использовать. Далее также нужно будет тратить время на правки и повторный разбор и обсуждение этих правок. Я не против написать свой валидатор, но в данном случае это займет слишком много времени и это того не стоит (это исключительно мое мнение, потому что свободного времени у меня не очень много).

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

@izac1 если ты все таки хочешь написать свой чтобы получить опыт - это тоже не проблема, только наверное нужно это согласовать как-нибудь со всеми.

@kubk
Copy link
Contributor

kubk commented Aug 11, 2017

Стоит отметить, что некоторым методам явно не место в классе Validator.
Например метод validateThreadSubject логичнее поместить в сервис для работы с тредами. Этот метод используется только в одном месте, вот тут, это низкоуровневые кишки, которые нужно инкупсулировать внутри класса для работы с тредами, а не делать глобально доступной статикой. Вот тут похожая ситуация, никаких глобальных функций валидации нет.
Часть методов вроде validateThreadLink, validateChainLink, validateRegistrationLink, validateLogoutLink вообще нигде не используется. И зачем валидировать ссылки регистрации и логаута?
Методы isPasswordsEquals и validateToken напрашиваются в класс аутентификации.

@kubk
Copy link
Contributor

kubk commented Oct 5, 2018

Закрываю, чтобы не сбивать новых людей с толку. В слаке обсуждали возможный переход на SF4, который по своей сути тоже микрофреймворк, поэтому будем использовать компоненты симфони.

@kubk kubk closed this as completed Oct 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants