diff --git a/Model/Connector/ContactData.php b/Model/Connector/ContactData.php index 87a487e2..f483e5e8 100644 --- a/Model/Connector/ContactData.php +++ b/Model/Connector/ContactData.php @@ -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) { @@ -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); @@ -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)); @@ -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; diff --git a/Model/Connector/ContactData/ProductLoader.php b/Model/Connector/ContactData/ProductLoader.php index 75a583d9..a42b8d5d 100644 --- a/Model/Connector/ContactData/ProductLoader.php +++ b/Model/Connector/ContactData/ProductLoader.php @@ -4,21 +4,15 @@ 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 @@ -26,34 +20,86 @@ class ProductLoader 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]; } } diff --git a/Test/Unit/Model/Connector/ContactData/ProductLoaderTest.php b/Test/Unit/Model/Connector/ContactData/ProductLoaderTest.php index 010d97a4..10a87cc6 100644 --- a/Test/Unit/Model/Connector/ContactData/ProductLoaderTest.php +++ b/Test/Unit/Model/Connector/ContactData/ProductLoaderTest.php @@ -4,9 +4,9 @@ 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; @@ -14,14 +14,9 @@ class ProductLoaderTest extends TestCase { /** - * @var ProductInterfaceFactory|MockObject + * @var CatalogCollectionFactory|MockObject */ - private $productFactoryMock; - - /** - * @var ProductResource|MockObject - */ - private $productResourceMock; + private $catalogCollectionFactoryMock; /** * @var ProductLoader @@ -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; @@ -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); } }