[Form] Finally document Data Mappers #6900

wouterj · 2016-08-21T22:42:01Z

It's probably the biggest undocumented feature in Symfony for years. Today, I was sad I had to tell yet another beginner "there is this awesome feature, but it's undocumented...". Let's finally document this great feature!

/fixes #5459
/ping @webmozart I would like your feedback on this article. The difference between transformers and mappers is hard to understand and we have to describe it pitch perfect to avoid confusion.

snoek09 · 2016-08-27T10:39:57Z

form/data_mappers.rst

+    reference. It recieves the form fields in a :phpclass:`RecursiveIteratorIterator`
+    and the view data.
+
+When an errors occurs, a


typo; When an error occurs

form/data_mappers.rst

+    single: Form; Data mappers
+
+How to Use Data Mappers
+=======================


weaverryan · 2016-09-18T23:46:56Z

form/data_mappers.rst

+the form. They are responsible for mapping the data to the fields and back. The
+built-in data mapper uses the :doc:`PropertyAccess component </components/property_access>`
+and will fit most cases. However, you can create your own data mapper for the
+other cases, for instance when dealing with immutable objects.


However, you can create your own data mapper that could, for example, pass data to immutable objects via their constructor.

weaverryan · 2016-09-18T23:47:45Z

form/data_mappers.rst

+
+* **Data transformers** change the representation of a value. E.g. from
+  ``"2016-08-12"`` to a ``DateTime`` instance;
+* **Data mappers** map data (e.g. an object) to form fields.


... to form fields, and vice versa.

Nice, short explanation between the two

weaverryan · 2016-09-18T23:48:26Z

form/data_mappers.rst

+
+Changing a ``YYYY-mm-dd`` string value to a ``DateTime`` instance is done by a
+data transformer. Mapping this ``DateTime`` instance as value of a property on
+the entity is done by a data mapper.


Mapping this ``DateTime`` instance to a property on your object (e.g. by calling a setter or some other method) is done by a data mapper.

weaverryan · 2016-09-18T23:54:14Z

form/data_mappers.rst

+Creating a Data Mapper
+----------------------
+
+Assume you're saving a set of colors in the database. For this, you're using an


Suppose that you want to save a set of colors to the database.

weaverryan · 2016-09-18T23:55:22Z

form/data_mappers.rst

+    }
+
+The form type should be allowed to edit a color. As the color object is
+immutable, a new color object has to be created each time one of the values is


~~As the color~~ -> But because you've decided to make the Color object immutable, a new...

weaverryan · 2016-09-19T00:00:49Z

form/data_mappers.rst

+            $green = $forms['green']->getData();
+            $blue = $forms['blue']->getData();
+
+            $data = new Color($red, $green, $blue);


woh! I pass by reference so rarely, that I'm honestly shocked that you can simply override the $data variable and this works! If so (I'm sure it does - Bernhard's blog post uses this same method), maybe we should add a quick comment:

// Overriding $data is enough, since it is passed by reference.

weaverryan · 2016-09-19T00:07:28Z

Status: Reviewed

This is wonderful! I left minor comments, but this is ready to go :)

👍

xabbuh · 2016-09-21T09:56:01Z

form/data_mappers.rst

+
+    The data passed to the mapper is *not yet validated*. This means that your
+    objects should allow being created in an invalid state in order to produce
+    user-friendly errors in the form.


I think we need to explain what you have to do when your object's constructor raises exceptions in case some assertions are not fulfilled (@webmozart didn't address that concern in his blog post neither and that's the reason I am not sure if we should really cover it here or rather advise to use DTOs instead).

This is also why scalar type hints in PHP7 cause some troubles. The form maps submitted data before validating its underlying data.

