Skip to content

[Serializer][PropertyInfo] Missing PropertyTypeExtractorInterface alias #27139

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

Closed
mdeboer opened this issue May 3, 2018 · 1 comment
Closed

Comments

@mdeboer
Copy link
Contributor

mdeboer commented May 3, 2018

Q A
Bug report? yes
Feature request? no
BC Break report? uncertain
RFC? no
Symfony version 3.3, 3.4, 4.x

Working with the Symfony serializer gave me quite a few headaches and this problem was no different. However the solution was easy yet I'm uncertain if this is the right approach and really a bug.

I found that nested objects (e.g. User having a Group) had problems deserializing. User deserializing went fine upon the point where it tries to set the group using setGroup(Group $group) which obviously requires a Group argument and it complains that an array was given instead. Also never an attempt was made to deserialize the Group.

So it knew it expected a Group but yet it ignored it and simply went on and then was like "I told you so, stupid.".

After an hour I finally figured out that the PropertyTypeExtractorInterface constructor argument was null even though property_info was enabled in the framework settings.

TL;DR;
An alias for Symfony\Component\PropertyInfo\PropertyTypeExtractorInterface to @property_info is missing from the service definitions in symfony/framework-bundle/Resources/config/property_info.xml

Edit: In hindsight I notice it's argument is given explicitly in the normalizer service definitions. Still I wonder why was chosen for this approach instead of aliasing the interface so custom normalizers can be autowired.

Please let me know if/why was chosen not to add this alias by default and if there is any other way to get the same functionality. I'd be happy to provide a PR.

@sroze
Copy link
Contributor

sroze commented May 30, 2018

Good shot. The original alias was added in #22615. I just opened #27430 to add the other one.

@fabpot fabpot closed this as completed May 31, 2018
fabpot added a commit that referenced this issue May 31, 2018
…extractor (sroze)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[PropertyInfo] Add an alias to the property info type extractor

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27139
| License       | MIT
| Doc PR        | ø

As spotted in #27139, we only alias the main PropertyInfo interface but we don't alias the other one, gathering only the types (while we implement it). This fixes the "problem" of auto-wiring it.

Commits
-------

bbbcd46 Add an alias to the property info type extractor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants