From 7d0d01e327a54b460d334165e7a68fcae7456f87 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 e266d9af43a01fce13d98001b335bf69b4219eee 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 83b34b89b5caffb1b2eef74423090e7f343b0e18 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 f3d4d9611fdbbec3c4eddca917e86e3fae272f0d 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 875fa4ce039a177d4f0f597f392a68218cfea007 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,