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

v3.3.8: Embedded subresources no longer denormalizing #6465

Closed
paullallier opened this issue Jul 12, 2024 · 19 comments
Closed

v3.3.8: Embedded subresources no longer denormalizing #6465

paullallier opened this issue Jul 12, 2024 · 19 comments

Comments

@paullallier
Copy link
Contributor

paullallier commented Jul 12, 2024

API Platform version(s) affected: 3.3.8 (works in 3.3.7)

Description
I've got a PUT endpoint like this:
Screenshot 2024-07-12 at 13 06 18

I'm having trouble with the embedded /accounts/{id} IRI in 3.3.8, which I think relates to the changes to IriConverter in #6451.

In 3.3.7, it pulls the account entity from the database fine. In 3.3.8, it fails on this check, which was added in that PR, I think.
https://github.com/ili101/api-platform-core/blob/14cc145ce646fe4a799652f5b451d0140076cf18/src/Symfony/Routing/IriConverter.php#L81

At that point, my $context points to the Tax entity, but it would still find the GET operation for the Account entity in 3.3.7. Now it complains that I have an IRI for the wrong type of resource instead.

I suspect this might be an edge case that the tests are missing? (Or I'm using API-P in an unusual way again...)

@soyuka
Copy link
Member

soyuka commented Jul 13, 2024

when denormalizing a relation the context should be reset ($context should not point to Tax) can you give me your configuration?

@fvozar
Copy link

fvozar commented Jul 13, 2024

I have the same issue with 3.3.8.

Operation:

new Post(
	uriTemplate: '/venues/{id}/validate-settings',
	uriVariables: ['id' => new Link(fromClass: Venue::class)],
	status: 200,
	input: ValidateVenueSettingsInput::class,
	output: VenueSettingsValidationOutput::class,
	processor: VenueSettingsValidatorProvider::class,
),

Input DTO:

class ValidateVenueSettingsInput
{

	#[Assert\NotBlank]
	public EmailType $template; // enum

	#[Assert\NotBlank]
	public Language $language; // doctrine entity and API resource

}

The error I got: 'Invalid IRI "/api/languages/018f15ed-e7d1-7c98-a407-3de1522dcd63".'. It was caused precisely by the abovementioned code from paullallier.

@soyuka
Copy link
Member

soyuka commented Jul 13, 2024

If there's the provider I need a provider to reproduce, can anyone create a minimal repro? Also what's your standard_put value in the configuration?

@fvozar
Copy link

fvozar commented Jul 13, 2024

During debugging, the code didn't even reach the processor (I have a typo in the class name; it's not the provider). The error occurred during deserializing the input DTO class.

Standard put is set to true. But the operation is POST in my case.

@soyuka
Copy link
Member

soyuka commented Jul 13, 2024

What's your body during the POST operation? What content-type? I need more to create a reproducer.

@fvozar
Copy link

fvozar commented Jul 13, 2024

Here is the example repo: https://github.com/fvozar/input-dto-bug/tree/main

There are two entities Foo and Bar. I've created one Foo and two Bar objects.

Then I called endpoint POST /api/foo/1/validate with body

{
  "bar": "/api/bars/2"
}

The IRI with different value for ID is important here, because when I use "bar": "/api/bars/1", it's working. I think it's because the id values match.

Response:

{
  "@id": "/api/errors/400",
  "@type": "hydra:Error",
  "title": "An error occurred",
  "detail": "Invalid IRI \"/api/bars/2\".",
  "status": 400,
  "type": "/errors/400",
  "trace": [],
  "hydra:title": "An error occurred",
  "hydra:description": "Invalid IRI \"/api/bars/2\"."
}

Full curl:

curl -X 'POST' \
  'https://127.0.0.1:8001/api/foo/1/validate' \
  -H 'accept: application/ld+json' \
  -H 'Content-Type: application/ld+json' \
  -d '{
  "bar": "/api/bars/2"
}'

