Search
Write a publication
Pull to refresh
30
0
Пётр Грибанов @ghost404

Symfony professional developer

Send message
Данные должны обрабатываться там, где для этого достаточно данных.

Вот об этом я и говорю. Функции changePassword в вашей первой реализации не достаточно данных для того чтобы изменить пароль. Она вынуждена использовать зависимости для хеширования пароля.


Такая же ситуация с файлами. Например картинка/обложка к CD это такое же свойство сущности как название альбома, группа, список песен и т. д. И исходя из вашей логики сущность должна инкапсулировать взаимодействие со своей обложкой, но в этом случае ей придется работать с фс, что означает очередные зависимости.


Приведу пример:
Есть 2 сущности с картинками:


  • обложка альбома
  • новость с картинкой на сайте группы.

Загружаемые картинки попадают во временную папку /upload/. После привязки к сущности они должны перемещаться каждая в свою папку:


  • обложка альбома — /image/album/{date}/cover/
  • новость с картинкой — /image/news/{date}/cover/

{date} это Y/m от даты создания сущности


Кто должен заниматься перемещением картинок? Напоминаю что только сущность знает путь загрузки картинки относительно корня проекта, но она не знает путь к корню проекта.


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


И это еще не все. А как на счет замены ссылки на файл другой ссылкой на файл. Привязка к сущности другого файла. Задача в принципе схожая удалению, но бизнеслогика находится в сущности. При смене ссылки нужно удалить старый файл. Если же мы укажем сущности ссылку на новый файл, а потом укажем ссылку на старый файл. Оба раза мы изменили ссылку на файл, но удалить мы должны только новый файл.


Суть моих изисканий в том что сущность это простой объект (не сервис) и она не должна использовать ни какие зависимости (может в доктрине и против сеттеров, но и сервисом они сущности не сделали).
Если появляется необходимость использовать зависимости, в той или иной форме, то нужно создавать еще один слой абстракции с требуемыми зависимостями, а не впихивать все в один класс.

В таком случае сущность превращается в помойку.


  • Сущность хеширует пароль;
  • Сущность загружает файл;
  • Сущность изменяет свои загруженные файлы;
  • Сущность удаляет свои загруженные файлы;
  • Сущность сохраняет последнего пользователя редактирующего ее;
  • Сущность сохраняет редактора который заапрувил ее;
  • Сущность изменяет свои даты в соответствии с часовым поясом пользователя полученным из запроса, которого кстати нет в консолм;
  • Сущность привязывает к себе соседние сущности в цепочке сущностей.

Не многова ли зависимостей и ответственности у сущности? Может стоит делегировать часть задач?

Интересный взгляд на сеттеры. Спасибо. Хотя с вашим решением не соглашусь.


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

ни кто не мешает в коде проекта не хешировать пароль


$user->changePassword('123', function($password) {
    return $password;
});

или использовать разные алгоритмы хеширования


$user->changePassword('123', function($password) {
    return password_hash($password, PASSWORD_DEFAULT);
});
$user->changePassword('123', 'md5');

Конечно за такое надо отрывать руки, но речь не об этом.
Лучше использовать классический сеттер и при сохранении сущности хэшировать пароль


function setPassword(string $password) : User
{
    $this->password = $password;
    $this->password_changed = true;

    return $this;
}

function isPasswordChanged() : bool
{
    return $this->password_changed;
}

и обработчик события


if ($user->isPasswordChanged()) {
    // изменяем через сеттер или напрямую пишем в свойство
    $user->setPassword($this->hasher->hash($user->getPassword()));
}

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


Ну и отказ от классических сеттеров может вызвать определённые проблемы в использовании стандартных пакетов.
Например SonataAdminBundle использует PropertyAccessor для изменения полей сущности.
То есть отказ от классических сеттеров потребует написания костылей для SonataAdminBundle или полный отказ от этого бандла и самостоятельное написание аналога с блекджеком и ...


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

Маленький офтоп.
Правильно ли я понимаю что автор уже живет в Германии? Какая там сейчас ситуация с мигрантами, да и вообще в стране?


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

Окей. А теперь представим стандартную сетуацию. Изменился phpunit.xml.dist. У всех пользователей он обновится и тесты будут отрабатывать по новому, а у тех кто использует phpunit.xml, тесты будут работать по старому. А по скольку файл phpunit.xml.dist меняется редко, то это изменение может пройти очень незаметно если явно не сказать всем разработчикам. Я конечно утрирую, но такой кейс не стоит исключать.