What we have to do in such case, is to validate data before mapping it ourselves :( I think he talked about it in his last conferences (suggestion to use custom data mappers for PHP 7).

Not sure what's the best way to approach this problem though, a "caution" note might be enough after all.

If the underlying fields use proper form types (like an IntegerType for an object property typehinted int in the constructor), the field itself will throw a TransformationFailedException which will be rendered as an error on the view on this particular field.
Important part is that the field won't be mapped back and (IIRC) keeps original value. Thus, in most cases, I think it won't cause any issue with scalar typehints.

See ogizanagi/symfony@4f76c21 and FormTypeTest.php#L745-L752
(it doesn't use a scalar typehint, but a \DomainException is thrown if Money::amount isn't numeric. The case is similar).

Appart from the above point, catching the error and throwing a TranformationFailedException ourself is a good safe guard (I recommend to use the invalid_message option in order to render a less cryptic error message, but that's limited).

@ogizanagi Your test does not cover what I'm talking about. When clear missing is true or if you submit an empty string, the returned model data will be null and the call of the setter will end with a php error because of the type hint which does not trigger a casting of null even if strict_type is false although it does cast a string to an int.

@HeahDude: Right, it won't cover everything anyway. However, regarding what you're talking about, maybe it would be interesting to make the transformers use the required option (or a new one if necessary for BC) in order to disallow the transformation and throw a TransformationFailedException('A number is required') in case the value is an empty string.

xabbuh · 2016-09-21T16:10:29Z

@HeahDude Would be nice to get your feedback on this too.

HeahDude · 2016-09-21T16:28:43Z

form/data_mappers.rst

+        {
+            $resolver->setDefaults(array(
+                // when creating a new color, the initial data should be null
+                'empty_data' => null,


This is the default unless data_class is set or if some data prepopulates the form

Afaics, the default it '' and not null. I think I might added this because of that

HeahDude · 2016-09-21T16:30:08Z

form/data_mappers.rst

+        }
+    }
+
+All data mappers have to implement :class:`Symfony\\Component\\Form\\DataMapperInterface`.


You should maybe introduce the interface before showing an implementation

HeahDude · 2016-09-21T16:31:52Z

form/data_mappers.rst

+    You can also implement ``DataMapperInterface`` in the ``ColorType`` and add
+    the ``mapDataToForms()`` and ``mapFormsToData()`` in the form type directly
+    to avoid creating a new class. You'll then have to call
+    ``$builder->setDataMapper($this)``.


The example should do it like that since this does not makes sense to reuse it elsewhere, and the note could then say that you can handle it in a specific class to easily share it between types

I started doing this, but I rejected it. If we directly implement it like that, the structure of the article gets less clear (now it's create DTO, create mapper, use mapper).

HeahDude · 2016-09-21T16:32:32Z

form/data_transformers.rst

@@ -13,9 +13,15 @@ to render the form, and then back into a ``DateTime`` object on submit.

 .. caution::

-    When a form field has the ``inherit_data`` option set, Data Transformers
+    When a form field has the ``inherit_data`` option set, data transformers


Are you sure about that one?

yceruto · 2016-10-06T20:41:19Z

form/data_mappers.rst

+    use Symfony\Component\Form\DataMapperInterface;
+    use Symfony\Component\Form\Exception\UnexpectedTypeException;
+
+    class ColorMapper extends DataMapperInterface


implements

mpdude · 2016-10-27T12:56:41Z

Awesome that finally some work has begun to fix this!

What the new section still does not explain is how DataMappers fit into the picture given at the events and data transformers page.

More specifically, does "data mapping" occur at a single point during the event flow? Could we add a note there saying that "at this point, a DataMapper (link) is used to ..."?

And with regard to data transformers, is the DataMapper responsible for reading/writing "model data" to/from the "model object"? Or does it also get in touch with the "norm" and "view" data?

mpdude · 2016-10-27T13:27:10Z

Skimming the code at https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L307, it looks as if the following happens:

setData($modelData) called on a Form
Dispatch FormEvents::PRE_SET_DATA which can change/replace the $modelData
Model transformer: transform() method to obtain normData from modelData
View transformer: transform() method to obtain viewData from normData
Data mapper: mapDataToForms() to map viewData to the form and its child elements
Dispatch FormEvents::POST_SET_DATA.

And then at https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Form/Form.php#L496:

submit($submittedData) called on a Form
Dispatch FormEvents::PRE_SUBMIT which can change/replace the $submittedData
Somehow the $submittedData is put on the form elements (I don't see it)
Data mapper: mapFormsToData() to read from form elements and put data onto viewData
View transformer: reverseTransform() viewData to normData
Dispatch FormEvents::SUBMIT
Model transformer: reverseTransform() normData to modelData
View transformer: transform()``modelData to viewData
Dispatch FormEvents::POST_SUBMIT

B-Galati · 2017-06-13T14:15:17Z

@wouterj Is this PR dead ?

HeahDude · 2017-06-23T18:24:04Z

For the record, here's a use case https://github.com/EnMarche/en-marche.fr/pull/1324/files#diff-8a3dda9de49dd1624e55e7e7d409853cR17.

I could have used a model transformer to handle string to array('entry' => string) conversion and let the property path mapper do its work.
But here's the point, I don't want to transform the value, I just want to map it. Adding a transformer has a performance cost and the default property path is already very heavy, using a simple mapper gets rid of both and this may count when you have thousands or millions of requests running complex code.
Using inherit_data does not play well with initialization and events which may be relied on for advanced cases.

Doing nothing won't work either as the component expects the type norm data to be an array, an object or null for a compound form, so it throws an exception on initializing data when trying to map the scalar data to the child. This cannot be changed in my case since I want to dynamically add this nested form.

Note that the form is compound by default anyway when extending AbstractType which has FormType as parent defining it. TextType originally simply extends it and override the compound option to false, and trying to add a child to it throws an exception.

There still are valid cases for using compound data (array and objects) with non compound types as long as we use a model transformer. For instance, a model date can be a text or a timestamp while the date form is compound when using select inputs or many text inputs, it then depends on a model transformer to normalize it to a date time object, a view transformer is also needed so the object is converting so a string for a single input or an array of sting that will be mapped to children choice or text fields).

Hope it helps while waiting for an improved docs.

jaikdean · 2018-04-04T08:06:32Z

I'd love for this PR to be resurrected. I've been using Symfony since it was released and only learned about data mappers yesterday.

wouterj · 2018-05-08T18:01:54Z

Sorry for ignoring this PR for that long. If I'm correct, I fixed all comments in this article. Let's do a final review + merge @symfony/team-symfony-docs

xabbuh · 2018-05-09T10:06:20Z

I think I would still add a warning that any exception being thrown in the constructor will not be handled or needs some extra code.

javiereguiluz · 2018-06-01T08:47:30Z

@wouterj do you agree with the last comment from @xabbuh? If you do, please let's make a one final effort to finish this PR. Thank you!

magnetik · 2018-09-27T06:49:07Z

Hey @wouterj do you need someone to take this over to finish it?

I'm willing to help but I'm not sure that I understood the limitation that needs to be explained.

HeahDude · 2018-12-08T15:09:59Z

Thank you @wouterj for your work. We continuated it in #10756, would be nice to have your review there. Closing here though, thanks!

@wouterj

This PR was squashed before being merged into the 3.4 branch (closes #10756). Discussion ---------- [Forms] Added data-mapper docs #SymfonyConHackday2018 Improved version of #6900 Thank you @wouterj for a great job and the initial text! Thanx @HeahDude for helping me with this PR. Commits ------- b1cb1c0 [Forms] Added data-mapper docs

wouterj added the Form label Aug 21, 2016

carsonbot added the Status: Needs Review label Aug 21, 2016

wouterj force-pushed the issue-5439/form-data-mappers branch 2 times, most recently from cdc959c to 1da6c48 Compare August 22, 2016 07:28

snoek09 reviewed Aug 27, 2016
View reviewed changes

weaverryan reviewed Sep 19, 2016

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Sep 19, 2016

xabbuh reviewed Sep 21, 2016

View reviewed changes

xabbuh added the needs comments label Sep 21, 2016

HeahDude reviewed Sep 21, 2016

View reviewed changes

yceruto reviewed Oct 6, 2016

View reviewed changes

ogizanagi mentioned this pull request Nov 6, 2016

[Form] Add documentation for DataMapperInterface #7121

Closed

HeahDude mentioned this pull request Nov 15, 2016

[3.0] [Form] Add/remove collection rows in PATCH requests without sending the existing rows symfony/symfony#8301

Closed

Guite mentioned this pull request Jan 26, 2018

Consider support of command pattern and DTO Guite/MostGenerator#724

Closed

weaverryan mentioned this pull request Mar 25, 2018

Deprecating doctrine:generate:entities and pointing to MakerBundle doctrine/DoctrineBundle#790

Merged

wouterj force-pushed the issue-5439/form-data-mappers branch from 1da6c48 to bc61395 Compare May 8, 2018 16:55

Wrote article about Data Mappers

ba0526a

wouterj force-pushed the issue-5439/form-data-mappers branch from bc61395 to ba0526a Compare May 8, 2018 18:01

vudaltsov mentioned this pull request Dec 8, 2018

[Forms] Added data-mapper docs #10756

Closed

HeahDude closed this Dec 8, 2018

javiereguiluz added Waiting feedback and removed needs comments labels Jan 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Form] Finally document Data Mappers #6900

[Form] Finally document Data Mappers #6900

wouterj commented Aug 21, 2016 •

edited by javiereguiluz

Loading

snoek09 Aug 27, 2016 •

edited

Loading

This comment was marked as resolved.

weaverryan Sep 18, 2016

weaverryan Sep 18, 2016

weaverryan Sep 18, 2016

weaverryan Sep 18, 2016

weaverryan Sep 18, 2016

weaverryan Sep 19, 2016

weaverryan commented Sep 19, 2016

xabbuh Sep 21, 2016

HeahDude Sep 21, 2016 •

edited

Loading

ogizanagi Sep 21, 2016 •

edited

Loading

HeahDude Sep 24, 2016

ogizanagi Sep 24, 2016 •

edited

Loading

xabbuh commented Sep 21, 2016

HeahDude Sep 21, 2016

wouterj May 8, 2018

HeahDude Sep 21, 2016

HeahDude Sep 21, 2016

wouterj May 8, 2018

HeahDude Sep 21, 2016

yceruto Oct 6, 2016

mpdude commented Oct 27, 2016

mpdude commented Oct 27, 2016

B-Galati commented Jun 13, 2017

HeahDude commented Jun 23, 2017

jaikdean commented Apr 4, 2018

wouterj commented May 8, 2018

xabbuh commented May 9, 2018

javiereguiluz commented Jun 1, 2018

magnetik commented Sep 27, 2018

HeahDude commented Dec 8, 2018

[Form] Finally document Data Mappers #6900

[Form] Finally document Data Mappers #6900

Conversation

wouterj commented Aug 21, 2016 • edited by javiereguiluz Loading

snoek09 Aug 27, 2016 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weaverryan commented Sep 19, 2016

Choose a reason for hiding this comment

HeahDude Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

ogizanagi Sep 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ogizanagi Sep 24, 2016 • edited Loading

Choose a reason for hiding this comment

xabbuh commented Sep 21, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mpdude commented Oct 27, 2016

mpdude commented Oct 27, 2016

B-Galati commented Jun 13, 2017

HeahDude commented Jun 23, 2017

jaikdean commented Apr 4, 2018

wouterj commented May 8, 2018

xabbuh commented May 9, 2018

javiereguiluz commented Jun 1, 2018

magnetik commented Sep 27, 2018

HeahDude commented Dec 8, 2018

wouterj commented Aug 21, 2016 •

edited by javiereguiluz

Loading

snoek09 Aug 27, 2016 •

edited

Loading

HeahDude Sep 21, 2016 •

edited

Loading

ogizanagi Sep 21, 2016 •

edited

Loading

ogizanagi Sep 24, 2016 •

edited

Loading