Skip to content

#21853: Allow mview indexers to use different entity columns. #21857

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Jan 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
891198c
#21853: Allow mview indexers to use different entity columns.
Mar 20, 2019
7da4940
Merge branch '2.4-develop' into 21853
slavvka Jan 24, 2020
b59fc5a
#21853: Simplify the code change.
Apr 12, 2020
4035ec0
#21853: Correct the comment.
Apr 12, 2020
320ced7
21853: Send the view object and do all processing in buildStatement.
Apr 14, 2020
0d1ac3a
Merge branch '2.4-develop' into 21853
lenaorobei Apr 14, 2020
6cbf85f
Merge branch '2.4-develop' into 21853
lenaorobei Aug 25, 2020
7c2d2a6
#21853: Fix tests.
Aug 29, 2020
8388a90
#21853: Fix tests.
Aug 31, 2020
3f67ed8
Merge branch '2.4-develop' into 21853
VladimirZaets Sep 11, 2020
873b814
Merge branch '2.4-develop' into 21853
engcom-Foxtrot Sep 25, 2020
392dbf6
Merge branch '2.4-develop-mainline' into 21853
engcom-Foxtrot Sep 25, 2020
ff56be5
magento/magento2#21853: Allow mview indexers to use different entity …
engcom-Foxtrot Sep 28, 2020
e16f2c2
Merge branch '2.4-develop-mainline' into 21853
engcom-Foxtrot Oct 1, 2020
922e029
Merge branch '2.4-develop' into 21853
engcom-Charlie Nov 12, 2020
330d595
Merge branch '2.4-develop' into 21853
engcom-Charlie Nov 19, 2020
4c1094a
extracted getting column to separate method
engcom-Charlie Nov 25, 2020
69cee97
Merge branch '2.4-develop' into 21853
engcom-Charlie Nov 25, 2020
46429a4
Merge branch '2.4-develop' into 21853
engcom-Charlie Nov 30, 2020
4ad7aae
fix unit test
engcom-Charlie Nov 30, 2020
8a9b768
define arguments & return types
engcom-Charlie Nov 30, 2020
db4cf23
Merge branch '2.4-develop' into 21853
engcom-Charlie Dec 1, 2020
094decc
Merge branch '2.4-develop' into 21853
engcom-Charlie Dec 28, 2020
910eb41
fixing conflict merge; fixed unit test
engcom-Charlie Dec 28, 2020
be3f84b
Merge branch '2.4-develop' into 21853
Jan 11, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public function testCreate()
->method('getColumnName')
->willReturn('entity_id');

$this->viewMock->expects($this->exactly(3))
$this->viewMock->expects($this->atLeastOnce())
->method('getChangelog')
->willReturn($changelogMock);

Expand Down Expand Up @@ -243,20 +243,23 @@ public function testCreate()
$otherViewMock->expects($this->exactly(1))
->method('getId')
->willReturn('other_id');
$otherViewMock->expects($this->exactly(1))
$otherViewMock->expects($this->exactly(4))
->method('getSubscriptions')
->willReturn([['name' => $this->tableName], ['name' => 'otherTableName']]);
$otherViewMock->expects($this->exactly(3))
->willReturn(
[
$this->tableName => ['name' => $this->tableName, 'column' => 'columnName'],
'otherTableName' => ['name' => 'otherTableName', 'column' => 'columnName']
]
);
$otherViewMock->expects($this->atLeastOnce())
->method('getChangelog')
->willReturn($otherChangelogMock);

$this->viewMock->expects($this->any())
->method('getId')
->willReturn('this_id');
$this->viewMock->expects($this->never())
->method('getSubscriptions');

$this->viewCollectionMock->expects($this->exactly(1))
$this->viewCollectionMock->expects($this->once())
->method('getViewsByStateMode')
->with(StateInterface::MODE_ENABLED)
->willReturn([$this->viewMock, $otherViewMock]);
Expand All @@ -268,6 +271,16 @@ public function testCreate()
$this->connectionMock->expects($this->exactly(3))
->method('createTrigger')
->with($triggerMock);

$this->viewMock->expects($this->exactly(3))
->method('getSubscriptions')
->willReturn(
[
$this->tableName => ['name' => $this->tableName, 'column' => 'columnName'],
'otherTableName' => ['name' => 'otherTableName', 'column' => 'columnName']
]
);

$this->model->create();
}

