MAGECLOUD-2626: Create Wrapper for Running bin/magento (#349)

Author: shiftedreality

src/Config/Environment.php

@@ -251,14 +251,6 @@ public function getCryptKey(): string
         return $this->getVariable('CRYPT_KEY', '');
     }
 
-    /**
-     * @return string
-     */
-    public function getMinLoggingLevel(): string
-    {
-        return $this->getVariable('MIN_LOGGING_LEVEL', '');
-    }
-
     /**
      * Checks that environment uses the main branch depending on environment variable MAGENTO_CLOUD_ENVIRONMENT
      * which contains the name of the git branch.

src/Config/Magento/System.php

@@ -9,30 +9,30 @@
 
 use Magento\MagentoCloud\Package\MagentoVersion;
 use Magento\MagentoCloud\Package\UndefinedPackageException;
-use Magento\MagentoCloud\Shell\ShellInterface;
+use Magento\MagentoCloud\Shell\ShellFactory;
 
 /**
  * Retrieves a value by running bin/magento config:show
  */
 class System
 {
     /**
-     * @var ShellInterface
+     * @var ShellFactory
      */
-    private $shell;
+    private $shellFactory;
 
     /**
      * @var MagentoVersion
      */
     private $magentoVersion;
 
     /**
-     * @param ShellInterface $shell
+     * @param ShellFactory $shellFactory
      * @param MagentoVersion $magentoVersion
      */
-    public function __construct(ShellInterface $shell, MagentoVersion $magentoVersion)
+    public function __construct(ShellFactory $shellFactory, MagentoVersion $magentoVersion)
     {
-        $this->shell = $shell;
+        $this->shellFactory = $shellFactory;
         $this->magentoVersion = $magentoVersion;
     }
 
@@ -50,15 +50,9 @@ public function get(string $key)
             return null;
         }
 
-        try {
-            $result = implode(PHP_EOL, $this->shell->execute(sprintf(
-                'php ./bin/magento config:show %s',
-                escapeshellarg($key)
-            )));
-        } catch (\Exception $e) {
-            return null;
-        }
-
-        return $result;
+        return implode(PHP_EOL, $this->shellFactory->create(ShellFactory::STRATEGY_MAGENTO_SHELL)->execute(
+            'config:show',
+            $key
+        ));
     }
 }

src/Shell/MagentoShell.php

