-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Serializer] add UidNormalizer #36406
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
Conversation
src/Symfony/Bundle/FrameworkBundle/Resources/config/serializer.xml
Outdated
Show resolved
Hide resolved
3e2abfb
to
74235ab
Compare
5ff8932
to
abe7069
Compare
@@ -18,6 +18,7 @@ CHANGELOG | |||
* Made `BrowserKitAssertionsTrait` report the original error message in case of a failure | |||
* Added ability for `config:dump-reference` and `debug:config` to dump and debug kernel container extension configuration. | |||
* Deprecated `session.attribute_bag` service and `session.flash_bag` service. | |||
* Added `UidNormalizer` to the framework serializer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
<service id="serializer.normalizer.uid" class="Symfony\Component\Serializer\Normalizer\UidNormalizer"> | ||
<!-- Run before serializer.normalizer.object --> | ||
<tag name="serializer.normalizer" priority="-880" /> | ||
</service> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be moved to the new PHP config file.
For the priority, I would use -915 to be consistent with the other very specialized normalizers.
@@ -8,6 +8,7 @@ CHANGELOG | |||
* added support for `\stdClass` to `ObjectNormalizer` | |||
* added the ability to ignore properties using metadata (e.g. `@Symfony\Component\Serializer\Annotation\Ignore`) | |||
* added an option to serialize constraint violations payloads (e.g. severity) | |||
* added `UidNormalizer` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be moved to the 5.2.0 section.
*/ | ||
public function normalize($object, string $format = null, array $context = []) | ||
{ | ||
if (!$object instanceof AbstractUid) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition can be removed as the supports method garuantees that already.
/** | ||
* {@inheritdoc} | ||
* | ||
* @throws InvalidArgumentException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
/** | ||
* {@inheritdoc} | ||
* | ||
* @throws NotNormalizableValueException |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
*/ | ||
public function denormalize($data, string $type, string $format = null, array $context = []) | ||
{ | ||
if (!class_exists(AbstractUid::class)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
try { | ||
$uid = Ulid::class === $type ? Ulid::fromString($data) : Uuid::fromString($data); | ||
} catch (\InvalidArgumentException $exception) { | ||
throw new NotNormalizableValueException('The data is not a valid '.$type.' string representation.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You should use sprintf here.
@guillbdx Can you have a look at the feedback? |
Anyone willing to take over this PR? |
Closing in favor of #38151 |
This PR was merged into the 5.2-dev branch. Discussion ---------- [Serializer] add UidNormalizer | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | #36102 | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> UUID and ULID normalizer. Continuation of #36406 Commits ------- d6a8993 Update based on feedback 1c21c78 UidNormalizer.
UUID and ULID normalizer.