[HLD] Inconsistent saving of Stock Data

// 1. set via quantity_and_stock_status attribute (used in product form, used in configurable product form for saving variations, used in functional tests)
$product->setQuantityAndStockStatus($data);
// 2. set via stock_data (used in test fixtures, used in several places with custom logic)
$product->setStockData($data);
// 3. set via Stock Item (now it is proper way for stock data managing)
$stockItem = $product->getExtensionAttributes()->getStockItem();
$this->dataObjectHelper->populateWithArray(
    $stockItem,
    $expectedData,
    StockItemInterface::class
);
$this->productRepository->save($product);

Also, we have a "strange" logic of "how to manage some fields of a stock item". And there are different ways of saving data via model or repository.

Proposed solution

Сreate a single point of data (source of truth) for write operations. A good point is StockItemRepositoryInterface and StockItemInterface
Remove quantity_and_stock_status attribute. Remove all logic based on quantity_and_stock_status and stock_data
Code base refactoring (forms, fixtures)

Implementation steps

1. Update Product model

Proxy all methods with stock data manipulation to StockItem.

// \Magento\Catalog\Model\Product
class Product 
...
    public function __construct(
        ...
        array $data = [],
        StockRegistryInterface $stockRegistry = null,
        HydratorInterface $hydrator = null    
    ) {
        ...
        $this->stockRegistry = $stockRegistry ?: \Magento\Framework\App\ObjectManager::getInstance()
            ->get(StockRegistryInterface::class);
        $this->hydrator = $hydrator ?: \Magento\Framework\App\ObjectManager::getInstance()
            ->get(HydratorInterface::class);
    }
    ... 
    /**
     * {@inheritdoc}
     *
     * This method has been overridden only for backward compatible work with stock item (stock_data,
     * quantity_and_stock_status keys)
     *
     * Use \Magento\Catalog\Api\Data\ProductExtensionInterface::getStockItem for retrieving of stock item data (or stock
     * status index)
     * Use \Magento\Catalog\Api\Data\ProductInterface for retrieving of product data
     *
     */
    public function getData($key = '', $index = null)
    {
        if ('stock_data' === $key) {
            $result = $this->getStockData();
        } elseif ('quantity_and_stock_status' === $key) {
            $result = $this->getQuantityAndStockStatus();
        } else {
            $result = parent::getData($key, $index);
        }
        return $result;
    }
    
    /**
     * {@inheritdoc}
     *
     * This method has been overridden only for backward compatible work with stock item (stock_data,
     * quantity_and_stock_status keys)
     *
     * Use \Magento\Catalog\Api\Data\ProductExtensionInterface::setStockItem for updating of stock item data
     * Use \Magento\Catalog\Api\Data\ProductInterface for updating of product data
     *
     */
    public function setData($key, $value = null)
    {
        if ('stock_data' === $key) {
            $result = $this->setStockData($value);
        } elseif ('quantity_and_stock_status' === $key && is_array($value)) {
            $result = $this->setQuantityAndStockStatus($value);
        } else {
            $result = parent::setData($key, $value);
        }
        return $result;
    }
    
    /**
     * @return array
     * @deprecated Use \Magento\Catalog\Api\Data\ProductExtensionInterface::getStockItem for retrieving of stock item
     * data (or stock status index)
     */
    public function getStockData()
    {
        $stockItem = $this->resolveStockItem();
        return $this->hydrator->extract($stockItem);
    }
    
    /**
     * @param array $stockData
     * @return $this
     * @deprecated Use \Magento\Catalog\Api\Data\ProductExtensionInterface::setStockItem for updating of stock item data
     */
    public function setStockData(array $stockData)
    {
        $stockItem = $this->resolveStockItem();
        $this->dataObjectHelper->populateWithArray(
            $stockItem,
            $stockData,
            StockItemInterface::class
        );
        return $this;
    }
    
    /**
     * @return array
     * @deprecated Use \Magento\Catalog\Api\Data\ProductExtensionInterface::getStockItem for retrieving of stock item
     * data (or stock status index)
     */
    public function getQuantityAndStockStatus()
    {
        return $this->getStockData();
    }
    
    /**
     * @param array $quantityAndStockStatusData
     * @return $this
     * @deprecated Use \Magento\Catalog\Api\Data\ProductExtensionInterface::setStockItem for updating of stock item data
     */
    public function setQuantityAndStockStatus(array $quantityAndStockStatusData)
    {
        return $this->setStockData($quantityAndStockStatusData);
    }
    
    /**
     * @return StockItemInterface
     */
    private function resolveStockItem()
    {
        $extensionAttributes = $this->getExtensionAttributes();
        $stockItem = $extensionAttributes->getStockItem();
        if (null === $stockItem) {
            $stockItem = $this->stockRegistry->getStockItem($this->getId());
            $extensionAttributes->setStockItem($stockItem);
            $stockItem->setProduct($this);
        }
        return $stockItem;
    }

