Skip to content

Commit

Permalink
Merge pull request #8229 from magento-arcticfoxes/B2B-2606
Browse files Browse the repository at this point in the history
B2B-2606: Graphql Parser called at least 3 times per request
  • Loading branch information
mmansoor-magento authored May 10, 2023
2 parents 3d49b24 + 3ee1541 commit 12da28c
Show file tree
Hide file tree
Showing 19 changed files with 266 additions and 71 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?xml version="1.0" encoding="UTF-8"?>
<!--
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
-->
<actionGroups xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:mftf:Test/etc/actionGroupSchema.xsd">
<actionGroup name="AdminDeleteAllTermConditionsActionGroup">
<annotations>
<description>Deletes all rows one by one on the 'Terms and Conditions' page.</description>
</annotations>
<waitForElementVisible selector="{{AdminLegacyDataGridFilterSection.clear}}" stepKey="waitForResetFilter"/>
<click selector="{{AdminLegacyDataGridFilterSection.clear}}" stepKey="clickResetFilter"/>
<waitForPageLoad stepKey="waitForGridReset"/>
<helper class="Magento\CheckoutAgreements\Test\Mftf\Helper\CheckoutAgreementsHelpers" method="deleteAllTermConditionRows" stepKey="deleteAllTermConditionRows">
<argument name="rowsToDelete">{{AdminTermGridSection.allTermRows}}</argument>
<argument name="deleteButton">{{AdminMainActionsSection.delete}}</argument>
<argument name="modalAcceptButton">{{AdminConfirmationModalSection.ok}}</argument>
<argument name="successMessage">You deleted the condition.</argument>
<argument name="successMessageContainer">{{AdminMessagesSection.success}}</argument>
</helper>
<waitForPageLoad stepKey="waitForGridLoad"/>
<waitForText userInput="We couldn't find any records." selector="{{AdminTermGridSection.emptyGrid}}" stepKey="waitForEmptyGrid"/>
</actionGroup>
</actionGroups>
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\CheckoutAgreements\Test\Mftf\Helper;

use Facebook\WebDriver\WebDriverBy;
use Magento\FunctionalTestingFramework\Helper\Helper;
use Magento\FunctionalTestingFramework\Module\MagentoWebDriver;
use Exception;

/**
* Class for MFTF helpers for CheckoutAgreements module.
*/
class CheckoutAgreementsHelpers extends Helper
{
/**
* Delete all term conditions one by one from the Terms & Conditions grid page.
*
* @param string $rowsToDelete
* @param string $deleteButton
* @param string $modalAcceptButton
* @param string $successMessage
* @param string $successMessageContainer
*
* @return void
*/
public function deleteAllTermConditionRows(
string $rowsToDelete,
string $deleteButton,
string $modalAcceptButton,
string $successMessage,
string $successMessageContainer
): void {
try {
/** @var MagentoWebDriver $magentoWebDriver */
$magentoWebDriver = $this->getModule("\\" . MagentoWebDriver::class);
$webDriver = $magentoWebDriver->webDriver;

$magentoWebDriver->waitForPageLoad(30);
$rows = $webDriver->findElements(WebDriverBy::xpath($rowsToDelete));
while (!empty($rows)) {
$rows[0]->click();
$magentoWebDriver->waitForPageLoad(30);
$magentoWebDriver->waitForElementVisible($deleteButton, 10);
$magentoWebDriver->click($deleteButton);
$magentoWebDriver->waitForPageLoad(30);
$magentoWebDriver->waitForElementVisible($modalAcceptButton, 10);
$magentoWebDriver->click($modalAcceptButton);
$magentoWebDriver->waitForPageLoad(60);
$magentoWebDriver->waitForText($successMessage, 10, $successMessageContainer);
$rows = $webDriver->findElements(WebDriverBy::xpath($rowsToDelete));
}
} catch (Exception $exception) {
$this->fail($exception->getMessage());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@
<element name="firstRowConditionName" type="text" selector=".data-grid>tbody>tr>td.col-name"/>
<element name="firstRowConditionId" type="text" selector=".data-grid>tbody>tr>td.col-id.col-agreement_id"/>
<element name="successMessage" type="text" selector=".message-success"/>
<element name="allTermRows" type="block" selector="//table[@id='agreementGrid_table']//tbody//tr[not(contains(@class,'data-grid-tr-no-data'))]"/>
<element name="emptyGrid" type="block" selector="//table[@id='agreementGrid_table']//tbody//tr[contains(@class,'data-grid-tr-no-data')]"/>
</section>
</sections>
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,9 @@
<deleteData createDataKey="createProduct" stepKey="deletedProduct"/>

<actionGroup ref="AdminTermsConditionsOpenGridActionGroup" stepKey="openTermsGridToDelete"/>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{activeHtmlTerm.name}}"/>
</actionGroup>
<actionGroup ref="AdminTermsConditionsDeleteTermByNameActionGroup" stepKey="deleteOpenedTerm"/>
<actionGroup ref="AdminDeleteAllTermConditionsActionGroup" stepKey="deleteAllTerms"/>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
<comment userInput="BIC workaround" stepKey="deleteOpenedTerm"/>

<actionGroup ref="AdminLogoutActionGroup" stepKey="adminLogout"/>
</after>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@
<group value="mtf_migrated"/>
</annotations>
<after>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{activeTextTerm.name}}"/>
</actionGroup>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
</after>

<actionGroup ref="AdminTermsConditionsFillTermEditFormActionGroup" stepKey="fillNewTerm">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@
<deleteData createDataKey="createProduct" stepKey="deletedProduct"/>

<actionGroup ref="AdminTermsConditionsOpenGridActionGroup" stepKey="openTermsGridToDelete"/>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{disabledTextTerm.name}}"/>
</actionGroup>
<actionGroup ref="AdminTermsConditionsDeleteTermByNameActionGroup" stepKey="deleteOpenedTerm"/>
<actionGroup ref="AdminDeleteAllTermConditionsActionGroup" stepKey="deleteAllTerms"/>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
<comment userInput="BIC workaround" stepKey="deleteOpenedTerm"/>

