Skip to content

Commit abc8e69

Browse files
devaruldevarul
committed
ACP2E-49: Incorrect Behavior from Data Patch Aliases
1 parent 65714d4 commit abc8e69

File tree

2 files changed

+32
-26
lines changed

2 files changed

+32
-26
lines changed

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ class PatchApplier
2222
/**
2323
* Flag means, that we need to read schema patches
2424
*/
25-
const SCHEMA_PATCH = 'schema';
25+
public const SCHEMA_PATCH = 'schema';
2626

2727
/**
2828
* Flag means, that we need to read data patches
2929
*/
30-
const DATA_PATCH = 'data';
30+
public const DATA_PATCH = 'data';
3131

3232
/**
3333
* @var PatchRegistryFactory
@@ -162,7 +162,9 @@ public function applyDataPatch($moduleName = null)
162162
$dataPatch->apply();
163163
$this->patchHistory->fixPatch(get_class($dataPatch));
164164
foreach ($dataPatch->getAliases() as $patchAlias) {
165-
$this->patchHistory->fixPatch($patchAlias);
165+
if (!$this->patchHistory->isApplied($patchAlias)) {
166+
$this->patchHistory->fixPatch($patchAlias);
167+
}
166168
}
167169
$this->moduleDataSetup->getConnection()->commit();
168170
} catch (\Exception $e) {
@@ -241,7 +243,9 @@ public function applySchemaPatch($moduleName = null)
241243
$schemaPatch->apply();
242244
$this->patchHistory->fixPatch(get_class($schemaPatch));
243245
foreach ($schemaPatch->getAliases() as $patchAlias) {
244-
$this->patchHistory->fixPatch($patchAlias);
246+
if (!$this->patchHistory->isApplied($patchAlias)) {
247+
$this->patchHistory->fixPatch($patchAlias);
248+
}
245249
}
246250
} catch (\Exception $e) {
247251
throw new SetupException(

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

Lines changed: 24 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,7 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
159159
]
160160
);
161161

162+
// phpstan:ignore
162163
$patches = [
163164
\SomeDataPatch::class,
164165
\OtherDataPatch::class
@@ -170,13 +171,16 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
170171
$this->patchRegistryFactoryMock->expects($this->any())
171172
->method('create')
172173
->willReturn($patchRegistryMock);
173-
174+
// phpstan:ignore "Class SomeDataPatch not found."
174175
$patch1 = $this->createMock(\SomeDataPatch::class);
175176
$patch1->expects($this->once())->method('apply');
176177
$patch1->expects($this->once())->method('getAliases')->willReturn([]);
178+
// phpstan:ignore "Class OtherDataPatch not found."
177179
$patch2 = $this->createMock(\OtherDataPatch::class);
178180
$patch2->expects($this->once())->method('apply');
179181
$patch2->expects($this->once())->method('getAliases')->willReturn([]);
182+
183+
// phpstan:ignore
180184
$this->objectManagerMock->expects($this->any())->method('create')->willReturnMap(
181185
[
182186
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
@@ -203,8 +207,6 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
203207
*/
204208
public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVersionInDb)
205209
{
206-
$this->expectException('Exception');
207-
$this->expectExceptionMessageMatches('"Unable to apply data patch .+ cannot be applied twice"');
208210
$this->dataPatchReaderMock->expects($this->once())
209211
->method('read')
210212
->with($moduleName)
@@ -233,15 +235,6 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer
233235
['\\' . $patchClass, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
234236
]
235237
);
236-
$this->connectionMock->expects($this->exactly(1))->method('beginTransaction');
237-
$this->connectionMock->expects($this->never())->method('commit');
238-
$this->patchHistoryMock->expects($this->any())->method('fixPatch')->willReturnCallback(
239-
function ($param1) {
240-
if ($param1 == 'PatchAlias') {
241-
throw new \LogicException(sprintf("Patch %s cannot be applied twice", $param1));
242-
}
243-
}
244-
);
245238
$this->patchApllier->applyDataPatch($moduleName);
246239
}
247240

