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

Fixed denormalization of a constructor argument which is a collection of non-resources #2859

Merged
merged 1 commit into from
Jun 18, 2019
Merged

Fixed denormalization of a constructor argument which is a collection of non-resources #2859

merged 1 commit into from
Jun 18, 2019

Conversation

karser
Copy link
Contributor

@karser karser commented Jun 13, 2019

Hi Folks!

After upgrading to 2.4.4 (from 2.4.2) I started getting an error.
The error wording is: The property path constructor needs a string or an instance of \"Symfony\\Component\\PropertyAccess\\PropertyPath\". Got: \"integer\" in symfony/serializer/Normalizer/AbstractObjectNormalizer.php, line 302.

I fixed this bug by adding '[]' to the class of the collection argument at this line.

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets PR adds the test case
License MIT
Doc PR api-platform/docs#1234

@soyuka
Copy link
Member

soyuka commented Jun 14, 2019

Mhhh shouldn't this case go through the normalizeCollection few lines before?

@karser
Copy link
Contributor Author

karser commented Jun 14, 2019

The few lines before case is for resource collection. It checks for isResourceClass. The case that I fixed with [] is apparently, for non-resource collection.
The createAttributeValue method was added in commit Fix ResourceClassResolver handling of inheritance. @teohhanhui Is this PR correct?

@teohhanhui
Copy link
Contributor

Yes, you're right. The [] suffix is important for ArrayDenormalizer. I'll check soon.

@teohhanhui
Copy link
Contributor

teohhanhui commented Jun 14, 2019

@karser You need to to do the same for the corresponding class mapped as MongoDB Document: https://github.com/api-platform/core/blob/v2.4.4/tests/Fixtures/TestBundle/Document/DummyEntityWithConstructor.php

@teohhanhui teohhanhui added the bug label Jun 14, 2019
@teohhanhui teohhanhui self-assigned this Jun 14, 2019
@karser
Copy link
Contributor Author

karser commented Jun 14, 2019

@teohhanhui done. MongoDB documents reuse behat features, don't they?

@teohhanhui
Copy link
Contributor

teohhanhui commented Jun 14, 2019

Hmm... Not sure why the same test for mongodb is still failing. I'll investigate.

@teohhanhui
Copy link
Contributor

Yikes. Looks like we're expecting the same order for the JSON keys, which is nonsensical.

@karser
Copy link
Contributor Author

karser commented Jun 14, 2019

Not fully following and it's not evident what you changed. I added $items in the corresponding places in the entity and document. If test passes for the entity, the order of json keys in the output shouldn't be different for the document.

@teohhanhui
Copy link
Contributor

It's because you added the getItems method in a different order in the document compared to the entity, so.... 🙈

@karser
Copy link
Contributor Author

karser commented Jun 14, 2019

I see now, ugh!

@karser
Copy link
Contributor Author

karser commented Jun 14, 2019

Just wondering, since the entities and documents must be 100% identical, wouldn't it make sense to use the same classes and move the orm and odm annotations to xml files?

@teohhanhui teohhanhui merged commit 8e343fe into api-platform:2.4 Jun 18, 2019
@teohhanhui
Copy link
Contributor

Thanks @karser! 🎉 🚀

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.

None yet

4 participants