<actionGroup ref="AdminLogoutActionGroup" stepKey="adminLogout"/>
</after>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,9 @@
<deleteData createDataKey="createdProduct2" stepKey="deletedProduct2"/>

<actionGroup ref="AdminTermsConditionsOpenGridActionGroup" stepKey="openTermsGridToDelete"/>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{activeTextTerm.name}}"/>
</actionGroup>
<actionGroup ref="AdminTermsConditionsDeleteTermByNameActionGroup" stepKey="deleteOpenedTerm"/>
<actionGroup ref="AdminDeleteAllTermConditionsActionGroup" stepKey="deleteAllTerms"/>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
<comment userInput="BIC workaround" stepKey="deleteOpenedTerm"/>

<actionGroup ref="AdminLogoutActionGroup" stepKey="logout"/>
</after>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,9 @@
<deleteData createDataKey="createProduct" stepKey="deletedProduct"/>

<actionGroup ref="AdminTermsConditionsOpenGridActionGroup" stepKey="openTermsGridToDelete"/>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{activeTextTerm.name}}"/>
</actionGroup>
<actionGroup ref="AdminTermsConditionsDeleteTermByNameActionGroup" stepKey="deleteOpenedTerm"/>
<actionGroup ref="AdminDeleteAllTermConditionsActionGroup" stepKey="deleteAllTerms"/>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
<comment userInput="BIC workaround" stepKey="deleteOpenedTerm"/>

<actionGroup ref="AdminLogoutActionGroup" stepKey="adminLogout"/>
</after>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
</annotations>

<after>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{activeHtmlTerm.name}}"/>
</actionGroup>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
</after>

<actionGroup ref="AdminTermsConditionsFillTermEditFormActionGroup" stepKey="fillNewTerm">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@
<group value="cloud"/>
</annotations>
<after>
<actionGroup ref="AdminOpenEditPageTermsConditionsByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{disabledHtmlTerm.name}}"/>
</actionGroup>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
</after>

<actionGroup ref="AdminTermsConditionsFillTermEditFormActionGroup" stepKey="fillNewTerm">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@
<deleteData createDataKey="testCategory" stepKey="deleteTestCategory"/>
<magentoCLI command="config:set checkout/options/enable_agreements 0" stepKey="setDisableTermsOnCheckout"/>
<actionGroup ref="AdminTermsConditionsOpenGridActionGroup" stepKey="openTermsGridToDelete"/>
<actionGroup ref="AdminTermsConditionsEditTermByNameActionGroup" stepKey="openTermToDelete">
<argument name="termName" value="{{newHtmlTerm.name}}"/>
</actionGroup>
<actionGroup ref="AdminTermsConditionsDeleteTermByNameActionGroup" stepKey="deleteOpenedTerm"/>
<actionGroup ref="AdminDeleteAllTermConditionsActionGroup" stepKey="deleteAllTerms"/>
<comment userInput="BIC workaround" stepKey="openTermToDelete"/>
<comment userInput="BIC workaround" stepKey="deleteOpenedTerm"/>
<magentoCLI command="config:set {{CashOnDeliveryDisabledConfigData.path}} {{CashOnDeliveryDisabledConfigData.value}}" stepKey="disabledCashOnDelivery"/>
<actionGroup ref="AdminLogoutActionGroup" stepKey="adminLogout"/>
</after>
Expand Down
23 changes: 17 additions & 6 deletions app/code/Magento/GraphQl/Controller/GraphQl.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
use Magento\Framework\Controller\Result\JsonFactory;
use Magento\Framework\GraphQl\Exception\ExceptionFormatter;
use Magento\Framework\GraphQl\Query\Fields as QueryFields;
use Magento\Framework\GraphQl\Query\QueryParser;
use Magento\Framework\GraphQl\Query\QueryProcessor;
use Magento\Framework\GraphQl\Query\Resolver\ContextInterface;
use Magento\Framework\GraphQl\Schema\SchemaGeneratorInterface;
Expand All @@ -41,6 +42,7 @@ class GraphQl implements FrontControllerInterface
/**
* @var \Magento\Framework\Webapi\Response
* @deprecated 100.3.2
* @see no replacement
*/
private $response;

