Skip to content

Commit 70a3adf

Browse files
p-bystritskyp-bystritsky
committed
#22870: ProductRepository fails to update an existing product with a changed SKU.
1 parent 211dd25 commit 70a3adf

File tree

3 files changed

+84
-18
lines changed

3 files changed

+84
-18
lines changed

‎app/code/Magento/Catalog/Model/ProductRepository.php

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,11 @@ protected function initializeProductData(array $productData, $createNew)
369369
if ($createNew) {
370370
$product = $this->productFactory->create();
371371
$this->assignProductToWebsites($product);
372+
} elseif (!empty($productData['id'])) {
373+
$this->removeProductFromLocalCacheById($productData['id']);
374+
$product = $this->getById($productData['id']);
372375
} else {
373-
$this->removeProductFromLocalCache($productData['sku']);
376+
$this->removeProductFromLocalCacheBySku($productData['sku']);
374377
$product = $this->get($productData['sku']);
375378
}
376379

@@ -512,7 +515,7 @@ public function save(ProductInterface $product, $saveOptions = false)
512515
$tierPrices = $product->getData('tier_price');
513516

514517
try {
515-
$existingProduct = $this->get($product->getSku());
518+
$existingProduct = $product->getId() ? $this->getById($product->getId()) : $this->get($product->getSku());
516519

517520
$product->setData(
518521
$this->resourceModel->getLinkField(),
@@ -570,7 +573,7 @@ public function save(ProductInterface $product, $saveOptions = false)
570573
$product->getCategoryIds()
571574
);
572575
}
573-
$this->removeProductFromLocalCache($product->getSku());
576+
$this->removeProductFromLocalCacheBySku($product->getSku());
574577
unset($this->instancesById[$product->getId()]);
575578

576579
return $this->get($product->getSku(), false, $product->getStoreId());
@@ -584,7 +587,7 @@ public function delete(ProductInterface $product)
584587
$sku = $product->getSku();
585588
$productId = $product->getId();
586589
try {
587-
$this->removeProductFromLocalCache($product->getSku());
590+
$this->removeProductFromLocalCacheBySku($product->getSku());
588591
unset($this->instancesById[$product->getId()]);
589592
$this->resourceModel->delete($product);
590593
} catch (ValidatorException $e) {
@@ -595,7 +598,7 @@ public function delete(ProductInterface $product)
595598
$e
596599
);
597600
}
598-
$this->removeProductFromLocalCache($sku);
601+
$this->removeProductFromLocalCacheBySku($sku);
599602
unset($this->instancesById[$productId]);
600603

601604
return true;
@@ -753,25 +756,36 @@ private function getProductFromLocalCache(string $sku, string $cacheKey)
753756
}
754757

755758
/**
756-
* Removes product in the local cache.
759+
* Removes product in the local cache by sku.
757760
*
758761
* @param string $sku
759762
* @return void
760763
*/
761-
private function removeProductFromLocalCache(string $sku) :void
764+
private function removeProductFromLocalCacheBySku(string $sku): void
762765
{
763766
$preparedSku = $this->prepareSku($sku);
764767
unset($this->instances[$preparedSku]);
765768
}
766769

