Skip to content

Get sitemap product images from image cache, if available #9082

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 19 commits into from
Jun 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 46 additions & 5 deletions app/code/Magento/Sitemap/Model/ResourceModel/Catalog/Product.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

// @codingStandardsIgnoreFile

namespace Magento\Sitemap\Model\ResourceModel\Catalog;

use Magento\CatalogUrlRewrite\Model\ProductUrlRewriteGenerator;
Expand All @@ -13,6 +16,9 @@
*
* @author Magento Core Team <core@magentocommerce.com>
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @SuppressWarnings(PHPMD.LongVariable)
* @SuppressWarnings(PHPMD.CamelCasePropertyName)
* @SuppressWarnings(PHPMD.CamelCaseMethodName)
*/
class Product extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb
{
Expand Down Expand Up @@ -71,9 +77,20 @@ class Product extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb

/**
* @var \Magento\Catalog\Model\Product\Media\Config
* @deprecated unused
*/
protected $_mediaConfig;

/**
* @var \Magento\Catalog\Model\Product
*/
protected $productModel;

/**
* @var \Magento\Catalog\Helper\Image
*/
protected $catalogImageHelper;

/**
* @param \Magento\Framework\Model\ResourceModel\Db\Context $context
* @param \Magento\Sitemap\Helper\Data $sitemapData
Expand All @@ -85,6 +102,8 @@ class Product extends \Magento\Framework\Model\ResourceModel\Db\AbstractDb
* @param \Magento\Catalog\Model\Product\Gallery\ReadHandler $mediaGalleryReadHandler
* @param \Magento\Catalog\Model\Product\Media\Config $mediaConfig
* @param string $connectionName
* @param \Magento\Catalog\Model\Product $productModel
* @param \Magento\Catalog\Helper\Image $catalogImageHelper
* @SuppressWarnings(PHPMD.ExcessiveParameterList)
*/
public function __construct(
Expand All @@ -97,7 +116,9 @@ public function __construct(
\Magento\Catalog\Model\ResourceModel\Product\Gallery $mediaGalleryResourceModel,
\Magento\Catalog\Model\Product\Gallery\ReadHandler $mediaGalleryReadHandler,
\Magento\Catalog\Model\Product\Media\Config $mediaConfig,
$connectionName = null
$connectionName = null,
\Magento\Catalog\Model\Product $productModel = null,
\Magento\Catalog\Helper\Image $catalogImageHelper = null
) {
$this->_productResource = $productResource;
$this->_storeManager = $storeManager;
Expand All @@ -107,6 +128,8 @@ public function __construct(
$this->mediaGalleryReadHandler = $mediaGalleryReadHandler;
$this->_mediaConfig = $mediaConfig;
$this->_sitemapData = $sitemapData;
$this->productModel = $productModel ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Catalog\Model\Product::class);
$this->catalogImageHelper = $catalogImageHelper ?: \Magento\Framework\App\ObjectManager::getInstance()->get(\Magento\Catalog\Helper\Image::class);
parent::__construct($context, $connectionName);
}

Expand Down Expand Up @@ -339,7 +362,7 @@ protected function _loadProductImages($product, $storeId)
) {
$imagesCollection = [
new \Magento\Framework\DataObject(
['url' => $this->_getMediaConfig()->getBaseMediaUrlAddition() . $product->getImage()]
['url' => $this->getProductImageUrl($product->getImage())]
),
];
}
Expand All @@ -348,7 +371,7 @@ protected function _loadProductImages($product, $storeId)
// Determine thumbnail path
$thumbnail = $product->getThumbnail();
if ($thumbnail && $product->getThumbnail() != self::NOT_SELECTED_IMAGE) {
$thumbnail = $this->_getMediaConfig()->getBaseMediaUrlAddition() . $thumbnail;
$thumbnail = $this->getProductImageUrl($thumbnail);
} else {
$thumbnail = $imagesCollection[0]->getUrl();
}
Expand Down Expand Up @@ -378,11 +401,10 @@ protected function _getAllProductImages($product, $storeId)

$imagesCollection = [];
if ($gallery) {
$productMediaPath = $this->_getMediaConfig()->getBaseMediaUrlAddition();
foreach ($gallery as $image) {
$imagesCollection[] = new \Magento\Framework\DataObject(
[
'url' => $productMediaPath . $image['file'],
'url' => $this->getProductImageUrl($image['file']),
'caption' => $image['label'] ? $image['label'] : $image['label_default'],
]
);
Expand All @@ -396,9 +418,28 @@ protected function _getAllProductImages($product, $storeId)
* Get media config
*
* @return \Magento\Catalog\Model\Product\Media\Config
* @deprecated No longer used, as we're getting full image URL from getProductImageUrl method
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add @see annotation with the reference to the updated implementation.

* @see getProductImageUrl()
*/
protected function _getMediaConfig()
{
return $this->_mediaConfig;
}

/**
* Get product image URL from image filename and path
*
* @param string $image
* @return mixed
*/
private function getProductImageUrl($image)
{
$productObject = $this->productModel;
$imgUrl = $this->catalogImageHelper
->init($productObject, 'product_page_image_large')
->setImageFile($image)
->getUrl();

return $imgUrl;
}
}
14 changes: 9 additions & 5 deletions app/code/Magento/Sitemap/Model/Sitemap.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
* @method \Magento\Sitemap\Model\Sitemap setStoreId(int $value)
* @SuppressWarnings(PHPMD.TooManyFields)
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
* @SuppressWarnings(PHPMD.CamelCasePropertyName)
* @SuppressWarnings(PHPMD.CamelCaseMethodName)
*/
class Sitemap extends \Magento\Framework\Model\AbstractModel
{
Expand Down Expand Up @@ -448,7 +451,7 @@ protected function _isSplitRequired($row)
* @param null|string $lastmod
* @param null|string $changefreq
* @param null|string $priority
* @param null|array $images
* @param null|array|\Magento\Framework\DataObject $images
* @return string
* Sitemap images
* @see http://support.google.com/webmasters/bin/answer.py?hl=en&answer=178636
Expand All @@ -473,7 +476,7 @@ protected function _getSitemapRow($url, $lastmod = null, $changefreq = null, $pr
// Add Images to sitemap
foreach ($images->getCollection() as $image) {
$row .= '<image:image>';
$row .= '<image:loc>' . htmlspecialchars($this->_getMediaUrl($image->getUrl())) . '</image:loc>';
$row .= '<image:loc>' . htmlspecialchars($image->getUrl()) . '</image:loc>';
$row .= '<image:title>' . htmlspecialchars($images->getTitle()) . '</image:title>';
if ($image->getCaption()) {
$row .= '<image:caption>' . htmlspecialchars($image->getCaption()) . '</image:caption>';
Expand All @@ -483,9 +486,7 @@ protected function _getSitemapRow($url, $lastmod = null, $changefreq = null, $pr
// Add PageMap image for Google web search
$row .= '<PageMap xmlns="http://www.google.com/schemas/sitemap-pagemap/1.0"><DataObject type="thumbnail">';
$row .= '<Attribute name="name" value="' . htmlspecialchars($images->getTitle()) . '"/>';
$row .= '<Attribute name="src" value="' . htmlspecialchars(
$this->_getMediaUrl($images->getThumbnail())
) . '"/>';
$row .= '<Attribute name="src" value="' . htmlspecialchars($images->getThumbnail()) . '"/>';
$row .= '</DataObject></PageMap>';
}

Expand Down Expand Up @@ -591,6 +592,7 @@ protected function _getBaseDir()
*/
protected function _getStoreBaseUrl($type = \Magento\Framework\UrlInterface::URL_TYPE_LINK)
{
/** @var \Magento\Store\Model\Store $store */
$store = $this->_storeManager->getStore($this->getStoreId());
$isSecure = $store->isUrlSecure();
return rtrim($store->getBaseUrl($type, $isSecure), '/') . '/';
Expand All @@ -613,6 +615,8 @@ protected function _getUrl($url, $type = \Magento\Framework\UrlInterface::URL_TY
*
* @param string $url
* @return string
* @deprecated No longer used, as we're generating product image URLs inside collection instead
* @see \Magento\Sitemap\Model\ResourceModel\Catalog\Product::_loadProductImages()
*/
protected function _getMediaUrl($url)
{
Expand Down
11 changes: 8 additions & 3 deletions app/code/Magento/Sitemap/Test/Unit/Model/SitemapTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,8 @@ protected function _getModelMock($mockBeforeSave = false)
]
)
);

$storeBaseMediaUrl = 'http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/';
$this->_sitemapProductMock->expects(
$this->any()
)->method(
Expand All @@ -553,13 +555,16 @@ protected function _getModelMock($mockBeforeSave = false)
[
'collection' => [
new \Magento\Framework\DataObject(
['url' => 'image1.png', 'caption' => 'caption & > title < "']
[
'url' => $storeBaseMediaUrl.'i/m/image1.png',
'caption' => 'caption & > title < "'
]
),
new \Magento\Framework\DataObject(
['url' => 'image_no_caption.png', 'caption' => null]
['url' => $storeBaseMediaUrl.'i/m/image_no_caption.png', 'caption' => null]
),
],
'thumbnail' => 'thumbnail.jpg',
'thumbnail' => $storeBaseMediaUrl.'t/h/thumbnail.jpg',
'title' => 'Product & > title < "',
]
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@
<changefreq>monthly</changefreq>
<priority>0.5</priority>
<image:image>
<image:loc>http://store.com/image1.png</image:loc>
<image:loc>http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/i/m/image1.png</image:loc>
<image:title>Product &amp; &gt; title &lt; &quot;</image:title>
<image:caption>caption &amp; &gt; title &lt; &quot;</image:caption>
</image:image>
<image:image>
<image:loc>http://store.com/image_no_caption.png</image:loc>
<image:loc>http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/i/m/image_no_caption.png</image:loc>
<image:title>Product &amp; &gt; title &lt; &quot;</image:title>
</image:image>
<PageMap xmlns="http://www.google.com/schemas/sitemap-pagemap/1.0">
<DataObject type="thumbnail">
<Attribute name="name" value="Product &amp; &gt; title &lt; &quot;"/>
<Attribute name="src" value="http://store.com/thumbnail.jpg"/>
<Attribute name="src" value="http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/t/h/thumbnail.jpg"/>
</DataObject>
</PageMap>
</url>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,18 @@
<changefreq>monthly</changefreq>
<priority>0.5</priority>
<image:image>
<image:loc>http://store.com/image1.png</image:loc>
<image:loc>http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/i/m/image1.png</image:loc>
<image:title>Product &amp; &gt; title &lt; &quot;</image:title>
<image:caption>caption &amp; &gt; title &lt; &quot;</image:caption>
</image:image>
<image:image>
<image:loc>http://store.com/image_no_caption.png</image:loc>
<image:loc>http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/i/m/image_no_caption.png</image:loc>
<image:title>Product &amp; &gt; title &lt; &quot;</image:title>
</image:image>
<PageMap xmlns="http://www.google.com/schemas/sitemap-pagemap/1.0">
<DataObject type="thumbnail">
<Attribute name="name" value="Product &amp; &gt; title &lt; &quot;"/>
<Attribute name="src" value="http://store.com/thumbnail.jpg"/>
<Attribute name="src" value="http://store.com/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff/t/h/thumbnail.jpg"/>
</DataObject>
</PageMap>
</url>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
*/
class ProductTest extends \PHPUnit_Framework_TestCase
{
/**
* Base product image path
*/
const BASE_IMAGE_PATH = 'http://localhost/pub/media/catalog/product/cache/c9e0b0ef589f3508e5ba515cde53c5ff';

/**
* Test getCollection None images
* 1) Check that image attributes were not loaded
Expand Down Expand Up @@ -72,20 +77,20 @@ public function testGetCollectionAll()
$this->assertNotEmpty($products[4]->getImages(), 'Images were not loaded');
$this->assertEquals('Simple Images', $products[4]->getImages()->getTitle(), 'Incorrect title');
$this->assertEquals(
'catalog/product/m/a/magento_image_sitemap.png',
self::BASE_IMAGE_PATH.'/m/a/magento_image_sitemap.png',
$products[4]->getImages()->getThumbnail(),
'Incorrect thumbnail'
);
$this->assertCount(2, $products[4]->getImages()->getCollection(), 'Not all images were loaded');

$imagesCollection = $products[4]->getImages()->getCollection();
$this->assertEquals(
'catalog/product/m/a/magento_image_sitemap.png',
self::BASE_IMAGE_PATH.'/m/a/magento_image_sitemap.png',
$imagesCollection[0]->getUrl(),
'Incorrect image url'
);
$this->assertEquals(
'catalog/product/s/e/second_image.png',
self::BASE_IMAGE_PATH.'/s/e/second_image.png',
$imagesCollection[1]->getUrl(),
'Incorrect image url'
);
Expand All @@ -97,12 +102,12 @@ public function testGetCollectionAll()
$imagesCollection = $products[5]->getImages()->getCollection();
$this->assertCount(1, $imagesCollection);
$this->assertEquals(
'catalog/product/s/e/second_image_1.png',
self::BASE_IMAGE_PATH.'/s/e/second_image_1.png',
$imagesCollection[0]->getUrl(),
'Image url is incorrect'
);
$this->assertEquals(
'catalog/product/s/e/second_image_1.png',
self::BASE_IMAGE_PATH.'/s/e/second_image_1.png',
$products[5]->getImages()->getThumbnail(),
'Product thumbnail is incorrect'
);
Expand Down Expand Up @@ -140,15 +145,15 @@ public function testGetCollectionBase()
$this->assertNotEmpty($products[4]->getImages(), 'Images were not loaded');
$this->assertEquals('Simple Images', $products[4]->getImages()->getTitle(), 'Incorrect title');
$this->assertEquals(
'catalog/product/s/e/second_image.png',
self::BASE_IMAGE_PATH.'/s/e/second_image.png',
$products[4]->getImages()->getThumbnail(),
'Incorrect thumbnail'
);
$this->assertCount(1, $products[4]->getImages()->getCollection(), 'Number of loaded images is incorrect');

$imagesCollection = $products[4]->getImages()->getCollection();
$this->assertEquals(
'catalog/product/s/e/second_image.png',
self::BASE_IMAGE_PATH.'/s/e/second_image.png',
$imagesCollection[0]->getUrl(),
'Incorrect image url'
);
Expand Down