Merge pull request #5965 from magento-performance/MC-35903

MC-35903
magento · Aug 6, 2020 · 5daadae · 5daadae
2 parents 00f8a8c + 14f3784
commit 5daadae
Show file tree

Hide file tree

Showing 16 changed files with 164 additions and 88 deletions.
diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php
@@ -101,9 +101,10 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte
         }
 
         if (!$searchCriteria->getSortOrders()) {
-            $this->addDefaultSortOrder($searchCriteria, $isSearch);
+            $this->addDefaultSortOrder($searchCriteria, $args, $isSearch);
         }
 
+        $this->addEntityIdSort($searchCriteria, $isSearch);
         $this->addVisibilityFilter($searchCriteria, $isSearch, !empty($args['filter']));
 
         $searchCriteria->setCurrentPage($args['currentPage']);
@@ -132,6 +133,25 @@ private function addVisibilityFilter(SearchCriteriaInterface $searchCriteria, bo
         $this->addFilter($searchCriteria, 'visibility', $visibilityIds, 'in');
     }
 
+    /**
+     * Add sort by Entity ID
+     *
+     * @param SearchCriteriaInterface $searchCriteria
+     * @param bool $isSearch
+     */
+    private function addEntityIdSort(SearchCriteriaInterface $searchCriteria, bool $isSearch): void
+    {
+        if ($isSearch) {
+            return;
+        }
+        $sortOrderArray = $searchCriteria->getSortOrders();
+        $sortOrderArray[] = $this->sortOrderBuilder
+            ->setField('_id')
+            ->setDirection(SortOrder::SORT_DESC)
+            ->create();
+        $searchCriteria->setSortOrders($sortOrderArray);
+    }
+
     /**
      * Prepare price aggregation algorithm
      *
@@ -179,18 +199,32 @@ private function addFilter(
      * Sort by relevance DESC by default
      *
      * @param SearchCriteriaInterface $searchCriteria
+     * @param array $args
      * @param bool $isSearch
      */
