Рефакторинг фабричного метода

Обсуждаем, как правильно строить приложения
Ответить
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Рефакторинг фабричного метода

Сообщение glagola »

Добрый день, начал разбираться с DDD и впал в ступор.

Суть: пользователь может бысть зарегистрирован в системе двумя способами: 1) email&password; 2) через oauth Facebook'a. На данный момент использую только один фабричный метод RoomMates\Domain\Model\User::register(...) для обоих способов регистрации, что неминуемо приводит к наличию null-able параметров (так как в разных способах доступны разные данные) и меня это честно говоря напрягает (подругому это смутное чувство объяснить не могу, так как в DDD я новичок :) ).

Вопросы:
  • стоит ли разделять RoomMates\Domain\Model\User::register(...) на два под каждый способ регистрации?
  • есть устойчивое желание разделить метод register на 2 и перенести их в UserFactory, насколько это правильно?
User планируется как Aggregate root:

Код: Выделить всё

<?php

/**
 * Aggregate root User
 */
class User
{
    // ...
    
    public function __construct(UserId $id)
    {
        $this->id = $id;
    }
    
    /**
     * Register new user
     *
     * @param EmailAddress         $email
     * @param null|UserDateOfBirth $dateOfBirth
     * @param string               $password
     * @param string               $firstName
     * @param string               $lastName
     * @param string               $authKey
     * @param null|string          $facebookId
     * @param null|Gender          $gender
     */
    public function register(EmailAddress $email,
                             ?UserDateOfBirth $dateOfBirth,
                             string $password,
                             string $firstName,
                             string $lastName,
                             string $authKey,
                             ?string $facebookId,
                             ?Gender $gender): void
    {
        $this->setEmail($email);
        $this->setPassword($password);
        $this->setFirstName($firstName);
        $this->setLastName($lastName);
        $this->setDateOfBirth($dateOfBirth);
        $this->setAuthKey($authKey);
        $this->setGender($gender);
        
        $this->setNotificationSettings(new UserNotificationSettings(
            true,
            true,
            true,
            true
        ));

        $this->facebookId = $facebookId;
        $this->loginCount = 0;
        $this->lastLogin = null;
        $this->credits = 0;
        $this->facebookCredits = 0;
        $this->registeredAt = DateTimeHelper::now();
        
        DomainEventPublisher::instance()->publish(
            new UserRegistered($this->id())
        );
    }
    
    // ...
}
Email&password Регистрация в Application Service

Код: Выделить всё

class SignUpService implements ApplicationService
{
    /** @var UserRepository */
    public $repository;
    
    /** @var AuthManager */
    public $authManager;
    
    /** @var PasswordManager */
    public $passwordManager;
    
    public function __construct(UserRepository $repository,
                                AuthManager $authManager,
                                PasswordManager $passwordManager)
    {
        $this->repository = $repository;
        $this->authManager = $authManager;
        $this->passwordManager = $passwordManager;
    }
    
    /**
     * @param SignUpRequest $request
     *
     * @return void
     * @throws UserAlreadyExistsException
     */
    public function execute($request = null): void
    {
        if ($user = $this->repository->ofEmail($request->email())) {
            throw new UserAlreadyExistsException;
        }
        
        $user = new User($this->repository->nextIdentity());
        $user->register(
            new EmailAddress($request->email()),
            null,
            $this->passwordManager->hashPassword($request->password()),
            $request->firstName(),
            $request->lastName(),
            $this->authManager->generateAuthKey(),
            null,
            null
        );
        
        $this->repository->store($user);
    }
}
Facebook регистрация в Application Service

Код: Выделить всё

class SignUpViaFacebookService implements ApplicationService
{
    /** @var UserRepository */
    public $repository;
    
    /** @var AuthManager */
    public $authManager;
    
    /** @var PasswordManager */
    public $passwordManager;
    
    public function __construct(UserRepository $repository,
                                AuthManager $authManager,
                                PasswordManager $passwordManager)
    {
        $this->repository = $repository;
        $this->authManager = $authManager;
        $this->passwordManager = $passwordManager;
    }
    
