WIP : Msi stock indexing #48 #49

larsroettig · 2017-07-29T14:59:18Z

No description provided.

larsroettig · 2017-07-30T21:33:59Z

@maghamed hi it is only basic version I think must also implement a bunching but the code is working some can be optimized. Have you a good example how can I test it?

maghamed

For Big Merchants, who have a lot of SKUs and a lot of Sales channels (scopes in which products would be sold). We want to prevent a multiplication of data in the index table :
SKUs Qty * Number of Sales Channels

Because that could introduce performance impact both on Read and Write index operations.

That's why it's proposed to split indexes by the Scope.
Taking into account that we have 1 - 1 relation between Scope (Sales Channel) and Stock.
We could Build independent index table per each Stock (Stock_id).

Stock Id could be used as Dimension which will help us to resolve correct index we need to refer to get Product Qty.

For example, having 5 websites with ids (1 .. 5)
we will end up creating 5 different index tables (with the same structure):

inventory_stock_item_stock_1
inventory_stock_item_stock_2
inventory_stock_item_stock_3
inventory_stock_item_stock_4
inventory_stock_item_stock_5

You can refer for similar implementation in the CatalogSearch Index
Magento\CatalogSearch\Model\Indexer\Fulltext
Also, we have a requirement to have index aliasing during the full reindex happened, so we need to have switchable indexes to make Front-End responsive while Full Reindex happen.

Also, please take into account that when the SourceItem data change -> we need to upgrade Stock Item Indexes for each of the Stock to which given Source was assigned.
If Source was assigned to 3 stocks, for example, we need to refresh data in the 3 StockItem indexes

Looks like, for this operation we need to introduce new service
Magento\InventoryApi\Api\GetAssignedStocksForSourceInterface::execute($sourceId);

maghamed · 2017-08-01T13:06:21Z

app/code/Magento/Inventory/Model/Indexer/StockItem/AbstractAction.php

+use \Psr\Log\LoggerInterface;
+use \Magento\Framework\App\ResourceConnection;
+
+abstract class AbstractAction


Please don't introduce AbstractAction class, as Magento doesn't recommend to use Inheritance in newly created code.

Create a dedicated indexer model which would be injected via DI into each of the class:

Row Rows Full

and make each of these classes to proxy call to that indexer model

maghamed · 2017-08-01T13:25:13Z

app/code/Magento/Inventory/Model/Indexer/StockItem/AbstractAction.php

