MAGETWO-99236 Fix for #22063 Category data is changed after saving category linkfor different stores

app/code/Magento/Catalog/Model/CategoryLinkRepository.php

@@ -26,6 +26,8 @@ class CategoryLinkRepository implements CategoryLinkRepositoryInterface, Categor
     protected $categoryRepository;
 
     /**
+     * @deprecated
+     * @see Product use faster resource model
      * @var ProductRepositoryInterface
      */
     protected $productRepository;
@@ -56,17 +58,17 @@ public function __construct(
     public function save(\Magento\Catalog\Api\Data\CategoryProductLinkInterface $productLink)
     {
         $category = $this->categoryRepository->get($productLink->getCategoryId());
-        $product = $this->productRepository->get($productLink->getSku());
+        $productId = $this->productResource->getIdBySku($productLink->getSku());
         $productPositions = $category->getProductsPosition();
-        $productPositions[$product->getId()] = $productLink->getPosition();
+        $productPositions[$productId] = $productLink->getPosition();
         $category->setPostedProducts($productPositions);
         try {
             $category->save();
         } catch (\Exception $e) {
             throw new CouldNotSaveException(
                 __(
                     'Could not save product "%1" with position %2 to category %3',
-                    $product->getId(),
+                    $productId,
                     $productLink->getPosition(),
                     $category->getId()
                 ),
@@ -90,15 +92,14 @@ public function delete(\Magento\Catalog\Api\Data\CategoryProductLinkInterface $p
     public function deleteByIds($categoryId, $sku)
     {
         $category = $this->categoryRepository->get($categoryId);
-        $product = $this->productRepository->get($sku);
+        $productId = $this->productResource->getIdBySku($sku);
         $productPositions = $category->getProductsPosition();
 
-        $productID = $product->getId();
-        if (!isset($productPositions[$productID])) {
+        if (!isset($productPositions[$productId])) {
             throw new InputException(__("The category doesn't contain the specified product."));
         }
-        $backupPosition = $productPositions[$productID];
-        unset($productPositions[$productID]);
+        $backupPosition = $productPositions[$productId];
+        unset($productPositions[$productId]);
 
         $category->setPostedProducts($productPositions);
         try {
@@ -108,7 +109,7 @@ public function deleteByIds($categoryId, $sku)
                 __(
                     'Could not save product "%product" with position %position to category %category',
                     [
-                        "product" => $product->getId(),
+                        "product" => $productId,
                         "position" => $backupPosition,
                         "category" => $category->getId()
                     ]

app/code/Magento/Catalog/Model/CategoryRepository.php

@@ -56,14 +56,14 @@ class CategoryRepository implements CategoryRepositoryInterface, ResetAfterReque
      * @var ExtensibleDataObjectConverter
      */
     private $extensibleDataObjectConverter;
-
+    // @codingStandardsIgnoreStart
     /**
-     * List of fields that can used config values in case when value does not defined directly
+     * List of fields that can use config values in case when value does not defined directly
      *
      * @var array
      */
     protected $useConfigFields = ['available_sort_by', 'default_sort_by', 'filter_price_range'];
-
+    // @codingStandardsIgnoreEnd
     /**
      * @var PopulateWithValues
      */
@@ -149,7 +149,7 @@ public function save(CategoryInterface $category)
      */
     public function get($categoryId, $storeId = null)
     {
-        $cacheKey = $storeId ?? 'all';
+        $cacheKey = $storeId ?? $this->storeManager->getStore()->getId();
         if (!isset($this->instances[$categoryId][$cacheKey])) {
             /** @var Category $category */
             $category = $this->categoryFactory->create();

app/code/Magento/Catalog/Model/ResourceModel/Category.php

@@ -37,8 +37,6 @@ class Category extends AbstractResource implements ResetAfterRequestInterface
     protected $_tree;
 
     /**
-     * Catalog products table name
-     *
      * @var string
      */
     protected $_categoryProductTable;
@@ -49,15 +47,11 @@ class Category extends AbstractResource implements ResetAfterRequestInterface
     private $entitiesWhereAttributesIs;
 
     /**
-     * Id of 'is_active' category attribute
-     *
      * @var int
      */
     protected $_isActiveAttributeId = null;
 
     /**
-     * Id of store
-     *
      * @var int
      */
     protected $_storeId = null;
@@ -455,7 +449,7 @@ protected function _saveCategoryProducts($category)
                     'position' => (int)$position,
                 ];
             }
-            $connection->insertMultiple($this->getCategoryProductTable(), $data);
+            $connection->insertOnDuplicate($this->getCategoryProductTable(), $data, ['position']);
         }
 
         /**

app/code/Magento/Catalog/Test/Unit/Model/CategoryLinkRepositoryTest.php

@@ -12,7 +12,6 @@
 use Magento\Catalog\Api\ProductRepositoryInterface;
 use Magento\Catalog\Model\Category;
 use Magento\Catalog\Model\CategoryLinkRepository;
-use Magento\Catalog\Model\Product as ProductModel;
 use Magento\Catalog\Model\ResourceModel\Product;
 use Magento\Framework\Exception\CouldNotSaveException;
 use Magento\Framework\Exception\InputException;
@@ -37,6 +36,8 @@ class CategoryLinkRepositoryTest extends TestCase
     private $categoryRepositoryMock;
 
     /**
+     * @deprecated
+     * @see \Magento\Catalog\Model\CategoryLinkRepository
      * @var ProductRepositoryInterface|MockObject
      */
     private $productRepositoryMock;
@@ -58,7 +59,7 @@ protected function setUp(): void
     {
         $this->productResourceMock = $this->getMockBuilder(Product::class)
             ->disableOriginalConstructor()
-            ->onlyMethods(['getProductsIdsBySkus'])
+            ->onlyMethods(['getProductsIdsBySkus', 'getIdBySku'])
             ->getMock();
         $this->categoryRepositoryMock = $this->getMockForAbstractClass(CategoryRepositoryInterface::class);
         $this->productRepositoryMock = $this->getMockForAbstractClass(ProductRepositoryInterface::class);
@@ -87,14 +88,12 @@ public function testSave(): void
             ->onlyMethods(['getProductsPosition', 'save'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->productLinkMock->expects($this->once())->method('getCategoryId')->willReturn($categoryId);
         $this->productLinkMock->expects($this->once())->method('getSku')->willReturn($sku);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($sku)->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn([]);
-        $productMock->expects($this->once())->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $this->productLinkMock->expects($this->once())->method('getPosition')->willReturn($productPosition);
         $categoryMock->expects($this->once())->method('setPostedProducts')->with($productPositions);
         $categoryMock->expects($this->once())->method('save');
@@ -119,14 +118,12 @@ public function testSaveWithCouldNotSaveException(): void
             ->onlyMethods(['getProductsPosition', 'save', 'getId'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->productLinkMock->expects($this->once())->method('getCategoryId')->willReturn($categoryId);
         $this->productLinkMock->expects($this->once())->method('getSku')->willReturn($sku);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($sku)->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn([]);
-        $productMock->expects($this->exactly(2))->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $this->productLinkMock->expects($this->exactly(2))->method('getPosition')->willReturn($productPosition);
         $categoryMock->expects($this->once())->method('setPostedProducts')->with($productPositions);
         $categoryMock->expects($this->once())->method('getId')->willReturn($categoryId);
@@ -153,13 +150,10 @@ public function testDeleteByIds(): void
             ->onlyMethods(['getProductsPosition', 'save', 'getId'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($productSku)
-            ->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn($productPositions);
-        $productMock->expects($this->once())->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $categoryMock->expects($this->once())->method('setPostedProducts')->with([]);
         $categoryMock->expects($this->once())->method('save');
 
@@ -182,13 +176,10 @@ public function testDeleteByIdsWithCouldNotSaveException(): void
             ->onlyMethods(['getProductsPosition', 'save', 'getId'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($productSku)
-            ->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn($productPositions);
-        $productMock->expects($this->exactly(2))->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $categoryMock->expects($this->once())->method('setPostedProducts')->with([]);
         $categoryMock->expects($this->once())->method('getId')->willReturn($categoryId);
         $categoryMock->expects($this->once())->method('save')->willThrowException(new \Exception());
@@ -216,13 +207,10 @@ public function testDeleteWithInputException(): void
             ->onlyMethods(['getProductsPosition', 'save', 'getId'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($productSku)
-            ->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn($productPositions);
-        $productMock->expects($this->once())->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $categoryMock->expects($this->never())->method('save');
 
         $this->expectExceptionMessage('The category doesn\'t contain the specified product.');
@@ -248,13 +236,10 @@ public function testDelete(): void
             ->onlyMethods(['getProductsPosition', 'save', 'getId'])
             ->disableOriginalConstructor()
             ->getMock();
-        $productMock = $this->createMock(ProductModel::class);
         $this->categoryRepositoryMock->expects($this->once())->method('get')->with($categoryId)
             ->willReturn($categoryMock);
-        $this->productRepositoryMock->expects($this->once())->method('get')->with($productSku)
-            ->willReturn($productMock);
         $categoryMock->expects($this->once())->method('getProductsPosition')->willReturn($productPositions);
-        $productMock->expects($this->once())->method('getId')->willReturn($productId);
+        $this->productResourceMock->expects($this->once())->method('getIdBySku')->willReturn($productId);
         $categoryMock->expects($this->once())->method('setPostedProducts')->with([]);
         $categoryMock->expects($this->once())->method('save');
 

dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Category/Save/UpdateCategoryTest.php

@@ -51,7 +51,13 @@ public function testUpdateCategoryForDefaultStoreView(array $postData): void
         $postData = array_merge($postData, ['store_id' => $storeId]);
         $responseData = $this->performSaveCategoryRequest($postData);
         $this->assertRequestIsSuccessfullyPerformed($responseData);
-        $category = $this->categoryRepository->get($postData['entity_id'], $postData['store_id']);
+        /**
+         * The problem is that
+         * @see \Magento\Catalog\Controller\Adminhtml\Category\Save::execute
+         * saves the category via resource model, so the cache in the repository is not cleaned
+         */
+        $newCategoryRepository = $this->_objectManager->create(CategoryRepositoryInterface::class);
+        $category = $newCategoryRepository->get($postData['entity_id'], $postData['store_id']);
         unset($postData['use_default']);
         unset($postData['use_config']);
         foreach ($postData as $key => $value) {

dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/CategoryTest.php

@@ -177,7 +177,13 @@ public function testDefaultValueForCategoryUrlPath(): void
             $this->equalTo([(string)__('You saved the category.')]),
             MessageInterface::TYPE_SUCCESS
         );
-        $category = $this->categoryRepository->get($categoryId);
+        /**
+         * The problem is that
+         * @see \Magento\Catalog\Controller\Adminhtml\Category\Save::execute
+         * saves the category via resource model, so the cache in the repository is not cleaned
+         */
+        $newCategoryRepository = $this->_objectManager->create(CategoryRepositoryInterface::class);
+        $category = $newCategoryRepository->get($categoryId);
         $this->assertEquals($defaultUrlPath, $category->getData('url_key'));
     }
 

dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryLinkRepositoryTest.php

@@ -0,0 +1,100 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Catalog\Model;
+
+use Magento\Catalog\Api\CategoryLinkRepositoryInterface;
+use Magento\Catalog\Api\CategoryRepositoryInterface;
+use Magento\Catalog\Api\Data\CategoryProductLinkInterface;
+use Magento\Framework\App\Area;
+use Magento\Store\Api\StoreRepositoryInterface;
+use Magento\Store\Model\App\Emulation;
+use Magento\Store\Model\Store;
+use Magento\TestFramework\Helper\Bootstrap;
+use Magento\TestFramework\ObjectManager;
+use PHPUnit\Framework\TestCase;
+
+class CategoryLinkRepositoryTest extends TestCase
+{
+    /**
+     * @var ObjectManager
+     */
+    private $objectManager;
+
+    /**
+     * @var CategoryLinkRepositoryInterface
+     */
+    private $categoryLinkRepository;
+
+    /**
+     * @var CategoryRepositoryInterface
+     */
+    private $categoryRepository;
+
+    /**
+     * @var StoreRepositoryInterface
+     */
+    private $storeRepository;
+
+    /**
+     * @var Emulation
+     */
+    private $appEmulation;
+
+    protected function setUp(): void
+    {
+        $this->objectManager = Bootstrap::getObjectManager();
+        $this->categoryLinkRepository = $this->objectManager->get(CategoryLinkRepositoryInterface::class);
+        $this->storeRepository = $this->objectManager->get(StoreRepositoryInterface::class);
+        $this->categoryRepository = $this->objectManager->get(CategoryRepositoryInterface::class);
+        $this->appEmulation = $this->objectManager->get(Emulation::class);
+        parent::setUp();
+    }
+
+    /**
+     * Make sure that if some custom code uses emulation and assigns products to categories,
+     * the categories are not overwritten by loading data via category repository from wrong store.
+     * The default store is used in
+     * @see \Magento\Catalog\Model\CategoryLinkRepository::save
+     *
+     * @magentoAppArea crontab
+     * @magentoDbIsolation disabled
+     * @magentoDataFixture Magento/Catalog/_files/saving_product_category_link_under_different_stores.php
+     */
+    public function testCategoryDataNotChanged()
+    {
+        $secondStore = $this->storeRepository->get('fixture_second_store');
+        $secondStoreId = (int)$secondStore->getId();
+        /** @var CategoryProductLinkInterface $categoryProductLink */
+        $categoryProductLink = $this->objectManager->create(CategoryProductLinkInterface::class);
+
+        $this->appEmulation->startEnvironmentEmulation($secondStoreId, Area::AREA_FRONTEND, true);
+
+        $categoryProductLink
+            ->setCategoryId(113)
+            ->setSku('simple116')
+            ->setPosition(2);
+
+        $this->categoryLinkRepository->save($categoryProductLink);
+
+        $this->appEmulation->stopEnvironmentEmulation();
+
+        $categoryFromDefaultStore = $this->categoryRepository->get(113);
+        $categoryFromSecondStore = $this->categoryRepository->get(113, $secondStoreId);
+
+        $categoryNameFromDefaultStore = $categoryFromDefaultStore->getName();
+        $categoryDescriptionFromDefaultStore = $categoryFromDefaultStore->getDescription();
+
+        $categoryNameFromSecondStore = $categoryFromSecondStore->getName();
+        $categoryDescriptionFromSecondStore = $categoryFromSecondStore->getDescription();
+
+        $this->assertEquals('Test Category In Default Store', $categoryNameFromDefaultStore);
+        $this->assertEquals('Test description in default store', $categoryDescriptionFromDefaultStore);
+        $this->assertEquals('Test Category In Second Store', $categoryNameFromSecondStore);
+        $this->assertEquals('Test description in second store', $categoryDescriptionFromSecondStore);
+    }
+}

dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryTest.php

@@ -376,8 +376,13 @@ public function testDeleteChildren(): void
      */
     public function testChildrenCountAfterDeleteParentCategory(): void
     {
+        $childrenCountInParent = $this->categoryResource->getChildrenCount(1);
+        $childrenCountInChild = $this->categoryResource->getChildrenCount(3);
+
         $this->categoryRepository->deleteByIdentifier(3);
-        $this->assertEquals(8, $this->categoryResource->getChildrenCount(1));
+
+        $expectedCountAfterDelete = $childrenCountInParent - $childrenCountInChild - 1;
+        $this->assertEquals($expectedCountAfterDelete, $this->categoryResource->getChildrenCount(1));
     }
 
     /**