Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[Serializer] Added a ConstraintViolationListNormalizer#22150
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[Serializer] Added a ConstraintViolationListNormalizer#22150
Uh oh!
There was an error while loading.Please reload this page.
Conversation
7fa114c to1bea195Comparesrc/Symfony/Bundle/FrameworkBundle/DependencyInjection/FrameworkExtension.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
javiereguiluz left a comment
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.
👍 it looks like a useful feature to me! Thanks@lyrixx.
src/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
1bea195 to72d93aeComparesrc/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
93d6d61 to9543d83Comparesrc/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
dunglas commentedMar 28, 2017
The original implementation (https://github.com/api-platform/core/blob/master/src/Hydra/Serializer/ConstraintViolationListNormalizer.php) was following the Hydra W3C draft. WDYT about keeping Hydra support in Symfony? |
dunglas commentedMar 28, 2017
Another alternative is to implement RFC7807 (supported by API Platform and Zend Apigility):https://github.com/api-platform/core/blob/master/src/Problem/Serializer/ConstraintViolationListNormalizer.php |
javiereguiluz commentedMar 28, 2017
RFC7807 looks like the ideal solution to create something standard ... but its status is still: "PROPOSED STANDARD". Can we guess the chances of being changed a lot in the future or even rejected? |
3d9be72 to13aabb3Comparelyrixx commentedMar 29, 2017
I implemented RFC7807 (supported by API Platform and Zend Apigility) |
fabpot commentedMar 29, 2017
Can you add a note about the fact that it implements RFC7807 in the phpdocs? |
src/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.phpShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
13aabb3 to6024134Comparelyrixx commentedApr 3, 2017
Comment addressed. PR updated. Note: I also renamed |
src/Symfony/Component/Serializer/Normalizer/ConstraintViolationListNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
dunglas commentedApr 3, 2017
Why this change? AFAIK, camel case is preferred (for instance, it's what Google and Schema.org use). |
lyrixx commentedApr 3, 2017
Ah ok. I will revert it so. Usually I prefer Snake Case in my array / API... |
6024134 to4d59983Comparelyrixx commentedApr 3, 2017
Reverted. The PR is now ready. |
lyrixx commentedOct 5, 2017
But it's not mandatory, and it's boring if you need to pass 4XX each time ... :/ |
b82f7a6 to82f6e0dComparelyrixx commentedFeb 16, 2018 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I have rebased the PR ; it's not ready |
82f6e0d to3bcbc5aComparelyrixx commentedMar 14, 2018
Ping |
fabpot commentedMar 22, 2018
@lyrixx "it's not ready" -> I suppose you meant "it's now ready", right? |
| of objects that needs data insertion in constructor | ||
| * added an optional`default_constructor_arguments` option of context to specify a default data in | ||
| case the object is not initializable by its constructor because of data missing | ||
| * added optional`bool $escapeFormulas = false` argument to`CsvEncoder::__construct` |
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.
Looks like you haven't added anything here.
| useSymfony\Component\Validator\ConstraintViolationListInterface; | ||
| /** | ||
| * A normalizer that normalize a ConstraintViolationListInterface instance. |
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.
normalizes
| /** | ||
| * A normalizer that normalize a ConstraintViolationListInterface instance. | ||
| * | ||
| * This Normalizer implements RFC7807 {@link https://tools.ietf.org/html/rfc7807}. |
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.
If we're not fully compliant, it should be added here.
3bcbc5a to971ddb5Comparelyrixx commentedMar 22, 2018
Oups, You are right. I have addressed your comment. It should be OK now |
fabpot left a comment
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.
minor comment
| /** | ||
| * A normalizer that normalizes a ConstraintViolationListInterface instance. | ||
| * | ||
| * This Normalizer implements partially RFC7807 {@link https://tools.ietf.org/html/rfc7807}. |
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.
I would sayimplements RFC7807 partially.
But can we be a bit more precise? Can we list what we do not support or how we diverge from the RFC? I think that would help our users.
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.
Actually I re-read the RFC and came up to the same conclusion as@teohhanhui'scomment.
We are allowed to omit thetype property.
That's why I removed the "partially" because the normalizer do implement the RFC.
971ddb5 to2a35d09Comparefabpot commentedMar 23, 2018
Thank you@lyrixx. |
… (lyrixx)This PR was merged into the 4.1-dev branch.Discussion----------[Serializer] Added a ConstraintViolationListNormalizer| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | yes| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets |#11309| License | MIT| Doc PR | ----It seems logical to me that Symfony is able to serialise natively some very common Symfony data structure. (and requested by@nicolas-grekas &@javiereguiluz )Usage example (from symfony/symfony-demo):```php /** *@route("", name="api_blog_new") *@method("POST") * @Security("is_granted('ROLE_ADMIN')") */ public function newAction(Request $request) { $data = $request->getContent(); $post = $this->get('serializer')->deserialize($data, Post::class, 'json', ['groups' => ['post_write']]); $post->setAuthor($this->getUser()); $violations = $this->get('validator')->validate($post); $post->setSlug($this->get('slugger')->slugify($post->getTitle())); if (count($violations) > 0) { $repr = $this->get('serializer')->serialize($violations, 'json'); return JsonResponse::fromJsonString($repr, 400); } $this->getDoctrine()->getManager()->persist($post); $this->getDoctrine()->getManager()->flush(); $repr = $this->get('serializer')->serialize($post, 'json', ['groups' => ['post_read']]); return JsonResponse::fromJsonString($repr); }```Commits-------2a35d09 [Serializer] Added a ConstraintViolationListNormalizer
| )); | ||
| $expected =array( | ||
| 'title' =>'An error occurred', |
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.
RFC 7807 is unambiguous on this point:
3.1. Members of a Problem Details Object
A problem details object can have the following members:
- "type" (string) - ... When
this member is not present, its value is assumed to be
"about:blank".
4.2. Predefined Problem Types
This specification reserves the use of one URI as a problem type:
The "about:blank" URI [RFC6694], when used as a problem type,
indicates that the problem has no additional semantics beyond that of
the HTTP status code.When "about:blank" is used, the title SHOULD be the same as the
recommended HTTP status phrase for that code (e.g., "Not Found" for
404, and so on), although it MAY be localized to suit client
preferences (expressed with the Accept-Language request header).Please note that according to how the "type" member is defined
(Section 3.1), the "about:blank" URI is the default value for that
member. Consequently, any problem details object not carrying an
explicit "type" member implicitly uses this URI.
Perhaps:
{"type":"/validation-error","title":"Validation failed",...}…izer's RFC7807 compliance (dunglas)This PR was squashed before being merged into the 4.1 branch (closes#27292).Discussion----------[Serializer] Fix and improve constraintViolationListNormalizer's RFC7807 compliance| Q | A| ------------- | ---| Branch? | 4.1| Bug fix? | yes| New feature? | no <!-- don't forget to update src/**/CHANGELOG.md files -->| BC breaks? | no <!-- seehttps://symfony.com/bc -->| Deprecations? | yes| Tests pass? | yes <!-- please add some, will be required by reviewers -->| Fixed tickets |#22150 (comment)| License | MIT| Doc PR | todoThis PR fixes and improves [RFC 7807](https://tools.ietf.org/html/rfc7807#section-3.2) compliance of `ConstraintViolationListNormalizer` (introduced in 4.1):* As recommended, use a specific namespace for Symfony validation error (`http://symfony.com/doc/current/validation.html`, because it already exists and gives information about the error.* Allow to set all properties defined in the RFC using the serialization context* Remove the `detail` key if no detail is provided (according to the spec)* Change the Symfony specific extension to use the same terminology than the RFC itself (type and title)* Use the proper `urn:uuid` scheme (RFC 4122) for the UUID code (more standard, and improve hypermedia capabilities).ping@teohhanhuiCommits-------3c789c6 [Serializer] Fix and improve constraintViolationListNormalizer's RFC7807 compliance
Uh oh!
There was an error while loading.Please reload this page.
It seems logical to me that Symfony is able to serialise natively some very common Symfony data structure. (and requested by@nicolas-grekas &@javiereguiluz )
Usage example (from symfony/symfony-demo):