Skip to content

Commit

Permalink
Hotfix for large number of queries in scenarios related to Shopping Cart
Browse files Browse the repository at this point in the history
  • Loading branch information
ishakhsuvarov committed Sep 16, 2019
1 parent ba2c699 commit ad57569
Show file tree
Hide file tree
Showing 17 changed files with 454 additions and 7 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\Inventory\Model\Source\Command;

use Magento\InventoryApi\Api\GetSourcesAssignedToStockOrderedByPriorityInterface;

/**
* @inheritdoc
*/
class GetSourcesAssignedToStockOrderedByPriorityCache implements GetSourcesAssignedToStockOrderedByPriorityInterface
{
/**
* @var GetSourcesAssignedToStockOrderedByPriority
*/
private $getSourcesAssignedToStock;

/**
* @var array
*/
private $sourcesAssignedToStock = [];

/**
* @param GetSourcesAssignedToStockOrderedByPriority $getSourcesAssignedToStockOrderedByPriority
*/
public function __construct(
GetSourcesAssignedToStockOrderedByPriority $getSourcesAssignedToStockOrderedByPriority
) {
$this->getSourcesAssignedToStock = $getSourcesAssignedToStockOrderedByPriority;
}

/**
* @inheritdoc
*/
public function execute(int $stockId): array
{
if (!isset($this->sourcesAssignedToStock[$stockId])) {
$this->sourcesAssignedToStock[$stockId] = $this->getSourcesAssignedToStock->execute($stockId);
}

return $this->sourcesAssignedToStock[$stockId];
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/Inventory/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<preference for="Magento\InventoryApi\Api\Data\SourceInterface" type="Magento\Inventory\Model\Source"/>
<preference for="Magento\InventoryApi\Api\Data\SourceCarrierLinkInterface" type="Magento\Inventory\Model\SourceCarrierLink"/>
<preference for="Magento\InventoryApi\Api\Data\SourceSearchResultsInterface" type="Magento\Inventory\Model\SourceSearchResults"/>
<preference for="Magento\InventoryApi\Api\GetSourcesAssignedToStockOrderedByPriorityInterface" type="Magento\Inventory\Model\Source\Command\GetSourcesAssignedToStockOrderedByPriority"/>
<preference for="Magento\InventoryApi\Api\GetSourcesAssignedToStockOrderedByPriorityInterface" type="Magento\Inventory\Model\Source\Command\GetSourcesAssignedToStockOrderedByPriorityCache"/>
<preference for="Magento\InventoryApi\Api\GetSourceItemsBySkuInterface" type="Magento\Inventory\Model\SourceItem\Command\GetSourceItemsBySku"/>
<preference for="Magento\InventoryApi\Model\GetSourceCodesBySkusInterface" type="Magento\Inventory\Model\GetSourceCodesBySkus"/>
<preference for="Magento\InventoryApi\Model\SourceCarrierLinkManagementInterface" type="Magento\Inventory\Model\SourceCarrierLinkManagement"/>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventoryCatalog\Model;

use Magento\InventoryCatalogApi\Model\GetProductIdsBySkusInterface;
use Magento\Framework\Serialize\Serializer\Json;

/**
* @inheritdoc
*/
class GetProductIdsBySkusCache implements GetProductIdsBySkusInterface
{
/**
* @var GetProductIdsBySkus
*/
private $getProductIdsBySkus;

/**
* @var Json
*/
private $jsonSerializer;

/**
* @var array
*/
private $productIdsBySkus = [];

/**
* @param GetProductIdsBySkus $getProductIdsBySkus
* @param Json $jsonSerializer
*/
public function __construct(
GetProductIdsBySkus $getProductIdsBySkus,
Json $jsonSerializer
) {
$this->getProductIdsBySkus = $getProductIdsBySkus;
$this->jsonSerializer = $jsonSerializer;
}

/**
* @inheritdoc
*/
public function execute(array $skus): array
{
$cacheKey = $this->jsonSerializer->serialize($skus);
if (!isset($this->productIdsBySkus[$cacheKey])) {
$this->productIdsBySkus[$cacheKey] = $this->getProductIdsBySkus->execute($skus);
}

return $this->productIdsBySkus[$cacheKey];
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventoryCatalog\Model\ResourceModel;

use Magento\InventoryCatalogApi\Model\GetProductTypesBySkusInterface;
use Magento\Framework\Serialize\Serializer\Json;

/**
* @inheritdoc
*/
class GetProductTypesBySkusCache implements GetProductTypesBySkusInterface
{
/**
* @var GetProductTypesBySkus
*/
private $getProductTypesBySkus;

/**
* @var Json
*/
private $jsonSerializer;

/**
* @var array
*/
private $productTypesBySkus = [];

/**
* @param GetProductTypesBySkus $getProductTypesBySkus
* @param Json $jsonSerializer
*/
public function __construct(
GetProductTypesBySkus $getProductTypesBySkus,
Json $jsonSerializer
) {
$this->getProductTypesBySkus = $getProductTypesBySkus;
$this->jsonSerializer = $jsonSerializer;
}

/**
* @inheritdoc
*/
public function execute(array $skus): array
{
$cacheKey = $this->jsonSerializer->serialize($skus);
if (!isset($this->productTypesBySkus[$cacheKey])) {
$this->productTypesBySkus[$cacheKey] = $this->getProductTypesBySkus->execute($skus);
}

return $this->productTypesBySkus[$cacheKey];
}
}
4 changes: 2 additions & 2 deletions app/code/Magento/InventoryCatalog/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
<preference for="Magento\InventoryCatalogApi\Api\DefaultStockProviderInterface" type="Magento\InventoryCatalog\Model\DefaultStockProvider"/>
<preference for="Magento\InventoryCatalogApi\Api\DefaultSourceProviderInterface" type="Magento\InventoryCatalog\Model\DefaultSourceProvider"/>
<preference for="Magento\InventoryCatalogApi\Model\GetProductIdsBySkusInterface" type="Magento\InventoryCatalog\Model\GetProductIdsBySkus"/>
<preference for="Magento\InventoryCatalogApi\Model\GetProductTypesBySkusInterface" type="Magento\InventoryCatalog\Model\ResourceModel\GetProductTypesBySkus" />
<preference for="Magento\InventoryCatalogApi\Model\GetProductIdsBySkusInterface" type="Magento\InventoryCatalog\Model\GetProductIdsBySkusCache"/>
<preference for="Magento\InventoryCatalogApi\Model\GetProductTypesBySkusInterface" type="Magento\InventoryCatalog\Model\ResourceModel\GetProductTypesBySkusCache" />
<preference for="Magento\InventoryCatalogApi\Model\GetSkusByProductIdsInterface" type="Magento\InventoryCatalog\Model\GetSkusByProductIds"/>
<preference for="Magento\InventoryCatalogApi\Model\IsSingleSourceModeInterface" type="Magento\InventoryCatalog\Model\IsSingleSourceMode"/>
<type name="Magento\InventoryApi\Api\StockRepositoryInterface">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventoryConfiguration\Plugin\InventoryConfiguration\Model;

use Magento\CatalogInventory\Api\Data\StockItemInterface;
use Magento\InventoryConfiguration\Model\GetLegacyStockItem;

/**
* Caching plugin for GetLegacyStockItem service.
*/
class GetLegacyStockItemCache
{
/**
* @var array
*/
private $legacyStockItemsBySku = [];

/**
* Cache the result of service to avoid duplicate queries to the database.
*
* @param GetLegacyStockItem $subject
* @param callable $proceed
* @param string $sku
* @return StockItemInterface
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function aroundExecute(GetLegacyStockItem $subject, callable $proceed, string $sku): StockItemInterface
{
if (!isset($this->legacyStockItemsBySku[$sku])) {
$this->legacyStockItemsBySku[$sku] = $proceed($sku);
}

return $this->legacyStockItemsBySku[$sku];
}
}
4 changes: 4 additions & 0 deletions app/code/Magento/InventoryConfiguration/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,8 @@
<plugin name="allow_negative_min_qty_in_config"
type="Magento\InventoryConfiguration\Plugin\CatalogInventory\Model\System\Config\Backend\Minqty\AllowNegativeMinQtyInConfigPlugin"/>
</type>
<type name="Magento\InventoryConfiguration\Model\GetLegacyStockItem">
<plugin name="cache_legacy_stock_item"
type="Magento\InventoryConfiguration\Plugin\InventoryConfiguration\Model\GetLegacyStockItemCache"/>
</type>
</config>
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventoryIndexer\Model\ResourceModel;

use Magento\InventorySalesApi\Model\GetStockItemDataInterface;

/**
* @inheritdoc
*/
class GetStockItemDataCache implements GetStockItemDataInterface
{
/**
* @var GetStockItemData
*/
private $getStockItemData;

/**
* @var array
*/
private $stockItemData = [[]];

/**
* @param GetStockItemData $getStockItemData
*/
public function __construct(
GetStockItemData $getStockItemData
) {
$this->getStockItemData = $getStockItemData;
}

/**
* @inheritdoc
*/
public function execute(string $sku, int $stockId): ?array
{
if (!isset($this->stockItemData[$stockId][$sku])) {
$this->stockItemData[$stockId][$sku] = $this->getStockItemData->execute($sku, $stockId);
}

return $this->stockItemData[$stockId][$sku];
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/InventoryIndexer/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
<plugin name="invalidate_after_stock_source_links_delete" type="Magento\InventoryIndexer\Plugin\InventoryApi\InvalidateAfterStockSourceLinksDeletePlugin"/>
</type>
<!-- Stock Item -->
<preference for="Magento\InventorySalesApi\Model\GetStockItemDataInterface" type="Magento\InventoryIndexer\Model\ResourceModel\GetStockItemData"/>
<preference for="Magento\InventorySalesApi\Model\GetStockItemDataInterface" type="Magento\InventoryIndexer\Model\ResourceModel\GetStockItemDataCache"/>
<preference for="Magento\InventoryIndexer\Model\StockIndexTableNameResolverInterface" type="Magento\InventoryIndexer\Model\StockIndexTableNameResolver"/>
<type name="Magento\InventoryIndexer\Model\StockIndexTableNameResolver">
<arguments>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventoryReservations\Model\ResourceModel;

use Magento\InventoryReservationsApi\Model\GetReservationsQuantityInterface;

/**
* @inheritdoc
*/
class GetReservationsQuantityCache implements GetReservationsQuantityInterface
{
/**
* @var GetReservationsQuantity
*/
private $getReservationsQuantity;

/**
* @var array
*/
private $reservationsQuantity = [[]];

/**
* @param GetReservationsQuantity $getReservationsQuantity
*/
public function __construct(
GetReservationsQuantity $getReservationsQuantity
) {
$this->getReservationsQuantity = $getReservationsQuantity;
}

/**
* @inheritdoc
*/
public function execute(string $sku, int $stockId): float
{
if (!isset($this->reservationsQuantity[$sku][$stockId])) {
$this->reservationsQuantity[$sku][$stockId] = $this->getReservationsQuantity->execute($sku, $stockId);
}

return $this->reservationsQuantity[$sku][$stockId];
}
}
2 changes: 1 addition & 1 deletion app/code/Magento/InventoryReservations/etc/di.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<preference for="Magento\InventoryReservationsApi\Model\ReservationInterface" type="Magento\InventoryReservations\Model\Reservation"/>
<preference for="Magento\InventoryReservationsApi\Model\ReservationBuilderInterface" type="Magento\InventoryReservations\Model\ReservationBuilder"/>
<preference for="Magento\InventoryReservationsApi\Model\CleanupReservationsInterface" type="Magento\InventoryReservations\Model\ResourceModel\CleanupReservations"/>
<preference for="Magento\InventoryReservationsApi\Model\GetReservationsQuantityInterface" type="Magento\InventoryReservations\Model\ResourceModel\GetReservationsQuantity"/>
<preference for="Magento\InventoryReservationsApi\Model\GetReservationsQuantityInterface" type="Magento\InventoryReservations\Model\ResourceModel\GetReservationsQuantityCache"/>
<type name="Magento\InventoryReservations\Model\ResourceModel\CleanupReservations">
<arguments>
<argument name="groupConcatMaxLen" xsi:type="number">32768</argument>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
declare(strict_types=1);

namespace Magento\InventorySales\Model;

use Magento\InventorySalesApi\Model\GetAssignedSalesChannelsForStockInterface;

/**
* @inheritdoc
*/
class GetAssignedSalesChannelsForStockCache implements GetAssignedSalesChannelsForStockInterface
{
/**
* @var GetAssignedSalesChannelsForStock
*/
private $getAssignedSalesChannelsForStock;

/**
* @var array
*/
private $salesChannelsAssignedToStocks = [];

/**
* @param GetAssignedSalesChannelsForStock $getAssignedSalesChannelsForStock
*/
public function __construct(
GetAssignedSalesChannelsForStock $getAssignedSalesChannelsForStock
) {
$this->getAssignedSalesChannelsForStock = $getAssignedSalesChannelsForStock;
}

/**
* @inheritdoc
*/
public function execute(int $stockId): array
{
if (!isset($this->salesChannelsAssignedToStocks[$stockId])) {
$this->salesChannelsAssignedToStocks[$stockId] = $this->getAssignedSalesChannelsForStock->execute($stockId);
}

return $this->salesChannelsAssignedToStocks[$stockId];
}
}
Loading

2 comments on commit ad57569

@k4emic
Copy link

@k4emic k4emic commented on ad57569 Oct 10, 2019

Choose a reason for hiding this comment

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

Disclaimer: This patch does wonder for performance, great work!

This hotfix causes some problems during the initial magento installation process when adding/modifying websites (in my case, we were adding one additional website and then changing the code of the default website) which gives a nondescript "Validation failed" message.

Stack trace

 () at <root>/vendor/magento/module-inventory/Model/Stock/Command/Save.php:59
 Magento\Inventory\Model\Stock\Command\Save->execute() at <root>/vendor/magento/module-inventory/Model/StockRepository.php:67
 Magento\Inventory\Model\StockRepository->save() at <root>/vendor/magento/framework/Interception/Interceptor.php:58
 Magento\Inventory\Model\StockRepository\Interceptor->___callParent() at <root>/vendor/magento/framework/Interception/Interceptor.php:138
 Magento\Inventory\Model\StockRepository\Interceptor->Magento\Framework\Interception\{closure}() at <root>/vendor/magento/framework/Interception/Interceptor.php:153
 Magento\Inventory\Model\StockRepository\Interceptor->___callPlugins() at <root>/generated/code/Magento/Inventory/Model/StockRepository/Interceptor.php:26
 Magento\Inventory\Model\StockRepository\Interceptor->save() at <root>/vendor/magento/module-inventory-sales/Plugin/Store/Model/ResourceModel/Website/AssignWebsiteToDefaultStockPlugin.php:100
 Magento\InventorySales\Plugin\Store\Model\ResourceModel\Website\AssignWebsiteToDefaultStockPlugin->afterSave() at <root>/vendor/magento/framework/Interception/Interceptor.php:146
 Magento\Store\Model\ResourceModel\Website\Interceptor->Magento\Framework\Interception\{closure}() at <root>/vendor/magento/framework/Interception/Interceptor.php:153
 Magento\Store\Model\ResourceModel\Website\Interceptor->___callPlugins() at <root>/generated/code/Magento/Store/Model/ResourceModel/Website/Interceptor.php:130
 Magento\Store\Model\ResourceModel\Website\Interceptor->save() at <root>/vendor/magento/framework/Model/AbstractModel.php:654
 Magento\Framework\Model\AbstractModel->save() at <root>/vendor/magento/framework/Interception/Interceptor.php:58
 Magento\Store\Model\Website\Interceptor->___callParent() at <root>/vendor/magento/framework/Interception/Interceptor.php:138
 Magento\Store\Model\Website\Interceptor->Magento\Framework\Interception\{closure}() at <root>/vendor/magento/module-theme/Model/Indexer/Design/Config/Plugin/Website.php:39
 Magento\Theme\Model\Indexer\Design\Config\Plugin\Website->aroundSave() at <root>/vendor/magento/framework/Interception/Interceptor.php:135
 Magento\Store\Model\Website\Interceptor->Magento\Framework\Interception\{closure}() at <root>/vendor/magento/framework/Interception/Interceptor.php:153
 Magento\Store\Model\Website\Interceptor->___callPlugins() at <root>/generated/code/Magento/Store/Model/Website/Interceptor.php:793

This is where the message originates from (but this is only a symptom of another problem):

if (SalesChannelInterface::TYPE_WEBSITE === $type) {
try {
$this->websiteRepository->get($code);
} catch (NoSuchEntityException $e) {
$errors[] = __('The website with code "%code" does not exist.', ['code' => $code]);
}
}

The rabbit hole goes deep, but what happens is that the cache will cache an empty response (before the default website is even installed), so Magento will think that the base website is not associated with the default stock for the entirety of the installation process (unless working from an already working database, so this mainly impacts CI users).

More specifically, this function now believes that there is no stock attached (despite the inventory_stock_sales_channel table being correct):

// checks is some stock already assigned to this website
if ($this->getAssignedStockIdForWebsite->execute($websiteCode) !== null) {
return $result;
}

This results in the sales channel link between the base website and the default stock to disappear.

I've devised the below patch which invalidates the cache following an update to the relation between sales channels and their stock.

diff -urN etc/di.xml etc/di.xml
--- etc/di.xml	2019-10-10 12:08:27.922598819 +0200
+++ etc/di.xml	2019-10-10 12:26:53.000000000 +0200
@@ -170,4 +170,8 @@
                 type="Magento\InventoryCatalog\Plugin\InventoryConfigurationApi\GetStockItemConfiguration\LoadIsInStockPlugin"/>
     </type>
     <preference for="Magento\InventorySalesApi\Model\GetSkuFromOrderItemInterface" type="Magento\InventorySales\Model\GetSkuFromOrderItem"/>
+
+    <type name="Magento\InventorySalesApi\Model\ReplaceSalesChannelsForStockInterface">
+        <plugin name="clear_sales_channel_cache" type="Magento\InventorySales\Plugin\SalesChannelCache\ResetSalesChannelCache" />
+    </type>
 </config>
diff -urN Model/GetAssignedSalesChannelsForStockCache.php Model/GetAssignedSalesChannelsForStockCache.php
--- Model/GetAssignedSalesChannelsForStockCache.php	2019-10-10 12:20:09.000000000 +0200
+++ Model/GetAssignedSalesChannelsForStockCache.php	2019-10-10 12:18:07.000000000 +0200
@@ -44,4 +44,9 @@

         return $this->salesChannelsAssignedToStocks[$stockId];
     }
+
+    public function reset()
+    {
+        $this->salesChannelsAssignedToStocks = [];
+    }
 }
diff -urN Model/GetStockBySalesChannelCache.php Model/GetStockBySalesChannelCache.php
--- Model/GetStockBySalesChannelCache.php	2019-10-10 12:21:08.000000000 +0200
+++ Model/GetStockBySalesChannelCache.php	2019-10-10 12:21:17.000000000 +0200
@@ -48,4 +48,9 @@

         return $this->stocksBySalesChannels[$type][$code];
     }
+
+    public function reset()
+    {
+        $this->stocksBySalesChannels = [[]];
+    }
 }
diff -urN Plugin/SalesChannelCache/ResetSalesChannelCache.php Plugin/SalesChannelCache/ResetSalesChannelCache.php
--- Plugin/SalesChannelCache/ResetSalesChannelCache.php	1970-01-01 01:00:00.000000000 +0100
+++ Plugin/SalesChannelCache/ResetSalesChannelCache.php	2019-10-10 12:22:23.000000000 +0200
@@ -0,0 +1,38 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\InventorySales\Plugin\SalesChannelCache;
+
+use Magento\InventorySales\Model\GetAssignedSalesChannelsForStockCache;
+use Magento\InventorySales\Model\GetStockBySalesChannelCache;
+
+class ResetSalesChannelCache
+{
+    /**
+     * @var GetStockBySalesChannelCache
+     */
+    private $stockBySalesChannelCache;
+    /**
+     * @var GetAssignedSalesChannelsForStockCache
+     */
+    private $assignedSalesChannelsCache;
+
+    public function __construct(
+        GetStockBySalesChannelCache $stockBySalesChannelCache,
+        GetAssignedSalesChannelsForStockCache $assignedSalesChannelsCache
+    ) {
+
+        $this->stockBySalesChannelCache = $stockBySalesChannelCache;
+        $this->assignedSalesChannelsCache = $assignedSalesChannelsCache;
+    }
+
+    public function afterExecute()
+    {
+        $this->stockBySalesChannelCache->reset();
+        $this->assignedSalesChannelsCache->reset();
+    }
+}

@ishakhsuvarov
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for this work @k4emic.

I'll work to get your solution included in the patch as well. We still haven't decided if this is something that we would include in the codebase at this point, but if we do, now it is 1 step closer :)

Please sign in to comment.