MAGETWO-65208: Store checksum for every section of configuration file & change behavior on read-only FS & add sorting of importers

Author: Bohdan Korablov

app/code/Magento/Deploy/Console/Command/App/ConfigImport/Importer.php

@@ -7,7 +7,7 @@
 
 use Magento\Framework\App\DeploymentConfig\ImporterInterface;
 use Magento\Framework\App\DeploymentConfig;
-use Magento\Deploy\Model\DeploymentConfig\ImportFailedException;
+use Magento\Framework\Exception\RuntimeException;
 use Psr\Log\LoggerInterface as Logger;
 use Magento\Deploy\Model\DeploymentConfig\Validator;
 use Magento\Deploy\Model\DeploymentConfig\ImporterPool;
@@ -91,7 +91,7 @@ public function __construct(
      *
      * @param OutputInterface $output the CLI output
      * @return void
-     * @throws ImportFailedException is thrown when import has failed
+     * @throws RuntimeException is thrown when import has failed
      */
     public function import(OutputInterface $output)
     {
@@ -106,20 +106,20 @@ public function import(OutputInterface $output)
 
             /**
              * @var string $section
-             * @var string $importer
+             * @var string $importerClassName
              */
             foreach ($importers as $section => $importerClassName) {
                 if (!$this->configValidator->isValid($section)) {
                     /** @var ImporterInterface $importer */
                     $importer = $this->importerFactory->create($importerClassName);
-                    $messages = $importer->import($this->deploymentConfig->getConfigData($section));
+                    $messages = $importer->import((array) $this->deploymentConfig->getConfigData($section));
                     $output->writeln($messages);
                     $this->configHash->regenerate($section);
                 }
             }
         } catch (\Exception $exception) {
             $this->logger->error($exception);
-            throw new ImportFailedException(__('Import is failed. Please, see the log report.'), $exception);
+            throw new RuntimeException(__('Import is failed. Please see the log report.'), $exception);
         }
     }
 }

app/code/Magento/Deploy/Console/Command/App/ConfigImportCommand.php

@@ -5,13 +5,12 @@
  */
 namespace Magento\Deploy\Console\Command\App;
 
-use Magento\Deploy\Model\DeploymentConfig\ImportFailedException;
+use Magento\Framework\Exception\RuntimeException;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Magento\Framework\Console\Cli;
 use Magento\Deploy\Console\Command\App\ConfigImport\Importer;
-use Magento\Framework\App\DeploymentConfig\Writer;
 
 /**
  * Runs the process of importing configuration data from shared source to appropriate application sources.
@@ -34,21 +33,12 @@ class ConfigImportCommand extends Command
      */
     private $importer;
 
-    /**
-     * Configuration file writer.
-     *
-     * @var Writer
-     */
-    private $writer;
-
     /**
      * @param Importer $importer the configuration importer
-     * @param Writer $writer the configuration file writer
      */
-    public function __construct(Importer $importer, Writer $writer)
+    public function __construct(Importer $importer)
     {
         $this->importer = $importer;
-        $this->writer = $writer;
 
         parent::__construct();
     }
