Skip to content

Add new serializer empty_data feature doc#8914

Merged
javiereguiluz merged 3 commits intosymfony:4.1from
Nek-:add-feature-25493
Jun 27, 2018
Merged

Add new serializer empty_data feature doc#8914
javiereguiluz merged 3 commits intosymfony:4.1from
Nek-:add-feature-25493

Conversation

@Nek-
Copy link
Copy Markdown
Contributor

@Nek- Nek- commented Dec 20, 2017

This is the documentation related to the following feature: symfony/symfony#25493

@Nek- Nek- mentioned this pull request Dec 20, 2017
Closed
@javiereguiluz javiereguiluz added the Waiting Code Merge Docs for features pending to be merged label Dec 23, 2017
@xabbuh xabbuh added this to the 4.1 milestone Jan 2, 2018
@Nek- Nek- force-pushed the add-feature-25493 branch 2 times, most recently from c4d61f9 to 2ba3fdd Compare January 15, 2018 06:52
fabpot
fabpot requested changes Jan 19, 2018
View reviewed changes
components/serializer.rst Outdated
----------------------

Value Objets are difficult to handle because they often require parameters in the constructor. If the input omit one
of theses parameters the serializer will throw an exeception because it can't create the object.
Copy link
Copy Markdown
Member

@fabpot fabpot Jan 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo exeception

symfony-splitter pushed a commit to symfony/serializer that referenced this pull request Jan 19, 2018
@fabpot
feature #25493 [Serializer] default_constructor_arguments context o…
a608dc4 
…ption for denormalization (Nek-)

