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] Also validate callbacks when given in the normalizer context#30950
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
Uh oh!
There was an error while loading.Please reload this page.
Conversation
8a67f0c tocf6cfeeCompare
dbu 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.
thanks for bringing back the is_array validation that i accidentally dropped.
i am confused by this thing:
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
cf6cfee to071b51eComparesrc/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
eac3831 to6363306Comparesrc/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
src/Symfony/Component/Serializer/Normalizer/AbstractObjectNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas commentedApr 7, 2019 • 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.
Sorry I started commenting using regular comments, here is one more to thank you for this PR :) |
maxhelias 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.
With@nicolas-grekas's comments
7f78a0e tode5a875Comparedbu commentedApr 8, 2019
thanks. i improved the exception messages and cleaned up the max_depth_handler to only validate when we need, and to refuse |
src/Symfony/Component/Serializer/Normalizer/AbstractNormalizer.php OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
nicolas-grekas 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.
(once the typo is fixed)
4b8061b to502bf35Compare…ler are actually callable
dbu commentedApr 8, 2019
typo fixed. i rebased on 4.2 because there where (unrelated) failing tests. |
fabpot commentedApr 8, 2019
Thank you@dbu. |
…malizer context (dbu)This PR was merged into the 4.2 branch.Discussion----------[Serializer] Also validate callbacks when given in the normalizer context| Q | A| ------------- | ---| Branch? | 4.2 (callbacks are handled differently in 3.4)| Bug fix? | yes| New feature? | no| BC breaks? | no (unless somebody relied on this bug ignoring `null` as callback| Deprecations? | no| Tests pass? | yes| Fixed tickets | Related to#30888| License | MIT| Doc PR | -callbacks configuration for the normalizer is validated to be valid callbacks when using setCallbacks or using the callbacks field in the default options. however, it was not validated when using the callbacks field in a context passed to `normalize()`Commits-------3789152 [serializer] validate that the specified callbacks and max_depth_handler are actually callable
This PR was squashed before being merged into the 4.3-dev branch (closes#30888).Discussion----------[serializer] extract normalizer tests to traitseufossa| Q | A| ------------- | ---| Branch? | master| Bug fix? | no| New feature? | no| BC breaks? | no| Deprecations? | no| Tests pass? | yes| Fixed tickets | Relates to#30818| License | MIT| Doc PR | -As discussed with@joelwurtz, extract normalizer functionality tests into traits to ensure consistent behaviour of all normalizers.* [x] Rebase when#30977,#30950 and#30907 are merged to master **blocker*** [x] Clean up order of trait inclusion and methods in the tests* [x] Clean up fixture classes of the traits. I started having one class named the same as the trait, where possibleStuff that we should do eventually, but can also do in separate pull requests, after this one has been merged:* [ ] Extract all features that we can (the existing normalizer tests should more or less only have the legacy tests in them, all functionality should be in trait)* [ ] Run test coverage and increase coverage so that we cover all important features and all relevant error cases.Commits-------2b6ebea [serializer] extract normalizer tests to traits
nullas callbackcallbacks configuration for the normalizer is validated to be valid callbacks when using setCallbacks or using the callbacks field in the default options. however, it was not validated when using the callbacks field in a context passed to
normalize()