@@ -70,14 +60,9 @@ protected function configure()
      */
     protected function execute(InputInterface $input, OutputInterface $output)
     {
-        if (!$this->writer->checkIfWritable()) {
-            $output->writeln('<error>Deployment configuration file is not writable.</error>');
-            return Cli::RETURN_FAILURE;
-        }
-
         try {
             $this->importer->import($output);
-        } catch (ImportFailedException $e) {
+        } catch (RuntimeException $e) {
             $output->writeln('<error>' . $e->getMessage() . '</error>');
 
             return Cli::RETURN_FAILURE;

app/code/Magento/Deploy/Model/DeploymentConfig/DataCollector.php

@@ -15,7 +15,7 @@
  * <type name="Magento\Deploy\Model\DeploymentConfig\ImporterPool">
  *     <arguments>
  *          <argument name="importers" xsi:type="array">
- *               <item name="scopes" xsi:type="string">Magento\Store\Model\StoreImporter</item>
+ *               <item name="scopes" xsi:type="string">Magento\SomeModule\Model\SomeImporter</item>
  *          </argument>
  *     </arguments>
  * </type>
@@ -30,7 +30,7 @@
  *  ]
  * ```
  *
- * In here we define section "scopes" and its importer Magento\Store\Model\StoreImporter.
+ * In here we define section "scopes" and its importer Magento\SomeModule\Model\SomeImporter.
  * The data of this section will be collected then will be used in importing process from the shared configuration
  * files to appropriate application sources.
  *
@@ -75,6 +75,13 @@ public function __construct(ImporterPool $configImporterPool, DeploymentConfig $
      *      ...
      *  ]
      * ```
+     *
+     * This method retrieves the same structure for the specific section with only its data.
+     * ```php
+     *  [
+     *      'scopes' => [...]
+     *  ]
+     *
      * In this example key of the array is the section name, value of the array is configuration data of the section.
      *
      * @param string $sectionName the section name for retrieving its configuration data
@@ -91,10 +98,7 @@ public function getConfig($sectionName = null)
         }
 
         foreach ($sections as $section) {
-            $data = $this->deploymentConfig->getConfigData($section);
-            if ($data) {
-                $result[$section] = $data;
-            }
+            $result[$section] = $this->deploymentConfig->getConfigData($section);
         }
 
         return $result;

app/code/Magento/Deploy/Model/DeploymentConfig/ImportFailedException.php

@@ -130,9 +130,8 @@ public function getImporters()
     {
         if (!$this->sortedImporters) {
             $sortedImporters = [];
-            $importers = $this->sort($this->importers);
 
-            foreach ($importers as $section => $importer) {
+            foreach ($this->sort($this->importers) as $section => $importer) {
                 if (empty($importer['class'])) {
                     throw new ConfigurationMismatchException(__('Parameter "class" must be present.'));
                 }

app/code/Magento/Deploy/Model/DeploymentConfig/ImporterPool.php

@@ -59,7 +59,7 @@ public function __construct(
      * If config data is empty always returns true.
      * In the other cases returns true.
      *
-     * @param string $sectionName is name section for check data of the specific section
+     * @param string $sectionName is section name for check data of the specific section
      * @return bool
      */
     public function isValid($sectionName = null)
@@ -68,8 +68,9 @@ public function isValid($sectionName = null)
         $hashes = $this->configHash->get();
 
         foreach ($configs as $section => $config) {
-            $hash = isset($hashes[$section]) ? $hashes[$section] : null;
-            if ($config && $this->hashGenerator->generate($config) !== $hash) {
+            $savedHash = isset($hashes[$section]) ? $hashes[$section] : null;
+            $generatedHash = empty($config) && !$savedHash ? null : $this->hashGenerator->generate($config);
+            if ($generatedHash !== $savedHash) {
                 return false;
             }
         }

app/code/Magento/Deploy/Model/DeploymentConfig/Validator.php

@@ -143,7 +143,7 @@ public function testImport()
     /**
      * @return void
      * @expectedException \Magento\Framework\Exception\LocalizedException
-     * @expectedExceptionMessage Import is failed
+     * @expectedExceptionMessage Import is failed. Please see the log report.
      */
     public function testImportWithException()
     {

app/code/Magento/Deploy/Test/Unit/Console/Command/App/ConfigImport/ImporterTest.php

@@ -8,9 +8,8 @@
 use Magento\Deploy\Console\Command\App\ConfigImportCommand;
 use Magento\Deploy\Console\Command\App\ConfigImport\Importer;
 use Magento\Framework\Console\Cli;
-use Magento\Deploy\Model\DeploymentConfig\ImportFailedException;
+use Magento\Framework\Exception\RuntimeException;
 use Symfony\Component\Console\Tester\CommandTester;
-use Magento\Framework\App\DeploymentConfig\Writer;
 
 class ConfigImportCommandTest extends \PHPUnit_Framework_TestCase
 {
@@ -19,11 +18,6 @@ class ConfigImportCommandTest extends \PHPUnit_Framework_TestCase
      */
     private $importerMock;
 
-    /**
-     * @var Writer|\PHPUnit_Framework_MockObject_MockObject
-     */
-    private $writerMock;
-
     /**
      * @var CommandTester
      */
@@ -37,14 +31,8 @@ protected function setUp()
         $this->importerMock = $this->getMockBuilder(Importer::class)
             ->disableOriginalConstructor()
             ->getMock();
-        $this->writerMock = $this->getMockBuilder(Writer::class)
-            ->disableOriginalConstructor()
-            ->getMock();
 
-        $configImportCommand = new ConfigImportCommand(
-            $this->importerMock,
-            $this->writerMock
-        );
+        $configImportCommand = new ConfigImportCommand($this->importerMock);
 
         $this->commandTester = new CommandTester($configImportCommand);
     }
@@ -54,41 +42,20 @@ protected function setUp()
      */
     public function testExecute()
     {
-        $this->writerMock->expects($this->once())
-            ->method('checkIfWritable')
-            ->willReturn(true);
         $this->importerMock->expects($this->once())
             ->method('import');
 
         $this->assertSame(Cli::RETURN_SUCCESS, $this->commandTester->execute([]));
     }
 
-    /**
-     * @return void
-     */
-    public function testExecuteWithoutWritePermissions()
-    {
-        $this->writerMock->expects($this->once())
-            ->method('checkIfWritable')
-            ->willReturn(false);
-        $this->importerMock->expects($this->never())
-            ->method('import');
-
-        $this->assertSame(Cli::RETURN_FAILURE, $this->commandTester->execute([]));
-        $this->assertContains('Deployment configuration file is not writable.', $this->commandTester->getDisplay());
-    }
-
     /**
      * @return void
      */
     public function testExecuteWithException()
     {
-        $this->writerMock->expects($this->once())
-            ->method('checkIfWritable')
-            ->willReturn(true);
         $this->importerMock->expects($this->once())
             ->method('import')
-            ->willThrowException(new ImportFailedException(__('Some error')));
+            ->willThrowException(new RuntimeException(__('Some error')));
 
         $this->assertSame(Cli::RETURN_FAILURE, $this->commandTester->execute([]));
         $this->assertContains('Some error', $this->commandTester->getDisplay());

app/code/Magento/Deploy/Test/Unit/Console/Command/App/ConfigImportCommandTest.php

@@ -54,7 +54,7 @@ public function testGetConfig()
             ->method('getConfigData')
             ->willReturnMap([['first', 'some data']]);
 
-        $this->assertSame(['first' => 'some data'], $this->dataCollector->getConfig());
+        $this->assertSame(['first' => 'some data', 'second' => null], $this->dataCollector->getConfig());
     }
 
     /**

app/code/Magento/Deploy/Test/Unit/Model/DeploymentConfig/DataCollectorTest.php

@@ -0,0 +1,14 @@
+<?php
+/**
+ * Copyright © 2013-2017 Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Framework\Exception;
+
+/**
+ * Exception thrown if an error which can only be found on runtime occurs.
+ */
+class RuntimeException extends LocalizedException
+{
+
+}