This PR was squashed before being merged into the 4.1-dev branch (closes #25493).

Discussion
----------

[Serializer] `default_constructor_arguments` context option for denormalization

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | (there is no RFC for this)
| License       | MIT
| Doc PR        | symfony/symfony-docs#8914

## Problems

In the case you want to deserialize value-objects, if all the data required by its constructor are **not** given as input, the serializer will throw a simple `RuntimeException` exception. This makes impossible to catch it. (as current fix on my projects I use exception message to be sure to catch the good one X.x")

The second problem is a missing feature to fill the required object with an empty one. This needs to be defined by the user because the serializer can't guess how to build it.

Here is a project that exposes the problem of the current behavior. https://github.com/Nek-/api-platform-value-object

## Solutions suggested

I suggest a solution in 2 parts because the second part is more touchy.

1. Replace the current exception by a new specific one
2. Add a new `empty_data` option to the context of deserialization so you can specify data for objects impossible to instantiate, this is great because the serializer no more throw exception and the validator can then work as expected and send violations to the user. This solution is inspired by forms solutions to fix the issue with value objects

Here is what you can do with this feature:
```php
class DummyValueObject
{
    public function __construct($foo, $bar) { $this->foo = $foo; $this->bar = $bar; }
}

$empty = new DummyValueObject('', '');
$result = $normalizer->denormalize(['foo' => 'Hello'], DummyValueObject::class, 'json', [
    'empty_data' => [
        DummyValueObject::class => $empty,
    ],
]);

// It's impossible to construct a DummyValueObject with only "foo" value. So the serializer
// will replace it with the given empty data
```

There are 2 commits so I can quickly provide you only the first point if you want. Hope you'll like this.

## Solution after discussion

1. New exception `MissingConstructorArgumentsException`
2. New context option `default_constructor_arguments`

```php
class DummyValueObject
{
    public function __construct($foo, $bar) { $this->foo = $foo; $this->bar = $bar; }
}

$result = $normalizer->denormalize(['foo' => 'Hello'], DummyValueObject::class, 'json', [
    'default_constructor_arguments' => [
        DummyValueObject::class => ['foo' => '', 'bar' => ''],
    ],
]);

// DummyValueObject is contructed with the given `foo` and empty `bar`
```

Commits
-------

1523a85 [Serializer]  context option for denormalization
fabpot added a commit to symfony/symfony that referenced this pull request Jan 19, 2018
@fabpot
feature #25493 [Serializer] default_constructor_arguments context o…
2aa54b8 
…ption for denormalization (Nek-)

This PR was squashed before being merged into the 4.1-dev branch (closes #25493).

Discussion
----------

[Serializer] `default_constructor_arguments` context option for denormalization

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | (there is no RFC for this)
| License       | MIT
| Doc PR        | symfony/symfony-docs#8914

## Problems

In the case you want to deserialize value-objects, if all the data required by its constructor are **not** given as input, the serializer will throw a simple `RuntimeException` exception. This makes impossible to catch it. (as current fix on my projects I use exception message to be sure to catch the good one X.x")

The second problem is a missing feature to fill the required object with an empty one. This needs to be defined by the user because the serializer can't guess how to build it.

Here is a project that exposes the problem of the current behavior. https://github.com/Nek-/api-platform-value-object

## Solutions suggested

I suggest a solution in 2 parts because the second part is more touchy.

1. Replace the current exception by a new specific one
2. Add a new `empty_data` option to the context of deserialization so you can specify data for objects impossible to instantiate, this is great because the serializer no more throw exception and the validator can then work as expected and send violations to the user. This solution is inspired by forms solutions to fix the issue with value objects

Here is what you can do with this feature:
```php
class DummyValueObject
{
    public function __construct($foo, $bar) { $this->foo = $foo; $this->bar = $bar; }
}

$empty = new DummyValueObject('', '');
$result = $normalizer->denormalize(['foo' => 'Hello'], DummyValueObject::class, 'json', [
    'empty_data' => [
        DummyValueObject::class => $empty,
    ],
]);

// It's impossible to construct a DummyValueObject with only "foo" value. So the serializer
// will replace it with the given empty data
```

There are 2 commits so I can quickly provide you only the first point if you want. Hope you'll like this.

## Solution after discussion

1. New exception `MissingConstructorArgumentsException`
2. New context option `default_constructor_arguments`

```php
class DummyValueObject
{
    public function __construct($foo, $bar) { $this->foo = $foo; $this->bar = $bar; }
}

$result = $normalizer->denormalize(['foo' => 'Hello'], DummyValueObject::class, 'json', [
    'default_constructor_arguments' => [
        DummyValueObject::class => ['foo' => '', 'bar' => ''],
    ],
]);

// DummyValueObject is contructed with the given `foo` and empty `bar`
```

Commits
-------

1523a85 [Serializer]  context option for denormalization
@xabbuh xabbuh added Serializer and removed Waiting Code Merge Docs for features pending to be merged labels Jan 22, 2018
@Nek- Nek- force-pushed the add-feature-25493 branch from 2ba3fdd to 844154e Compare February 1, 2018 14:08
@Nek-
Copy link
Copy Markdown
Contributor Author

Nek- commented Feb 1, 2018

@fabpot fixed

javiereguiluz
javiereguiluz approved these changes Feb 20, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@javiereguiluz javiereguiluz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Nek- thanks for contributing this doc. I liked it a lot and everything is well explained. I just made a minor reword to focus more on the general use case ("Handling Constructor Arguments") instead of the "value object" detail. Thanks!

@xabbuh xabbuh changed the base branch from master to 4.1 May 8, 2018 08:43
dunglas
dunglas reviewed May 16, 2018
View reviewed changes
components/serializer.rst
Handling Constructor Arguments
------------------------------

If the constructor of a class defines arguments, as usually happens with
Copy link
Copy Markdown
Member

@dunglas dunglas May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not exact: the serializer deals well with constructor arguments, if they are present in the data to deserialize.

The default_constructor_arguments is useful only when the value has not been provided in the data to deserialize.

@Nek- Nek- force-pushed the add-feature-25493 branch from a81b7bf to db38bad Compare June 4, 2018 17:23
@Nek- Nek- force-pushed the add-feature-25493 branch from db38bad to 9f31bbb Compare June 4, 2018 17:29
@Nek-
Copy link
Copy Markdown
Contributor Author

Nek- commented Jun 4, 2018

Fixed and rebased. 👍

@javiereguiluz
Copy link
Copy Markdown
Member

javiereguiluz commented Jun 27, 2018

@Nek- I'm sorry it took us so long to merge. Thanks!

@javiereguiluz javiereguiluz merged commit 9f31bbb into symfony:4.1 Jun 27, 2018
javiereguiluz added a commit that referenced this pull request Jun 27, 2018
@javiereguiluz
minor #8914 Add new serializer empty_data feature doc (Nek-, javiereg…
cab06a1 
…uiluz)

This PR was merged into the 4.1 branch.

Discussion
----------

Add new serializer empty_data feature doc

This is the documentation related to the following feature: symfony/symfony#25493

Commits
-------

9f31bbb Fix not precise enough sentence
b0d3fe8 Minor reword
5a48201 Add new serializer empty_data feature doc
@Nek- Nek- deleted the add-feature-25493 branch June 27, 2018 14:19
@javiereguiluz javiereguiluz mentioned this pull request Jun 29, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fabpot fabpot fabpot requested changes

@javiereguiluz javiereguiluz javiereguiluz approved these changes

+1 more reviewer

@dunglas dunglas dunglas left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Serializer Status: Reviewed

Projects

None yet

Milestone

4.1

Development

Successfully merging this pull request may close these issues.

6 participants

@Nek- @javiereguiluz @fabpot @dunglas @xabbuh @carsonbot