Expand Down Expand Up @@ -319,21 +332,24 @@ public function testRemove()
true,
[]
);
$otherViewMock->expects($this->exactly(1))
$otherViewMock->expects($this->atLeastOnce())
->method('getId')
->willReturn('other_id');
$otherViewMock->expects($this->exactly(1))
->method('getSubscriptions')
->willReturn([['name' => $this->tableName], ['name' => 'otherTableName']]);
$otherViewMock->expects($this->exactly(3))
$otherViewMock->expects($this->atLeastOnce())
->method('getChangelog')
->willReturn($otherChangelogMock);

$this->viewMock->expects($this->any())
$this->viewMock->expects($this->atLeastOnce())
->method('getId')
->willReturn('this_id');
$this->viewMock->expects($this->never())
->method('getSubscriptions');
$otherViewMock->expects($this->atLeastOnce())
->method('getSubscriptions')
->willReturn(
[
$this->tableName => ['name' => $this->tableName, 'column' => 'columnName'],
'otherTableName' => ['name' => 'otherTableName', 'column' => 'columnName']
]
);

$this->viewCollectionMock->expects($this->exactly(1))
->method('getViewsByStateMode')
Expand Down Expand Up @@ -411,6 +427,18 @@ public function testBuildStatementIgnoredColumnSubscriptionLevel(): void
->method('getColumnName')
->willReturn('entity_id');

$this->viewMock->expects($this->once())
->method('getSubscriptions')
->willReturn(
[
$this->tableName => ['name' => $this->tableName, 'column' => 'columnName'],
'cataloginventory_stock_item' => ['name' => 'otherTableName', 'column' => 'columnName']
]
);
$this->viewMock->expects($this->atLeastOnce())
->method('getChangeLog')
->willReturn($otherChangelogMock);

