From 605d847e997a8ab9177b0cd48474984f56706628 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Mon, 27 Jul 2020 20:21:52 -0400 Subject: [PATCH 01/20] Added a filter to sort categories by position by default --- .../Magento/CatalogGraphQl/Model/Category/CategoryFilter.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/code/Magento/CatalogGraphQl/Model/Category/CategoryFilter.php b/app/code/Magento/CatalogGraphQl/Model/Category/CategoryFilter.php index 1fae247c981d2..dc93005983776 100644 --- 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; From 262445fbc6f635e8a5e5c3b4ecabb93411f93a4d Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 28 Jul 2020 07:36:52 -0400 Subject: [PATCH 02/20] Test Fixes for the new filter that sorts categories by position --- .../CategoriesQuery/CategoriesFilterTest.php | 18 +++++++++--------- .../CategoriesPaginationTest.php | 2 +- .../GraphQl/Catalog/CategoryListTest.php | 18 +++++++++--------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php index 4da588794b2a9..4444b81619bd3 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php @@ -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/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesPaginationTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesPaginationTest.php index c7fbcbd38c7e4..bbc84a82737bd 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesPaginationTest.php +++ b/dev/tests/api-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 index d6477c82513e9..c49baf7333dde 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php @@ -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' ] ] ], From 4c9adb5d653cd296c160f65e25ea0922f73f9ce2 Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Wed, 29 Jul 2020 15:29:39 -0500 Subject: [PATCH 03/20] MC-35903: Refactor place where lock mechanism is used regarding proper lock description --- .../Console/StartConsumerCommand.php | 32 +++++++++---------- .../Framework/Lock/Backend/CacheTest.php | 29 ++++++----------- .../Cache/LockGuardedCacheLoader.php | 2 +- .../Magento/Framework/Lock/Backend/Cache.php | 28 ++++++++++++---- 4 files changed, 48 insertions(+), 43 deletions(-) diff --git a/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php b/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php index fc2207dcd7c86..f0f9cf4b68bdb 100644 --- a/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php +++ b/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php @@ -79,22 +79,20 @@ protected function execute(InputInterface $input, OutputInterface $output) $singleThread = $input->getOption(self::OPTION_SINGLE_THREAD); - if ($singleThread && $this->lockManager->isLocked(md5($consumerName))) { //phpcs:ignore - $output->writeln('Consumer with the same name is running'); - 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 + try { + if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore + $output->writeln('Consumer with the same name is running'); + return \Magento\Framework\Console\Cli::RETURN_FAILURE; + } + + $this->appState->setAreaCode($areaCode ?? 'global'); + + $consumer = $this->consumerFactory->get($consumerName, $batchSize); + $consumer->process($numberOfMessages); + } finally { + if ($singleThread) { + $this->lockManager->unlock(md5($consumerName)); //phpcs:ignore + } } return \Magento\Framework\Console\Cli::RETURN_SUCCESS; @@ -163,7 +161,7 @@ protected function configure() To specify the preferred area: %command.full_name% someConsumer --area-code='adminhtml' - + To do not run multiple copies of one consumer simultaneously: %command.full_name% someConsumer --single-thread' diff --git a/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php b/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php index 306bda462820a..bf5b282c805e6 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php @@ -47,16 +47,10 @@ public function testParallelLock(): void { $identifier1 = \uniqid('lock_name_1_', true); - $this->assertTrue($this->cacheInstance1->lock($identifier1, 2)); + $this->assertTrue($this->cacheInstance1->lock($identifier1)); - $this->assertFalse($this->cacheInstance1->lock($identifier1, 2)); - $this->assertFalse($this->cacheInstance2->lock($identifier1, 2)); - sleep(4); - $this->assertFalse($this->cacheInstance1->isLocked($identifier1)); - - $this->assertTrue($this->cacheInstance2->lock($identifier1, -1)); - sleep(4); - $this->assertTrue($this->cacheInstance1->isLocked($identifier1)); + $this->assertFalse($this->cacheInstance1->lock($identifier1, 0)); + $this->assertFalse($this->cacheInstance2->lock($identifier1, 0)); } /** @@ -66,19 +60,16 @@ public function testParallelLock(): void */ public function testParallelLockExpired(): void { - $identifier1 = \uniqid('lock_name_1_', true); + $lifeTime = \Closure::bind(function (Cache $class) { + return $class->defaultLifetime; + }, null, $this->cacheInstance1)($this->cacheInstance1); - $this->assertTrue($this->cacheInstance1->lock($identifier1, 1)); - sleep(2); - $this->assertFalse($this->cacheInstance1->isLocked($identifier1)); + $identifier1 = \uniqid('lock_name_1_', true); - $this->assertTrue($this->cacheInstance1->lock($identifier1, 1)); - sleep(2); - $this->assertFalse($this->cacheInstance1->isLocked($identifier1)); + $this->assertTrue($this->cacheInstance1->lock($identifier1, 0)); + $this->assertTrue($this->cacheInstance2->lock($identifier1, $lifeTime + 1)); - $this->assertTrue($this->cacheInstance2->lock($identifier1, 1)); - sleep(2); - $this->assertFalse($this->cacheInstance1->isLocked($identifier1)); + $this->cacheInstance2->unlock($identifier1); } /** diff --git a/lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php b/lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php index 439648b3cc32b..d3b4c8852267a 100644 --- a/lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php +++ b/lib/internal/Magento/Framework/Cache/LockGuardedCacheLoader.php @@ -131,7 +131,7 @@ public function lockedLoadData( return $dataCollector(); } - if ($this->locker->lock($lockName, $this->lockTimeout / 1000)) { + if ($this->locker->lock($lockName, 0)) { try { $data = $dataCollector(); $dataSaver($data); diff --git a/lib/internal/Magento/Framework/Lock/Backend/Cache.php b/lib/internal/Magento/Framework/Lock/Backend/Cache.php index 612d8541281b0..c8517c9cf73c4 100644 --- a/lib/internal/Magento/Framework/Lock/Backend/Cache.php +++ b/lib/internal/Magento/Framework/Lock/Backend/Cache.php @@ -31,6 +31,20 @@ class Cache implements \Magento\Framework\Lock\LockManagerInterface */ private $lockSign; + /** + * How many microseconds to wait before re-try to acquire a lock + * + * @var int + */ + private $sleepCycle = 100000; + + /** + * Lifetime of lock data in seconds. + * + * @var int + */ + private $defaultLifetime = 10; + /** * @param FrontendInterface $cache */ @@ -49,14 +63,16 @@ public function lock(string $name, int $timeout = -1): bool $this->lockSign = $this->generateLockSign(); } - $data = $this->cache->load($this->getIdentifier($name)); - - if (false !== $data) { - return false; + $skipDeadline = $timeout < 0; + $deadline = microtime(true) + $timeout; + while ($this->cache->load($this->getIdentifier($name))) { + if (!$skipDeadline && $deadline <= microtime(true)) { + return false; + } + usleep($this->sleepCycle); } - $timeout = $timeout <= 0 ? null : $timeout; - $this->cache->save($this->lockSign, $this->getIdentifier($name), [], $timeout); + $this->cache->save($this->lockSign, $this->getIdentifier($name), [], $this->defaultLifetime); $data = $this->cache->load($this->getIdentifier($name)); From 3df1b6197805cf4d8c450af4f2dd4cf5bea0c8c7 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Thu, 30 Jul 2020 15:17:39 -0400 Subject: [PATCH 04/20] Added a new default sort by ID when searching Elasticsearch in SearchCriteriaBuilder.php Remove the _id from the order that is being passed to the product collection in ProductSearch.php Updated the position to be DESC passed to the product collection in ProductSearch.php Updated the ProductSearchTest.php to have the items in the attended order --- .../Product/SearchCriteriaBuilder.php | 20 +++++++++++++++++++ .../Products/DataProvider/ProductSearch.php | 9 ++++++++- .../GraphQl/Catalog/ProductSearchTest.php | 2 +- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php index 1057d21283ea8..a91910f3d578a 100644 --- a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php +++ b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php @@ -104,6 +104,7 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte $this->addDefaultSortOrder($searchCriteria, $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 * diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index c35caa07b4785..45721eff12b48 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -7,6 +7,7 @@ namespace Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider; +use Magento\Catalog\Api\Data\EavAttributeInterface; use Magento\Catalog\Api\Data\ProductSearchResultsInterfaceFactory; use Magento\Catalog\Model\ResourceModel\Product\Collection; use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; @@ -18,6 +19,7 @@ use Magento\Framework\Api\Search\SearchResultInterface; use Magento\Framework\Api\SearchCriteriaInterface; use Magento\Framework\Api\SearchResultsInterface; +use Magento\Framework\Api\SortOrder; use Magento\GraphQl\Model\Query\ContextInterface; /** @@ -153,7 +155,12 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) $sortOrders = $searchCriteria->getSortOrders(); if (is_array($sortOrders)) { foreach ($sortOrders as $sortOrder) { - $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection(); + if ($sortOrder->getField() !== '_id') { + if ($sortOrder->getField() == EavAttributeInterface::POSITION) { + $sortOrder->setDirection(SortOrder::SORT_DESC); + } + $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection(); + } } } diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php index dd5b5827c8017..315b0b51d22bf 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php @@ -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(); From 62ba8aa8833444aaf130fd861a66f9b881c1a171 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Thu, 30 Jul 2020 15:41:30 -0400 Subject: [PATCH 05/20] Added a new default sort by ID when searching Elasticsearch in SearchCriteriaBuilder.php Remove the _id from the order that is being passed to the product collection in ProductSearch.php Updated the position to be DESC passed to the product collection in ProductSearch.php Updated the ProductSearchTest.php to have the items in the attended order --- .../Products/DataProvider/ProductSearch.php | 13 ++++++++++--- .../Model/Resolver/Products/Query/Search.php | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index 45721eff12b48..afc5fc77bc46d 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -86,6 +86,7 @@ public function __construct( * * @param SearchCriteriaInterface $searchCriteria * @param SearchResultInterface $searchResult + * @param array $args * @param array $attributes * @param ContextInterface|null $context * @return SearchResultsInterface @@ -93,6 +94,7 @@ public function __construct( public function getList( SearchCriteriaInterface $searchCriteria, SearchResultInterface $searchResult, + array $args, array $attributes = [], ContextInterface $context = null ): SearchResultsInterface { @@ -105,7 +107,7 @@ public function getList( $this->getSearchResultsApplier( $searchResult, $collection, - $this->getSortOrderArray($searchCriteriaForCollection) + $this->getSortOrderArray($searchCriteriaForCollection, $args) )->apply(); $this->collectionPreProcessor->process($collection, $searchCriteriaForCollection, $attributes, $context); @@ -147,9 +149,10 @@ private function getSearchResultsApplier( * E.g. ['field1' => 'DESC', 'field2' => 'ASC", ...] * * @param SearchCriteriaInterface $searchCriteria + * @param array $args * @return array */ - private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) + private function getSortOrderArray(SearchCriteriaInterface $searchCriteria, $args) { $ordersArray = []; $sortOrders = $searchCriteria->getSortOrders(); @@ -157,7 +160,11 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) foreach ($sortOrders as $sortOrder) { if ($sortOrder->getField() !== '_id') { if ($sortOrder->getField() == EavAttributeInterface::POSITION) { - $sortOrder->setDirection(SortOrder::SORT_DESC); + if (isset($args['sort'][EavAttributeInterface::POSITION])) { + $sortOrder->setDirection($args['sort'][EavAttributeInterface::POSITION]); + } else { + $sortOrder->setDirection(SortOrder::SORT_DESC); + } } $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 index 29c3ce279e6a1..faf1d56452a24 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php @@ -103,7 +103,7 @@ 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, $args, $queryFields, $context); $totalPages = $realPageSize ? ((int)ceil($searchResults->getTotalCount() / $realPageSize)) : 0; From 565f63ee540d6c6d3ced666c1dc816315e26d54f Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Thu, 30 Jul 2020 16:53:49 -0500 Subject: [PATCH 06/20] MC-35903: Refactor place where lock mechanism is used regarding proper lock description --- .../Unit/Console/StartConsumerCommandTest.php | 23 +++++-------- .../Framework/Lock/Backend/CacheTest.php | 7 ++-- .../Test/Unit/LockGuardedCacheLoaderTest.php | 6 ++-- .../Magento/Framework/Lock/Backend/Cache.php | 33 ++++++++++++++++++- .../Lock/Test/Unit/Backend/CacheTest.php | 2 +- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php b/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php index b73fcc278f970..274386a9bb685 100644 --- 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,8 +177,7 @@ 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, @@ -191,10 +185,9 @@ public function executeDataProvider() [ 'pidFilePath' => '/var/consumer.pid', 'singleThread' => true, - 'lockExpects' => 0, - 'isLockedExpects' => 1, - 'isLocked' => true, - 'unlockExpects' => 0, + 'lockExpects' => 1, + 'isLocked' => false, + 'unlockExpects' => 1, 'runProcessExpects' => 0, 'expectedReturn' => Cli::RETURN_FAILURE, ], diff --git a/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php b/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php index bf5b282c805e6..81ab34fae9b98 100644 --- a/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php +++ b/dev/tests/integration/testsuite/Magento/Framework/Lock/Backend/CacheTest.php @@ -60,14 +60,15 @@ public function testParallelLock(): void */ public function testParallelLockExpired(): void { - $lifeTime = \Closure::bind(function (Cache $class) { - return $class->defaultLifetime; + $testLifeTime = 2; + \Closure::bind(function (Cache $class) use ($testLifeTime) { + $class->defaultLifetime = $testLifeTime; }, null, $this->cacheInstance1)($this->cacheInstance1); $identifier1 = \uniqid('lock_name_1_', true); $this->assertTrue($this->cacheInstance1->lock($identifier1, 0)); - $this->assertTrue($this->cacheInstance2->lock($identifier1, $lifeTime + 1)); + $this->assertTrue($this->cacheInstance2->lock($identifier1, $testLifeTime + 1)); $this->cacheInstance2->unlock($identifier1); } diff --git a/lib/internal/Magento/Framework/Cache/Test/Unit/LockGuardedCacheLoaderTest.php b/lib/internal/Magento/Framework/Cache/Test/Unit/LockGuardedCacheLoaderTest.php index aa3df00953fda..9dcfb89373c2b 100644 --- a/lib/internal/Magento/Framework/Cache/Test/Unit/LockGuardedCacheLoaderTest.php +++ b/lib/internal/Magento/Framework/Cache/Test/Unit/LockGuardedCacheLoaderTest.php @@ -95,7 +95,7 @@ public function testDataCollectedAfterDeadlineReached(): void $this->lockManagerInterfaceMock ->expects($this->atLeastOnce())->method('lock') - ->with($lockName, 10) + ->with($lockName, 0) ->willReturn(false); $this->lockManagerInterfaceMock->expects($this->never())->method('unlock'); @@ -129,7 +129,7 @@ public function testDataWrite(): void $this->lockManagerInterfaceMock ->expects($this->once())->method('lock') - ->with($lockName, 10) + ->with($lockName, 0) ->willReturn(true); $this->lockManagerInterfaceMock->expects($this->once())->method('unlock'); @@ -168,7 +168,7 @@ public function testDataCollectedWithParallelGeneration(): void $this->lockManagerInterfaceMock ->expects($this->once())->method('lock') - ->with($lockName, 10) + ->with($lockName, 0) ->willReturn(false); $this->lockManagerInterfaceMock->expects($this->never())->method('unlock'); diff --git a/lib/internal/Magento/Framework/Lock/Backend/Cache.php b/lib/internal/Magento/Framework/Lock/Backend/Cache.php index c8517c9cf73c4..ae777a6701cde 100644 --- a/lib/internal/Magento/Framework/Lock/Backend/Cache.php +++ b/lib/internal/Magento/Framework/Lock/Backend/Cache.php @@ -43,7 +43,14 @@ class Cache implements \Magento\Framework\Lock\LockManagerInterface * * @var int */ - private $defaultLifetime = 10; + private $defaultLifetime = 7200; + + /** + * Array for keeping all lock attempt to release them on destruct. + * + * @var string[] + */ + private $lockArrayState = []; /** * @param FrontendInterface $cache @@ -77,6 +84,7 @@ public function lock(string $name, int $timeout = -1): bool $data = $this->cache->load($this->getIdentifier($name)); if ($data === $this->lockSign) { + $this->lockArrayState[$name] = 1; return true; } @@ -101,6 +109,7 @@ public function unlock(string $name): bool $removeResult = false; if ($data === $this->lockSign) { $removeResult = (bool)$this->cache->remove($this->getIdentifier($name)); + unset($this->lockArrayState[$name]); } return $removeResult; @@ -147,4 +156,26 @@ private function generateLockSign() return $sign; } + + /** + * Destruct method should release all locks that left. + * + * @return void + */ + public function __destruct() + { + $this->releaseLocks(); + } + + /** + * Release all locks that were not removed with unlock method. + * + * @return void + */ + private function releaseLocks() + { + foreach ($this->lockArrayState as $name => $value) { + $this->unlock($name); + } + } } diff --git a/lib/internal/Magento/Framework/Lock/Test/Unit/Backend/CacheTest.php b/lib/internal/Magento/Framework/Lock/Test/Unit/Backend/CacheTest.php index 5b5c87ce454b3..3e46d4fe6fc76 100644 --- a/lib/internal/Magento/Framework/Lock/Test/Unit/Backend/CacheTest.php +++ b/lib/internal/Magento/Framework/Lock/Test/Unit/Backend/CacheTest.php @@ -162,6 +162,6 @@ public function testLockWithNotEmptyData(): void ->with(self::LOCK_PREFIX . $identifier) ->willReturn(\uniqid('some_rand-', true)); - $this->assertEquals(false, $this->cache->lock($identifier)); + $this->assertEquals(false, $this->cache->lock($identifier, 0)); } } From 340001779400e91f82c58ef2e34f6cc55b79e292 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Fri, 31 Jul 2020 10:21:23 -0400 Subject: [PATCH 07/20] Updated tests to cover changes to the sort orders --- .../Catalog/CategoriesQuery/CategoriesFilterTest.php | 8 ++++---- .../GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php | 4 ++-- .../Magento/GraphQl/Catalog/CategoryListTest.php | 8 ++++---- .../Magento/GraphQl/Catalog/ProductSearchTest.php | 2 +- .../testsuite/Magento/GraphQl/Catalog/ProductViewTest.php | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php index 4444b81619bd3..e8246ac36f406 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php @@ -187,9 +187,9 @@ public function testQueryChildCategoriesWithProducts() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $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'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children @@ -280,9 +280,9 @@ public function testQueryCategoryWithDisabledChildren() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $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'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php index c2e82e734cd9b..ad9ffa9eb7edd 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php @@ -441,7 +441,7 @@ public function testCategoryProducts() $this->assertArrayHasKey('products', $response['categories']['items'][0]); $baseCategory = $response['categories']['items'][0]; $this->assertArrayHasKey('total_count', $baseCategory['products']); - $this->assertGreaterThanOrEqual(1, $baseCategory['products']['total_count']); + //$this->assertGreaterThanOrEqual(1, $baseCategory['products']['total_count']); $this->assertEquals(1, $baseCategory['products']['page_info']['current_page']); $this->assertEquals(20, $baseCategory['products']['page_info']['page_size']); $this->assertArrayHasKey('sku', $baseCategory['products']['items'][0]); @@ -453,7 +453,7 @@ public function testCategoryProducts() $this->assertAttributes($firstProduct); $this->assertWebsites($firstProductModel, $firstProduct['websites']); $this->assertEquals('Category 1', $firstProduct['categories'][0]['name']); - $this->assertEquals('category-1/category-1-1', $firstProduct['categories'][1]['url_path']); + $this->assertEquals('movable-position-2', $firstProduct['categories'][1]['url_path']); $this->assertCount(3, $firstProduct['categories']); } diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php index c49baf7333dde..3dcba3277dfa7 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php @@ -186,9 +186,9 @@ public function testQueryChildCategoriesWithProducts() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $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'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children @@ -277,9 +277,9 @@ public function testQueryCategoryWithDisabledChildren() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $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'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductSearchTest.php index 315b0b51d22bf..226f240a247ec 100644 --- 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); diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/ProductViewTest.php index c6719f1862ddc..87f8b62ed84a1 100644 --- 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 = << Date: Fri, 31 Jul 2020 13:12:57 -0400 Subject: [PATCH 08/20] Remove the position from the filter and replaced it with Entity Id --- .../Product/SearchCriteriaBuilder.php | 28 +++++++++++++------ .../Products/DataProvider/ProductSearch.php | 20 +++++++------ 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php index a91910f3d578a..6a686cf301fcc 100644 --- a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php +++ b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php @@ -101,7 +101,7 @@ public function build(array $args, bool $includeAggregation): SearchCriteriaInte } if (!$searchCriteria->getSortOrders()) { - $this->addDefaultSortOrder($searchCriteria, $isSearch); + $this->addDefaultSortOrder($searchCriteria, $args, $isSearch); } $this->addEntityIdSort($searchCriteria, $isSearch); @@ -199,18 +199,28 @@ 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 && 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/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index afc5fc77bc46d..e51c7c60648ac 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -20,6 +20,7 @@ use Magento\Framework\Api\SearchCriteriaInterface; use Magento\Framework\Api\SearchResultsInterface; use Magento\Framework\Api\SortOrder; +use Magento\Framework\Exception\InputException; use Magento\GraphQl\Model\Query\ContextInterface; /** @@ -151,6 +152,7 @@ private function getSearchResultsApplier( * @param SearchCriteriaInterface $searchCriteria * @param array $args * @return array + * @throws InputException */ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria, $args) { @@ -158,16 +160,18 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria, $arg $sortOrders = $searchCriteria->getSortOrders(); if (is_array($sortOrders)) { foreach ($sortOrders as $sortOrder) { - if ($sortOrder->getField() !== '_id') { - if ($sortOrder->getField() == EavAttributeInterface::POSITION) { - if (isset($args['sort'][EavAttributeInterface::POSITION])) { - $sortOrder->setDirection($args['sort'][EavAttributeInterface::POSITION]); - } else { - $sortOrder->setDirection(SortOrder::SORT_DESC); - } + if ($sortOrder->getField() === '_id') { + $sortOrder->setField('entity_id'); + $categoryIdFilter = isset($args['filter']['category_id']) ? $args['filter']['category_id'] : false; + if ($categoryIdFilter && count($categoryIdFilter[array_key_first($categoryIdFilter)]) <= 1) { + $sortOrder->setDirection(SortOrder::SORT_ASC); } - $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection(); } + $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection(); + } + if (isset($ordersArray[EavAttributeInterface::POSITION])) { + $ordersArray['entity_id'] = $ordersArray[EavAttributeInterface::POSITION]; + unset($ordersArray[EavAttributeInterface::POSITION]); } } From 30c79f336b42cb381ed57a6c23654b469e261313 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Fri, 31 Jul 2020 13:48:15 -0400 Subject: [PATCH 09/20] Removed bad code that was not needed --- .../Resolver/Products/DataProvider/ProductSearch.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index e51c7c60648ac..e4823b4aa557a 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -162,17 +162,9 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria, $arg foreach ($sortOrders as $sortOrder) { if ($sortOrder->getField() === '_id') { $sortOrder->setField('entity_id'); - $categoryIdFilter = isset($args['filter']['category_id']) ? $args['filter']['category_id'] : false; - if ($categoryIdFilter && count($categoryIdFilter[array_key_first($categoryIdFilter)]) <= 1) { - $sortOrder->setDirection(SortOrder::SORT_ASC); - } } $ordersArray[$sortOrder->getField()] = $sortOrder->getDirection(); } - if (isset($ordersArray[EavAttributeInterface::POSITION])) { - $ordersArray['entity_id'] = $ordersArray[EavAttributeInterface::POSITION]; - unset($ordersArray[EavAttributeInterface::POSITION]); - } } return $ordersArray; From c137e0ec7cd852e5a8ed66e685cd9613504d01f0 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Sun, 2 Aug 2020 09:59:22 -0400 Subject: [PATCH 10/20] Updated the default sort to check if the category id is passed as an array. Reverted the tests back to check on products --- .../DataProvider/Product/SearchCriteriaBuilder.php | 5 ++++- .../GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php | 4 ++-- .../GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php | 2 +- .../testsuite/Magento/GraphQl/Catalog/CategoryListTest.php | 4 ++-- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php index 6a686cf301fcc..173d055180119 100644 --- a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php +++ b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php @@ -212,7 +212,10 @@ private function addDefaultSortOrder(SearchCriteriaInterface $searchCriteria, ar ->create(); } else { $categoryIdFilter = isset($args['filter']['category_id']) ? $args['filter']['category_id'] : false; - if ($categoryIdFilter && count($categoryIdFilter[array_key_first($categoryIdFilter)]) <= 1) { + if ($categoryIdFilter && + !is_array($categoryIdFilter[array_key_first($categoryIdFilter)]) || + count($categoryIdFilter[array_key_first($categoryIdFilter)]) <= 1 + ) { $defaultSortOrder[] = $this->sortOrderBuilder ->setField(EavAttributeInterface::POSITION) ->setDirection(SortOrder::SORT_ASC) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php index e8246ac36f406..3ba7805ba2d25 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php @@ -280,9 +280,9 @@ public function testQueryCategoryWithDisabledChildren() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $expectedBaseCategoryProducts = [ + ['sku' => 'simple', 'name' => 'Simple Product'], ['sku' => 'simple-4', 'name' => 'Simple Product Three'], - ['sku' => '12345', 'name' => 'Simple Product Two'], - ['sku' => 'simple', 'name' => 'Simple Product'] + ['sku' => '12345', 'name' => 'Simple Product Two'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php index ad9ffa9eb7edd..d4089161cd894 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php @@ -441,7 +441,7 @@ public function testCategoryProducts() $this->assertArrayHasKey('products', $response['categories']['items'][0]); $baseCategory = $response['categories']['items'][0]; $this->assertArrayHasKey('total_count', $baseCategory['products']); - //$this->assertGreaterThanOrEqual(1, $baseCategory['products']['total_count']); + $this->assertGreaterThanOrEqual(1, $baseCategory['products']['total_count']); $this->assertEquals(1, $baseCategory['products']['page_info']['current_page']); $this->assertEquals(20, $baseCategory['products']['page_info']['page_size']); $this->assertArrayHasKey('sku', $baseCategory['products']['items'][0]); diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php index 3dcba3277dfa7..03e0aefa3a732 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php @@ -186,9 +186,9 @@ public function testQueryChildCategoriesWithProducts() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $expectedBaseCategoryProducts = [ + ['sku' => 'simple', 'name' => 'Simple Product'], ['sku' => 'simple-4', 'name' => 'Simple Product Three'], - ['sku' => '12345', 'name' => 'Simple Product Two'], - ['sku' => 'simple', 'name' => 'Simple Product'] + ['sku' => '12345', 'name' => 'Simple Product Two'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children From 34cf76dc80633e2a558aa9f583e8f51604f13417 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Sun, 2 Aug 2020 13:27:36 -0400 Subject: [PATCH 11/20] Updated the default sort to check if the category id is passed as an array. Reverted the tests back to check on products --- .../Product/SearchCriteriaBuilder.php | 15 +++++++-------- .../CategoriesQuery/CategoriesFilterTest.php | 6 +++--- .../Catalog/CategoriesQuery/CategoryTreeTest.php | 2 +- .../Magento/GraphQl/Catalog/CategoryListTest.php | 10 +++++----- 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php index 173d055180119..fe7272b933f75 100644 --- a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php +++ b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php @@ -212,14 +212,13 @@ private function addDefaultSortOrder(SearchCriteriaInterface $searchCriteria, ar ->create(); } else { $categoryIdFilter = isset($args['filter']['category_id']) ? $args['filter']['category_id'] : false; - if ($categoryIdFilter && - !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(); + 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(); + } } } diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php index 3ba7805ba2d25..679d7659cbf9e 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php @@ -187,9 +187,9 @@ public function testQueryChildCategoriesWithProducts() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $expectedBaseCategoryProducts = [ + ['sku' => 'simple', 'name' => 'Simple Product'], ['sku' => 'simple-4', 'name' => 'Simple Product Three'], ['sku' => '12345', 'name' => 'Simple Product Two'], - ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children @@ -297,8 +297,8 @@ public function testQueryCategoryWithDisabledChildren() $this->assertEquals('Its a description of Test Category 1.2', $firstChildCategory['description']); $firstChildCategoryExpectedProducts = [ - ['sku' => 'simple-4', 'name' => 'Simple Product Three'], - ['sku' => 'simple', 'name' => 'Simple Product'] + ['sku' => 'simple', 'name' => 'Simple Product'], + ['sku' => 'simple-4', 'name' => 'Simple Product Three'] ]; $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php index d4089161cd894..c2e82e734cd9b 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoryTreeTest.php @@ -453,7 +453,7 @@ public function testCategoryProducts() $this->assertAttributes($firstProduct); $this->assertWebsites($firstProductModel, $firstProduct['websites']); $this->assertEquals('Category 1', $firstProduct['categories'][0]['name']); - $this->assertEquals('movable-position-2', $firstProduct['categories'][1]['url_path']); + $this->assertEquals('category-1/category-1-1', $firstProduct['categories'][1]['url_path']); $this->assertCount(3, $firstProduct['categories']); } diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php index 03e0aefa3a732..9f2ca5fe6de4d 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php @@ -214,8 +214,8 @@ public function testQueryChildCategoriesWithProducts() $this->assertEquals('Category 1.2', $secondChildCategory['name']); $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-4', 'name' => 'Simple Product Three'] ]; $this->assertCategoryProducts($secondChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; @@ -277,9 +277,9 @@ public function testQueryCategoryWithDisabledChildren() $this->assertArrayHasKey('products', $baseCategory); //Check base category products $expectedBaseCategoryProducts = [ + ['sku' => 'simple', 'name' => 'Simple Product'], ['sku' => 'simple-4', 'name' => 'Simple Product Three'], - ['sku' => '12345', 'name' => 'Simple Product Two'], - ['sku' => 'simple', 'name' => 'Simple Product'] + ['sku' => '12345', 'name' => 'Simple Product Two'] ]; $this->assertCategoryProducts($baseCategory, $expectedBaseCategoryProducts); //Check base category children @@ -294,8 +294,8 @@ public function testQueryCategoryWithDisabledChildren() $this->assertEquals('Its a description of Test Category 1.2', $firstChildCategory['description']); $firstChildCategoryExpectedProducts = [ - ['sku' => 'simple-4', 'name' => 'Simple Product Three'], - ['sku' => 'simple', 'name' => 'Simple Product'] + ['sku' => 'simple', 'name' => 'Simple Product'], + ['sku' => 'simple-4', 'name' => 'Simple Product Three'] ]; $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; From d3aa6b61c467748e17045d8ca0d062a50178dd21 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Sun, 2 Aug 2020 13:45:41 -0400 Subject: [PATCH 12/20] Updated test --- .../Magento/GraphQl/Catalog/CategoryListTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php index 9f2ca5fe6de4d..43612575a7dcb 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php @@ -214,8 +214,8 @@ public function testQueryChildCategoriesWithProducts() $this->assertEquals('Category 1.2', $secondChildCategory['name']); $this->assertEquals('Its a description of Test Category 1.2', $secondChildCategory['description']); $firstChildCategoryExpectedProducts = [ - ['sku' => 'simple', 'name' => 'Simple Product'], - ['sku' => 'simple-4', 'name' => 'Simple Product Three'] + ['sku' => 'simple-4', 'name' => 'Simple Product Three'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($secondChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; @@ -294,8 +294,8 @@ public function testQueryCategoryWithDisabledChildren() $this->assertEquals('Its a description of Test Category 1.2', $firstChildCategory['description']); $firstChildCategoryExpectedProducts = [ - ['sku' => 'simple', 'name' => 'Simple Product'], - ['sku' => 'simple-4', 'name' => 'Simple Product Three'] + ['sku' => 'simple-4', 'name' => 'Simple Product Three'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; From 7e5d864a2e574e69a571e2e7f4650cd1fb0fff3e Mon Sep 17 00:00:00 2001 From: dnyomo Date: Sun, 2 Aug 2020 15:58:50 -0400 Subject: [PATCH 13/20] missed this test --- .../GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php index 679d7659cbf9e..a3daf89631c17 100644 --- a/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php +++ b/dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoriesQuery/CategoriesFilterTest.php @@ -297,8 +297,8 @@ public function testQueryCategoryWithDisabledChildren() $this->assertEquals('Its a description of Test Category 1.2', $firstChildCategory['description']); $firstChildCategoryExpectedProducts = [ - ['sku' => 'simple', 'name' => 'Simple Product'], - ['sku' => 'simple-4', 'name' => 'Simple Product Three'] + ['sku' => 'simple-4', 'name' => 'Simple Product Three'], + ['sku' => 'simple', 'name' => 'Simple Product'] ]; $this->assertCategoryProducts($firstChildCategory, $firstChildCategoryExpectedProducts); $firstChildCategoryChildren = []; From 1c7318324a5ba6672bdfaf31ebc3f51033cf16ae Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Mon, 3 Aug 2020 13:47:18 -0500 Subject: [PATCH 14/20] MC-35903: Refactor place where lock mechanism is used regarding proper lock description --- .../Console/StartConsumerCommand.php | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php b/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php index f0f9cf4b68bdb..8ea6290a2a430 100644 --- a/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php +++ b/app/code/Magento/MessageQueue/Console/StartConsumerCommand.php @@ -79,20 +79,17 @@ protected function execute(InputInterface $input, OutputInterface $output) $singleThread = $input->getOption(self::OPTION_SINGLE_THREAD); - try { - if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore - $output->writeln('Consumer with the same name is running'); - return \Magento\Framework\Console\Cli::RETURN_FAILURE; - } - - $this->appState->setAreaCode($areaCode ?? 'global'); - - $consumer = $this->consumerFactory->get($consumerName, $batchSize); - $consumer->process($numberOfMessages); - } finally { - if ($singleThread) { - $this->lockManager->unlock(md5($consumerName)); //phpcs:ignore - } + if ($singleThread && !$this->lockManager->lock(md5($consumerName),0)) { //phpcs:ignore + $output->writeln('Consumer with the same name is running'); + return \Magento\Framework\Console\Cli::RETURN_FAILURE; + } + + $this->appState->setAreaCode($areaCode ?? 'global'); + + $consumer = $this->consumerFactory->get($consumerName, $batchSize); + $consumer->process($numberOfMessages); + if ($singleThread) { + $this->lockManager->unlock(md5($consumerName)); //phpcs:ignore } return \Magento\Framework\Console\Cli::RETURN_SUCCESS; From 0c6de1dc4578ded5ad22746a0b4b4e53898573c2 Mon Sep 17 00:00:00 2001 From: Vitalii Zabaznov Date: Mon, 3 Aug 2020 17:07:10 -0500 Subject: [PATCH 15/20] MC-35903: Refactor place where lock mechanism is used regarding proper lock description --- .../MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php b/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php index 274386a9bb685..1aa805f0e323b 100644 --- a/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php +++ b/app/code/Magento/MessageQueue/Test/Unit/Console/StartConsumerCommandTest.php @@ -187,7 +187,7 @@ public function executeDataProvider() 'singleThread' => true, 'lockExpects' => 1, 'isLocked' => false, - 'unlockExpects' => 1, + 'unlockExpects' => 0, 'runProcessExpects' => 0, 'expectedReturn' => Cli::RETURN_FAILURE, ], From 056cb380f6d9bdcf7e68a5abde0eb6c52eaca1d2 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 4 Aug 2020 13:15:24 -0400 Subject: [PATCH 16/20] Code style fixes --- .../DataProvider/Product/SearchCriteriaBuilder.php | 4 +++- .../Resolver/Products/DataProvider/ProductSearch.php | 7 +++---- .../Model/Resolver/Products/Query/Search.php | 9 ++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php index fe7272b933f75..b6837b334fdd8 100644 --- a/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php +++ b/app/code/Magento/CatalogGraphQl/DataProvider/Product/SearchCriteriaBuilder.php @@ -213,7 +213,9 @@ private function addDefaultSortOrder(SearchCriteriaInterface $searchCriteria, ar } 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) { + 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) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index e4823b4aa557a..c2ee0cc89468a 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -87,15 +87,14 @@ public function __construct( * * @param SearchCriteriaInterface $searchCriteria * @param SearchResultInterface $searchResult - * @param array $args * @param array $attributes * @param ContextInterface|null $context * @return SearchResultsInterface + * @throws InputException */ public function getList( SearchCriteriaInterface $searchCriteria, SearchResultInterface $searchResult, - array $args, array $attributes = [], ContextInterface $context = null ): SearchResultsInterface { @@ -108,7 +107,7 @@ public function getList( $this->getSearchResultsApplier( $searchResult, $collection, - $this->getSortOrderArray($searchCriteriaForCollection, $args) + $this->getSortOrderArray($searchCriteriaForCollection) )->apply(); $this->collectionPreProcessor->process($collection, $searchCriteriaForCollection, $attributes, $context); @@ -154,7 +153,7 @@ private function getSearchResultsApplier( * @return array * @throws InputException */ - private function getSortOrderArray(SearchCriteriaInterface $searchCriteria, $args) + private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) { $ordersArray = []; $sortOrders = $searchCriteria->getSortOrders(); diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/Query/Search.php index faf1d56452a24..4eb76fb5c2d5b 100644 --- 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, $args, $queryFields, $context); + $searchResults = $this->productsProvider->getList( + $searchCriteria, + $itemsResults, + $queryFields, + $context + ); $totalPages = $realPageSize ? ((int)ceil($searchResults->getTotalCount() / $realPageSize)) : 0; From 2f1b09c3eaa64acf0b6688d2e92edb44d5ed244b Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 4 Aug 2020 14:05:40 -0400 Subject: [PATCH 17/20] Code style fixes --- .../Model/Resolver/Products/DataProvider/ProductSearch.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index c2ee0cc89468a..071c0786a9bfc 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -7,7 +7,6 @@ namespace Magento\CatalogGraphQl\Model\Resolver\Products\DataProvider; -use Magento\Catalog\Api\Data\EavAttributeInterface; use Magento\Catalog\Api\Data\ProductSearchResultsInterfaceFactory; use Magento\Catalog\Model\ResourceModel\Product\Collection; use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory; @@ -19,7 +18,6 @@ use Magento\Framework\Api\Search\SearchResultInterface; use Magento\Framework\Api\SearchCriteriaInterface; use Magento\Framework\Api\SearchResultsInterface; -use Magento\Framework\Api\SortOrder; use Magento\Framework\Exception\InputException; use Magento\GraphQl\Model\Query\ContextInterface; @@ -149,7 +147,6 @@ private function getSearchResultsApplier( * E.g. ['field1' => 'DESC', 'field2' => 'ASC", ...] * * @param SearchCriteriaInterface $searchCriteria - * @param array $args * @return array * @throws InputException */ From e1f12471d36cdd94165d2023f34e060de49eba83 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 4 Aug 2020 15:03:29 -0400 Subject: [PATCH 18/20] Code style fixes and added a message around converting _id to entity_id --- .../Model/Resolver/Products/DataProvider/ProductSearch.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index 071c0786a9bfc..66d89f050899c 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -18,11 +18,11 @@ use Magento\Framework\Api\Search\SearchResultInterface; use Magento\Framework\Api\SearchCriteriaInterface; use Magento\Framework\Api\SearchResultsInterface; -use Magento\Framework\Exception\InputException; use Magento\GraphQl\Model\Query\ContextInterface; /** * Product field data provider for product search, used for GraphQL resolver processing. + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ProductSearch { @@ -88,7 +88,6 @@ public function __construct( * @param array $attributes * @param ContextInterface|null $context * @return SearchResultsInterface - * @throws InputException */ public function getList( SearchCriteriaInterface $searchCriteria, @@ -148,7 +147,6 @@ private function getSearchResultsApplier( * * @param SearchCriteriaInterface $searchCriteria * @return array - * @throws InputException */ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) { @@ -156,6 +154,8 @@ 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();. if ($sortOrder->getField() === '_id') { $sortOrder->setField('entity_id'); } From a50dae1c2090bd8ed17967c3ff6565c0f94ca933 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 4 Aug 2020 15:11:22 -0400 Subject: [PATCH 19/20] Updated the message with the related method --- .../Model/Resolver/Products/DataProvider/ProductSearch.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index 66d89f050899c..d8b46beb6ba2b 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -156,6 +156,7 @@ private function getSortOrderArray(SearchCriteriaInterface $searchCriteria) 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'); } From 1c340c33735218f98840b025b92e39a77e0f53e5 Mon Sep 17 00:00:00 2001 From: dnyomo Date: Tue, 4 Aug 2020 15:15:07 -0400 Subject: [PATCH 20/20] Remvoed @SuppressWarnings(PHPMD.CouplingBetweenObjects) --- .../Model/Resolver/Products/DataProvider/ProductSearch.php | 1 - 1 file changed, 1 deletion(-) diff --git a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php index d8b46beb6ba2b..4807cad54bd50 100644 --- a/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php +++ b/app/code/Magento/CatalogGraphQl/Model/Resolver/Products/DataProvider/ProductSearch.php @@ -22,7 +22,6 @@ /** * Product field data provider for product search, used for GraphQL resolver processing. - * @SuppressWarnings(PHPMD.CouplingBetweenObjects) */ class ProductSearch {