Skip to content

Commit bac0677

Browse files
joanhejoanhe
committed
B2B-2606: Graphql Parser called at least 3 times per request
1 parent 323c32c commit bac0677

File tree

5 files changed

+55
-25
lines changed

5 files changed

+55
-25
lines changed

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Magento\Framework\Controller\Result\JsonFactory;
2121
use Magento\Framework\GraphQl\Exception\ExceptionFormatter;
2222
use Magento\Framework\GraphQl\Query\Fields as QueryFields;
23+
use Magento\Framework\GraphQl\Query\QueryParser;
2324
use Magento\Framework\GraphQl\Query\QueryProcessor;
2425
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
2526
use Magento\Framework\GraphQl\Schema\SchemaGeneratorInterface;
@@ -110,6 +111,11 @@ class GraphQl implements FrontControllerInterface
110111
*/
111112
private $areaList;
112113

114+
/**
115+
* @var QueryParser
116+
*/
117+
private $queryParser;
118+
113119
/**
114120
* @param Response $response
115121
* @param SchemaGeneratorInterface $schemaGenerator
@@ -125,6 +131,7 @@ class GraphQl implements FrontControllerInterface
125131
* @param LogData|null $logDataHelper
126132
* @param LoggerPool|null $loggerPool
127133
* @param AreaList|null $areaList
134+
* @param QueryParser|null $queryParser
128135
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
129136
*/
130137
public function __construct(
@@ -141,7 +148,8 @@ public function __construct(
141148
ContextFactoryInterface $contextFactory = null,
142149
LogData $logDataHelper = null,
143150
LoggerPool $loggerPool = null,
144-
AreaList $areaList = null
151+
AreaList $areaList = null,
152+
QueryParser $queryParser = null
145153
) {
146154
$this->response = $response;
147155
$this->schemaGenerator = $schemaGenerator;
@@ -157,6 +165,7 @@ public function __construct(
157165
$this->logDataHelper = $logDataHelper ?: ObjectManager::getInstance()->get(LogData::class);
158166
$this->loggerPool = $loggerPool ?: ObjectManager::getInstance()->get(LoggerPool::class);
159167
$this->areaList = $areaList ?: ObjectManager::getInstance()->get(AreaList::class);
168+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
160169
}
161170

162171
/**
@@ -172,25 +181,22 @@ public function dispatch(RequestInterface $request): ResponseInterface
172181

173182
$statusCode = 200;
174183
$jsonResult = $this->jsonFactory->create();
175-
$data = $this->getDataFromRequest($request);
176184
$result = [];
177185

178186
$schema = null;
179187
try {
180188
/** @var Http $request */
181189
$this->requestProcessor->validateRequest($request);
182-
183-
$query = $data['query'] ?? '';
184-
$variables = $data['variables'] ?? null;
190+
$data = $this->getDataFromRequest($request);
185191

186192
// We must extract queried field names to avoid instantiation of unnecessary fields in webonyx schema
187193
// Temporal coupling is required for performance optimization
188-
$this->queryFields->setQuery($query, $variables);
194+
$this->queryFields->setQuery($data['parsedQuery'], $data['variables'] ?? null);
189195
$schema = $this->schemaGenerator->generate();
190196

191197
$result = $this->queryProcessor->process(
192198
$schema,
193-
$query,
199+
$data['parsedQuery'],
194200
$this->contextFactory->create(),
195201
$data['variables'] ?? []
196202
);
@@ -231,9 +237,11 @@ private function getDataFromRequest(RequestInterface $request): array
231237
$data['variables'] = is_array($data['variables']) ?
232238
$data['variables'] : null;
233239
} else {
234-
return [];
240+
$data = [];
235241
}
236-
242+
$query = $data['query'] ?? '';
243+
$parsedQuery = $this->queryParser->parse($query);
244+
$data['parsedQuery'] = $parsedQuery;
237245
return $data;
238246
}
239247
}

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

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace Magento\GraphQl\Helper\Query\Logger;
99

