Skip to content

Commit b11f053

Browse files
SeruyVSeruyV
committed
MC-20259: PUT /V1/products/:sku/media/:entryId is not idempotent
1 parent 8112a38 commit b11f053

File tree

6 files changed

+135
-44
lines changed

6 files changed

+135
-44
lines changed

‎app/code/Magento/Catalog/Model/Product/Gallery/CreateHandler.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public function execute($product, $arguments = [])
162162

163163
if (!empty($image['removed'])) {
164164
$clearImages[] = $image['file'];
165-
} elseif (empty($image['value_id'])) {
165+
} elseif (empty($image['value_id']) || !empty($image['recreate'])) {
166166
$newFile = $this->moveImageFromTmp($image['file']);
167167
$image['new_file'] = $newFile;
168168
$newImages[$image['file']] = $image;

‎app/code/Magento/Catalog/Model/Product/Gallery/GalleryManagement.php

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -71,10 +71,12 @@ public function create($sku, ProductAttributeMediaGalleryEntryInterface $entry)
7171
$product->setMediaGalleryEntries($existingMediaGalleryEntries);
7272
try {
7373
$product = $this->productRepository->save($product);
74-
} catch (InputException $inputException) {
75-
throw $inputException;
7674
} catch (\Exception $e) {
77-
throw new StateException(__("The product can't be saved."));
75+
if ($e instanceof InputException) {
76+
throw $e;
77+
} else {
78+
throw new StateException(__("The product can't be saved."));
79+
}
7880
}
7981

8082
foreach ($product->getMediaGalleryEntries() as $entry) {
@@ -98,19 +100,13 @@ public function update($sku, ProductAttributeMediaGalleryEntryInterface $entry)
98100
);
99101
}
100102
$found = false;
103+
$entryTypes = (array)$entry->getTypes();
101104
foreach ($existingMediaGalleryEntries as $key => $existingEntry) {
102-
$entryTypes = (array)$entry->getTypes();
103-
$existingEntryTypes = (array)$existingMediaGalleryEntries[$key]->getTypes();
104-
$existingMediaGalleryEntries[$key]->setTypes(array_diff($existingEntryTypes, $entryTypes));
105+
$existingEntryTypes = (array)$existingEntry->getTypes();
106+
$existingEntry->setTypes(array_diff($existingEntryTypes, $entryTypes));
105107

106108
if ($existingEntry->getId() == $entry->getId()) {
107109
$found = true;
108-
109-
$file = $entry->getContent();
110-
111-
if ($file && $file->getBase64EncodedData() || $entry->getFile()) {
112-
$entry->setId(null);
113-
}
114110
$existingMediaGalleryEntries[$key] = $entry;
115111
}
116112
}

‎app/code/Magento/Catalog/Model/Product/Gallery/UpdateHandler.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace Magento\Catalog\Model\Product\Gallery;
77

8+
use Magento\Catalog\Model\ResourceModel\Product\Gallery;
89
use Magento\Framework\EntityManager\Operation\ExtensionInterface;
910