I haven't changed anything special in config, it's new project with Api platform installed.


For real-world example in my application I'm using application/ld+json as content type and body is

{"template":"reservationContractCreated","language":"/api/languages/018f15ed-e7d1-7c98-a407-3de1522dcd63"}

@soyuka
Copy link
Member

soyuka commented Jul 13, 2024

nice thanks, I'll check that out!

@paullallier
Copy link
Contributor Author

Thanks guys

@soyuka
Copy link
Member

soyuka commented Jul 15, 2024

can you check my patch (install a dev version of 3.3) and let me know, I'll tag a release shortly if it's alright

@paullallier
Copy link
Contributor Author

paullallier commented Jul 15, 2024

Thank Antoine. It seems to work for me. (I had to apply the patch manually because I was getting dev-main when I asked for 3.3.x-dev, but I think you might have fixed that on packagist already).

No - I've patched 3.3.7 which wasn't broken. I'll run it again.

@paullallier
Copy link
Contributor Author

It's not working for me because $this->getInputClass($context) returns null, but that might be a config error in the way I've defined my classes?
Screenshot 2024-07-15 at 12 03 21

@fvozar
Copy link

fvozar commented Jul 15, 2024

Hello @soyuka, I can confirm that your patch works in my case. Thanks a lot 👍

@paullallier
Copy link
Contributor Author

If I explicitly define input: Tax::class on my Tax entity, it looks like it works for me too. I'm re-running my test suite, which will take a while.

I suspect I might not be the only person this changes catches out though...

@soyuka
Copy link
Member

soyuka commented Jul 15, 2024

@paullallier can you please give me a stack trace? it must be a similar issue where uri_variables should've been removed from the context but are still available when denormalizing a relation. Basically send me the same details as I asked above so that I can reproduce.

@soyuka
Copy link
Member

soyuka commented Jul 15, 2024

I'm guessing:

diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php
index 0042511fe..ede5ebfcb 100644
--- a/src/Serializer/AbstractItemNormalizer.php
+++ b/src/Serializer/AbstractItemNormalizer.php
@@ -761,6 +761,7 @@ protected function getAttributeValue(object $object, string $attribute, ?string
             unset(
                 $context['resource_class'],
                 $context['force_resource_class'],
+                $context['uri_variables'],
             );
 
             // Anonymous resources
@@ -791,8 +792,11 @@ protected function getAttributeValue(object $object, string $attribute, ?string
             throw new LogicException(sprintf('The injected serializer must be an instance of "%s".', NormalizerInterface::class));
         }
 
-        unset($context['resource_class']);
-        unset($context['force_resource_class']);
+        unset(
+            $context['resource_class'],
+            $context['force_resource_class'],
+            $context['uri_variables']
+        );
 
         $attributeValue = $this->propertyAccessor->getValue($object, $attribute);
 

soyuka added a commit to soyuka/core that referenced this issue Jul 15, 2024
@paullallier
Copy link
Contributor Author

It looks like declaring the input class explicitly does fix things (it's still failing for me in a few tests, but I suspect they are other places that also need the input class defined).

If we don't want to force people to define the input class (which I suspect we is undesirable to require), your patch to getAttributeValue() doesn't help me.

This is the stack trace were it's failing for me:

AbstractItemNormalizer.php:247, ApiPlatform\Serializer\AbstractItemNormalizer->denormalize()
ItemNormalizer.php:186, ApiPlatform\JsonLd\Serializer\ItemNormalizer->denormalize()
Serializer.php:238, Symfony\Component\Serializer\Serializer->denormalize()
AbstractObjectNormalizer.php:790, Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->validateAndDenormalize()
AbstractObjectNormalizer.php:380, Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize()
Serializer.php:238, Symfony\Component\Serializer\Serializer->denormalize()
ArrayDenormalizer.php:74, Symfony\Component\Serializer\Normalizer\ArrayDenormalizer->denormalize()
Serializer.php:238, Symfony\Component\Serializer\Serializer->denormalize()
AbstractItemNormalizer.php:942, ApiPlatform\Serializer\AbstractItemNormalizer->createAndValidateAttributeValue()
AbstractItemNormalizer.php:877, ApiPlatform\Serializer\AbstractItemNormalizer->createAttributeValue()
AbstractItemNormalizer.php:504, ApiPlatform\Serializer\AbstractItemNormalizer->setAttributeValue()
AbstractObjectNormalizer.php:394, Symfony\Component\Serializer\Normalizer\AbstractObjectNormalizer->denormalize()
AbstractItemNormalizer.php:260, ApiPlatform\Serializer\AbstractItemNormalizer->denormalize()
ItemNormalizer.php:186, ApiPlatform\JsonLd\Serializer\ItemNormalizer->denormalize()
Serializer.php:238, Symfony\Component\Serializer\Serializer->denormalize()
Serializer.php:143, Symfony\Component\Serializer\Serializer->deserialize()
DeserializeProvider.php:93, ApiPlatform\State\Provider\DeserializeProvider->provide()
AccessCheckerProvider.php:62, ApiPlatform\Symfony\Security\State\AccessCheckerProvider->provide()
DeserializeListener.php:108, ApiPlatform\Symfony\EventListener\DeserializeListener->onKernelRequest()
WrappedListener.php:115, Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke()
EventDispatcher.php:206, Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
EventDispatcher.php:56, Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
TraceableEventDispatcher.php:122, Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch()
HttpKernel.php:159, Symfony\Component\HttpKernel\HttpKernel->handleRaw()
HttpKernel.php:76, Symfony\Component\HttpKernel\HttpKernel->handle()
Kernel.php:182, Symfony\Component\HttpKernel\Kernel->handle()
HttpKernelBrowser.php:63, Symfony\Component\HttpKernel\HttpKernelBrowser->doRequest()
KernelBrowser.php:157, Symfony\Bundle\FrameworkBundle\KernelBrowser->doRequest()
AbstractBrowser.php:377, Symfony\Component\BrowserKit\AbstractBrowser->request()
Client.php:116, ApiPlatform\Symfony\Bundle\Test\Client->request()
CustomApiTestCase.php:176, App\Tests\Helpers\CustomApiTestCase->makeRequest()
TaxRatesTest.php:120, App\Tests\Functional\Bookkeeping\TaxRatesTest->test_user_can_get_and_update_a_list_of_taxes()
TestCase.php:1613, PHPUnit\Framework\TestCase->runTest()
TestCase.php:1219, PHPUnit\Framework\TestCase->runBare()
TestResult.php:729, PHPUnit\Framework\TestResult->run()
TestCase.php:969, PHPUnit\Framework\TestCase->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestSuite.php:685, PHPUnit\Framework\TestSuite->run()
TestRunner.php:651, PHPUnit\TextUI\TestRunner->run()
Command.php:144, PHPUnit\TextUI\Command->run()
Command.php:97, PHPUnit\TextUI\Command::main()
phpunit:22, include()
simple-phpunit.php:465, require()
simple-phpunit:13, include()
simple-phpunit:119, {main}()```

soyuka added a commit to soyuka/core that referenced this issue Jul 15, 2024
@soyuka
Copy link
Member

soyuka commented Jul 15, 2024

thanks, updated my patch accordingly see https://github.com/api-platform/core/pull/6469/files

@paullallier
Copy link
Contributor Author

Yes, #6469 works for me without needing to add input class definitions to the entity. Thanks @soyuka.

soyuka added a commit that referenced this issue Jul 15, 2024
@soyuka
Copy link
Member

soyuka commented Jul 15, 2024

released

@soyuka soyuka closed this as completed Jul 15, 2024
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

No branches or pull requests

3 participants