Skip to content
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

Indexation improvement #141

Merged
merged 16 commits into from
Nov 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
77 changes: 0 additions & 77 deletions app/code/Magento/Inventory/Indexer/ActiveTableSwitcher.php

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Inventory\Indexer\StockItem;
namespace Magento\Inventory\Indexer;

use Magento\Framework\App\ResourceConnection;
use Magento\Inventory\Model\ResourceModel\Source as SourceResourceModel;
Expand Down Expand Up @@ -33,13 +34,49 @@ public function __construct(ResourceConnection $resourceConnection)
}

/**
* Returns all data for the index to
* Returns all data for the index.
*
* @param int $stockId
* @return \ArrayIterator
*/
public function getData(int $stockId): \ArrayIterator
{
$connection = $this->resourceConnection->getConnection();
$select = $this->prepareSelect($stockId);

return new \ArrayIterator($connection->fetchAll($select));
}

/**
* Returns all data for the index by SKU List condition.
*
* @param int $stockId
* @param array $skuList
* @return \ArrayIterator
*/
public function getData(int $stockId, array $skuList = []): \ArrayIterator
public function getDataBySkuList(int $stockId, array $skuList): \ArrayIterator
{
$connection = $this->resourceConnection->getConnection();
$conditions = [];
if (count($skuList)) {
$conditions[] = [
'condition' => 'source_item.' . SourceItemInterface::SKU . ' IN (?)',
'value' => $skuList
];
}
$select = $this->prepareSelect($stockId, $conditions);

return new \ArrayIterator($connection->fetchAll($select));
}

