Skip to content

Commit c8304be

Browse files
cparticacpartica
authored
Merge pull request #5369 from magento-honey-badgers/MC-31551
[honey] MC-31405: Category images are broken for long image filenames
2 parents 161b243 + b4f49c9 commit c8304be

File tree

8 files changed

+224
-15
lines changed

8 files changed

+224
-15
lines changed

‎app/code/Magento/CatalogGraphQl/Model/Resolver/Category/Image.php

Lines changed: 38 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use Magento\Framework\Exception\LocalizedException;
1414
use Magento\Store\Api\Data\StoreInterface;
1515
use Magento\Framework\Filesystem\DirectoryList;
16+
use Magento\Catalog\Model\Category\FileInfo;
17+
use Magento\Framework\GraphQl\Exception\GraphQlInputException;
1618

1719
/**
1820
* Resolve category image to a fully qualified URL
@@ -22,12 +24,19 @@ class Image implements ResolverInterface
2224
/** @var DirectoryList */
2325
private $directoryList;
2426

27+
/** @var FileInfo */
28+
private $fileInfo;
29+
2530
/**
2631
* @param DirectoryList $directoryList
32+
* @param FileInfo $fileInfo
2733
*/
28-
public function __construct(DirectoryList $directoryList)
29-
{
34+
public function __construct(
35+
DirectoryList $directoryList,
36+
FileInfo $fileInfo
37+
) {
3038
$this->directoryList = $directoryList;
39+
$this->fileInfo = $fileInfo;
3140
}
3241

3342
/**
@@ -45,21 +54,40 @@ public function resolve(
4554
}
4655
/** @var \Magento\Catalog\Model\Category $category */
4756
$category = $value['model'];
48-
$imagePath = $category->getImage();
57+
$imagePath = $category->getData('image');
4958
if (empty($imagePath)) {
5059
return null;
5160
}
5261
/** @var StoreInterface $store */
5362
$store = $context->getExtensionAttributes()->getStore();
54-
$baseUrl = $store->getBaseUrl('media');
63+
$baseUrl = $store->getBaseUrl();
5564

56-
$mediaPath = $this->directoryList->getUrlPath('media');
57-
$pos = strpos($imagePath, $mediaPath);
58-
if ($pos !== false) {
59-
$imagePath = substr($imagePath, $pos + strlen($mediaPath), strlen($baseUrl));
65+
$filenameWithMedia = $this->fileInfo->isBeginsWithMediaDirectoryPath($imagePath)
66+
? $imagePath : $this->formatFileNameWithMediaCategoryFolder($imagePath);
67+
68+
if (!$this->fileInfo->isExist($filenameWithMedia)) {
69+
throw new GraphQlInputException(__('Category image not found.'));
6070
}
61-
$imageUrl = rtrim($baseUrl, '/') . '/' . ltrim($imagePath, '/');
6271

63-
return $imageUrl;
72+
// return full url
73+
return rtrim($baseUrl, '/') . $filenameWithMedia;
74+
}
75+
76+
/**
77+
* Format category media folder to filename
78+
*
79+
* @param string $fileName
80+
* @return string
81+
*/
82+
private function formatFileNameWithMediaCategoryFolder(string $fileName): string
83+
{
84+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
85+
$baseFileName = basename($fileName);
86+
return '/'
87+
. $this->directoryList->getUrlPath('media')
88+
. '/'
89+
. ltrim(FileInfo::ENTITY_MEDIA_PATH, '/')
90+
. '/'
91+
. $baseFileName;
6492
}
6593
}

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryListTest.php

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
namespace Magento\GraphQl\Catalog;
99

