From 23648e45bd60db8e0fbac69cbf29b690501673e4 Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Fri, 1 Dec 2023 15:09:04 +0100 Subject: [PATCH 01/10] Fix issue #31396 --- .../Framework/Setup/Patch/PatchApplier.php | 112 +++++++++++------- 1 file changed, 67 insertions(+), 45 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index 6a4c256241aa6..e61acdbdd7ad9 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -153,19 +153,15 @@ public function applyDataPatch($moduleName = null) new Phrase("Patch %1 should implement DataPatchInterface", [get_class($dataPatch)]) ); } + if ($this->isApplied($dataPatch)) { + continue; + } if ($dataPatch instanceof NonTransactionableInterface) { - $dataPatch->apply(); - $this->patchHistory->fixPatch(get_class($dataPatch)); + $this->applyPatch($dataPatch); } 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); - } - } + $this->applyPatch($dataPatch); $this->moduleDataSetup->getConnection()->commit(); } catch (\Exception $e) { $this->moduleDataSetup->getConnection()->rollBack(); @@ -187,35 +183,6 @@ public function applyDataPatch($moduleName = null) } } - /** - * Register all patches in registry in order to manipulate chains and dependencies of patches of patches - * - * @param string $moduleName - * @param string $patchType - * @return PatchRegistry - */ - private function prepareRegistry($moduleName, $patchType) - { - $reader = $patchType === self::DATA_PATCH ? $this->dataPatchReader : $this->schemaPatchReader; - $registry = $this->patchRegistryFactory->create(); - - //Prepare modules to read - if ($moduleName === null) { - $patchNames = []; - foreach ($this->moduleList->getNames() as $moduleName) { - $patchNames += $reader->read($moduleName); - } - } else { - $patchNames = $reader->read($moduleName); - } - - foreach ($patchNames as $patchName) { - $registry->registerPatch($patchName); - } - - return $registry; - } - /** * Apply all patches for one module * @@ -240,12 +207,8 @@ 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->isApplied($schemaPatch)) { + $this->applyPatch($schemaPatch); } } catch (\Exception $e) { $schemaPatchClass = is_object($schemaPatch) ? get_class($schemaPatch) : $schemaPatch; @@ -281,7 +244,7 @@ public function revertDataPatches($moduleName = null) '\\' . $dataPatch, ['moduleDataSetup' => $this->moduleDataSetup] ); - if ($dataPatch instanceof PatchRevertableInterface) { + if ($dataPatch instanceof PatchRevertableInterface && $this->isApplied($dataPatch)) { try { $adapter->beginTransaction(); /** @var PatchRevertableInterface|DataPatchInterface $dataPatch */ @@ -297,4 +260,63 @@ public function revertDataPatches($moduleName = null) } } } + + /** + * Register all patches in registry in order to manipulate chains and dependencies of patches of patches + * + * @param string $moduleName + * @param string $patchType + * @return PatchRegistry + */ + private function prepareRegistry($moduleName, $patchType) + { + $reader = $patchType === self::DATA_PATCH ? $this->dataPatchReader : $this->schemaPatchReader; + $registry = $this->patchRegistryFactory->create(); + + //Prepare modules to read + if ($moduleName === null) { + $patchNames = []; + foreach ($this->moduleList->getNames() as $moduleName) { + $patchNames += $reader->read($moduleName); + } + } else { + $patchNames = $reader->read($moduleName); + } + + foreach ($patchNames as $patchName) { + $registry->registerPatch($patchName); + } + + return $registry; + } + + /** + * Apply the given patch. The patch is and its aliases are added to the history. + */ + private function applyPatch((PatchInterface $patch): void + { + $patch->apply(); + $this->patchHistory->fixPatch(get_class($schemaPatch)); + foreach ($schemaPatch->getAliases() as $patchAlias) { + if (!$this->patchHistory->isApplied($patchAlias)) { + $this->patchHistory->fixPatch($patchAlias); + } + } + } + + /** + * Check wether the given patch or any of its aliases are already applied or not. + */ + private function isApplied(PatchInterface $patch): bool + { + if (!$this->patchHistory->isApplied(get_class($patch))) { + foreach ($patch->getAliases() as $alias) { + if ($this->patchHistory->isApplied($alias)) { + return true; + } + } + } + + return false; + } } From f7f812312628918bfd17f58ae951e2d4db6c0cff Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Fri, 1 Dec 2023 15:15:46 +0100 Subject: [PATCH 02/10] Add patch to history if alias exists --- lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index e61acdbdd7ad9..fdbafd441f233 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -3,6 +3,7 @@ * Copyright © Magento, Inc. All rights reserved. * See COPYING.txt for license details. */ +declare(strict_types=1); namespace Magento\Framework\Setup\Patch; @@ -309,9 +310,11 @@ private function applyPatch((PatchInterface $patch): void */ private function isApplied(PatchInterface $patch): bool { - if (!$this->patchHistory->isApplied(get_class($patch))) { + $patchIdentity = get_class($patch); + if (!$this->patchHistory->isApplied($patchIdentity)) { foreach ($patch->getAliases() as $alias) { if ($this->patchHistory->isApplied($alias)) { + $this->patchHistory->fixPatch($patchIdentity); return true; } } From d48abff1b6e45423ba203444e5c139a46bcc88f3 Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Fri, 1 Dec 2023 15:45:12 +0100 Subject: [PATCH 03/10] Fix syntax error --- lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index fdbafd441f233..41b3d974cc01f 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -294,7 +294,7 @@ private function prepareRegistry($moduleName, $patchType) /** * Apply the given patch. The patch is and its aliases are added to the history. */ - private function applyPatch((PatchInterface $patch): void + private function applyPatch(PatchInterface $patch): void { $patch->apply(); $this->patchHistory->fixPatch(get_class($schemaPatch)); From c51a459cdda69f32e9b02dff8ee3aa5fb367ed5b Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Fri, 1 Dec 2023 20:43:25 +0100 Subject: [PATCH 04/10] prevent null cases & fix typo --- lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index 41b3d974cc01f..ae5062a20acfc 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -297,8 +297,8 @@ private function prepareRegistry($moduleName, $patchType) private function applyPatch(PatchInterface $patch): void { $patch->apply(); - $this->patchHistory->fixPatch(get_class($schemaPatch)); - foreach ($schemaPatch->getAliases() as $patchAlias) { + $this->patchHistory->fixPatch(get_class($patch)); + foreach ($patch->getAliases() ?? [] as $patchAlias) { if (!$this->patchHistory->isApplied($patchAlias)) { $this->patchHistory->fixPatch($patchAlias); } @@ -312,7 +312,7 @@ private function isApplied(PatchInterface $patch): bool { $patchIdentity = get_class($patch); if (!$this->patchHistory->isApplied($patchIdentity)) { - foreach ($patch->getAliases() as $alias) { + foreach ($patch->getAliases() ?? [] as $alias) { if ($this->patchHistory->isApplied($alias)) { $this->patchHistory->fixPatch($patchIdentity); return true; From 3ba438804a2db84e22115d0d3b14b559228b9d0c Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Wed, 6 Dec 2023 14:56:44 +0100 Subject: [PATCH 05/10] Revert already check for applied patches --- .../Magento/Framework/Setup/Patch/PatchApplier.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index ae5062a20acfc..cd8cdd2bc4593 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -245,7 +245,7 @@ public function revertDataPatches($moduleName = null) '\\' . $dataPatch, ['moduleDataSetup' => $this->moduleDataSetup] ); - if ($dataPatch instanceof PatchRevertableInterface && $this->isApplied($dataPatch)) { + if ($dataPatch instanceof PatchRevertableInterface) { try { $adapter->beginTransaction(); /** @var PatchRevertableInterface|DataPatchInterface $dataPatch */ @@ -269,7 +269,7 @@ public function revertDataPatches($moduleName = null) * @param string $patchType * @return PatchRegistry */ - private function prepareRegistry($moduleName, $patchType) + private function prepareRegistry(string $moduleName, string $patchType): PatchRegistry { $reader = $patchType === self::DATA_PATCH ? $this->dataPatchReader : $this->schemaPatchReader; $registry = $this->patchRegistryFactory->create(); @@ -293,6 +293,8 @@ private function prepareRegistry($moduleName, $patchType) /** * Apply the given patch. The patch is and its aliases are added to the history. + * + * @param PatchInterface $patch */ private function applyPatch(PatchInterface $patch): void { @@ -307,6 +309,8 @@ private function applyPatch(PatchInterface $patch): void /** * Check wether the given patch or any of its aliases are already applied or not. + * + * @param PatchInterface $patch */ private function isApplied(PatchInterface $patch): bool { From cd91e89223c038aace3d437d8e3c4c3e160ce48e Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Wed, 6 Dec 2023 21:52:29 +0100 Subject: [PATCH 06/10] Fix unit tests --- .../Setup/Test/Unit/Patch/PatchApplierTest.php | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php index 8c2e237918577..80c88411ca614 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -166,20 +166,17 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc \OtherDataPatch::class ]; $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, $patches, ['registerPatch']); - $patchRegistryMock->expects($this->exactly(2)) - ->method('registerPatch'); + $patchRegistryMock->expects($this->exactly(2))->method('registerPatch'); - $this->patchRegistryFactoryMock->expects($this->any()) - ->method('create') - ->willReturn($patchRegistryMock); + $this->patchRegistryFactoryMock->expects($this->any())->method('create')->willReturn($patchRegistryMock); // 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->any())->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->any())->method('getAliases')->willReturn([]); $this->objectManagerMock->expects($this->any())->method('create')->willReturnMap( [ From 0095402e88cccf2e476db7bce352b2694e2c1d25 Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Wed, 6 Dec 2023 21:54:23 +0100 Subject: [PATCH 07/10] Fix typo --- .../Framework/Setup/Test/Unit/Patch/PatchApplierTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php index 80c88411ca614..9e9e72408e58c 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -323,7 +323,7 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches, public function applyDataPatchDataInstalledModuleProvider() { return [ - 'upgrade module iwth only OtherDataPatch' => [ + 'upgrade module with only OtherDataPatch' => [ 'moduleName' => 'Module1', 'dataPatches' => [ // phpstan:ignore @@ -608,7 +608,7 @@ public function testRevertDataPatches() public function schemaPatchDataProvider() { return [ - 'upgrade module iwth only OtherSchemaPatch' => [ + 'upgrade module with only OtherSchemaPatch' => [ 'moduleName' => 'Module1', 'schemaPatches' => [ // phpstan:ignore From cad219a4f25234a4231165fd10c005d4182c933f Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Thu, 7 Dec 2023 00:21:59 +0100 Subject: [PATCH 08/10] Fix unit tests --- .../Framework/Setup/Test/Unit/Patch/PatchApplierTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php index 9e9e72408e58c..20a4b82487ac7 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -218,7 +218,7 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer ); $patch1 = $this->getMockForAbstractClass(DataPatchInterface::class); - $patch1->expects($this->once())->method('getAliases')->willReturn(['PatchAlias']); + $patch1->expects($this->any())->method('getAliases')->willReturn(['PatchAlias']); $patchClass = get_class($patch1); $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, [$patchClass], ['registerPatch']); @@ -545,7 +545,7 @@ public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $ ); $patch1 = $this->getMockForAbstractClass(PatchInterface::class); - $patch1->expects($this->once())->method('getAliases')->willReturn(['PatchAlias']); + $patch1->expects($this->any())->method('getAliases')->willReturn(['PatchAlias']); $patchClass = get_class($patch1); $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, [$patchClass], ['registerPatch']); From f971bf839db708884c6802237017e9707d4141e5 Mon Sep 17 00:00:00 2001 From: Thomas Klein Date: Tue, 1 Jul 2025 22:16:29 +0200 Subject: [PATCH 09/10] Update copyright --- lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index cd8cdd2bc4593..344be99981761 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -1,7 +1,7 @@ Date: Tue, 1 Jul 2025 22:17:27 +0200 Subject: [PATCH 10/10] Update copyright --- .../Framework/Setup/Test/Unit/Patch/PatchApplierTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php index 5080d1af7bcf4..8e69a710bee81 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -1,7 +1,7 @@