Merge pull request #1784 from magento-panda/MAGETWO-84444

igrybkov · web-flow · commit bd6eebb04850 · 2017-11-30T19:02:22.000+02:00
Fixed issue:
MAGETWO-84444: MySQL triggers for Indexers not removed after upgrade
diff --git a/app/code/Magento/CatalogInventory/etc/mview.xml b/app/code/Magento/CatalogInventory/etc/mview.xml
@@ -5,10 +5,13 @@
  * See COPYING.txt for license details.
  */
 -->
-<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Mview/etc/mview.xsd">
+<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:noNamespaceSchemaLocation="urn:magento:framework:Mview/etc/mview.xsd">
     <view id="cataloginventory_stock" class="Magento\CatalogInventory\Model\Indexer\Stock" group="indexer">
         <subscriptions>
             <table name="cataloginventory_stock_item" entity_column="product_id" />
+            <table name="catalog_product_entity" entity_column="entity_id" />
+            <table name="catalog_product_entity_int" entity_column="entity_id" />
         </subscriptions>
     </view>
 </config>
diff --git a/app/code/Magento/Indexer/Setup/RecurringData.php b/app/code/Magento/Indexer/Setup/RecurringData.php
@@ -0,0 +1,55 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Indexer\Setup;
+
+use Magento\Framework\Setup\InstallDataInterface;
+use Magento\Framework\Setup\ModuleContextInterface;
+use Magento\Framework\Setup\ModuleDataSetupInterface;
+use Magento\Indexer\Model\IndexerFactory;
+use Magento\Framework\Indexer\ConfigInterface;
+
+/**
+ * Recurring data upgrade for indexer module
+ */
+class RecurringData implements InstallDataInterface
+{
+    /**
+     * @var IndexerFactory
+     */
+    private $indexerFactory;
+
+    /**
+     * @var ConfigInterface
+     */
+    private $configInterface;
+
+    /**
+     * @param IndexerFactory $indexerFactory
+     * @param ConfigInterface $configInterface
+     */
+    public function __construct(
+        IndexerFactory $indexerFactory,
+        ConfigInterface $configInterface
+    ) {
+        $this->indexerFactory = $indexerFactory;
+        $this->configInterface = $configInterface;
+    }
+
+    /**
+     * {@inheritdoc}
+     * @throws \Exception
+     */
+    public function install(ModuleDataSetupInterface $setup, ModuleContextInterface $context)
+    {
+        foreach (array_keys($this->configInterface->getIndexers()) as $indexerId) {
+            $indexer = $this->indexerFactory->create()->load($indexerId);
+            if ($indexer->isScheduled()) {
+                $indexer->getView()->unsubscribe()->subscribe();
+            }
+        }
+    }
+}
diff --git a/app/etc/di.xml b/app/etc/di.xml
@@ -1221,4 +1221,11 @@
             </argument>
         </arguments>
     </type>
+    <type name="Magento\Framework\Mview\View\Subscription">
+        <arguments>
+            <argument name="ignoredUpdateColumns" xsi:type="array">
+                <item name="updated_at" xsi:type="string">updated_at</item>
+            </argument>
+        </arguments>
+    </type>
 </config>
diff --git a/lib/internal/Magento/Framework/Mview/Test/Unit/View/SubscriptionTest.php b/lib/internal/Magento/Framework/Mview/Test/Unit/View/SubscriptionTest.php
@@ -62,7 +62,7 @@ protected function setUp()
 
         $this->resourceMock->expects($this->any())
             ->method('getTableName')
-            ->willReturn($this->tableName);
+            ->will($this->returnArgument(0));
 
         $this->model = new Subscription(
             $this->resourceMock,
@@ -89,11 +89,15 @@ public function testGetColumnName()
         $this->assertEquals('columnName', $this->model->getColumnName());
     }
 
