Skip to content

Commit 5bcdc75

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

File tree

2 files changed

+42
-47
lines changed

2 files changed

+42
-47
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: 34 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,8 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
160160
);
161161

162162
$patches = [
163-
\SomeDataPatch::class,
164-
\OtherDataPatch::class
163+
\SomeDataPatch::class,// phpstan:ignore "Class SomeDataPatch not found."
164+
\OtherDataPatch::class// phpstan:ignore "Class OtherDataPatch not found."
165165
];
166166
$patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, $patches, ['registerPatch']);
167167
$patchRegistryMock->expects($this->exactly(2))
@@ -170,17 +170,19 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
170170
$this->patchRegistryFactoryMock->expects($this->any())
171171
->method('create')
172172
->willReturn($patchRegistryMock);
173-
173+
// phpstan:ignore "Class SomeDataPatch not found."
174174
$patch1 = $this->createMock(\SomeDataPatch::class);
175175
$patch1->expects($this->once())->method('apply');
176176
$patch1->expects($this->once())->method('getAliases')->willReturn([]);
177+
// phpstan:ignore "Class OtherDataPatch not found."
177178
$patch2 = $this->createMock(\OtherDataPatch::class);
178179
$patch2->expects($this->once())->method('apply');
179180
$patch2->expects($this->once())->method('getAliases')->willReturn([]);
181+
180182
$this->objectManagerMock->expects($this->any())->method('create')->willReturnMap(
181183
[
182-
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
183-
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],
184+
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],// phpstan:ignore "Class SomeDataPatch not found."
185+
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],// phpstan:ignore "Class OtherDataPatch not found."
184186
]
185187
);
186188
$this->connectionMock->expects($this->exactly(2))->method('beginTransaction');
@@ -203,8 +205,6 @@ public function testApplyDataPatchForNewlyInstalledModule($moduleName, $dataPatc
203205
*/
204206
public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVersionInDb)
205207
{
206-
$this->expectException('Exception');
207-
$this->expectExceptionMessageMatches('"Unable to apply data patch .+ cannot be applied twice"');
208208
$this->dataPatchReaderMock->expects($this->once())
209209
->method('read')
210210
->with($moduleName)
@@ -233,15 +233,6 @@ public function testApplyDataPatchForAlias($moduleName, $dataPatches, $moduleVer
233233
['\\' . $patchClass, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
234234
]
235235
);
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-
);
245236
$this->patchApllier->applyDataPatch($moduleName);
246237
}
247238

@@ -254,8 +245,8 @@ public function applyDataPatchDataNewModuleProvider()
254245
'newly installed module' => [
255246
'moduleName' => 'Module1',
256247
'dataPatches' => [
257-
\SomeDataPatch::class,
258-
\OtherDataPatch::class
248+
\SomeDataPatch::class,// phpstan:ignore "Class SomeDataPatch not found."
249+
\OtherDataPatch::class// phpstan:ignore "Class OtherDataPatch not found."
259250
],
260251
'moduleVersionInDb' => null,
261252
],
@@ -283,8 +274,8 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches,
283274
);
284275

285276
$patches = [
286-
\SomeDataPatch::class,
287-
\OtherDataPatch::class
277+
\SomeDataPatch::class,// phpstan:ignore "Class SomeDataPatch not found."
278+
\OtherDataPatch::class// phpstan:ignore "Class OtherDataPatch not found."
288279
];
289280
$patchRegistryMock = $this->createAggregateIteratorMock(
290281
PatchRegistry::class,
@@ -298,16 +289,19 @@ public function testApplyDataPatchForInstalledModule($moduleName, $dataPatches,
298289
->method('create')
299290
->willReturn($patchRegistryMock);
300291

292+
// phpstan:ignore "Class SomeDataPatch not found."
301293
$patch1 = $this->createMock(\SomeDataPatch::class);
302294
$patch1->expects(self::never())->method('apply');
303295
$patch1->expects(self::any())->method('getAliases')->willReturn([]);
296+
// phpstan:ignore "Class OtherDataPatch not found."
304297
$patch2 = $this->createMock(\OtherDataPatch::class);
305298
$patch2->expects(self::once())->method('apply');
306299
$patch2->expects(self::any())->method('getAliases')->willReturn([]);
300+
307301
$this->objectManagerMock->expects(self::any())->method('create')->willReturnMap(
308302
[
309-
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
310-
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],
303+
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],// phpstan:ignore "Class SomeDataPatch not found."
304+
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],// phpstan:ignore "Class OtherDataPatch not found."
311305
]
312306
);
313307
$this->connectionMock->expects(self::exactly(1))->method('beginTransaction');
@@ -325,8 +319,8 @@ public function applyDataPatchDataInstalledModuleProvider()
325319
'upgrade module iwth only OtherDataPatch' => [
326320
'moduleName' => 'Module1',
327321
'dataPatches' => [
328-
\SomeDataPatch::class,
329-
\OtherDataPatch::class
322+
\SomeDataPatch::class,// phpstan:ignore "Class SomeDataPatch not found."
323+
\OtherDataPatch::class// phpstan:ignore "Class OtherDataPatch not found."
330324
],
331325
'moduleVersionInDb' => '2.0.0',
332326
]
@@ -357,8 +351,8 @@ public function testApplyDataPatchRollback($moduleName, $dataPatches, $moduleVer
357351
);
358352

