Skip to content

Commit e02ce06

Browse files
pradeep1819pradeep1819
committed
ACP2E-2152: Error while making concurrent requests to rest/V1/products/<sku>/media
1 parent 00aa174 commit e02ce06

File tree

4 files changed

+226
-96
lines changed

4 files changed

+226
-96
lines changed

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

Lines changed: 2 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -374,44 +374,6 @@ private function cacheProduct($cacheKey, ProductInterface $product)
374374
}
375375
}
376376

377-
/**
378-
* If the request is for adding, keep the initial media_gallery data; if it is for deleting, unset it.
379-
*
380-
* @param array $productData
381-
* @return array
382-
*/
383-
private function resetMediaGalleryData(array $productData): array
384-
{
385-
$unset = true;
386-
if (isset($productData['media_gallery'])) {
387-
krsort($productData['media_gallery']['images']);
388-
$isNewEntry = false;
389-
foreach ($productData['media_gallery']['images'] as $entry) {
390-
if (!isset($entry['value_id'])) {
391-
$isNewEntry = true;
392-
break;
393-
}
394-
}
395-
if ($isNewEntry) {
396-
$existingMediaGallery = [];
397-
foreach ($productData['media_gallery']['images'] as $entry) {
398-
if (isset($entry['value_id'])) {
399-
$existingMediaGallery['media_gallery']['images'][$entry['value_id']] = $entry;
400-
}
401-
}
402-
if (isset($existingMediaGallery['media_gallery'])) {
403-
$productData['media_gallery'] = $existingMediaGallery['media_gallery'];
404-
$unset = false;
405-
}
406-
}
407-
}
408-
if ($unset) {
409-
unset($productData['media_gallery']);
410-
}
411-
412-
return $productData;
413-
}
414-
415377
/**
416378
* Merge data from DB and updates from request
417379
*
@@ -422,7 +384,7 @@ private function resetMediaGalleryData(array $productData): array
422384
*/
423385
protected function initializeProductData(array $productData, $createNew)
424386
{
425-
$productData = $this->resetMediaGalleryData($productData);
387+
unset($productData['media_gallery']);
426388
if ($createNew) {
427389
$product = $this->productFactory->create();
428390
$this->assignProductToWebsites($product);
@@ -978,7 +940,7 @@ private function joinPositionField(
978940
foreach ($filterGroup->getFilters() as $filter) {
979941
if ($filter->getField() === 'category_id') {
980942
$filterValue = $filter->getValue();
981-
$categoryIds[] = is_array($filterValue) ? $filterValue : explode(',', $filterValue);
943+
$categoryIds[] = is_array($filterValue) ? $filterValue : explode(',', $filterValue ?? '');
982944
}
983945
}
984946
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,14 @@ public function processMediaGallery(ProductInterface $product, array $mediaGalle
8888
$existingMediaGallery = $product->getMediaGallery('images');
8989
$newEntries = [];
9090
$entriesById = [];
91+
$addMedia = false;
9192
if (!empty($existingMediaGallery)) {
9293
foreach ($mediaGalleryEntries as $entry) {
9394
if (isset($entry['value_id'])) {
9495
$entriesById[$entry['value_id']] = $entry;
9596
} else {
9697
$newEntries[] = $entry;
98+
$addMedia = true;
9799
}
98100
}
99101
foreach ($existingMediaGallery as $key => &$existingEntry) {
@@ -113,6 +115,9 @@ public function processMediaGallery(ProductInterface $product, array $mediaGalle
113115
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
114116
$existingMediaGallery[$key] = array_merge($existingEntry, $updatedEntry);
115117
}
118+
} elseif ($addMedia && isset($existingEntry['value_id'])) {
119+
//avoid deleting an exiting image while adding a new one
120+
unset($existingMediaGallery[$key]);
116121
} elseif ($this->canRemoveImage($product, $existingEntry)) {
117122
//set the removed flag
118123
$existingEntry['removed'] = true;

‎app/code/Magento/Catalog/Test/Unit/Model/ProductRepository/MediaGalleryProcessorTest.php

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Catalog\Test\Unit\Model\ProductRepository;
9+
10+
use Magento\Catalog\Model\Product;
11+
use Magento\Catalog\Model\Product\Gallery\DeleteValidator;
12+
use Magento\Catalog\Model\Product\Gallery\Processor;
13+
use Magento\Catalog\Model\Product\Media\Config;
14+
use Magento\Catalog\Model\ProductRepository\MediaGalleryProcessor;
15+
use Magento\Framework\Api\Data\ImageContentInterface;
16+
use Magento\Framework\Api\Data\ImageContentInterfaceFactory;
17+
use Magento\Framework\Api\ImageContent;
18+
use Magento\Framework\Api\ImageProcessorInterface;
19+
use PHPUnit\Framework\MockObject\MockObject;
20+
use PHPUnit\Framework\TestCase;
21+
22+
class MediaGalleryProcessorTest extends TestCase
23+
{
24+
/**
25+
* @var MediaGalleryProcessor
26+
*/
27+
private $galleryProcessor;
28+
29+
/**
30+
* @var Processor|MockObject
31+
*/
32+
private $processorMock;
33+
34+
/**
35+
* @var ImageContentInterfaceFactory|MockObject
36+
*/
37+
private $contentFactoryMock;
38+
39+
/**
40+
* @var ImageProcessorInterface|MockObject
41+
*/
42+
private $imageProcessorMock;
43+
44+
/**
45+
* @var DeleteValidator|MockObject
46+
*/
47+
private $deleteValidatorMock;
48+
49+
/**
50+
* @var Product|MockObject
51+
*/
52+
private $productMock;
53+
54+
protected function setUp(): void
55+
{
56+
$this->processorMock = $this->getMockBuilder(Processor::class)
57+
->disableOriginalConstructor()
58+
->getMock();
59+
$this->contentFactoryMock = $this->getMockBuilder(ImageContentInterfaceFactory::class)
60+
->disableOriginalConstructor()
61+
->getMock();
62+
$this->imageProcessorMock = $this->getMockBuilder(ImageProcessorInterface::class)
63+
->disableOriginalConstructor()
64+
->getMock();
65+
$this->deleteValidatorMock = $this->getMockBuilder(DeleteValidator::class)
66+
->disableOriginalConstructor()
67+
->getMock();
68+
$this->productMock = $this->getMockBuilder(Product::class)
69+
->addMethods(['getMediaGallery'])
70+
->onlyMethods(['hasGalleryAttribute', 'getMediaConfig', 'getMediaAttributes'])
71+
->disableOriginalConstructor()
72+
->getMock();
73+
74+
$this->galleryProcessor = new MediaGalleryProcessor(
75+
$this->processorMock,
76+
$this->contentFactoryMock,
77+
$this->imageProcessorMock,
78+
$this->deleteValidatorMock
79+
);
80+
}
81+
82+
/**
83+
* The media gallery array should not have "removed" key while adding the new entry
84+
*
85+
* @return void
86+
*/
87+
public function testProcessMediaGallery(): void
88+
{
89+
$initialExitingEntry = [
90+
'value_id' => 5,
91+
"label" => "new_label_text",
92+
'file' => 'filename1',
93+
'position' => 10,
94+
'disabled' => false,
95+
'types' => ['image', 'small_image']
96+
];
97+
$newEntriesData = [
98+
'images' => [
99+
$initialExitingEntry,
100+
[
101+
'value_id' => null,
102+
'label' => "label_text",
103+
'position' => 10,
104+
'disabled' => false,
105+
'types' => ['image', 'small_image'],
106+
'content' => [
107+
'data' => [
108+
ImageContentInterface::NAME => 'filename',
109+
ImageContentInterface::TYPE => 'image/jpeg',
110+
ImageContentInterface::BASE64_ENCODED_DATA => 'encoded_content'
111+
]
112+
],
113+
'media_type' => 'media_type'
114+
]
115+
]
116+
];
117+
$newExitingEntriesData = [
118+
'images' => [
119+
$initialExitingEntry,
120+
[
121+
'value_id' => 6,
122+
"label" => "label_text2",
123+
'file' => 'filename2',
124+
'position' => 10,
125+
'disabled' => false,
126+
'types' => ['image', 'small_image']
127+
]
128+
]
129+
];
130+
$this->productMock->method('getMediaGallery')
131+
->willReturnOnConsecutiveCalls(
132+
$newExitingEntriesData['images'],
133+
$newExitingEntriesData['images']
134+
);
135+
136+
$this->productMock->expects($this->any())
137+
->method('getMediaAttributes')
138+
->willReturn(["image" => "imageAttribute", "small_image" => "small_image_attribute"]);
139+
140+
$this->productMock->method('hasGalleryAttribute')->willReturn(true);
141+
142+
//setup media attribute backend
143+
$mediaTmpPath = '/tmp';
144+
$absolutePath = '/a/b/filename.jpg';
145+
146+
$this->processorMock->expects($this->once())->method('clearMediaAttribute')
147+
->with($this->productMock, ['image', 'small_image']);
148+
149+
$mediaConfigMock = $this->getMockBuilder(Config::class)
150+
->disableOriginalConstructor()
151+
->getMock();
152+
$mediaConfigMock->expects($this->once())
153+
->method('getTmpMediaShortUrl')
154+
->with($absolutePath)
155+
->willReturn($mediaTmpPath . $absolutePath);
156+
$this->productMock->expects($this->once())
157+
->method('getMediaConfig')
158+
->willReturn($mediaConfigMock);
159+
160+
//verify new entries
161+
$contentDataObject = $this->getMockBuilder(ImageContent::class)
162+
->disableOriginalConstructor()
163+
->setMethods(null)
164+
->getMock();
165+
$this->contentFactoryMock->expects($this->once())
166+
->method('create')
167+
->willReturn($contentDataObject);
168+
169+
$this->imageProcessorMock->expects($this->once())
170+
->method('processImageContent')
171+
->willReturn($absolutePath);
172+
173+
$imageFileUri = $mediaTmpPath . $absolutePath;
174+
175+
$this->processorMock->expects($this->once())
176+
->method('addImage')
177+
->willReturnCallback(
178+
function ($product, $imageFileUri) use ($newEntriesData) {
179+
foreach ($product['media_gallery']['images'] as $entry) {
180+
if (isset($entry['value_id'])) {
181+
$this->assertArrayNotHasKey('removed', $entry);
182+
}
183+
}
184+
$this->productMock->setData('media_gallery', $newEntriesData);
185+
return $imageFileUri;
186+
}
187+
);
188+
$this->processorMock->expects($this->once())->method('updateImage')
189+
->with(
190+
$this->productMock,
191+
$imageFileUri,
192+
[
193+
'label' => 'label_text',
194+
'position' => 10,
195+
'disabled' => false,
196+
'media_type' => 'media_type',
197+
]
198+
);
199+
200+
$this->galleryProcessor->processMediaGallery($this->productMock, $newEntriesData['images']);
201+
}
202+
}

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

Lines changed: 17 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1367,53 +1367,23 @@ protected function setupProductMocksForSave(): void
13671367
public function testSaveExistingWithNewMediaGalleryEntries(): void
13681368
{
13691369
$this->storeManager->expects($this->any())->method('getWebsites')->willReturn([1 => 'default']);
1370-
$newEntry = [
1371-
'value_id' => null,
1372-
'label' => "label_text",
1373-
'position' => 10,
1374-
'disabled' => false,
1375-
'types' => ['image', 'small_image'],
1376-
'content' => [
1377-
'data' => [
1378-
ImageContentInterface::NAME => 'filename',
1379-
ImageContentInterface::TYPE => 'image/jpeg',
1380-
ImageContentInterface::BASE64_ENCODED_DATA => 'encoded_content'
1381-
]
1382-
],
1383-
'media_type' => 'media_type'
1384-
];
13851370
$newEntriesData = [
13861371
'images' => [
13871372
[
1388-
'value_id' => 5,
1389-
"label" => "new_label_text",
1390-
'file' => 'filename1',
1391-
'position' => 10,
1392-
'disabled' => false,
1393-
'types' => ['image', 'small_image']
1394-
],
1395-
$newEntry
1396-
]
1397-
];
1398-
$initializedEntriesData = [
1399-
'images' => [
1400-
[
1401-
'value_id' => 5,
1402-
"label" => "label_text",
1403-
'file' => 'filename1',
1404-
'position' => 10,
1405-
'disabled' => false,
1406-
'types' => ['image', 'small_image']
1407-
],
1408-
[
1409-
'value_id' => 6,
1410-
"label" => "label_text2",
1411-
'file' => 'filename2',
1373+
'value_id' => null,
1374+
'label' => "label_text",
14121375
'position' => 10,
14131376
'disabled' => false,
1414-
'types' => ['image', 'small_image']
1415-
],
1416-
$newEntry
1377+
'types' => ['image', 'small_image'],
1378+
'content' => [
1379+
'data' => [
1380+
ImageContentInterface::NAME => 'filename',
1381+
ImageContentInterface::TYPE => 'image/jpeg',
1382+
ImageContentInterface::BASE64_ENCODED_DATA => 'encoded_content'
1383+
]
1384+
],
1385+
'media_type' => 'media_type'
1386+
]
14171387
]
14181388
];
14191389

@@ -1425,7 +1395,7 @@ public function testSaveExistingWithNewMediaGalleryEntries(): void
14251395
->method('toNestedArray')
14261396
->willReturn($this->productData);
14271397

1428-
$this->initializedProduct->setData('media_gallery', $initializedEntriesData);
1398+
$this->initializedProduct->setData('media_gallery', $newEntriesData);
14291399
$this->initializedProduct->expects($this->any())
14301400
->method('getMediaAttributes')
14311401
->willReturn(["image" => "imageAttribute", "small_image" => "small_image_attribute"]);
@@ -1461,19 +1431,10 @@ public function testSaveExistingWithNewMediaGalleryEntries(): void
14611431
->method('processImageContent')
14621432
->willReturn($absolutePath);
14631433

1464-
$imageFileUri = $mediaTmpPath . $absolutePath;
1465-
1466-
$this->processor->expects($this->once())
1467-
->method('addImage')
1468-
->willReturnCallback(
1469-
function ($product, $imageFileUri) use ($newEntriesData) {
1470-
foreach ($product['media_gallery']['images'] as $entry) {
1471-
$this->assertArrayNotHasKey('removed', $entry);
1472-
}
1473-
$this->initializedProduct->setData('media_gallery', $newEntriesData);
1474-
return $imageFileUri;
1475-
}
1476-
);
1434+
$imageFileUri = "imageFileUri";
1435+
$this->processor->expects($this->once())->method('addImage')
1436+
->with($this->initializedProduct, $mediaTmpPath . $absolutePath, ['image', 'small_image'], true, false)
1437+
->willReturn($imageFileUri);
14771438
$this->processor->expects($this->once())->method('updateImage')
14781439
->with(
14791440
$this->initializedProduct,

0 commit comments

Comments
 (0)