MAGECLOUD-2514: Add List of Modules which were Enabled by module:refresh Command to cloud.log (#548)

oshmyheliuk · BaDos · commit 699ca216ecbf · 2019-07-29T16:12:43.000-05:00
diff --git a/src/App/Container.php b/src/App/Container.php
@@ -10,6 +10,7 @@
 use Magento\MagentoCloud\Command\Build;
 use Magento\MagentoCloud\Command\CronKill;
 use Magento\MagentoCloud\Command\Deploy;
+use Magento\MagentoCloud\Command\ModuleRefresh;
 use Magento\MagentoCloud\Command\PostDeploy;
 use Magento\MagentoCloud\Config\Database\ConfigInterface;
 use Magento\MagentoCloud\Config\Database\MergedConfig;
@@ -390,6 +391,9 @@ function () {
         $this->container->when(CronKill::class)
             ->needs(ProcessInterface::class)
             ->give(DeployProcess\CronProcessKill::class);
+        $this->container->when(ModuleRefresh::class)
+            ->needs(ProcessInterface::class)
+            ->give(BuildProcess\RefreshModules::class);
 
         $this->container->singleton(ConfigInterface::class, MergedConfig::class);
     }
diff --git a/src/Command/ModuleRefresh.php b/src/Command/ModuleRefresh.php
@@ -5,7 +5,8 @@
  */
 namespace Magento\MagentoCloud\Command;
 
-use Magento\MagentoCloud\Config\Module;
+use Magento\MagentoCloud\Process\ProcessException;
+use Magento\MagentoCloud\Process\ProcessInterface;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Console\Command\Command;
 use Symfony\Component\Console\Input\InputInterface;
@@ -19,23 +20,23 @@ class ModuleRefresh extends Command
     const NAME = 'module:refresh';
 
     /**
-     * @var LoggerInterface
+     * @var ProcessInterface
      */
-    private $logger;
+    private $process;
 
     /**
-     * @var Module
+     * @var LoggerInterface
      */
-    private $config;
+    private $logger;
 
     /**
+     * @param ProcessInterface $process
      * @param LoggerInterface $logger
-     * @param Module $config
      */
-    public function __construct(LoggerInterface $logger, Module $config)
+    public function __construct(ProcessInterface $process, LoggerInterface $logger)
     {
+        $this->process = $process;
         $this->logger = $logger;
-        $this->config = $config;
 
         parent::__construct();
     }
@@ -52,15 +53,13 @@ protected function configure()
     /**
      * {@inheritdoc}
      *
-     * @throws \Exception
+     * @throws ProcessException
      */
     public function execute(InputInterface $input, OutputInterface $output)
     {
         try {
-            $this->logger->info('Refreshing modules started.');
-            $this->config->refresh();
-            $this->logger->info('Refreshing modules completed.');
-        } catch (\Exception $exception) {
+            $this->process->execute();
+        } catch (ProcessException $exception) {
             $this->logger->critical($exception->getMessage());
 
             throw $exception;
diff --git a/src/Config/Module.php b/src/Config/Module.php
@@ -37,22 +37,25 @@ public function __construct(ConfigInterface $config, ShellFactory $shellFactory)
 
     /**
      * Reconciling installed modules with shared config.
+     * Returns list of new enabled modules or an empty array if no modules were enabled.
      *
      * @throws ShellException
      * @throws FileSystemException
      */
-    public function refresh()
+    public function refresh(): array
     {
         $moduleConfig = (array)$this->config->get('modules');
 
-        if (!$moduleConfig) {
-            $this->magentoShell->execute('module:enable --all');
-            $this->config->reset();
+        $this->magentoShell->execute('module:enable --all');
+
+        $this->config->reset();
 
-            return;
+        $updatedModuleConfig = (array)$this->config->get('modules');
+
+        if ($moduleConfig) {
+            $this->config->update(['modules' => $moduleConfig]);
         }
 
-        $this->magentoShell->execute('module:enable --all');
-        $this->config->update(['modules' => $moduleConfig]);
+        return array_keys(array_diff_key($updatedModuleConfig, $moduleConfig));
     }
 }
diff --git a/src/Process/Build/RefreshModules.php b/src/Process/Build/RefreshModules.php
@@ -8,7 +8,6 @@
 use Magento\MagentoCloud\Config\Module;
 use Magento\MagentoCloud\Process\ProcessException;
 use Magento\MagentoCloud\Process\ProcessInterface;
-use Magento\MagentoCloud\Shell\ShellException;
 use Psr\Log\LoggerInterface;
 
 /**
@@ -46,8 +45,13 @@ public function execute()
         $this->logger->notice('Reconciling installed modules with shared config.');
 
         try {
-            $this->config->refresh();
-        } catch (ShellException $exception) {
+            $enabledModules = $this->config->refresh();
+            $this->logger->info(
+                $enabledModules ?
+                    'The following modules have been enabled:' . PHP_EOL . implode(PHP_EOL, $enabledModules) :
+                    'No modules were changed.'
+            );
+        } catch (\Exception $exception) {
             throw new ProcessException($exception->getMessage(), $exception->getCode(), $exception);
         }
         $this->logger->notice('End of reconciling modules.');
diff --git a/src/Test/Unit/Command/ModuleRefreshTest.php b/src/Test/Unit/Command/ModuleRefreshTest.php
@@ -6,7 +6,8 @@
 namespace Magento\MagentoCloud\Test\Unit\Command;
 
 use Magento\MagentoCloud\Command\ModuleRefresh;
-use Magento\MagentoCloud\Config\Module;
+use Magento\MagentoCloud\Process\ProcessException;
+use Magento\MagentoCloud\Process\ProcessInterface;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
@@ -23,39 +24,35 @@ class ModuleRefreshTest extends TestCase
     private $command;
 
     /**
-     * @var LoggerInterface|MockObject
+     * @var ProcessInterface|MockObject
      */
-    private $loggerMock;
+    private $processMock;
 
     /**
-     * @var Module|MockObject
+     * @var LoggerInterface|MockObject
      */
-    private $configMock;
+    private $loggerMock;
 
     /**
      * @inheritdoc
      */
     protected function setUp()
     {
+        $this->processMock = $this->getMockForAbstractClass(ProcessInterface::class);
         $this->loggerMock = $this->getMockForAbstractClass(LoggerInterface::class);
-        $this->configMock = $this->createMock(Module::class);
 
         $this->command = new ModuleRefresh(
-            $this->loggerMock,
-            $this->configMock
+            $this->processMock,
+            $this->loggerMock
         );
     }
 
     public function testExecute()
     {
-        $this->loggerMock->expects($this->exactly(2))
-            ->method('info')
-            ->withConsecutive(
-                ['Refreshing modules started.'],
-                ['Refreshing modules completed.']
-            );
-        $this->configMock->expects($this->once())
-            ->method('refresh');
+        $this->processMock->expects($this->once())
+            ->method('execute');
+        $this->loggerMock->expects($this->never())
+            ->method('critical');
 
         $tester = new CommandTester(
             $this->command
@@ -64,17 +61,17 @@ public function testExecute()
     }
 
     /**
-     * @expectedException \RuntimeException
-     * @expectedExceptionMessage Some exception
+     * @expectedException \Magento\MagentoCloud\Process\ProcessException
+     * @expectedExceptionMessage Some error
      */
     public function testExecuteWithException()
     {
-        $this->configMock->expects($this->once())
-            ->method('refresh')
-            ->willThrowException(new \RuntimeException('Some exception'));
+        $this->processMock->expects($this->once())
+            ->method('execute')
+            ->willThrowException(new ProcessException('Some error'));
         $this->loggerMock->expects($this->once())
             ->method('critical')
-            ->with('Some exception');
+            ->with('Some error');
 
         $tester = new CommandTester(
             $this->command
diff --git a/src/Test/Unit/Config/ModuleTest.php b/src/Test/Unit/Config/ModuleTest.php
@@ -53,10 +53,16 @@ protected function setUp()
 
     public function testRefreshWithMissingModuleConfig()
     {
-        $this->configMock->expects($this->once())
+        $this->configMock->expects($this->exactly(2))
             ->method('get')
             ->with('modules')
-            ->willReturn(null);
+            ->willReturnOnConsecutiveCalls(
+                null,
+                [
+                    'Magento_Module1' => 1,
+                    'Magento_Module2' => 1,
+                ]
+            );
         $this->configMock->expects($this->once())
             ->method('reset');
         $this->magentoShellMock->expects($this->once())
@@ -65,41 +71,53 @@ public function testRefreshWithMissingModuleConfig()
         $this->configMock->expects($this->never())
             ->method('update');
 
-        $this->module->refresh();
+        $this->assertEquals(
+            [
+                'Magento_Module1',
+                'Magento_Module2',
+            ],
+            $this->module->refresh()
+        );
     }
 
     public function testRefreshWithNewModules()
     {
-        $this->configMock->expects($this->once())
+        $this->configMock->expects($this->exactly(2))
             ->method('get')
             ->with('modules')
-            ->willReturn(['Some_OtherModule' => 1]);
+            ->willReturnOnConsecutiveCalls(
+                [
+                    'Magento_Module1' => 1,
+                    'Magento_Module2' => 0,
+                    'Magento_Module3' => 1,
+                ],
+                [
+                    'Magento_Module1' => 1,
+                    'Magento_Module2' => 1,
+                    'Magento_Module3' => 1,
+                    'Magento_Module4' => 1,
+                    'Magento_Module5' => 1,
+                ]
+            );
         $this->magentoShellMock->expects($this->once())
             ->method('execute')
             ->with('module:enable --all');
-        $this->configMock->expects($this->never())
-            ->method('reset');
         $this->configMock->expects($this->once())
-            ->method('update')
-            ->with(['modules' => ['Some_OtherModule' => 1]])
-            ->willReturn(null);
-
-        $this->module->refresh();
-    }
-
-    public function testRefreshWithNoNewModules()
-    {
+            ->method('reset');
         $this->configMock->expects($this->once())
-            ->method('get')
-            ->with('modules')
-            ->willReturn(['Some_ExistingModule' => 1]);
-        $this->configMock->expects($this->any())
-            ->method('all')
-            ->willReturn(['modules' => ['Some_ExistingModule' => 1]]);
-        $this->configMock->expects($this->any())
             ->method('update')
-            ->willReturn(null);
+            ->with(['modules' => [
+                'Magento_Module1' => 1,
+                'Magento_Module2' => 0,
+                'Magento_Module3' => 1,
+            ]]);
 
-        $this->module->refresh();
+        $this->assertEquals(
+            [
+                'Magento_Module4',
+                'Magento_Module5',
+            ],
+            $this->module->refresh()
+        );
     }
 }
diff --git a/src/Test/Unit/Process/Build/RefreshModulesTest.php b/src/Test/Unit/Process/Build/RefreshModulesTest.php
@@ -6,8 +6,9 @@
 namespace Magento\MagentoCloud\Test\Unit\Process\Build;
 
 use Magento\MagentoCloud\Config\Module;
-use Magento\MagentoCloud\Process\ProcessInterface;
 use Magento\MagentoCloud\Process\Build\RefreshModules;
+use Magento\MagentoCloud\Process\ProcessInterface;
+use Magento\MagentoCloud\Shell\ShellException;
 use PHPUnit\Framework\MockObject\MockObject;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
@@ -54,8 +55,43 @@ public function testExecute()
                 ['Reconciling installed modules with shared config.'],
                 ['End of reconciling modules.']
             );
+        $this->loggerMock->expects($this->once())
+            ->method('info')
+            ->with('The following modules have been enabled:' . PHP_EOL . 'module1' . PHP_EOL . 'module2');
+        $this->configMock->expects($this->once())
+            ->method('refresh')
+            ->willReturn(['module1', 'module2']);
+
+        $this->process->execute();
+    }
+
+    public function testExecuteNoModulesChanged()
+    {
+        $this->loggerMock->expects($this->exactly(2))
+            ->method('notice')
+            ->withConsecutive(
+                ['Reconciling installed modules with shared config.'],
+                ['End of reconciling modules.']
+            );
+        $this->loggerMock->expects($this->once())
+            ->method('info')
+            ->with('No modules were changed.');
+        $this->configMock->expects($this->once())
+            ->method('refresh')
+            ->willReturn([]);
+
+        $this->process->execute();
+    }
+
+    /**
+     * @expectedException \Magento\MagentoCloud\Process\ProcessException
+     * @expectedExceptionMessage some error
+     */
+    public function testExecuteWithException()
+    {
         $this->configMock->expects($this->once())
-            ->method('refresh');
+            ->method('refresh')
+            ->willThrowException(new ShellException('some error'));
 
         $this->process->execute();
     }

Original file line number	Diff line number	Diff line change
`@@ -37,22 +37,25 @@ public function __construct(ConfigInterface $config, ShellFactory $shellFactory)`
`37`	`37`
`38`	`38`	`/**`
`39`	`39`	`* Reconciling installed modules with shared config.`
	`40`	`+ * Returns list of new enabled modules or an empty array if no modules were enabled.`
`40`	`41`	`*`
`41`	`42`	`* @throws ShellException`
`42`	`43`	`* @throws FileSystemException`
`43`	`44`	`*/`
`44`		`- public function refresh()`
	`45`	`+ public function refresh(): array`
`45`	`46`	`{`
`46`	`47`	`$moduleConfig = (array)$this->config->get('modules');`
`47`	`48`
`48`		`- if (!$moduleConfig) {`
`49`		`- $this->magentoShell->execute('module:enable --all');`
`50`		`- $this->config->reset();`
	`49`	`+ $this->magentoShell->execute('module:enable --all');`
	`50`	`+`
	`51`	`+ $this->config->reset();`
`51`	`52`
`52`		`- return;`
	`53`	`+ $updatedModuleConfig = (array)$this->config->get('modules');`
	`54`	`+`
	`55`	`+ if ($moduleConfig) {`
	`56`	`+ $this->config->update(['modules' => $moduleConfig]);`
`53`	`57`	`}`
`54`	`58`
`55`		`- $this->magentoShell->execute('module:enable --all');`
`56`		`- $this->config->update(['modules' => $moduleConfig]);`
	`59`	`+ return array_keys(array_diff_key($updatedModuleConfig, $moduleConfig));`
`57`	`60`	`}`
`58`	`61`	`}`