Ну и для облегчения запуска из консоли со своими флагами, не проще ли написать bash скрипт? Да и вводить команду в консоли целиком не обязательно. Ctrl+R ни кто не отменял. Команду на code coverage я никогда не ввожу сам

Я честно вообще не понял какой смысл копировать phpunit.xml и запихивать в gitignore. Какой смысл делать свой, эксклюзивный конфиг для тестов которые должны у всех отрабатывать одинаково?

и еще. В PHPStorm с установленным плагином Symfony можно указать в начале шаблона комментарий


{# @controller BloggerBlogBundle:Page:contact #}

и тогда в шаблоне автокомплит будет видеть все параметры передаваемые контроллером и по щелчку Ctrl+Mouse Left Click мы перейдем в контроллер, а при аналогичном щелчке по ссылке на шаблон перейдем в сам шаблон


return $this->render('BloggerBlogBundle:Page:contact.html.twig', array(
    'form' => $form->createView()
));
Несколько замечаний:
  • Symfony best practices рекомендует использовать аннотации для описания роутов.
  • Валидаторы сущности лучше тоже делать через аннотации. По крайней мере это наглядней.
  • Хоть это и не описано в best practices, но если вы протестируете свой проект в SensioLabsInsight, то выясните что все формы должны находится в директории src/Blogger/BlogBundle/Form/Type/.
  • Параметры приложения принято описывать в файле parameters.yml, а не config.yml.
  • Конфиги бандла принято подглючать через DI extension, а не через app/config/config.yml.

Symfony2 автозагрузчик будет искать необходимые файлы в директории src


В Symfony нет автозагрузчика. Symfony использует для автозагрузки Composer.
Он использует расширение .txt.twig. Первой частью расширения, .txt определяется формат файла для генерации. Общие форматы включают, .txt, .html, .css, .js, XML и .json. В последней части расширения определяет, какой движок шаблона использовать, в данном случае Twig. Расширение .php использовало бы PHP для отображения шаблона.


Расширение .html.twig это только рекомендации к наименованию расширения. Расширение файлов ни как не влияет на работу шаблонизаторов. Можно указать любое расширение, хоть .foo.bar.

Вместо
$this->container->getParameter('blogger_blog.emails.contact_email')


можно использовать getParameter
$this->getParameter('blogger_blog.emails.contact_email')


Вместо
$this->get('session')->getFlashBag()->add('blogger-notice', 'Your contact enquiry was successfully sent. Thank you!');


можно использовать addFlash
$this->addFlash('blogger-notice', 'Your contact enquiry was successfully sent. Thank you!');


Несколько мелких замечаний по оформлению:
  • В сущностях рекомендуется описывать реальные типы, а не писать везде mixed.
  • В сущностях рекомендуется указывать значение по умолчанию. Например email всегда должен быть строкой и не когда не должен становится null-ом.
  • В setter-ах рекомендуется возвращать ссылку на текущий объект $this чтобы можно было использовать цепочки вызовов.
  • В классе формы рекомендуется использовать цепочку вызовов для конфигурирования формы.
  • В форме метод configureOptions не обязательный и если он пустой, то лучше его вообще не указывать.


PS: В целом статья хорошая. Спасибо за проделанную работу. Хотя лучше писать качественный код чтобы новички не перенимали плохое.
PSS: Не сочтите за рекламу. Несколько полезных сервисов для тестирования проекта:

по оформлению форм в шаблоне, правильней всего так:


{{ form_start(form, {action: path('BloggerBlogBundle_contact'), attr: {class: 'blogger'}}) }}
{{ form_widget(form) }}
<button type="submit">Submit</button>
{{ form_end(form) }}

method у форм по умолчанию POST и поэтому его явно указывать не нужно.


Ну и обработку формы можно оптимизировать


$form = $this
    ->createForm(EnquiryType::class, $enquiry)
    ->handleRequest($request);

if ($form->isValid()) {
    // отправка письма
}

Нет необходимости проверять запрос на POST, это сделает автоматически компонент форм

Я ожидал этот комментарий. Есть одно но. Мы используем регклярку для проверки email введенного пользователем, а он не будет переводить свой интернациональный email в punycode только чтоб угодить вам
Добавлю свои 5 копеек
  1. Самая первая регулярка из статьи посчитает валидным email: #@*%ab
  2. В локальной части email могут быть русские буквы. Встречал компании в которых все корпоративные email были такими.
  3. Домен верхнего уровня может содержать цифры (.i2p) и русские буквы (.рф)
  4. Домен верхнего уровня может быть длиннее 5 символов. Пример: .example, .localhost (RFC2606) это конечно не рабочие домены, но все равно домены.

Желающие могут полистать RFC4185
А я и не говорил что валидация должна выполнятся в entity. Я говорю что валидация и entity это связанные вещи. Бизнес сущность, которой является entity, определяет правила собственной валидации. И я не считаю правельным описывать правила валидации сущности вне её контекста. Об этом же говорит best practice от Symfony
Некоторое дублирование имеет место между «парными» операциями типа «создать/изменить»

Если есть дублирование, то может стоит от него избавится. Вы так не считаете?
Есть ORM-сущности, которые по сути описывают схему данных и являются прямым отображением ваших таблиц из БД в код

Для какого ни будь Yii это так, но в случае Doctrine нет. Как говорили выше, Doctrine предлагает работать с сущностями не зацикливаясь на структуре БД. Почему вы считаете что ORM не может быть объектом предметной области?
Да, ORM только описывает сами сущность и не позволяет выполнять какие-то действия напрямую из сущность как например в случае Active Record. Но у нас есть еще один уровень Repository который позволяет выполнять бизнес процессы над конкретной сущностью.
Можно даже пойти дальше и создать сервис который помимо действий заложенных в Repository сможет выполнять дополнительные действия обращаясь к своим зависимостям. Например логировать события добавления новой записи.
А валидация это неотъемлемая часть бизнес логики и должна быть как можно ближе к сущности, то есть в аннотациях ORM, и не как не в командах которые ничего не знают о бизнес сущностях.
PS: Можно конечно валидацию и в конфиги вынести, но это не best practice.
Чем плоха анонимная функция в данном контексте? «Можно и без нее» — слабый аргумент. :)

  1. такой код ИМХО некрасивый
  2. есть более подходящие инструменты для этой задачи
  3. я могу ошибаться, но по моему анонимные функции создаются каждый раз заново при каждом выполнении куска кода с ее инициализацией

