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

Author: sivaschenko

lib/internal/Magento/Framework/Validator/Factory.php

@@ -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;
 
@@ -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;
     }
 
     /**
@@ -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);
@@ -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]
         );
     }
@@ -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)
     {
@@ -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;
-    }
 }

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')