+    public function __construct(ResourceConnection $resource, LoggerInterface $logger)
+    {
+        $this->resource = $resource;
+        $this->connection = $resource->getConnection();


it's not recommended to make anything except assignment in the constructors.
Like in your case where you do
$this->connection = $resource->getConnection();

maghamed · 2017-08-01T13:43:51Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+/**
+ * Stock item Indexer @todo
+ */
+class StockItem implements \Magento\Framework\Indexer\ActionInterface, \Magento\Framework\Mview\ActionInterface


It's wrong to Make indexer implementing both of these interfaces, because these interfaces responsible for different things. Doing so we violate Single Responsibility Principle.

We need to have two different classes, each one implementing own interface.

Like in CatalogSearch module
https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogSearch/Model/Indexer/Mview/Action.php
implements \Magento\Framework\Mview\ActionInterface
which is declared in - https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogSearch/etc/mview.xml#L9

and Magento\CatalogSearch\Model\Indexer\Fulltext implements only \Magento\Framework\Indexer\ActionInterface
which is declared in
https://github.com/magento/magento2/blob/develop/app/code/Magento/CatalogSearch/etc/indexer.xml#L9

Let's do the same way

maghamed · 2017-08-01T13:58:14Z

app/code/Magento/Inventory/Model/Indexer/StockItem/AbstractAction.php

+            $indexItems = $this->fetchIndexItems($sourceItems);
+            if (is_array($indexItems) && count($indexItems) > 0) {
+                $this->cleanIndexTable($sourceItems);
+                $this->updateIndexTable($indexItems);


It's better to introduce implementation of IndexerInterface
https://github.com/magento/magento2/blob/develop/lib/internal/Magento/Framework/Indexer/SaveHandler/IndexerInterface.php

and use it for the operations of clean and delete index

maghamed · 2017-08-01T14:49:09Z

app/code/Magento/Inventory/Setup/Operation/CreateStockItemIndexTable.php

+
+use Magento\InventoryApi\Api\Data\StockItemInterface;
+
+class CreateStockItemIndexTable


This should be implemented similarly to what we have in CatalogSearch
where
Magento\CatalogSearch\Model\Indexer\IndexStructure
responsible for creating StockItem index structure.

IndexStructure implements interface
Magento\Framework\Indexer\IndexStructureInterface

we will reuse this class for index structure creation because we will create dedicated index table for each particular Stock (stock_id). To make the system being scalable horizontally.
As each index table will hold StockItems for each particular Scope (SalesChannel).

For example, having 5 websites each of which represents Sales Channel we will end up with 5 index tables:

inventory_stock_item_index_1 - for website_id 1 inventory_stock_item_index_2 - for website_id 2 inventory_stock_item_index_3 - for website_id 3 inventory_stock_item_index_4 - for website_id 4 inventory_stock_item_index_5 - for website_id 5

maghamed · 2017-08-01T15:08:08Z

app/code/Magento/Inventory/Model/Indexer/StockItem/Action/Rows.php

+        try {
+            $this->reindexRows($ids);
+        } catch (\Exception $e) {
+            throw new \Magento\Framework\Exception\LocalizedException(__($e->getMessage()), $e);


it's not correct translation,
__($e->getMessage()), $e);
Because \Exception $e ,so it's not a successor of Localized Exception, thus $e->getMessage()
could return a string for which you don't have precise translation

maghamed · 2017-08-01T15:08:29Z

app/code/Magento/Inventory/Model/Indexer/StockItem/Action/Row.php

+        try {
+            $this->reindexRows([$id]);
+        } catch (\Exception $e) {
+            throw new \Magento\Framework\Exception\LocalizedException(__($e->getMessage()), $e);


it's not correct translation,
__($e->getMessage()), $e);
Because \Exception $e ,so it's not a successor of Localized Exception, thus $e->getMessage()
could return a string for which you don't have precise translation

maghamed · 2017-08-01T15:10:09Z

app/code/Magento/Inventory/Model/Indexer/StockItem/Action/Row.php

+     *
+     * @return void
+     */
+    public function execute($id = null)


why do we allow to accept NULL value if we throw an exception if NULL passed as a parameter?

maghamed · 2017-08-01T15:15:39Z

app/code/Magento/InventoryApi/Api/Data/StockItemInterface.php

+ * Interface StockItemInterface
+ * @api
+ */
+interface StockItemInterface extends ExtensibleDataInterface


For now we don't have a Service API to retrieve Stock Items for a given Stock_ID

That's why for now we can't get StockItems in any case
These API should be introduced to InventoryApi\Api\StockItemManagerInterface

maghamed · 2017-08-01T15:16:45Z

app/code/Magento/Inventory/etc/indexer.xml

+<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Indexer/etc/indexer.xsd">
+    <indexer id="inventory_stock_item" view_id="inventory_stock_item" class="Magento\Inventory\Model\Indexer\StockItem">
+        <title translate="true">StockItem</title>
+        <description translate="true">Index stock item</description>


Please provide more clear description

maghamed

In general everything good, @larsroettig please keep going

maghamed · 2017-08-08T15:34:32Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+     * @var string
+     */
+    const DATA_KEY_INDEXER_ID = 'indexer_id';
+    const DATA_KEY_INDEXER_SUFFIX = 'indexer_suffix';


no need to make these fields as constants, because constants are the contract which could be accessible outside of the class (like public methods)

it's better to make them private static in this case, so, no one from outside could rely on these values.
And we will not duplicate these data for all instantiated classes.

maghamed · 2017-08-08T15:45:21Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+     * @param int $batchSize #
+     */
+    public function __construct(
+        IndexStructure $indexStructure,


it's better to use IndexStructureinterface here and specify in DI Type what exact implementation should use here

maghamed · 2017-08-08T15:48:29Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+        Batch $batch,
+        ResourceConnection $resource,
+        array $data,
+        $batchSize = 100


use DI Type configuration for batchSize, especially taking into account that you will use Type for IndexStructure

maghamed · 2017-08-08T15:53:58Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+    /**
+     * Indexer ID in configuration
+     */
+    const INDEXER_ID = 'inventory_stock_item_index';


hm... it's better to keep this constant away from the concrete implementation, but for now I don't see a place where we could move it

maghamed · 2017-08-08T15:56:40Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+    private function getIndexSuffix()
+    {
+        if (isset($this->data[self::DATA_KEY_INDEXER_SUFFIX])) {
+            return $this->data[self::DATA_KEY_INDEXER_SUFFIX];


how can we set these data?

maghamed · 2017-08-08T15:59:39Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+/**
+ * Stock item Indexer @todo
+ */
+class StockItem implements \Magento\Framework\Indexer\ActionInterface


let's introduce

interface StockItemIndexerInterface extends \Magento\Framework\Indexer\ActionInterface { /** * Indexer ID in configuration */ const INDEXER_ID = 'inventory_stock_item_index'; }

and make this class to implement this interface.

But Ideally to have getName method in ActionInterface which returned value initialized from the configuration, which we can't add because of BackwardCompatibility

maghamed · 2017-08-08T16:02:31Z

app/code/Magento/Inventory/Model/StockItemManager.php

+ *
+ * @codeCoverageIgnore
+ */
+class StockItemManager extends AbstractExtensibleModel implements StockItemManagerInterface


why do we have StockItemManager which behaves like DataInterface,

Manager means to make some actions over some data.

maghamed · 2017-08-08T16:05:29Z

app/code/Magento/InventoryApi/Api/Data/StockItemManagerInterface.php

+ * Interface StockItemManagerInterface
+ * @api
+ */
+interface StockItemManagerInterface extends ExtensibleDataInterface


why do we need this DataInterface?
it duplicated StockItemInterface, but called manager and never used

looks like a mistake

naydav · 2017-08-08T15:24:23Z

app/code/Magento/Inventory/Model/GetAssignedStocksForSource.php

+    private function getAssignedStockIds($sourceId)
+    {
+        $searchCriteria = $this->searchCriteriaBuilder
+            ->addFilter(StockSourceLink::SOURCE_ID, (int)$sourceId)


Cast to int is extra

naydav · 2017-08-08T15:24:39Z

app/code/Magento/Inventory/Model/GetAssignedStocksForSource.php

+    /**
+     * Get all linked StockIds by given sourceId.
+     *
+     * @param $sourceId


Missed parameter type

naydav · 2017-08-08T15:25:15Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+use Magento\Inventory\Model\Indexer\StockItem\IndexStructure;
+
+/**
+ * Stock item Indexer @todo


Pls delete or add more description to @todo annotation

naydav · 2017-08-08T15:26:02Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+        Batch $batch,
+        ResourceConnection $resource,
+        array $data,
+        $batchSize = 100


Move default value to di configuration

naydav · 2017-08-08T15:27:41Z

app/code/Magento/Inventory/Model/Indexer/StockItem/IndexHandler.php

+    /**
+     * @var array
+     */
+    private $data;


Need to more clear name?
What is mean data?

naydav · 2017-08-08T15:31:02Z

app/code/Magento/Inventory/Model/StockItemManager.php

+ *
+ * @codeCoverageIgnore
+ */
+class StockItemManager extends AbstractExtensibleModel implements StockItemManagerInterface


As for me *Manager is not good name for "entity"
Maybe it is better simple "StockItem"

But pay attention we already have \Magento\InventoryApi\Api\Data\StockItemInterface

naydav · 2017-08-08T15:34:31Z

app/code/Magento/InventoryApi/Api/Data/StockItemManagerInterface.php

+    public function setStockId($stockId);
+
+    /**
+     * Get stock item sku


it is not stock item sku

naydav · 2017-08-08T15:36:10Z

app/code/Magento/InventoryApi/Api/GetAssignedStocksForSourceInterface.php

+interface GetAssignedStocksForSourceInterface
+{
+    /**
+     * Get Stock assigned to Sources


visa versa
Get Stocks assigned to Source

naydav · 2017-08-08T15:42:30Z

app/code/Magento/Inventory/etc/mview.xml

+<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Mview/etc/mview.xsd">
+    <view id="inventory_stock_item" class="Magento\Inventory\Model\Indexer\Mview\StockItemAction" group="indexer">
+        <subscriptions>
+            <table name="inventory_source_item" entity_column="source_id" />


Do we need to process disabling/enabling of products?

ishakhsuvarov · 2017-08-14T15:16:42Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+    /**
+     * StockItem constructor.
+     * @param DimensionFactory $dimensionFactory
+     * @param StoreManagerInterface $storeManager


Please update doc block.

ishakhsuvarov · 2017-08-14T15:18:42Z

app/code/Magento/Inventory/Model/Indexer/StockItem/DataProvider.php

+{
+
+    const TABLE_NAME_SOURCE_ITEM = 'inventory_source_item';
+    const TABLE_NAME_STOCK_SOURCE_LINK = 'inventory_source_stock_link';


Please describe constants.
More info is available in the devdocs

ishakhsuvarov · 2017-08-14T15:19:46Z

app/code/Magento/Inventory/Model/Indexer/StockItem/DataProvider.php

+     * @param \Magento\Framework\DB\Adapter\AdapterInterface $connection
+     * @return string
+     */
+    private function getSourceItemTableName($connection)


Please add a type hint

ishakhsuvarov · 2017-08-14T15:20:03Z

app/code/Magento/Inventory/Model/Indexer/StockItem/DataProvider.php

+     * @param \Magento\Framework\DB\Adapter\AdapterInterface $connection
+     * @return string
+     */
+    private function getLinkTableName($connection)


Please add a type hint

maghamed · 2017-08-15T11:41:50Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+ * See COPYING.txt for license details.
+ */
+
+namespace Magento\Inventory\Model\Indexer;


It's better to have a separate namespace for Indexer. Not to put all indexers to
Magento\Inventory\Model\*

Let's create dedicated namespace like

Magento\Inventory\Indexer\

maghamed · 2017-08-15T12:21:44Z

app/code/Magento/Inventory/Model/Indexer/StockItem.php

+        $storeIds = array_keys($this->storeManager->getStores());
+
+        foreach ($storeIds as $storeId) {
+            $dimension = [$this->dimensionFactory->create(['name' => 'scope', 'value' => $storeId])];


In the documentation https://github.com/magento-engcom/magento2/wiki/New-Indexer-for-StockItems#index-dimensions stated

That's why it's proposed to split indexes by the Scope. Taking into account that we have 1 - 1 relation between Scope (Sales Channel) and Stock
For example, having 5 websites with ids (1 .. 5) we will end up creating 5 different index tables (with the same structure):

inventory_stock_item_stock_1 inventory_stock_item_stock_2 inventory_stock_item_stock_3 inventory_stock_item_stock_4 inventory_stock_item_stock_5

Current code where dimension is StoreId is copy-pasted from Search Indexer

maghamed · 2017-08-15T12:46:02Z

app/code/Magento/Inventory/Model/Indexer/StockItem/Dimension.php

+/**
+ * Stock Item Dimension
+ */
+class Dimension extends \Magento\Framework\Search\AbstractKeyValuePair


we already have
\Magento\Framework\Search\Request\Dimension
why do we create own Dimension object?

naydav · 2017-08-21T11:49:36Z

app/code/Magento/Inventory/Indexer/Scope/ScopeProxy.php

+ * depending on current scope state
+ * @todo refactoring it copy from catalog search module
+ */
+class ScopeProxy implements \Magento\Framework\Search\Request\IndexScopeResolverInterface


Pls provide more clear class name
class name is proxy, class interface is resolver, due to code it is factory... it is not clear what is responsibility of this class

naydav · 2017-08-21T11:52:18Z

app/code/Magento/Inventory/Indexer/Scope/ScopeProxy.php

+        if (!array_key_exists($state, $this->states)) {
+            throw new UnknownStateException(__("Requested resolver for unknown indexer state: $state"));
+        }
+        return $this->objectManager->create($this->states[$state]);


Do we need to create a new object each time?

naydav · 2017-08-21T11:56:18Z

app/code/Magento/Inventory/Indexer/Scope/ScopeProxy.php

+    private function create($state)
+    {
+        if (!array_key_exists($state, $this->states)) {
+            throw new UnknownStateException(__("Requested resolver for unknown indexer state: $state"));


This logic is better to place in State object. In this way, we don't provide possibility to set the wrong state

But if we look code of State object, we can not find possibility to set the wrong state.
In State objectwWe have only useTemporaryIndex and useRegularIndex methods, thus we can not set state different from these two.
So this checking looks like redundant

agree with @naydav in the scope of UnknownStateException I described the idea of State Machine, so don't think that we need to check the state and have a factory for state creation

naydav · 2017-08-21T12:09:58Z

app/code/Magento/Inventory/Indexer/Scope/TemporaryResolver.php

+ * Resolves name of a temporary table for indexation
+ * @todo refactoring it copy from catalog search module
+ */
+class TemporaryResolver implements \Magento\Framework\Search\Request\IndexScopeResolverInterface


I understand that is taken from search index but I don't like naming (TemporaryResolver, IndexScopeResolver, ScopeProxy)
@maghamed what do you think about this naming?

@naydav makes sense, naming should be aligned to outline the best practices and make it understandable for external developers what's the responsibility of the class reading its name.

naydav · 2017-08-21T12:11:18Z

app/code/Magento/Inventory/Indexer/Scope/UnknownStateException.php

+ * @todo refactoring it copy from catalog search module
+ * @api
+ */
+class UnknownStateException extends LocalizedException


Need to think about moving this exception to framework lib if we will use it

@naydav agree, already pointed that out for IndexTableNotExistException.
These should be framework level exceptions, we can't provide them to client because they provide details of how this functionality is implemented (leaky abstractions).

So, these exceptions are not successors of Localized

Btw, I don't know why we have this Exception in our codebase.
So, from what I see we have a pre-defined finite list of states which could be switched in pre-defined order. (aka State machine design pattern)

our state object initially being initialized in 'REGULAR' state.

after that, we've decided to move it to 'TEMPORARY' state while making re-indexation

after re-indexation complete - we move the object to 'REGULAR' state again.

Is it correct? I suppose so, then why do we have a possibility that State object would be created in a wrong state?
@larsroettig

naydav · 2017-08-21T13:28:10Z

dev/tests/integration/testsuite/Magento/Inventory/Indexer/StockItemTest.php

+
+    /**
+     * @magentoDbIsolation disabled
+     * @magentoDataFixture Magento/Inventory/_files/products.php


Maybe it is better to use one fixture with all preconditions

naydav · 2017-08-21T13:29:09Z

dev/tests/integration/testsuite/Magento/Inventory/_files/products.php

+    ->setStatus(\Magento\Catalog\Model\Product\Attribute\Source\Status::STATUS_ENABLED);
+
+/** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepositoryFactory */
+$productRepository = $objectManager->create(\Magento\Catalog\Api\ProductRepositoryInterface::class);


Why do you create an instance of repository each time?

naydav · 2017-08-21T13:30:03Z

dev/tests/integration/testsuite/Magento/Inventory/_files/products.php

+ * See COPYING.txt for license details.
+ */
+
+\Magento\TestFramework\Helper\Bootstrap::getInstance()->reinitialize();


You can reuse fixture from catalog module

naydav · 2017-08-21T13:31:33Z

dev/tests/integration/testsuite/Magento/Inventory/_files/source.php

@@ -0,0 +1,66 @@
+<?php
+/**


We already have this fixture
app/code/Magento/InventoryApi/Test/_files/source/source_list.php

naydav · 2017-08-21T13:31:50Z

dev/tests/integration/testsuite/Magento/Inventory/_files/stock.php

@@ -0,0 +1,38 @@
+<?php


app/code/Magento/InventoryApi/Test/_files/source/stock_list.php

- Add Setup for index table - Add new Interface"

- Refactoring to new Setup structure - Implement SourceItem add entry to di.xml - WIP Register Basic Indexer SourceItem

- Impelemnt basic import funktion TODOS: - Add some bunching function for index - Add Mysql Index to inventory_stock_item_index table - Documentation and clean up

- Clean Up and implement basic structure

- Impelemnt basic import function TODOS: - Index switch from tmp to idx table - Documentation and clean up

- Add Setup for index table - Add new Interface I magento-engcom/magento#48: - Add Setup for index table - Add new Interface

- Implement switch for Index TODO: - Clean Up and more Doc - Discuss the missing website to stock mapping

- Impelemnt Basic intigration test fixtures

- Bugfix for di.xml - Add basic call for reindex full

- Basic Indexer with switch for tmp and normal index TODO: - Refactoring to remove the state folder from the module - Doc blocks and clean up - Add assertions to the test case

-- fixes after merge

magento-cicd2 · 2017-08-22T14:10:49Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ larsroettig
❌ naydav

maghamed

@naydav please help @larsroettig
and join this story development.

It's very sophisticated Story, and there are no good examples in Magento where re-indexation was implemented totally correct.

So, let's make a good example of introducing New Index to the Magento 2. Outlining all the steps and document them afterwards.

We already agreed that ActiveTableSwitcher would be moved to Framework maybe we need some additional things to move to the framework level?

maghamed · 2017-08-23T14:38:17Z

app/code/Magento/Inventory/Indexer/Scope/IndexTableNotExistException.php

+ * @todo refactoring it copy from catalog search module
+ * @api
+ */
+class IndexTableNotExistException extends LocalizedException


If we are introducing IndexTableNotExistException, this exception should be low-level (framework level).
Not inherited from LocalizedException.
Because we can't provide this Exception o the client code, even its name is a leaky abstraction of the internal details.

maghamed · 2017-08-23T14:43:23Z

app/code/Magento/Inventory/Indexer/Scope/IndexSwitcherInterface.php

@@ -0,0 +1,24 @@
+<?php


In Magento there is another Switcher mechanism - https://github.com/magento-engcom/magento2/blob/develop/app/code/Magento/Catalog/Model/ResourceModel/Indexer/ActiveTableSwitcher.php

by our request, it would be moved to the Framework soon (in the scope of Magento 2.2.0 regression).
So, we could consider using ActiveTableSwitcher for our purposes.

@naydav please consider this and provide a solution for @larsroettig

maybe we need some additional classes to be moved to the Framework?

maghamed · 2017-08-23T15:04:43Z

app/code/Magento/Inventory/Indexer/Scope/TemporaryResolver.php

+ * Resolves name of a temporary table for indexation
+ * @todo refactoring it copy from catalog search module
+ */
+class TemporaryResolver implements \Magento\Framework\Search\Request\IndexScopeResolverInterface


@naydav makes sense, naming should be aligned to outline the best practices and make it understandable for external developers what's the responsibility of the class reading its name.

maghamed · 2017-08-23T15:06:53Z

app/code/Magento/Inventory/Indexer/Scope/UnknownStateException.php

+ * @todo refactoring it copy from catalog search module
+ * @api
+ */
+class UnknownStateException extends LocalizedException


@naydav agree, already pointed that out for IndexTableNotExistException.
These should be framework level exceptions, we can't provide them to client because they provide details of how this functionality is implemented (leaky abstractions).

So, these exceptions are not successors of Localized

maghamed · 2017-08-23T15:14:03Z

app/code/Magento/Inventory/Indexer/Scope/UnknownStateException.php

+ * @todo refactoring it copy from catalog search module
+ * @api
+ */
+class UnknownStateException extends LocalizedException


Btw, I don't know why we have this Exception in our codebase.
So, from what I see we have a pre-defined finite list of states which could be switched in pre-defined order. (aka State machine design pattern)

our state object initially being initialized in 'REGULAR' state.

after that, we've decided to move it to 'TEMPORARY' state while making re-indexation

after re-indexation complete - we move the object to 'REGULAR' state again.

Is it correct? I suppose so, then why do we have a possibility that State object would be created in a wrong state?
@larsroettig

maghamed · 2017-08-23T15:18:41Z

app/code/Magento/Inventory/Indexer/Scope/ScopeProxy.php

+    private function create($state)
+    {
+        if (!array_key_exists($state, $this->states)) {
+            throw new UnknownStateException(__("Requested resolver for unknown indexer state: $state"));


agree with @naydav in the scope of UnknownStateException I described the idea of State Machine, so don't think that we need to check the state and have a factory for state creation

- Add test for index row

# Conflicts: # app/code/Magento/Inventory/etc/di.xml

- refactoring

- fixes

- fix static tests - update db structure

- fix static tests

naydav · 2017-09-01T16:48:28Z

Internal build
https://bamboo.corp.magento.com/browse/M2-AT2478-3

Internal static build (INTEGRITY-SANITY-EE) is red due to MAGETWO-71596
Internal static build (Static LEGACY-EE-L64 and EE-L64) is red.
But coupling between objects for \Magento\Inventory\Test\Integration\Indexer\StockItemTest will be fixed during https://github.com/magento-engcom/magento2/issues/69

maghamed · 2017-09-01T16:56:47Z

Approved to be merged into the Develop branch

Great job guys @larsroettig @naydav , well done!

larsroettig changed the title ~~WIP : Msi stock indexing~~ WIP : Msi stock indexing #48 Jul 30, 2017

larsroettig mentioned this pull request Jul 30, 2017

Stock item indexer #48

Closed

maghamed suggested changes Aug 1, 2017

View reviewed changes

maghamed suggested changes Aug 8, 2017

View reviewed changes

naydav reviewed Aug 9, 2017

View reviewed changes

ishakhsuvarov reviewed Aug 14, 2017

View reviewed changes

maghamed reviewed Aug 15, 2017

View reviewed changes

naydav reviewed Aug 21, 2017

View reviewed changes

naydav force-pushed the develop branch from 9627b00 to a64ea9b Compare August 22, 2017 09:52

larsroettig added 17 commits August 22, 2017 17:00

MSI magento-engcom/magento#48:

512901b

- Add Setup for index table - Add new Interface"

MSI magento-engcom/magento#48:

e6188e5

- Refactoring to new Setup structure - Implement SourceItem add entry to di.xml - WIP Register Basic Indexer SourceItem

MSI magento-engcom/magento#48:

d420097

- Impelemnt basic import funktion TODOS: - Add some bunching function for index - Add Mysql Index to inventory_stock_item_index table - Documentation and clean up

Add bugfix for tests

d607cf4

Remove setup

3fa9e32

MSI magento-engcom/magento#48:

1ebb5b7

- Clean Up and implement basic structure

Add basic structure

886edf6

Implement mview

39edf65

WIP

178439f

wip

9ac0c36

MSI magento-engcom/magento#48:

201c60f

- Impelemnt basic import function TODOS: - Index switch from tmp to idx table - Documentation and clean up

MSI magento-engcom/magento#48:

223e3b9

- Add Setup for index table - Add new Interface I magento-engcom/magento#48: - Add Setup for index table - Add new Interface

MSI magento-engcom/magento#48:

bddf88c

- Implement switch for Index TODO: - Clean Up and more Doc - Discuss the missing website to stock mapping

wip

91c2aea

MSI magento-engcom/magento#48:

f9945c4

- Impelemnt Basic intigration test fixtures

MSI magento-engcom/magento#48:

8ee0afe

- Bugfix for di.xml - Add basic call for reindex full

MSI magento-engcom/magento#48:

859a726

- Basic Indexer with switch for tmp and normal index TODO: - Refactoring to remove the state folder from the module - Doc blocks and clean up - Add assertions to the test case

MSI: WIP : Msi stock indexing

31e5df0

-- fixes after merge

naydav force-pushed the msi-stock-indexing branch from 23864df to 31e5df0 Compare August 22, 2017 14:10

maghamed reviewed Aug 23, 2017

View reviewed changes

larsroettig and others added 15 commits August 26, 2017 23:05

MSI magento-engcom/magento#48:

e5f0b21

- Add test for index row

Merge remote-tracking branch 'origin/develop' into msi-stock-indexing

09cb38b

# Conflicts: # app/code/Magento/Inventory/etc/di.xml

WIP: Msi stock indexing #49

6abbedd

- refactoring

WIP: Msi stock indexing #49

d09d345

- refactoring

WIP: Msi stock indexing #49

9d184c2

- refactoring

WIP: Msi stock indexing #49

953705f

- refactoring

WIP: Msi stock indexing #49

de908a8

- refactoring

WIP: Msi stock indexing #49

e563f96

- refactoring

WIP: Msi stock indexing #49

ee79fd4

- refactoring

WIP: Msi stock indexing #49

0dc8be6

- refactoring

WIP: Msi stock indexing #49

bdb20ed

- refactoring

WIP: Msi stock indexing #49

10b7933

- fixes

WIP: Msi stock indexing #49

e08a7a4

- fixes

WIP: Msi stock indexing #49

5820160

- fix static tests - update db structure

WIP: Msi stock indexing #49

555cfb9

- fix static tests

naydav merged commit b5c9550 into develop Sep 1, 2017

vrann added msi and removed msi labels Sep 29, 2017

maghamed added Award: advanced Award: special achievement labels Oct 2, 2017

smoskaluk mentioned this pull request Mar 23, 2018

Exception in Low stock report for product with another name on additional storeview #737

Closed

maghamed deleted the msi-stock-indexing branch December 11, 2018 18:12


		use Magento\InventoryApi\Api\Data\StockItemInterface;

		class CreateStockItemIndexTable

WIP : Msi stock indexing #48 #49

WIP : Msi stock indexing #48 #49

Conversation

larsroettig commented Jul 29, 2017

larsroettig commented Jul 30, 2017

maghamed left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maghamed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naydav Aug 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

magento-cicd2 commented Aug 22, 2017 • edited Loading

maghamed left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

naydav commented Sep 1, 2017

maghamed commented Sep 1, 2017

maghamed left a comment •

edited

Loading

naydav Aug 8, 2017 •

edited

Loading

magento-cicd2 commented Aug 22, 2017 •

edited

Loading