@@ -250,6 +243,7 @@ function ($param1) {
250243
*/
251244
public function applyDataPatchDataNewModuleProvider()
252245
{
246+
// phpstan:ignore
253247
return [
254248
'newly installed module' => [
255249
'moduleName' => 'Module1',
@@ -282,6 +276,7 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches,
282276
]
283277
);
284278

279+
// phpstan:ignore
285280
$patches = [
286281
\SomeDataPatch::class,
287282
\OtherDataPatch::class
@@ -298,12 +293,16 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches,
298293
->method('create')
299294
->willReturn($patchRegistryMock);
300295

296+
// phpstan:ignore "Class SomeDataPatch not found."
301297
$patch1 = $this->createMock(\SomeDataPatch::class);
302298
$patch1->expects(self::never())->method('apply');
303299
$patch1->expects(self::any())->method('getAliases')->willReturn([]);
300+
// phpstan:ignore "Class OtherDataPatch not found."
304301
$patch2 = $this->createMock(\OtherDataPatch::class);
305302
$patch2->expects(self::once())->method('apply');
306303
$patch2->expects(self::any())->method('getAliases')->willReturn([]);
304+
305+
// phpstan:ignore
307306
$this->objectManagerMock->expects(self::any())->method('create')->willReturnMap(
308307
[
309308
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
@@ -321,6 +320,7 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches,
321320
*/
322321
public function applyDataPatchDataInstalledModuleProvider()
323322
{
323+
// phpstan:ignore
324324
return [
325325
'upgrade module iwth only OtherDataPatch' => [
326326
'moduleName' => 'Module1',
@@ -356,6 +356,7 @@ public function testApplyDataPatchRollback($moduleName, $dataPatches, $moduleVer
356356
]
357357
);
358358

359+
// phpstan:ignore
359360
$patches = [
360361
\SomeDataPatch::class,
361362
\OtherDataPatch::class
@@ -368,14 +369,17 @@ public function testApplyDataPatchRollback($moduleName, $dataPatches, $moduleVer
368369
->method('create')
369370
->willReturn($patchRegistryMock);
370371

372+
// phpstan:ignore "Class SomeDataPatch not found."
371373
$patch1 = $this->createMock(\SomeDataPatch::class);
372374
$patch1->expects($this->never())->method('apply');
375+
// phpstan:ignore "Class OtherDataPatch not found."
373376
$patch2 = $this->createMock(\OtherDataPatch::class);
374377
$exception = new \Exception('Patch Apply Error');
375378
$patch2->expects($this->once())->method('apply')->willThrowException($exception);
379+
// phpstan:ignore
376380
$this->objectManagerMock->expects($this->any())->method('create')->willReturnMap(
377381
[
378-
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
382+
['\\' . \SomeDataPatch::class , ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
379383
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],
380384
]
381385
);
@@ -421,6 +425,7 @@ public function testNonDataPatchApply()
421425

422426
public function testNonTransactionablePatch()
423427
{
428+
// phpstan:ignore "Class NonTransactionableDataPatch not found."
424429
$patches = [\NonTransactionableDataPatch::class];
425430
$this->dataPatchReaderMock->expects($this->once())
426431
->method('read')
@@ -477,6 +482,7 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
477482
]
478483
);
479484

485+
// phpstan:ignore
480486
$patches = [
481487
\SomeSchemaPatch::class,
482488
\OtherSchemaPatch::class
@@ -489,12 +495,15 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
489495
->method('create')
490496
->willReturn($patchRegistryMock);
491497

498+
// phpstan:ignore "Class SomeSchemaPatch not found."
492499
$patch1 = $this->createMock(\SomeSchemaPatch::class);
493500
$patch1->expects($this->never())->method('apply');
494501
$patch1->expects($this->any())->method('getAliases')->willReturn([]);
502+
// phpstan:ignore "Class OtherSchemaPatch not found."
495503
$patch2 = $this->createMock(\OtherSchemaPatch::class);
496504
$patch2->expects($this->once())->method('apply');
497505
$patch2->expects($this->any())->method('getAliases')->willReturn([]);
506+
// phpstan:ignore
498507
$this->patchFactoryMock->expects($this->any())->method('create')->willReturnMap(
499508
[
500509
[\SomeSchemaPatch::class, ['schemaSetup' => $this->schemaSetupMock], $patch1],
@@ -516,8 +525,6 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
516525
*/
517526
public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $moduleVersionInDb)
518527
{
519-
$this->expectException('Exception');
520-
$this->expectExceptionMessageMatches('"Unable to apply patch .+ cannot be applied twice"');
521528
$this->schemaPatchReaderMock->expects($this->once())
522529
->method('read')
523530
->with($moduleName)
@@ -542,19 +549,13 @@ public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $
542549
->willReturn($patchRegistryMock);
543550

544551
$this->patchFactoryMock->expects($this->any())->method('create')->willReturn($patch1);
545-
$this->patchHistoryMock->expects($this->any())->method('fixPatch')->willReturnCallback(
546-
function ($param1) {
547-
if ($param1 == 'PatchAlias') {
548-
throw new \LogicException(sprintf("Patch %s cannot be applied twice", $param1));
549-
}
550-
}
551-
);
552552

553553
$this->patchApllier->applySchemaPatch($moduleName);
554554
}
555555

556556
public function testRevertDataPatches()
557557
{
558+
// phpstan:ignore "Class RevertableDataPatch not found."
558559
$patches = [\RevertableDataPatch::class];
559560
$this->dataPatchReaderMock->expects($this->once())
560561
->method('read')
@@ -598,6 +599,7 @@ public function testRevertDataPatches()
598599
*/
599600
public function schemaPatchDataProvider()
600601
{
602+
// phpstan:ignore
601603
return [
602604
'upgrade module iwth only OtherSchemaPatch' => [
603605
'moduleName' => 'Module1',

0 commit comments

Comments
 (0)