MC-38593: Complex products children ids are not cached in Type class leading to multiple unnecessary db requests

Author: engcom-Echo

app/code/Magento/Bundle/Model/Plugin/Frontend/Product.php renamed to app/code/Magento/Bundle/Model/Plugin/Frontend/ProductIdentitiesExtender.php

@@ -7,23 +7,23 @@
 
 namespace Magento\Bundle\Model\Plugin\Frontend;
 
-use Magento\Bundle\Model\Product\Type;
+use Magento\Bundle\Model\Product\Type as BundleType;
 use Magento\Catalog\Model\Product as CatalogProduct;
 
 /**
  * Add child identities to product identities on storefront.
  */
-class Product
+class ProductIdentitiesExtender
 {
     /**
-     * @var Type
+     * @var BundleType
      */
     private $type;
 
     /**
-     * @param Type $type
+     * @param BundleType $type
      */
-    public function __construct(Type $type)
+    public function __construct(BundleType $type)
     {
         $this->type = $type;
     }
@@ -37,12 +37,15 @@ public function __construct(Type $type)
      */
     public function afterGetIdentities(CatalogProduct $product, array $identities): array
     {
+        if ($product->getTypeId() !== BundleType::TYPE_CODE) {
+
+            return $identities;
+        }
         foreach ($this->type->getChildrenIds($product->getEntityId()) as $childIds) {
             foreach ($childIds as $childId) {
                 $identities[] = CatalogProduct::CACHE_TAG . '_' . $childId;
             }
         }
-
         return array_unique($identities);
     }
 }

app/code/Magento/Bundle/Model/Product/Type.php

@@ -168,6 +168,11 @@ class Type extends \Magento\Catalog\Model\Product\Type\AbstractType
      */
     private $arrayUtility;
 
+    /**
+     * @var array
+     */
+    private $cacheChildrenIds = [];
+
     /**
      * @param \Magento\Catalog\Model\Product\Option $catalogProductOption
      * @param \Magento\Eav\Model\Config $eavConfig
@@ -285,7 +290,12 @@ public function getRelationInfo()
      */
     public function getChildrenIds($parentId, $required = true)
     {
-        return $this->_bundleSelection->getChildrenIds($parentId, $required);
+        $cacheKey = $parentId . '-' . ($required ? 1 : 0);
+        if (!isset($this->cacheChildrenIds[$cacheKey])) {
+            $this->cacheChildrenIds[$cacheKey] = $this->_bundleSelection->getChildrenIds($parentId, $required);
+        }
+
+        return $this->cacheChildrenIds[$cacheKey];
     }
 
     /**

app/code/Magento/Bundle/Test/Unit/Model/Plugin/Frontend/ProductTest.php

@@ -7,7 +7,7 @@
 
 namespace Magento\Bundle\Test\Unit\Model\Plugin\Frontend;
 
-use Magento\Bundle\Model\Plugin\Frontend\Product as ProductPlugin;
+use Magento\Bundle\Model\Plugin\Frontend\ProductIdentitiesExtender as ProductPlugin;
 use Magento\Bundle\Model\Product\Type;
 use Magento\Catalog\Model\Product;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -28,7 +28,7 @@ protected function setUp(): void
     {
         $this->product = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods(['getEntityId'])
+            ->setMethods(['getEntityId', 'getTypeId'])
             ->getMock();
 
         $this->type = $this->getMockBuilder(Type::class)
@@ -65,6 +65,9 @@ public function testAfterGetIdentities()
         $this->product->expects($this->once())
             ->method('getEntityId')
             ->willReturn($id);
+        $this->product->expects($this->once())
+            ->method('getTypeId')
+            ->willReturn(Type::TYPE_CODE);
         $this->type->expects($this->once())
             ->method('getChildrenIds')
             ->with($id)

app/code/Magento/Bundle/Test/Unit/Model/Product/TypeTest.php

@@ -11,6 +11,7 @@
 use Magento\Bundle\Model\Product\Type;
 use Magento\Bundle\Model\ResourceModel\BundleFactory;
 use Magento\Bundle\Model\ResourceModel\Option\Collection;
+use Magento\Bundle\Model\ResourceModel\Selection as ResourceSelection;
 use Magento\CatalogRule\Model\ResourceModel\Product\CollectionProcessor;
 use Magento\Bundle\Model\ResourceModel\Selection\Collection as SelectionCollection;
 use Magento\Bundle\Model\ResourceModel\Selection\CollectionFactory;
@@ -45,6 +46,7 @@
 /**
  * Test for bundle product type
  *
+ * @SuppressWarnings(PHPMD.TooManyFields)
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
 class TypeTest extends TestCase
@@ -124,6 +126,11 @@ class TypeTest extends TestCase
      */
     private $catalogRuleProcessor;
 
