Merge remote-tracking branch 'origin/MC-32411' into 2.4-develop-pr21

app/code/Magento/ConfigurableProduct/Model/Plugin/ProductRepositorySave.php

@@ -5,15 +5,17 @@
  */
 namespace Magento\ConfigurableProduct\Model\Plugin;
 
-use Magento\Catalog\Model\ProductFactory;
 use Magento\Catalog\Api\Data\ProductInterface;
 use Magento\Framework\Exception\InputException;
 use Magento\Catalog\Api\ProductRepositoryInterface;
-use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\ConfigurableProduct\Api\Data\OptionInterface;
 use Magento\Catalog\Api\ProductAttributeRepositoryInterface;
 use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
+use Magento\Framework\Exception\NoSuchEntityException;
 
+/**
+ * Plugin to validate product links of configurable product and reset configurable attributes
+ */
 class ProductRepositorySave
 {
     /**
@@ -22,46 +24,45 @@ class ProductRepositorySave
     private $productAttributeRepository;
 
     /**
-     * @var ProductFactory
+     * @var ProductRepositoryInterface
      */
-    private $productFactory;
+    private $productRepository;
 
     /**
      * @param ProductAttributeRepositoryInterface $productAttributeRepository
-     * @param ProductFactory $productFactory
+     * @param ProductRepositoryInterface $productRepository
      */
     public function __construct(
         ProductAttributeRepositoryInterface $productAttributeRepository,
-        ProductFactory $productFactory
+        ProductRepositoryInterface $productRepository
     ) {
         $this->productAttributeRepository = $productAttributeRepository;
-        $this->productFactory = $productFactory;
+        $this->productRepository = $productRepository;
     }
 
     /**
-     * Validate product links and reset configurable attributes to configurable product
+     * Validate product links of configurable product
      *
      * @param ProductRepositoryInterface $subject
-     * @param ProductInterface $result
      * @param ProductInterface $product
      * @param bool $saveOptions
-     * @return ProductInterface
-     * @throws CouldNotSaveException
+     * @return array
      * @throws InputException
+     * @throws NoSuchEntityException
      *
      * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function afterSave(
+    public function beforeSave(
         ProductRepositoryInterface $subject,
-        ProductInterface $result,
         ProductInterface $product,
         $saveOptions = false
     ) {
+        $result[] = $product;
         if ($product->getTypeId() !== Configurable::TYPE_CODE) {
             return $result;
         }
 
-        $extensionAttributes = $result->getExtensionAttributes();
+        $extensionAttributes = $product->getExtensionAttributes();
         if ($extensionAttributes === null) {
             return $result;
         }
@@ -81,23 +82,49 @@ public function afterSave(
             $attributeCodes[] = $attributeCode;
         }
         $this->validateProductLinks($attributeCodes, $configurableLinks);
+
+        return $result;
+    }
+
+    /**
+     * Reset configurable attributes to configurable product
+     *
+     * @param ProductRepositoryInterface $subject
+     * @param ProductInterface $result
+     * @param ProductInterface $product
+     * @param bool $saveOptions
+     * @return ProductInterface
+     *
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     */
+    public function afterSave(
+        ProductRepositoryInterface $subject,
+        ProductInterface $result,
+        ProductInterface $product,
+        $saveOptions = false
+    ) {
+        if ($product->getTypeId() !== Configurable::TYPE_CODE) {
+            return $result;
+        }
         $result->getTypeInstance()->resetConfigurableAttributes($product);
 
         return $result;
     }
 
     /**
+     * Validate product links
+     *
      * @param array $attributeCodes
      * @param array $linkIds
-     * @return $this
+     * @return void
      * @throws InputException
+     * @throws NoSuchEntityException
      */
     private function validateProductLinks(array $attributeCodes, array $linkIds)
     {
         $valueMap = [];
-
         foreach ($linkIds as $productId) {
-            $variation = $this->productFactory->create()->load($productId);
+            $variation = $this->productRepository->getById($productId);
             $valueKey = '';
             foreach ($attributeCodes as $attributeCode) {
                 if (!$variation->getData($attributeCode)) {

app/code/Magento/ConfigurableProduct/Test/Unit/Model/Plugin/ProductRepositorySaveTest.php

@@ -9,30 +9,26 @@
 use Magento\Catalog\Api\ProductAttributeRepositoryInterface;
 use Magento\Catalog\Api\ProductRepositoryInterface;
 use Magento\Catalog\Model\Product;
-use Magento\Catalog\Model\ProductFactory;
 use Magento\ConfigurableProduct\Api\Data\OptionInterface;
 use Magento\ConfigurableProduct\Model\Plugin\ProductRepositorySave;
 use Magento\ConfigurableProduct\Model\Product\Type\Configurable;
 use Magento\ConfigurableProduct\Test\Unit\Model\Product\ProductExtensionAttributes;
 use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
-use PHPUnit_Framework_MockObject_MockObject as MockObject;
+use PHPUnit\Framework\MockObject\MockObject;
+use PHPUnit\Framework\TestCase;
 
 /**
- * Class ProductRepositorySaveTest
+ * Test for ProductRepositorySave plugin
+ *
  * @SuppressWarnings(PHPMD.CouplingBetweenObjects)
  */
-class ProductRepositorySaveTest extends \PHPUnit\Framework\TestCase
+class ProductRepositorySaveTest extends TestCase
 {
     /**
      * @var ProductAttributeRepositoryInterface|MockObject
      */
     private $productAttributeRepository;
 
-    /**
-     * @var ProductFactory|MockObject
-     */
-    private $productFactory;
-
     /**
      * @var Product|MockObject
      */
@@ -68,15 +64,13 @@ class ProductRepositorySaveTest extends \PHPUnit\Framework\TestCase
      */
     private $plugin;
 
+    /**
+     * @inheritdoc
+     */
     protected function setUp()
     {
         $this->productAttributeRepository = $this->getMockForAbstractClass(ProductAttributeRepositoryInterface::class);
 
-        $this->productFactory = $this->getMockBuilder(ProductFactory::class)
-            ->disableOriginalConstructor()
-            ->setMethods(['create'])
-            ->getMock();
-
         $this->product = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
             ->setMethods(['getTypeId', 'getExtensionAttributes'])
@@ -102,12 +96,15 @@ protected function setUp()
             ProductRepositorySave::class,
             [
                 'productAttributeRepository' => $this->productAttributeRepository,
-                'productFactory' => $this->productFactory
+                'productRepository' => $this->productRepository
             ]
         );
     }
 
-    public function testAfterSaveWhenProductIsSimple()
+    /**
+     * Validating the result after saving a configurable product
+     */
+    public function testBeforeSaveWhenProductIsSimple()
     {
         $this->product->expects(static::once())
             ->method('getTypeId')
@@ -116,18 +113,21 @@ public function testAfterSaveWhenProductIsSimple()
             ->method('getExtensionAttributes');
 
         $this->assertEquals(
-            $this->result,
-            $this->plugin->afterSave($this->productRepository, $this->result, $this->product)
+            $this->product,
+            $this->plugin->beforeSave($this->productRepository, $this->product)[0]
         );
     }
 
-    public function testAfterSaveWithoutOptions()
+    /**
+     * Test saving a configurable product without attribute options
+     */
+    public function testBeforeSaveWithoutOptions()
     {
         $this->product->expects(static::once())
             ->method('getTypeId')
             ->willReturn(Configurable::TYPE_CODE);
 
-        $this->result->expects(static::once())
+        $this->product->expects(static::once())
             ->method('getExtensionAttributes')
             ->willReturn($this->extensionAttributes);
 
@@ -142,23 +142,25 @@ public function testAfterSaveWithoutOptions()
             ->method('get');
 
         $this->assertEquals(
-            $this->result,
-            $this->plugin->afterSave($this->productRepository, $this->result, $this->product)
+            $this->product,
+            $this->plugin->beforeSave($this->productRepository, $this->product)[0]
         );
     }
 
     /**
+     * Test saving a configurable product with same set of attribute values
+     *
      * @expectedException \Magento\Framework\Exception\InputException
      * @expectedExceptionMessage Products "5" and "4" have the same set of attribute values.
      */
-    public function testAfterSaveWithLinks()
+    public function testBeforeSaveWithLinks()
     {
         $links = [4, 5];
         $this->product->expects(static::once())
             ->method('getTypeId')
             ->willReturn(Configurable::TYPE_CODE);
 
-        $this->result->expects(static::once())
+        $this->product->expects(static::once())
             ->method('getExtensionAttributes')
             ->willReturn($this->extensionAttributes);
         $this->extensionAttributes->expects(static::once())
@@ -173,27 +175,26 @@ public function testAfterSaveWithLinks()
 
         $product = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods(['load', 'getData', '__wakeup'])
+            ->setMethods(['getData', '__wakeup'])
             ->getMock();
 
-        $this->productFactory->expects(static::exactly(2))
-            ->method('create')
+        $this->productRepository->expects(static::exactly(2))
+            ->method('getById')
             ->willReturn($product);
 
-        $product->expects(static::exactly(2))
-            ->method('load')
-            ->willReturnSelf();
         $product->expects(static::never())
             ->method('getData');
 
-        $this->plugin->afterSave($this->productRepository, $this->result, $this->product);
+        $this->plugin->beforeSave($this->productRepository, $this->product);
     }
 
     /**
+     * Test saving a configurable product with missing attribute
+     *
      * @expectedException \Magento\Framework\Exception\InputException
      * @expectedExceptionMessage Product with id "4" does not contain required attribute "color".
      */
-    public function testAfterSaveWithLinksWithMissingAttribute()
+    public function testBeforeSaveWithLinksWithMissingAttribute()
     {
         $simpleProductId = 4;
         $links = [$simpleProductId, 5];
@@ -208,7 +209,7 @@ public function testAfterSaveWithLinksWithMissingAttribute()
             ->method('getTypeId')
             ->willReturn(Configurable::TYPE_CODE);
 
-        $this->result->expects(static::once())
+        $this->product->expects(static::once())
             ->method('getExtensionAttributes')
             ->willReturn($this->extensionAttributes);
         $this->extensionAttributes->expects(static::once())
@@ -228,29 +229,28 @@ public function testAfterSaveWithLinksWithMissingAttribute()
 
         $product = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods(['load', 'getData', '__wakeup'])
+            ->setMethods(['getData', '__wakeup'])
             ->getMock();
 
-        $this->productFactory->expects(static::once())
-            ->method('create')
+        $this->productRepository->expects(static::once())
+            ->method('getById')
             ->willReturn($product);
-        $product->expects(static::once())
-            ->method('load')
-            ->with($simpleProductId)
-            ->willReturnSelf();
+
         $product->expects(static::once())
             ->method('getData')
             ->with($attributeCode)
             ->willReturn(false);
 
-        $this->plugin->afterSave($this->productRepository, $this->result, $this->product);
+        $this->plugin->beforeSave($this->productRepository, $this->product);
     }
 
     /**
+     * Test saving a configurable product with duplicate attributes
+     *
      * @expectedException \Magento\Framework\Exception\InputException
      * @expectedExceptionMessage Products "5" and "4" have the same set of attribute values.
      */
-    public function testAfterSaveWithLinksWithDuplicateAttributes()
+    public function testBeforeSaveWithLinksWithDuplicateAttributes()
     {
         $links = [4, 5];
         $attributeCode = 'color';
@@ -264,7 +264,7 @@ public function testAfterSaveWithLinksWithDuplicateAttributes()
             ->method('getTypeId')
             ->willReturn(Configurable::TYPE_CODE);
 
-        $this->result->expects(static::once())
+        $this->product->expects(static::once())
             ->method('getExtensionAttributes')
             ->willReturn($this->extensionAttributes);
         $this->extensionAttributes->expects(static::once())
@@ -284,20 +284,18 @@ public function testAfterSaveWithLinksWithDuplicateAttributes()
 
         $product = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->setMethods(['load', 'getData', '__wakeup'])
+            ->setMethods(['getData', '__wakeup'])
             ->getMock();
 
-        $this->productFactory->expects(static::exactly(2))
-            ->method('create')
+        $this->productRepository->expects(static::exactly(2))
+            ->method('getById')
             ->willReturn($product);
-        $product->expects(static::exactly(2))
-            ->method('load')
-            ->willReturnSelf();
+
         $product->expects(static::exactly(4))
             ->method('getData')
             ->with($attributeCode)
             ->willReturn($attributeId);
 
-        $this->plugin->afterSave($this->productRepository, $this->result, $this->product);
+        $this->plugin->beforeSave($this->productRepository, $this->product);
     }
 }