$model = new Subscription(
$this->resourceMock,
$this->triggerFactoryMock,
Expand All @@ -425,7 +453,7 @@ public function testBuildStatementIgnoredColumnSubscriptionLevel(): void

$method = new \ReflectionMethod($model, 'buildStatement');
$method->setAccessible(true);
$statement = $method->invoke($model, Trigger::EVENT_UPDATE, $otherChangelogMock);
$statement = $method->invoke($model, Trigger::EVENT_UPDATE, $this->viewMock);

$this->assertStringNotContainsString($ignoredColumnName, $statement);
$this->assertStringContainsString($notIgnoredColumnName, $statement);
Expand Down
88 changes: 56 additions & 32 deletions lib/internal/Magento/Framework/Mview/View/Subscription.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,29 +8,31 @@

use Magento\Framework\App\ObjectManager;
use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DB\Adapter\AdapterInterface;
use Magento\Framework\DB\Ddl\Trigger;
use Magento\Framework\DB\Ddl\TriggerFactory;
use Magento\Framework\Mview\Config;
use Magento\Framework\Mview\View\StateInterface;
use Magento\Framework\Mview\ViewInterface;

/**
* Class Subscription for handling partial indexation triggers
* Mview subscription.
*/
class Subscription implements SubscriptionInterface
{
/**
* Database connection
*
* @var \Magento\Framework\DB\Adapter\AdapterInterface
* @var AdapterInterface
*/
protected $connection;

/**
* @var \Magento\Framework\DB\Ddl\TriggerFactory
* @var TriggerFactory
*/
protected $triggerFactory;

/**
* @var \Magento\Framework\Mview\View\CollectionInterface
* @var CollectionInterface
*/
protected $viewCollection;

Expand Down Expand Up @@ -62,7 +64,7 @@ class Subscription implements SubscriptionInterface
*
* @var array
*/
private $ignoredUpdateColumns = [];
private $ignoredUpdateColumns;

/**
* List of columns that can be updated in a specific subscribed table
Expand All @@ -74,16 +76,17 @@ class Subscription implements SubscriptionInterface
* @var Resource
*/
protected $resource;

/**
* @var Config
*/
private $mviewConfig;

/**
* @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
Expand All @@ -92,9 +95,9 @@ class Subscription implements SubscriptionInterface
*/
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,
$ignoredUpdateColumns = [],
Expand All @@ -114,9 +117,9 @@ public function __construct(
}

/**
* Create subsciption
* Create subscription
*
* @return \Magento\Framework\Mview\View\SubscriptionInterface
* @return SubscriptionInterface
*/
public function create()
{
Expand All @@ -129,12 +132,12 @@ public function create()
->setEvent($event)
->setTable($this->resource->getTableName($this->tableName));

$trigger->addStatement($this->buildStatement($event, $this->getView()->getChangelog()));
$trigger->addStatement($this->buildStatement($event, $this->getView()));

// Add statements for linked views
foreach ($this->getLinkedViews() as $view) {
/** @var \Magento\Framework\Mview\ViewInterface $view */
$trigger->addStatement($this->buildStatement($event, $view->getChangelog()));
/** @var ViewInterface $view */
$trigger->addStatement($this->buildStatement($event, $view));
}

$this->connection->dropTrigger($trigger->getName());
Expand All @@ -147,7 +150,7 @@ public function create()
/**
* Remove subscription
*
* @return \Magento\Framework\Mview\View\SubscriptionInterface
* @return SubscriptionInterface
*/
public function remove()
{
Expand All @@ -162,8 +165,8 @@ public function remove()

// Add statements for linked views
foreach ($this->getLinkedViews() as $view) {
/** @var \Magento\Framework\Mview\ViewInterface $view */
$trigger->addStatement($this->buildStatement($event, $view->getChangelog()));
/** @var ViewInterface $view */
$trigger->addStatement($this->buildStatement($event, $view));
}

$this->connection->dropTrigger($trigger->getName());
Expand All @@ -188,7 +191,7 @@ protected function getLinkedViews()
$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;
Expand All @@ -208,21 +211,22 @@ protected function getLinkedViews()
/**
* Prepare columns for trigger statement. Should be protected in order to serve new approach
*
* @param ChangelogInterface $changelog
* @param ViewInterface $view
* @param string $event
* @return array
* @throws \Exception
*/
protected function prepareColumns(ChangelogInterface $changelog, string $event): array
protected function prepareColumns(ViewInterface $view, string $event): array
{
$changelog = $view->getChangelog();
$prefix = $event === Trigger::EVENT_DELETE ? 'OLD.' : 'NEW.';
$subscriptionData = $this->mviewConfig->getView($changelog->getViewId())['subscriptions'][$this->getTableName()];
$columns = [
'column_names' => [
'entity_id' => $this->connection->quoteIdentifier($changelog->getColumnName())
],
'column_values' => [
'entity_id' => $this->getEntityColumn($prefix)
'entity_id' => $this->getEntityColumn($prefix, $view)
]
];

Expand All @@ -241,12 +245,14 @@ protected function prepareColumns(ChangelogInterface $changelog, string $event):
* Build trigger statement for INSERT, UPDATE, DELETE events
*
* @param string $event
* @param \Magento\Framework\Mview\View\ChangelogInterface $changelog
* @param ViewInterface $view
* @return string
*/
protected function buildStatement($event, $changelog)
protected function buildStatement(string $event, ViewInterface $view): string
{
$trigger = "%sINSERT IGNORE INTO %s (%s) VALUES (%s);";
$changelog = $view->getChangelog();

switch ($event) {
case Trigger::EVENT_UPDATE:
$tableName = $this->resource->getTableName($this->getTableName());
Expand Down Expand Up @@ -279,13 +285,14 @@ protected function buildStatement($event, $changelog)
}
break;
}
$columns = $this->prepareColumns($changelog, $event);
$columns = $this->prepareColumns($view, $event);

return sprintf(
$trigger,
$this->getProcessor()->getPreStatements(),
$this->connection->quoteIdentifier($this->resource->getTableName($changelog->getName())),
implode(", " , $columns['column_names']),
implode(", ", $columns['column_values'])
implode(', ', $columns['column_names']),
implode(', ', $columns['column_values'])
);
}

Expand All @@ -312,11 +319,28 @@ private function getProcessor(): AdditionalColumnProcessorInterface

/**
* @param string $prefix
* @param ViewInterface $view
* @return string
*/
public function getEntityColumn(string $prefix, ViewInterface $view): string
{
return $prefix . $this->connection->quoteIdentifier($this->getSubscriptionColumn($view));
}

/**
* Returns subscription column name by view
*
* @param ViewInterface $view
* @return string
*/
public function getEntityColumn(string $prefix): string
private function getSubscriptionColumn(ViewInterface $view): string
{
return $prefix . $this->connection->quoteIdentifier($this->getColumnName());
$subscriptions = $view->getSubscriptions();
if (!isset($subscriptions[$this->getTableName()]['column'])) {
throw new \RuntimeException(sprintf('Column name for view with id "%s" doesn\'t exist', $view->getId()));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably should catch this exception somewhere, as it is I guess it would return an error to the user and not useful message, we should add a proper error message for the user (wherever we catch this exception), and probably log a more descriptive error on logs.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}

return $subscriptions[$this->getTableName()]['column'];
}

/**
Expand All @@ -338,7 +362,7 @@ private function getAfterEventTriggerName($event)
/**
* Retrieve View related to subscription
*
* @return \Magento\Framework\Mview\ViewInterface
* @return ViewInterface
* @codeCoverageIgnore
*/
public function getView()
Expand Down