diff --git a/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php b/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php index de40210ebc76f..b444d746f8d0f 100644 --- a/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php +++ b/app/code/Magento/CatalogImportExport/Model/Import/Product/Validator/Media.php @@ -6,15 +6,36 @@ namespace Magento\CatalogImportExport\Model\Import\Product\Validator; use Magento\CatalogImportExport\Model\Import\Product\RowValidatorInterface; +use Magento\Framework\App\ObjectManager; +use Magento\Framework\Url\Validator; class Media extends AbstractImportValidator implements RowValidatorInterface { + /** + * @deprecated As this regexp doesn't give guarantee of correct url validation + * @see \Magento\Framework\Url\Validator::isValid() + */ const URL_REGEXP = '|^http(s)?://[a-z0-9-]+(.[a-z0-9-]+)*(:[0-9]+)?(/.*)?$|i'; const PATH_REGEXP = '#^(?!.*[\\/]\.{2}[\\/])(?!\.{2}[\\/])[-\w.\\/]+$#'; const ADDITIONAL_IMAGES = 'additional_images'; + /** + * The url validator. Checks if given url is valid. + * + * @var Validator + */ + private $validator; + + /** + * @param Validator $validator The url validator + */ + public function __construct(Validator $validator = null) + { + $this->validator = $validator ?: ObjectManager::getInstance()->get(Validator::class); + } + /** * @deprecated * @see \Magento\CatalogImportExport\Model\Import\Product::getMultipleValueSeparator() @@ -27,6 +48,8 @@ class Media extends AbstractImportValidator implements RowValidatorInterface /** * @param string $string * @return bool + * @deprecated As this method doesn't give guarantee of correct url validation. + * @see \Magento\Framework\Url\Validator::isValid() It provides better url validation. */ protected function checkValidUrl($string) { @@ -64,7 +87,7 @@ public function isValid($value) $valid = true; foreach ($this->mediaAttributes as $attribute) { if (isset($value[$attribute]) && strlen($value[$attribute])) { - if (!$this->checkPath($value[$attribute]) && !$this->checkValidUrl($value[$attribute])) { + if (!$this->checkPath($value[$attribute]) && !$this->validator->isValid($value[$attribute])) { $this->_addMessages( [ sprintf( @@ -79,7 +102,7 @@ public function isValid($value) } if (isset($value[self::ADDITIONAL_IMAGES]) && strlen($value[self::ADDITIONAL_IMAGES])) { foreach (explode($this->context->getMultipleValueSeparator(), $value[self::ADDITIONAL_IMAGES]) as $image) { - if (!$this->checkPath($image) && !$this->checkValidUrl($image)) { + if (!$this->checkPath($image) && !$this->validator->isValid($image)) { $this->_addMessages( [ sprintf( diff --git a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php index ba3f1f77954e7..209d999320906 100644 --- a/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php +++ b/app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/Product/Validator/MediaTest.php @@ -10,6 +10,8 @@ use Magento\CatalogImportExport\Model\Import\Product\Validator\Media; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager as ObjectManagerHelper; use Magento\ImportExport\Model\Import; +use Magento\Framework\Url\Validator; +use PHPUnit_Framework_MockObject_MockObject as MockObject; class MediaTest extends \PHPUnit_Framework_TestCase { @@ -19,16 +21,35 @@ class MediaTest extends \PHPUnit_Framework_TestCase /** @var ObjectManagerHelper */ protected $objectManagerHelper; + /** + * @var Validator|MockObject + */ + private $validatorMock; + protected function setUp() { - + $this->validatorMock = $this->getMockBuilder(Validator::class) + ->disableOriginalConstructor() + ->getMock(); + $contextMock = $this->getMockBuilder(Product::class) + ->disableOriginalConstructor() + ->getMock(); + $contextMock->expects($this->any()) + ->method('getMultipleValueSeparator') + ->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR); + $contextMock->expects($this->any()) + ->method('retrieveMessageTemplate') + ->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH) + ->willReturn('%s'); + $this->objectManagerHelper = new ObjectManagerHelper($this); $this->media = $this->objectManagerHelper->getObject( Media::class, [ - + 'validator' => $this->validatorMock ] ); + $this->media->init($contextMock); } public function testInit() @@ -44,17 +65,8 @@ public function testInit() */ public function testIsValid($data, $expected) { - $contextMock = $this->getMockBuilder(Product::class) - ->disableOriginalConstructor() - ->getMock(); - $contextMock->expects($this->any()) - ->method('getMultipleValueSeparator') - ->willReturn(Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR); - $contextMock->expects($this->any()) - ->method('retrieveMessageTemplate') - ->with(Media::ERROR_INVALID_MEDIA_URL_OR_PATH) - ->willReturn('%s'); - $this->media->init($contextMock); + $this->validatorMock->expects($this->never()) + ->method('isValid'); $result = $this->media->isValid($data); $this->assertEquals($expected['result'], $result); @@ -76,6 +88,47 @@ public function testIsValidClearMessagesCall() $media->isValid([]); } + /** + * @param array $data + * @param array $expected + * @dataProvider isValidAdditionalImagesPathDataProvider + */ + public function testIsValidAdditionalImagesPath($data, $expected) + { + if ($expected['result']) { + $this->validatorMock->expects($this->never()) + ->method('isValid'); + } else { + $this->validatorMock->expects($this->once()) + ->method('isValid') + ->with($data['additional_images']) + ->willReturn(false); + } + + $result = $this->media->isValid($data); + $this->assertEquals($expected['result'], $result); + $messages = $this->media->getMessages(); + $this->assertEquals($expected['messages'], $messages); + } + + /** + * @param array $data + * @param array $expected + * @dataProvider isValidAdditionalImagesUrlDataProvider + */ + public function testIsValidAdditionalImagesUrl($data, $expected) + { + $this->validatorMock->expects($this->once()) + ->method('isValid') + ->with($data['additional_images']) + ->willReturn($expected['result']); + + $result = $this->media->isValid($data); + $this->assertEquals($expected['result'], $result); + $messages = $this->media->getMessages(); + $this->assertEquals($expected['messages'], $messages); + } + /** * @return array */ @@ -94,6 +147,15 @@ public function isMediaValidDataProvider() ['_media_image' => 1], ['result' => true,'messages' => []], ], + ]; + } + + /** + * @return array + */ + public function isValidAdditionalImagesPathDataProvider() + { + return [ 'additional_images' => [ ['additional_images' => 'image1.png,image2.jpg'], ['result' => true, 'messages' => []] @@ -101,6 +163,23 @@ public function isMediaValidDataProvider() 'additional_images_fail' => [ ['additional_images' => 'image1.png|image2.jpg|image3.gif'], ['result' => false, 'messages' => [0 => 'additional_images']] + ], + ]; + } + + /** + * @return array + */ + public function isValidAdditionalImagesUrlDataProvider() + { + return [ + 'additional_images_wrong_domain' => [ + ['additional_images' => 'https://example/images/some-name.jpg'], + ['result' => false, 'messages' => [0 => 'additional_images']], + ], + 'additional_images_url_multiple_underscores' => [ + ['additional_images' => 'https://example.com/images/some-name__with___multiple____underscores.jpg'], + ['result' => true, 'messages' => []] ] ]; } diff --git a/app/code/Magento/Config/App/Config/Type/System.php b/app/code/Magento/Config/App/Config/Type/System.php index a10462d87b205..9208d2bf2f816 100644 --- a/app/code/Magento/Config/App/Config/Type/System.php +++ b/app/code/Magento/Config/App/Config/Type/System.php @@ -6,10 +6,11 @@ namespace Magento\Config\App\Config\Type; use Magento\Framework\App\Config\ConfigTypeInterface; -use Magento\Framework\DataObject; +use Magento\Framework\App\ObjectManager; +use Magento\Config\App\Config\Type\System\Reader; /** - * Class process source, cache them and retrieve value by path + * System configuration type */ class System implements ConfigTypeInterface { @@ -23,9 +24,9 @@ class System implements ConfigTypeInterface private $source; /** - * @var DataObject + * @var array */ - private $data; + private $data = []; /** * @var \Magento\Framework\App\Config\Spi\PostProcessorInterface @@ -65,14 +66,18 @@ class System implements ConfigTypeInterface private $configType; /** - * Key name for flag which displays whether configuration is cached or not. + * @var Reader + */ + private $reader; + + /** + * List of scopes that were retrieved from configuration storage * - * Once configuration is cached additional flag pushed to cache storage - * to be able check cache existence without data load. + * Is used to make sure that we don't try to load non-existing configuration scopes. * - * @var string + * @var array */ - private $cacheExistenceKey; + private $availableDataScopes = null; /** * @param \Magento\Framework\App\Config\ConfigSourceInterface $source @@ -83,6 +88,7 @@ class System implements ConfigTypeInterface * @param \Magento\Framework\App\Config\Spi\PreProcessorInterface $preProcessor * @param int $cachingNestedLevel * @param string $configType + * @param Reader $reader */ public function __construct( \Magento\Framework\App\Config\ConfigSourceInterface $source, @@ -92,7 +98,8 @@ public function __construct( \Magento\Framework\Serialize\SerializerInterface $serializer, \Magento\Framework\App\Config\Spi\PreProcessorInterface $preProcessor, $cachingNestedLevel = 1, - $configType = self::CONFIG_TYPE + $configType = self::CONFIG_TYPE, + Reader $reader = null ) { $this->source = $source; $this->postProcessor = $postProcessor; @@ -102,150 +109,174 @@ public function __construct( $this->fallback = $fallback; $this->serializer = $serializer; $this->configType = $configType; - $this->cacheExistenceKey = $this->configType . '_CACHE_EXISTS'; + $this->reader = $reader ?: ObjectManager::getInstance()->get(Reader::class); } /** + * System configuration is separated by scopes (default, websites, stores). Configuration of a scope is inherited + * from its parent scope (store inherits website). + * + * Because there can be many scopes on single instance of application, the configuration data can be pretty large, + * so it does not make sense to load all of it on every application request. That is why we cache configuration + * data by scope and only load configuration scope when a value from that scope is requested. + * + * Possible path values: + * '' - will return whole system configuration (default scope + all other scopes) + * 'default' - will return all default scope configuration values + * '{scopeType}' - will return data from all scopes of a specified {scopeType} (websites, stores) + * '{scopeType}/{scopeCode}' - will return data for all values of the scope specified by {scopeCode} and scope type + * '{scopeType}/{scopeCode}/some/config/variable' - will return value of the config variable in the specified scope + * * @inheritdoc */ public function get($path = '') { - if ($path === null) { - $path = ''; + if ($path === '') { + $this->data = array_replace_recursive($this->loadAllData(), $this->data); + return $this->data; } - if ($this->isConfigRead($path)) { - return $this->data->getData($path); + $pathParts = explode('/', $path); + if (count($pathParts) === 1 && $pathParts[0] !== 'default') { + if (!isset($this->data[$pathParts[0]])) { + $data = $this->reader->read(); + $this->data = array_replace_recursive($data, $this->data); + } + return $this->data[$pathParts[0]]; } - - if (!empty($path) && $this->isCacheExists()) { - return $this->readFromCache($path); + $scopeType = array_shift($pathParts); + if ($scopeType === 'default') { + if (!isset($this->data[$scopeType])) { + $this->data = array_replace_recursive($this->loadDefaultScopeData($scopeType), $this->data); + } + return $this->getDataByPathParts($this->data[$scopeType], $pathParts); } - - $config = $this->loadConfig(); - $this->cacheConfig($config); - $this->data = new DataObject($config); - return $this->data->getData($path); - } - - /** - * Check whether configuration is cached - * - * In case configuration cache exists method 'load' returns - * value equal to $this->cacheExistenceKey - * - * @return bool - */ - private function isCacheExists() - { - return $this->cache->load($this->cacheExistenceKey) !== false; + $scopeId = array_shift($pathParts); + if (!isset($this->data[$scopeType][$scopeId])) { + $this->data = array_replace_recursive($this->loadScopeData($scopeType, $scopeId), $this->data); + } + return isset($this->data[$scopeType][$scopeId]) + ? $this->getDataByPathParts($this->data[$scopeType][$scopeId], $pathParts) + : null; } /** - * Explode path by '/'(forward slash) separator - * - * In case $path string contains forward slash symbol(/) the $path is exploded and parts array is returned - * In other case empty array is returned + * Load configuration data for all scopes * - * @param string $path * @return array */ - private function getPathParts($path) + private function loadAllData() { - $pathParts = []; - if (strpos($path, '/') !== false) { - $pathParts = explode('/', $path); + $cachedData = $this->cache->load($this->configType); + if ($cachedData === false) { + $data = $this->reader->read(); + } else { + $data = $this->serializer->unserialize($cachedData); } - return $pathParts; + return $data; } /** - * Check whether requested configuration data is read to memory - * - * Because of configuration is cached partially each part can be loaded separately - * Method performs check if corresponding system configuration part is already loaded to memory - * and value can be retrieved directly without cache look up + * Load configuration data for default scope * - * - * @param string $path - * @return bool + * @param string $scopeType + * @return array */ - private function isConfigRead($path) + private function loadDefaultScopeData($scopeType) { - $pathParts = $this->getPathParts($path); - return !empty($pathParts) && isset($this->data[$pathParts[0]][$pathParts[1]]); + $cachedData = $this->cache->load($this->configType . '_' . $scopeType); + if ($cachedData === false) { + $data = $this->reader->read(); + $this->cacheData($data); + } else { + $data = [$scopeType => $this->serializer->unserialize($cachedData)]; + } + return $data; } /** - * Load configuration from all the sources - * - * System configuration is loaded in 3 steps performing consecutive calls to - * Pre Processor, Fallback Processor, Post Processor accordingly + * Load configuration data for a specified scope * + * @param string $scopeType + * @param string $scopeId * @return array */ - private function loadConfig() + private function loadScopeData($scopeType, $scopeId) { - $data = $this->preProcessor->process($this->source->get()); - $this->data = new DataObject($data); - $data = $this->fallback->process($data); - $this->data = new DataObject($data); - - return $this->postProcessor->process($data); + $cachedData = $this->cache->load($this->configType . '_' . $scopeType . '_' . $scopeId); + if ($cachedData === false) { + if ($this->availableDataScopes === null) { + $cachedScopeData = $this->cache->load($this->configType . '_scopes'); + if ($cachedScopeData !== false) { + $this->availableDataScopes = $this->serializer->unserialize($cachedScopeData); + } + } + if (is_array($this->availableDataScopes) && !isset($this->availableDataScopes[$scopeType][$scopeId])) { + return [$scopeType => [$scopeId => []]]; + } + $data = $this->reader->read(); + $this->cacheData($data); + } else { + $data = [$scopeType => [$scopeId => $this->serializer->unserialize($cachedData)]]; + } + return $data; } /** - * - * Load configuration and caching it by parts. - * - * To be cached configuration is loaded first. - * Then it is cached by parts to minimize memory usage on load. - * Additional flag cached as well to give possibility check cache existence without data load. + * Cache configuration data. + * Caches data per scope to avoid reading data for all scopes on every request * * @param array $data * @return void */ - private function cacheConfig($data) + private function cacheData(array $data) { - foreach ($data as $scope => $scopeData) { - foreach ($scopeData as $key => $config) { + $this->cache->save( + $this->serializer->serialize($data), + $this->configType, + [self::CACHE_TAG] + ); + $this->cache->save( + $this->serializer->serialize($data['default']), + $this->configType . '_default', + [self::CACHE_TAG] + ); + $scopes = []; + foreach (['websites', 'stores'] as $curScopeType) { + foreach ($data[$curScopeType] as $curScopeId => $curScopeData) { + $scopes[$curScopeType][$curScopeId] = 1; $this->cache->save( - $this->serializer->serialize($config), - $this->configType . '_' . $scope . $key, + $this->serializer->serialize($curScopeData), + $this->configType . '_' . $curScopeType . '_' . $curScopeId, [self::CACHE_TAG] ); } } - $this->cache->save($this->cacheExistenceKey, $this->cacheExistenceKey, [self::CACHE_TAG]); + $this->cache->save( + $this->serializer->serialize($scopes), + $this->configType . "_scopes", + [self::CACHE_TAG] + ); } /** - * Read cached configuration + * Walk nested hash map by keys from $pathParts * - * Read section of system configuration corresponding to requested $path from cache - * Configuration stored to internal property right after load to prevent additional - * requests to cache storage - * - * @param string $path + * @param array $data to walk in + * @param array $pathParts keys path * @return mixed */ - private function readFromCache($path) + private function getDataByPathParts($data, $pathParts) { - if ($this->data === null) { - $this->data = new DataObject(); - } - - $result = null; - $pathParts = $this->getPathParts($path); - if (!empty($pathParts)) { - $result = $this->cache->load($this->configType . '_' . $pathParts[0] . $pathParts[1]); - if ($result !== false) { - $readData = $this->data->getData(); - $readData[$pathParts[0]][$pathParts[1]] = $this->serializer->unserialize($result); - $this->data->setData($readData); + foreach ($pathParts as $key) { + if ((array)$data === $data && isset($data[$key])) { + $data = $data[$key]; + } elseif ($data instanceof \Magento\Framework\DataObject) { + $data = $data->getDataByKey($key); + } else { + return null; } } - - return $this->data->getData($path); + return $data; } /** @@ -259,7 +290,7 @@ private function readFromCache($path) */ public function clean() { - $this->data = null; + $this->data = []; $this->cache->clean(\Zend_Cache::CLEANING_MODE_MATCHING_TAG, [self::CACHE_TAG]); } } diff --git a/app/code/Magento/Config/App/Config/Type/System/Reader.php b/app/code/Magento/Config/App/Config/Type/System/Reader.php new file mode 100644 index 0000000000000..d5a0f09cdd631 --- /dev/null +++ b/app/code/Magento/Config/App/Config/Type/System/Reader.php @@ -0,0 +1,71 @@ +source = $source; + $this->fallback = $fallback; + $this->preProcessor = $preProcessor; + $this->postProcessor = $postProcessor; + } + + /** + * Retrieve and process system configuration data + * + * Processing includes configuration fallback (default, website, store) and placeholder replacement + * + * @return array + */ + public function read() + { + return $this->postProcessor->process( + $this->fallback->process( + $this->preProcessor->process( + $this->source->get() + ) + ) + ); + } +} diff --git a/app/code/Magento/Config/Model/Config/Importer.php b/app/code/Magento/Config/Model/Config/Importer.php index 58b667b9ef465..58fc46d35747e 100644 --- a/app/code/Magento/Config/Model/Config/Importer.php +++ b/app/code/Magento/Config/Model/Config/Importer.php @@ -13,8 +13,7 @@ use Magento\Framework\App\State; use Magento\Framework\Config\ScopeInterface; use Magento\Framework\Exception\State\InvalidTransitionException; -use Magento\Framework\Flag\FlagResource; -use Magento\Framework\FlagFactory; +use Magento\Framework\FlagManager; use Magento\Framework\Stdlib\ArrayUtils; /** @@ -32,18 +31,11 @@ class Importer implements ImporterInterface const FLAG_CODE = 'system_config_snapshot'; /** - * The flag factory. + * The flag manager. * - * @var FlagFactory + * @var FlagManager */ - private $flagFactory; - - /** - * The flag resource. - * - * @var FlagResource - */ - private $flagResource; + private $flagManager; /** * An array utils. @@ -81,8 +73,7 @@ class Importer implements ImporterInterface private $saveProcessor; /** - * @param FlagFactory $flagFactory The flag factory - * @param FlagResource $flagResource The flag resource + * @param FlagManager $flagManager The flag manager * @param ArrayUtils $arrayUtils An array utils * @param SaveProcessor $saveProcessor Saves configuration data * @param ScopeConfigInterface $scopeConfig The application config storage. @@ -90,16 +81,14 @@ class Importer implements ImporterInterface * @param ScopeInterface $scope The application scope */ public function __construct( - FlagFactory $flagFactory, - FlagResource $flagResource, + FlagManager $flagManager, ArrayUtils $arrayUtils, SaveProcessor $saveProcessor, ScopeConfigInterface $scopeConfig, State $state, ScopeInterface $scope ) { - $this->flagFactory = $flagFactory; - $this->flagResource = $flagResource; + $this->flagManager = $flagManager; $this->arrayUtils = $arrayUtils; $this->saveProcessor = $saveProcessor; $this->scopeConfig = $scopeConfig; @@ -118,13 +107,10 @@ public function import(array $data) $currentScope = $this->scope->getCurrentScope(); try { - $flag = $this->flagFactory->create(['data' => ['flag_code' => static::FLAG_CODE]]); - $this->flagResource->load($flag, static::FLAG_CODE, 'flag_code'); - $previouslyProcessedData = $flag->getFlagData() ?: []; - + $savedFlag = $this->flagManager->getFlagData(static::FLAG_CODE) ?: []; $changedData = array_replace_recursive( - $this->arrayUtils->recursiveDiff($previouslyProcessedData, $data), - $this->arrayUtils->recursiveDiff($data, $previouslyProcessedData) + $this->arrayUtils->recursiveDiff($savedFlag, $data), + $this->arrayUtils->recursiveDiff($data, $savedFlag) ); /** @@ -142,8 +128,7 @@ public function import(array $data) $this->saveProcessor->process($changedData); }); - $flag->setFlagData($data); - $this->flagResource->save($flag); + $this->flagManager->saveFlag(static::FLAG_CODE, $data); } catch (\Exception $e) { throw new InvalidTransitionException(__('%1', $e->getMessage()), $e); } finally { diff --git a/app/code/Magento/Config/Model/PreparedValueFactory.php b/app/code/Magento/Config/Model/PreparedValueFactory.php index 842b1dff002bc..dc10bdd767bef 100644 --- a/app/code/Magento/Config/Model/PreparedValueFactory.php +++ b/app/code/Magento/Config/Model/PreparedValueFactory.php @@ -80,7 +80,6 @@ public function __construct( * @return ValueInterface * @throws RuntimeException If Value can not be created * @see ValueInterface - * @see Value */ public function create($path, $value, $scope, $scopeCode = null) { diff --git a/app/code/Magento/Config/Test/Unit/App/Config/Type/System/ReaderTest.php b/app/code/Magento/Config/Test/Unit/App/Config/Type/System/ReaderTest.php new file mode 100644 index 0000000000000..3f02e22465aa0 --- /dev/null +++ b/app/code/Magento/Config/Test/Unit/App/Config/Type/System/ReaderTest.php @@ -0,0 +1,93 @@ +source = $this->getMockBuilder(ConfigSourceInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->fallback = $this->getMockBuilder(Fallback::class) + ->disableOriginalConstructor() + ->getMock(); + $this->preProcessor = $this->getMockBuilder(PreProcessorInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->postProcessor = $this->getMockBuilder(PostProcessorInterface::class) + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + + $this->model = $helper->getObject( + Reader::class, + [ + 'source' => $this->source, + 'fallback' => $this->fallback, + 'preProcessor' => $this->preProcessor, + 'postProcessor' => $this->postProcessor + ] + ); + } + + public function testGetCachedWithLoadDefaultScopeData() + { + $data = [ + 'default' => [], + 'websites' => [], + 'stores' => [] + ]; + $this->source->expects($this->once()) + ->method('get') + ->willReturn($data); + $this->preProcessor->expects($this->once()) + ->method('process') + ->with($data) + ->willReturn($data); + $this->fallback->expects($this->once()) + ->method('process') + ->with($data) + ->willReturn($data); + $this->postProcessor->expects($this->once()) + ->method('process') + ->with($data) + ->willReturn($data); + $this->assertEquals($data, $this->model->read()); + } +} diff --git a/app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php b/app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php index 985ef34fe0238..cbc89df802890 100644 --- a/app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php +++ b/app/code/Magento/Config/Test/Unit/App/Config/Type/SystemTest.php @@ -13,6 +13,7 @@ use Magento\Framework\Cache\FrontendInterface; use Magento\Framework\Serialize\SerializerInterface; use Magento\Store\Model\Config\Processor\Fallback; +use Magento\Config\App\Config\Type\System\Reader; /** * Test how Class process source, cache them and retrieve value by path @@ -55,6 +56,11 @@ class SystemTest extends \PHPUnit_Framework_TestCase */ private $serializer; + /** + * @var Reader|\PHPUnit_Framework_MockObject_MockObject + */ + private $reader; + public function setUp() { $this->source = $this->getMockBuilder(ConfigSourceInterface::class) @@ -70,6 +76,9 @@ public function setUp() ->getMockForAbstractClass(); $this->serializer = $this->getMockBuilder(SerializerInterface::class) ->getMock(); + $this->reader = $this->getMockBuilder(Reader::class) + ->disableOriginalConstructor() + ->getMock(); $this->configType = new System( $this->source, @@ -77,23 +86,27 @@ public function setUp() $this->fallback, $this->cache, $this->serializer, - $this->preProcessor + $this->preProcessor, + 1, + 'system', + $this->reader ); } - public function testGetCached() + public function testGetCachedWithLoadDefaultScopeData() { $path = 'default/dev/unsecure/url'; $url = 'http://magento.test/'; $data = [ - 'unsecure' => [ - 'url' => $url + 'dev' => [ + 'unsecure' => [ + 'url' => $url + ] ] ]; $this->cache->expects($this->any()) ->method('load') - ->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev']) ->willReturnOnConsecutiveCalls('1', serialize($data)); $this->serializer->expects($this->once()) ->method('unserialize') @@ -101,62 +114,62 @@ public function testGetCached() $this->assertEquals($url, $this->configType->get($path)); } - public function testGetNotCached() + public function testGetCachedWithLoadAllData() { - $path = 'default/dev/unsecure/url'; $url = 'http://magento.test/'; $data = [ - 'default' => [ - 'dev' => [ - 'unsecure' => [ - 'url' => $url - ] + 'dev' => [ + 'unsecure' => [ + 'url' => $url ] ] ]; + $this->cache->expects($this->any()) + ->method('load') + ->willReturnOnConsecutiveCalls('1', serialize($data)); + $this->serializer->expects($this->once()) + ->method('unserialize') + ->willReturn($data); + $this->assertEquals($data, $this->configType->get('')); + } + + public function testGetNotCached() + { + $path = 'stores/default/dev/unsecure/url'; + $url = 'http://magento.test/'; + $dataToCache = [ 'unsecure' => [ 'url' => $url ] ]; + $data = [ + 'default' => [], + 'websites' => [], + 'stores' => [ + 'default' => [ + 'dev' => [ + 'unsecure' => [ + 'url' => $url + ] + ] + ] + ] + ]; $this->cache->expects($this->any()) ->method('load') - ->withConsecutive(['system_CACHE_EXISTS'], ['system_defaultdev']) ->willReturnOnConsecutiveCalls(false, false); - $this->serializer->expects($this->once()) + $this->serializer->expects($this->atLeastOnce()) ->method('serialize') ->willReturn(serialize($dataToCache)); - $this->source->expects($this->once()) - ->method('get') - ->willReturn($data); - $this->fallback->expects($this->once()) - ->method('process') - ->with($data) - ->willReturnArgument(0); - $this->preProcessor->expects($this->once()) - ->method('process') - ->with($data) - ->willReturnArgument(0); - $this->postProcessor->expects($this->once()) - ->method('process') - ->with($data) - ->willReturnArgument(0); - $this->cache->expects($this->any()) + $this->cache->expects($this->atLeastOnce()) ->method('save') - ->withConsecutive( - [ - serialize($dataToCache), - 'system_defaultdev', - [System::CACHE_TAG] - ], - [ - 'system_CACHE_EXISTS', - 'system_CACHE_EXISTS', - [System::CACHE_TAG] - ] - ); + ->willReturnSelf(); + $this->reader->expects($this->once()) + ->method('read') + ->willReturn($data); $this->assertEquals($url, $this->configType->get($path)); } diff --git a/app/code/Magento/Config/Test/Unit/Model/Config/ImporterTest.php b/app/code/Magento/Config/Test/Unit/Model/Config/ImporterTest.php index a8504e567142f..8e9ae506e7fb3 100644 --- a/app/code/Magento/Config/Test/Unit/Model/Config/ImporterTest.php +++ b/app/code/Magento/Config/Test/Unit/Model/Config/ImporterTest.php @@ -15,7 +15,7 @@ use Magento\Framework\Config\ScopeInterface; use Magento\Framework\Flag; use Magento\Framework\Flag\FlagResource; -use Magento\Framework\FlagFactory; +use Magento\Framework\FlagManager; use Magento\Framework\Stdlib\ArrayUtils; use PHPUnit_Framework_MockObject_MockObject as Mock; @@ -33,20 +33,15 @@ class ImporterTest extends \PHPUnit_Framework_TestCase private $model; /** - * @var FlagFactory|Mock + * @var FlagManager|Mock */ - private $flagFactoryMock; + private $flagManagerMock; /** * @var Flag|Mock */ private $flagMock; - /** - * @var FlagResource|Mock - */ - private $flagResourceMock; - /** * @var ArrayUtils|Mock */ @@ -87,7 +82,7 @@ class ImporterTest extends \PHPUnit_Framework_TestCase */ protected function setUp() { - $this->flagFactoryMock = $this->getMockBuilder(FlagFactory::class) + $this->flagManagerMock = $this->getMockBuilder(FlagManager::class) ->disableOriginalConstructor() ->getMock(); $this->flagMock = $this->getMockBuilder(Flag::class) @@ -116,13 +111,12 @@ protected function setUp() ->disableOriginalConstructor() ->getMock(); - $this->flagFactoryMock->expects($this->any()) + $this->flagManagerMock->expects($this->any()) ->method('create') ->willReturn($this->flagMock); $this->model = new Importer( - $this->flagFactoryMock, - $this->flagResourceMock, + $this->flagManagerMock, $this->arrayUtilsMock, $this->saveProcessorMock, $this->scopeConfigMock, @@ -131,16 +125,17 @@ protected function setUp() ); } + /** + * @SuppressWarnings(PHPMD.UnusedLocalVariable) + */ public function testImport() { $data = []; $currentData = ['current' => '2']; - $this->flagResourceMock->expects($this->once()) - ->method('load') - ->with($this->flagMock, Importer::FLAG_CODE, 'flag_code'); - $this->flagMock->expects($this->once()) + $this->flagManagerMock->expects($this->once()) ->method('getFlagData') + ->with(Importer::FLAG_CODE) ->willReturn($currentData); $this->arrayUtilsMock->expects($this->exactly(2)) ->method('recursiveDiff') @@ -153,16 +148,22 @@ public function testImport() ->willReturn('oldScope'); $this->stateMock->expects($this->once()) ->method('emulateAreaCode') - ->with(Area::AREA_ADMINHTML, $this->anything()); - $this->scopeMock->expects($this->once()) + ->with(Area::AREA_ADMINHTML, $this->anything()) + ->willReturnCallback(function ($area, $function) { + return $function(); + }); + $this->saveProcessorMock->expects($this->once()) + ->method('process') + ->with([]); + $this->scopeMock->expects($this->at(1)) + ->method('setCurrentScope') + ->with(Area::AREA_ADMINHTML); + $this->scopeMock->expects($this->at(2)) ->method('setCurrentScope') ->with('oldScope'); - $this->flagMock->expects($this->once()) - ->method('setFlagData') - ->with($data); - $this->flagResourceMock->expects($this->once()) - ->method('save') - ->with($this->flagMock); + $this->flagManagerMock->expects($this->once()) + ->method('saveFlag') + ->with(Importer::FLAG_CODE, $data); $this->assertSame(['System config was processed'], $this->model->import($data)); } @@ -176,10 +177,7 @@ public function testImportWithException() $data = []; $currentData = ['current' => '2']; - $this->flagResourceMock->expects($this->once()) - ->method('load') - ->with($this->flagMock, Importer::FLAG_CODE, 'flag_code'); - $this->flagMock->expects($this->once()) + $this->flagManagerMock->expects($this->once()) ->method('getFlagData') ->willReturn($currentData); $this->arrayUtilsMock->expects($this->exactly(2)) diff --git a/app/code/Magento/Config/etc/di.xml b/app/code/Magento/Config/etc/di.xml index 1abc5e35f578f..23d5d39a6e5bc 100644 --- a/app/code/Magento/Config/etc/di.xml +++ b/app/code/Magento/Config/etc/di.xml @@ -84,6 +84,14 @@ Magento\Framework\App\Cache\Type\Config systemConfigPreProcessorComposite Magento\Framework\Serialize\Serializer\Serialize + Magento\Config\App\Config\Type\System\Reader\Proxy + + + + + systemConfigSourceAggregated + systemConfigPostProcessorComposite + systemConfigPreProcessorComposite @@ -288,7 +296,8 @@ - Magento\Config\Model\Config\Importer + Magento\Config\Model\Config\Importer + 30 diff --git a/app/code/Magento/Contact/Model/Mail.php b/app/code/Magento/Contact/Model/Mail.php index 9a1f1ff8a2063..d63efbbca573b 100644 --- a/app/code/Magento/Contact/Model/Mail.php +++ b/app/code/Magento/Contact/Model/Mail.php @@ -6,7 +6,9 @@ namespace Magento\Contact\Model; use Magento\Framework\Mail\Template\TransportBuilder; +use Magento\Framework\App\ObjectManager; use Magento\Framework\Translate\Inline\StateInterface; +use Magento\Store\Model\StoreManagerInterface; class Mail implements MailInterface { @@ -26,18 +28,29 @@ class Mail implements MailInterface private $inlineTranslation; /** + * @var StoreManagerInterface + */ + private $storeManager; + + /** + * Initialize dependencies. + * * @param ConfigInterface $contactsConfig * @param TransportBuilder $transportBuilder * @param StateInterface $inlineTranslation + * @param StoreManagerInterface|null $storeManager */ public function __construct( ConfigInterface $contactsConfig, TransportBuilder $transportBuilder, - StateInterface $inlineTranslation + StateInterface $inlineTranslation, + StoreManagerInterface $storeManager = null ) { $this->contactsConfig = $contactsConfig; $this->transportBuilder = $transportBuilder; $this->inlineTranslation = $inlineTranslation; + $this->storeManager = $storeManager ?: + ObjectManager::getInstance()->get(StoreManagerInterface::class); } /** @@ -58,8 +71,8 @@ public function send($replyTo, array $variables) ->setTemplateIdentifier($this->contactsConfig->emailTemplate()) ->setTemplateOptions( [ - 'area' => 'adminhtml', - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, + 'area' => 'frontend', + 'store' => $this->storeManager->getStore()->getId() ] ) ->setTemplateVars($variables) diff --git a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php index e1e8fec435125..f432a4fc5ccee 100644 --- a/app/code/Magento/Contact/Test/Unit/Model/MailTest.php +++ b/app/code/Magento/Contact/Test/Unit/Model/MailTest.php @@ -8,6 +8,8 @@ use Magento\Contact\Model\ConfigInterface; use Magento\Contact\Model\Mail; +use Magento\Store\Model\StoreManagerInterface; +use Magento\Store\Api\Data\StoreInterface; class MailTest extends \PHPUnit_Framework_TestCase { @@ -32,6 +34,11 @@ class MailTest extends \PHPUnit_Framework_TestCase */ private $inlineTranslationMock; + /** + * @var \PHPUnit_Framework_MockObject_MockObject + */ + private $storeManagerMock; + /** * @var Mail */ @@ -50,10 +57,13 @@ protected function setUp() )->disableOriginalConstructor( )->getMock(); + $this->storeManagerMock = $this->getMock(StoreManagerInterface::class); + $this->mail = new Mail( $this->configMock, $this->transportBuilderMock, - $this->inlineTranslationMock + $this->inlineTranslationMock, + $this->storeManagerMock ); } @@ -64,6 +74,11 @@ public function testSendMail() $transport = $this->getMock(\Magento\Framework\Mail\TransportInterface::class, [], [], '', false); + $store = $this->getMock(StoreInterface::class); + $store->expects($this->once())->method('getId')->willReturn(555); + + $this->storeManagerMock->expects($this->once())->method('getStore')->willReturn($store); + $this->transportBuilderMock->expects($this->once()) ->method('setTemplateIdentifier') ->will($this->returnSelf()); @@ -71,8 +86,8 @@ public function testSendMail() $this->transportBuilderMock->expects($this->once()) ->method('setTemplateOptions') ->with([ - 'area' => \Magento\Backend\App\Area\FrontNameResolver::AREA_CODE, - 'store' => \Magento\Store\Model\Store::DEFAULT_STORE_ID, + 'area' => 'frontend', + 'store' => 555, ]) ->will($this->returnSelf()); diff --git a/app/code/Magento/Contact/etc/email_templates.xml b/app/code/Magento/Contact/etc/email_templates.xml index 8ae3b643f43c8..8f3f5ee442f7d 100644 --- a/app/code/Magento/Contact/etc/email_templates.xml +++ b/app/code/Magento/Contact/etc/email_templates.xml @@ -6,5 +6,5 @@ */ --> -