Expand All @@ -66,7 +68,8 @@ class GraphQl implements FrontControllerInterface

/**
* @var ContextInterface
* @deprecated 100.3.3 $contextFactory is used for creating Context object
* @deprecated 100.3.3
* @see $contextFactory is used for creating Context object
*/
private $resolverContext;

Expand Down Expand Up @@ -110,6 +113,11 @@ class GraphQl implements FrontControllerInterface
*/
private $areaList;

/**
* @var QueryParser
*/
private $queryParser;

/**
* @param Response $response
* @param SchemaGeneratorInterface $schemaGenerator
Expand All @@ -125,6 +133,7 @@ class GraphQl implements FrontControllerInterface
* @param LogData|null $logDataHelper
* @param LoggerPool|null $loggerPool
* @param AreaList|null $areaList
* @param QueryParser|null $queryParser
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -141,7 +150,8 @@ public function __construct(
ContextFactoryInterface $contextFactory = null,
LogData $logDataHelper = null,
LoggerPool $loggerPool = null,
AreaList $areaList = null
AreaList $areaList = null,
QueryParser $queryParser = null
) {
$this->response = $response;
$this->schemaGenerator = $schemaGenerator;
Expand All @@ -157,6 +167,7 @@ public function __construct(
$this->logDataHelper = $logDataHelper ?: ObjectManager::getInstance()->get(LogData::class);
$this->loggerPool = $loggerPool ?: ObjectManager::getInstance()->get(LoggerPool::class);
$this->areaList = $areaList ?: ObjectManager::getInstance()->get(AreaList::class);
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
}

/**
Expand All @@ -179,18 +190,18 @@ public function dispatch(RequestInterface $request): ResponseInterface
try {
/** @var Http $request */
$this->requestProcessor->validateRequest($request);

$query = $data['query'] ?? '';
$variables = $data['variables'] ?? null;
$parsedQuery = $this->queryParser->parse($query);
$data['parsedQuery'] = $parsedQuery;

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

$result = $this->queryProcessor->process(
$schema,
$query,
$parsedQuery,
$this->contextFactory->create(),
$data['variables'] ?? []
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@

use GraphQL\Language\AST\Node;
use GraphQL\Language\AST\NodeKind;
use GraphQL\Language\Parser;
use GraphQL\Language\Source;
use GraphQL\Language\Visitor;
use Magento\Framework\App\HttpRequestInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\Request\Http;
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
use Magento\Framework\GraphQl\Query\QueryParser;
use Magento\Framework\Phrase;
use Magento\GraphQl\Controller\HttpRequestValidatorInterface;

Expand All @@ -23,6 +23,19 @@
*/
class HttpVerbValidator implements HttpRequestValidatorInterface
{
/**
* @var QueryParser
*/
private $queryParser;

/**
* @param QueryParser|null $queryParser
*/
public function __construct(QueryParser $queryParser = null)
{
$this->queryParser = $queryParser ?: ObjectManager::getInstance()->get(QueryParser::class);
}

/**
* Check if request is using correct verb for query or mutation
*
Expand All @@ -37,9 +50,9 @@ public function validate(HttpRequestInterface $request): void
$query = $request->getParam('query', '');
if (!empty($query)) {
$operationType = '';
$queryAst = Parser::parse(new Source($query ?: '', 'GraphQL'));
$parsedQuery = $this->queryParser->parse($query);
Visitor::visit(
$queryAst,
$parsedQuery,
[
'leave' => [
NodeKind::OPERATION_DEFINITION => function (Node $node) use (&$operationType) {
Expand Down
Loading

0 comments on commit 12da28c

Please sign in to comment.