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

PR #5604 breaks operations on subresources for PUT operation on Resource when placeholders are named differently than "id" #5736

Closed
Aerendir opened this issue Aug 9, 2023 · 2 comments

Comments

@Aerendir
Copy link
Contributor

Aerendir commented Aug 9, 2023

API Platform version(s) affected: 3.1.13

Description
With PR #5546, was introduced the ability to operate on subresources also if the placeholders of the IDs are not named "id".

The PR #5604 actually breaks this feature.

If placeholders are named "id", the code continues to work as there is a fallback in ItemNormalizer that returns $data['id'] if the key uri_variables is not set in the $context:

private function getContextUriVariables(array $data, $operation, array $context): array
{
if (!isset($context['uri_variables'])) {
return ['id' => $data['id']];
}

But, as the key uri_variables is now always removed, the method ItemNormalizer::getContextUriVariables() never works as it should.

The PR #5604 was published due to those issues:

Reading them behind the lines, they are originates from the same problem: both the Resource and the SubResource have the placeholder named "id".

This causes the confusion in ItemNormalizer that uses the wrong "id" key to retrieve the SubResource.

How to reproduce

Create two resources, for example Alpha and Beta
Create a field in Beta which is a relation to Alpha
Try to create/update Beta

Working on 3.1.12 | NOT working on 3.1.13

PATCH /betas/{beta_id}
{
  "alpha": "/alphas/{alpha_id}"
}

NOT Working on 3.1.12 | (Now) Working on 3.1.13

PATCH /betas/{id}
{
  "alpha": "/alphas/{id}"
}

Possible Solution

#5739

@soyuka
Copy link
Member

soyuka commented Aug 11, 2023

I could not reproduce, please give me a full reproducer, or even better try to make this test fail: https://github.com/api-platform/core/pull/5743/files#diff-9e1ca94383b69fcdb49b1cc19b6e03b2ae37dc880cf2aee0bcdb33fea87e98d6

@Aerendir
Copy link
Contributor Author

@soyuka , I've created a proof of concept of a failing scenario:

#5747

I have some problems running it, as it seems not finding the PUT endpoint for the resource.

Anyway, it reproduces the situation I have and that make versions >3.1.12 make my tests fail.

I added a tag to the scenario, so it can be run easily:

vendor/bin/behat --tags issue5736

Please, let me know if it is sufficient for you to understand the context and, if it is not, please, let me know and I will explain further ❤️

PS
I think this issue should be reopened as I suppose it was automatically closed by GitHub when the PR was merged...

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 a pull request may close this issue.

2 participants