2. Deprecate the 'quantity_and_stock_status' attribute backend model

Deprecate the \Magento\Catalog\Model\Product\Attribute\Backend\Stock attribute backend model. Remove all logic. Taking into account proposed changes, it makes no sense to keep this logic.

3. Create a single point of stock data processing

3.1. Unite logic in a single processing point

The logic of the following classes:

\Magento\CatalogInventory\Model\Plugin\AroundProductRepositorySave
\Magento\CatalogInventory\Observer\SaveInventoryDataObserver should be united in a single "processing" point. It could be a SaveInventoryProcessor class, like in the example below.

PAY ATTENTION: this logic is needed ONLY for BI (saving stock item data on product save). In proper way, we need to save StockItem only via StockItemRepositoryInterface Also, we now save via stockregistry only for BI.

/**
 *  Dockblock like as "this logic is needed ONLY for BI (saving stock item data on product save). In propel way, we need to save StockItem only via StockItemRepository"
 */
class SaveInventoryProcessor (NOT INTERFACE)
{
    /**
     * Saving product inventory data
     *
     * @param Product $product
     * @return $this
     * @throws LocalizedException
     */
    public function execute(Product $product)
    {
        $stockItem = $this->getStockItemToBeUpdated($product);
        if (null !== $stockItem) {
            $stockItem->setProductId($product->getId());
            $stockItem->setWebsiteId($this->stockConfiguration->getDefaultScopeId());
            $this->stockItemValidator->validate($product, $stockItem);
            $this->stockRegistry->updateStockItemBySku($product->getSku(), $stockItem);
        }
        return $this;
    }
    
    /**
     * Return the stock item that needs to be updated
     *
     * @param ProductInterface $product
     * @return StockItemInterface|null
     */
    private function getStockItemToBeUpdated($product)
    {
        $stockItem = null;
        $extendedAttributes = $product->getExtensionAttributes();
        if ($extendedAttributes !== null) {
            $stockItem = $extendedAttributes->getStockItem();
        }
    
        if ($stockItem === null) {
            $stockItem = $this->stockRegistry->getStockItem($product->getId());
        }
        return $stockItem;
    }
}

The processing logic of saving data via model and via product repository must be the same!

3.2. Move validation logic from ProductRepositoryPlugin to a separate class.

For example:

<?php
/**
 * Copyright © 2016 Magento. All rights reserved.
 * See COPYING.txt for license details.
 */
namespace Magento\CatalogInventory\Model;

use Magento\Catalog\Api\Data\ProductInterface;
use Magento\CatalogInventory\Api\Data\StockItemInterface;
use Magento\CatalogInventory\Api\StockConfigurationInterface;
use Magento\CatalogInventory\Api\StockRegistryInterface;
use Magento\Framework\Exception\LocalizedException;

/**
 * StockItemValidator
 */
class StockItemValidator (NOT INTERFACE)
{
    /**
     * @var StockConfigurationInterface
     */
    private $stockConfiguration;

    /**
     * @var StockRegistryInterface
     */
    private $stockRegistry;

    /**
     * @param StockConfigurationInterface $stockConfiguration
     * @param StockRegistryInterface $stockRegistry
     */
    public function _construct(
        StockConfigurationInterface $stockConfiguration,
        StockRegistryInterface $stockRegistry
    ) {
        $this->stockConfiguration = $stockConfiguration;
        $this->stockRegistry = $stockRegistry;
    }

    /**
     * @param ProductInterface $product
     * @param StockItemInterface $stockItem
     * @throws LocalizedException
     * @return void
     */
    public function validate(ProductInterface $product, StockItemInterface $stockItem)
    {
        $defaultScopeId = $this->stockConfiguration->getDefaultScopeId();
        $defaultStockId = (int)$this->stockRegistry->getStock($defaultScopeId)->getStockId();
        $stockId = (int)$stockItem->getStockId();
        if ($stockId !== null && $stockId !== $defaultStockId) {
            throw new LocalizedException(
                __('Invalid stock id: %1. Only default stock with id %2 allowed', $stockId, $defaultStockId)
            );
        }

        $stockItemId = $stockItem->getItemId();
        if ($stockItemId !== null && (!is_numeric($stockItemId) || $stockItemId <= 0)) {
            throw new LocalizedException(
                __('Invalid stock item id: %1. Should be null or numeric value greater than 0', $stockItemId)
            );
        }

        $defaultStockItemId = $this->stockRegistry->getStockItem($product->getId())->getItemId();
        if ($defaultStockItemId && $stockItemId !== null && $defaultStockItemId !== $stockItemId) {
            throw new LocalizedException(
                __('Invalid stock item id: %1. Assigned stock item id is %2', $stockItemId, $defaultStockItemId)
            );
        }
    }
}

Also need to investigate about moving this logic in StockItemReposioty

4. Refactor the Product form

Currently, the product form sends data to the server in two keys:

stock_data
quantity_and_stock_status After refactoring, the form should send data in a single field.

The best way is to keep the data consistent with Magento WebAPI:

[
    ProductInterface::EXTENSION_ATTRIBUTES_KEY => [
        'stock_item' => [
            StockItemInterface::QTY => 1000,
            StockItemInterface::IS_IN_STOCK => true,
            ...
        ],
    ]

You will also need to update:

\Magento\CatalogInventory\Ui\DataProvider\Product\Form\Modifier\AdvancedInventory::modifyData

5. Refactor the Configurable Product form

For this refactoring, you will need to:

update the Configurations section in configurable product form (creating and updating)
remove logic from \Magento\ConfigurableProduct\Model\Product\VariationHandler

if (!isset($postData['stock_data']['is_in_stock'])) {
    $stockStatus = $parentProduct->getQuantityAndStockStatus();
    $postData['stock_data']['is_in_stock'] = $stockStatus['is_in_stock'];
}

6. Refactor functional tests

The table below shows how many times the entities we're changing occur in functional tests. First of all, it is fixtures creating.

Name	Occurrences in CE
stock_data	56
quantity_and_stock_status	264

7. Fix import and export

7.1. Skip attribute from import

For \Magento\CatalogImportExport\Model\Import\Product\Type\AbstractType:

namespace Magento\CatalogImportExport\Model\Import\Product\Type;
abstract class AbstractType
{
    /**
     * This property has been added in scope of work for backward compatible of stock item processing (work with
     * stock_data and quantity_and_stock_status keys)
     * quantity_and_stock_status attribute will be deleted
     *
     * @var array
     */
    private $attributesToSkip = ['quantity_and_stock_status']; // values must be configurable via DI
    ...
    public function prepareAttributesWithDefaultValueForSave(array $rowData, $withDefaultValue = true)
    {
        $resultAttrs = [];
    
        foreach ($this->_getProductAttributes($rowData) as $attrCode => $attrParams) {
            if ($attrParams['is_static'] || in_array($attrCode, $this->attributesToSkip)) {
                continue;
            }
    ...
}

For the import test, \Magento\CatalogImportExport\Model\AbstractProductExportImportTestCase:

namespace Magento\CatalogImportExport\Model;

abstract class AbstractProductExportImportTestCase extends \PHPUnit_Framework_TestCase
{
    ...
    public static $skippedAttributes = [
    ...
    'quantity_and_stock_status',
    ];
    ...
}

7.2. [Important!] Check possibility to disable/enable the Out of Stock status when importing products

7.3. Check and if needed replace 'quantity_and_stock_status' attribute with stock item data in Import/Export.

8. Fix Product model

8.1. Replace 'getExtensionAttributes' method

/**
 * {@inheritdoc}
 *
 * @return \Magento\Catalog\Api\Data\ProductExtensionInterface
 */
public function getExtensionAttributes()
{
    $extensionAttributes = $this->_getExtensionAttributes();
    if (!$extensionAttributes) {
        /** @var \Magento\Catalog\Api\Data\ProductExtensionInterface $extensionAttributes */
        $extensionAttributes = $this->extensionAttributesFactory->create(
            \Magento\Catalog\Api\Data\ProductInterface::class
        );
        $this->setExtensionAttributes($extensionAttributes);
    }
    return $extensionAttributes;
}

After replacing, we nave a problem: the extensionAttributes is recreated every time the method is called.

8.2. Fix 'toArray' and 'fromArray'

The entities:

\Magento\Catalog\Model\Product::toArray
\Magento\Catalog\Model\Product::fromArray must be changed to get the stock inventory data from the current StockItem.

8.3. Remove reindexing from 'afterSave'

Remove the following from the \Magento\Catalog\Model\Product::afterSave:

if ($this->getStockData()) {
    $this->setForceReindexEavRequired(true);
}

9. Resolve problem with origin data

We have the following situation in _\Magento\Catalog\Model\Product.php#L2298

/**
 * Check whether stock status changed
 *
 * @return bool
 */
private function isStockStatusChanged()
{
    $stockItem = null;
    $extendedAttributes = $this->getExtensionAttributes();
    if ($extendedAttributes !== null) {
        $stockItem = $extendedAttributes->getStockItem();
    }
    $stockData = $this->getStockData();
    return (
        (is_array($stockData))
        && array_key_exists('is_in_stock', $stockData)
        && (null !== $stockItem)
        && ($stockItem->getIsInStock() != $stockData['is_in_stock'])
    );
}

We need to DELETE or REFACT this code. So, we need to check the origin of the Stock Status value. We can solve in several ways:

[Recommended] Create a mechanism for tracking the origin entity data, based on EntityStateInterface. For this, a separate HLD is required.
Use origin data from StockItem like as $stockItem->getOriginData('status')
Create a simple model like this:

class ProductOriginStockStatusChecker
{
    public function getValue()
    {
        // resolving from origin data or direct call to db or 
    } 
}

Using of private property in Product model for storing stock status data.

10. Required integration tests

10.1. Create tests

\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductModel\ByQuantityAndStockStatusTest
\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductModel\ByStockDataTest
\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductModel\ByStockItemTest
\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductRepository\ByQuantityAndStockStatusTest
\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductRepository\ByStockDataTest
\Magento\CatalogInventory\StockItemSave\OnProductCreate\ByProductRepository\ByStockItemTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductModel\ByQuantityAndStockStatusTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductRepository\ByStockDataTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductRepository\ByStockItemTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductRepository\ByQuantityAndStockStatusTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductRepository\ByStockDataTest
\Magento\CatalogInventory\StockItemSave\OnProductUpdate\ByProductRepository\ByStockItemTest
\Magento\CatalogInventory\StockItemSave\ByStockItemRepositoryTest

Important: All tests have already been created. They are available in internal Magento team repository.

10.2. Remove test \Magento\CatalogInventory\Api\StockItemSaveTest

11. Update all integration tests

Integration tests should use the only eligible API for writing data using StockItem interface

Acceptance Criteria

Single point of data (source of truth) for write operations are created
'quantity_and_stock_status' attribute is removed (related logic are removed)
The processing logic of saving data via model and via product repository is the same!
Required Integration tests are created

Multi-Source Inventory developed by Magento 2 Community

Provide feedback

Saved searches

Use saved searches to filter your results more quickly