Merge pull request #1173 from magento-falcons/MAGETWO-69556

[Falcons] Delivery of bug fixes for Setup and Deployment
magento · Jun 9, 2017 · 760bb52 · 760bb52
2 parents 70c8c2e + 9a8adb9
commit 760bb52
Show file tree

Hide file tree

Showing 8 changed files with 167 additions and 121 deletions.
diff --git a/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php b/app/code/Magento/Config/Console/Command/ConfigSet/LockProcessor.php
@@ -8,6 +8,7 @@
 use Magento\Config\App\Config\Type\System;
 use Magento\Config\Model\PreparedValueFactory;
 use Magento\Framework\App\Config\ConfigPathResolver;
+use Magento\Framework\App\Config\Value;
 use Magento\Framework\App\DeploymentConfig;
 use Magento\Framework\Config\File\ConfigFilePool;
 use Magento\Framework\Exception\CouldNotSaveException;
@@ -79,19 +80,27 @@ public function process($path, $value, $scope, $scopeCode)
             $configPath = $this->configPathResolver->resolve($path, $scope, $scopeCode, System::CONFIG_TYPE);
             $backendModel = $this->preparedValueFactory->create($path, $value, $scope, $scopeCode);
 
-            /**
-             * Temporary solution until Magento introduce unified interface
-             * for storing system configuration into database and configuration files.
-             */
-            $backendModel->validateBeforeSave();
-            $backendModel->beforeSave();
+            if ($backendModel instanceof Value) {
+                /**
+                 * Temporary solution until Magento introduce unified interface
+                 * for storing system configuration into database and configuration files.
+                 */
+                $backendModel->validateBeforeSave();
+                $backendModel->beforeSave();
 
-            $this->deploymentConfigWriter->saveConfig(
-                [ConfigFilePool::APP_ENV => $this->arrayManager->set($configPath, [], $backendModel->getValue())],
-                false
-            );
+                $value = $backendModel->getValue();
 
-            $backendModel->afterSave();
+                $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
+                );
+            }
         } catch (\Exception $exception) {
             throw new CouldNotSaveException(__('%1', $exception->getMessage()), $exception);
         }

diff --git a/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php b/app/code/Magento/Config/Test/Unit/Console/Command/ConfigSet/LockProcessorTest.php
@@ -206,10 +206,10 @@ public function testCustomException()
             ->willReturn($this->valueMock);
         $this->arrayManagerMock->expects($this->never())
             ->method('set');
-        $this->valueMock->expects($this->never())
+        $this->valueMock->expects($this->once())
             ->method('getValue');
         $this->valueMock->expects($this->once())
-            ->method('validateBeforeSave')
+            ->method('afterSave')
             ->willThrowException(new \Exception('Invalid values'));
         $this->deploymentConfigWriterMock->expects($this->never())
             ->method('saveConfig');

diff --git a/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php b/dev/tests/integration/testsuite/Magento/Sitemap/Model/ResourceModel/Catalog/ProductTest.php
@@ -9,7 +9,7 @@
  * Test class for \Magento\Sitemap\Model\ResourceModel\Catalog\Product.
  * - test products collection generation for sitemap
  *
- * @magentoDataFixtureBeforeTransaction Magento/CatalogSearch/_files/full_reindex.php
+ * @magentoDataFixtureBeforeTransaction Magento/Catalog/_files/enable_reindex_schedule.php
  * @magentoDataFixture Magento/Sitemap/_files/sitemap_products.php
  */
 class ProductTest extends \PHPUnit_Framework_TestCase

diff --git a/lib/internal/Magento/Framework/Console/Cli.php b/lib/internal/Magento/Framework/Console/Cli.php
@@ -11,7 +11,7 @@
 use Magento\Framework\App\ProductMetadata;
 use Magento\Framework\App\State;
 use Magento\Framework\Composer\ComposerJsonFinder;
-use Magento\Framework\Exception\FileSystemException;
+use Magento\Framework\Console\Exception\GenerationDirectoryAccessException;
 use Magento\Framework\Filesystem\Driver\File;
 use Magento\Framework\ObjectManagerInterface;
 use Magento\Framework\Shell\ComplexParameter;
