Skip to content

Commit c987618

Browse files
joanhejoanhe
committed
B2B-2606: Graphql Parser called at least 3 times per request
1 parent 72a6c01 commit c987618

File tree

7 files changed

+112
-42
lines changed

7 files changed

+112
-42
lines changed

‎app/code/Magento/GraphQl/Controller/GraphQl.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ class GraphQl implements FrontControllerInterface
4141
/**
4242
* @var \Magento\Framework\Webapi\Response
4343
* @deprecated 100.3.2
44-
* @see was only added to fix the constructor
4544
*/
4645
private $response;
4746

@@ -67,8 +66,7 @@ class GraphQl implements FrontControllerInterface
6766

6867
/**
6968
* @var ContextInterface
70-
* @deprecated 100.3.3
71-
* @see $contextFactory is used for creating Context object
69+
* @deprecated 100.3.3 $contextFactory is used for creating Context object
7270
*/
7371
private $resolverContext;
7472

@@ -187,14 +185,12 @@ public function dispatch(RequestInterface $request): ResponseInterface
187185

188186
// We must extract queried field names to avoid instantiation of unnecessary fields in webonyx schema
189187
// Temporal coupling is required for performance optimization
190-
$data['parsedQuery'] =
191-
\GraphQL\Language\Parser::parse(new \GraphQL\Language\Source($query ?: '', 'GraphQL'));
192-
$this->queryFields->setQuery($data['parsedQuery'], $variables);
188+
$this->queryFields->setQuery($query, $variables);
193189
$schema = $this->schemaGenerator->generate();
194190

195191
$result = $this->queryProcessor->process(
196192
$schema,
197-
$data['parsedQuery'],
193+
$query,
198194
$this->contextFactory->create(),
199195
$data['variables'] ?? []
200196
);

‎app/code/Magento/GraphQl/Controller/HttpRequestValidator/HttpVerbValidator.php

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99

1010
use GraphQL\Language\AST\Node;
1111
use GraphQL\Language\AST\NodeKind;
12-
use GraphQL\Language\Parser;
13-
use GraphQL\Language\Source;
1412
use GraphQL\Language\Visitor;
1513
use Magento\Framework\App\HttpRequestInterface;
14+
use Magento\Framework\App\ObjectManager;
1615
use Magento\Framework\App\Request\Http;
1716
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
17+
use Magento\Framework\GraphQl\Query\QueryParser;
1818
use Magento\Framework\Phrase;
1919
use Magento\GraphQl\Controller\HttpRequestValidatorInterface;
2020

