Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Serializer] Allow to provide (de)normalization context in mapping #39399

Merged
merged 1 commit into from
Feb 16, 2021
Merged

[Serializer] Allow to provide (de)normalization context in mapping #39399

merged 1 commit into from
Feb 16, 2021

Conversation

ogizanagi
Copy link
Member

@ogizanagi ogizanagi commented Dec 9, 2020

Q A
Branch? 5.x
Bug fix? no
New feature? yes
Deprecations? no
Tickets Fix #39039
License MIT
Doc PR TODO

As explained in the linked feature request, this brings the ability to configure context on a per-property basis, using Serializer mapping.

Considering:

use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Context({ DateTimeNormalizer::FORMAT_KEY = 'Y-m-d' })
     */
    public \DateTime $date;

    public \DateTime $anotherDate;
}

$date will be formatted with a specific format, while $anotherDate will use the default configured one (or the one provided in the context while calling ->serialize() / ->normalize()).

It can also differentiate normalization and denormalization contexts:

use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Context(
     *   normalizationContext = { DateTimeNormalizer::FORMAT_KEY = 'Y-m-d' },
     *   denormalizationContext = { DateTimeNormalizer::FORMAT_KEY = \DateTime::COOKIE },
     * )
     */
    public \DateTime $date;
}

As well as act differently depending on groups:

use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Groups({ "extended" })
     * @Serializer\Context({ DateTimeNormalizer::FORMAT_KEY = \DateTime::RFC3339 })
     * @Serializer\Context(
     *   context = { DateTimeNormalizer::FORMAT_KEY = \DateTime::RFC3339_EXTENDED },
     *   groups = {"extended"},
     * )
     */
    public \DateTime $date;
}

The annotation can be repeated as much as you want to handle the different cases.
Context without groups is always applied first, then context for groups are merged in the provided order.
Context provided when calling ->serialize() / ->normalize() acts as the defaults for the properties without context provided in the metadata.

XML mapping (see tests) is a lot verbose due to the required structure to handle groups.

Such metadata contexts are also forwarded to name converters, max depth handlers, callbacks, ...

Of course, PHP 8 attributes are also supported:

use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    #[Serializer\Groups(["extended"])]
    #[Serializer\Context([DateTimeNormalizer::FORMAT_KEY => \DateTime::RFC3339])]
    #[Serializer\Context(
      context: [DateTimeNormalizer::FORMAT_KEY => \DateTime::RFC3339_EXTENDED],
      groups: ["extended"],
    )]
    public \DateTime $date;
}

The PR should be ready for first batch of reviews / discussions.

  • Make Fabbot happy in 5.2
  • Missing @Context unit tests
  • rework xml & phpize values
  • Fix lowest build issue with annotations => bumped doctrine annotations to 1.7, as for other components

@dunglas
Copy link
Member

dunglas commented Dec 9, 2020

I haven't reviewed the code yet but I like the idea! Could we also allow to set this annotation on classes? In this case, the context will be applied to the class and all its properties.

public $normalizationContexts = [];

/**
* @var array[] Denormalization contexts per group name ("*" applies to all groups)
Copy link
Member Author

@ogizanagi ogizanagi Dec 9, 2020

Choose a reason for hiding this comment

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

Using * should be fine since #33540 already relies on this. So no one should name their groups *.
Otherwise, previous version used an empty string, but we could simply opt for a distinct defaultDenormalizationContext property as well.

@ogizanagi
Copy link
Member Author

ogizanagi commented Dec 9, 2020

I haven't reviewed the code yet but I like the idea! Could we also allow to set this annotation on classes? In this case, the context will be applied to the class and all its properties.

Indeed, I also had this case in mind. I'd say let's polish this one first and iterate over. I don't think it would bring any compatibility issue (anyway we have time until 5.3 release).

Note that applying a context on an object property should already do it right now. But we miss a way to add it on class-level, for the root object for instance.

@ogizanagi ogizanagi added this to the 5.x milestone Dec 9, 2020
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

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

Looks good!

symfony-splitter pushed a commit to symfony/validator that referenced this pull request Dec 17, 2020
…ces (ogizanagi)

This PR was merged into the 4.4 branch.

Discussion
----------

Normalize exceptions messages containing methods references

| Q             | A
| ------------- | ---
| Branch?       | 4.4 <!-- see below -->
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix symfony/symfony#39399 (comment) <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | N/A

Normalizes across the codebase any exception message mentioning methods to contain a trailing `()`

(Seems OK on 5.1 and 5.2 branch after this on is merged up)

Commits
-------

e2da2acd6d Normalize exceptions messages containing methods references
@fabpot fabpot closed this in 5eeb957 Dec 18, 2020
@ogizanagi ogizanagi deleted the serializer-context-meta branch December 18, 2020 07:35
@ogizanagi ogizanagi restored the serializer-context-meta branch December 18, 2020 07:36
@ogizanagi ogizanagi reopened this Dec 18, 2020
* @author Maxime Steinhausser <maxime.steinhausser@gmail.com>
*/
#[\Attribute(\Attribute::TARGET_PROPERTY | \Attribute::TARGET_METHOD | \Attribute::IS_REPEATABLE)]
class Context
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class Context
final class Context

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. But we can imagine some legit use-cases for extending it. See #38993 (comment) for instance.
WDYT?