-    private function addDefaultSortOrder(SearchCriteriaInterface $searchCriteria, $isSearch = false): void
+    private function addDefaultSortOrder(SearchCriteriaInterface $searchCriteria, array $args, $isSearch = false): void
     {
-        $sortField = $isSearch ? 'relevance' : EavAttributeInterface::POSITION;
-        $sortDirection = $isSearch ? SortOrder::SORT_DESC : SortOrder::SORT_ASC;
-        $defaultSortOrder = $this->sortOrderBuilder
-            ->setField($sortField)
-            ->setDirection($sortDirection)
-            ->create();
+        $defaultSortOrder = [];
+        if ($isSearch) {
+            $defaultSortOrder[] = $this->sortOrderBuilder
+                ->setField('relevance')
+                ->setDirection(SortOrder::SORT_DESC)
+                ->create();
+        } else {
+            $categoryIdFilter = isset($args['filter']['category_id']) ? $args['filter']['category_id'] : false;
+            if ($categoryIdFilter) {
+                if (!is_array($categoryIdFilter[array_key_first($categoryIdFilter)])
+                    || count($categoryIdFilter[array_key_first($categoryIdFilter)]) <= 1
+                ) {
+                    $defaultSortOrder[] = $this->sortOrderBuilder
+                        ->setField(EavAttributeInterface::POSITION)
+                        ->setDirection(SortOrder::SORT_ASC)
+                        ->create();
+                }
+            }
+        }
 
-        $searchCriteria->setSortOrders([$defaultSortOrder]);
+        $searchCriteria->setSortOrders($defaultSortOrder);
     }
 
     /**

diff --git a/app/code/Magento/CatalogGraphQl/Model/Category/CategoryFilter.php b/app/code/Magento/CatalogGraphQl/Model/Category/CategoryFilter.php
@@ -13,6 +13,7 @@
 use Magento\Framework\Exception\InputException;
 use Magento\Framework\GraphQl\Exception\GraphQlInputException;
 use Magento\Framework\GraphQl\Query\Resolver\Argument\SearchCriteria\ArgumentApplier\Filter;
+use Magento\Framework\GraphQl\Query\Resolver\Argument\SearchCriteria\ArgumentApplier\Sort;
 use Magento\Search\Model\Query;
 use Magento\Store\Api\Data\StoreInterface;
 use Magento\Store\Model\ScopeInterface;
@@ -71,6 +72,7 @@ public function getResult(array $criteria, StoreInterface $store)
         $categoryIds = [];
         $criteria[Filter::ARGUMENT_NAME] = $this->formatMatchFilters($criteria['filters'], $store);
         $criteria[Filter::ARGUMENT_NAME][CategoryInterface::KEY_IS_ACTIVE] = ['eq' => 1];
+        $criteria[Sort::ARGUMENT_NAME][CategoryInterface::KEY_POSITION] = ['ASC'];
         $searchCriteria = $this->searchCriteriaBuilder->build('categoryList', $criteria);
         $pageSize = $criteria['pageSize'] ?? 20;
         $currentPage = $criteria['currentPage'] ?? 1;

diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php
@@ -153,6 +153,12 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria)
         $sortOrders = $searchCriteria->getSortOrders();
         if (is_array($sortOrders)) {
             foreach ($sortOrders as $sortOrder) {
+                // I am replacing _id with entity_id because in ElasticSearch _id is required for sorting by ID.
+                // Where as entity_id is required when using ID as the sort in $collection->load();.
+                // @see \Magento\CatalogGraphQl\Model\Resolver\Products\Query\Search::getResult
+                if ($sortOrder->getField() === '_id') {
+                    $sortOrder->setField('entity_id');
+                }
                 $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection();
             }
         }

diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php
@@ -12,6 +12,7 @@
 use Magento\CatalogGraphQl\Model\Resolver\Products\SearchResult;
 use Magento\CatalogGraphQl\Model\Resolver\Products\SearchResultFactory;
 use Magento\Framework\Api\Search\SearchCriteriaInterface;
+use Magento\Framework\Exception\InputException;
 use Magento\Framework\GraphQl\Schema\Type\ResolveInfo;
 use Magento\GraphQl\Model\Query\ContextInterface;
 use Magento\Search\Api\SearchInterface;
@@ -83,6 +84,7 @@ public function __construct(
      * @param ResolveInfo $info
      * @param ContextInterface $context
      * @return SearchResult
+     * @throws InputException
      */
     public function getResult(
         array $args,
@@ -103,7 +105,12 @@ public function getResult(
         //Address limitations of sort and pagination on search API apply original pagination from GQL query
         $searchCriteria->setPageSize($realPageSize);
         $searchCriteria->setCurrentPage($realCurrentPage);
-        $searchResults = $this->productsProvider->getList($searchCriteria, $itemsResults, $queryFields, $context);
+        $searchResults = $this->productsProvider->getList(
+            $searchCriteria,
+            $itemsResults,
+            $queryFields,
+            $context
+        );
 
         $totalPages = $realPageSize ? ((int)ceil($searchResults->getTotalCount() / $realPageSize)) : 0;
 

diff --git a/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php b/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php
@@ -79,20 +79,15 @@ protected function execute(InputInterface $input, OutputInterface $output)
 
         $singleThread = $input->getOption(self::OPTION_SINGLE_THREAD);
 
-        if ($singleThread && $this->lockManager->isLocked(md5($consumerName))) { //phpcs:ignore
+        if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore
             $output->writeln('<error>Consumer with the same name is running</error>');
             return \Magento\Framework\Console\Cli::RETURN_FAILURE;
         }
 
-        if ($singleThread) {
-            $this->lockManager->lock(md5($consumerName)); //phpcs:ignore
-        }
-
         $this->appState->setAreaCode($areaCode ?? 'global');
 
         $consumer = $this->consumerFactory->get($consumerName, $batchSize);
         $consumer->process($numberOfMessages);
-
         if ($singleThread) {
             $this->lockManager->unlock(md5($consumerName)); //phpcs:ignore
         }
@@ -163,7 +158,7 @@ protected function configure()
 To specify the preferred area:
 
     <comment>%command.full_name% someConsumer --area-code='adminhtml'</comment>
-    
+
 To do not run multiple copies of one consumer simultaneously:
 
     <comment>%command.full_name% someConsumer --single-thread'</comment>

diff --git a/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php b/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php
@@ -103,7 +103,6 @@ public function testExecute(
         $pidFilePath,
         $singleThread,
         $lockExpects,
-        $isLockedExpects,
         $isLocked,
         $unlockExpects,
         $runProcessExpects,
@@ -144,14 +143,11 @@ public function testExecute(
             ->method('get')->with($consumerName, $batchSize)->willReturn($consumer);
         $consumer->expects($this->exactly($runProcessExpects))->method('process')->with($numberOfMessages);
 
-        $this->lockManagerMock->expects($this->exactly($isLockedExpects))
-            ->method('isLocked')
-            ->with(md5($consumerName)) //phpcs:ignore
-            ->willReturn($isLocked);
-
         $this->lockManagerMock->expects($this->exactly($lockExpects))
             ->method('lock')
-            ->with(md5($consumerName)); //phpcs:ignore
+            ->with(md5($consumerName))//phpcs:ignore
+            ->willReturn($isLocked);
+
         $this->lockManagerMock->expects($this->exactly($unlockExpects))
             ->method('unlock')
             ->with(md5($consumerName)); //phpcs:ignore
@@ -172,8 +168,7 @@ public function executeDataProvider()
                 'pidFilePath' => null,
                 'singleThread' => false,
                 'lockExpects' => 0,
-                'isLockedExpects' => 0,
-                'isLocked' => false,
+                'isLocked' => true,
                 'unlockExpects' => 0,
                 'runProcessExpects' => 1,
                 'expectedReturn' => Cli::RETURN_SUCCESS,
@@ -182,18 +177,16 @@ public function executeDataProvider()
                 'pidFilePath' => '/var/consumer.pid',
                 'singleThread' => true,
                 'lockExpects' => 1,
-                'isLockedExpects' => 1,
-                'isLocked' => false,
+                'isLocked' => true,
                 'unlockExpects' => 1,
                 'runProcessExpects' => 1,
                 'expectedReturn' => Cli::RETURN_SUCCESS,
             ],
             [
                 'pidFilePath' => '/var/consumer.pid',
                 'singleThread' => true,
-                'lockExpects' => 0,
-                'isLockedExpects' => 1,
-                'isLocked' => true,
+                'lockExpects' => 1,
+                'isLocked' => false,
                 'unlockExpects' => 0,
                 'runProcessExpects' => 0,
                 'expectedReturn' => Cli::RETURN_FAILURE,

diff --git a/...api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/...api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php
@@ -189,7 +189,7 @@ public function testQueryChildCategoriesWithProducts()
         $expectedBaseCategoryProducts = [
             ['sku' => 'simple', 'name' => 'Simple Product'],
             ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
-            ['sku' => '12345', 'name' => 'Simple Product Two']
+            ['sku' => '12345', 'name' => 'Simple Product Two'],
         ];
         $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts);
         //Check base category children
@@ -648,15 +648,6 @@ public function filterMultipleCategoriesDataProvider(): array
                 'in',
                 '["category-1-2", "movable"]',
                 [
-                    [
-                        'id' => '7',
-                        'name' => 'Movable',
-                        'url_key' => 'movable',
-                        'url_path' => 'movable',
-                        'children_count' => '0',
-                        'path' => '1/2/7',
-                        'position' => '3'
-                    ],
                     [
                         'id' => '13',
                         'name' => 'Category 1.2',
@@ -665,6 +656,15 @@ public function filterMultipleCategoriesDataProvider(): array
                         'children_count' => '0',
                         'path' => '1/2/3/13',
                         'position' => '2'
+                    ],
+                    [
+                        'id' => '7',
+                        'name' => 'Movable',
+                        'url_key' => 'movable',
+                        'url_path' => 'movable',
+                        'children_count' => '0',
+                        'path' => '1/2/7',
+                        'position' => '3'
                     ]
                 ]
             ],

diff --git a/...functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesPaginationTest.php b/...functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesPaginationTest.php
@@ -155,7 +155,7 @@ public function testPaging()
         $lastPageQuery = sprintf($baseQuery, $page1Result['categories']['page_info']['total_pages']);
         $lastPageResult = $this->graphQlQuery($lastPageQuery);
         $this->assertCount(1, $lastPageResult['categories']['items']);
-        $this->assertEquals('Category 1.2', $lastPageResult['categories']['items'][0]['name']);
+        $this->assertEquals('Category 12', $lastPageResult['categories']['items'][0]['name']);
     }
 
     /**

diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php
@@ -215,7 +215,7 @@ public function testQueryChildCategoriesWithProducts()
         $this->assertEquals('Its a description of Test Category 1.2', $secondChildCategory['description']);
         $firstChildCategoryExpectedProducts = [
             ['sku' => 'simple-4', 'name' => 'Simple Product Three'],
-            ['sku' => 'simple', 'name' => 'Simple Product'],
+            ['sku' => 'simple', 'name' => 'Simple Product']
         ];
         $this->assertCategoryProducts($secondChildCategory, $firstChildCategoryExpectedProducts);
         $firstChildCategoryChildren = [];
@@ -629,15 +629,6 @@ public function filterMultipleCategoriesDataProvider(): array
                 'in',
                 '["category-1-2", "movable"]',
                 [
-                    [
-                        'id' => '7',
-                        'name' => 'Movable',
-                        'url_key' => 'movable',
-                        'url_path' => 'movable',
-                        'children_count' => '0',
-                        'path' => '1/2/7',
-                        'position' => '3'
-                    ],
                     [
                         'id' => '13',
                         'name' => 'Category 1.2',
@@ -646,6 +637,15 @@ public function filterMultipleCategoriesDataProvider(): array
                         'children_count' => '0',
                         'path' => '1/2/3/13',
                         'position' => '2'
+                    ],
+                    [
+                        'id' => '7',
+                        'name' => 'Movable',
+                        'url_key' => 'movable',
+                        'url_path' => 'movable',
+                        'children_count' => '0',
+                        'path' => '1/2/7',
+                        'position' => '3'
                     ]
                 ]
             ],

diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php
@@ -313,7 +313,7 @@ public function testFilterProductsByDropDownCustomAttribute()
         $product1 = $productRepository->get('simple');
         $product2 = $productRepository->get('12345');
         $product3 = $productRepository->get('simple-4');
-        $filteredProducts = [$product1, $product2, $product3 ];
+        $filteredProducts = [$product3, $product2, $product1];
         $countOfFilteredProducts = count($filteredProducts);
         $this->reIndexAndCleanCache();
         $response = $this->graphQlQuery($query);
@@ -898,7 +898,7 @@ public function testFilterByMultipleProductUrlKeys()
         $product1 = $productRepository->get('simple');
         $product2 = $productRepository->get('12345');
         $product3 = $productRepository->get('simple-4');
-        $filteredProducts = [$product1, $product2, $product3];
+        $filteredProducts = [$product3, $product2, $product1];
         $urlKey =[];
         foreach ($filteredProducts as $product) {
             $urlKey[] = $product->getUrlKey();

diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php
@@ -232,7 +232,7 @@ public function testQueryAllFieldsSimpleProduct()
             special_from_date
             special_price
             special_to_date
-            swatch_image            
+            swatch_image
             tier_price
             tier_prices
             {
@@ -578,8 +578,8 @@ public function testProductLinks()
      */
     public function testProductPrices()
     {
-        $firstProductSku = 'simple-249';
-        $secondProductSku = 'simple-156';
+        $firstProductSku = 'simple-156';
+        $secondProductSku = 'simple-249';
         $query = <<<QUERY
        {
            products(filter: {price: {from: "150.0", to: "250.0"}})
@@ -1050,7 +1050,7 @@ public function testProductInNonAnchoredSubCategories()
     {
         $query = <<<QUERY
 {
-    products(filter: 
+    products(filter:
              {
              sku: {in:["12345"]}
              }