From f8bed9b1a47c6dd2fdb286fe07ed8178fb130fda Mon Sep 17 00:00:00 2001 From: Oleh Usik Date: Wed, 13 Jan 2021 14:52:00 +0200 Subject: [PATCH 1/4] Fixed issue with Data/Schema patch applying --- .../Framework/Setup/Patch/PatchApplier.php | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index e1b0e2842628d..36a7198f83173 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -159,10 +159,9 @@ public function applyDataPatch($moduleName = null) } else { try { $this->moduleDataSetup->getConnection()->beginTransaction(); - $dataPatch->apply(); - $this->patchHistory->fixPatch(get_class($dataPatch)); - foreach ($dataPatch->getAliases() as $patchAlias) { - $this->patchHistory->fixPatch($patchAlias); + if (!$this->checkPatchAliases($dataPatch)) { + $dataPatch->apply(); + $this->patchHistory->fixPatch(get_class($dataPatch)); } $this->moduleDataSetup->getConnection()->commit(); } catch (\Exception $e) { @@ -238,10 +237,9 @@ 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) { - $this->patchHistory->fixPatch($patchAlias); + if (!$this->checkPatchAliases($schemaPatch)) { + $schemaPatch->apply(); + $this->patchHistory->fixPatch(get_class($schemaPatch)); } } catch (\Exception $e) { throw new SetupException( @@ -292,4 +290,18 @@ public function revertDataPatches($moduleName = null) } } } + + /** + * Checks is patch was applied with current alias name + * + * @param DataPatchInterface|SchemaPatchInterface $dataPatch + * @return bool + */ + private function checkPatchAliases($dataPatch): bool + { + foreach ($dataPatch->getAliases() as $patchAlias) { + return $this->patchHistory->isApplied($patchAlias); + } + return false; + } } From 015ac916445c98e1b0153fb116e91e80c773c9a9 Mon Sep 17 00:00:00 2001 From: Oleh Usik Date: Wed, 13 Jan 2021 15:15:09 +0200 Subject: [PATCH 2/4] minor fix --- 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 36a7198f83173..26c6f84e07bab 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -300,7 +300,7 @@ public function revertDataPatches($moduleName = null) private function checkPatchAliases($dataPatch): bool { foreach ($dataPatch->getAliases() as $patchAlias) { - return $this->patchHistory->isApplied($patchAlias); + return $this->patchHistory->isApplied($patchAlias); } return false; } From d88bd43265422f11aa339a4f635c213a3d8fbe75 Mon Sep 17 00:00:00 2001 From: Oleh Usik Date: Mon, 15 Feb 2021 14:40:41 +0200 Subject: [PATCH 3/4] Fixed failed tests --- lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php | 6 ++++-- .../Framework/Setup/Test/Unit/Patch/PatchApplierTest.php | 4 ---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index 26c6f84e07bab..069fac89ceea2 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -299,8 +299,10 @@ public function revertDataPatches($moduleName = null) */ private function checkPatchAliases($dataPatch): bool { - foreach ($dataPatch->getAliases() as $patchAlias) { - return $this->patchHistory->isApplied($patchAlias); + if ($dataPatchAliases = $dataPatch->getAliases()) { + foreach ($dataPatchAliases as $patchAlias) { + return $this->patchHistory->isApplied($patchAlias); + } } return false; } 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 82c267dc7d51f..7674a1c667268 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -203,8 +203,6 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc */ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVersionInDb) { - $this->expectException('Exception'); - $this->expectExceptionMessageMatches('"Unable to apply data patch .+ cannot be applied twice"'); $this->dataPatchReaderMock->expects($this->once()) ->method('read') ->with($moduleName) @@ -516,8 +514,6 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion */ public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $moduleVersionInDb) { - $this->expectException('Exception'); - $this->expectExceptionMessageMatches('"Unable to apply patch .+ cannot be applied twice"'); $this->schemaPatchReaderMock->expects($this->once()) ->method('read') ->with($moduleName) From 13e4a2d362f4313ee0468080ebfc952a9152469e Mon Sep 17 00:00:00 2001 From: Oleh Usik Date: Tue, 16 Feb 2021 13:15:50 +0200 Subject: [PATCH 4/4] Fixed failed tests --- .../Magento/Framework/Setup/Patch/PatchApplier.php | 2 +- .../Setup/Test/Unit/Patch/PatchApplierTest.php | 11 ++--------- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php index 069fac89ceea2..cac6783b3bbbc 100644 --- a/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php +++ b/lib/internal/Magento/Framework/Setup/Patch/PatchApplier.php @@ -301,7 +301,7 @@ private function checkPatchAliases($dataPatch): bool { if ($dataPatchAliases = $dataPatch->getAliases()) { foreach ($dataPatchAliases as $patchAlias) { - return $this->patchHistory->isApplied($patchAlias); + return $this->patchHistory->isApplied($patchAlias) ?? true; } } return false; 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 7674a1c667268..98fb52ceabf76 100644 --- a/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php +++ b/lib/internal/Magento/Framework/Setup/Test/Unit/Patch/PatchApplierTest.php @@ -24,6 +24,7 @@ use Magento\Framework\Setup\SchemaSetupInterface; use Magento\Framework\Setup\SetupInterface; use Magento\Framework\TestFramework\Unit\Helper\ObjectManager; +use phpDocumentor\Reflection\Types\True_; use PHPUnit\Framework\MockObject\MockObject; use PHPUnit\Framework\TestCase; @@ -216,6 +217,7 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer $patch1 = $this->getMockForAbstractClass(DataPatchInterface::class); $patch1->expects($this->once())->method('getAliases')->willReturn(['PatchAlias']); + $patch1->expects($this->never())->method('apply'); $patchClass = get_class($patch1); $patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, [$patchClass], ['registerPatch']); @@ -231,15 +233,6 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer ['\\' . $patchClass, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1], ] ); - $this->connectionMock->expects($this->exactly(1))->method('beginTransaction'); - $this->connectionMock->expects($this->never())->method('commit'); - $this->patchHistoryMock->expects($this->any())->method('fixPatch')->willReturnCallback( - function ($param1) { - if ($param1 == 'PatchAlias') { - throw new \LogicException(sprintf("Patch %s cannot be applied twice", $param1)); - } - } - ); $this->patchApllier->applyDataPatch($moduleName); }