@@ -23,6 +23,19 @@
2323
*/
2424
class HttpVerbValidator implements HttpRequestValidatorInterface
2525
{
26+
/**
27+
* @var QueryParser
28+
*/
29+
private $queryParser;
30+
31+
/**
32+
* @param QueryParser|null $queryParser
33+
*/
34+
public function __construct(QueryParser $queryParser = null)
35+
{
36+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
37+
}
38+
2639
/**
2740
* Check if request is using correct verb for query or mutation
2841
*
@@ -37,11 +50,9 @@ public function validate(HttpRequestInterface $request): void
3750
$query = $request->getParam('query', '');
3851
if (!empty($query)) {
3952
$operationType = '';
40-
if (is_string($query)) {
41-
$query = Parser::parse(new Source($query, 'GraphQL'));
42-
}
53+
$parsedQuery = $this->queryParser->parse($query);
4354
Visitor::visit(
44-
$query,
55+
$parsedQuery,
4556
[
4657
'leave' => [
4758
NodeKind::OPERATION_DEFINITION => function (Node $node) use (&$operationType) {

‎app/code/Magento/GraphQl/Helper/Query/Logger/LogData.php

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,13 @@
88
namespace Magento\GraphQl\Helper\Query\Logger;
99

1010
use GraphQL\Error\SyntaxError;
11-
use GraphQL\Language\AST\DocumentNode;
1211
use GraphQL\Language\AST\Node;
1312
use GraphQL\Language\AST\NodeKind;
14-
use GraphQL\Language\Parser;
15-
use GraphQL\Language\Source;
1613
use GraphQL\Language\Visitor;
14+
use Magento\Framework\App\ObjectManager;
1715
use Magento\Framework\App\RequestInterface;
1816
use Magento\Framework\App\Response\Http as HttpResponse;
17+
use Magento\Framework\GraphQl\Query\QueryParser;
1918
use Magento\Framework\GraphQl\Schema;
2019
use Magento\GraphQl\Model\Query\Logger\LoggerInterface;
2120

@@ -24,6 +23,19 @@
2423
*/
2524
class LogData
2625
{
26+
/**
27+
* @var QueryParser
28+
*/
29+
private $queryParser;
30+
31+
/**
32+
* @param QueryParser|null $queryParser
33+
*/
34+
public function __construct(QueryParser $queryParser = null)
35+
{
36+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
37+
}
38+
2739
/**
2840
* Extracts relevant information about the request
2941
*
@@ -44,7 +56,7 @@ public function getLogData(
4456
$logData = array_merge($logData, $this->gatherRequestInformation($request));
4557

4658
try {
47-
$complexity = $this->getFieldCount($data['parsedQuery'] ?? $data['query'] ?? '');
59+
$complexity = $this->getFieldCount($data['query'] ?? '');
4860
$logData[LoggerInterface::COMPLEXITY] = $complexity;
4961
if ($schema) {
5062
$logData = array_merge($logData, $this->gatherQueryInformation($schema));
@@ -115,20 +127,18 @@ private function gatherResponseInformation(HttpResponse $response) : array
115127
*
116128
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
117129
*
118-
* @param DocumentNode|string $query
130+
* @param string $query
119131
* @return int
120132
* @throws SyntaxError
121133
* @throws \Exception
122134
*/
123-
private function getFieldCount(DocumentNode|string $query): int
135+
private function getFieldCount(string $query): int
124136
{
125137
if (!empty($query)) {
126138
$totalFieldCount = 0;
127-
if (is_string($query)) {
128-
$query = Parser::parse(new Source($query, 'GraphQL'));
129-
}
139+
$parsedQuery = $this->queryParser->parse($query);
130140
Visitor::visit(
131-
$query,
141+
$parsedQuery,
132142
[
133143
'leave' => [
134144
NodeKind::FIELD => function (Node $node) use (&$totalFieldCount) {

‎lib/internal/Magento/Framework/GraphQl/Query/Fields.php

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77

88
namespace Magento\Framework\GraphQl\Query;
99

10-
use GraphQL\Language\AST\DocumentNode;
1110
use GraphQL\Language\AST\Node;
1211
use GraphQL\Language\AST\NodeKind;
12+
use Magento\Framework\App\ObjectManager;
1313

1414
/**
1515
* This class holds a list of all queried fields and is used to enable performance optimization for schema loading.
@@ -21,10 +21,23 @@ class Fields
2121
*/
2222
private $fieldsUsedInQuery = [];
2323

24+
/**
25+
* @var QueryParser
26+
*/
27+
private $queryParser;
28+
29+
/**
30+
* @param QueryParser|null $queryParser
31+
*/
32+
public function __construct(QueryParser $queryParser = null)
33+
{
34+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
35+
}
36+
2437
/**
2538
* Set Query for extracting list of fields.
2639
*
27-
* @param DocumentNode|string $query
40+
* @param string $query
2841
* @param array|null $variables
2942
*
3043
* @return void
@@ -33,11 +46,9 @@ public function setQuery($query, array $variables = null)
3346
{
3447
$queryFields = [];
3548
try {
36-
if (is_string($query)) {
37-
$query = \GraphQL\Language\Parser::parse(new \GraphQL\Language\Source($query ?: '', 'GraphQL'));
38-
}
49+
$parsedQuery = $this->queryParser->parse($query);
3950
\GraphQL\Language\Visitor::visit(
40-
$query,
51+
$parsedQuery,
4152
[
4253
'leave' => [
4354
NodeKind::NAME => function (Node $node) use (&$queryFields) {

‎lib/internal/Magento/Framework/GraphQl/Query/QueryComplexityLimiter.php

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,13 @@
77

88
namespace Magento\Framework\GraphQl\Query;
99

10-
use GraphQL\Language\AST\DocumentNode;
1110
use GraphQL\Language\AST\NodeKind;
12-
use GraphQL\Language\Parser;
13-
use GraphQL\Language\Source;
1411
use GraphQL\Language\Visitor;
1512
use GraphQL\Validator\DocumentValidator;
1613
use GraphQL\Validator\Rules\DisableIntrospection;
1714
use GraphQL\Validator\Rules\QueryDepth;
1815
use GraphQL\Validator\Rules\QueryComplexity;
16+
use Magento\Framework\App\ObjectManager;
1917
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
2018

2119
/**
@@ -44,19 +42,27 @@ class QueryComplexityLimiter
4442
*/
4543
private $introspectionConfig;
4644

45+
/**
46+
* @var QueryParser
47+
*/
48+
private $queryParser;
49+
4750
/**
4851
* @param int $queryDepth
4952
* @param int $queryComplexity
5053
* @param IntrospectionConfiguration $introspectionConfig
54+
* @param QueryParser|null $queryParser
5155
*/
5256
public function __construct(
5357
int $queryDepth,
5458
int $queryComplexity,
55-
IntrospectionConfiguration $introspectionConfig
59+
IntrospectionConfiguration $introspectionConfig,
60+
QueryParser $queryParser = null
5661
) {
5762
$this->queryDepth = $queryDepth;
5863
$this->queryComplexity = $queryComplexity;
5964
$this->introspectionConfig = $introspectionConfig;
65+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
6066
}
6167

6268
/**
@@ -80,19 +86,17 @@ public function execute(): void
8086
* This is necessary for performance optimization, as extremely large queries require a substantial
8187
* amount of time to fully validate and can affect server performance.
8288
*
83-
* @param DocumentNode|string $query
89+
* @param string $query
8490
* @throws GraphQlInputException
8591
*/
86-
public function validateFieldCount(DocumentNode|string $query): void
92+
public function validateFieldCount(string $query): void
8793
{
8894
if (!empty($query)) {
8995
$totalFieldCount = 0;
90-
if (is_string($query)) {
91-
$query = Parser::parse(new Source($query, 'GraphQL'));
92-
}
96+
$parsedQuery = $this->queryParser->parse($query);
9397

9498
Visitor::visit(
95-
$query,
99+
$parsedQuery,
96100
[
97101
'leave' => [
98102
NodeKind::FIELD => function () use (&$totalFieldCount) {

‎lib/internal/Magento/Framework/GraphQl/Query/QueryParser.php

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Framework\GraphQl\Query;
9+
10+
use GraphQL\Language\AST\DocumentNode;
11+
use GraphQL\Language\Parser;
12+
use GraphQL\Language\Source;
13+
14+
/**
15+
* Wrapper for GraphQl query parser. It parses query string into a `GraphQL\Language\AST\DocumentNode`
16+
*/
17+
class QueryParser
18+
{
19+
/**
20+
* @var string[]
21+
*/
22+
private $parsedQueries = [];
23+
24+
/**
25+
* Parse query string into a `GraphQL\Language\AST\DocumentNode`.
26+
*
27+
* @param string $query
28+
* @return DocumentNode
29+
* @throws \GraphQL\Error\SyntaxError
30+
*/
31+
public function parse(string $query): DocumentNode
32+
{
33+
$cacheKey = sha1($query);
34+
if (!isset($this->parsedQueries[$cacheKey])) {
35+
$this->parsedQueries[$cacheKey] = Parser::parse(new Source($query ?: '', 'GraphQL'));
36+
}
37+
return $this->parsedQueries[$cacheKey];
38+
}
39+
}

‎lib/internal/Magento/Framework/GraphQl/Query/QueryProcessor.php

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
use GraphQL\Error\DebugFlag;
1111
use GraphQL\GraphQL;
12-
use GraphQL\Language\AST\DocumentNode;
1312
use Magento\Framework\GraphQl\Exception\ExceptionFormatter;
1413
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
1514
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
@@ -55,7 +54,7 @@ public function __construct(
5554
* Process a GraphQl query according to defined schema
5655
*
5756
* @param Schema $schema
58-
* @param DocumentNode|string $source
57+
* @param string $source
5958
* @param ContextInterface|null $contextValue
6059
* @param array|null $variableValues
6160
* @param string|null $operationName
@@ -64,7 +63,7 @@ public function __construct(
6463
*/
6564
public function process(
6665
Schema $schema,
67-
DocumentNode|string $source,
66+
string $source,
6867
ContextInterface $contextValue = null,
6968
array $variableValues = null,
7069
string $operationName = null

0 commit comments

Comments
 (0)