+    /**
+     * @var ResourceSelection|MockObject
+     */
+    private $bundleSelection;
+
     /**
      * @return void
      */
@@ -173,7 +180,7 @@ protected function setUp(): void
             ->disableOriginalConstructor()
             ->getMockForAbstractClass();
         $this->bundleModelSelection = $this->getMockBuilder(SelectionFactory::class)
-            ->setMethods(['create'])
+            ->setMethods(['create', 'getChildrenIds'])
             ->disableOriginalConstructor()
             ->getMock();
         $this->bundleFactory = $this->getMockBuilder(BundleFactory::class)
@@ -194,13 +201,18 @@ protected function setUp(): void
         $this->catalogRuleProcessor = $this->getMockBuilder(CollectionProcessor::class)
             ->disableOriginalConstructor()
             ->getMock();
+        $this->bundleSelection = $this->getMockBuilder(ResourceSelection::class)
+            ->onlyMethods(['getChildrenIds'])
+            ->disableOriginalConstructor()
+            ->getMock();
 
         $objectHelper = new ObjectManager($this);
         $this->model = $objectHelper->getObject(
             Type::class,
             [
                 'bundleModelSelection' => $this->bundleModelSelection,
                 'bundleFactory' => $this->bundleFactory,
+                'bundleSelection' => $this->bundleSelection,
                 'bundleCollection' => $this->bundleCollectionFactory,
                 'bundleOption' => $this->bundleOptionFactory,
                 'catalogData' => $this->catalogData,
@@ -216,6 +228,34 @@ protected function setUp(): void
         );
     }
 
+    /**
+     * @return void
+     */
+    public function testGetChildrenIds()
+    {
+        $this->assertClassHasAttribute('cacheChildrenIds', Type::class);
+
+        $parentId = 1;
+        $childrenIds = [
+            [
+                1 => [
+                    26 => "26",
+                    39 => "39",
+                ],
+            ],
+        ];
+
+        $this->bundleSelection->expects($this->once())
+            ->method('getChildrenIds')
+            ->with($parentId, true)
+            ->willReturn($childrenIds);
+        $this->assertIsArray($this->model->getChildrenIds($parentId, true));
+
+        $this->bundleSelection->expects($this->never())
+            ->method('getChildrenIds');
+        $this->assertEquals($childrenIds, $this->model->getChildrenIds($parentId, true));
+    }
+
     /**
      * @return void
      * @SuppressWarnings(PHPMD.ExcessiveMethodLength)

app/code/Magento/Bundle/etc/frontend/di.xml

@@ -14,6 +14,6 @@
         </arguments>
     </type>
     <type name="Magento\Catalog\Model\Product">
-        <plugin name="bundle" type="Magento\Bundle\Model\Plugin\Frontend\Product" sortOrder="100" />
+        <plugin name="add_child_identities_bundle" type="Magento\Bundle\Model\Plugin\Frontend\ProductIdentitiesExtender"/>
     </type>
 </config>

dev/tests/integration/testsuite/Magento/Bundle/Model/Plugin/Frontend/ProductTest.php

@@ -28,7 +28,10 @@ public function testProductIsRegistered(): void
     {
         $pluginInfo = Bootstrap::getObjectManager()->get(PluginList::class)
             ->get(\Magento\Catalog\Model\Product::class, []);
-        $this->assertSame(Product::class, $pluginInfo['bundle']['instance']);
+        $this->assertSame(
+            ProductIdentitiesExtender::class,
+            $pluginInfo['add_child_identities_bundle']['instance']
+        );
     }
 
     /**