MAGETWO-69430: Magento allows to save not valid data using config:set command

Author: shiftedreality

app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php

@@ -81,24 +81,13 @@ public function process($path, $value, $scope, $scopeCode)
             $backendModel = $this->preparedValueFactory->create($path, $value, $scope, $scopeCode);
 
             if ($backendModel instanceof Value) {
-                /**
-                 * Temporary solution until Magento introduce unified interface
-                 * for storing system configuration into database and configuration files.
-                 */
-                $backendModel->validateBeforeSave();
-                $backendModel->beforeSave();
+                $resourceModel = $backendModel->getResource();
+                $resourceModel->save($backendModel);
 
                 $value = $backendModel->getValue();
 
-                $backendModel->afterSave();
-
-                /**
-                 * Because FS does not support transactions,
-                 * we'll write value just after all validations are triggered.
-                 */
                 $this->deploymentConfigWriter->saveConfig(
-                    [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)],
-                    false
+                    [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $value)]
                 );
             }
         } catch (\Exception $exception) {

app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php

@@ -13,6 +13,7 @@
 use Magento\Framework\App\DeploymentConfig;
 use Magento\Framework\Config\File\ConfigFilePool;
 use Magento\Framework\Exception\FileSystemException;
+use Magento\Framework\Model\ResourceModel\AbstractResource;
 use Magento\Framework\Stdlib\ArrayManager;
 use Magento\Store\Model\ScopeInterface;
 use PHPUnit_Framework_MockObject_MockObject as Mock;
@@ -55,6 +56,11 @@ class LockProcessorTest extends \PHPUnit_Framework_TestCase
      */
     private $valueMock;
 
+    /**
+     * @var AbstractResource|Mock
+     */
+    private $resourceMock;
+
     /**
      * @inheritdoc
      */
@@ -73,9 +79,13 @@ protected function setUp()
             ->disableOriginalConstructor()
             ->getMock();
         $this->valueMock = $this->getMockBuilder(Value::class)
-            ->setMethods(['validateBeforeSave', 'beforeSave', 'setValue', 'getValue', 'afterSave'])
+            ->setMethods(['save', 'getResource', 'setValue', 'getValue', 'afterSave'])
             ->disableOriginalConstructor()
             ->getMock();
+        $this->resourceMock = $this->getMockBuilder(AbstractResource::class)
+            ->setMethods(['save'])
+            ->disableOriginalConstructor()
+            ->getMockForAbstractClass();
 
         $this->model = new LockProcessor(
             $this->preparedValueFactory,
@@ -120,6 +130,12 @@ public function testProcess($path, $value, $scope, $scopeCode)
         $this->valueMock->expects($this->once())
             ->method('getValue')
             ->willReturn($value);
+        $this->valueMock->expects($this->once())
+            ->method('getResource')
+            ->willReturn($this->resourceMock);
+        $this->resourceMock->expects($this->once())
+            ->method('save')
+            ->with($this->valueMock);
         $this->deploymentConfigWriterMock->expects($this->once())
             ->method('saveConfig')
             ->with(
@@ -138,12 +154,6 @@ public function testProcess($path, $value, $scope, $scopeCode)
                 ],
                 false
             );
-        $this->valueMock->expects($this->once())
-            ->method('validateBeforeSave');
-        $this->valueMock->expects($this->once())
-            ->method('beforeSave');
-        $this->valueMock->expects($this->once())
-            ->method('afterSave');
 
         $this->model->process($path, $value, $scope, $scopeCode);
     }
@@ -175,6 +185,12 @@ public function testProcessNotReadableFs()
         $this->valueMock->expects($this->once())
             ->method('getValue')
             ->willReturn($value);
+        $this->valueMock->expects($this->once())
+            ->method('getResource')
+            ->willReturn($this->resourceMock);
+        $this->resourceMock->expects($this->once())
+            ->method('save')
+            ->with($this->valueMock);
         $this->configPathResolver->expects($this->once())
             ->method('resolve')
             ->willReturn('system/default/test/test/test');
@@ -207,9 +223,10 @@ public function testCustomException()
         $this->arrayManagerMock->expects($this->never())
             ->method('set');
         $this->valueMock->expects($this->once())
-            ->method('getValue');
-        $this->valueMock->expects($this->once())
-            ->method('afterSave')
+            ->method('getResource')
+            ->willReturn($this->resourceMock);
+        $this->resourceMock->expects($this->once())
+            ->method('save')
             ->willThrowException(new \Exception('Invalid values'));
         $this->deploymentConfigWriterMock->expects($this->never())
             ->method('saveConfig');