-
-
Notifications
You must be signed in to change notification settings - Fork 901
additional_allowed_attributes #6274
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
Conversation
@@ -420,7 +420,7 @@ protected function getAllowedAttributes(string|object $classOrObject, array $con | |||
$options = $this->getFactoryOptions($context); | |||
$propertyNames = $this->propertyNameCollectionFactory->create($resourceClass, $options); | |||
|
|||
$allowedAttributes = []; | |||
$allowedAttributes = $context['additional_allowed_attributes'] ?? []; |
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.
is this a symfony option?
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.
No, allow_extra_attributes
is Symfony, and additional_allowed_attributes
is something I added.
As I can see Symfony AbstractObjectNormalizer is extended by API-Platform AbstractItemNormalizer where getAllowedAttributes()
is overeaten. I added there a new additional_allowed_attributes
option to allow additional attributes when allow_extra_attributes
is set.
I don't see any bulletin option to do this (maybe I missed it?) and later AbstractItemNormalizer extended again and made final so getAllowedAttributes()
cannot be modified again by the API-Platform user.
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.
Isn't allow_extra_attributes
working?
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.
allow_extra_attributes
is Symfony way of making the system throw a validation error if the API has extra attributes. It works and is needed for additional_allowed_attributes
to have a meaning.
additional_allowed_attributes
comes to solve the problem of when you need to whitelist some attributes when using allow_extra_attributes = false
.
For example, as described in https://api-platform.com/docs/core/serialization/#denormalization
If an @id key is present in the embedded resource, then the object corresponding to the given URI will be retrieved through the state provider. Any changes in the embedded relation will also be applied to that object.
When trying to do this in combination with allow_extra_attributes = false
you will get an error that @id
is not valid as it doesn't exist on the object. to solve this you can add, in addition, the config 'additional_allowed_attributes' => ['@id']
to tell the deserializer that even that extra attributes are not allowed, and the object doesn't have an @id
property, allow it anyway.
Additionally, this also lets you whitelist other attributes you wish to allow while using allow_extra_attributes = false
, for example 'additional_allowed_attributes' => ['@id', '@type', 'id', '@context', 'extraParam']
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.
Okay I understand now, so you want allow_extra_attributes
to false
but denormalize a JSON-LD document. I think that this should work from scratch, @dunglas any thoughts?
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.
Possibly. My use case is basically I want to POST a new record /action
(including sub-records) and then edit the response and POST it again like /action/1/submit
.
So for the sub-records to work I must have the @id
s. Additionally, for ease of use, I also like to allow the other stuff so the user will not need to remove it while resubmitting. So in my use-case with this fix I used 'additional_allowed_attributes' => ['@id', '@type', 'id', '@context']
.
So allowing ['@id', '@type', 'id', '@context']
out of the box or adding 'additional_allowed_attributes'
dynamic option, or possibly both so it will work out of the box and be extendable works for me.
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.
Appreciate it if this (additional_allowed_attributes
) gets approved or alternatively, if you want to go with a hardcoded solution as @soyuka suggested like allowing @id
or ['@id', '@type', 'id', '@context']
in src/JsonLd/Serializer/ItemNormalizer.php and allowing id
in src/JsonApi/Serializer/ItemNormalizer.php in some way.
@dunglas @soyuka
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'd like to avoid a new serialization context, can you check #6402 ?
Tested it on my project and it works and solves my use case and the id case from https://api-platform.com/docs/core/serialization/#denormalization. But then tried to add to test tests/JsonLd/Serializer/ItemNormalizerTest.php something like:
But the test fails not sure why. I probably set the test wrong?