1011
/**
@@ -75,6 +76,16 @@ protected function processNewImage($product, array &$image)
7576
$image['value_id'],
7677
$product->getData($this->metadata->getLinkField())
7778
);
79+
} elseif (!empty($image['recreate'])) {
80+
$data['value_id'] = $image['value_id'];
81+
$data['value'] = $image['file'];
82+
$data['attribute_id'] = $this->getAttribute()->getAttributeId();
83+
84+
if (!empty($image['media_type'])) {
85+
$data['media_type'] = $image['media_type'];
86+
}
87+
88+
$this->resourceModel->saveDataRow(Gallery::GALLERY_TABLE, $data);
7889
}
7990

8091
return $data;

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,17 @@ public function processMediaGallery(ProductInterface $product, array $mediaGalle
9292
if ($updatedEntry['file'] === null) {
9393
unset($updatedEntry['file']);
9494
}
95-
$existingMediaGallery[$key] = array_merge($existingEntry, $updatedEntry);
95+
if (isset($updatedEntry['content'])) {
96+
//need to recreate image and reset object
97+
$existingEntry['recreate'] = true;
98+
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
99+
$newEntry = array_merge($existingEntry, $updatedEntry);
100+
$newEntries[] = $newEntry;
101+
unset($existingMediaGallery[$key]);
102+
} else {
103+
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
104+
$existingMediaGallery[$key] = array_merge($existingEntry, $updatedEntry);
105+
}
96106
} else {
97107
//set the removed flag
98108
$existingEntry['removed'] = true;

‎app/code/Magento/Catalog/Test/Unit/Model/Product/Gallery/GalleryManagementTest.php

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77

88
namespace Magento\Catalog\Test\Unit\Model\Product\Gallery;
99

10+
/**
11+
* Tests for \Magento\Catalog\Model\Product\Gallery\GalleryManagement.
12+
*/
1013
class GalleryManagementTest extends \PHPUnit\Framework\TestCase
1114
{
1215
/**
@@ -39,19 +42,25 @@ class GalleryManagementTest extends \PHPUnit\Framework\TestCase
3942
*/
4043
protected $attributeValueMock;
4144

45+
/**
46+
* @inheritdoc
47+
*/
4248
protected function setUp()
4349
{
4450
$this->productRepositoryMock = $this->createMock(\Magento\Catalog\Api\ProductRepositoryInterface::class);
4551
$this->contentValidatorMock = $this->createMock(\Magento\Framework\Api\ImageContentValidatorInterface::class);
46-
$this->productMock = $this->createPartialMock(\Magento\Catalog\Model\Product::class, [
52+
$this->productMock = $this->createPartialMock(
53+
\Magento\Catalog\Model\Product::class,
54+
[
4755
'setStoreId',
4856
'getData',
4957
'getStoreId',
5058
'getSku',
5159
'getCustomAttribute',
5260
'getMediaGalleryEntries',
5361
'setMediaGalleryEntries',
54-
]);
62+
]
63+
);
5564
$this->mediaGalleryEntryMock =
5665
$this->createMock(\Magento\Catalog\Api\Data\ProductAttributeMediaGalleryEntryInterface::class);
5766
$this->model = new \Magento\Catalog\Model\Product\Gallery\GalleryManagement(
@@ -151,6 +160,8 @@ public function testUpdateWithNonExistingImage()
151160
$existingEntryMock->expects($this->once())->method('getId')->willReturn(43);
152161
$this->productMock->expects($this->once())->method('getMediaGalleryEntries')
153162
->willReturn([$existingEntryMock]);
163+
$existingEntryMock->expects($this->once())->method('getTypes')->willReturn([]);
164+
$entryMock->expects($this->once())->method('getTypes')->willReturn([]);
154165
$entryMock->expects($this->once())->method('getId')->willReturn($entryId);
155166
$this->model->update($productSku, $entryMock);
156167
}
@@ -172,12 +183,19 @@ public function testUpdateWithCannotSaveException()
172183
$existingEntryMock->expects($this->once())->method('getId')->willReturn($entryId);
173184
$this->productMock->expects($this->once())->method('getMediaGalleryEntries')
174185
->willReturn([$existingEntryMock]);
186+
$existingEntryMock->expects($this->once())->method('getTypes')->willReturn([]);
187+
$entryMock->expects($this->once())->method('getTypes')->willReturn([]);
175188
$entryMock->expects($this->once())->method('getId')->willReturn($entryId);
176189
$this->productRepositoryMock->expects($this->once())->method('save')->with($this->productMock)
177190
->willThrowException(new \Exception());
178191
$this->model->update($productSku, $entryMock);
179192
}
180193

194+
/**
195+
* Check update gallery entry behavior.
196+
*
197+
* @return void
198+
*/
181199
public function testUpdate()
182200
{
183201
$productSku = 'testProduct';
@@ -203,14 +221,13 @@ public function testUpdate()
203221
->willReturn([$existingEntryMock, $existingSecondEntryMock]);
204222

205223
$entryMock->expects($this->exactly(2))->method('getId')->willReturn($entryId);
206-
$entryMock->expects($this->once())->method('getFile')->willReturn("base64");
207-
$entryMock->expects($this->once())->method('setId')->with(null);
208-
$entryMock->expects($this->exactly(2))->method('getTypes')->willReturn(['image']);
224+
$entryMock->expects($this->once())->method('getTypes')->willReturn(['image']);
209225

210226
$this->productMock->expects($this->once())->method('setMediaGalleryEntries')
211227
->with([$entryMock, $existingSecondEntryMock])
212228
->willReturnSelf();
213229
$this->productRepositoryMock->expects($this->once())->method('save')->with($this->productMock);
230+
214231
$this->assertTrue($this->model->update($productSku, $entryMock));
215232
}
216233

‎dev/tests/api-functional/testsuite/Magento/Catalog/Api/ProductAttributeMediaGalleryManagementInterfaceTest.php

Lines changed: 82 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@
1111
use Magento\Framework\Api\Data\ImageContentInterface;
1212
use Magento\TestFramework\Helper\Bootstrap;
1313
use Magento\Catalog\Model\ProductFactory;
14-
use Magento\TestFramework\ObjectManager;
1514
use Magento\Catalog\Model\Product\Attribute\Backend\Media\ImageEntryConverter;
1615
use Magento\Catalog\Model\ProductRepository;
1716
use Magento\Framework\Webapi\Rest\Request;
1817
use Magento\TestFramework\TestCase\WebapiAbstract;
18+
use Magento\Framework\ObjectManagerInterface;
1919

2020
/**
2121
* Class ProductAttributeMediaGalleryManagementInterfaceTest
@@ -48,11 +48,18 @@ class ProductAttributeMediaGalleryManagementInterfaceTest extends WebapiAbstract
4848
*/
4949
protected $testImagePath;
5050

51+
/**
52+
* @var ObjectManagerInterface
53+
*/
54+
private $objectManager;
55+
5156
/**
5257
* @inheritDoc
5358
*/
5459
protected function setUp()
5560
{
61+
$this->objectManager = Bootstrap::getObjectManager();
62+
5663
$this->createServiceInfo = [
5764
'rest' => [
5865
'resourcePath' => '/V1/products/simple/media',
@@ -98,9 +105,7 @@ protected function setUp()
98105
*/
99106
protected function getTargetSimpleProduct()
100107
{
101-
$objectManager = Bootstrap::getObjectManager();
102-
103-
return $objectManager->get(ProductFactory::class)->create()->load(1);
108+
return $this->objectManager->get(ProductFactory::class)->create()->load(1);
104109
}
105110

106111
/**
@@ -241,6 +246,10 @@ public function testCreateWithNotDefaultStoreId()
241246
*/
242247
public function testUpdate()
243248
{
249+
$productRepository = $this->objectManager->create(ProductRepositoryInterface::class);
250+
/** @var \Magento\Catalog\Api\Data\ProductInterface $product */
251+
$product = $productRepository->get('simple');
252+
$imageId = (int)$product->getMediaGalleryImages()->getFirstItem()->getValueId();
244253
$requestData = [
245254
'sku' => 'simple',
246255
'entry' => [
@@ -257,19 +266,48 @@ public function testUpdate()
257266
. '/' . $this->getTargetGalleryEntryId();
258267

259268
$this->assertTrue($this->_webApiCall($this->updateServiceInfo, $requestData, null, 'all'));
269+
$updatedImage = $this->assertMediaGalleryData($imageId, '/m/a/magento_image.jpg', 'Updated Image Text');
270+
$this->assertEquals(10, $updatedImage['position_default']);
271+
$this->assertEquals(1, $updatedImage['disabled_default']);
272+
}
260273

261-
$targetProduct = $this->getTargetSimpleProduct();
262-
$this->assertEquals('/m/a/magento_image.jpg', $targetProduct->getData('thumbnail'));
263-
$this->assertEquals('no_selection', $targetProduct->getData('image'));
264-
$this->assertEquals('no_selection', $targetProduct->getData('small_image'));
265-
$mediaGallery = $targetProduct->getData('media_gallery');
266-
$this->assertCount(1, $mediaGallery['images']);
267-
$updatedImage = array_shift($mediaGallery['images']);
268-
$this->assertEquals('Updated Image Text', $updatedImage['label']);
269-
$this->assertEquals('/m/a/magento_image.jpg', $updatedImage['file']);
270-
$this->assertEquals(10, $updatedImage['position']);
271-
$this->assertEquals(1, $updatedImage['disabled']);
272-
$this->assertEquals('Updated Image Text', $updatedImage['label_default']);
274+
/**
275+
* Update media gallery entity with new image.
276+
*
277+
* @magentoApiDataFixture Magento/Catalog/_files/product_with_image.php
278+
* @return void
279+
*/
280+
public function testUpdateWithNewImage(): void
281+
{
282+
$productRepository = $this->objectManager->create(ProductRepositoryInterface::class);
283+
/** @var \Magento\Catalog\Api\Data\ProductInterface $product */
284+
$product = $productRepository->get('simple');
285+
$imageId = (int)$product->getMediaGalleryImages()->getFirstItem()->getValueId();
286+
287+
$requestData = [
288+
'sku' => 'simple',
289+
'entry' => [
290+
'id' => $this->getTargetGalleryEntryId(),
291+
'label' => 'Updated Image Text',
292+
'position' => 10,
293+
'types' => ['thumbnail'],
294+
'disabled' => true,
295+
'media_type' => 'image',
296+
'content' => [
297+
'base64_encoded_data' => 'iVBORw0KGgoAAAANSUhEUgAAAP8AAADGCAMAAAAqo6adAAAAA1BMVEUAAP79f'
298+
. '+LBAAAASElEQVR4nO3BMQEAAADCoPVPbQwfoAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
299+
. 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAA+BsYAAAF7hZJ0AAAAAElFTkSuQmCC',
300+
'type' => 'image/png',
301+
'name' => 'testname_updated.png',
302+
],
303+
],
304+
];
305+
306+
$this->updateServiceInfo['rest']['resourcePath'] = $this->updateServiceInfo['rest']['resourcePath']
307+
. '/' . $this->getTargetGalleryEntryId();
308+
309+
$this->assertTrue($this->_webApiCall($this->updateServiceInfo, $requestData, null, 'all'));
310+
$updatedImage = $this->assertMediaGalleryData($imageId, '/t/e/testname_updated.png', 'Updated Image Text');
273311
$this->assertEquals(10, $updatedImage['position_default']);
274312
$this->assertEquals(1, $updatedImage['disabled_default']);
275313
}
@@ -281,6 +319,11 @@ public function testUpdate()
281319
*/
282320
public function testUpdateWithNotDefaultStoreId()
283321
{
322+
$productRepository = $this->objectManager->create(ProductRepositoryInterface::class);
323+
/** @var \Magento\Catalog\Api\Data\ProductInterface $product */
324+
$product = $productRepository->get('simple');
325+
$imageId = (int)$product->getMediaGalleryImages()->getFirstItem()->getValueId();
326+
284327
$requestData = [
285328
'sku' => 'simple',
286329
'entry' => [
@@ -297,21 +340,36 @@ public function testUpdateWithNotDefaultStoreId()
297340
. '/' . $this->getTargetGalleryEntryId();
298341

299342
$this->assertTrue($this->_webApiCall($this->updateServiceInfo, $requestData, null, 'default'));
343+
$updatedImage = $this->assertMediaGalleryData($imageId, '/m/a/magento_image.jpg', 'Image Alt Text');
344+
$this->assertEquals(1, $updatedImage['position_default']);
345+
$this->assertEquals(0, $updatedImage['disabled_default']);
346+
}
300347

348+
/**
349+
* Check that Media Gallery data is correct.
350+
*
351+
* @param int $imageId
352+
* @param string $file
353+
* @param string $label
354+
* @return array
355+
*/
356+
private function assertMediaGalleryData(int $imageId, string $file, string $label): array
357+
{
301358
$targetProduct = $this->getTargetSimpleProduct();
302-
$this->assertEquals('/m/a/magento_image.jpg', $targetProduct->getData('thumbnail'));
359+
$this->assertEquals($file, $targetProduct->getData('thumbnail'));
360+
$this->assertEquals('no_selection', $targetProduct->getData('image'));
361+
$this->assertEquals('no_selection', $targetProduct->getData('small_image'));
303362
$mediaGallery = $targetProduct->getData('media_gallery');
304363
$this->assertCount(1, $mediaGallery['images']);
305364
$updatedImage = array_shift($mediaGallery['images']);
306-
// Not default store view values were updated
365+
$this->assertEquals($imageId, $updatedImage['value_id']);
307366
$this->assertEquals('Updated Image Text', $updatedImage['label']);
308-
$this->assertEquals('/m/a/magento_image.jpg', $updatedImage['file']);
367+
$this->assertEquals($file, $updatedImage['file']);
309368
$this->assertEquals(10, $updatedImage['position']);
310369
$this->assertEquals(1, $updatedImage['disabled']);
311-
// Default store view values were not updated
312-
$this->assertEquals('Image Alt Text', $updatedImage['label_default']);
313-
$this->assertEquals(1, $updatedImage['position_default']);
314-
$this->assertEquals(0, $updatedImage['disabled_default']);
370+
$this->assertEquals($label, $updatedImage['label_default']);
371+
372+
return $updatedImage;
315373
}
316374

317375
/**
@@ -564,9 +622,8 @@ public function testGet()
564622
{
565623
$productSku = 'simple';
566624

567-
$objectManager = ObjectManager::getInstance();
568625
/** @var ProductRepository $repository */
569-
$repository = $objectManager->create(ProductRepository::class);
626+
$repository = $this->objectManager->create(ProductRepository::class);
570627
$product = $repository->get($productSku);
571628
$image = current($product->getMediaGallery('images'));
572629
$imageId = $image['value_id'];

0 commit comments

Comments
 (0)