+    /**
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
+     */
     public function testCreate()
     {
         $triggerName = 'trigger_name';
         $this->resourceMock->expects($this->atLeastOnce())->method('getTriggerName')->willReturn($triggerName);
         $triggerMock = $this->getMockBuilder('Magento\Framework\DB\Ddl\Trigger')
+            ->setMethods(['setName', 'getName', 'setTime', 'setEvent', 'setTable', 'addStatement'])
             ->disableOriginalConstructor()
             ->getMock();
         $triggerMock->expects($this->exactly(3))
@@ -114,8 +118,35 @@ public function testCreate()
             ->method('setTable')
             ->with($this->tableName)
             ->will($this->returnSelf());
-        $triggerMock->expects($this->exactly(6))
+
+        $triggerMock->expects($this->at(4))
+            ->method('addStatement')
+            ->with("INSERT IGNORE INTO test_view_cl (entity_id) VALUES (NEW.columnName);")
+            ->will($this->returnSelf());
+
+        $triggerMock->expects($this->at(5))
+            ->method('addStatement')
+            ->with("INSERT IGNORE INTO other_test_view_cl (entity_id) VALUES (NEW.columnName);")
+            ->will($this->returnSelf());
+
+        $triggerMock->expects($this->at(11))
+            ->method('addStatement')
+            ->with("INSERT IGNORE INTO test_view_cl (entity_id) VALUES (NEW.columnName);")
+            ->will($this->returnSelf());
+
+        $triggerMock->expects($this->at(12))
+            ->method('addStatement')
+            ->with("INSERT IGNORE INTO other_test_view_cl (entity_id) VALUES (NEW.columnName);")
+            ->will($this->returnSelf());
+
+        $triggerMock->expects($this->at(18))
+            ->method('addStatement')
+            ->with("INSERT IGNORE INTO test_view_cl (entity_id) VALUES (OLD.columnName);")
+            ->will($this->returnSelf());
+
+        $triggerMock->expects($this->at(19))
             ->method('addStatement')
+            ->with("INSERT IGNORE INTO other_test_view_cl (entity_id) VALUES (OLD.columnName);")
             ->will($this->returnSelf());
 
         $changelogMock = $this->getMockForAbstractClass(
diff --git a/lib/internal/Magento/Framework/Mview/View/Subscription.php b/lib/internal/Magento/Framework/Mview/View/Subscription.php
@@ -10,6 +10,8 @@
 
 use Magento\Framework\App\ResourceConnection;
 use Magento\Framework\DB\Ddl\Trigger;
+use Magento\Framework\DB\Ddl\TriggerFactory;
+use Magento\Framework\Mview\ViewInterface;
 
 class Subscription implements SubscriptionInterface
 {
@@ -21,12 +23,12 @@ class Subscription implements SubscriptionInterface
     protected $connection;
 
     /**
-     * @var \Magento\Framework\DB\Ddl\TriggerFactory
+     * @var TriggerFactory
      */
     protected $triggerFactory;
 
     /**
-     * @var \Magento\Framework\Mview\View\CollectionInterface
+     * @var CollectionInterface
      */
     protected $viewCollection;
 
@@ -57,21 +59,32 @@ class Subscription implements SubscriptionInterface
      */
     protected $resource;
 
+    /**
+     * List of columns that can be updated in a subscribed table
+     * without creating a new change log entry
+     *
+     * @var array
+     */
+    private $ignoredUpdateColumns = [];
+
     /**
      * @param ResourceConnection $resource
-     * @param \Magento\Framework\DB\Ddl\TriggerFactory $triggerFactory
-     * @param \Magento\Framework\Mview\View\CollectionInterface $viewCollection
-     * @param \Magento\Framework\Mview\ViewInterface $view
+     * @param TriggerFactory $triggerFactory
+     * @param CollectionInterface $viewCollection
+     * @param ViewInterface $view
      * @param string $tableName
      * @param string $columnName
+     * @param array $ignoredUpdateColumns
+     * @throws \DomainException
      */
     public function __construct(
         ResourceConnection $resource,
-        \Magento\Framework\DB\Ddl\TriggerFactory $triggerFactory,
-        \Magento\Framework\Mview\View\CollectionInterface $viewCollection,
-        \Magento\Framework\Mview\ViewInterface $view,
+        TriggerFactory $triggerFactory,
+        CollectionInterface $viewCollection,
+        ViewInterface $view,
         $tableName,
-        $columnName
+        $columnName,
+        array $ignoredUpdateColumns = []
     ) {
         $this->connection = $resource->getConnection();
         $this->triggerFactory = $triggerFactory;
@@ -80,12 +93,14 @@ public function __construct(
         $this->tableName = $tableName;
         $this->columnName = $columnName;
         $this->resource = $resource;
+        $this->ignoredUpdateColumns = $ignoredUpdateColumns;
     }
 
     /**
-     * Create subsciption
+     * Create subscription
      *
-     * @return \Magento\Framework\Mview\View\SubscriptionInterface
+     * @return SubscriptionInterface
+     * @throws \InvalidArgumentException
      */
     public function create()
     {
@@ -102,7 +117,7 @@ public function create()
 
             // Add statements for linked views
             foreach ($this->getLinkedViews() as $view) {
-                /** @var \Magento\Framework\Mview\ViewInterface $view */
+                /** @var ViewInterface $view */
                 $trigger->addStatement($this->buildStatement($event, $view->getChangelog()));
             }
 
@@ -116,7 +131,8 @@ public function create()
     /**
      * Remove subscription
      *
-     * @return \Magento\Framework\Mview\View\SubscriptionInterface
+     * @return SubscriptionInterface
+     * @throws \InvalidArgumentException
      */
     public function remove()
     {
@@ -131,7 +147,7 @@ public function remove()
 
             // Add statements for linked views
             foreach ($this->getLinkedViews() as $view) {
-                /** @var \Magento\Framework\Mview\ViewInterface $view */
+                /** @var ViewInterface $view */
                 $trigger->addStatement($this->buildStatement($event, $view->getChangelog()));
             }
 
@@ -154,10 +170,10 @@ public function remove()
     protected function getLinkedViews()
     {
         if (!$this->linkedViews) {
-            $viewList = $this->viewCollection->getViewsByStateMode(\Magento\Framework\Mview\View\StateInterface::MODE_ENABLED);
+            $viewList = $this->viewCollection->getViewsByStateMode(StateInterface::MODE_ENABLED);
 
             foreach ($viewList as $view) {
-                /** @var \Magento\Framework\Mview\ViewInterface $view */
+                /** @var ViewInterface $view */
                 // Skip the current view
                 if ($view->getId() == $this->getView()->getId()) {
                     continue;
@@ -175,35 +191,58 @@ protected function getLinkedViews()
     }
 
     /**
-     * Build trigger statement for INSER, UPDATE, DELETE events
+     * Build trigger statement for INSERT, UPDATE, DELETE events
      *
      * @param string $event
-     * @param \Magento\Framework\Mview\View\ChangelogInterface $changelog
+     * @param ChangelogInterface $changelog
      * @return string
      */
     protected function buildStatement($event, $changelog)
     {
         switch ($event) {
             case Trigger::EVENT_INSERT:
+                $trigger = 'INSERT IGNORE INTO %s (%s) VALUES (NEW.%s);';
+                break;
+
             case Trigger::EVENT_UPDATE:
-                return sprintf(
-                    "INSERT IGNORE INTO %s (%s) VALUES (NEW.%s);",
-                    $this->connection->quoteIdentifier($this->resource->getTableName($changelog->getName())),
-                    $this->connection->quoteIdentifier($changelog->getColumnName()),
-                    $this->connection->quoteIdentifier($this->getColumnName())
-                );
+                $trigger = 'INSERT IGNORE INTO %s (%s) VALUES (NEW.%s);';
+
+                if ($this->connection->isTableExists($this->getTableName())
+                    && $describe = $this->connection->describeTable($this->getTableName())
+                ) {
+                    $columnNames = array_column($describe, 'COLUMN_NAME');
+                    $columnNames = array_diff($columnNames, $this->ignoredUpdateColumns);
+                    if ($columnNames) {
+                        $columns = [];
+                        foreach ($columnNames as $columnName) {
+                            $columns[] = sprintf(
+                                'NEW.%1$s != OLD.%1$s',
+                                $this->connection->quoteIdentifier($columnName)
+                            );
+                        }
+                        $trigger = sprintf(
+                            "IF (%s) THEN %s END IF;",
+                            implode(' OR ', $columns),
+                            $trigger
+                        );
+                    }
+                }
+                break;
 
             case Trigger::EVENT_DELETE:
-                return sprintf(
-                    "INSERT IGNORE INTO %s (%s) VALUES (OLD.%s);",
-                    $this->connection->quoteIdentifier($this->resource->getTableName($changelog->getName())),
-                    $this->connection->quoteIdentifier($changelog->getColumnName()),
-                    $this->connection->quoteIdentifier($this->getColumnName())
-                );
+                $trigger = 'INSERT IGNORE INTO %s (%s) VALUES (OLD.%s);';
+                break;
 
             default:
                 return '';
         }
+
+        return sprintf(
+            $trigger,
+            $this->connection->quoteIdentifier($this->resource->getTableName($changelog->getName())),
+            $this->connection->quoteIdentifier($changelog->getColumnName()),
+            $this->connection->quoteIdentifier($this->getColumnName())
+        );
     }
 
     /**
@@ -225,7 +264,7 @@ private function getAfterEventTriggerName($event)
     /**
      * Retrieve View related to subscription
      *
-     * @return \Magento\Framework\Mview\ViewInterface
+     * @return ViewInterface
      * @codeCoverageIgnore
      */
     public function getView()
diff --git a/lib/internal/Magento/Framework/Mview/etc/mview.xsd b/lib/internal/Magento/Framework/Mview/etc/mview.xsd
@@ -106,7 +106,7 @@
     <xs:simpleType name="subscriptionModelType">
         <xs:annotation>
             <xs:documentation>
-                Subscription model must be a valid PHP class or interface name.
+                DEPRECATED. Subscription model must be a valid PHP class or interface name.
             </xs:documentation>
         </xs:annotation>
         <xs:restriction base="xs:string">
