Merge pull request #7395 from magento-performance/MCP-789

MCP-789

Author: vzabaznov

app/code/Magento/Checkout/view/frontend/web/js/model/totals.js

@@ -22,7 +22,7 @@ define([
         quoteItems(newValue.items);
     });
 
-    if (!isNaN(subtotalAmount) && quoteSubtotal !== subtotalAmount) {
+    if (!isNaN(subtotalAmount) && quoteSubtotal !== subtotalAmount && quoteSubtotal !== 0) {
         customerData.reload(['cart'], false);
     }
 

dev/tests/integration/testsuite/Magento/Framework/GraphQl/GraphQlSchemaStitching/GraphQlReaderTest.php

@@ -0,0 +1,67 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+declare(strict_types=1);
+
+namespace Magento\Framework\GraphQl\GraphQlSchemaStitching;
+
+use Magento\Framework\GraphQlSchemaStitching\GraphQlReader;
+use Magento\Framework\ObjectManagerInterface;
+use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
+use Magento\Framework\Config\FileResolverInterface;
+use Magento\Framework\Config\FileIterator;
+use Magento\Framework\Component\ComponentRegistrar;
+
+/**
+ * Test of the stitching of graphql schemas together
+ */
+class GraphQlReaderTest extends TestCase
+{
+    /**
+     * Object Manager Instance
+     *
+     * @var ObjectManager
+     */
+    private $objectManager;
+
+    /**
+     * @var GraphQlReader|MockObject
+     */
+    private $graphQlReader;
+
+    protected function setUp(): void
+    {
+        /** @var ObjectManagerInterface $objectManager */
+        $this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
+
+        $this->graphQlReader = $this->objectManager->create(
+            GraphQlReader::class
+        );
+    }
+
+    /**
+     * This test ensures that the global graphql schemas have all the required dependencies and can be stitched together
+     *
+     * The $results variables contains the actual schema as it will be on a production site which will vary per each
+     * update of magento, so asserting the array matches the entire schema does not make full sense here as any change
+     * in graphql in any magento module would break the test.
+     *
+     * Testing this way means we do not need to store the module meta data that was introduced in
+     * https://github.com/magento/magento2/pull/28747 which means we can greatly improve the performance of this
+     */
+    public function testStitchGlobalGraphQLSchema()
+    {
+        $results = $this->graphQlReader->read('global');
+
+        $this->assertArrayHasKey('Price', $results);
+        $this->assertArrayHasKey('Query', $results);
+        $this->assertArrayHasKey('Mutation', $results);
+        $this->assertArrayHasKey('ProductInterface', $results);
+        $this->assertArrayHasKey('SimpleProduct', $results);
+    }
+}

lib/internal/Magento/Framework/GraphQlSchemaStitching/GraphQlReader.php

@@ -9,7 +9,6 @@
 
 use GraphQL\Type\Definition\ScalarType;
 use GraphQL\Utils\BuildSchema;
-use Magento\Framework\Component\ComponentRegistrar;
 use Magento\Framework\Config\FileResolverInterface;
 use Magento\Framework\Config\ReaderInterface;
 use Magento\Framework\GraphQl\Type\TypeManagement;
@@ -48,11 +47,6 @@ class GraphQlReader implements ReaderInterface
      */
     private $defaultScope;
 
-    /**
-     * @var ComponentRegistrar
-     */
-    private static $componentRegistrar;
-
     /**
      * @param FileResolverInterface $fileResolver
      * @param TypeReaderComposite $typeReader
@@ -75,7 +69,7 @@ public function __construct(
 
     /**
      * @inheritdoc
-     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      * @param string|null $scope
      * @return array
      */
@@ -84,28 +78,103 @@ public function read($scope = null): array
         $results = [];
         $scope = $scope ?: $this->defaultScope;
         $schemaFiles = $this->fileResolver->get($this->fileName, $scope);
-
         if (!count($schemaFiles)) {
             return $results;
         }
 
         /**
+         * Gather as many schema together to be parsed in one go for performance
+         * Collect any duplicate types in an array to retry after the initial large parse
+         *
          * Compatible with @see GraphQlReader::parseTypes
          */
+        $typesToRedo = [];
         $knownTypes = [];
-        foreach ($schemaFiles as $filePath => $partialSchemaContent) {
-            $partialSchemaTypes = $this->parseTypes($partialSchemaContent);
+        foreach ($schemaFiles as $partialSchemaContent) {
+            $partialSchemaTypes = $this->parseTypesWithUnionHandling($partialSchemaContent);
+
+            // Filter out duplicated ones and save them into a list to be retried
+            $tmpTypes = $knownTypes;
+            foreach ($partialSchemaTypes as $intendedKey => $partialSchemaType) {
+                if (isset($tmpTypes[$intendedKey])) {
+                    if (!isset($typesToRedo[$intendedKey])) {
+                        $typesToRedo[$intendedKey] = [];
+                    }
+                    $typesToRedo[$intendedKey][] = $partialSchemaType;
+                    continue;
+                }
+                $tmpTypes[$intendedKey] = $partialSchemaType;
+            }
+            $knownTypes = $tmpTypes;
+        }
+
+        /**
+         * Read this large batch of data, this builds most of the $results array
+         */
+        $schemaContent = implode("\n", $knownTypes);
+        $results = $this->readPartialTypes($schemaContent);
+
+        /**
+         * Go over the list of types to be retried and batch them up into as few batches as possible
+         */
+        $typesToRedoBatches = [];
+        foreach ($typesToRedo as $type => $batches) {
+            foreach ($batches as $id => $data) {
+                if (!isset($typesToRedoBatches[$id])) {
+                    $typesToRedoBatches[$id] = [];
+                }
+                $typesToRedoBatches[$id][$type] = $data;
+            }
+        }
 
-            // Keep declarations from current partial schema, add missing declarations from all previously read schemas
-            $knownTypes = $partialSchemaTypes + $knownTypes;
-            $schemaContent = implode("\n", $knownTypes);
+        /**
+         * Process each remaining batch with the minimal amount of additional schema data for performance
+         */
+        foreach ($typesToRedoBatches as $typesToRedoBatch) {
+            $typesToUse =  $this->getTypesToUse($typesToRedoBatch, $knownTypes);
+            $knownTypes = $typesToUse + $knownTypes;
+            $schemaContent = implode("\n", $typesToUse);
 
             $partialResults = $this->readPartialTypes($schemaContent);
             $results = array_replace_recursive($results, $partialResults);
-            $results = $this->addModuleNameToTypes($results, $filePath);
         }
 
-        return $this->copyInterfaceFieldsToConcreteTypes($results);
+        $results = $this->copyInterfaceFieldsToConcreteTypes($results);
+        return $results;
+    }
+
+    /**
+     * Get the minimum amount of additional types so that performance is improved
+     *
+     * The use of a strpos check here is a bit odd in the context of feeding data into an AST but for the performance
+     * gains and to prevent downtime it is necessary
+     *
+     * @link https://github.com/webonyx/graphql-php/issues/244
+     * @link https://github.com/webonyx/graphql-php/issues/244#issuecomment-383912418
+     *
+     * @param array $typesToRedoBatch
+     * @param array $types
+     * @return array
+     */
+    private function getTypesToUse($typesToRedoBatch, $types): array
+    {
+        $totalKnownSymbolsCount = count($typesToRedoBatch) + count($types);
+
+        $typesToUse = $typesToRedoBatch;
+        for ($i=0; $i < $totalKnownSymbolsCount; $i++) {
+            $changesMade = false;
+            $schemaContent = implode("\n", $typesToUse);
+            foreach ($types as $type => $schema) {
+                if ((!isset($typesToUse[$type]) && strpos($schemaContent, $type) !== false)) {
+                    $typesToUse[$type] = $schema;
+                    $changesMade = true;
+                }
+            }
+            if (!$changesMade) {
+                break;
+            }
+        }
+        return $typesToUse;
     }
 
     /**
@@ -137,6 +206,56 @@ private function readPartialTypes(string $graphQlSchemaContent): array
         return $this->removePlaceholderFromResults($partialResults);
     }
 
+    /**
+     * Extract types as string from a larger string that represents the graphql schema using regular expressions
+     *
+     * The regex in parseTypes does not have the ability to split out the union data from the type below it for example
+     *
+     *  > union X = Y | Z
+     *  >
+     *  > type foo {}
+     *
+     * This would produce only type key from parseTypes, X, which would contain also the type foo entry.
+     *
+     * This wrapper does some post processing as a workaround to split out the union data from the type data below it
+     * which would give us two entries, X and foo
+     *
+     * @param string $graphQlSchemaContent
+     * @return string[] [$typeName => $typeDeclaration, ...]
+     */
+    private function parseTypesWithUnionHandling(string $graphQlSchemaContent): array
+    {
+        $types = $this->parseTypes($graphQlSchemaContent);
+
+        /*
+         * A union schema contains also the data from the schema below it
+         *
+         * If there are two newlines in this union schema then it has data below its definition, meaning it contains
+         * type information not relevant to its actual type
+         */
+        $unionTypes = array_filter(
+            $types,
+            function ($t) {
+                return (strpos($t, 'union ') !== false) && (strpos($t, PHP_EOL . PHP_EOL) !== false);
+            }
+        );
+
+        foreach ($unionTypes as $type => $schema) {
+            $splitSchema = explode(PHP_EOL . PHP_EOL, $schema);
+            // Get the type data at the bottom, this will be the additional type data not related to the union
+            $additionalTypeSchema = end($splitSchema);
+            // Parse the additional type from the bottom so we can have its type key => schema pair
+            $additionalTypeData = $this->parseTypes($additionalTypeSchema);
+            // Fix the union type schema so it does not contain the definition below it
+            $types[$type] = str_replace($additionalTypeSchema, '', $schema);
+            // Append the additional data to types array
+            $additionalTypeKey = array_key_first($additionalTypeData);
+            $types[$additionalTypeKey] = $additionalTypeData[$additionalTypeKey];
+        }
+
+        return $types;
+    }
+
     /**
      * Extract types as string from a larger string that represents the graphql schema using regular expressions
      *
@@ -147,7 +266,7 @@ private function parseTypes(string $graphQlSchemaContent): array
     {
         $typeKindsPattern = '(type|interface|union|enum|input)';
         $typeNamePattern = '([_A-Za-z][_0-9A-Za-z]+)';
-        $typeDefinitionPattern = '([^\{]*)(\{[^\}]*\})';
+        $typeDefinitionPattern = '([^\{\}]*)(\{[^\}]*\})';
         $spacePattern = '[\s\t\n\r]+';
 
         preg_match_all(
@@ -259,7 +378,7 @@ private function addPlaceHolderInSchema(string $graphQlSchemaContent): string
         $typesKindsPattern = '(type|interface|input|union)';
         $enumKindsPattern = '(enum)';
         $typeNamePattern = '([_A-Za-z][_0-9A-Za-z]+)';
-        $typeDefinitionPattern = '([^\{]*)(\{[\s\t\n\r^\}]*\})';
+        $typeDefinitionPattern = '([^\{\}]*)(\{[\s\t\n\r^\}]*\})';
         $spacePattern = '([\s\t\n\r]+)';
 
         //add placeholder in empty types
@@ -299,49 +418,4 @@ private function removePlaceholderFromResults(array $partialResults): array
         }
         return $partialResults;
     }
-
-    /**
-     * Get a module name by file path
-     *
-     * @param string $file
-     * @return string
-     */
-    private static function getModuleNameForRelevantFile(string $file): string
-    {
-        if (!isset(self::$componentRegistrar)) {
-            self::$componentRegistrar = new ComponentRegistrar();
-        }
-        $foundModuleName = '';
-        foreach (self::$componentRegistrar->getPaths(ComponentRegistrar::MODULE) as $moduleName => $moduleDir) {
-            if (strpos($file, $moduleDir . '/') !== false) {
-                $foundModuleName = str_replace('_', '\\', $moduleName);
-                break;
-            }
-        }
-
-        return $foundModuleName;
-    }
-
-    /**
-     * Add a module name to types
-     *
-     * @param array $source
-     * @param string $filePath
-     * @return array
-     */
-    private function addModuleNameToTypes(array $source, string $filePath): array
-    {
-        foreach ($source as $typeName => $typeDefinition) {
-            if (!isset($typeDefinition['module'])) {
-                $hasTypeResolver = (bool)($typeDefinition['typeResolver'] ?? false);
-                $hasImplements = (bool)($typeDefinition['implements'] ?? false);
-                $typeDefinition = (bool)($typeDefinition['type'] ?? false);
-                if ((($typeDefinition === InterfaceType::GRAPHQL_INTERFACE && $hasTypeResolver) || $hasImplements)) {
-                    $source[$typeName]['module'] = self::getModuleNameForRelevantFile($filePath);
-                }
-            }
-        }
-
-        return $source;
-    }
 }