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

[Backport] 21842: don't cache absolute file paths in validator factory #22805

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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,
osrecio marked this conversation as resolved.
Show resolved Hide resolved
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