    /**
     * @param SignUpViaFacebookRequest $request
     *
     * @return void
     * @throws UserAlreadyExistsException
     */
    public function execute($request = null): void
    {
        if ($this->repository->facebookUserExists($request->facebookId())) {
            throw new UserAlreadyExistsException;
        }
        
        if ($user = $this->repository->ofEmail($request->email())) {
            throw new UserAlreadyExistsException;
        }
        
        // Todo extract to domain service
        $dateOfBirth = null;
        if (null !== $request->dobDay() && null !== $request->dobMonth()) {
            $dateOfBirth = new UserDateOfBirth(
                $request->dobYear(),
                $request->dobMonth(),
                $request->dobDay()
            );
        }
        
        $password = $this->passwordManager->generatePassword();
        
        $user = new User($this->repository->nextIdentity());
        $user->register(
            new EmailAddress($request->email()),
            $dateOfBirth,
            $this->passwordManager->hashPassword($password),
            $request->firstName(),
            $request->lastName(),
            $this->authManager->generateAuthKey(),
            $request->facebookId(),
            $request->gender()
        );
        
        $this->repository->store($user);
    }
}
P.S. Буду также признателен конструктивной критике моего понимания DDD!

P.S.S. Спасибо заранее!
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Рефакторинг фабричного метода

Сообщение ElisDN »

Лучше разделить.
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

Благодарю, что подтвердили мои сомнения. А как насчет того чтобы вынести эти методы в UserFactory? и назвать их соответствующе - registerViaEmail, registerViaFacebook (ну или как-то так)?

Просто есть еще один момент, который меня смущает и подталкивает к созданию UserFactory - необходимость хранения auth key в модели (который в свою очередь нужен при реализации интерфейса IdentityInterface). Auth key является чисто Yii'шной фишкой и как мне кажется было бы правильно вынести ее в инфраструктурный слой, отнаследовавшись от User и добавив туда $authKey, а уже за счет инкапсуляции создания User'a в UserFactory, я бы смог прозрачно пропихнуть инфраструктурную версию User всюду.

А вообще, реально ли инкапсулировать создание агрегата полностью в Factory?
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Рефакторинг фабричного метода

Сообщение ElisDN »

Если хотите наследовать, то да, создавайте в фабрике.
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

Благодарю, пришлось перелопатить приличные объемы кода, но цели добился. Кстати, возникли проблемы гидратором Samdark'a - в моем случае отсутствует публичный конструктор у агрегата, создание происходит в фабрике, поэтому приходится создавать объект, а потом только его наполнять через гидратор в репозитории. Надеюсь в ближайшее время сделать пул-реквест для samdark/hydrator, добавить возможность гидрировать уже созданный объект. Хотя может быть это то самое место где можно применить паттерн Builder, чтобы все-таки пользоваться рефлексией всего один раз...
anton_z
Сообщения: 483
Зарегистрирован: 2017.01.15, 15:01

Re: Рефакторинг фабричного метода

Сообщение anton_z »

glagola писал(а): 2017.02.23, 00:37 Благодарю, что подтвердили мои сомнения. А как насчет того чтобы вынести эти методы в UserFactory? и назвать их соответствующе - registerViaEmail, registerViaFacebook (ну или как-то так)?
Если будете делать на каждый способ регистрации по методу - нарушите принцип открытости/закрытости. Надо делать отдельный сервис на каждый способ регистрации
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

Если будете делать на каждый способ регистрации по методу - нарушите принцип открытости/закрытости. Надо делать отдельный сервис на каждый способ регистрации
На данный момент у меня 2 Application сервиса, каждый из которых отвечает (каждый со своим DTO на входе) за свой способ регистрации и оба используют UserFactory. Дело в том, что сигнатуры методов регистрации отличаются, поэтому, честно говоря, не представляю как абстрагироваться от способа регистрации:

