Skip to content

Commit

Permalink
Merge pull request #1214 from magento-falcons/MAGETWO-69983
Browse files Browse the repository at this point in the history
Fixed issues:
- MAGETWO-69383 Site is down when scopes are not shared anymore through app/etc/config.php
- MAGETWO-67159 [TD] Deployment commands improvements
- MAGETWO-69535 Error during importing new list of scopes through shared file
- MAGETWO-65658 [FT] User with restricted access can't log in to Admin in UpdateAdminUserEntityTest
- MAGETWO-55429 Exception Logging Does Not Follow PSR-3
- MAGETWO-67321 Importing Products with images from external URL
- MAGETWO-69983 Bugfixes delivery
  • Loading branch information
igrybkov authored Jun 22, 2017
2 parents ca01449 + 153c1d9 commit 82416d7
Show file tree
Hide file tree
Showing 40 changed files with 1,270 additions and 292 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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)
{
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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()
Expand All @@ -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);
Expand All @@ -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
*/
Expand All @@ -94,13 +147,39 @@ 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' => []]
],
'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' => []]
]
];
}
Expand Down
37 changes: 11 additions & 26 deletions app/code/Magento/Config/Model/Config/Importer.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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.
Expand Down Expand Up @@ -81,25 +73,22 @@ 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.
* @param State $state The application scope to run
* @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;
Expand All @@ -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)
);

/**
Expand All @@ -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 {
Expand Down
1 change: 0 additions & 1 deletion app/code/Magento/Config/Model/PreparedValueFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Loading

0 comments on commit 82416d7

Please sign in to comment.