MC-31304: [ElasticSearch] Exception on catalog search result page

ameysar · ameysar · commit 22dd903c67ff · 2020-07-28T12:46:46.000+03:00
diff --git a/app/code/Magento/Elasticsearch/Model/DataProvider/Base/Suggestions.php b/app/code/Magento/Elasticsearch/Model/DataProvider/Base/Suggestions.php
@@ -5,19 +5,23 @@
  */
 namespace Magento\Elasticsearch\Model\DataProvider\Base;
 
-use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProviderInterface;
-use Magento\Store\Model\ScopeInterface;
-use Magento\Search\Model\QueryInterface;
+use Elasticsearch\Common\Exceptions\BadRequest400Exception;
 use Magento\AdvancedSearch\Model\SuggestedQueriesInterface;
+use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProviderInterface;
 use Magento\Elasticsearch\Model\Config;
 use Magento\Elasticsearch\SearchAdapter\ConnectionManager;
-use Magento\Search\Model\QueryResultFactory;
-use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
+use Magento\Framework\App\Config\ScopeConfigInterface;
+use Magento\Framework\App\ObjectManager;
+use Magento\Search\Model\QueryInterface;
+use Magento\Search\Model\QueryResultFactory;
+use Magento\Store\Model\ScopeInterface;
 use Magento\Store\Model\StoreManagerInterface as StoreManager;
+use Psr\Log\LoggerInterface;
 
 /**
  * Default implementation to provide suggestions mechanism for Elasticsearch
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class Suggestions implements SuggestedQueriesInterface
 {
@@ -56,6 +60,11 @@ class Suggestions implements SuggestedQueriesInterface
      */
     private $fieldProvider;
 
+    /**
+     * @var LoggerInterface
+     */
+    private $logger;
+
     /**
      * Suggestions constructor.
      *
@@ -66,6 +75,7 @@ class Suggestions implements SuggestedQueriesInterface
      * @param SearchIndexNameResolver $searchIndexNameResolver
      * @param StoreManager $storeManager
      * @param FieldProviderInterface $fieldProvider
+     * @param LoggerInterface|null $logger
      */
     public function __construct(
         ScopeConfigInterface $scopeConfig,
@@ -74,7 +84,8 @@ public function __construct(
         ConnectionManager $connectionManager,
         SearchIndexNameResolver $searchIndexNameResolver,
         StoreManager $storeManager,
-        FieldProviderInterface $fieldProvider
+        FieldProviderInterface $fieldProvider,
+        LoggerInterface $logger = null
     ) {
         $this->queryResultFactory = $queryResultFactory;
         $this->connectionManager = $connectionManager;
@@ -83,6 +94,7 @@ public function __construct(
         $this->searchIndexNameResolver = $searchIndexNameResolver;
         $this->storeManager = $storeManager;
         $this->fieldProvider = $fieldProvider;
+        $this->logger = $logger ?: ObjectManager::getInstance()->get(LoggerInterface::class);
     }
 
     /**
@@ -93,8 +105,14 @@ public function getItems(QueryInterface $query)
         $result = [];
         if ($this->isSuggestionsAllowed()) {
             $isResultsCountEnabled = $this->isResultsCountEnabled();
+            try {
+                $suggestions = $this->getSuggestions($query);
+            } catch (BadRequest400Exception $e) {
+                $this->logger->critical($e);
+                $suggestions = [];
+            }
 
-            foreach ($this->getSuggestions($query) as $suggestion) {
+            foreach ($suggestions as $suggestion) {
                 $count = null;
                 if ($isResultsCountEnabled) {
                     $count = isset($suggestion['freq']) ? $suggestion['freq'] : null;
diff --git a/app/code/Magento/Elasticsearch/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php b/app/code/Magento/Elasticsearch/Test/Unit/Model/DataProvider/Base/SuggestionsTest.php
@@ -7,7 +7,10 @@
 
 namespace Magento\Elasticsearch\Test\Unit\Model\DataProvider\Base;
 
+use Elasticsearch\Common\Exceptions\BadRequest400Exception;
+use Magento\Elasticsearch\Model\Adapter\FieldMapper\Product\FieldProviderInterface;
 use Magento\Elasticsearch\Model\Config;
+use Magento\Elasticsearch\Model\DataProvider\Base\Suggestions;
 use Magento\Elasticsearch\Model\DataProvider\Suggestions as SuggestionsDataProvider;
 use Magento\Elasticsearch\SearchAdapter\ConnectionManager;
 use Magento\Elasticsearch\SearchAdapter\SearchIndexNameResolver;
@@ -21,6 +24,7 @@
 use Magento\Store\Model\StoreManagerInterface as StoreManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
+use Psr\Log\LoggerInterface;
 
 /**
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
@@ -62,6 +66,21 @@ class SuggestionsTest extends TestCase
      */
     private $storeManager;
 
+    /**
+     * @var FieldProviderInterface|MockObject
+     */
+    private $fieldProvider;
+
+    /**
+     * @var LoggerInterface|MockObject
+     */
+    private $logger;
+
+    /**
+     * @var Elasticsearch|MockObject
+     */
+    private $client;
+
     /**
      * @var QueryInterface|MockObject
      */
@@ -99,7 +118,19 @@ protected function setUp(): void
             ->setMethods(['getIndexName'])
             ->getMock();
 
-        $this->storeManager = $this->getMockBuilder(\Magento\Store\Model\StoreManagerInterface::class)
+        $this->storeManager = $this->getMockBuilder(StoreManager::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $this->fieldProvider = $this->getMockBuilder(FieldProviderInterface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $this->logger = $this->getMockBuilder(LoggerInterface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $this->client = $this->getMockBuilder(Elasticsearch::class)
             ->disableOriginalConstructor()
             ->getMock();
 
@@ -110,81 +141,155 @@ protected function setUp(): void
         $objectManager = new ObjectManagerHelper($this);
 
         $this->model = $objectManager->getObject(
-            \Magento\Elasticsearch\Model\DataProvider\Base\Suggestions::class,
+            Suggestions::class,
             [
                 'queryResultFactory' => $this->queryResultFactory,
                 'connectionManager' => $this->connectionManager,
                 'scopeConfig' => $this->scopeConfig,
                 'config' => $this->config,
                 'searchIndexNameResolver' => $this->searchIndexNameResolver,
-                'storeManager' => $this->storeManager
+                'storeManager' => $this->storeManager,
+                'fieldProvider' => $this->fieldProvider,
+                'logger' => $this->logger,
             ]
         );
     }
 
     /**
-     * Test getItems() method
+     * Test get items process with search suggestions disabled.
+     * @return void
      */
-    public function testGetItems()
+    public function testGetItemsWithDisabledSearchSuggestion(): void
     {
-        $this->scopeConfig->expects($this->any())
-            ->method('getValue')
-            ->willReturn(1);
-
-        $this->config->expects($this->any())
-            ->method('isElasticsearchEnabled')
-            ->willReturn(1);
-
-        $store = $this->getMockBuilder(StoreInterface::class)
-            ->disableOriginalConstructor()
-            ->getMockForAbstractClass();
+        $this->scopeConfig->expects($this->once())
+            ->method('isSetFlag')
+            ->willReturn(false);
 
-        $this->storeManager->expects($this->any())
-            ->method('getStore')
-            ->willReturn($store);
-
-        $store->expects($this->any())
-            ->method('getId')
-            ->willReturn(1);
+        $this->scopeConfig->expects($this->never())
+            ->method('getValue');
 
-        $this->searchIndexNameResolver->expects($this->any())
-            ->method('getIndexName')
-            ->willReturn('magento2_product_1');
+        $this->config->expects($this->once())
+            ->method('isElasticsearchEnabled')
+            ->willReturn(true);
 
-        $this->query->expects($this->any())
-            ->method('getQueryText')
-            ->willReturn('query');
+        $this->logger->expects($this->never())
+            ->method('critical');
 
-        $client = $this->getMockBuilder(Elasticsearch::class)
-            ->disableOriginalConstructor()
-            ->getMock();
+        $this->queryResultFactory->expects($this->never())
+            ->method('create');
 
-        $this->connectionManager->expects($this->any())
-            ->method('getConnection')
-            ->willReturn($client);
+        $this->assertEmpty($this->model->getItems($this->query));
+    }
 
-        $client->expects($this->any())
+    /**
+     * Test get items process with search suggestions enabled.
+     * @return void
+     */
+    public function testGetItemsWithEnabledSearchSuggestion(): void
+    {
+        $this->prepareSearchQuery();
+        $this->client->expects($this->once())
             ->method('query')
             ->willReturn([
                 'suggest' => [
                     'phrase_field' => [
-                        'options' => [
-                            'text' => 'query',
-                            'score' => 1,
-                            'freq' => 1,
+                        [
+                            'options' => [
+                                'suggestion' => [
+                                    'text' => 'query',
+                                    'score' => 1,
+                                    'freq' => 1,
+                                ]
+                            ]
                         ]
                     ],
                 ],
             ]);
 
+        $this->logger->expects($this->never())
+            ->method('critical');
+
         $query = $this->getMockBuilder(QueryResult::class)
             ->disableOriginalConstructor()
             ->getMock();
 
-        $this->queryResultFactory->expects($this->any())
+        $this->queryResultFactory->expects($this->once())
             ->method('create')
             ->willReturn($query);
 
-        $this->assertIsArray($this->model->getItems($this->query));
+        $this->assertEquals([$query], $this->model->getItems($this->query));
+    }
+
+    /**
+     * Test get items process when throwing an exception.
+     * @return void
+     */
+    public function testGetItemsException(): void
+    {
+        $this->prepareSearchQuery();
+        $exception = new BadRequest400Exception();
+
+        $this->client->expects($this->once())
+            ->method('query')
+            ->willThrowException($exception);
+
+        $this->logger->expects($this->once())
+            ->method('critical')
+            ->with($exception);
+
+        $this->queryResultFactory->expects($this->never())
+            ->method('create');
+
+        $this->assertEmpty($this->model->getItems($this->query));
+    }
+
+    /**
+     * Prepare Mocks for default get items process.
+     * @return void
+     */
+    private function prepareSearchQuery(): void
+    {
+        $storeId = 1;
+
+        $this->scopeConfig->expects($this->exactly(2))
+            ->method('isSetFlag')
+            ->willReturn(true);
+
+        $this->scopeConfig->expects($this->once())
+            ->method('getValue')
+            ->willReturn(1);
+
+        $this->config->expects($this->once())
+            ->method('isElasticsearchEnabled')
+            ->willReturn(true);
+
+        $store = $this->getMockBuilder(StoreInterface::class)
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
+
+        $store->expects($this->once())
+            ->method('getId')
+            ->willReturn($storeId);
+
+        $this->storeManager->expects($this->once())
+            ->method('getStore')
+            ->willReturn($store);
+
+        $this->searchIndexNameResolver->expects($this->once())
+            ->method('getIndexName')
+            ->with($storeId, Config::ELASTICSEARCH_TYPE_DEFAULT)
+            ->willReturn('magento2_product_1');
+
+        $this->query->expects($this->once())
+            ->method('getQueryText')
+            ->willReturn('query');
+
+        $this->fieldProvider->expects($this->once())
+            ->method('getFields')
+            ->willReturn([]);
+
+        $this->connectionManager->expects($this->once())
+            ->method('getConnection')
+            ->willReturn($this->client);
     }
 }