@@ -0,0 +1,37 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\MagentoCloud\Shell;
+
+/**
+ * ./bin/magento shell wrapper.
+ */
+class MagentoShell implements ShellInterface
+{
+    /**
+     * @var Shell
+     */
+    private $shell;
+
+    /**
+     * @param Shell $shell
+     */
+    public function __construct(Shell $shell)
+    {
+        $this->shell = $shell;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function execute(string $command, $args = []): array
+    {
+        $defaultArgs = ['--ansi', '--no-interaction'];
+
+        return $this->shell->execute('php ./bin/magento ' . $command, array_merge($defaultArgs, (array)$args));
+    }
+}

src/Shell/Shell.php

@@ -3,9 +3,11 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\MagentoCloud\Shell;
 
-use Magento\MagentoCloud\Filesystem\DirectoryList;
+use Magento\MagentoCloud\Filesystem\SystemList;
 use Psr\Log\LoggerInterface;
 use Monolog\Logger;
 
@@ -20,18 +22,18 @@ class Shell implements ShellInterface
     private $logger;
 
     /**
-     * @var DirectoryList
+     * @var SystemList
      */
-    private $directoryList;
+    private $systemList;
 
     /**
      * @param LoggerInterface $logger
-     * @param DirectoryList $directoryList
+     * @param SystemList $systemList
      */
-    public function __construct(LoggerInterface $logger, DirectoryList $directoryList)
+    public function __construct(LoggerInterface $logger, SystemList $systemList)
     {
         $this->logger = $logger;
-        $this->directoryList = $directoryList;
+        $this->systemList = $systemList;
     }
 
     /**
@@ -43,18 +45,47 @@ public function __construct(LoggerInterface $logger, DirectoryList $directoryLis
      * $this->shell->execute('/bin/bash -c "set -o pipefail; firstCommand | secondCommand"');
      * ```
      */
-    public function execute(string $command)
+    public function execute(string $command, $args = []): array
     {
+        $args = array_map(
+            'escapeshellarg',
+            array_filter((array)$args)
+        );
+
+        if ($args) {
+            $command .= ' ' . implode(' ', $args);
+        }
+
         $this->logger->info($command);
 
-        $rootPathCommand = sprintf(
+        $fullCommand = sprintf(
             'cd %s && %s 2>&1',
-            $this->directoryList->getMagentoRoot(),
+            $this->systemList->getMagentoRoot(),
             $command
         );
 
-        exec($rootPathCommand, $output, $status);
+        exec($fullCommand, $output, $status);
+
+        /**
+         * config:show will return non-zero exit code, if the value was not changed and remains default.
+         */
+        if ($status !== 0 && strpos($command, 'config:show') !== false) {
+            return [];
+        }
 
+        return $this->handleOutput($command, $output, $status);
+    }
+
+    /**
+     * @param string $command
+     * @param array $output
+     * @param int $status
+     * @return array
+     *
+     * @throws ShellException
+     */
+    private function handleOutput(string $command, array $output, int $status): array
+    {
         if ($output) {
             $message = array_reduce(
                 $output,
@@ -64,11 +95,11 @@ function ($message, $line) {
                 ''
             );
 
-            $this->logger->log($status != 0 ? Logger::CRITICAL : Logger::DEBUG, $message);
+            $this->logger->log($status !== 0 ? Logger::CRITICAL : Logger::DEBUG, $message);
         }
 
-        if ($status != 0) {
-            throw new \RuntimeException("Command $command returned code $status", $status);
+        if ($status !== 0) {
+            throw new ShellException("Command $command returned code $status", $status);
         }
 
         return $output;

src/Shell/ShellException.php

@@ -0,0 +1,13 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+namespace Magento\MagentoCloud\Shell;
+
+/**
+ * Exception for shell operations.
+ */
+class ShellException extends \RuntimeException
+{
+}

src/Shell/ShellFactory.php

@@ -0,0 +1,45 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\MagentoCloud\Shell;
+
+use Magento\MagentoCloud\App\ContainerInterface;
+
+/**
+ * Factory class for shell wrappers.
+ */
+class ShellFactory
+{
+    const STRATEGY_SHELL = 'shell';
+    const STRATEGY_MAGENTO_SHELL = 'magento_shell';
+
+    /**
+     * @var ContainerInterface
+     */
+    private $container;
+
+    /**
+     * @param ContainerInterface $container
+     */
+    public function __construct(ContainerInterface $container)
+    {
+        $this->container = $container;
+    }
+
+    /**
+     * @param string $strategy
+     * @return ShellInterface
+     */
+    public function create(string $strategy): ShellInterface
+    {
+        if ($strategy === self::STRATEGY_MAGENTO_SHELL) {
+            return $this->container->create(MagentoShell::class);
+        }
+
+        return $this->container->create(Shell::class);
+    }
+}

src/Shell/ShellInterface.php

@@ -3,6 +3,8 @@
  * Copyright © Magento, Inc. All rights reserved.
  * See COPYING.txt for license details.
  */
+declare(strict_types=1);
+
 namespace Magento\MagentoCloud\Shell;
 
 /**
@@ -14,8 +16,9 @@ interface ShellInterface
      * Runs shell command.
      *
      * @param string $command The command.
+     * @param string|array $args Arguments to pass
      * @return array The output of command.
-     * @throws \RuntimeException If command was executed with error
+     * @throws ShellException If command was executed with error
      */
-    public function execute(string $command);
+    public function execute(string $command, $args = []): array;
 }

src/Test/Unit/Config/Magento/SystemTest.php

@@ -10,6 +10,7 @@
 use Magento\MagentoCloud\Config\Magento\System;
 use Magento\MagentoCloud\Package\MagentoVersion;
 use Magento\MagentoCloud\Package\UndefinedPackageException;
+use Magento\MagentoCloud\Shell\ShellFactory;
 use Magento\MagentoCloud\Shell\ShellInterface;
 use PHPUnit\Framework\TestCase;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -24,6 +25,11 @@ class SystemTest extends TestCase
      */
     private $config;
 
+    /**
+     * @var ShellFactory|MockObject
+     */
+    private $shellFactoryMock;
+
     /**
      * @var ShellInterface|MockObject
      */
@@ -39,11 +45,16 @@ class SystemTest extends TestCase
      */
     protected function setUp()
     {
-        $this->shellMock = $this->getMockForAbstractClass(ShellInterface::class);
+        $this->shellFactoryMock = $this->createMock(ShellFactory::class);
         $this->magentoVersionMock = $this->createMock(MagentoVersion::class);
+        $this->shellMock = $this->getMockForAbstractClass(ShellInterface::class);
+
+        $this->shellFactoryMock->method('create')
+            ->with(ShellFactory::STRATEGY_MAGENTO_SHELL)
+            ->willReturn($this->shellMock);
 
         $this->config = new System(
-            $this->shellMock,
+            $this->shellFactoryMock,
             $this->magentoVersionMock
         );
     }
@@ -61,7 +72,7 @@ public function testValidate($expectedResult)
             ->willReturn(true);
         $this->shellMock->expects($this->once())
             ->method('execute')
-            ->with('php ./bin/magento config:show \'some/key\'')
+            ->with('config:show', 'some/key')
             ->willReturn([$expectedResult]);
 
         $this->assertSame($expectedResult, $this->config->get('some/key'));
@@ -90,10 +101,10 @@ public function testGetDefaultValue()
             ->willReturn(true);
         $this->shellMock->expects($this->once())
             ->method('execute')
-            ->with('php ./bin/magento config:show \'some/key\'')
-            ->willThrowException(new \Exception('Command bin/magento returned code 1', 1));
+            ->with('config:show', 'some/key')
+            ->willReturn([]);
 
-        $this->assertNull($this->config->get('some/key'));
+        $this->assertSame('', $this->config->get('some/key'));
     }
 
     /**

src/Test/Unit/Patch/ApplierTest.php

@@ -219,13 +219,14 @@ public function testApplyPatchAlreadyApplied()
 
     /**
      * @param string $command
-     * @throws RuntimeException when the command isn't a reverse
+     * @return array
+     * @throws \RuntimeException when the command isn't a reverse
      */
     public function shellMockReverseCallback(string $command)
     {
         if (strpos($command, '--reverse') !== false && strpos($command, '--check') !== false) {
             // Command was the reverse check, it's all good.
-            return;
+            return [];
         }
 
         // Not a reverse, better throw an exception.