@@ -66,17 +66,27 @@ class Cli extends Console\Application
     /**
      * @param string $name the application name
      * @param string $version the application version
+     * @SuppressWarnings(PHPMD.ExitExpression)
      */
     public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
     {
-        $configuration = require BP . '/setup/config/application.config.php';
-        $bootstrapApplication = new Application();
-        $application = $bootstrapApplication->bootstrap($configuration);
-        $this->serviceManager = $application->getServiceManager();
+        try {
+            $configuration = require BP . '/setup/config/application.config.php';
+            $bootstrapApplication = new Application();
+            $application = $bootstrapApplication->bootstrap($configuration);
+            $this->serviceManager = $application->getServiceManager();
+
+            $this->assertCompilerPreparation();
+            $this->initObjectManager();
+            $this->assertGenerationPermissions();
+        } catch (\Exception $exception) {
+            $output = new \Symfony\Component\Console\Output\ConsoleOutput();
+            $output->writeln(
+                '<error>' . $exception->getMessage() . '</error>'
+            );
 
-        $this->assertCompilerPreparation();
-        $this->initObjectManager();
-        $this->assertGenerationPermissions();
+            exit(static::RETURN_FAILURE);
+        }
 
         if ($version == 'UNKNOWN') {
             $directoryList = new DirectoryList(BP);
@@ -91,20 +101,13 @@ public function __construct($name = 'UNKNOWN', $version = 'UNKNOWN')
     /**
      * {@inheritdoc}
      *
-     * @throws \Exception the exception in case of unexpected error
+     * @throws \Exception The exception in case of unexpected error
      */
     public function doRun(Console\Input\InputInterface $input, Console\Output\OutputInterface $output)
     {
         $exitCode = parent::doRun($input, $output);
 
         if ($this->initException) {
-            $output->writeln(
-                "<error>We're sorry, an error occurred. Try clearing the cache and code generation directories. "
-                . "By default, they are: " . $this->getDefaultDirectoryPath(DirectoryList::CACHE) . ", "
-                . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_METADATA) . ", "
-                . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ", and var/page_cache.</error>"
-            );
-
             throw $this->initException;
         }
 
@@ -154,24 +157,17 @@ protected function getApplicationCommands()
      * Object Manager initialization.
      *
      * @return void
-     * @SuppressWarnings(PHPMD.ExitExpression)
      */
     private function initObjectManager()
     {
-        try {
-            $params = (new ComplexParameter(self::INPUT_KEY_BOOTSTRAP))->mergeFromArgv($_SERVER, $_SERVER);
-            $params[Bootstrap::PARAM_REQUIRE_MAINTENANCE] = null;
-
-            $this->objectManager = Bootstrap::create(BP, $params)->getObjectManager();
+        $params = (new ComplexParameter(self::INPUT_KEY_BOOTSTRAP))->mergeFromArgv($_SERVER, $_SERVER);
+        $params[Bootstrap::PARAM_REQUIRE_MAINTENANCE] = null;
 
-            /** @var ObjectManagerProvider $omProvider */
-            $omProvider = $this->serviceManager->get(ObjectManagerProvider::class);
-            $omProvider->setObjectManager($this->objectManager);
-        } catch (FileSystemException $exception) {
-            $this->writeGenerationDirectoryReadError();
+        $this->objectManager = Bootstrap::create(BP, $params)->getObjectManager();
 
-            exit(static::RETURN_FAILURE);
-        }
+        /** @var ObjectManagerProvider $omProvider */
+        $omProvider = $this->serviceManager->get(ObjectManagerProvider::class);
+        $omProvider->setObjectManager($this->objectManager);
     }
 
     /**
@@ -182,7 +178,7 @@ private function initObjectManager()
      *      developer - application will be terminated
      *
      * @return void
-     * @SuppressWarnings(PHPMD.ExitExpression)
+     * @throws GenerationDirectoryAccessException If generation directory is read-only in developer mode
      */
     private function assertGenerationPermissions()
     {
@@ -197,17 +193,15 @@ private function assertGenerationPermissions()
         if ($state->getMode() !== State::MODE_PRODUCTION
             && !$generationDirectoryAccess->check()
         ) {
-            $this->writeGenerationDirectoryReadError();
-
-            exit(static::RETURN_FAILURE);
+            throw new GenerationDirectoryAccessException();
         }
     }
 
     /**
      * Checks whether compiler is being prepared.
      *
      * @return void
-     * @SuppressWarnings(PHPMD.ExitExpression)
+     * @throws GenerationDirectoryAccessException If generation directory is read-only
      */
     private function assertCompilerPreparation()
     {
@@ -222,33 +216,10 @@ private function assertCompilerPreparation()
                 new File()
             );
 
-            try {
-                $compilerPreparation->handleCompilerEnvironment();
-            } catch (FileSystemException $e) {
-                $this->writeGenerationDirectoryReadError();
-
-                exit(static::RETURN_FAILURE);
-            }
+            $compilerPreparation->handleCompilerEnvironment();
         }
     }
 
-    /**
-     * Writes read error to console.
-     *
-     * @return void
-     */
-    private function writeGenerationDirectoryReadError()
-    {
-        $output = new \Symfony\Component\Console\Output\ConsoleOutput();
-        $output->writeln(
-            '<error>'
-            . 'Command line user does not have read and write permissions on '
-            . $this->getDefaultDirectoryPath(DirectoryList::GENERATED_CODE) . ' directory. '
-            . 'Please address this issue before using Magento command line.'
-            . '</error>'
-        );
-    }
-
     /**
      * Retrieves vendor commands.
      *
@@ -270,22 +241,4 @@ protected function getVendorCommands($objectManager)
 
         return $commands;
     }
-
-    /**
-     * Get default directory path by code
-     *
-     * @param string $code
-     * @return string
-     */
-    private function getDefaultDirectoryPath($code)
-    {
-        $config = DirectoryList::getDefaultConfig();
-        $result = '';
-
-        if (isset($config[$code][DirectoryList::PATH])) {
-            $result = $config[$code][DirectoryList::PATH];
-        }
-
-        return $result;
-    }
 }
