Skip to content

Commit

Permalink
Merge pull request #484 from magento-falcons/MAGETWO-58976
Browse files Browse the repository at this point in the history 
Bugs:
MAGETWO-58976: [Magento Cloud] - Import issue with a multiselect option having special symbols (, and |)
  • Loading branch information
@magicbunneh
magicbunneh authored Oct 10, 2016
2 parents dbbf925 + b4f69c3 commit 492f0ca
Show file tree
Hide file tree
Showing 16 changed files with 348 additions and 37 deletions.
23 changes: 21 additions & 2 deletions app/code/Magento/CatalogImportExport/Model/Export/Product.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -953,7 +953,7 @@ protected function collectRawData()
if (is_scalar($attrValue)) {
if (!in_array($fieldName, $this->_getExportMainAttrCodes())) {
$additionalAttributes[$fieldName] = $fieldName .
ImportProduct::PAIR_NAME_VALUE_SEPARATOR . $attrValue;
ImportProduct::PAIR_NAME_VALUE_SEPARATOR . $this->wrapValue($attrValue);
}
$data[$itemId][$storeId][$fieldName] = htmlspecialchars_decode($attrValue);
}
Expand All @@ -963,7 +963,7 @@ protected function collectRawData()
$additionalAttributes[$code] = $fieldName .
ImportProduct::PAIR_NAME_VALUE_SEPARATOR . implode(
ImportProduct::PSEUDO_MULTI_LINE_SEPARATOR,
$this->collectedMultiselectsData[$storeId][$productLinkId][$code]
$this->wrapValue($this->collectedMultiselectsData[$storeId][$productLinkId][$code])
);
}
}
Expand Down Expand Up @@ -994,6 +994,25 @@ protected function collectRawData()
return $data;
}

/**
* Wrap values with double quotes if "Fields Enclosure" option is enabled
*
* @param string|array $value
* @return string|array
*/
private function wrapValue($value)
{
if (!empty($this->_parameters[\Magento\ImportExport\Model\Export::FIELDS_ENCLOSURE])) {
$wrap = function ($value) {
return sprintf('"%s"', str_replace('"', '""', $value));
};

$value = is_array($value) ? array_map($wrap, $value) : $wrap($value);
}

return $value;
}

/**
* @return array
*/
Expand Down
108 changes: 105 additions & 3 deletions app/code/Magento/CatalogImportExport/Model/Import/Product.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,13 @@ class Product extends \Magento\ImportExport\Model\Import\Entity\AbstractEntity
*/
private $productEntityIdentifierField;

/**
* Escaped separator value for regular expression.
* The value is based on PSEUDO_MULTI_LINE_SEPARATOR constant.
* @var string
*/
private $multiLineSeparatorForRegexp;

/**
* @param \Magento\Framework\Json\Helper\Data $jsonHelper
* @param \Magento\ImportExport\Helper\Data $importExportData
Expand Down Expand Up @@ -2438,14 +2445,40 @@ private function _parseAdditionalAttributes($rowData)
}

/**
* Retrieves additional attributes as array code=>value.
* Retrieves additional attributes in format:
* [
* code1 => value1,
* code2 => value2,
* ...
* codeN => valueN
* ]
*
* @param string $additionalAttributes
* @param string $additionalAttributes Attributes data that will be parsed
* @return array
*/
private function parseAdditionalAttributes($additionalAttributes)
{
$attributeNameValuePairs = explode($this->getMultipleValueSeparator(), $additionalAttributes);
return empty($this->_parameters[Import::FIELDS_ENCLOSURE])
? $this->parseAttributesWithoutWrappedValues($additionalAttributes)
: $this->parseAttributesWithWrappedValues($additionalAttributes);
}