767770
/**
768-
* Saves product in the local cache.
771+
* Removes product in the local cache by id.
772+
*
773+
* @param string $id
774+
* @return void
775+
*/
776+
private function removeProductFromLocalCacheById(string $id): void
777+
{
778+
unset($this->instancesById[$id]);
779+
}
780+
781+
/**
782+
* Saves product in the local cache by sku.
769783
*
770784
* @param Product $product
771785
* @param string $cacheKey
772786
* @return void
773787
*/
774-
private function saveProductInLocalCache(Product $product, string $cacheKey) : void
788+
private function saveProductInLocalCache(Product $product, string $cacheKey): void
775789
{
776790
$preparedSku = $this->prepareSku($product->getSku());
777791
$this->instances[$preparedSku][$cacheKey] = $product;
@@ -800,7 +814,7 @@ private function prepareSku(string $sku): string
800814
private function saveProduct($product): void
801815
{
802816
try {
803-
$this->removeProductFromLocalCache($product->getSku());
817+
$this->removeProductFromLocalCacheBySku($product->getSku());
804818
unset($this->instancesById[$product->getId()]);
805819
$this->resourceModel->save($product);
806820
} catch (ConnectionException $exception) {

‎app/code/Magento/Catalog/Test/Unit/Model/ProductRepositoryTest.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -527,12 +527,14 @@ private function getProductMocksForReducedCache($productsCount)
527527
for ($i = 1; $i <= $productsCount; $i++) {
528528
$productMock = $this->getMockBuilder(Product::class)
529529
->disableOriginalConstructor()
530-
->setMethods([
530+
->setMethods(
531+
[
531532
'getId',
532533
'getSku',
533534
'load',
534535
'setData',
535-
])
536+
]
537+
)
536538
->getMock();
537539
$productMock->expects($this->once())->method('load');
538540
$productMock->expects($this->atLeastOnce())->method('getId')->willReturn($i);
@@ -679,7 +681,7 @@ public function testSaveException()
679681
->willReturn(true);
680682
$this->resourceModel->expects($this->once())->method('save')->with($this->product)
681683
->willThrowException(new \Magento\Eav\Model\Entity\Attribute\Exception(__('123')));
682-
$this->product->expects($this->once())->method('getId')->willReturn(null);
684+
$this->product->expects($this->exactly(2))->method('getId')->willReturn(null);
683685
$this->extensibleDataObjectConverter
684686
->expects($this->once())
685687
->method('toNestedArray')
@@ -703,7 +705,7 @@ public function testSaveInvalidProductException()
703705
$this->initializationHelper->expects($this->never())->method('initialize');
704706
$this->resourceModel->expects($this->once())->method('validate')->with($this->product)
705707
->willReturn(['error1', 'error2']);
706-
$this->product->expects($this->never())->method('getId');
708+
$this->product->expects($this->once())->method('getId')->willReturn(null);
707709
$this->extensibleDataObjectConverter
708710
->expects($this->once())
709711
->method('toNestedArray')
@@ -1340,11 +1342,13 @@ public function testSaveWithDifferentWebsites()
13401342
->willReturn($storeMock);
13411343
$this->storeManager->expects($this->once())
13421344
->method('getWebsites')
1343-
->willReturn([
1345+
->willReturn(
1346+
[
13441347
1 => ['first'],
13451348
2 => ['second'],
13461349
3 => ['third']
1347-
]);
1350+
]
1351+
);
13481352
$this->product->expects($this->once())->method('setWebsiteIds')->willReturn([2,3]);
13491353
$this->product->method('getSku')->willReturn('simple');
13501354

‎dev/tests/integration/testsuite/Magento/Catalog/Model/ProductRepositoryTest.php

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,15 @@
88
namespace Magento\Catalog\Model;
99

1010
use Magento\Catalog\Api\ProductRepositoryInterface;
11+
use Magento\Catalog\Model\ResourceModel\Product as ProductResource;
1112
use Magento\TestFramework\Helper\Bootstrap;
1213

1314
/**
1415
* Provide tests for ProductRepository model.
1516
*
1617
* @magentoDbIsolation enabled
1718
* @magentoAppIsolation enabled
19+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
1820
*/
1921
class ProductRepositoryTest extends \PHPUnit\Framework\TestCase
2022
{
@@ -30,6 +32,16 @@ class ProductRepositoryTest extends \PHPUnit\Framework\TestCase
3032
*/
3133
private $searchCriteriaBuilder;
3234

35+
/**
36+
* @var ProductFactory
37+
*/
38+
private $productFactory;
39+
40+
/**
41+
* @var ProductResource
42+
*/
43+
private $productResource;
44+
3345
/**
3446
* Sets up common objects
3547
*/
@@ -42,6 +54,12 @@ protected function setUp()
4254
$this->searchCriteriaBuilder = \Magento\Framework\App\ObjectManager::getInstance()->create(
4355
\Magento\Framework\Api\SearchCriteriaBuilder::class
4456
);
57+
58+
$this->productFactory = Bootstrap::getObjectManager()->get(ProductFactory::class);
59+
60+
$this->productResource = Bootstrap::getObjectManager()->get(ProductResource::class);
61+
62+
$this->productRepository = Bootstrap::getObjectManager()->get(ProductRepositoryInterface::class);
4563
}
4664

4765
/**
@@ -116,10 +134,15 @@ public function testSaveProductWithGalleryImage(): void
116134

117135
$path = $mediaConfig->getBaseMediaPath() . '/magento_image.jpg';
118136
$absolutePath = $mediaDirectory->getAbsolutePath() . $path;
119-
$product->addImageToMediaGallery($absolutePath, [
137+
$product->addImageToMediaGallery(
138+
$absolutePath,
139+
[
120140
'image',
121141
'small_image',
122-
], false, false);
142+
],
143+
false,
144+
false
145+
);
123146

124147
/** @var \Magento\Catalog\Api\ProductRepositoryInterface $productRepository */
125148
$productRepository = Bootstrap::getObjectManager()
@@ -138,4 +161,29 @@ public function testSaveProductWithGalleryImage(): void
138161
$this->assertStringStartsWith('/m/a/magento_image', $product->getData('image'));
139162
$this->assertStringStartsWith('/m/a/magento_image', $product->getData('small_image'));
140163
}
164+
165+
/**
166+
* Test Product Repository can change(update) "sku" for given product.
167+
*
168+
* @magentoDataFixture Magento/Catalog/_files/product_simple.php
169+
* @magentoDbIsolation enabled
170+
* @magentoAppArea adminhtml
171+
*/
172+
public function testUpdateProductSku()
173+
{
174+
$newSku = 'simple-edited';
175+
$productId = $this->productResource->getIdBySku('simple');
176+
$initialProduct = $this->productFactory->create();
177+
$this->productResource->load($initialProduct, $productId);
178+
179+
$initialProduct->setSku($newSku);
180+
$this->productRepository->save($initialProduct);
181+
182+
$updatedProduct = $this->productFactory->create();
183+
$this->productResource->load($updatedProduct, $productId);
184+
self::assertSame($newSku, $updatedProduct->getSku());
185+
186+
//clean up.
187+
$this->productRepository->delete($updatedProduct);
188+
}
141189
}

0 commit comments

Comments
 (0)