webdevilopers / php-ddd

PHP Symfony Doctrine Domain-driven Design
201 stars 10 forks source link

Using a Command as DTO to populate Symfony Form #5

Open webdevilopers opened 8 years ago

webdevilopers commented 8 years ago

@beberlei shows a good example on how to use this approach here: http://whitewashing.de/2012/08/22/building_an_object_model__no_setters_allowed.html

With the following code you can add the EditPostCommand to the data_class of the EditPostType form:

<?php
class EditPostCommand
{
    public $id;
    public $headline;
    public $text;
    public $tags = array();
}
<?php
class PostController
{
    public function editAction(Request $request)
    {
        $post = $this->findPostViewModel($request->get('id'));

        // This could need some more automation/generic code
        $editPostCommand           = new EditPostCommand();
        $editPostCommand->id       = $request->get('id');
        $editPostCommand->headline = $post->headline;
        $editPostCommand->text     = $post->text;
        $editPostCommand->tags     = $post->tags;

        // here be the form framework handling...
        $form = $this->createForm(new EditPostType(), $editPostCommand);
        $form->bind($request);

        if (!$form->isValid()) {
            // invalid, show errors
        }

        // here we invoke the model, finally, through the service layer
        $this->postService->edit($editPostCommand);
    }
}

But it seems like Commands should be regarded immutable too which means the properties should not be public. Of course Symfony Form needs at least these to populate the Command.

@matthiasnoback has an interesting solution for this Symfony Form Use Case: https://github.com/matthiasnoback/convenient-immutability

A Groovy example by @bodiam: http://www.jworks.nl/2011/08/26/friday-repost-groovys-immutable-pitfalls/

General discussion: https://groups.google.com/forum/#!msg/dddinphp/a5ielXU3EZg/qEoi5ppWAgAJ

Is there any approach how we could use the Command with Symfony Form if the properties were private?

Maybe using the empty_data callback on the Command / DTO and a static method as suggested by @webmozart: http://whitewashing.de/2012/08/22/building_an_object_model__no_setters_allowed.html#comment-1091314235

matthiasnoback commented 8 years ago

The Convenient Immutability library is not 100% serious actually. The thing with immutability is that it isn't really needed in this case. You won't be tempted to modify the object, right? And you won't need to pass it around, you just hand it over to a command handler of some sorts. Mutable objects are mainly a danger when they can be modified by other parts of the application. If you're the one working on that application, considering immutability a good thing, you don't really need actual immutability anymore ;) Or you use my library, which will at least prevent anyone less smart than you from modifying previously populated fields.

webdevilopers commented 8 years ago

Thanks @matthiasnoback ! I agree - the only modification comes from the form happening once and the the Command is passed to the Handler.

The only thing I have to find our now is how much smarter I am! ;)

yvoyer commented 8 years ago

To add more sugar to the command, maybe we could add a static factory method EditPostCommand::fromPost (Post $post): EditPostCommand. That way instead of changing the controller when adding a new property to the command for the edit, it could be wrapped directly in the command.

webdevilopers commented 8 years ago

I do something similar @yvoyer to pre-populate a Symfony Form for Orders if an optional Offer already exists:

        $placeOrderCommand = ($offer instanceof Offer)
            ? PlaceOrderCommand::fromOffer($offer)
            : new PlaceOrderCommand($customer->getId());
chrisguitarguy commented 8 years ago

The Convenient Immutability library is not 100% serious actually. The thing with immutability is that it isn't really needed in this case. You won't be tempted to modify the object, right? And you won't need to pass it around, you just hand it over to a command handler of some sorts.

100% agree. I leave public properties in commands for the sake of forms quite a bit. But I do provider getters on those same commands and treat them as immutable in their handlers.

$form->bind($request);

Unrelated, but this should be...

$this->createForm(EditPostType::class, $editPostCommand);
$form->handleRequest($request)
if ($form->isValid()) {
  // ...
}

Bind is deprecated (and removed in Symfony 3.x), and handleRequest is the new way to submit the form with a symfony request object.

rommsen commented 8 years ago

my 2 cents:

We are currently in the process of moving away from having mutable command objects (and from using them as a data-source for Symfony forms)

Reasons (all imho of course):

But how do you get the data to your forms and how do you create a command with form data?

We use DataMappers provided by the Form Library (read this awesome article by @webmozart)

First of all we use the third and not the second parameter to give data to the form

 // second parameter is empty
$form = $this->createForm(ReportingDataType::class, [], ['job' => $job]);

Then we define which data is needed by the form

public function configureOptions(OptionsResolver $resolver)
{
    ...
    $resolver->setRequired(['job']);
}

We can implement the DataMapperInterface and set it to the form itself