359353
$patches = [
360-
\SomeDataPatch::class,
361-
\OtherDataPatch::class
354+
\SomeDataPatch::class,// phpstan:ignore "Class SomeDataPatch not found."
355+
\OtherDataPatch::class// phpstan:ignore "Class OtherDataPatch not found."
362356
];
363357
$patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, $patches, ['registerPatch']);
364358
$patchRegistryMock->expects($this->exactly(2))
@@ -368,15 +362,17 @@ public function testApplyDataPatchRollback($moduleName, $dataPatches, $moduleVer
368362
->method('create')
369363
->willReturn($patchRegistryMock);
370364

365+
// phpstan:ignore "Class SomeDataPatch not found."
371366
$patch1 = $this->createMock(\SomeDataPatch::class);
372367
$patch1->expects($this->never())->method('apply');
368+
// phpstan:ignore "Class OtherDataPatch not found."
373369
$patch2 = $this->createMock(\OtherDataPatch::class);
374370
$exception = new \Exception('Patch Apply Error');
375371
$patch2->expects($this->once())->method('apply')->willThrowException($exception);
376372
$this->objectManagerMock->expects($this->any())->method('create')->willReturnMap(
377373
[
378-
['\\' . \SomeDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],
379-
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],
374+
['\\' . \SomeDataPatch::class , ['moduleDataSetup' => $this->moduleDataSetupMock], $patch1],// phpstan:ignore "Class SomeDataPatch not found."
375+
['\\' . \OtherDataPatch::class, ['moduleDataSetup' => $this->moduleDataSetupMock], $patch2],// phpstan:ignore "Class OtherDataPatch not found."
380376
]
381377
);
382378
$this->connectionMock->expects($this->exactly(1))->method('beginTransaction');
@@ -421,6 +417,7 @@ public function testNonDataPatchApply()
421417

422418
public function testNonTransactionablePatch()
423419
{
420+
// phpstan:ignore "Class NonTransactionableDataPatch not found."
424421
$patches = [\NonTransactionableDataPatch::class];
425422
$this->dataPatchReaderMock->expects($this->once())
426423
->method('read')
@@ -478,8 +475,8 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
478475
);
479476

480477
$patches = [
481-
\SomeSchemaPatch::class,
482-
\OtherSchemaPatch::class
478+
\SomeSchemaPatch::class,// phpstan:ignore "Class SomeSchemaPatch not found."
479+
\OtherSchemaPatch::class// phpstan:ignore "Class OtherSchemaPatch not found."
483480
];
484481
$patchRegistryMock = $this->createAggregateIteratorMock(PatchRegistry::class, $patches, ['registerPatch']);
485482
$patchRegistryMock->expects($this->exactly(2))
@@ -489,16 +486,18 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
489486
->method('create')
490487
->willReturn($patchRegistryMock);
491488

489+
// phpstan:ignore "Class SomeSchemaPatch not found."
492490
$patch1 = $this->createMock(\SomeSchemaPatch::class);
493491
$patch1->expects($this->never())->method('apply');
494492
$patch1->expects($this->any())->method('getAliases')->willReturn([]);
493+
// phpstan:ignore "Class OtherSchemaPatch not found."
495494
$patch2 = $this->createMock(\OtherSchemaPatch::class);
496495
$patch2->expects($this->once())->method('apply');
497496
$patch2->expects($this->any())->method('getAliases')->willReturn([]);
498497
$this->patchFactoryMock->expects($this->any())->method('create')->willReturnMap(
499498
[
500-
[\SomeSchemaPatch::class, ['schemaSetup' => $this->schemaSetupMock], $patch1],
501-
[\OtherSchemaPatch::class, ['schemaSetup' => $this->schemaSetupMock], $patch2],
499+
[\SomeSchemaPatch::class, ['schemaSetup' => $this->schemaSetupMock], $patch1],// phpstan:ignore "Class SomeSchemaPatch not found."
500+
[\OtherSchemaPatch::class, ['schemaSetup' => $this->schemaSetupMock], $patch2],// phpstan:ignore "Class OtherSchemaPatch not found."
502501
]
503502
);
504503
$this->connectionMock->expects($this->never())->method('beginTransaction');
@@ -516,8 +515,6 @@ public function testSchemaPatchAplly($moduleName, $schemaPatches, $moduleVersion
516515
*/
517516
public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $moduleVersionInDb)
518517
{
519-
$this->expectException('Exception');
520-
$this->expectExceptionMessageMatches('"Unable to apply patch .+ cannot be applied twice"');
521518
$this->schemaPatchReaderMock->expects($this->once())
522519
->method('read')
523520
->with($moduleName)
@@ -542,19 +539,13 @@ public function testSchemaPatchApplyForPatchAlias($moduleName, $schemaPatches, $
542539
->willReturn($patchRegistryMock);
543540

544541
$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-
);
552542

553543
$this->patchApllier->applySchemaPatch($moduleName);
554544
}
555545

556546
public function testRevertDataPatches()
557547
{
548+
// phpstan:ignore "Class RevertableDataPatch not found."
558549
$patches = [\RevertableDataPatch::class];
559550
$this->dataPatchReaderMock->expects($this->once())
560551
->method('read')
@@ -602,8 +593,8 @@ public function schemaPatchDataProvider()
602593
'upgrade module iwth only OtherSchemaPatch' => [
603594
'moduleName' => 'Module1',
604595
'schemaPatches' => [
605-
\SomeSchemaPatch::class,
606-
\OtherSchemaPatch::class
596+
\SomeSchemaPatch::class,// phpstan:ignore "Class SomeSchemaPatch not found."
597+
\OtherSchemaPatch::class// phpstan:ignore "Class OtherSchemaPatch not found."
607598
],
608599
'moduleVersionInDb' => '2.0.0',
609600
]

0 commit comments

Comments
 (0)