diff --git a/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php b/lib/internal/Magento/Framework/Console/Exception/GenerationDirectoryAccessException.php
@@ -0,0 +1,48 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Framework\Console\Exception;
+
+use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Exception\FileSystemException;
+use Magento\Framework\Phrase;
+
+/**
+ * The default exception for missing write permissions on compilation generated folder.
+ */
+class GenerationDirectoryAccessException extends FileSystemException
+{
+    /**
+     * @inheritdoc
+     */
+    public function __construct(Phrase $phrase = null, \Exception $cause = null, $code = 0)
+    {
+        $phrase = $phrase ?: new Phrase(
+            'Command line user does not have read and write permissions on '
+            . $this->getDefaultDirectoryPath(DirectoryList::GENERATED) . ' directory. '
+            . 'Please address this issue before using Magento command line.'
+        );
+
+        parent::__construct($phrase, $cause, $code);
+    }
+
+    /**
+     * Get default directory path by code
+     *
+     * @param string $code
+     * @return string
+     */
+    private function getDefaultDirectoryPath($code)
+    {
+        $config = DirectoryList::getDefaultConfig();
+        $result = '';
+
+        if (isset($config[$code][DirectoryList::PATH])) {
+            $result = $config[$code][DirectoryList::PATH];
+        }
+
+        return $result;
+    }
+}
diff --git a/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php b/lib/internal/Magento/Framework/Console/GenerationDirectoryAccess.php
@@ -7,9 +7,8 @@
 
 use Magento\Framework\App\Bootstrap;
 use Magento\Framework\App\Filesystem\DirectoryList;
+use Magento\Framework\Filesystem\Directory\WriteFactory;
 use Magento\Framework\Filesystem\DriverPool;
-use Magento\Framework\Filesystem\File\WriteFactory;
-use Magento\Framework\Filesystem\Directory\Write;
 use Zend\ServiceManager\ServiceManager;
 use Magento\Setup\Mvc\Bootstrap\InitParamListener;
 
@@ -33,7 +32,7 @@ public function __construct(
     }
 
     /**
-     * Check generated/code read and write access
+     * Check write permissions to generation folders
      *
      * @return bool
      */
@@ -44,33 +43,32 @@ public function check()
             ? $initParams[Bootstrap::INIT_PARAM_FILESYSTEM_DIR_PATHS]
             : [];
         $directoryList = new DirectoryList(BP, $filesystemDirPaths);
-        $generationDirectoryPath = $directoryList->getPath(DirectoryList::GENERATED_CODE);
         $driverPool = new DriverPool();
         $fileWriteFactory = new WriteFactory($driverPool);
-        /** @var \Magento\Framework\Filesystem\DriverInterface $driver */
-        $driver = $driverPool->getDriver(DriverPool::FILE);
-        $directoryWrite = new Write($fileWriteFactory, $driver, $generationDirectoryPath);
-        if ($directoryWrite->isExist()) {
-            if ($directoryWrite->isDirectory()
-                || $directoryWrite->isReadable()
-            ) {
+
+        $generationDirs = [
+            DirectoryList::GENERATED,
+            DirectoryList::GENERATED_CODE,
+            DirectoryList::GENERATED_METADATA
+        ];
+
+        foreach ($generationDirs as $generationDirectory) {
+            $directoryPath = $directoryList->getPath($generationDirectory);
+            $directoryWrite = $fileWriteFactory->create($directoryPath);
+
+            if (!$directoryWrite->isExist()) {
                 try {
-                    $probeFilePath = $generationDirectoryPath . DIRECTORY_SEPARATOR . uniqid(mt_rand()).'tmp';
-                    $fileWriteFactory->create($probeFilePath, DriverPool::FILE, 'w');
-                    $driver->deleteFile($probeFilePath);
+                    $directoryWrite->create();
                 } catch (\Exception $e) {
                     return false;
                 }
-            } else {
-                return false;
             }
-        } else {
-            try {
-                $directoryWrite->create();
-            } catch (\Exception $e) {
+
+            if (!$directoryWrite->isWritable()) {
                 return false;
             }
         }
+
         return true;
     }
 }
diff --git a/.../Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php b/.../Magento/Framework/Console/Test/Unit/Exception/GenerationDirectoryAccessExceptionTest.php
@@ -0,0 +1,21 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\Framework\Console\Test\Unit\Exception;
+
+use Magento\Framework\Console\Exception\GenerationDirectoryAccessException;
+
+class GenerationDirectoryAccessExceptionTest extends \PHPUnit_Framework_TestCase
+{
+    public function testConstructor()
+    {
+        $exception = new GenerationDirectoryAccessException();
+
+        $this->assertContains(
+            'Command line user does not have read and write permissions on generated directory.',
+            $exception->getMessage()
+        );
+    }
+}