1010
use GraphQL\Error\SyntaxError;
11+
use GraphQL\Language\AST\DocumentNode;
1112
use GraphQL\Language\AST\Node;
1213
use GraphQL\Language\AST\NodeKind;
1314
use GraphQL\Language\Visitor;
@@ -56,7 +57,7 @@ public function getLogData(
5657
$logData = array_merge($logData, $this->gatherRequestInformation($request));
5758

5859
try {
59-
$complexity = $this->getFieldCount($data['query'] ?? '');
60+
$complexity = $this->getFieldCount($data['parsedQuery'] ?? $data['query'] ?? '');
6061
$logData[LoggerInterface::COMPLEXITY] = $complexity;
6162
if ($schema) {
6263
$logData = array_merge($logData, $this->gatherQueryInformation($schema));
@@ -127,18 +128,20 @@ private function gatherResponseInformation(HttpResponse $response) : array
127128
*
128129
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
129130
*
130-
* @param string $query
131+
* @param DocumentNode|string $query
131132
* @return int
132133
* @throws SyntaxError
133134
* @throws \Exception
134135
*/
135-
private function getFieldCount(string $query): int
136+
private function getFieldCount(DocumentNode|string $query): int
136137
{
137138
if (!empty($query)) {
138139
$totalFieldCount = 0;
139-
$parsedQuery = $this->queryParser->parse($query);
140+
if (is_string($query)) {
141+
$query = $this->queryParser->parse($query);
142+
}
140143
Visitor::visit(
141-
$parsedQuery,
144+
$query,
142145
[
143146
'leave' => [
144147
NodeKind::FIELD => function (Node $node) use (&$totalFieldCount) {

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

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

88
namespace Magento\Framework\GraphQl\Query;
99

10+
use GraphQL\Language\AST\DocumentNode;
1011
use GraphQL\Language\AST\Node;
1112
use GraphQL\Language\AST\NodeKind;
1213
use Magento\Framework\App\ObjectManager;
@@ -37,18 +38,20 @@ public function __construct(QueryParser $queryParser = null)
3738
/**
3839
* Set Query for extracting list of fields.
3940
*
40-
* @param string $query
41+
* @param DocumentNode|string $query
4142
* @param array|null $variables
4243
*
4344
* @return void
4445
*/
45-
public function setQuery($query, array $variables = null)
46+
public function setQuery(DocumentNode|string $query, array $variables = null)
4647
{
4748
$queryFields = [];
4849
try {
49-
$parsedQuery = $this->queryParser->parse($query);
50+
if (is_string($query)) {
51+
$query = $this->queryParser->parse($query);
52+
}
5053
\GraphQL\Language\Visitor::visit(
51-
$parsedQuery,
54+
$query,
5255
[
5356
'leave' => [
5457
NodeKind::NAME => function (Node $node) use (&$queryFields) {

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

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

88
namespace Magento\Framework\GraphQl\Query;
99

10+
use GraphQL\Language\AST\DocumentNode;
1011
use GraphQL\Language\AST\NodeKind;
1112
use GraphQL\Language\Visitor;
1213
use GraphQL\Validator\DocumentValidator;
@@ -86,17 +87,19 @@ public function execute(): void
8687
* This is necessary for performance optimization, as extremely large queries require a substantial
8788
* amount of time to fully validate and can affect server performance.
8889
*
89-
* @param string $query
90+
* @param DocumentNode|string $query
9091
* @throws GraphQlInputException
9192
*/
92-
public function validateFieldCount(string $query): void
93+
public function validateFieldCount(DocumentNode|string $query): void
9394
{
9495
if (!empty($query)) {
9596
$totalFieldCount = 0;
96-
$parsedQuery = $this->queryParser->parse($query);
97+
if (is_string($query)) {
98+
$query = $this->queryParser->parse($query);
99+
}
97100

98101
Visitor::visit(
99-
$parsedQuery,
102+
$query,
100103
[
101104
'leave' => [
102105
NodeKind::FIELD => function () use (&$totalFieldCount) {

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

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

1010
use GraphQL\Error\DebugFlag;
1111
use GraphQL\GraphQL;
12+
use GraphQL\Language\AST\DocumentNode;
13+
use Magento\Framework\App\ObjectManager;
1214
use Magento\Framework\GraphQl\Exception\ExceptionFormatter;
1315
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
1416
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
@@ -34,27 +36,35 @@ class QueryProcessor
3436
*/
3537
private $errorHandler;
3638

39+
/**
40+
* @var QueryParser
41+
*/
42+
private $queryParser;
43+
3744
/**
3845
* @param ExceptionFormatter $exceptionFormatter
3946
* @param QueryComplexityLimiter $queryComplexityLimiter
4047
* @param ErrorHandlerInterface $errorHandler
48+
* @param QueryParser|null $queryParser
4149
* @SuppressWarnings(PHPMD.LongVariable)
4250
*/
4351
public function __construct(
4452
ExceptionFormatter $exceptionFormatter,
4553
QueryComplexityLimiter $queryComplexityLimiter,
46-
ErrorHandlerInterface $errorHandler
54+
ErrorHandlerInterface $errorHandler,
55+
QueryParser $queryParser = null
4756
) {
4857
$this->exceptionFormatter = $exceptionFormatter;
4958
$this->queryComplexityLimiter = $queryComplexityLimiter;
5059
$this->errorHandler = $errorHandler;
60+
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
5161
}
5262

5363
/**
5464
* Process a GraphQl query according to defined schema
5565
*
5666
* @param Schema $schema
57-
* @param string $source
67+
* @param DocumentNode|string $source
5868
* @param ContextInterface|null $contextValue
5969
* @param array|null $variableValues
6070
* @param string|null $operationName
@@ -63,11 +73,14 @@ public function __construct(
6373
*/
6474
public function process(
6575
Schema $schema,
66-
string $source,
76+
DocumentNode|string $source,
6777
ContextInterface $contextValue = null,
6878
array $variableValues = null,
6979
string $operationName = null
7080
): array {
81+
if (is_string($source)) {
82+
$source = $this->queryParser->parse($source);
83+
}
7184
if (!$this->exceptionFormatter->shouldShowDetail()) {
7285
$this->queryComplexityLimiter->validateFieldCount($source);
7386
$this->queryComplexityLimiter->execute();

0 commit comments

Comments
 (0)