Doctrine: stateOptions IRI now can't be generated with default entity providers

[weaverryan]

API Platform version(s) affected: 3.1-dev (unreleased)

Description
@soyuka is helping me out BIG time to get stateOptions: new Options(entityClass: User::class), working nicely. #5726 fixed a bug where IRI generation would fail if you implemented a custom provider that returned your DTO objects. That seems to work great!

However, now the default case looks broken in the same way. Consider:

#[ApiResource(
 stateOptions: new Options(entityClass: User::class),
)]

class UserApi

This will now fail with Unable to generate an IRI for the item of type "App\ApiResource\UserApi. It's the opposite problem as before: IdentifiersExtractor will not grab the id because here -

core/src/Api/IdentifiersExtractor.php

Line 98 in 92a81f0

if ($item instanceof $class) {

- $item will be the entity (User) but $class will be UserApi.

How to reproduce

I have an easy reproducer here: https://github.com/symfonycasts/api-platform3/tree/playing-with-stateOptions

You can see it working, at first, with a custom provider that returns the DTOs. Then, by commenting-out the provider option, you can see it fail when the core provider returns the entity objects.

Thanks for your attention - this is all nearly working perfectly.

[mrossard]

Isn't this similar to this issue : #5510 ?

I've been working with / on entityClass for a while now, and I've in fact ended up always using custom providers. A solution for the "simple" use case would be welcome! I'll try to find some time to look into it if nobody else does it before i can.

[weaverryan]

Isn't this similar to this issue : #5510 ?

It's definitely in the same neighborhood :)

I've been working with / on entityClass for a while now, and I've in fact ended up always using custom providers

Interesting. For our tutorial, I want to show both (no custom provider at first, then adding the custom provider so we can return the DTO's). What's interesting is that, before #5726, when I used a custom data provider that returned the DTO's, the IRI couldn't be generated. I wonder what you are doing different than I am 🤔. This was my reproducer problem from before updating to the #5726 code - https://github.com/SymfonyCasts/api-platform3/tree/d36019ead21ff1238a1090f07259dc442fb55038 - notice:

• UserApi with custom resource and stateOptions: https://github.com/SymfonyCasts/api-platform3/blob/d36019ead21ff1238a1090f07259dc442fb55038/src/ApiResource/UserApi.php#L18-L19
• UserApiStateProvider with VERY code to return DTO's - https://github.com/SymfonyCasts/api-platform3/blob/d36019ead21ff1238a1090f07259dc442fb55038/src/State/UserApiStateProvider.php#L23-L44 - the weird $userApi->id = $user; part is what was required before fix(doctrine): use stateOptions only within doctrine context #5726 to get the IRI to generate. After fix(doctrine): use stateOptions only within doctrine context #5726, I can do the normal $userApi->id = $user->getId();... but now the "simple" case has the IRI problem

Were you doing something different with your custom providers (using pre #5726 code) that didn't any IRI-generation problems?

thx :)

[mrossard]

Isn't there some side effect in your reproducer due to the entity also being an ApiReource?
It's bound to become confusing at some point while trying to convert from the resource to the entity and vice versa - the identifiersExtractor (which probably triggers the error you get right now) doesn't always have the necessary context to chose which path to follow.

If that's the problem it's actually also getting close to another issue i've be meaning to tackle for a while : #5687
In that one I end up using additional context to help the identifiersExtractor chose which path to follow, but as I tried to explain in the PR, that's just a quick fix for something that probably requires much more thinking to do correctly.

My current production code using entityclass is pretty straightforward, for simple mapped entities it goes something like this :

#[ApiResource(
 operations : [
 new GetCollection(
 uriTemplate: self::COLLECTION_URI
 ),
 new Get(
 uriTemplate : self::ITEM_URI,
 uriVariables: ['id'],
 ),
 new Post(
 uriTemplate: self::COLLECTION_URI,
 ),
 new Patch(
 uriTemplate: self::ITEM_URI,
 uriVariables: ['id'],
 ),
 ],
 provider : SomeResourceProvider::class,
 processor : SomeResourceProcessor::class,
 stateOptions : new Options(entityClass: \App\Entity\SomeEntity::class)
)]
final class SomeResource
{
 public const COLLECTION_URI = '/somethings';
 public const ITEM_URI = '/somethings/{id}';
 
 #[ApiProperty(identifier: true)]
 public int $id;
//...

The explicitly defined uriTemplates/uriVariables might play a role in this...

[mrossard]

(and the providers do a simple call to the default provider + a transformation, pretty much like yours does)

[edit]

No, not exactly like yours!
Mine call the default provider like this instead, using the entityClass in the operation:

$data = $this->collectionProvider->provide(
 $operation->withClass($operation->getStateOptions()->getEntityClass()),
 $uriVariables,
 $context);

[weaverryan]

Isn't there some side effect in your reproducer due to the entity also being an ApiReource?
It's bound to become confusing at some point while trying to convert from the resource to the entity and vice versa

Yea, sorry - the entity is an resource just temporarily - that has been confusing everyone I show this too :p. Ignore it.

The explicitly defined uriTemplates/uriVariables might play a role in this

That's it! Or, at least, it's a work-around. Defining uriVariables: ['id'], for the Get operation fixes the problem.

Unfortunately, after #5726, the "simple" case (where you use the core provider and return entities) still has an IRI problem. So, you've given me a workaround (though #5726 made this workaround not longer necessary)... but #5726 breaks the "simple" case.

[mrossard]

I should probably check whether #5726 breaks anything on more complex cases then...have you tried using Links to other entities with that version?

[mrossard]

In my app I ended up not using Links at all, found them too complicated to make work with entityClass anyway - I just added the variables I needed for generating the URI in the ApiResource without exposing them, then declared them as uriVariables.

[soyuka]

@weaverryan I don't get where things are failing, we've this test running just fine:

core/features/doctrine/separated_resource.feature

Line 6 in 92a81f0

@!mongodb

core/tests/Fixtures/TestBundle/ApiResource/ResourceWithSeparatedEntity.php

Line 22 in 92a81f0

#[ApiResource(shortName: 'SeparatedEntity', stateOptions: new Options(entityClass: SeparatedEntity::class))]

[mrossard]

The difference seems to be the presence of a Filter in @weaverryan 's version; the Serializelistener's $controllerResult is a Paginator, while in the test the result is an array af resources.
The result is that in the identifiersExtractor, the item ends up being the resource in one case, and the entity in the other.

[edit]

nevermind, i was looking at the wrong test.

[mrossard]

I think it's actually this :

core/src/Api/IdentifiersExtractor.php

Lines 52 to 54 in 92a81f0

	if (!$this->isResourceClass($this->getObjectClass($item))) {
	return ['id' => $this->propertyAccessor->getValue($item, 'id')];
	}

The User Entity also being an ApiResource in @weaverryan 's example, it doesn't enter this condition and fails later. This code is actually also bound to fail if the identifier is not "id".

[weaverryan]

You nailed it - TOTALLY my fault keeping the entity as an resource temporarily. That breaks things - I don't consider that a bug - so I'm closing.

Apologies for the noise but thank you for all the attention! I'm excited to jump back into things now ❤️