MC-41912: 'magento-patches apply' command doesn't allow to apply some patches multiple times (#25)

Author: viktym

src/Command/Apply.php

@@ -88,7 +88,9 @@ public function execute(InputInterface $input, OutputInterface $output)
         try {
             $this->applyOptional->run($input, $output);
         } catch (RuntimeException $e) {
+            $output->writeln($this->magentoVersion->get());
             $output->writeln('<error>' . $e->getMessage() . '</error>');
+
             $this->logger->info($this->magentoVersion->get());
             $this->logger->error($e->getMessage());
 

src/Command/Ece/Apply.php

@@ -98,6 +98,7 @@ public function execute(InputInterface $input, OutputInterface $output)
             $this->applyOptional->run($input, $output);
             $this->applyLocal->run($input, $output);
         } catch (RuntimeException $e) {
+            $output->writeln($this->magentoVersion->get());
             $output->writeln('<error>' . $e->getMessage() . '</error>');
             $this->logger->info($this->magentoVersion->get());
             $this->logger->error($e->getMessage());

src/Command/Ece/Revert.php

@@ -78,6 +78,7 @@ public function execute(InputInterface $input, OutputInterface $output)
         try {
             $this->revert->run($input, $output);
         } catch (RuntimeException $e) {
+            $output->writeln($this->magentoVersion->get());
             $output->writeln('<error>' . $e->getMessage() . '</error>');
             $this->logger->info($this->magentoVersion->get());
             $this->logger->error($e->getMessage());

src/Command/Process/Action/ProcessDeprecatedAction.php

@@ -9,7 +9,6 @@
 
 use Magento\CloudPatches\App\RuntimeException;
 use Magento\CloudPatches\Command\Process\Renderer;
-use Magento\CloudPatches\Patch\Data\AggregatedPatch;
 use Magento\CloudPatches\Patch\Data\AggregatedPatchInterface;
 use Magento\CloudPatches\Patch\Pool\OptionalPool;
 use Magento\CloudPatches\Patch\Aggregator;
@@ -93,6 +92,13 @@ public function execute(InputInterface $input, OutputInterface $output, array $p
             throw new RuntimeException($e->getMessage(), $e->getCode(), $e);
         }
 
+        $patches = array_filter(
+            $patches,
+            function ($patch) {
+                return !$this->statusPool->isApplied($patch->getId());
+            }
+        );
+
         foreach ($patches as $patch) {
             $this->processDeprecation($patch, $output, $input);
             $this->processReplacement($patch, $output, $input);
@@ -144,21 +150,23 @@ function ($patchId) {
             }
         );
 
-        if ($requireRevertAndReplace) {
-            $ids = implode(' ', $requireRevertAndReplace);
-            $warning = sprintf('%s should be reverted and replaced with %s', $ids, $patch->getId());
-            $output->writeln('<info>' . $warning . '</info>');
-            $this->logger->warning($warning);
+        if (empty($requireRevertAndReplace)) {
+            return;
+        }
 
-            $question = 'Do you want to proceed with reverting?';
-            if (!$this->renderer->printQuestion($input, $output, $question)) {
-                $errorMessage = sprintf('%s can\'t be applied without reverting %s', $patch->getId(), $ids);
+        $ids = implode(' ', $requireRevertAndReplace);
+        $warning = sprintf('%s should be reverted and replaced with %s', $ids, $patch->getId());
+        $output->writeln('<info>' . $warning . '</info>');
+        $this->logger->warning($warning);
 
-                throw new RuntimeException($errorMessage);
-            }
+        $question = 'Do you want to proceed with reverting?';
+        if (!$this->renderer->printQuestion($input, $output, $question)) {
+            $errorMessage = sprintf('%s can\'t be applied without reverting %s', $patch->getId(), $ids);
 
-            $this->revert->execute($input, $output, $requireRevertAndReplace);
+            throw new RuntimeException($errorMessage);
         }
+
+        $this->revert->execute($input, $output, $requireRevertAndReplace);
     }
 
     /**

src/Command/Process/ApplyLocal.php

@@ -89,7 +89,7 @@ public function run(InputInterface $input, OutputInterface $output)
                 $this->printInfo($output, $message);
                 array_push($appliedPatches, $patch);
             } catch (ApplierException $exception) {
-                $this->printError($output, 'Error: patch can\'t be applied');
+                $this->printError($output, 'Error: patch ' . $patch->getPath() . ' can\'t be applied');
                 $messages = $this->rollbackProcessor->process($appliedPatches);
                 $output->writeln($messages);
                 $errorMessage = sprintf(

src/Command/Revert.php

@@ -101,6 +101,7 @@ public function execute(InputInterface $input, OutputInterface $output)
         try {
             $this->revert->run($input, $output);
         } catch (RuntimeException $e) {
+            $output->writeln($this->magentoVersion->get());
             $output->writeln('<error>' . $e->getMessage() . '</error>');
             $this->logger->info($this->magentoVersion->get());
             $this->logger->error($e->getMessage());

src/Patch/Conflict/Processor.php

@@ -73,7 +73,7 @@ public function process(
         array $appliedPatches,
         string $exceptionMessage
     ) {
-        $errorMessage = 'Error: patch can\'t be applied';
+        $errorMessage = 'Error: patch ' . $patch->getId() . ' can\'t be applied';
         $this->logger->error($errorMessage);
         $output->writeln('<error>' . $errorMessage . '</error>');
 

src/Test/Unit/Command/Process/Action/ProcessDeprecatedActionTest.php

@@ -208,6 +208,40 @@ public function testProcessReplacementSuccessful()
         $this->action->execute($inputMock, $outputMock, $patchFilter);
     }
 
+    /**
+     * Tests successful skipping of replacement check when patch is already applied.
+     */
+    public function testSkippingReplacementProcessForAppliedPatch()
+    {
+        $patch1 = $this->createPatch('MC-11111', false);
+        $this->statusPool->method('isApplied')
+            ->willReturnMap([
+                ['MC-11111', true]
+            ]);
+        $patchFilter = [$patch1->getId()];
+
+        /** @var InputInterface|MockObject $inputMock */
+        $inputMock = $this->getMockForAbstractClass(InputInterface::class);
+        /** @var OutputInterface|MockObject $outputMock */
+        $outputMock = $this->getMockForAbstractClass(OutputInterface::class);
+
+        $patchMock = $this->getMockForAbstractClass(PatchInterface::class);
+
+        $this->optionalPool->expects($this->once())
+            ->method('getList')
+            ->withConsecutive([$patchFilter])
+            ->willReturn([$patchMock]);
+
+        $this->aggregator->expects($this->once())
+            ->method('aggregate')
+            ->willReturn([$patch1]);
+
+        $this->renderer->expects($this->never())
+            ->method('printQuestion');
+
+        $this->action->execute($inputMock, $outputMock, $patchFilter);
+    }
+
     /**
      * Tests a case when user rejected to revert deprecated patches before applying a new one.
      */

src/Test/Unit/Patch/Conflict/ProcessorTest.php

@@ -92,7 +92,7 @@ public function testProcess()
         $outputMock->expects($this->exactly(2))
             ->method('writeln')
             ->withConsecutive(
-                [$this->stringContains('Error: patch can\'t be applied')],
+                [$this->stringContains('Error: patch ' . $failedPatch->getId() . ' can\'t be applied')],
                 [$rollbackMessages]
             );