Skip to content
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

21842: don't cache absolute file paths in validator factory #21856

104 changes: 31 additions & 73 deletions lib/internal/Magento/Framework/Validator/Factory.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,22 +6,33 @@

namespace Magento\Framework\Validator;

use Magento\Framework\Module\Dir\Reader;
use Magento\Framework\ObjectManagerInterface;
use Magento\Framework\Phrase;
use Magento\Framework\Validator;
use Magento\Framework\Cache\FrontendInterface;

/**
* Factory for \Magento\Framework\Validator and \Magento\Framework\Validator\Builder.
*/
class Factory
{
/** cache key */
/**
* cache key
*
* @deprecated
*/
const CACHE_KEY = __CLASS__;

/**
* @var \Magento\Framework\ObjectManagerInterface
* @var ObjectManagerInterface
*/
protected $_objectManager;

/**
* Validator config files
*
* @var array|null
* @var iterable|null
*/
protected $_configFiles = null;

Expand All @@ -31,40 +42,25 @@ class Factory
private $isDefaultTranslatorInitialized = false;

/**
* @var \Magento\Framework\Module\Dir\Reader
* @var Reader
*/
private $moduleReader;

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

/**
* @var \Magento\Framework\Serialize\SerializerInterface
*/
private $serializer;

/**
* @var \Magento\Framework\Config\FileIteratorFactory
*/
private $fileIteratorFactory;

/**
* Initialize dependencies
*
* @param \Magento\Framework\ObjectManagerInterface $objectManager
* @param \Magento\Framework\Module\Dir\Reader $moduleReader
* @param FrontendInterface $cache
* @param ObjectManagerInterface $objectManager
* @param Reader $moduleReader
* @param FrontendInterface $cache @deprecated
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
*/
public function __construct(
\Magento\Framework\ObjectManagerInterface $objectManager,
\Magento\Framework\Module\Dir\Reader $moduleReader,
ObjectManagerInterface $objectManager,
Reader $moduleReader,
FrontendInterface $cache
) {
$this->_objectManager = $objectManager;
$this->moduleReader = $moduleReader;
$this->cache = $cache;
}

/**
Expand All @@ -75,32 +71,23 @@ public function __construct(
protected function _initializeConfigList()
{
if (!$this->_configFiles) {
$this->_configFiles = $this->cache->load(self::CACHE_KEY);
if (!$this->_configFiles) {
$this->_configFiles = $this->moduleReader->getConfigurationFiles('validation.xml');
$this->cache->save(
$this->getSerializer()->serialize($this->_configFiles->toArray()),
self::CACHE_KEY
);
} else {
$filesArray = $this->getSerializer()->unserialize($this->_configFiles);
$this->_configFiles = $this->getFileIteratorFactory()->create(array_keys($filesArray));
}
$this->_configFiles = $this->moduleReader->getConfigurationFiles('validation.xml');
}
}

/**
* Create and set default translator to \Magento\Framework\Validator\AbstractValidator.
*
* @return void
* @throws \Zend_Translate_Exception
*/
protected function _initializeDefaultTranslator()
{
if (!$this->isDefaultTranslatorInitialized) {
// Pass translations to \Magento\Framework\TranslateInterface from validators
$translatorCallback = function () {
$argc = func_get_args();
return (string)new \Magento\Framework\Phrase(array_shift($argc), $argc);
return (string)new Phrase(array_shift($argc), $argc);
};
/** @var \Magento\Framework\Translate\Adapter $translator */
$translator = $this->_objectManager->create(\Magento\Framework\Translate\Adapter::class);
Expand All @@ -115,14 +102,15 @@ protected function _initializeDefaultTranslator()
*
* Will instantiate \Magento\Framework\Validator\Config
*
* @return \Magento\Framework\Validator\Config
* @return Config
* @throws \Zend_Translate_Exception
*/
public function getValidatorConfig()
{
$this->_initializeConfigList();
$this->_initializeDefaultTranslator();
return $this->_objectManager->create(
\Magento\Framework\Validator\Config::class,
Config::class,
['configFiles' => $this->_configFiles]
);
}
Expand All @@ -133,7 +121,8 @@ public function getValidatorConfig()
* @param string $entityName
* @param string $groupName
* @param array|null $builderConfig
* @return \Magento\Framework\Validator\Builder
* @return Builder
* @throws \Zend_Translate_Exception
*/
public function createValidatorBuilder($entityName, $groupName, array $builderConfig = null)
{
Expand All @@ -147,43 +136,12 @@ public function createValidatorBuilder($entityName, $groupName, array $builderCo
* @param string $entityName
* @param string $groupName
* @param array|null $builderConfig
* @return \Magento\Framework\Validator
* @return Validator
* @throws \Zend_Translate_Exception
*/
public function createValidator($entityName, $groupName, array $builderConfig = null)
{
$this->_initializeDefaultTranslator();
return $this->getValidatorConfig()->createValidator($entityName, $groupName, $builderConfig);
}

/**
* Get serializer
*
* @return \Magento\Framework\Serialize\SerializerInterface
* @deprecated 100.2.0
*/
private function getSerializer()
{
if ($this->serializer === null) {
$this->serializer = $this->_objectManager->get(
\Magento\Framework\Serialize\SerializerInterface::class
);
}
return $this->serializer;
}

/**
* Get file iterator factory
*
* @return \Magento\Framework\Config\FileIteratorFactory
* @deprecated 100.2.0
*/
private function getFileIteratorFactory()
{
if ($this->fileIteratorFactory === null) {
$this->fileIteratorFactory = $this->_objectManager->get(
\Magento\Framework\Config\FileIteratorFactory::class
);
}
return $this->fileIteratorFactory;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,6 @@ class FactoryTest extends \PHPUnit\Framework\TestCase
*/
private $validatorConfigMock;

/**
* @var \Magento\Framework\Cache\FrontendInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $cacheMock;

/**
* @var \Magento\Framework\Serialize\SerializerInterface|\PHPUnit_Framework_MockObject_MockObject
*/
private $serializerMock;

/**
* @var \Magento\Framework\Config\FileIteratorFactory|\PHPUnit_Framework_MockObject_MockObject
*/
private $fileIteratorFactoryMock;

/**
* @var \Magento\Framework\Config\FileIterator|\PHPUnit_Framework_MockObject_MockObject
*/
Expand All @@ -55,11 +40,6 @@ class FactoryTest extends \PHPUnit\Framework\TestCase
*/
private $factory;

/**
* @var string
*/
private $jsonString = '["\/tmp\/moduleOne\/etc\/validation.xml"]';

/**
* @var array
*/
Expand Down Expand Up @@ -99,23 +79,9 @@ protected function setUp()
\Magento\Framework\Validator\Factory::class,
[
'objectManager' => $this->objectManagerMock,
'moduleReader' => $this->readerMock,
'cache' => $this->cacheMock
'moduleReader' => $this->readerMock
]
);

$this->serializerMock = $this->createMock(\Magento\Framework\Serialize\SerializerInterface::class);
$this->fileIteratorFactoryMock = $this->createMock(\Magento\Framework\Config\FileIteratorFactory::class);
$objectManager->setBackwardCompatibleProperty(
$this->factory,
'serializer',
$this->serializerMock
);
$objectManager->setBackwardCompatibleProperty(
$this->factory,
'fileIteratorFactory',
$this->fileIteratorFactoryMock
);
}

/**
Expand Down Expand Up @@ -147,46 +113,6 @@ public function testGetValidatorConfig()
);
}

public function testGetValidatorConfigCacheNotExist()
{
$this->cacheMock->expects($this->once())
->method('load')
->willReturn(false);
$this->readerMock->expects($this->once())
->method('getConfigurationFiles')
->willReturn($this->fileIteratorMock);
$this->fileIteratorMock->method('toArray')
->willReturn($this->data);
$this->cacheMock->expects($this->once())
->method('save')
->with($this->jsonString);
$this->serializerMock->expects($this->once())
->method('serialize')
->with($this->data)
->willReturn($this->jsonString);
$this->factory->getValidatorConfig();
$this->factory->getValidatorConfig();
}

public function testGetValidatorConfigCacheExist()
{
$this->cacheMock->expects($this->once())
->method('load')
->willReturn($this->jsonString);
$this->readerMock->expects($this->never())
->method('getConfigurationFiles');
$this->cacheMock->expects($this->never())
->method('save');
$this->serializerMock->expects($this->once())
->method('unserialize')
->with($this->jsonString)
->willReturn($this->data);
$this->fileIteratorFactoryMock->method('create')
->willReturn($this->fileIteratorMock);
$this->factory->getValidatorConfig();
$this->factory->getValidatorConfig();
}

public function testCreateValidatorBuilder()
{
$this->readerMock->method('getConfigurationFiles')
Expand Down