MC-42841: Revert of MC-42026

dyushkin · dyushkin · commit 6ee22207ed4b · 2021-07-13T13:17:54.000-05:00
diff --git a/app/code/Magento/Deploy/Process/Queue.php b/app/code/Magento/Deploy/Process/Queue.php
@@ -46,11 +46,6 @@ class Queue
      */
     private $inProgress = [];
 
-    /**
-     * @var Package[]
-     */
-    private $failed = [];
-
     /**
      * @var int
      */
@@ -167,7 +162,7 @@ public function getPackages()
      * Process jobs
      *
      * @return int
-     * @throws \RuntimeException
+     * @throws TimeoutException
      */
     public function process()
     {
@@ -198,13 +193,8 @@ public function process()
 
         $this->awaitForAllProcesses();
 
-        $failedPackages = array_merge($this->failed, $packages);
-        if (!empty($failedPackages)) {
-            throw new \RuntimeException(
-                'The following packages have failed to deploy '
-                .PHP_EOL
-                .implode(PHP_EOL, array_keys($failedPackages))
-            );
+        if (!empty($packages)) {
+            throw new TimeoutException('Not all packages are deployed.');
         }
 
         return $returnStatus;
@@ -287,12 +277,7 @@ private function awaitForAllProcesses()
     {
         while ($this->inProgress && $this->checkTimeout()) {
             foreach ($this->inProgress as $name => $package) {
-                $isDeployed = $this->isDeployed($package);
-
-                if ($isDeployed === false) {
-                    $this->failed[$package->getPath()]= $this->packages[$package->getPath()];
-                    unset($this->inProgress[$name]);
-                } elseif ($isDeployed) {
+                if ($this->isDeployed($package)) {
                     unset($this->inProgress[$name]);
                 }
             }
@@ -366,25 +351,22 @@ function () use ($package) {
 
             // process child process
             $this->inProgress = [];
-            $isDeployed = $this->deployPackageService->deploy($package, $this->options, true);
+            $this->deployPackageService->deploy($package, $this->options, true);
             // phpcs:ignore Magento2.Security.LanguageConstruct.ExitUsage
-            exit((int)!$isDeployed);
+            exit(0);
         } else {
-            $isDeployed = $this->deployPackageService->deploy($package, $this->options);
-            if ($isDeployed === false) {
-                $this->failed[$package->getPath()]= $this->packages[$package->getPath()];
-            }
-            return $isDeployed;
+            $this->deployPackageService->deploy($package, $this->options);
+            return true;
         }
     }
 
     /**
      * Checks if package is deployed.
      *
      * @param Package $package
-     * @return null|bool
+     * @return bool
      */
-    private function isDeployed(Package $package): ?bool
+    private function isDeployed(Package $package)
     {
         if ($this->isCanBeParalleled()) {
             if ($package->getState() === null) {
@@ -393,7 +375,7 @@ private function isDeployed(Package $package): ?bool
                 // When $pid comes back as null the child process for this package has not yet started; prevents both
                 // hanging until timeout expires (which was behaviour in 2.2.x) and the type error from strict_types
                 if ($pid === null) {
-                    return null;
+                    return false;
                 }
 
                 // phpcs:ignore Magento2.Functions.DiscouragedFunction
@@ -424,10 +406,10 @@ private function isDeployed(Package $package): ?bool
                         "Error encountered checking child process status (PID: $pid): $strerror (errno: $errno)"
                     );
                 }
-                return null;
+                return false;
             }
         }
-        return (bool)$package->getState();
+        return $package->getState();
     }
 
     /**
diff --git a/app/code/Magento/Deploy/Process/TimeoutException.php b/app/code/Magento/Deploy/Process/TimeoutException.php
@@ -0,0 +1,15 @@
+<?php
+/**
+ * Copyright © Magento, Inc. All rights reserved.
+ * See COPYING.txt for license details.
+ */
+declare(strict_types=1);
+
+namespace Magento\Deploy\Process;
+
+/**
+ * Exception is thrown if deploy process is finished due to timeout.
+ */
+class TimeoutException extends \RuntimeException
+{
+}
diff --git a/app/code/Magento/Deploy/Service/DeployPackage.php b/app/code/Magento/Deploy/Service/DeployPackage.php
@@ -99,7 +99,7 @@ public function deploy(Package $package, array $options, $skipLogging = false)
             function () use ($package, $options, $skipLogging) {
                 // emulate application locale needed for correct file path resolving
                 $this->localeResolver->setLocale($package->getLocale());
-                return $this->deployEmulated($package, $options, $skipLogging);
+                $this->deployEmulated($package, $options, $skipLogging);
             }
         );
         $package->setState(Package::STATE_COMPLETED);
@@ -151,7 +151,7 @@ public function deployEmulated(Package $package, array $options, $skipLogging =
             $processor->process($package, $options);
         }
 
-        return !(bool)$this->errorsCount;
+        return true;
     }
 
     /**
@@ -204,14 +204,14 @@ private function processFile(PackageFile $file, Package $package)
     private function checkIfCanCopy(PackageFile $file, Package $package, Package $parentPackage = null)
     {
         return $parentPackage
-        && $file->getOrigPackage() !== $package
-        && (
-            $file->getArea() !== $package->getArea()
-            || $file->getTheme() !== $package->getTheme()
-            || $file->getLocale() !== $package->getLocale()
-        )
-        && $file->getOrigPackage() === $parentPackage
-        && $this->deployStaticFile->readFile($file->getDeployedFileId(), $parentPackage->getPath());
+            && $file->getOrigPackage() !== $package
+            && (
+                $file->getArea() !== $package->getArea()
+                || $file->getTheme() !== $package->getTheme()
+                || $file->getLocale() !== $package->getLocale()
+            )
+            && $file->getOrigPackage() === $parentPackage
+            && $this->deployStaticFile->readFile($file->getDeployedFileId(), $parentPackage->getPath());
     }
 
     /**
diff --git a/app/code/Magento/Deploy/Test/Unit/Process/QueueTest.php b/app/code/Magento/Deploy/Test/Unit/Process/QueueTest.php
@@ -8,16 +8,15 @@
 namespace Magento\Deploy\Test\Unit\Process;
 
 use Magento\Deploy\Package\Package;
-use Magento\Deploy\Package\PackageFile;
 use Magento\Deploy\Process\Queue;
 use Magento\Deploy\Service\DeployPackage;
-use Magento\Deploy\Service\DeployStaticFile;
 use Magento\Framework\App\ResourceConnection;
+
 use Magento\Framework\App\State as AppState;
-use Magento\Framework\Config\ScopeInterface;
 use Magento\Framework\Locale\ResolverInterface as LocaleResolver;
 use PHPUnit\Framework\MockObject\MockObject as Mock;
 use PHPUnit\Framework\TestCase;
+
 use Psr\Log\LoggerInterface;
 
 /**
@@ -28,7 +27,12 @@
 class QueueTest extends TestCase
 {
     /**
-     * @var AppState
+     * @var Queue
+     */
+    private $queue;
+
+    /**
+     * @var AppState|Mock
      */
     private $appState;
 