Copy link
Member

@derrabus derrabus left a comment

Choose a reason for hiding this comment

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

This feature could come in handy in my current project.

@fabpot
Copy link
Member

fabpot commented Feb 16, 2021

Thank you @ogizanagi.

@fabpot fabpot merged commit e2b1d9c into symfony:5.x Feb 16, 2021
@ogizanagi ogizanagi deleted the serializer-context-meta branch February 16, 2021 07:01
@fabpot fabpot mentioned this pull request Apr 18, 2021
hultberg pushed a commit to hultberg/symfony that referenced this pull request Sep 17, 2021
… context in mapping (ogizanagi)

This PR was squashed before being merged into the 5.3-dev branch.

Discussion
----------

[Serializer] Allow to provide (de)normalization context in mapping

| Q             | A
| ------------- | ---
| Branch?       | 5.x
| Bug fix?      | no
| New feature?  | yes <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix symfony#39039 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | TODO <!-- required for new features -->

As explained in the linked feature request, this brings the ability to configure context on a per-property basis, using Serializer mapping.

Considering:

```php
use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Context({ DateTimeNormalizer::FORMAT_KEY = 'Y-m-d' })
     */
    public \DateTime $date;

    public \DateTime $anotherDate;
}
```

`$date` will be formatted with a specific format, while `$anotherDate` will use the default configured one (or the one provided in the context while calling `->serialize()` / `->normalize()`).

It can also differentiate normalization and denormalization contexts:

```php
use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Context(
     *   normalizationContext = { DateTimeNormalizer::FORMAT_KEY = 'Y-m-d' },
     *   denormalizationContext = { DateTimeNormalizer::FORMAT_KEY = \DateTime::COOKIE },
     * )
     */
    public \DateTime $date;
}
```

As well as act differently depending on groups:

```php
use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    /**
     * @Serializer\Groups({ "extended" })
     * @Serializer\Context({ DateTimeNormalizer::FORMAT_KEY = \DateTime::RFC3339 })
     * @Serializer\Context(
     *   context = { DateTimeNormalizer::FORMAT_KEY = \DateTime::RFC3339_EXTENDED },
     *   groups = {"extended"},
     * )
     */
    public \DateTime $date;
}
```

The annotation can be repeated as much as you want to handle the different cases.
Context without groups is always applied first, then context for groups are merged in the provided order.
Context provided when calling `->serialize()` / `->normalize()` acts as the defaults for the properties without context provided in the metadata.

XML mapping (see tests) is a lot verbose due to the required structure to handle groups.

Such metadata contexts are also forwarded to name converters, max depth handlers, callbacks, ...

Of course, PHP 8 attributes are also supported:

```php
use Symfony\Component\Serializer\Annotation as Serializer;
use Symfony\Component\Serializer\Normalizer\DateTimeNormalizer;

class Foo
{
    #[Serializer\Groups(["extended"])]
    #[Serializer\Context([DateTimeNormalizer::FORMAT_KEY => \DateTime::RFC3339])]
    #[Serializer\Context(
      context: [DateTimeNormalizer::FORMAT_KEY => \DateTime::RFC3339_EXTENDED],
      groups: ["extended"],
    )]
    public \DateTime $date;
}
```

The PR should be ready for first batch of reviews / discussions.

- [x] Make Fabbot happy in 5.2
- [x] Missing `@Context` unit tests
- [x] rework xml & phpize values
- [x] Fix lowest build issue with annotations => bumped doctrine annotations to 1.7, as for other components

Commits
-------

7229fa1 [Serializer] Allow to provide (de)normalization context in mapping
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Serializer] Add a way to provide a context in mapping
9 participants