в общем это мое субъективное мнение
идея конечно интересная, но вот с реализацией я не согласен
  1. Валидирование размазано по командам.
    Правила валидирования должны описываться в сущности, то есть в Project. А так вы дублируете правила валидации от команды к команде и велика вероятность что где-то что-то потеряете. А бизнес логика должна быть рядом с сущностью
  2. Не нужно создавать анонимную функцию $empty2null.
    1. Ни что не мешает создать приватную функцию
    2. Правильней использовать трансформеры

  3. Для преобразования запроса в сущность лучше подходит механизм форм

Такой вариант проще и эластичней
$project = new Project();

$form = $this->createForm(ProjectForm::class, $project);
$form->handleRequest($request);

чем такой
$data = $request->request->get('project');

$entity = (new Project())
    ->setName($data['name'])
    ->setDescription($data['description']);

это у вас только 2 простых поля, а что если полей 15 и некоторые из них являются связями
Ну и напоследок.
На мой взгляд будет проще и удобней если команда на вход будет принимать http запрос и внутри его преобразовывать в сущность.
То есть команда инкапсулирует преобразование запроса в сущность, а обработчик команды уже сохраняет сущность из команды, обрабатывает ошибки, логирует что надо и бросает евенты какие надо.
Разработчики Symfony: https://github.com/orgs/symfony/people
Главный у них: Fabien Potencier
На тему скорости разработки:
  • Когда я начинал изучать Symfony я написал сайт с нуля за одну неделю
  • Недавно писал CRM с нуля. Через неделю у меня был уже рабочий прототип, а еще через неделю я сдал проект

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

картинка в тему
image
Если количество зависимостей в объекте достаточно высоко, а реализуемый им функционал довольно сложен

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

Если логика слишком сложная то её нужно разносить на отдельные сервисы. Где-то можно сгруппировать действия, где-то бросить событие, где-то ввести уровень абстракции. Хорошую планку задает SensioLabsInsight — метод должен быть не длиннее 50 строк.
+1 это ещё и короче с namespase-ами. И зависимости лучше видно
Уязвимость через регистрацию будет если форма регистрации состоит предположим из полей email и password.
Есть проверки которые выполняются в указанном порядке:

  1. указан email
  2. формат email-а
  3. наличие email в бд
  4. указан пароль
  5. длинна пароля
  6. сложность пароля

То хакер может указать корректный email и не корректный пароль и тогда если пользователя в бд нет регистрация свалится после 3-его шага. Пользователь зарегистрирован не будет, а email будет проверен.

Но решается эта проблема очень просто. Проверка корректности данных должна выполнятся до запроса в бд

Information

Rating
10,293-rd
Location
Россия
Registered
Activity