/**
* Prepare select.
*
* @param int $stockId
* @param array $conditions
* @return \ArrayIterator|\Magento\Framework\DB\Select
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be much more "defensive" to return just one of the types

*/
private function prepareSelect($stockId, array $conditions = [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add type hints to the method declaration

{
$connection = $this->resourceConnection->getConnection();
$sourceTable = $this->resourceConnection->getTableName(SourceResourceModel::TABLE_NAME_SOURCE);
Expand Down Expand Up @@ -76,12 +113,14 @@ public function getData(int $stockId, array $skuList = []): \ArrayIterator
->where('stock_source_link.' . StockSourceLink::STOCK_ID . ' = ?', $stockId)
->where('stock_source_link.' . StockSourceLink::SOURCE_ID . ' IN (?)', $sourceIds);

if (count($skuList) !== 0) {
$select->where('source_item.' . SourceItemInterface::SKU . ' IN (?)', $skuList);
if (!empty($conditions)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

!empty seems to be redundant here

foreach ($conditions as $condition) {
$select->where($condition['condition'], $condition['value']);
}
}

$select->group([SourceItemInterface::SKU]);

return new \ArrayIterator($connection->fetchAll($select));
return $select;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,12 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Inventory\Indexer\StockItem;
namespace Magento\Inventory\Indexer;

use Magento\Framework\App\ResourceConnection;
use Magento\Framework\Indexer\SaveHandler\Batch;
use Magento\Inventory\Indexer\IndexHandlerInterface;
use Magento\Inventory\Indexer\IndexName;
use Magento\Inventory\Indexer\IndexNameResolverInterface;

/**
* Index handler is responsible for index data manipulation
Expand Down Expand Up @@ -76,6 +74,6 @@ public function cleanIndex(IndexName $indexName, \Traversable $documents, string
{
$connection = $this->resourceConnection->getConnection($connectionName);
$tableName = $this->indexNameResolver->resolveName($indexName);
$connection->delete($tableName, ['sku in (?)' => iterator_to_array($documents)]);
$connection->delete($tableName, ['sku IN (?)' => iterator_to_array($documents)]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,39 +3,37 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Inventory\Indexer\StockItem;
declare(strict_types=1);

namespace Magento\Inventory\Indexer;

use Magento\Framework\Search\Request\IndexScopeResolverInterface;
use Magento\Inventory\Indexer\ActiveTableSwitcher;
use Magento\Inventory\Indexer\Alias;
use Magento\Inventory\Indexer\IndexName;
use Magento\Inventory\Indexer\IndexNameResolverInterface;

/**
* @inheritdoc
*/
class IndexNameResolver implements IndexNameResolverInterface
{
/**
* @var IndexScopeResolverInterface
* TODO: move to separate configurable interface (https://github.com/magento-engcom/msi/issues/213)
* Suffix for replica index table
*
* @var string
*/
private $indexScopeResolver;
private $additionalTableSuffix = '_replica';

/**
* @var ActiveTableSwitcher
* @var IndexScopeResolverInterface
*/
private $activeTableSwitcher;
private $indexScopeResolver;

/**
* @param IndexScopeResolverInterface $indexScopeResolver
* @param ActiveTableSwitcher $activeTableSwitcher
*/
public function __construct(
IndexScopeResolverInterface $indexScopeResolver,
ActiveTableSwitcher $activeTableSwitcher
IndexScopeResolverInterface $indexScopeResolver
) {
$this->indexScopeResolver = $indexScopeResolver;
$this->activeTableSwitcher = $activeTableSwitcher;
}

/**
Expand All @@ -46,8 +44,18 @@ public function resolveName(IndexName $indexName): string
$tableName = $this->indexScopeResolver->resolve($indexName->getIndexId(), $indexName->getDimensions());

if ($indexName->getAlias()->getValue() === Alias::ALIAS_REPLICA) {
$tableName = $this->activeTableSwitcher->getAdditionalTableName($tableName);
$tableName = $this->getAdditionalTableName($tableName);
}
return $tableName;
}

/**
* TODO: move to separate configurable interface (https://github.com/magento-engcom/msi/issues/213)
* @param string $tableName
* @return string
*/
public function getAdditionalTableName(string $tableName): string
{
return $tableName . $this->additionalTableSuffix;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,13 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Inventory\Indexer\StockItem;
namespace Magento\Inventory\Indexer;

use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DB\Ddl\Table;
use Magento\Framework\Exception\StateException;
use Magento\Inventory\Indexer\IndexName;
use Magento\Inventory\Indexer\IndexNameResolverInterface;
use Magento\Inventory\Indexer\IndexStructureInterface;

/**
* @inheritdoc
Expand Down
102 changes: 102 additions & 0 deletions app/code/Magento/Inventory/Indexer/IndexTableSwitcher.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Inventory\Indexer;

use Magento\Framework\App\ResourceConnection;
use Magento\Framework\DB\Adapter\AdapterInterface;

/**
* @inheritdoc
*/
class IndexTableSwitcher implements IndexTableSwitcherInterface
{
/**
* TODO: move to separate configurable interface (https://github.com/magento-engcom/msi/issues/213)
* Suffix for replica index table
*
* @var string
*/
private $additionalTableSuffix = '_replica';

/**
* TODO: move to separate configurable interface (https://github.com/magento-engcom/msi/issues/213)
* Suffix for outdated index table
*
* @var string
*/
private $outdatedTableSuffix = '_outdated';

/**
* @var ResourceConnection
*/
private $resourceConnection;

/**
* @var IndexNameResolverInterface
*/
private $indexNameResolver;

/**
* @param ResourceConnection $resourceConnection
* @param IndexNameResolverInterface $indexNameResolver
*/
public function __construct(
ResourceConnection $resourceConnection,
IndexNameResolverInterface $indexNameResolver
) {
$this->resourceConnection = $resourceConnection;
$this->indexNameResolver = $indexNameResolver;
}

/**
* @inheritdoc
*/
public function switch(IndexName $indexName, string $connectionName)
{
$connection = $this->resourceConnection->getConnection($connectionName);
$tableName = $this->indexNameResolver->resolveName($indexName);

$this->switchTable($connection, [$tableName]);
}

/**
* Switch index tables from replica to active.
*
* @param AdapterInterface $connection
* @param array $tableNames
* @return void
*/
private function switchTable(AdapterInterface $connection, array $tableNames)
{
$toRename = [];
foreach ($tableNames as $tableName) {
$outdatedTableName = $tableName . $this->outdatedTableSuffix;
$replicaTableName = $tableName . $this->additionalTableSuffix;

$renameBatch = [
[
'oldName' => $tableName,
'newName' => $outdatedTableName,
],
[
'oldName' => $replicaTableName,
'newName' => $tableName,
],
[
'oldName' => $outdatedTableName,
'newName' => $replicaTableName,
]
];
$toRename = array_merge($toRename, $renameBatch);
}

if (!empty($toRename)) {
$connection->renameTablesBatch($toRename);
}
}
}
Loading