[Fixed: #31396] Data/Schema Patches getAliases() not working as expected

lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php

@@ -159,13 +159,11 @@ public function applyDataPatch($moduleName = null)
             } else {
                 try {
                     $this->moduleDataSetup->getConnection()->beginTransaction();
-                    $dataPatch->apply();
-                    $this->patchHistory->fixPatch(get_class($dataPatch));
-                    foreach ($dataPatch->getAliases() as $patchAlias) {
-                        if (!$this->patchHistory->isApplied($patchAlias)) {
-                            $this->patchHistory->fixPatch($patchAlias);
-                        }
+                    if ($this->shouldApply($dataPatch)) {
+                        $dataPatch->apply();
+                        $this->patchHistory->fixPatch(get_class($dataPatch));
                     }
+                    $this->fixAliases($dataPatch);
                     $this->moduleDataSetup->getConnection()->commit();
                 } catch (\Exception $e) {
                     $this->moduleDataSetup->getConnection()->rollBack();
@@ -240,13 +238,11 @@ public function applySchemaPatch($moduleName = null)
                  * @var SchemaPatchInterface $schemaPatch
                  */
                 $schemaPatch = $this->patchFactory->create($schemaPatch, ['schemaSetup' => $this->schemaSetup]);
-                $schemaPatch->apply();
-                $this->patchHistory->fixPatch(get_class($schemaPatch));
-                foreach ($schemaPatch->getAliases() as $patchAlias) {
-                    if (!$this->patchHistory->isApplied($patchAlias)) {
-                        $this->patchHistory->fixPatch($patchAlias);
-                    }
+                if ($this->shouldApply($schemaPatch)) {
+                    $schemaPatch->apply();
+                    $this->patchHistory->fixPatch(get_class($schemaPatch));
                 }
+                $this->fixAliases($schemaPatch);
             } catch (\Exception $e) {
                 throw new SetupException(
                     new Phrase(
@@ -296,4 +292,35 @@ public function revertDataPatches($moduleName = null)
             }
         }
     }
+
+    /**
+     * Check if patch should be applied by already applied patch alias names
+     *
+     * @param PatchInterface $patch
+     * @return bool
+     */
+    private function shouldApply(PatchInterface $patch): bool
+    {
+        foreach ($patch->getAliases() as $patchAlias) {
+            if ($this->patchHistory->isApplied($patchAlias)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Save all patch aliases
+     *
+     * @param PatchInterface $patch
+     * @return void
+     */
+    private function fixAliases(PatchInterface $patch): void
+    {
+        foreach ($patch->getAliases() as $patchAlias) {
+            if (!$this->patchHistory->isApplied($patchAlias)) {
+                $this->patchHistory->fixPatch($patchAlias);
+            }
+        }
+    }
 }

lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php

@@ -175,11 +175,11 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
         // phpstan:ignore "Class SomeDataPatch not found."
         $patch1 = $this->createMock(\SomeDataPatch::class);
         $patch1->expects($this->once())->method('apply');
-        $patch1->expects($this->once())->method('getAliases')->willReturn([]);
+        $patch1->expects($this->exactly(2))->method('getAliases')->willReturn([]);
         // phpstan:ignore "Class OtherDataPatch not found."
         $patch2 = $this->createMock(\OtherDataPatch::class);
         $patch2->expects($this->once())->method('apply');
-        $patch2->expects($this->once())->method('getAliases')->willReturn([]);
+        $patch2->expects($this->exactly(2))->method('getAliases')->willReturn([]);
 
         $this->objectManagerMock->expects($this->any())->method('create')->willReturnMap(
             [
@@ -219,9 +219,14 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer
                 [$moduleName, $moduleVersionInDb]
             ]
         );
-
+        $this->patchHistoryMock->method('isApplied')->willReturnCallback(
+            function ($patchAlias) {
+                return in_array($patchAlias, ['PatchAlias']);
+            }
+        );
         $patch1 = $this->getMockForAbstractClass(DataPatchInterface::class);
-        $patch1->expects($this->once())->method('getAliases')->willReturn(['PatchAlias']);
+        $patch1->expects($this->exactly(2))->method('getAliases')->willReturn(['PatchAlias']);
+        $patch1->expects($this->never())->method('apply');
         $patchClass = get_class($patch1);
 
         $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, [$patchClass], ['registerPatch']);
@@ -237,6 +242,7 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer
                 ['\\' . $patchClass, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
             ]
         );
+
         $this->patchApllier->applyDataPatch($moduleName);
     }
 
@@ -381,6 +387,7 @@ public function testApplyDataPatchRollback($moduleName, $dataPatches, $moduleVer
         $patch1->expects($this->never())->method('apply');
         // phpstan:ignore "Class OtherDataPatch not found."
         $patch2 = $this->createMock(\OtherDataPatch::class);
+        $patch2->expects($this->exactly(1))->method('getAliases')->willReturn([]);
         $exception = new \Exception('Patch Apply Error');
         $patch2->expects($this->once())->method('apply')->willThrowException($exception);
 
@@ -548,7 +555,7 @@ public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $
         );
 
         $patch1 = $this->getMockForAbstractClass(PatchInterface::class);
-        $patch1->expects($this->once())->method('getAliases')->willReturn(['PatchAlias']);
+        $patch1->expects($this->exactly(2))->method('getAliases')->willReturn(['PatchAlias']);
         $patchClass = get_class($patch1);
 
         $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, [$patchClass], ['registerPatch']);