ENGCOM-8337: Fix array to string conversion error when saving row system config with defaults #30322

Author: sidolov

app/code/Magento/Config/Model/Config/Backend/Serialized.php

@@ -5,6 +5,7 @@
  */
 namespace Magento\Config\Model\Config\Backend;
 
+use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\App\ObjectManager;
 use Magento\Framework\Serialize\Serializer\Json;
 
@@ -84,4 +85,26 @@ public function beforeSave()
         parent::beforeSave();
         return $this;
     }
+
+    /**
+     * Get old value from existing config
+     *
+     * @return string
+     */
+    public function getOldValue()
+    {
+        // If the value is retrieved from defaults defined in config.xml
+        // it may be returned as an array.
+        $value = $this->_config->getValue(
+            $this->getPath(),
+            $this->getScope() ?: ScopeConfigInterface::SCOPE_TYPE_DEFAULT,
+            $this->getScopeCode()
+        );
+
+        if (is_array($value)) {
+            return $this->serializer->serialize($value);
+        }
+
+        return (string)$value;
+    }
 }

app/code/Magento/Config/Test/Unit/Model/Config/Backend/SerializedTest.php

@@ -8,6 +8,7 @@
 namespace Magento\Config\Test\Unit\Model\Config\Backend;
 
 use Magento\Config\Model\Config\Backend\Serialized;
+use Magento\Framework\App\Config\ScopeConfigInterface;
 use Magento\Framework\Event\ManagerInterface;
 use Magento\Framework\Model\Context;
 use Magento\Framework\Serialize\Serializer\Json;
@@ -27,11 +28,14 @@ class SerializedTest extends TestCase
     /** @var LoggerInterface|MockObject */
     private $loggerMock;
 
+    private $scopeConfigMock;
+
     protected function setUp(): void
     {
         $objectManager = new ObjectManager($this);
         $this->serializerMock = $this->createMock(Json::class);
         $this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class);
+        $this->scopeConfigMock = $this->createMock(ScopeConfigInterface::class);
         $contextMock = $this->createMock(Context::class);
         $eventManagerMock = $this->getMockForAbstractClass(ManagerInterface::class);
         $contextMock->method('getEventDispatcher')
@@ -43,6 +47,7 @@ protected function setUp(): void
             [
                 'serializer' => $this->serializerMock,
                 'context' => $contextMock,
+                'config' => $this->scopeConfigMock,
             ]
         );
     }
@@ -135,4 +140,29 @@ public function beforeSaveDataProvider()
             ]
         ];
     }
+
+    /**
+     * If a config value is not available in core_confid_data the defaults are
+     * loaded from the config.xml file. Those defaults may be arrays.
+     * The Serialized backend model has to override its parent
+     * getOldValue function, to prevent an array to string conversion error
+     * and serialize those values.
+     */
+    public function testGetOldValueWithNonScalarDefaultValue(): void
+    {
+        $value = [
+            ['foo' => '1', 'bar' => '2'],
+        ];
+        $serializedValue = \json_encode($value);
+
+        $this->scopeConfigMock->method('getValue')->willReturn($value);
+        $this->serializerMock->method('serialize')->willReturn($serializedValue);
+
+        $this->serializedConfig->setData('value', $serializedValue);
+
+        $oldValue = $this->serializedConfig->getOldValue();
+
+        $this->assertIsString($oldValue, 'Default value from the config is not serialized.');
+        $this->assertSame($serializedValue, $oldValue);
+    }
 }