/**
* Parses data and returns attributes in format:
* [
* code1 => value1,
* code2 => value2,
* ...
* codeN => valueN
* ]
*
* @param string $attributesData Attributes data that will be parsed. It keeps data in format:
* code=value,code2=value2...,codeN=valueN
* @return array
*/
private function parseAttributesWithoutWrappedValues($attributesData)
{
$attributeNameValuePairs = explode($this->getMultipleValueSeparator(), $attributesData);
$preparedAttributes = [];
$code = '';
foreach ($attributeNameValuePairs as $attributeData) {
Expand All @@ -2463,6 +2496,75 @@ private function parseAdditionalAttributes($additionalAttributes)
return $preparedAttributes;
}

/**
* Parses data and returns attributes in format:
* [
* code1 => value1,
* code2 => value2,
* ...
* codeN => valueN
* ]
* All values have unescaped data except mupliselect attributes,
* they should be parsed in additional method - parseMultiselectValues()
*
* @param string $attributesData Attributes data that will be parsed. It keeps data in format:
* code="value",code2="value2"...,codeN="valueN"
* where every value is wrapped in double quotes. Double quotes as part of value should be duplicated.
* E.g. attribute with code 'attr_code' has value 'my"value'. This data should be stored as attr_code="my""value"
*
* @return array
*/
private function parseAttributesWithWrappedValues($attributesData)
{
$attributes = [];
preg_match_all('~((?:[a-z0-9_])+)="((?:[^"]|""|"' . $this->getMultiLineSeparatorForRegexp() . '")+)"+~',
$attributesData,
$matches
);
foreach ($matches[1] as $i => $attributeCode) {
$attribute = $this->retrieveAttributeByCode($attributeCode);
$value = 'multiselect' != $attribute->getFrontendInput()
? str_replace('""', '"', $matches[2][$i])
: '"' . $matches[2][$i] . '"';
$attributes[$attributeCode] = $value;
}
return $attributes;
}

/**
* Parse values of multiselect attributes depends on "Fields Enclosure" parameter
*
* @param string $values
* @return array
*/
public function parseMultiselectValues($values)
{
if (empty($this->_parameters[Import::FIELDS_ENCLOSURE])) {
return explode(self::PSEUDO_MULTI_LINE_SEPARATOR, $values);
}
if (preg_match_all('~"((?:[^"]|"")*)"~', $values, $matches)) {
return $values = array_map(function ($value) {
return str_replace('""', '"', $value);
}, $matches[1]);
}
return [$values];
}

/**
* Retrieves escaped PSEUDO_MULTI_LINE_SEPARATOR if it is metacharacter for regular expression
*
* @return string
*/
private function getMultiLineSeparatorForRegexp()
{
if (!$this->multiLineSeparatorForRegexp) {
$this->multiLineSeparatorForRegexp = in_array(self::PSEUDO_MULTI_LINE_SEPARATOR, str_split('[\^$.|?*+(){}'))
? '\\' . self::PSEUDO_MULTI_LINE_SEPARATOR
: self::PSEUDO_MULTI_LINE_SEPARATOR;
}
return $this->multiLineSeparatorForRegexp;
}

/**
* Set values in use_config_ fields.
*
Expand Down
30 changes: 16 additions & 14 deletions app/code/Magento/CatalogImportExport/Model/Import/Product/Type/AbstractType.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -490,23 +490,25 @@ public function prepareAttributesWithDefaultValueForSave(array $rowData, $withDe
$resultAttrs = [];

foreach ($this->_getProductAttributes($rowData) as $attrCode => $attrParams) {
if (!$attrParams['is_static']) {
if (isset($rowData[$attrCode]) && strlen($rowData[$attrCode])) {
$resultAttrs[$attrCode] = in_array($attrParams['type'], ['select', 'boolean'])
? $attrParams['options'][strtolower($rowData[$attrCode])]
: $rowData[$attrCode];
if ('multiselect' == $attrParams['type']) {
$resultAttrs[$attrCode] = [];
foreach (explode(Product::PSEUDO_MULTI_LINE_SEPARATOR, $rowData[$attrCode]) as $value) {
$resultAttrs[$attrCode][] = $attrParams['options'][strtolower($value)];
}
$resultAttrs[$attrCode] = implode(',', $resultAttrs[$attrCode]);
if ($attrParams['is_static']) {
continue;
}
if (isset($rowData[$attrCode]) && strlen($rowData[$attrCode])) {
if (in_array($attrParams['type'], ['select', 'boolean'])) {
$resultAttrs[$attrCode] = $attrParams['options'][strtolower($rowData[$attrCode])];
} elseif ('multiselect' == $attrParams['type']) {
$resultAttrs[$attrCode] = [];
foreach ($this->_entityModel->parseMultiselectValues($rowData[$attrCode]) as $value) {
$resultAttrs[$attrCode][] = $attrParams['options'][strtolower($value)];
}
} elseif (array_key_exists($attrCode, $rowData)) {
$resultAttrs[$attrCode] = implode(',', $resultAttrs[$attrCode]);
} else {
$resultAttrs[$attrCode] = $rowData[$attrCode];
} elseif ($withDefaultValue && null !== $attrParams['default_value']) {
$resultAttrs[$attrCode] = $attrParams['default_value'];
}
} elseif (array_key_exists($attrCode, $rowData)) {
$resultAttrs[$attrCode] = $rowData[$attrCode];
} elseif ($withDefaultValue && null !== $attrParams['default_value']) {
$resultAttrs[$attrCode] = $attrParams['default_value'];
}
}

Expand Down
48 changes: 33 additions & 15 deletions app/code/Magento/CatalogImportExport/Model/Import/Product/Validator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,32 @@ protected function textValidation($attrCode, $type)
return $valid;
}

/**
* Check if value is valid attribute option
*
* @param string $attrCode
* @param array $possibleOptions
* @param string $value
* @return bool
*/
private function validateOption($attrCode, $possibleOptions, $value)
{
if (!isset($possibleOptions[strtolower($value)])) {
$this->_addMessages(
[
sprintf(
$this->context->retrieveMessageTemplate(
RowValidatorInterface::ERROR_INVALID_ATTRIBUTE_OPTION
),
$attrCode
)
]
);
return false;
}
return true;
}

/**
* @param mixed $attrCode
* @param string $type
Expand Down Expand Up @@ -166,23 +192,15 @@ public function isAttributeValid($attrCode, array $attrParams, array $rowData)
break;
case 'select':
case 'boolean':
$valid = $this->validateOption($attrCode, $attrParams['options'], $rowData[$attrCode]);
break;
case 'multiselect':
$values = explode(Product::PSEUDO_MULTI_LINE_SEPARATOR, $rowData[$attrCode]);
$valid = true;
$values = $this->context->parseMultiselectValues($rowData[$attrCode]);
foreach ($values as $value) {
$valid = $valid && isset($attrParams['options'][strtolower($value)]);
}
if (!$valid) {
$this->_addMessages(
[
sprintf(
$this->context->retrieveMessageTemplate(
RowValidatorInterface::ERROR_INVALID_ATTRIBUTE_OPTION
),
$attrCode
)
]
);
$valid = $this->validateOption($attrCode, $attrParams['options'], $value);
if (!$valid) {
break;
}
}
break;
case 'datetime':
Expand Down
10 changes: 10 additions & 0 deletions app/code/Magento/ImportExport/Block/Adminhtml/Export/Edit/Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ protected function _prepareForm()
'values' => $this->_formatFactory->create()->toOptionArray()
]
);
$fieldset->addField(
\Magento\ImportExport\Model\Export::FIELDS_ENCLOSURE,
'checkbox',
[
'name' => \Magento\ImportExport\Model\Export::FIELDS_ENCLOSURE,
'label' => __('Fields Enclosure'),
'title' => __('Fields Enclosure'),
'value' => 1,
]
);

$form->setUseContainer(true);
$this->setForm($form);
Expand Down
10 changes: 10 additions & 0 deletions app/code/Magento/ImportExport/Block/Adminhtml/Import/Edit/Form.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,16 @@ protected function _prepareForm()
'value' => Import::DEFAULT_GLOBAL_MULTI_VALUE_SEPARATOR,
]
);
$fieldsets[$behaviorCode]->addField(
$behaviorCode . \Magento\ImportExport\Model\Import::FIELDS_ENCLOSURE,
'checkbox',
[
'name' => \Magento\ImportExport\Model\Import::FIELDS_ENCLOSURE,
'label' => __('Fields enclosure'),
'title' => __('Fields enclosure'),
'value' => 1,
]
);
}

// fieldset for file uploading
Expand Down
5 changes: 5 additions & 0 deletions app/code/Magento/ImportExport/Model/Export.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ class Export extends \Magento\ImportExport\Model\AbstractModel

const FILTER_ELEMENT_SKIP = 'skip_attr';

/**
* Allow multiple values wrapping in double quotes for additional attributes.
*/
const FIELDS_ENCLOSURE = 'fields_enclosure';

/**
* Filter fields types.
*/
Expand Down
5 changes: 5 additions & 0 deletions app/code/Magento/ImportExport/Model/Import.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,11 @@ class Import extends \Magento\ImportExport\Model\AbstractModel
*/
const FIELD_FIELD_MULTIPLE_VALUE_SEPARATOR = '_import_multiple_value_separator';

/**
* Allow multiple values wrapping in double quotes for additional attributes.
*/
const FIELDS_ENCLOSURE = 'fields_enclosure';

/**#@-*/

/**
Expand Down
3 changes: 3 additions & 0 deletions app/code/Magento/ImportExport/view/adminhtml/templates/export/form/before.phtml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ require([
var oldAction = form.action;
var url = oldAction + ((oldAction.slice(-1) != '/') ? '/' : '') + 'entity/' + $F('entity')
+ '/file_format/' + $F('file_format');
if ($F('fields_enclosure')) {
url += '/fields_enclosure/' + $F('fields_enclosure');
}
form.action = url;
form.submit();
form.action = oldAction;
Expand Down
2 changes: 1 addition & 1 deletion dev/tests/integration/testsuite/Magento/Catalog/_files/multiselect_attribute.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
'option_1' => ['Option 1'],
'option_2' => ['Option 2'],
'option_3' => ['Option 3'],
'option_4' => ['Option 4 "!@#$%^&*'],
'option_4' => ['Option 4 "!@#$%^&*']
],
'order' => [
'option_1' => 1,
Expand Down
Loading

0 comments on commit 492f0ca

Please sign in to comment.