Код: Выделить всё

interface UserFactory
{
    public function registerNewUserByEmail(UserId $id,
                                           EmailAddress $email,
                                           string $password,
                                           string $firstName,
                                           string $lastName): User;
    
    public function registerNewUserByFacebook(UserId $id,
                                              EmailAddress $email,
                                              ?UserDateOfBirth $dateOfBirth,
                                              string $firstName,
                                              string $lastName,
                                              string $facebookId,
                                              ?Gender $gender): User;
}
paurlift
Сообщения: 26
Зарегистрирован: 2017.01.29, 20:16

Re: Рефакторинг фабричного метода

Сообщение paurlift »

glagola писал(а): 2017.02.25, 11:29
Если будете делать на каждый способ регистрации по методу - нарушите принцип открытости/закрытости. Надо делать отдельный сервис на каждый способ регистрации
На данный момент у меня 2 Application сервиса, каждый из которых отвечает (каждый со своим DTO на входе) за свой способ регистрации и оба используют UserFactory. Дело в том, что сигнатуры методов регистрации отличаются, поэтому, честно говоря, не представляю как абстрагироваться от способа регистрации:

Код: Выделить всё

interface UserFactory
{
    public function registerNewUserByEmail(UserId $id,
                                           EmailAddress $email,
                                           string $password,
                                           string $firstName,
                                           string $lastName): User;
    
    public function registerNewUserByFacebook(UserId $id,
                                              EmailAddress $email,
                                              ?UserDateOfBirth $dateOfBirth,
                                              string $firstName,
                                              string $lastName,
                                              string $facebookId,
                                              ?Gender $gender): User;
}
Вы писали, что User у Вас агрегат, в этом случае его лучше создавать фабрикой, при этом конструктор не забудьте объявить приватным. Способ создать новый объект - это либо фабрика либо конструктор, но он должен быть один. При этом при создание должны проверяться все инварианты создаваемого объекта (модель должны быть всегда валидна и согласована).

Если к регистрации относится, как к созданию новой сущности User, то я бы сделал так:

Код: Выделить всё


interface UserFactory
{
	public function register($email, UserDto $userDto);
}



Dto заполнял бы в application. Все данные по всем соц. сетям можно привести к одному формату accountId вместо facebookId. Обязательные свойства без которых не может быть создан User передать параметрами. Все остальное через Dto. Сохраняешь в агрегат всегда все, что пришло в Dto. (даже null, если свойство является не обязательным) Внутри метода register создаешь агрегат, в том числе его VO (UserId, EmailAdress). Тем самым все создание Агрегата находится в одном методе одной фабрики.

Если User может быть создан не только через регистрацию, не стал бы называть метод фабрики register.
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

Не согласен, раз уж пытаться обобщить регистрацию, то нужно ее все-таки разделить по способам:
  • Email&Password
  • Social network
Для каждого способа сделать свой метод регистрации в фабрике, а уже потом обощать в рамках одного способа - facebook, vk, twitter. Так как инварианты будут разные. Если не попытаться сгруппировать инварианты то есть риск, что придется писать много if'ов пытаясь разобрать все сценарии в UserFactory::register, и тогда все вернется к тому с чего начинали, за тем лишь исключением, что теперь метод register будет вынесен в фабрику.
paurlift
Сообщения: 26
Зарегистрирован: 2017.01.29, 20:16

Re: Рефакторинг фабричного метода

Сообщение paurlift »

glagola писал(а): 2017.02.26, 20:14 Не согласен, раз уж пытаться обобщить регистрацию, то нужно ее все-таки разделить по способам:
  • Email&Password
  • Social network
