Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issues/33718 #35770

Open
wants to merge 3 commits into
base: 2.4-develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions app/code/Magento/Config/Model/Config/Importer.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,9 @@ public function import(array $data)

try {
$savedFlag = $this->flagManager->getFlagData(static::FLAG_CODE) ?: [];
$cachedData = $this->arrayUtils->recursiveDiff($savedFlag, $data);
$changedData = array_replace_recursive(
$this->arrayUtils->recursiveDiff($savedFlag, $data),
$cachedData,
$this->arrayUtils->recursiveDiff($data, $savedFlag)
);

Expand All @@ -124,11 +125,11 @@ public function import(array $data)
$this->scopeConfig->clean();
}

$this->state->emulateAreaCode(Area::AREA_ADMINHTML, function () use ($changedData) {
$this->state->emulateAreaCode(Area::AREA_ADMINHTML, function () use ($changedData, $cachedData) {
$this->scope->setCurrentScope(Area::AREA_ADMINHTML);

// Invoke saving of new values.
$this->saveProcessor->process($changedData);
$this->saveProcessor->process($changedData, $cachedData);
});

$this->scope->setCurrentScope($currentScope);
Expand Down
13 changes: 7 additions & 6 deletions app/code/Magento/Config/Model/Config/Importer/SaveProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,16 +55,17 @@ public function __construct(
* Emulates saving of data array.
*
* @param array $data The data to be saved
* @param array $cachedData The data before changes
* @return void
*/
public function process(array $data)
public function process(array $data, array $cachedData = [])
{
foreach ($data as $scope => $scopeData) {
if ($scope === ScopeConfigInterface::SCOPE_TYPE_DEFAULT) {
$this->invokeSave($scopeData, $scope);
$this->invokeSave($scopeData, $scope, null, $cachedData);
} else {
foreach ($scopeData as $scopeCode => $scopeCodeData) {
$this->invokeSave($scopeCodeData, $scope, $scopeCode);
$this->invokeSave($scopeCodeData, $scope, $scopeCode, $cachedData);
}
}
}
Expand All @@ -90,19 +91,19 @@ public function process(array $data)
* @param array $scopeData The data for specific scope
* @param string $scope The configuration scope (default, website, or store)
* @param string $scopeCode The scope code
* @param array $cachedData The data before changes
* @return void
* @throws \Magento\Framework\Exception\RuntimeException
*/
private function invokeSave(array $scopeData, $scope, $scopeCode = null)
private function invokeSave(array $scopeData, $scope, $scopeCode = null, array $cachedData = [])
{
$scopeData = array_keys($this->arrayUtils->flatten($scopeData));

foreach ($scopeData as $path) {
$value = $this->scopeConfig->getValue($path, $scope, $scopeCode);
if ($value !== null) {
$backendModel = $this->valueFactory->create($path, $value, $scope, $scopeCode);

if ($backendModel instanceof Value) {
$backendModel->setConfigFromCache($path, $cachedData);
$backendModel->beforeSave();
$backendModel->afterSave();
}
Expand Down
34 changes: 29 additions & 5 deletions lib/internal/Magento/Framework/App/Config/Value.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ class Value extends \Magento\Framework\Model\AbstractModel implements \Magento\F
*/
protected $cacheTypeList;

/**
* @var array
*/
protected $scopeConfigFromCache;

/**
* @param \Magento\Framework\Model\Context $context
* @param \Magento\Framework\Registry $registry
Expand Down Expand Up @@ -94,11 +99,18 @@ public function isValueChanged()
*/
public function getOldValue()
{
return (string)$this->_config->getValue(
$this->getPath(),
$this->getScope() ?: ScopeConfigInterface::SCOPE_TYPE_DEFAULT,
$this->getScopeCode()
);
$value = $this->scopeConfigFromCache[$this->getPath()];
if (!empty($value)) {
while (is_array($value)) {
$value = end($value);
}
} else {
$value = $this->_config->getValue(
$this->getPath(),
$this->getScope() ?: ScopeConfigInterface::SCOPE_TYPE_DEFAULT,
$this->getScopeCode());
}
return (string) $value;
}

/**
Expand All @@ -113,6 +125,18 @@ public function getFieldsetDataValue($key)
return is_array($data) && isset($data[$key]) ? $data[$key] : null;
}

/**
* Set scope configuration that was before changes
*
* @param string $path
* @param array $config
* @return void
*/
public function setConfigFromCache(string $path, array $config = [])
{
isset($this->scopeConfigFromCache[$path]) ?: $this->scopeConfigFromCache[$path] = $config;
}

/**
* Processing object after save data
*
Expand Down
48 changes: 29 additions & 19 deletions lib/internal/Magento/Framework/App/Test/Unit/Config/ValueTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,36 +59,44 @@ protected function setUp(): void
);
}

public function testSetConfigFromCache()
{
$path = 'some/test/path';
$config = [
'some' => [
'test' => [
'config' => 'value'
]
]
];
$this->assertEquals(false, $this->model->setConfigFromCache($path, $config));
}

/**
* @return void
*/
public function testGetOldValue(): void
{
$this->configMock->expects($this->once())
->method('getValue')
->with(null, 'default')
->willReturn('old_value');

$path = 'some/test/path';
$this->model->setPath($path);
$this->model->setConfigFromCache($path, ['key' => 'old_value']);
$this->assertEquals('old_value', $this->model->getOldValue());
}

/**
* @param string $oldValue
* @param array $oldValue
* @param string $value
* @param string $path
* @param bool $result
*
* @return void
* @dataProvider dataIsValueChanged
*/
public function testIsValueChanged($oldValue, $value, $result): void
public function testIsValueChanged($oldValue, $value, $path, $result): void
{
$this->configMock->expects($this->once())
->method('getValue')
->with(null, 'default')
->willReturn($oldValue);

$this->model->setValue($value);

$this->model->setPath($path);
$this->model->setConfigFromCache($path, $oldValue);
$this->assertEquals($result, $this->model->isValueChanged());
}

Expand All @@ -98,8 +106,8 @@ public function testIsValueChanged($oldValue, $value, $result): void
public function dataIsValueChanged(): array
{
return [
['value', 'value', false],
['value', 'new_value', true]
[['key' => 'old_value'], 'old_value', 'some/test/path', false],
[['key' => 'old_value'], 'new_value', 'some/test/path', true]
];
}

Expand Down Expand Up @@ -168,18 +176,20 @@ public function dataProviderGetFieldsetDataValue(): array
/**
* @param int $callNumber
* @param string $oldValue
*
* @param string $path
* @return void
* @dataProvider afterSaveDataProvider
*/
public function testAfterSave($callNumber, $oldValue): void
public function testAfterSave($callNumber, $oldValue, $path): void
{
$this->cacheTypeListMock->expects($this->exactly($callNumber))
->method('invalidate');
$this->configMock->expects($this->any())
->method('getValue')
->willReturn($oldValue);
$this->model->setValue('some_value');
$this->model->setPath($path);
$this->model->setConfigFromCache($path, []);
$this->assertInstanceOf(get_class($this->model), $this->model->afterSave());
}

Expand All @@ -189,8 +199,8 @@ public function testAfterSave($callNumber, $oldValue): void
public function afterSaveDataProvider(): array
{
return [
[0, 'some_value'],
[1, 'other_value']
[0, 'some_value', 'some/test/path'],
[1, 'other_value', 'some/test/path']
];
}

Expand Down