class ReportingDataType extends AbstractType implements DataMapperInterface
{
    private $job;

    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $this->job = $options['job'];
        // ... 
        $builder ->setDataMapper($this)

and we implement the mapping methods


public function mapDataToForms($data, $forms)
{
    $getIds = function (Questionnaire $questionnaire) {
        return $questionnaire->getId();
    };
    $reportingData = $this->job->getReportingData();
    $forms = iterator_to_array($forms);
    $forms['inherit_reporting_data']->setData($this->job->inheritsReportingData());
    $forms['questionnaires_for_jobs']->setData(array_map($getIds, $reportingData->getQuestionnairesForJobs()));
    $forms['questionnaires_for_slots']->setData(array_map($getIds, $reportingData->getQuestionnairesForSlots()));
}

public function mapFormsToData($forms, &$data)
{
    $forms = iterator_to_array($forms);
    $data = new ChangeJobReportingDataCommand(
        $this->job->getId(),
        $forms['inherit_reporting_data']->getData(),
        $forms['questionnaires_for_jobs']->getData(),
        $forms['questionnaires_for_slots']->getData()
    );
}

finally we can normally validate and dispatch our command:

$form->handleRequest($this->request);
if ($form->isValid()) {
    $this->dispatchCommand($form->getData());

What do we win?

What do we lose:

RFC :smile:

webdevilopers commented 8 years ago

Well that's a very interesting approach @rommsen . Havn't seen it before.

Did you ever experience problems using your Data Transformers with let's say Symfony FormEvents? What do your commands look like regarding "Validation" e.g. Constraints?

rommsen commented 8 years ago

Hey @webdevilopers. Sorry had some busy days.

Concerning validation: We are using commands with a payload array and not with multiple properties. To validate this we use the collection constraint. The cool thing is, that it allows us to define, whether it is allowed to have fields, that are not mentioned in the validations or not.

/**
     * @var array
     * @AssertThat\Collection(
     *     fields = {
     *         "id" = {
     *             @AssertThat\NotBlank(),
     *             @AssertThat\NotNull()
     *         },
     *         "phone_home" = @Assert\PhoneNumber(defaultRegion="DE"),
     *         "phone_mobile" = @Assert\PhoneNumber(defaultRegion="DE"),
     *         "phone_business" = @Assert\PhoneNumber(defaultRegion="DE"),
     *         "fax" = @Assert\PhoneNumber(defaultRegion="DE"),
     *         "second_phone_home" = @Assert\PhoneNumber(defaultRegion="DE"),
     *         "email" = {
     *             @AssertThat\NotBlank(),
     *             @AssertThat\Email()
     *         }
     *     },
     *     allowExtraFields = true
     * )
     */
    protected $payload;

Concerning form events: As I said we are currently in the process of refactoring towards the described workflow. We have yet to refactor a form with events attached. But I dont think there are big problems. That said, I have to remark that since we started to move towards a task based UI, our forms tend to become MUCH smaller. They don't have as much edge cases as they had when we where mainly doing huge CRUD forms (i.e. we need form events very seldom).

rommsen commented 8 years ago

Here is a current amendment:

With the described approach we had a problem, when the form creates a Command VO which itself needed a VO for its creation. If the needed VO is never created (e.g. a user is not selecting a value in a choice list), the Command can not be created, and we can not use the normal validation.

To solve this problem we used DataTransformers (inline, i.e. CallbackTransformers) which throw a TransformationException, when the VO was not created. With this approach we are able to map the exception directly to the not validationg form field. Example

...

class CancellationType extends AbstractType implements DataMapperInterface
{
    /**
     * @var CancellationId
     */
    private $cancellationId;

    public function buildForm(FormBuilderInterface $builder, array $options)
    {
        $this->cancellationId = $options['cancellationId'];

        $builder
            ->setDataMapper($this) // maps form data to the resulting command
            ->add(
                'reason', ValueObjectChoiceType::class, [
                    'loader_callback' => function () {
                        return CancellationReason::all();
                    },
                    'label' => 'cancellation.reason',
                    'invalid_message' => 'validation.application.choose_value',
                    'allowClear' => true,
                ]
            )
            ->add('notice', NoticeType::class, [
                'label' => 'cancellation.notice',
            ]);

        // custom ModelTransformer which throws an Exception when the $value created by the 
        // form is not a VO (CancellationReason)
        // the form uses then the value form the 'invalid_message' option defined above
        $builder->get('reason')
            ->addModelTransformer(new CallbackTransformer(
                function ($value) {
                    return $value;
                },
                function ($value) {
                    if ($value instanceof CancellationReason) {
                        return $value;
                    }
                    throw new Exception\TransformationFailedException();
                }
            ));
    }

...

    public function mapFormsToData($forms, &$data)
    {
        $forms = iterator_to_array($forms);

        $reason = $forms['reason']->getData();
        $notice = $forms['notice']->getData();

        // needs to be checked because this method is called 
        // even when there was a TransformationException
        if ($reason === null || $notice === null) {
            return;
        }
        $data = BeginCancellation::withData($this->cancellationId,$reason,$notice );
    }
}

I have to say that I am very happy with this approach. Many people complain about all the magic in the form component (which is bad when not doing RAD) but it provides so many nice extension points. I think, when used correctly, it is a very good fit for building an application with DDD in mind.

webdevilopers commented 7 years ago

@webmozart also added some DDD and CQRS examples to his latest form slides:

deeky666 commented 6 years ago

There might be another approach using https://github.com/codete/FormGeneratorBundle and https://github.com/Roave/StrictPhp

The former allows to define the command corresponding form via property annotations and the latter allows for immutability of public porperties. The Symfony form generator is awesome but I have not yet tested the AOP StrictPHP lib. But AOP in general seems to be a good candidate for DDD type applications.

In general I like the idea of having the command/query DTO as a contract to the outside world and put as much metadata as possible via annotations to it. That opens a lot of possibilities of auto generating stuff like routing, API docs etc.

Just my 0.02 $ ...

webdevilopers commented 3 years ago

Quote from @spike31:

I have stopped mapping class, entity, on my #symfony forms for 3-4 years, on small or very large projects. Everything is fine, my code is better decoupled. Goodbye unnecessary getter/setter. Forms are just inputs with validations.

As asked by ๐— ๐˜†๐—ฟ๐—ผ๐˜€๐—น๐—ฎ๐˜ƒ 2.0:

Could you please share an example?

Came from:

chrisguitarguy commented 3 years ago

I got a notification on this thread, kinda forgot about it. But I started using data transfer objects and the serializer + validator in my apps and I really like the approach.

https://www.pmg.com/blog/trading-symfonys-form-component-for-data-transfer-objects/

So no form component at all. Only the serializer. And the DTOs replace any custom logic around normalizers and stuff. All the "what does the api look like" stuff is in one place along with validation.

cherifGsoul commented 3 years ago

I think we go through some steps depending on the the trade-offs:

1- Assuming that the business logic is trivial that needs just CRUD operation, in this case coupling the entity to the form type class is fine. 2- When The business logic gets complicated and the model needs to evolve, in this case extracting the form DTO class and designing DDDesign entities and value objects becomes a necessity to decouples the layers.

Some folks (I don't remember where I read it) argue that having getters in the DTO that returns value objects is considered as coupling domain layers object to the infrastructure, but I think it is fine to do it and it might helps to not write domain factories.

webdevilopers commented 3 years ago

Some folks (I don't remember where I read it) argue that having getters in the DTO that returns value objects is considered as coupling domain layers object to the infrastructure, but I think it is fine to do it and it might helps to not write domain factories.

Indeed we use the getters for convenience. Otherwise our command handlers had to do all the conversion from command DTO primitives to value objects.

If our Commands needed to be serialized for async operations they would of course NOT have the getters anyway.

But in our case Commands and Handlers live in an extra Application Layer. Command Handlers ARE application services. Some put both into the Domain Layer.

I will post some more thoughts on the heavy Symfony Forms usage and coupling later.

webdevilopers commented 3 years ago

Feel free to join the discussion @iosifch.

iosifch commented 3 years ago

Thanks for the invitation, @webdevilopers ! That example showed by @rommsen is what I would do if I had to use the Symfony Forms component with the specification that I would try to use a read model in order to show any data to the user. I would try to use the DTO command strictly to fill it with the data from the form and to send it to be executed by a command handler. I believe that the form should have a read model as data source combined with the data already filled by the user.

webdevilopers commented 3 years ago

We have some older applications where we still use Symfony Forms. Even there we introduced Commands to decouple our Domain Models.

All this can be easily achieved.

But I think there are two problems people face when starting with it:

1 Not using primitives but objects

There is no need to use objects as properties of the command. Even if you are using value objects in the end. Simply use primitives. Then you will not need any "Data Transformers" that would add sooo much additional code. Just transform the primitives to objects when getting them from the Command and then pass the objects to your Domain Model e.g. inside the command handler.

2 Relying on Doctrine associations instead of reference by ID only

One of the best things I learned from #DDDesign is to keep aggregates small and reference other aggregates by ID only. Doctrine and Symfony Form offer a lot of convenience here in the first place. You have a Customer Entity that has multiple Order Entites. Just annotate the relationship and Symfony Form can build you a select. Even a custom Query Builder can be attached. But that is where the headache starts. You never know which queries you fire where. The solution is a single Repository with dedicated methods e.g. "getFormOptionsByCustomerId" and then add the options inside the Symfony Form instead of the Element and the Query Builder.

Completely remove the references from your Entity. An Order Entity only needs a CustomerId. A repository can fetch additional required data. No need to build the entire graph when loading an Entity. And don't make it even more complicated by introducing "lazy loading".

Conclusion

If your doing CRUD or RAD you get some convenience, yes. But as soon as your app starts growing you will face typical problems like memory usage, performance issues, lack of encapsulation, no immutability, hidden magic etc. etc. - all the things that make your life as a developer "comfortable" and that make you feel safe in a team.

I would never want to go back.

Hope this helps.

webdevilopers commented 3 years ago

BTW: The symfony-5-es-cqrs-boilerplate repo by @jorge07 will maybe soon present their example of how to deal w/ Symfony Forms, Entities and Command DTOs. As requested by @Romaixn.

https://github.com/jorge07/symfony-5-es-cqrs-boilerplate/issues/221