Skip to content

Commit

Permalink
Merge pull request #1114 from magento-tango/MAGETWO-66825-1
Browse files Browse the repository at this point in the history
[Tango] MAGETWO-66825: Caching of attribute options for Layered Navigation
  • Loading branch information
Volodymyr Klymenko authored May 22, 2017
2 parents bdb0464 + 512ee7c commit 938313e
Show file tree
Hide file tree
Showing 3 changed files with 258 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,46 @@
*/
namespace Magento\Eav\Model\Entity\Attribute\Frontend;

use Magento\Framework\App\CacheInterface;
use Magento\Framework\Serialize\Serializer\Json as Serializer;
use Magento\Store\Api\StoreResolverInterface;
use Magento\Framework\App\ObjectManager;
use Magento\Eav\Model\Cache\Type as CacheType;
use Magento\Eav\Model\Entity\Attribute;
use Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory;

/**
* @api
*/
abstract class AbstractFrontend implements \Magento\Eav\Model\Entity\Attribute\Frontend\FrontendInterface
{
/**
* Default cache tags values
* will be used if no values in the constructor provided
* @var array
*/
private static $defaultCacheTags = [CacheType::CACHE_TAG, Attribute::CACHE_TAG];

/**
* @var CacheInterface
*/
private $cache;

/**
* @var StoreResolverInterface
*/
private $storeResolver;

/**
* @var Serializer
*/
private $serializer;

/**
* @var array
*/
private $cacheTags;

/**
* Reference to the attribute instance
*
Expand All @@ -24,17 +59,30 @@ abstract class AbstractFrontend implements \Magento\Eav\Model\Entity\Attribute\F
protected $_attribute;

/**
* @var \Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory
* @var BooleanFactory
*/
protected $_attrBooleanFactory;

/**
* @param \Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory $attrBooleanFactory
* @param BooleanFactory $attrBooleanFactory
* @param CacheInterface $cache
* @param StoreResolverInterface $storeResolver
* @param array $cacheTags
* @param Serializer $serializer
* @codeCoverageIgnore
*/
public function __construct(\Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory $attrBooleanFactory)
{
public function __construct(
BooleanFactory $attrBooleanFactory,
CacheInterface $cache = null,
StoreResolverInterface $storeResolver = null,
array $cacheTags = null,
Serializer $serializer = null
) {
$this->_attrBooleanFactory = $attrBooleanFactory;
$this->cache = $cache ?: ObjectManager::getInstance()->get(CacheInterface::class);
$this->storeResolver = $storeResolver ?: ObjectManager::getInstance()->get(StoreResolverInterface::class);
$this->cacheTags = $cacheTags ?: self::$defaultCacheTags;
$this->serializer = $serializer ?: ObjectManager::getInstance()->get(Serializer::class);
}

/**
Expand Down Expand Up @@ -249,7 +297,21 @@ public function getConfigField($fieldName)
*/
public function getSelectOptions()
{
return $this->getAttribute()->getSource()->getAllOptions();
$cacheKey = 'attribute-navigation-option-' .
$this->getAttribute()->getAttributeCode() . '-' .
$this->storeResolver->getCurrentStoreId();
$optionString = $this->cache->load($cacheKey);
if (false === $optionString) {
$options = $this->getAttribute()->getSource()->getAllOptions();
$this->cache->save(
$this->serializer->serialize($options),
$cacheKey,
$this->cacheTags
);
} else {
$options = $this->serializer->unserialize($optionString);
}
return $options;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@
namespace Magento\Eav\Test\Unit\Model\Entity\Attribute\Frontend;

use Magento\Eav\Model\Entity\Attribute\Frontend\DefaultFrontend;
use Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory;
use Magento\Framework\Serialize\Serializer\Json as Serializer;
use Magento\Store\Api\StoreResolverInterface;
use Magento\Framework\App\CacheInterface;
use Magento\Eav\Model\Entity\Attribute\AbstractAttribute;
use Magento\Eav\Model\Entity\Attribute\Source\AbstractSource;

class DefaultFrontendTest extends \PHPUnit_Framework_TestCase
{
Expand All @@ -15,18 +21,73 @@ class DefaultFrontendTest extends \PHPUnit_Framework_TestCase
protected $model;

/**
* @var \Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory|\PHPUnit_Framework_MockObject_MockObject
* @var BooleanFactory|\PHPUnit_Framework_MockObject_MockObject
*/
protected $booleanFactory;

/**
* @var Serializer|\PHPUnit_Framework_MockObject_MockObject
*/
private $serializerMock;

/**
* @var StoreResolverInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $storeResolverMock;

/**
* @var CacheInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $cacheMock;

/**
* @var AbstractAttribute|\PHPUnit_Framework_MockObject_MockObject
*/
private $attributeMock;

/**
* @var array
*/
private $cacheTags;

/**
* @var AbstractSource|\PHPUnit_Framework_MockObject_MockObject
*/
private $sourceMock;

protected function setUp()
{
$this->booleanFactory = $this->getMockBuilder(\Magento\Eav\Model\Entity\Attribute\Source\BooleanFactory::class)
$this->cacheTags = ['tag1', 'tag2'];

$this->booleanFactory = $this->getMockBuilder(BooleanFactory::class)
->disableOriginalConstructor()
->getMock();
$this->serializerMock = $this->getMockBuilder(Serializer::class)
->getMock();
$this->storeResolverMock = $this->getMockBuilder(StoreResolverInterface::class)
->getMockForAbstractClass();
$this->cacheMock = $this->getMockBuilder(CacheInterface::class)
->getMockForAbstractClass();
$this->attributeMock = $this->getMockBuilder(AbstractAttribute::class)
->disableOriginalConstructor()
->setMethods(['getAttributeCode', 'getSource'])
->getMockForAbstractClass();
$this->sourceMock = $this->getMockBuilder(AbstractSource::class)
->disableOriginalConstructor()
->setMethods(['getAllOptions'])
->getMockForAbstractClass();

$this->model = new DefaultFrontend(
$this->booleanFactory
$objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$this->model = $objectManager->getObject(
DefaultFrontend::class,
[
'_attrBooleanFactory' => $this->booleanFactory,
'cache' => $this->cacheMock,
'storeResolver' => $this->storeResolverMock,
'serializer' => $this->serializerMock,
'_attribute' => $this->attributeMock,
'cacheTags' => $this->cacheTags
]
);
}

Expand Down Expand Up @@ -118,4 +179,39 @@ public function testGetClassLength()
$this->assertContains('maximum-length-2', $result);
$this->assertContains('validate-length', $result);
}

public function testGetSelectOptions()
{
$storeId = 1;
$attributeCode = 'attr1';
$cacheKey = 'attribute-navigation-option-' . $attributeCode . '-' . $storeId;
$options = ['option1', 'option2'];
$serializedOptions = "{['option1', 'option2']}";

$this->storeResolverMock->expects($this->once())
->method('getCurrentStoreId')
->willReturn($storeId);
$this->attributeMock->expects($this->once())
->method('getAttributeCode')
->willReturn($attributeCode);
$this->cacheMock->expects($this->once())
->method('load')
->with($cacheKey)
->willReturn(false);
$this->attributeMock->expects($this->once())
->method('getSource')
->willReturn($this->sourceMock);
$this->sourceMock->expects($this->once())
->method('getAllOptions')
->willReturn($options);
$this->serializerMock->expects($this->once())
->method('serialize')
->with($options)
->willReturn($serializedOptions);
$this->cacheMock->expects($this->once())
->method('save')
->with($serializedOptions, $cacheKey, $this->cacheTags);

$this->assertSame($options, $this->model->getSelectOptions());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
<?php
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/
namespace Magento\Eav\Model\Entity\Attribute\Frontend;

use Magento\TestFramework\Helper\Bootstrap;
use Magento\TestFramework\Helper\CacheCleaner;
use Magento\Eav\Model\Entity\Attribute\AbstractAttribute;
use Magento\Framework\App\CacheInterface;
use Magento\Store\Api\StoreResolverInterface;
use Magento\Framework\Serialize\Serializer\Json as Serializer;
use Magento\Eav\Model\Entity\Attribute;

/**
* @magentoAppIsolation enabled
*/
class DefaultFrontendTest extends \PHPUnit_Framework_TestCase
{
/**
* @var DefaultFrontend
*/
private $defaultFrontend;

/**
* @var \Magento\Framework\ObjectManagerInterface
*/
private $objectManager;

/**
* @var AbstractAttribute
*/
private $attribute;

/**
* @var array
*/
private $options;

/**
* @var CacheInterface
*/
private $cache;

/**
* @var StoreResolverInterface
*/
private $storeResolver;

/**
* @var Serializer
*/
private $serializer;

protected function setUp()
{
CacheCleaner::cleanAll();
$this->objectManager = Bootstrap::getObjectManager();

$this->defaultFrontend = $this->objectManager->get(DefaultFrontend::class);
$this->cache = $this->objectManager->get(CacheInterface::class);
$this->storeResolver = $this->objectManager->get(StoreResolverInterface::class);
$this->serializer = $this->objectManager->get(Serializer::class);
$this->attribute = $this->objectManager->get(Attribute::class);

$this->attribute->setAttributeCode('store_id');
$this->options = $this->attribute->getSource()->getAllOptions();
$this->defaultFrontend->setAttribute($this->attribute);
}

public function testGetSelectOptions()
{
$this->assertSame($this->options, $this->defaultFrontend->getSelectOptions());
$this->assertSame(
$this->serializer->serialize($this->options),
$this->cache->load($this->getCacheKey())
);
}

/**
* Cache key generation
* @return string
*/
private function getCacheKey()
{
return 'attribute-navigation-option-' .
$this->defaultFrontend->getAttribute()->getAttributeCode() . '-' .
$this->storeResolver->getCurrentStoreId();
}
}

1 comment on commit 938313e

@hostep
Copy link
Contributor

@hostep hostep commented on 938313e May 22, 2017

Choose a reason for hiding this comment

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

@voleye: be aware that this commit introduces a nasty bug where the wrong translation of the attribute option is getting cached if you work with multiple storeviews. This is because the StoreResolver isn't the correct class to get the current store id, see #9704 for more details.

Please sign in to comment.