Смысл в том что создается по факту один и тот же User, только с разными кол-вами свойств. Но есть свойства, которые есть всегда.
Что делать если появляется новый источник пользователей? (регистрация через рассылку по уникальному хэшу и email)
Что делать если понадобится регистрация по номеру телефона?
Что делать если бизнес решит, что имя и фамилия не обязательна и пользователь может изменить персональные данные после регистрация?
glagola писал(а): 2017.02.26, 20:14 Для каждого способа сделать свой метод регистрации в фабрике, а уже потом обобщать в рамках одного способа - facebook, vk, twitter. Так как инварианты будут разные. Если не попытаться сгруппировать инварианты то есть риск, что придется писать много if'ов пытаясь разобрать все сценарии в UserFactory::register, и тогда все вернется к тому с чего начинали, за тем лишь исключением, что теперь метод register будет вынесен в фабрику.
При том способе реализации, который я предложил, if не должно быть много. Там где они нужны, можно реализовать стратегию.
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

Хм, хорошая идея со стратегией - ниже набросал псевдокод код, правильно ли я понял, что вы предлагаете что-то на подобии подхода ниже?

Код: Выделить всё

<?php

// Aggregate root
class User 
{
	protected $id;
	protected $email;
	protected $gender;
	// other fields
}

class SomeInfrastructureUser extends User
{
	protected $someAdditionalFieldWhichIsNeededByInfrastructureLayer;	
}

interface IUserBuildStrategy
{
	public function fill(IUserBuilder $builder):void;
}

interface IUserBuilder
{
	public function setId(int $id): void;
	public function setEmail(string $email): void;
	public function setGender(string $gender): void;

	public function build(): User;
}

interface IUserFactory
{
	public function build(IUserBuildStrategy $strategy): User;
}

interface IUserBuilderFactory
{
	public function build(string $userClassToBuild): IUserBuilder;
}

class UserFactory implements IUserFactory
{
	/** @var IUserBuilderFactory */
	private $builerFactory;

	public function __construct(IUserBuilderFactory $builerFactory)
	{
		$this->builderFactory = $builerFactory;	
	}

	public function build(IUserBuildStrategy $strategy): User
	{
		$builder = $this->builderFactory->build(SomeInfrastructureUser::class);
		$strategy->fill($builder);
		return $builder->build();
	}
}

class EmailRegistrationStrategy implements IUserBuildStrategy
{
	private $email;
	public function __construct(string $email)
	{
		$this->email = $email;	
	}

	public function fill(IUserBuilder $builder): void
	{
		$builder->setEmail($this->email);
	}
}

class FacebookRegistrationStrategy implements IUserBuildStrategy
{
	private $email;
	private $gender;

	public function __construct(string $email, string $gender)
	{
		$this->email = $email;	
		$this->gender = $gender;
	}

	public function fill(IUserBuilder $builder): void
	{
		$builder->setEmail($this->email);
		$builder->setGender($this->gender);
	}
}

class RestoreFromPersistenceStrategy implements IUserBuildStrategy
{
	private $email;
	private $gender;
	private $id;

	public function __construct(string $id, string $email, string $gender)
	{
		$this->email = $email;	
		$this->gender = $gender;
		$this->id = $id;
	}

	public function fill(IUserBuilder $builder): void
	{
		$builder->setEmail($this->email);
		$builder->setGender($this->gender);
		$builder->setId($this->id);
	}
}

UserBuilder будет создавать объект User (точнее его наследника) через рефлексию

P.S. У меня вопрос, не перебор ли это с точки зрения самой задачи? :D
P.S.S В псевдокоде могут быть синтаксические ошибки - не было возможности проверить)
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Рефакторинг фабричного метода

Сообщение ElisDN »

Код: Выделить всё

$user = new User($id, $name, $authKey);
$user->signupByEmail($email, $password);

Код: Выделить всё

$user = new User($id, $name, $authKey);
$user->signupByFacebook($identity);
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

ElisDN писал(а): 2017.02.26, 22:57

Код: Выделить всё

$user = new User($id, $name, $authKey);
$user->signupByEmail($email, $password);

Код: Выделить всё

$user = new User($id, $name, $authKey);
$user->signupByFacebook($identity);
Загвоздка в том чтобы ограничить создание пользователя только через фабрику т.к. он должен содержать дополнительные поля, необходимые для инфраструктуры (в частности это $authKey для IdentityInterface в Yii2)

