Skip to content

Conversation

nikophil
Copy link
Contributor

@nikophil nikophil commented Mar 8, 2023

Q A
Branch? 3.1
License MIT

PropertyFilter does not presently work with arrays of objects which are not ApiResource: it just always serializes these objects as null arrays.

FYI I've tested with ArrayCollection of objects which are not ApiResource and it works like a charm

@nikophil nikophil force-pushed the fix/property-filter branch 3 times, most recently from fb3eddb to 6aae5b9 Compare March 9, 2023 18:06
@nikophil nikophil changed the title fix(PropertyFilter): should apply to arrays as well fix(serializer): should apply to arrays as well Mar 9, 2023
@nikophil nikophil changed the title fix(serializer): should apply to arrays as well fix(serializer): PropertyFilter should apply to arrays as well Mar 9, 2023
@nikophil nikophil force-pushed the fix/property-filter branch from 6aae5b9 to f071bcb Compare March 9, 2023 18:07

if ($type && $type->getBuiltinType() === 'array') {
$childContext = $this->createChildContext($context, $attribute, $format);
unset($childContext['iri'], $childContext['uri_variables']);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

todo: check if we shouldn't remove operation also

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you explain me what choices lead to unset() these contexts?
because we're serializing "non api resource" objects, there would be no need to have the operation available in the context?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because it'll re-use the old operation if any when creating an IRI

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless I'm misunderstanding something, no IRI should be generated, since we're dealing with non-apiresource objects?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they have Skolem IRIs unless disabled by using the genId: false attribute.

soyuka added a commit to soyuka/core that referenced this pull request Mar 9, 2023
@nikophil nikophil changed the title fix(serializer): PropertyFilter should apply to arrays as well fix(serializer): propertyFilter should apply to arrays as well Mar 10, 2023
@nikophil nikophil force-pushed the fix/property-filter branch from f071bcb to c845ea3 Compare March 10, 2023 09:40
fix(PropertyFilter): should apply to arrays as well
@nikophil nikophil force-pushed the fix/property-filter branch from c845ea3 to 20210d1 Compare March 13, 2023 08:15
@soyuka soyuka merged commit 9421ba5 into api-platform:3.1 Mar 13, 2023
@nikophil
Copy link
Contributor Author

🎉

soyuka pushed a commit to soyuka/core that referenced this pull request Mar 17, 2023
@temp
Copy link

temp commented May 3, 2023

This MR broke our code in unexpected ways, we now receive "Unable to generate an IRI for the item of type X" in our unit tests.
Our resource "Wheelset" has a property of type array, and Wheel has a property of type WheelManufacturer, which is a resource itself.
The wheel-array is now normalized within the if-block of this MR in AbstractItemNormalizer, and leads deep down in IriConverter and IdentifiersExtractor to the above error. We haven't got a clue what to change. When we comment out the new if-block, everything works as expected. Seems like with the new if-block it tried to generate an IRI, without it it doesn't try to generate an IRI for the nested resource.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants