Skip to content

Commit

Permalink
Merged PR 60009: Load products in bulk to reduce expensive queries
Browse files Browse the repository at this point in the history
## What's being changed

This PR further reduces SQL queries by loading products in bulk wherever possible. Products are cached by store id because, for brand values:
- attributes can be global, website or store scoped
- for store-scoped attributes, options can change between store scopes, as can the values of those options

Example:
- The Dotdigital Brand Attribute is set to 'Manufacturer'
- Products can in theory have a different Manufacturer per store
- The options for Manufacturer may be presented as 'Company A, 'Company B' and 'Company C', but in the attribute configuration the actual option text for each can vary (for example 'Le Company A' etc for a French store view).

## Why it's being changed

Individual orders may have many associated products. All these need loading in order to arrive at a string of category ids for FIRST_CATEGORY_PUR and LAST_CATEGORY_PUR. Each time we replace e.g. 10 product entity loads with a single query, there is a reduction in resource utilisation.

## How to review / test this change

- Compare the JSON exported from develop with the JSON exported on this branch
- See no change
- Further test steps specific to brand values in different scopes to follow in the next PR.

## Notes

We can go further and restrict the attributes loaded in the collection query in the ProductLoader. This will follow in a separate PR.

Related work items: #278457
  • Loading branch information
sta1r committed Nov 18, 2024
1 parent dc7eaea commit ee461c7
Show file tree
Hide file tree
Showing 3 changed files with 121 additions and 59 deletions.
19 changes: 10 additions & 9 deletions Model/Connector/ContactData.php
Original file line number Diff line number Diff line change
Expand Up @@ -503,8 +503,8 @@ public function getMostPurBrand()

//if the id and attribute found
if ($productId && $attributeCode) {
$product = $this->productLoader->getProduct((int) $productId, (int) $this->model->getStoreId());
if ($product->getId()) {
$product = $this->productLoader->getCachedProductById((int) $productId, (int) $this->model->getStoreId());
if ($product && $product->getId()) {
$attribute = $this->productResource->getAttribute($attributeCode);
$value = is_object($attribute) ? $attribute->getFrontend()->getValue($product) : null;
if ($value) {
Expand Down Expand Up @@ -552,9 +552,9 @@ public function getMostPurCategory()
$productId = $this->model->getProductIdForMostSoldProduct();
//sales data found for customer with product id
if ($productId) {
$product = $this->productLoader->getProduct((int) $productId, (int) $this->model->getStoreId());
$product = $this->productLoader->getCachedProductById((int) $productId, (int) $this->model->getStoreId());
//product found
if ($product->getId()) {
if ($product && $product->getId()) {
$categoryIds = $product->getCategoryIds();
if (count($categoryIds)) {
$categories = $this->getCategoryNamesFromIds($categoryIds);
Expand Down Expand Up @@ -658,10 +658,10 @@ private function getBrandAttributeValue($id, $attributeCode, $storeId)
{
//if the id and attribute found
if ($id && $attributeCode) {
$product = $this->productLoader->getProduct((int) $id, (int) $storeId);
$product = $this->productLoader->getCachedProductById((int) $id, (int) $storeId);
$attribute = $this->productResource->getAttribute($attributeCode);

if ($attribute && $product->getId()) {
if ($attribute && $product && $product->getId()) {
$value = $attribute->setStoreId($storeId)
->getSource()
->getOptionText($product->getData($attributeCode));
Expand Down Expand Up @@ -751,9 +751,10 @@ public function getSubscriberStatusString($statusCode)
private function getCategoriesFromProducts(array $productIds): array
{
$categoryIds = [];
foreach ($productIds as $productId) {
$product = $this->productLoader->getProduct((int) $productId, (int) $this->model->getStoreId());
if ($product->getId()) {
$products = $this->productLoader->getCachedProducts($productIds, (int) $this->model->getStoreId());

foreach ($products as $product) {
if ($product && $product->getId()) {
foreach ($product->getCategoryIds() as $categoryId) {
if (!in_array($categoryId, $categoryIds)) {
$categoryIds[] = $categoryId;
Expand Down
96 changes: 71 additions & 25 deletions Model/Connector/ContactData/ProductLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,56 +4,102 @@

namespace Dotdigitalgroup\Email\Model\Connector\ContactData;

use Magento\Catalog\Api\Data\ProductInterfaceFactory;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\ResourceModel\Product as ProductResource;
use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory as CatalogCollectionFactory;

class ProductLoader
{
/**
* @var ProductInterfaceFactory
* @var CatalogCollectionFactory
*/
private $productFactory;

/**
* @var ProductResource
*/
private $productResource;
private $catalogCollectionFactory;

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

/**
* @param ProductInterfaceFactory $productFactory
* @param ProductResource $productResource
* @param CatalogCollectionFactory $catalogCollectionFactory
*/
public function __construct(
ProductInterfaceFactory $productFactory,
ProductResource $productResource
CatalogCollectionFactory $catalogCollectionFactory
) {
$this->productFactory = $productFactory;
$this->productResource = $productResource;
$this->catalogCollectionFactory = $catalogCollectionFactory;
}

/**
* Load a product by id and store it to prevent repeat loading of the same entity.
* Get cached product by id.
*
* @param int $productId
* @param int $storeId
*
* @return Product
* @return Product|null
*/
public function getCachedProductById(int $productId, int $storeId)
{
if (!isset($this->products[$productId][$storeId])) {
$this->setProducts([$productId], $storeId);
}

return $this->products[$productId][$storeId] ?? null;
}

/**
* Get cached products.
*
* @param array $productIds
* @param int $storeId
*
* @return Product[]
*/
public function getProduct(int $productId, int $storeId)
public function getCachedProducts(array $productIds, int $storeId)
{
if (!isset($this->products[$productId])) {
/** @var Product $product */
$product = $this->productFactory->create();
$product->setStoreId($storeId);
$this->productResource->load($product, $productId);
$this->products[$productId] = $product;
$productIdsNotAlreadyCached = array_diff($productIds, array_keys($this->products));
if (! empty($productIdsNotAlreadyCached)) {
$this->setProducts($productIdsNotAlreadyCached, $storeId);
}

$productIdsNotAlreadyCachedForThisStore = array_filter($productIds, function ($productId) use ($storeId) {
return !isset($this->products[$productId][$storeId]);
});
if (! empty($productIdsNotAlreadyCachedForThisStore)) {
$this->setProducts($productIdsNotAlreadyCachedForThisStore, $storeId);
}

$productsToReturn = [];
foreach ($productIds as $productId) {
if (! isset($this->products[$productId][$storeId])) {
continue;
}
$productsToReturn[] = $this->products[$productId][$storeId];
}

return $productsToReturn;
}

/**
* Set products.
*
* Load a product by id and store it to prevent repeat loading of the same entity.
* If we don't find a match for a product id, set it to null to prevent repeat attempts
* to load the missing product.
*
* @param array $productIds
* @param int $storeId
*
* @return void
*/
private function setProducts(array $productIds, int $storeId)
{
$productsCollection = $this->catalogCollectionFactory->create()
->addStoreFilter($storeId)
->addIdFilter($productIds)
->addAttributeToSelect('*')
->load();

foreach ($productIds as $productId) {
$this->products[$productId][$storeId] = $productsCollection->getItemById($productId) ?? null;
}
return $this->products[$productId];
}
}
65 changes: 40 additions & 25 deletions Test/Unit/Model/Connector/ContactData/ProductLoaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,19 @@

namespace Dotdigitalgroup\Email\Test\Unit\Model\Connector\ContactData;

use Magento\Catalog\Api\Data\ProductInterfaceFactory;
use ArrayIterator;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\ResourceModel\Product as ProductResource;
use Magento\Catalog\Model\ResourceModel\Product\CollectionFactory as CatalogCollectionFactory;
use Dotdigitalgroup\Email\Model\Connector\ContactData\ProductLoader;
use PHPUnit\Framework\MockObject\MockObject;
use PHPUnit\Framework\TestCase;

class ProductLoaderTest extends TestCase
{
/**
* @var ProductInterfaceFactory|MockObject
* @var CatalogCollectionFactory|MockObject
*/
private $productFactoryMock;

/**
* @var ProductResource|MockObject
*/
private $productResourceMock;
private $catalogCollectionFactoryMock;

/**
* @var ProductLoader
Expand All @@ -30,20 +25,16 @@ class ProductLoaderTest extends TestCase

protected function setUp(): void
{
$this->productFactoryMock = $this->getMockBuilder(ProductInterfaceFactory::class)
->disableOriginalConstructor()
->getMock();
$this->productResourceMock = $this->getMockBuilder(ProductResource::class)
$this->catalogCollectionFactoryMock = $this->getMockBuilder(CatalogCollectionFactory::class)
->disableOriginalConstructor()
->getMock();

$this->productLoader = new ProductLoader(
$this->productFactoryMock,
$this->productResourceMock
$this->catalogCollectionFactoryMock
);
}

public function testGetProduct(): void
public function testGetCachedProductById(): void
{
$productId = 1;
$storeId = 1;
Expand All @@ -52,23 +43,47 @@ public function testGetProduct(): void
->disableOriginalConstructor()
->getMock();

$this->productFactoryMock->expects($this->once())
$collectionMock = $this->getMockBuilder(\Magento\Catalog\Model\ResourceModel\Product\Collection::class)
->disableOriginalConstructor()
->getMock();

$this->catalogCollectionFactoryMock->expects($this->once())
->method('create')
->willReturn($productMock);
->willReturn($collectionMock);

$collectionMock->expects($this->once())
->method('addStoreFilter')
->with($storeId)
->willReturn($collectionMock);

$collectionMock->expects($this->once())
->method('addIdFilter')
->with([$productId])
->willReturn($collectionMock);

$productMock->expects($this->once())
->method('setStoreId')
->with($storeId);
$collectionMock->expects($this->once())
->method('addAttributeToSelect')
->with('*')
->willReturn($collectionMock);

$this->productResourceMock->expects($this->once())
$collectionMock->expects($this->once())
->method('load')
->with($productMock, $productId);
->willReturn($collectionMock);

$collectionMock->expects($this->any())
->method('getIterator')
->willReturn(new ArrayIterator([$productMock]));

$collectionMock->expects($this->once())
->method('getItemById')
->with($productId)
->willReturn($productMock);

$result = $this->productLoader->getProduct($productId, $storeId);
$result = $this->productLoader->getCachedProductById($productId, $storeId);
$this->assertSame($productMock, $result);

// Test caching of loaded product
$resultCached = $this->productLoader->getProduct($productId, $storeId);
$resultCached = $this->productLoader->getCachedProductById($productId, $storeId);
$this->assertSame($productMock, $resultCached);
}
}

0 comments on commit ee461c7

Please sign in to comment.