Eager loading extension doesn't allow change context dynamically

[CvekCoding]

In my graphql-based application I would like to change normalization context dynamically while EagerLoadingExtension builds query. E.g. i want to have different Groups for different queries (I can distinguish between them by request names).

As I see, you deliberately skip the place where I could connect my ContextBuilder and change the context (I already have Groups and Attributes in my entity):

core/src/Bridge/Doctrine/Orm/Extension/EagerLoadingExtension.php

Lines 105 to 108 in 797cea6

	if (!isset($context['groups']) && !isset($context['attributes'])) {
	$contextType = isset($context['api_denormalize']) ? 'denormalization_context' : 'normalization_context';
	$context += $this->getNormalizationContext($context['resource_class'] ?? $resourceClass, $contextType, $options);
	}

Why is this done? Or did I choose the wrong way to solve my problem?

Thanks.

[soyuka]

Not sure I get your issue, if you have a (de)normalization_groups settings per request it works as mentioned. There might be a missing feature?

[CvekCoding]

@soyuka

normalization group per request

what do you mean? I don't see here this option. I see the following ways:

• set context in ApiResourse(). It's a global setting and doesn't fit my needs.
• set context per operation (query, delete...). Global again.
• change context in ContextBuilder. Looks good for me, but createFromRequest method never invoke for GraphQl.
• register my own normalizer, where I can set my own context. It invokes in GraphQl application, but only after EagerLoading extension - when sql query already buit and data fetched.

As far as I see, eagerLoadingExtension leverages this method to find the proper Groups:

core/src/Metadata/Property/Factory/SerializerPropertyMetadataFactory.php

Lines 142 to 164 in 797cea6

	private function getEffectiveSerializerGroups(array $options, string $resourceClass): array
	{
	if (isset($options['serializer_groups'])) {
	return [$options['serializer_groups'], $options['serializer_groups']];
	}
	$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
	if (isset($options['collection_operation_name'])) {
	$normalizationContext = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'normalization_context', null, true);
	$denormalizationContext = $resourceMetadata->getCollectionOperationAttribute($options['collection_operation_name'], 'denormalization_context', null, true);
	} elseif (isset($options['item_operation_name'])) {
	$normalizationContext = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], 'normalization_context', null, true);
	$denormalizationContext = $resourceMetadata->getItemOperationAttribute($options['item_operation_name'], 'denormalization_context', null, true);
	} elseif (isset($options['graphql_operation_name'])) {
	$normalizationContext = $resourceMetadata->getGraphqlAttribute($options['graphql_operation_name'], 'normalization_context', null, true);
	$denormalizationContext = $resourceMetadata->getGraphqlAttribute($options['graphql_operation_name'], 'denormalization_context', null, true);
	} else {
	$normalizationContext = $resourceMetadata->getAttribute('normalization_context');
	$denormalizationContext = $resourceMetadata->getAttribute('denormalization_context');
	}
	return [$normalizationContext['groups'] ?? null, $denormalizationContext['groups'] ?? null];
	}

May be this will be good to be able to decorate/override this method to pass the groups dynamically?

[soyuka]

set context per operation (query, delete...). Global again.

Okay my bad I confused per "operation" vs per "request".

Indeed EagerLoading is using groups to build the query. Isn't there a way we could fix the EagerLoadingExtension to use dynamic groups?

[CvekCoding]

@soyuka, I thought about this regarding GraphQl.

I think that because of the most important GraphQl-specific feature - user can select fields he wants to fetch, I believe there is a sense to consider all the fields specified in graphQl query as a dynamic group - to pull them eagerly. After all, the query itself describes the required fields, in my mind this is equivalent to the fact that the user has created a serializer group and wants to pull them out as quickly as possible (eager). Why not make all the necessary joins on them automatically and pull them all out with one query even without of requiring developer to build his own context?

What do you think?

[soyuka]

Yes indeed, I've built the EagerLoadingExtension with that in mind, it's why I based the query upon serialization groups. They indeed describe what we need.

I'm really not familiar with GraphQL but if possible it would definitely be a good feature. We must be careful that GraphQl queries don't interfere too much with groups though.

If you're up for it, we could add such a feature to the core, with a possibility to disable it and still use groups and graphql independently.

@teohhanhui @alanpoulain @dunglas opinion on this?

[dunglas]

The problem is: eager loading is not always the fastest solution. Often, it is faster to do several queries than a single complex one. It’s why I disabled this behavior with GraphQL. We may change this default behavior, but we need to do it very carefully (i.e allowing to disabling eager loading easily). Would you want to work on this?

[CvekCoding]

@dunglas as I wrote above, in my mind Graphql query's list of fields is analogue of serializer's group (user wants to fetch only particular fields of Entities). And here should work the same logic (quote from your site):

By default Doctrine comes with lazy loading. Usually a killer time-saving feature and also a performance killer with large applications.

By default in API Platform, we made the choice to force eager loading for all relations, with or without the Doctrine fetch attribute. Thanks to the eager loading extension.

So considering graphql query fields like dynamic serializer group (and automatically eager loading on them by default) looks more logical to me.

Otherwise I have the following issue now. Lets take easy query:

{
   productList {
      products {
        edges {
          node {
            id
            name
          }
        }
      }
   }
}

If I have the relation Product *->1 ProductList with normalization groups on both sides of relation (which is usual I think), the EagerLoadingExtension will throw exception about exceeding MaxJoins. But my query does not contain any cyclic relations and looks fully normal. This is bad and counter-intuitive behavior I think.

[CvekCoding]

I implemented this behavior (eager loading of all the fields from query and ignoring Groups for now) and dramatically reduced amount of DB queries in my application.

But then I faced the other problem here:

core/src/GraphQl/Serializer/ItemNormalizer.php

Lines 48 to 54 in 797cea6

	public function normalize($object, $format = null, array $context = [])
	{
	$data = parent::normalize($object, $format, $context);
	$data[self::ITEM_KEY] = serialize($object); // calling serialize prevent weird normalization process done by Webonyx's GraphQL PHP
	return $data;
	}

The problem is in $data[self::ITEM_KEY] = serialize($object). Due to the fact that eager-loaded objects are much bigger, serialization (and then - deserialization in Webonyx) becomes very expensive. As far as I understand here we don't need the real full object - empty object of the proper type with the real id will be enough. So instead of serializing the real object (which can include relations) here I instantiate empty object, copy into it id from real object, and serialize it.

In my application this works fine and time of query execution was reduced in about 5-20 times (depending on query complexity and amount of entities in collections).

[dunglas]

Would you mind opening a PR?

[CvekCoding]

@dunglas , sorry for late answer. Yes, of course I'll do this. I'll try to do this on weekend.

[soyuka]

sorry for late answer

No rush, it's open source :). Take all the time you need, we look forward to review this improvement!