10+
use Magento\Catalog\Model\ResourceModel\Category\Collection as CategoryCollection;
11+
use Magento\Framework\ObjectManagerInterface;
1012
use Magento\Store\Model\StoreManagerInterface;
1113
use Magento\TestFramework\Helper\Bootstrap;
1214
use Magento\TestFramework\TestCase\GraphQlAbstract;
@@ -16,6 +18,16 @@
1618
*/
1719
class CategoryListTest extends GraphQlAbstract
1820
{
21+
/**
22+
* @var ObjectManagerInterface
23+
*/
24+
private $objectManager;
25+
26+
protected function setUp()
27+
{
28+
$this->objectManager = Bootstrap::getObjectManager();
29+
}
30+
1931
/**
2032
* @magentoApiDataFixture Magento/Catalog/_files/categories.php
2133
* @dataProvider filterSingleCategoryDataProvider
@@ -333,7 +345,7 @@ public function testEmptyFiltersReturnRootCategory()
333345
}
334346
}
335347
QUERY;
336-
$storeManager = Bootstrap::getObjectManager()->get(StoreManagerInterface::class);
348+
$storeManager = $this->objectManager->get(StoreManagerInterface::class);
337349
$storeRootCategoryId = $storeManager->getStore()->getRootCategoryId();
338350

339351
$result = $this->graphQlQuery($query);
@@ -369,6 +381,46 @@ public function testMinimumMatchQueryLength()
369381
$this->assertEquals([], $result['categoryList']);
370382
}
371383

384+
/**
385+
* Test category image full name is returned
386+
*
387+
* @magentoApiDataFixture Magento/Catalog/_files/catalog_category_with_long_image_name.php
388+
*/
389+
public function testCategoryImageName()
390+
{
391+
/** @var CategoryCollection $categoryCollection */
392+
$categoryCollection = $this->objectManager->get(CategoryCollection::class);
393+
$categoryModel = $categoryCollection
394+
->addAttributeToSelect('image')
395+
->addAttributeToFilter('name', ['eq' => 'Parent Image Category'])
396+
->getFirstItem();
397+
$categoryId = $categoryModel->getId();
398+
399+
$query = <<<QUERY
400+
{
401+
categoryList(filters: {ids: {in: ["$categoryId"]}}) {
402+
id
403+
name
404+
image
405+
}
406+
}
407+
QUERY;
408+
$storeManager = $this->objectManager->get(StoreManagerInterface::class);
409+
$storeBaseUrl = $storeManager->getStore()->getBaseUrl('media');
410+
411+
$expected = "catalog/category/magento_long_image_name_magento_long_image_name_magento_long_image_name.jpg";
412+
$expectedImageUrl = rtrim($storeBaseUrl, '/') . '/' . $expected;
413+
414+
$response = $this->graphQlQuery($query);
415+
$categoryList = $response['categoryList'];
416+
$this->assertArrayNotHasKey('errors', $response);
417+
$this->assertNotEmpty($response['categoryList']);
418+
$expectedImageUrl = str_replace('index.php/', '', $expectedImageUrl);
419+
$categoryList[0]['image'] = str_replace('index.php/', '', $categoryList[0]['image']);
420+
$this->assertEquals('Parent Image Category', $categoryList[0]['name']);
421+
$this->assertEquals($expectedImageUrl, $categoryList[0]['image']);
422+
}
423+
372424
/**
373425
* @return array
374426
*/

‎dev/tests/api-functional/testsuite/Magento/GraphQl/Catalog/CategoryTest.php

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@
1313
use Magento\Catalog\Model\CategoryRepository;
1414
use Magento\Catalog\Model\ResourceModel\Category\Collection as CategoryCollection;
1515
use Magento\Framework\DataObject;
16+
use Magento\Framework\EntityManager\MetadataPool;
17+
use Magento\Store\Model\Store;
1618
use Magento\Store\Model\StoreManagerInterface;
1719
use Magento\TestFramework\ObjectManager;
1820
use Magento\TestFramework\TestCase\GraphQl\ResponseContainsErrorsException;
@@ -33,10 +35,22 @@ class CategoryTest extends GraphQlAbstract
3335
*/
3436
private $categoryRepository;
3537

38+
/**
39+
* @var Store
40+
*/
41+
private $store;
42+
43+
/**
44+
* @var MetadataPool
45+
*/
46+
private $metadataPool;
47+
3648
protected function setUp()
3749
{
3850
$this->objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
3951
$this->categoryRepository = $this->objectManager->get(CategoryRepository::class);
52+
$this->store = $this->objectManager->get(Store::class);
53+
$this->metadataPool = $this->objectManager->get(MetadataPool::class);
4054
}
4155

4256
/**
@@ -211,7 +225,7 @@ public function testCategoriesTreeWithDisabledCategory()
211225
productImagePreview: products(pageSize: 1) {
212226
items {
213227
id
214-
}
228+
}
215229
}
216230
}
217231
}
@@ -557,17 +571,49 @@ public function testBreadCrumbs()
557571
/**
558572
* Test category image is returned as full url (not relative path)
559573
*
574+
* @param string $imagePrefix
560575
* @magentoApiDataFixture Magento/Catalog/_files/catalog_category_with_image.php
576+
* @dataProvider categoryImageDataProvider
561577
*/
562-
public function testCategoryImage()
578+
public function testCategoryImage(?string $imagePrefix)
563579
{
580+
/** @var CategoryCollection $categoryCollection */
564581
$categoryCollection = $this->objectManager->get(CategoryCollection::class);
565582
$categoryModel = $categoryCollection
566583
->addAttributeToSelect('image')
567584
->addAttributeToFilter('name', ['eq' => 'Parent Image Category'])
568585
->getFirstItem();
569586
$categoryId = $categoryModel->getId();
570587

588+
if ($imagePrefix !== null) {
589+
// update image to account for different stored image formats
590+
$connection = $categoryCollection->getConnection();
591+
$productLinkField = $this->metadataPool
592+
->getMetadata(\Magento\Catalog\Api\Data\ProductInterface::class)
593+
->getLinkField();
594+
595+
$defaultStoreId = $this->store->getId();
596+
597+
$imageAttributeValue = $imagePrefix . basename($categoryModel->getImage());
598+
599+
if (!empty($imageAttributeValue)) {
600+
$query = sprintf(
601+
'UPDATE %s SET `value` = "%s" ' .
602+
'WHERE `%s` = %d ' .
603+
'AND `store_id`= %d ' .
604+
'AND `attribute_id` = ' .
605+
'(SELECT `ea`.`attribute_id` FROM %s ea WHERE `ea`.`attribute_code` = "image" LIMIT 1)',
606+
$connection->getTableName('catalog_category_entity_varchar'),
607+
$imageAttributeValue,
608+
$productLinkField,
609+
$categoryModel->getData($productLinkField),
610+
$defaultStoreId,
611+
$connection->getTableName('eav_attribute')
612+
);
613+
$connection->query($query);
614+
}
615+
}
616+
571617
$query = <<<QUERY
572618
{
573619
categoryList(filters: {ids: {in: ["$categoryId"]}}) {
@@ -591,17 +637,41 @@ public function testCategoryImage()
591637
$this->assertNotEmpty($response['categoryList']);
592638
$categoryList = $response['categoryList'];
593639
$storeBaseUrl = $this->objectManager->get(StoreManagerInterface::class)->getStore()->getBaseUrl('media');
594-
$expectedImageUrl = rtrim($storeBaseUrl, '/'). '/' . ltrim($categoryModel->getImage(), '/');
640+
$expectedImageUrl = rtrim($storeBaseUrl, '/') . '/' . ltrim($categoryModel->getImage(), '/');
641+
$expectedImageUrl = str_replace('index.php/', '', $expectedImageUrl);
595642

596643
$this->assertEquals($categoryId, $categoryList[0]['id']);
597644
$this->assertEquals('Parent Image Category', $categoryList[0]['name']);
645+
$categoryList[0]['image'] = str_replace('index.php/', '', $categoryList[0]['image']);
598646
$this->assertEquals($expectedImageUrl, $categoryList[0]['image']);
599647

600648
$childCategory = $categoryList[0]['children'][0];
601649
$this->assertEquals('Child Image Category', $childCategory['name']);
650+
$childCategory['image'] = str_replace('index.php/', '', $childCategory['image']);
602651
$this->assertEquals($expectedImageUrl, $childCategory['image']);
603652
}
604653

654+
/**
655+
* @return array
656+
*/
657+
public function categoryImageDataProvider(): array
658+
{
659+
return [
660+
'default_filename_strategy' => [
661+
'image_prefix' => null
662+
],
663+
'just_filename_strategy' => [
664+
'image_prefix' => ''
665+
],
666+
'with_pub_media_strategy' => [
667+
'image_prefix' => '/pub/media/catalog/category/'
668+
],
669+
'catalog_category_strategy' => [
670+
'image_prefix' => 'catalog/category/'
671+
],
672+
];
673+
}
674+
605675
/**
606676
* @param ProductInterface $product
607677
* @param array $actualResponse

‎dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_image.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,10 @@
1313
$mediaDirectory = $objectManager->get(\Magento\Framework\Filesystem::class)
1414
->getDirectoryWrite(DirectoryList::MEDIA);
1515
$fileName = 'magento_small_image.jpg';
16+
$fileNameLong = 'magento_long_image_name_magento_long_image_name_magento_long_image_name.jpg';
1617
$filePath = 'catalog/category/' . $fileName;
18+
$filePathLong = 'catalog/category/' . $fileNameLong;
1719
$mediaDirectory->create('catalog/category');
1820

1921
copy(__DIR__ . DIRECTORY_SEPARATOR . $fileName, $mediaDirectory->getAbsolutePath($filePath));
22+
copy(__DIR__ . DIRECTORY_SEPARATOR . $fileNameLong, $mediaDirectory->getAbsolutePath($filePathLong));

‎dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_with_image.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
* See COPYING.txt for license details.
55
*/
66

7-
require_once 'catalog_category_image.php';
7+
require 'catalog_category_image.php';
88

99
/** @var $category \Magento\Catalog\Model\Category */
1010
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();

‎dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_with_long_image_name.php

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
require 'catalog_category_image.php';
8+
9+
/** @var $category \Magento\Catalog\Model\Category */
10+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
11+
12+
$categoryParent = $objectManager->create(\Magento\Catalog\Model\Category::class);
13+
$categoryParent->setName('Parent Image Category')
14+
->setPath('1/2')
15+
->setLevel(2)
16+
->setImage($filePathLong)
17+
->setAvailableSortBy('name')
18+
->setDefaultSortBy('name')
19+
->setIsActive(true)
20+
->setPosition(1)
21+
->save();
22+
23+
$categoryChild = $objectManager->create(\Magento\Catalog\Model\Category::class);
24+
$categoryChild->setName('Child Image Category')
25+
->setPath($categoryParent->getPath())
26+
->setLevel(3)
27+
->setImage($fileNameLong)
28+
->setAvailableSortBy('name')
29+
->setDefaultSortBy('name')
30+
->setIsActive(true)
31+
->setPosition(2)
32+
->save();

‎dev/tests/integration/testsuite/Magento/Catalog/_files/catalog_category_with_long_image_name_rollback.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
/** @var \Magento\Framework\ObjectManagerInterface $objectManager */
8+
$objectManager = \Magento\TestFramework\Helper\Bootstrap::getObjectManager();
9+
10+
/** @var \Magento\Framework\Registry $registry */
11+
$registry = $objectManager->get(\Magento\Framework\Registry::class);
12+
13+
$registry->unregister('isSecureArea');
14+
$registry->register('isSecureArea', true);
15+
16+
/** @var \Magento\Catalog\Model\ResourceModel\Product\Collection $collection */
17+
$collection = $objectManager->create(\Magento\Catalog\Model\ResourceModel\Category\Collection::class);
18+
$collection
19+
->addAttributeToFilter('name', ['in' => ['Parent Image Category', 'Child Image Category']])
20+
->load()
21+
->delete();
22+
23+
$registry->unregister('isSecureArea');
24+
$registry->register('isSecureArea', false);

‎dev/tests/integration/testsuite/Magento/Catalog/_files/magento_long_image_name_magento_long_image_name_magento_long_image_name.jpg

3.93 KB
Loading

0 commit comments

Comments
 (0)