Skip to content

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.

Sign up for GitHub

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
wants to merge 1 commit into from
Closed

additional_allowed_attributes #6274

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Serializer/AbstractItemNormalizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'] ?? [];
Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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']

Copy link
Member

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?

Copy link
Contributor Author

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 @ids. 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.

Copy link
Contributor Author

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

Copy link
Member

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 ?

foreach ($propertyNames as $propertyName) {
$propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName, $options);

Expand Down