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

fix(jsonld): allow @id, @context and @type on denormalization #6402

Closed
wants to merge 1 commit into from

Conversation

soyuka
Copy link
Member

@soyuka soyuka commented May 31, 2024

@ili101
Copy link
Contributor

ili101 commented May 31, 2024

General question. I think I saw it written somewhere but I can't find it now. What is the easiest way to test a fork? like in an existing api-platform template clone, replace the /vendor/api-platform/core with the peached version to see if it works.

Anyway I think it's not working as in Symfony here
isAllowedAttribute() is not called if it's not in $allowedAttributes that is set here from:

protected function getAllowedAttributes(string|object $classOrObject, array $context, bool $attributesAsString = false): array|bool
{
if (!$this->resourceClassResolver->isResourceClass($context['resource_class'])) {
return parent::getAllowedAttributes($classOrObject, $context, $attributesAsString);
}
$resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']); // fix for abstract classes and interfaces
$options = $this->getFactoryOptions($context);
$propertyNames = $this->propertyNameCollectionFactory->create($resourceClass, $options);
$allowedAttributes = [];
foreach ($propertyNames as $propertyName) {
$propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName, $options);
if (
$this->isAllowedAttribute($classOrObject, $propertyName, null, $context)
&& (isset($context['api_normalize']) && $propertyMetadata->isReadable()
|| isset($context['api_denormalize']) && ($propertyMetadata->isWritable() || !\is_object($classOrObject) && $propertyMetadata->isInitializable())
)
) {
$allowedAttributes[] = $propertyName;
}
}
return $allowedAttributes;
}

where $allowedAttributes = []; is empty and gets filled only with items from the object && isAllowedAttribute() and in our case there are not in the object so isAllowedAttribute() is not checked.

@soyuka
Copy link
Member Author

soyuka commented Jun 14, 2024

Hi, the best way to test the branch is to use https://getcomposer.org/doc/05-repositories.md#vcs with the fork and the branch

@ili101
Copy link
Contributor

ili101 commented Jun 30, 2024

So as I said above #6402 (comment) this seems not to have any effect.
I don't quite understand Symfony's point of isAllowedAttribute() if it's being superseded by getAllowedAttributes().
But this approach I made before works #6274. So using the same approach but instead of adding a new context option I just did what you did and added the items. branch: https://github.com/ili101/api-platform-core/tree/id (ili101@7f48307)
Edit: moved to JsonLd ItemNormalizer ili101@ad132ad

And with this it's possible to use the @id as described in the documentation while using allow_extra_attributes: false

@soyuka
Copy link
Member Author

soyuka commented Jul 3, 2024

nice, can you open a new PR to replace mine?

@soyuka
Copy link
Member Author

soyuka commented Jul 4, 2024

superseeded by #6451

@soyuka soyuka closed this Jul 4, 2024
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.

2 participants