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

[GraphQL] Avoid to call serialize in ItemNormalizer #2576

Conversation

alanpoulain
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? yes
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

Following #2358.

Replace calls to serialize with computed data in the normalizer to improve performance.

In master because there are some BC breaks (but no deprecated since it's still experimental).

@alanpoulain alanpoulain requested a review from dunglas March 2, 2019 21:32
@alanpoulain alanpoulain force-pushed the graphql-avoid-serialize-normalizer branch 2 times, most recently from 3a8e28a to e3da7e6 Compare March 3, 2019 14:17
Copy link
Member

@soyuka soyuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I read this is a good performance improvement :p.

@alanpoulain alanpoulain force-pushed the graphql-avoid-serialize-normalizer branch from e3da7e6 to 258f4a8 Compare March 6, 2019 16:05
@alanpoulain alanpoulain merged commit b3bd652 into api-platform:master Mar 6, 2019
@CvekCoding
Copy link
Contributor

@alanpoulain , when this patch will be merged to release branch?
Thanks.

@teohhanhui
Copy link
Contributor

@Siregacvek It'll be in 2.5, so not so soon...

@CvekCoding
Copy link
Contributor

@teohhanhui , well, it's better than in 2.6 :)

@@ -36,8 +36,8 @@ public function __construct(IriConverterInterface $iriConverter)
public function __invoke($source, $args, $context, ResolveInfo $info)
{
$property = null;
if ('id' === $info->fieldName && isset($source[ItemNormalizer::ITEM_KEY])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to make 'id' and '_id' as consts of IndentifiersExtractor class.

CvekCoding added a commit to CvekCoding/core that referenced this pull request May 26, 2019
CvekCoding pushed a commit to CvekCoding/core that referenced this pull request May 26, 2019
* Avoid to call serialize in ItemNormalizer

* Allow composite identifiers in getItemIriFromResourceClass

(cherry picked from commit b3bd652)
CvekCoding added a commit to CvekCoding/core that referenced this pull request May 26, 2019
CvekCoding pushed a commit to CvekCoding/core that referenced this pull request May 26, 2019
* Avoid to call serialize in ItemNormalizer

* Allow composite identifiers in getItemIriFromResourceClass

(cherry picked from commit b3bd652)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants