Merge remote-tracking branch 'origin/MAGETWO-90331' into 2.3-develop-pr16

DianaRusin · DianaRusin · commit 24885014826e · 2018-05-02T17:17:40.000+03:00
diff --git a/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php b/app/code/Magento/UrlRewrite/Model/Storage/DbStorage.php
@@ -3,16 +3,24 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+
 namespace Magento\UrlRewrite\Model\Storage;
 
 use Magento\Framework\Api\DataObjectHelper;
 use Magento\Framework\App\ResourceConnection;
+use Magento\Framework\DB\Select;
 use Magento\UrlRewrite\Model\OptionProvider;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewriteFactory;
 use Psr\Log\LoggerInterface;
-use Magento\UrlRewrite\Service\V1\Data\UrlRewrite as UrlRewriteData;
+use Magento\Framework\App\ObjectManager;
+use Magento\Framework\DB\Adapter\AdapterInterface;
 
+/**
+ * Url rewrites DB storage.
+ *
+ * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
+ */
 class DbStorage extends AbstractStorage
 {
     /**
@@ -26,7 +34,7 @@ class DbStorage extends AbstractStorage
     const ERROR_CODE_DUPLICATE_ENTRY = 1062;
 
     /**
-     * @var \Magento\Framework\DB\Adapter\AdapterInterface
+     * @var AdapterInterface
      */
     protected $connection;
 
@@ -36,15 +44,15 @@ class DbStorage extends AbstractStorage
     protected $resource;
 
     /**
-     * @var \Psr\Log\LoggerInterface
+     * @var LoggerInterface
      */
     private $logger;
 
     /**
-     * @param \Magento\UrlRewrite\Service\V1\Data\UrlRewriteFactory $urlRewriteFactory
+     * @param UrlRewriteFactory $urlRewriteFactory
      * @param DataObjectHelper $dataObjectHelper
-     * @param \Magento\Framework\App\ResourceConnection $resource
-     * @param \Psr\Log\LoggerInterface|null $logger
+     * @param ResourceConnection $resource
+     * @param LoggerInterface|null $logger
      */
     public function __construct(
         UrlRewriteFactory $urlRewriteFactory,
@@ -54,8 +62,8 @@ public function __construct(
     ) {
         $this->connection = $resource->getConnection();
         $this->resource = $resource;
-        $this->logger = $logger ?: \Magento\Framework\App\ObjectManager::getInstance()
-            ->get(\Psr\Log\LoggerInterface::class);
+        $this->logger = $logger ?: ObjectManager::getInstance()
+            ->get(LoggerInterface::class);
 
         parent::__construct($urlRewriteFactory, $dataObjectHelper);
     }
@@ -64,7 +72,7 @@ public function __construct(
      * Prepare select statement for specific filter
      *
      * @param array $data
-     * @return \Magento\Framework\DB\Select
+     * @return Select
      */
     protected function prepareSelect(array $data)
     {
@@ -74,6 +82,7 @@ protected function prepareSelect(array $data)
         foreach ($data as $column => $value) {
             $select->where($this->connection->quoteIdentifier($column) . ' IN (?)', $value);
         }
+
         return $select;
     }
 
@@ -141,17 +150,61 @@ protected function doFindOneByData(array $data)
     }
 
     /**
-     * {@inheritdoc}
+     * Delete old URLs from DB.
+     *
+     * @param UrlRewrite[] $urls
+     * @return void
      */
-    protected function doReplace(array $urls)
+    private function deleteOldUrls(array $urls): void
     {
-        foreach ($this->createFilterDataBasedOnUrls($urls) as $type => $urlData) {
-            $urlData[UrlRewrite::ENTITY_TYPE] = $type;
-            // prevent query locking in a case when nothing to delete
-            if ($this->connection->fetchRow($this->prepareSelect($urlData))) {
-                $this->deleteByData($urlData);
-            }
+        $oldUrlsSelect = $this->connection->select();
+        $oldUrlsSelect->from(
+            $this->resource->getTableName(self::TABLE_NAME)
+        );
+        /** @var UrlRewrite $url */
+        foreach ($urls as $url) {
+            $oldUrlsSelect->orWhere(
+                $this->connection->quoteIdentifier(
+                    UrlRewrite::ENTITY_TYPE
+                ) . ' = ?',
+                $url->getEntityType()
+            );
+            $oldUrlsSelect->where(
+                $this->connection->quoteIdentifier(
+                    UrlRewrite::ENTITY_ID
+                ) . ' = ?',
+                $url->getEntityId()
+            );
+            $oldUrlsSelect->where(
+                $this->connection->quoteIdentifier(
+                    UrlRewrite::STORE_ID
+                ) . ' = ?',
+                $url->getStoreId()
+            );
         }
+
+        // prevent query locking in a case when nothing to delete
+        $checkOldUrlsSelect = clone $oldUrlsSelect;
+        $checkOldUrlsSelect->reset(Select::COLUMNS);
+        $checkOldUrlsSelect->columns('count(*)');
+        $hasOldUrls = (bool)$this->connection->fetchOne($checkOldUrlsSelect);
+
+        if ($hasOldUrls) {
+            $this->connection->query(
+                $oldUrlsSelect->deleteFromSelect(
+                    $this->resource->getTableName(self::TABLE_NAME)
+                )
+            );
+        }
+    }
+
+    /**
+     * @inheritDoc
+     */
+    protected function doReplace(array $urls)
+    {
+        $this->deleteOldUrls($urls);
+
         $data = [];
         foreach ($urls as $url) {
             $data[] = $url->toArray();
@@ -164,12 +217,12 @@ protected function doReplace(array $urls)
             foreach ($urls as $url) {
                 $urlFound = $this->doFindOneByData(
                     [
-                        UrlRewriteData::REQUEST_PATH => $url->getRequestPath(),
-                        UrlRewriteData::STORE_ID => $url->getStoreId()
+                        UrlRewrite::REQUEST_PATH => $url->getRequestPath(),
+                        UrlRewrite::STORE_ID => $url->getStoreId(),
                     ]
                 );
-                if (isset($urlFound[UrlRewriteData::URL_REWRITE_ID])) {
-                    $urlConflicted[$urlFound[UrlRewriteData::URL_REWRITE_ID]] = $url->toArray();
+                if (isset($urlFound[UrlRewrite::URL_REWRITE_ID])) {
+                    $urlConflicted[$urlFound[UrlRewrite::URL_REWRITE_ID]] = $url->toArray();
                 }
             }
             if ($urlConflicted) {
@@ -217,6 +270,7 @@ protected function insertMultiple($data)
      *
      * @param UrlRewrite[] $urls
      * @return array
+     * @deprecated Not used anymore.
      */
     protected function createFilterDataBasedOnUrls($urls)
     {
@@ -230,6 +284,7 @@ protected function createFilterDataBasedOnUrls($urls)
                 }
             }
         }
+
         return $data;
     }
 
diff --git a/app/code/Magento/UrlRewrite/Test/Unit/Model/Storage/DbStorageTest.php b/app/code/Magento/UrlRewrite/Test/Unit/Model/Storage/DbStorageTest.php
@@ -6,6 +6,7 @@
 
 namespace Magento\UrlRewrite\Test\Unit\Model\Storage;
 
+use Magento\Framework\DB\Select;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
 use Magento\UrlRewrite\Model\Storage\DbStorage;
 use Magento\UrlRewrite\Service\V1\Data\UrlRewrite;
@@ -49,10 +50,9 @@ protected function setUp()
             ->disableOriginalConstructor()->getMock();
         $this->dataObjectHelper = $this->createMock(\Magento\Framework\Api\DataObjectHelper::class);
         $this->connectionMock = $this->createMock(\Magento\Framework\DB\Adapter\AdapterInterface::class);
-        $this->select = $this->createPartialMock(
-            \Magento\Framework\DB\Select::class,
-            ['from', 'where', 'deleteFromSelect']
-        );
+        $this->select = $this->getMockBuilder(Select::class)
+            ->disableOriginalConstructor()
+            ->getMock();
         $this->resource = $this->createMock(\Magento\Framework\App\ResourceConnection::class);
 
         $this->resource->expects($this->any())
@@ -447,87 +447,32 @@ public function testReplace()
         // delete
 
         $urlFirst->expects($this->any())
-            ->method('getByKey')
-            ->will($this->returnValueMap([
-                [UrlRewrite::ENTITY_TYPE, 'product'],
-                [UrlRewrite::ENTITY_ID, 'entity_1'],
-                [UrlRewrite::STORE_ID, 'store_id_1'],
-            ]));
-        $urlFirst->expects($this->any())->method('getEntityType')->willReturn('product');
+            ->method('getEntityType')
+            ->willReturn('product');
+        $urlFirst->expects($this->any())
+            ->method('getEntityId')
+            ->willReturn('entity_1');
+        $urlFirst->expects($this->any())
+            ->method('getStoreId')
+            ->willReturn('store_id_1');
+
+        $urlSecond->expects($this->any())
+            ->method('getEntityType')
+            ->willReturn('category');
+        $urlSecond->expects($this->any())
+            ->method('getEntityId')
+            ->willReturn('entity_2');
         $urlSecond->expects($this->any())
-            ->method('getByKey')
-            ->will($this->returnValueMap([
-                [UrlRewrite::ENTITY_TYPE, 'category'],
-                [UrlRewrite::ENTITY_ID, 'entity_2'],
-                [UrlRewrite::STORE_ID, 'store_id_2'],
-            ]));
-        $urlSecond->expects($this->any())->method('getEntityType')->willReturn('category');
+            ->method('getStoreId')
+            ->willReturn('store_id_2');
 
         $this->connectionMock->expects($this->any())
             ->method('quoteIdentifier')
             ->will($this->returnArgument(0));
 
-        $this->select->expects($this->at(1))
-            ->method('where')
-            ->with('entity_id IN (?)', ['entity_1']);
-
-        $this->select->expects($this->at(2))
-            ->method('where')
-            ->with('store_id IN (?)', ['store_id_1']);
-
-        $this->select->expects($this->at(3))
-            ->method('where')
-            ->with('entity_type IN (?)', 'product');
-
-        $this->select->expects($this->at(5))
-            ->method('where')
-            ->with('entity_id IN (?)', ['entity_1']);
-
-        $this->select->expects($this->at(6))
-            ->method('where')
-            ->with('store_id IN (?)', ['store_id_1']);
-
-        $this->select->expects($this->at(7))
-            ->method('where')
-            ->with('entity_type IN (?)', 'product');
-
-        $this->connectionMock->expects($this->any())
-            ->method('fetchRow')
-            ->willReturn(['some-data']);
-
-        $this->select->expects($this->at(8))
-            ->method('deleteFromSelect')
-            ->with('table_name')
-            ->will($this->returnValue('sql delete query'));
-
-        $this->select->expects($this->at(10))
-            ->method('where')
-            ->with('entity_id IN (?)', ['entity_2']);
-
-        $this->select->expects($this->at(11))
-            ->method('where')
-            ->with('store_id IN (?)', ['store_id_2']);
-
-        $this->select->expects($this->at(12))
-            ->method('where')
-            ->with('entity_type IN (?)', 'category');
-
-        $this->select->expects($this->at(14))
-            ->method('where')
-            ->with('entity_id IN (?)', ['entity_2']);
-
-        $this->select->expects($this->at(15))
-            ->method('where')
-            ->with('store_id IN (?)', ['store_id_2']);
-
-        $this->select->expects($this->at(16))
-            ->method('where')
-            ->with('entity_type IN (?)', 'category');
-
-        $this->select->expects($this->at(17))
-            ->method('deleteFromSelect')
-            ->with('table_name')
-            ->will($this->returnValue('sql delete query'));
+        $this->select->expects($this->any())
+            ->method($this->anything())
+            ->willReturnSelf();
 
         $this->resource->expects($this->any())
             ->method('getTableName')
