fix(doctrine): fix partial fetch when relation switches context

[jderusse]

Q	A
Branch?	4.2
Tickets	n/a
License	MIT
Doc PR	n/a

When an entity has a relation and changes its serialization context (using #[Context(...)] attribute, the EagerLoadingExtension fails to resolve the partial properties to fetch.

reproducer

#[ApiResource(
    operations: [...],
    normalizationContext: ['groups' => ['A']],
)]
class User
{
    #[ORM\ManyToOne]
    #[Groups(['A'])]
    #[Context(normalizationContext: ['groups' => ['B']])]
    public Address|null $address = null;
}

class Address
{
    #[ORM\Column]
    #[Groups(['B'])]
    public string $label;
}

In that case the EagerLoadingExtension iterates over entities that are included in the group A, therefore does not include Address::label because it bellong to the group B.

This PR uses the existing $attributesMetadata and its getNormalizationContextForGroups/getDenormalizationContextForGroups methods to update the serialization context when resolving relations.

[soyuka]

really interesting use case. I'm wondering though if this isn't making unnecessary overhead when there is no context switch. Indeed this runs for every association, what do you think about caching the computed propertyOptions if called multiple times for the same association?

[jderusse]

I'm not sure about the gain, let me explain why I think that:

• This class is called when building the DQL query (not serializing objects). So once per HTTP request.
• This code should be called once per entity + once per relation - recursively => In most projects, it should be just a bunch of entities
• The call to getNormalizationContextForGroups is super light (foreach groups + array_merge) and should be very fast.
• Since my last refactoring and early return, this code is not called when the relation does not have context switch

Using a cache will be effective when we need to serialize the same class multiple times for different relations. Which is (IMHO) a rare case.

But if it's true, let's says, a class has hundred of relations of the same class. Then:

• the SQL query will have hundreds of joins
• doctrine will hydrate hundred × N objects (N = number of rows)
• The Symfony Serializer will call similar code hundred × N times

So I think, using cache for this will add complexity for a rare edge case and will not change the overall response time anyway.

[jderusse]

If this function has performance issue, maybe we should cache (in a persistent storage like redis/filesystem/...) ALL the class's logic instead 🤔

For the same $resourceClass, $operation and $context we collect (and cache) joins and selects, and then we just have to re-apply them on next request.

[soyuka]

		if (isset($attributesMetadata[$association])) {
	         $hasCustomContext = $attributesMetadata[$association]->hasNormalizationContext()
	                          || $attributesMetadata[$association]->hasDenormalizationContext();
	
	         // Early return if no custom context
	         if (!$hasCustomContext) {

but probably move your code to a function to make things more readable

[jderusse]

good idea, I moved code in dedicated method, and early return when no context is defined.

[soyuka]

thanks!