From 1db265d90369de1a647c8aead9fc7fc600b7008d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20F=C3=BChr?= Date: Wed, 20 Mar 2019 00:30:02 +0100 Subject: [PATCH 1/5] 21842 - do not cache absolute file paths Github issue: https://github.com/magento/magento2/issues/21842 In customer and customer_address validation context the validator factory no longer caches absolute file paths for the validation.xml files (currently two file) as the file content is evaluated later in the filesystem directly and the file paths may not be correct in a multi server setup with shared cache (id_prefix) --- lib/internal/Magento/Framework/Validator/Factory.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/lib/internal/Magento/Framework/Validator/Factory.php b/lib/internal/Magento/Framework/Validator/Factory.php index f2089c662e955..4e82c89720462 100644 --- a/lib/internal/Magento/Framework/Validator/Factory.php +++ b/lib/internal/Magento/Framework/Validator/Factory.php @@ -75,17 +75,7 @@ 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'); } } From 30f3fe1c23492b57266ed65e062d1f9b9f9c6ae7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20F=C3=BChr?= Date: Wed, 20 Mar 2019 11:46:41 +0100 Subject: [PATCH 2/5] clean up Validator Factory - adapt unit test --- .../Magento/Framework/Validator/Factory.php | 90 +++++-------------- .../Validator/Test/Unit/FactoryTest.php | 76 +--------------- 2 files changed, 24 insertions(+), 142 deletions(-) diff --git a/lib/internal/Magento/Framework/Validator/Factory.php b/lib/internal/Magento/Framework/Validator/Factory.php index 4e82c89720462..7a9eacac6ddbe 100644 --- a/lib/internal/Magento/Framework/Validator/Factory.php +++ b/lib/internal/Magento/Framework/Validator/Factory.php @@ -6,24 +6,26 @@ namespace Magento\Framework\Validator; -use Magento\Framework\Cache\FrontendInterface; +use Magento\Framework\Module\Dir\Reader; +use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\Validator; +/** + * Factory for \Magento\Framework\Validator and \Magento\Framework\Validator\Builder. + */ class Factory { - /** cache key */ - const CACHE_KEY = __CLASS__; - /** - * @var \Magento\Framework\ObjectManagerInterface + * @var ObjectManagerInterface */ protected $_objectManager; /** * Validator config files * - * @var array|null + * @var iterable|null */ - protected $_configFiles = null; + protected $_configFiles; /** * @var bool @@ -31,40 +33,22 @@ 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 */ public function __construct( - \Magento\Framework\ObjectManagerInterface $objectManager, - \Magento\Framework\Module\Dir\Reader $moduleReader, - FrontendInterface $cache + ObjectManagerInterface $objectManager, + Reader $moduleReader ) { $this->_objectManager = $objectManager; $this->moduleReader = $moduleReader; - $this->cache = $cache; } /** @@ -83,6 +67,7 @@ protected function _initializeConfigList() * Create and set default translator to \Magento\Framework\Validator\AbstractValidator. * * @return void + * @throws \Zend_Translate_Exception */ protected function _initializeDefaultTranslator() { @@ -95,7 +80,7 @@ protected function _initializeDefaultTranslator() /** @var \Magento\Framework\Translate\Adapter $translator */ $translator = $this->_objectManager->create(\Magento\Framework\Translate\Adapter::class); $translator->setOptions(['translator' => $translatorCallback]); - \Magento\Framework\Validator\AbstractValidator::setDefaultTranslator($translator); + AbstractValidator::setDefaultTranslator($translator); $this->isDefaultTranslatorInitialized = true; } } @@ -105,14 +90,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] ); } @@ -123,7 +109,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) { @@ -137,43 +124,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; - } } diff --git a/lib/internal/Magento/Framework/Validator/Test/Unit/FactoryTest.php b/lib/internal/Magento/Framework/Validator/Test/Unit/FactoryTest.php index 5511627c6dcc3..73a8c95c9a2ff 100644 --- a/lib/internal/Magento/Framework/Validator/Test/Unit/FactoryTest.php +++ b/lib/internal/Magento/Framework/Validator/Test/Unit/FactoryTest.php @@ -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 */ @@ -55,11 +40,6 @@ class FactoryTest extends \PHPUnit\Framework\TestCase */ private $factory; - /** - * @var string - */ - private $jsonString = '["\/tmp\/moduleOne\/etc\/validation.xml"]'; - /** * @var array */ @@ -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 - ); } /** @@ -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') From 18f3a05cfff05472f0c22bdfe615913773dc7ad6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20F=C3=BChr?= Date: Wed, 20 Mar 2019 14:48:40 +0100 Subject: [PATCH 3/5] untouch static method call to prevent codacy fail - refactoring of static method use should be in a separate pull request --- lib/internal/Magento/Framework/Validator/Factory.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Validator/Factory.php b/lib/internal/Magento/Framework/Validator/Factory.php index 7a9eacac6ddbe..198f4fb6730fa 100644 --- a/lib/internal/Magento/Framework/Validator/Factory.php +++ b/lib/internal/Magento/Framework/Validator/Factory.php @@ -8,6 +8,7 @@ use Magento\Framework\Module\Dir\Reader; use Magento\Framework\ObjectManagerInterface; +use Magento\Framework\Phrase; use Magento\Framework\Validator; /** @@ -75,12 +76,12 @@ protected function _initializeDefaultTranslator() // 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); $translator->setOptions(['translator' => $translatorCallback]); - AbstractValidator::setDefaultTranslator($translator); + \Magento\Framework\Validator\AbstractValidator::setDefaultTranslator($translator); $this->isDefaultTranslatorInitialized = true; } } From 6c27d26778442a5e36dba8b9bf4a56e9b62bc1aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20F=C3=BChr?= Date: Mon, 1 Apr 2019 10:27:38 +0200 Subject: [PATCH 4/5] changes due to backword compatiblity constraints --- .../Magento/Framework/Validator/Factory.php | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Validator/Factory.php b/lib/internal/Magento/Framework/Validator/Factory.php index 198f4fb6730fa..87c29dd6681c3 100644 --- a/lib/internal/Magento/Framework/Validator/Factory.php +++ b/lib/internal/Magento/Framework/Validator/Factory.php @@ -10,12 +10,20 @@ 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 + * + * @deprecated + */ + const CACHE_KEY = __CLASS__; + /** * @var ObjectManagerInterface */ @@ -26,7 +34,7 @@ class Factory * * @var iterable|null */ - protected $_configFiles; + protected $_configFiles = null; /** * @var bool @@ -43,10 +51,12 @@ class Factory * * @param ObjectManagerInterface $objectManager * @param Reader $moduleReader + * @param FrontendInterface $cache @deprecated */ public function __construct( ObjectManagerInterface $objectManager, - Reader $moduleReader + Reader $moduleReader, + FrontendInterface $cache ) { $this->_objectManager = $objectManager; $this->moduleReader = $moduleReader; From ac0559e60f0269765eacdd66e4e6586ef36e6f34 Mon Sep 17 00:00:00 2001 From: Pavel Bystritsky Date: Thu, 18 Apr 2019 15:31:02 +0300 Subject: [PATCH 5/5] magento/magento2#21856: Static test fix. --- lib/internal/Magento/Framework/Validator/Factory.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/Magento/Framework/Validator/Factory.php b/lib/internal/Magento/Framework/Validator/Factory.php index 87c29dd6681c3..2a296f7cdcb24 100644 --- a/lib/internal/Magento/Framework/Validator/Factory.php +++ b/lib/internal/Magento/Framework/Validator/Factory.php @@ -52,6 +52,7 @@ class Factory * @param ObjectManagerInterface $objectManager * @param Reader $moduleReader * @param FrontendInterface $cache @deprecated + * @SuppressWarnings(PHPMD.UnusedFormalParameter) */ public function __construct( ObjectManagerInterface $objectManager,