diff --git a/app/code/Magento/Catalog/Model/CategoryLinkRepository.php b/app/code/Magento/Catalog/Model/CategoryLinkRepository.php index f8de9a37f4ed7..9eb9e3a55ad26 100644 --- a/app/code/Magento/Catalog/Model/CategoryLinkRepository.php +++ b/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,9 +58,9 @@ 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(); @@ -66,7 +68,7 @@ public function save(\Magento\Catalog\Api\Data\CategoryProductLinkInterface $pro 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() ] diff --git a/app/code/Magento/Catalog/Model/CategoryRepository.php b/app/code/Magento/Catalog/Model/CategoryRepository.php index 4e85853cd33bc..dd69c78e7944b 100644 --- a/app/code/Magento/Catalog/Model/CategoryRepository.php +++ b/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(); diff --git a/app/code/Magento/Catalog/Model/ResourceModel/Category.php b/app/code/Magento/Catalog/Model/ResourceModel/Category.php index f56fecb8dc556..7ae3319173c31 100644 --- a/app/code/Magento/Catalog/Model/ResourceModel/Category.php +++ b/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']); } /** diff --git a/app/code/Magento/Catalog/Test/Unit/Model/CategoryLinkRepositoryTest.php b/app/code/Magento/Catalog/Test/Unit/Model/CategoryLinkRepositoryTest.php index 0f36a415c3f56..2dc0a734260a6 100644 --- a/app/code/Magento/Catalog/Test/Unit/Model/CategoryLinkRepositoryTest.php +++ b/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'); diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Category/Save/UpdateCategoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Category/Save/UpdateCategoryTest.php index 3cecc5f1a76c8..545b10abb873b 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/Category/Save/UpdateCategoryTest.php +++ b/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) { diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/CategoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/CategoryTest.php index a3c9eeb3226a6..0eb6e1d9d1245 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Controller/Adminhtml/CategoryTest.php +++ b/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')); } diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryLinkRepositoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryLinkRepositoryTest.php new file mode 100644 index 0000000000000..3d89db8508040 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryLinkRepositoryTest.php @@ -0,0 +1,100 @@ +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); + } +} diff --git a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryTest.php b/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryTest.php index ae6777102ef59..f8325a8290648 100644 --- a/dev/tests/integration/testsuite/Magento/Catalog/Model/CategoryTest.php +++ b/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)); } /** diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores.php new file mode 100644 index 0000000000000..0b99750a6e9f8 --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores.php @@ -0,0 +1,71 @@ +requireDataFixture('Magento/Store/_files/second_store.php'); + +$objectManager = Bootstrap::getObjectManager(); + +/** @var StoreRepositoryInterface $storeRepository */ +$storeRepository = $objectManager->get(StoreRepositoryInterface::class); +$secondStore = $storeRepository->get('fixture_second_store'); + +/** @var CategoryResource $categoryResource */ +$categoryResource = $objectManager->get(CategoryResource::class); + +/** @var ProductResource $productResource */ +$productResource = $objectManager->get(ProductResource::class); + +/** @var CategoryInterface|Category $category */ +$category = $objectManager->create(CategoryInterface::class); +$category->isObjectNew(true); +$category + ->setId(113) + ->setIsAnchor(true) + ->setStoreId(Store::DEFAULT_STORE_ID) + ->setName('Test Category In Default Store') + ->setDescription('Test description in default store') + ->setParentId(2) + ->setPath('1/2/113') + ->setLevel(2) + ->setAvailableSortBy('name') + ->setDefaultSortBy('name') + ->setIsActive(true); +$categoryResource->save($category); + +$category + ->setStoreId($secondStore->getId()) + ->setName('Test Category In Second Store') + ->setDescription('Test description in second store'); +$categoryResource->save($category); + +/** @var Product $product */ +$product = $objectManager->create(Product::class); +$product + ->setTypeId('simple') + ->setId(116) + ->setAttributeSetId(4) + ->setWebsiteIds([$secondStore->getWebsiteId()]) + ->setName('Simple Product116') + ->setSku('simple116') + ->setPrice(10) + ->setMetaTitle('meta title2') + ->setMetaKeyword('meta keyword2') + ->setMetaDescription('meta description2') + ->setVisibility(\Magento\Catalog\Model\Product\Visibility::VISIBILITY_BOTH) + ->setStatus(\Magento\Catalog\Model\Product\Attribute\Source\Status::STATUS_ENABLED) + ->setStockData(['use_config_manage_stock' => 0]); +$productResource->save($product); diff --git a/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores_rollback.php b/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores_rollback.php new file mode 100644 index 0000000000000..91c4c2217b73e --- /dev/null +++ b/dev/tests/integration/testsuite/Magento/Catalog/_files/saving_product_category_link_under_different_stores_rollback.php @@ -0,0 +1,38 @@ +get(CategoryRepositoryInterface::class); + +/** @var ProductRepositoryInterface $productRepository */ +$productRepository = $objectManager->get(ProductRepositoryInterface::class); + +/** @var Registry $registry */ +$registry = $objectManager->get(Registry::class); + +$isSecurePreviousValue = $registry->registry('isSecureArea'); + +$registry->unregister('isSecureArea'); +$registry->register('isSecureArea', true); + +$productRepository->deleteById('simple116'); +$productRepository->cleanCache(); + +$categoryRepository->deleteByIdentifier(113); + +Resolver::getInstance()->requireDataFixture('Magento/Store/_files/second_store_rollback.php'); + +$registry->unregister('isSecureArea'); +$registry->register('isSecureArea', $isSecurePreviousValue);