@@ -48,64 +52,39 @@ class QueueTest extends TestCase
     private $logger;
 
     /**
-     * @var DeployPackage
+     * @var DeployPackage|Mock
      */
     private $deployPackageService;
 
-    /**
-     * @var DeployStaticFile|Mock
-     */
-    private $deployStaticFile;
-
-    /**
-     * @var Package|Mock
-     */
-    private $package;
-
-    /**
-     * @var PackageFile|Mock
-     */
-    private $packageFile;
-
     /**
      * @inheritdoc
      */
     protected function setUp(): void
     {
-        $this->resourceConnection = $this->createMock(ResourceConnection::class);
-        $this->package = $this->createMock(Package::class);
-        $this->deployStaticFile = $this->createMock(DeployStaticFile::class);
-
-        $this->packageFile = $this->createMock(PackageFile::class);
-        $this->packageFile
-            ->expects($this->any())
-            ->method('getContent')
-            ->willReturn('{}');
-
+        $this->appState = $this->createMock(AppState::class);
         $this->localeResolver = $this->getMockForAbstractClass(
             LocaleResolver::class,
             ['setLocale'],
             '',
             false
         );
-
+        $this->resourceConnection = $this->createMock(ResourceConnection::class);
         $this->logger = $this->getMockForAbstractClass(
             LoggerInterface::class,
             ['notice', 'info'],
             '',
             false
         );
+        $this->deployPackageService = $this->createPartialMock(DeployPackage::class, ['deploy']);
 
-        $configScope = $this->createMock(ScopeInterface::class);
-        $this->appState = new AppState(
-            $configScope
-        );
-
-        $this->deployPackageService = new DeployPackage(
+        $this->queue = new Queue(
             $this->appState,
             $this->localeResolver,
-            $this->deployStaticFile,
-            $this->logger
+            $this->resourceConnection,
+            $this->logger,
+            $this->deployPackageService,
+            [],
+            1
         );
     }
 
@@ -114,21 +93,13 @@ protected function setUp(): void
      */
     public function testAdd()
     {
-        $queue = new Queue(
-            $this->appState,
-            $this->localeResolver,
-            $this->resourceConnection,
-            $this->logger,
-            $this->deployPackageService,
-            [],
-            0
-        );
+        $package = $this->createMock(Package::class);
+        $package->expects($this->once())->method('getPath')->willReturn('path');
 
-        $this->package->expects($this->once())->method('getPath')->willReturn('path');
-        $this->assertTrue($queue->add($this->package));
-        $packages = $queue->getPackages();
+        $this->assertTrue($this->queue->add($package));
+        $packages = $this->queue->getPackages();
         $this->assertEquals(
-            $this->package,
+            $package,
             isset($packages['path']['package']) ? $packages['path']['package'] : null
         );
     }
@@ -138,72 +109,20 @@ public function testAdd()
      */
     public function testProcess()
     {
-        $queue = new Queue(
-            $this->appState,
-            $this->localeResolver,
-            $this->resourceConnection,
-            $this->logger,
-            $this->deployPackageService,
-            [],
-            0
-        );
+        $package = $this->createMock(Package::class);
+        $package->expects($this->any())->method('getState')->willReturn(0);
+        $package->expects($this->exactly(2))->method('getParent')->willReturn(true);
+        $package->expects($this->any())->method('getArea')->willReturn('area');
+        $package->expects($this->any())->method('getPath')->willReturn('path');
+        $package->expects($this->any())->method('getFiles')->willReturn([]);
+        $this->logger->expects($this->exactly(2))->method('info')->willReturnSelf();
 
-        $this->package->expects($this->any())->method('getState')->willReturn(0);
-        $this->package->expects($this->exactly(2))->method('getParent')->willReturn(true);
-        $this->package->expects($this->any())->method('getArea')->willReturn('global');
-        $this->package->expects($this->any())->method('getPath')->willReturn('path');
-        $this->package->expects($this->any())->method('getFiles')->willReturn([]);
-        $this->package->expects($this->any())->method('getPreProcessors')->willReturn([]);
-        $this->package->expects($this->any())->method('getPostProcessors')->willReturn([]);
-        $this->logger->expects($this->exactly(3))->method('info')->willReturnSelf();
-        $queue->add($this->package, []);
-        $this->resourceConnection->expects(self::never())->method('closeConnection');
-        $this->assertEquals(0, $queue->process());
-    }
+        $this->appState->expects($this->once())->method('emulateAreaCode');
 
-    /**
-     * @see Queue::process()
-     * @dataProvider maxProcessesDataProvider
-     */
-    public function testProcessFailedPackagesToThrowAnException($maxProcesses)
-    {
-        $this->deployStaticFile
-            ->expects($this->any())
-            ->method('writeFile')
-            ->willThrowException(new \Exception);
+        $this->queue->add($package, []);
 
-        $queue = new Queue(
-            $this->appState,
-            $this->localeResolver,
-            $this->resourceConnection,
-            $this->logger,
-            $this->deployPackageService,
-            [],
-            $maxProcesses
-        );
+        $this->resourceConnection->expects(self::never())->method('closeConnection');
 
-        $this->package->expects($this->any())->method('getState')->willReturn(0);
-        $this->package->expects($this->any())->method('getParent')->willReturn(true);
-        $this->package->expects($this->any())->method('getArea')->willReturn('global');
-        $this->package->expects($this->any())->method('getPath')->willReturn('path');
-        $this->package->expects($this->any())->method('getFiles')->willReturn([$this->packageFile]);
-        $this->package->expects($this->any())->method('getPreProcessors')->willReturn([]);
-        $this->package->expects($this->any())->method('getPostProcessors')->willReturn([]);
-        $this->logger->expects($this->any())->method('info')->willReturnSelf();
-        $queue->add($this->package, []);
-        $this->expectException(\RuntimeException::class);
-        $queue->process();
+        $this->assertEquals(0, $this->queue->process());
     }
-
-    /**
-     * @return int[]
-     */
-    public function maxProcessesDataProvider(): array
-    {
-        return [
-            [0],
-            [1]
-        ];
-    }
-
 }
diff --git a/setup/src/Magento/Setup/Test/Unit/Console/Command/DeployStaticContentCommandTest.php b/setup/src/Magento/Setup/Test/Unit/Console/Command/DeployStaticContentCommandTest.php
@@ -11,6 +11,7 @@
 use Magento\Deploy\Console\ConsoleLoggerFactory;
 use Magento\Deploy\Console\DeployStaticOptions;
 use Magento\Deploy\Console\InputValidator;
+use Magento\Deploy\Process\TimeoutException;
 use Magento\Deploy\Service\DeployStaticContent;
 use Magento\Framework\App\State;
 use Magento\Framework\Console\Cli;
@@ -155,7 +156,7 @@ public function testExecuteWithError()
             ->willReturn($this->deployService);
         $this->deployService->expects($this->once())
             ->method('deploy')
-            ->willThrowException(new \RuntimeException());
+            ->willThrowException(new TimeoutException());
 
         $tester = new CommandTester($this->command);
         $exitCode = $tester->execute([]);