При подходе из моего предыдущего сообщения код Application сервиса будет выглядеть как-то так:

Обычная регистрация

Код: Выделить всё

$this->userFactory->build(new EmailRegistrationStrategy(
	'some.email@domain.com'
));
Или

Регистрация через Facebook

Код: Выделить всё

$this->userFactory->build(new FacebookRegistrationStrategy(
	'some.email@domain.com',
	'male'
));
Таким образом стратегия будет одновременно DTO'шкой и стратегией, тем самым контролируем входные параметры для каждого типа регистрации и заполняем билдер, который в свою очередь сможет создать нужную для инфраструктуры версию агрегата (с дополнительными полями, такими как $authKey) с помощью гидратора от SamDark'а.
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Рефакторинг фабричного метода

Сообщение ElisDN »

glagola писал(а): 2017.02.26, 23:08 Загвоздка в том чтобы ограничить создание пользователя только через фабрику т.к. он должен содержать дополнительные поля, необходимые для инфраструктуры (в частности это $authKey для IdentityInterface в Yii2)

Код: Выделить всё

$user = $this->userFactory->create($id, $name);
$user->signupByEmail($email, $password);

Код: Выделить всё

$user = $this->userFactory->create($id, $name);
$user->signupByFacebook($identity);
glagola
Сообщения: 47
Зарегистрирован: 2017.02.22, 19:43

Re: Рефакторинг фабричного метода

Сообщение glagola »

ElisDN писал(а): 2017.02.26, 23:18
glagola писал(а): 2017.02.26, 23:08 Загвоздка в том чтобы ограничить создание пользователя только через фабрику т.к. он должен содержать дополнительные поля, необходимые для инфраструктуры (в частности это $authKey для IdentityInterface в Yii2)

Код: Выделить всё

$user = $this->userFactory->create($id, $name);
$user->signupByEmail($email, $password);

Код: Выделить всё

$user = $this->userFactory->create($id, $name);
$user->signupByFacebook($identity);
Новая идея с переусложнением кода, появилась как решение текущей проблемы (которую вы цитируете) и следующим замечанием (забыл упомянуть):
paurlift писал(а): Что делать если появляется новый источник пользователей? (регистрация через рассылку по уникальному хэшу и email)
Что делать если понадобится регистрация по номеру телефона?
Что делать если бизнес решит, что имя и фамилия не обязательна и пользователь может изменить персональные данные после регистрация?
Аватара пользователя
ElisDN
Сообщения: 5845
Зарегистрирован: 2012.10.07, 10:24
Контактная информация:

Re: Рефакторинг фабричного метода

Сообщение ElisDN »

paurlift писал(а): 2017.02.26, 21:04 Что делать если появляется новый источник пользователей? Что делать если понадобится регистрация по номеру телефона?
Добавить регистрацию по телефону:

Код: Выделить всё

$user = $this->userFactory->create($id, $name);
$user->signupByPhone($phone, $hash);
paurlift писал(а): 2017.02.26, 21:04 Что делать если бизнес решит, что имя и фамилия не обязательна...
Удалить $name из конструктора.
paurlift писал(а): 2017.02.26, 21:04 ...и пользователь может изменить персональные данные после регистрация?
Добавить метод смены данных:

Код: Выделить всё

$user->changePersonalData($name, $address);
paurlift
Сообщения: 26
Зарегистрирован: 2017.01.29, 20:16

Re: Рефакторинг фабричного метода

Сообщение paurlift »

glagola писал(а): 2017.02.26, 22:42 P.S. У меня вопрос, не перебор ли это с точки зрения самой задачи? :D
P.S.S В псевдокоде могут быть синтаксические ошибки - не было возможности проверить)
Вроде гибко получилось, но действительно для такой тривиальной задачи слишком сложно)
https://habrahabr.ru/post/321892/ - очень похоже на Ваш кейс.
Ответить