Skip to content

Commit afd6260

Browse files
weaverryanweaverryan
committed
Fixing bug where update changes were made immediately vs reported to RecipeUpdate
1 parent 7e8967a commit afd6260

File tree

2 files changed

+77
-61
lines changed

2 files changed

+77
-61
lines changed

‎src/Configurator/AddLinesConfigurator.php

Lines changed: 62 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -23,64 +23,62 @@ class AddLinesConfigurator extends AbstractConfigurator
2323
self::POSITION_AFTER_TARGET,
2424
];
2525

26+
/**
27+
* Holds file contents for files that have been loaded.
28+
* This allows us to "change" the contents of a file multiple
29+
* times before we actually write it out.
30+
*
31+
* @var string[]
32+
*/
33+
private $fileContents = [];
34+
2635
public function configure(Recipe $recipe, $config, Lock $lock, array $options = []): void
2736
{
28-
$changes = $this->getConfigureFileChanges($recipe, $config);
37+
$this->fileContents = [];
38+
$this->executeConfigure($recipe, $config);
2939

30-
foreach ($changes as $file => $change) {
31-
$this->write(sprintf('[add-lines] Patching file "%s"', $file));
32-
file_put_contents($file, $change);
40+
foreach ($this->fileContents as $file => $contents) {
41+
$this->write(sprintf('[add-lines] Patching file "%s"', $this->relativize($file)));
42+
file_put_contents($file, $contents);
3343
}
3444
}
3545

3646
public function unconfigure(Recipe $recipe, $config, Lock $lock): void
3747
{
38-
$changes = $this->getUnconfigureFileChanges($recipe, $config);
48+
$this->fileContents = [];
49+
$this->executeUnconfigure($recipe, $config);
3950

40-
foreach ($changes as $file => $change) {
41-
$this->write(sprintf('[add-lines] Reverting file "%s"', $file));
51+
foreach ($this->fileContents as $file => $change) {
52+
$this->write(sprintf('[add-lines] Reverting file "%s"', $this->relativize($file)));
4253
file_put_contents($file, $change);
4354
}
4455
}
4556

4657
public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array $newConfig): void
4758
{
59+
// manually check for "requires", as unconfigure ignores it
4860
$originalConfig = array_filter($originalConfig, function ($item) {
4961
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
5062
});
51-
$newConfig = array_filter($newConfig, function ($item) {
52-
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
53-
});
54-
55-
$filterDuplicates = function (array $sourceConfig, array $comparisonConfig) {
56-
$filtered = [];
57-
foreach ($sourceConfig as $sourceItem) {
58-
$found = false;
59-
foreach ($comparisonConfig as $comparisonItem) {
60-
if ($sourceItem['file'] === $comparisonItem['file'] && $sourceItem['content'] === $comparisonItem['content']) {
61-
$found = true;
62-
break;
63-
}
64-
}
65-
if (!$found) {
66-
$filtered[] = $sourceItem;
67-
}
68-
}
69-
70-
return $filtered;
71-
};
72-
73-
// remove any config where the file+value is the same before & after
74-
$filteredOriginalConfig = $filterDuplicates($originalConfig, $newConfig);
75-
$filteredNewConfig = $filterDuplicates($newConfig, $originalConfig);
7663

77-
$this->unconfigure($recipeUpdate->getOriginalRecipe(), $filteredOriginalConfig, $recipeUpdate->getLock());
78-
$this->configure($recipeUpdate->getNewRecipe(), $filteredNewConfig, $recipeUpdate->getLock());
64+
// reset the file content cache
65+
$this->fileContents = [];
66+
$this->executeUnconfigure($recipeUpdate->getOriginalRecipe(), $originalConfig);
67+
$this->executeConfigure($recipeUpdate->getNewRecipe(), $newConfig);
68+
$newFiles = [];
69+
$originalFiles = [];
70+
foreach ($this->fileContents as $file => $contents) {
71+
// set the original file to the current contents
72+
$originalFiles[$this->relativize($file)] = file_get_contents($file);
73+
// and the new file where the old recipe was unconfigured, and the new configured
74+
$newFiles[$this->relativize($file)] = $contents;
75+
}
76+
$recipeUpdate->addOriginalFiles($originalFiles);
77+
$recipeUpdate->addNewFiles($newFiles);
7978
}
8079

81-
public function getConfigureFileChanges(Recipe $recipe, $config): array
80+
public function executeConfigure(Recipe $recipe, $config): void
8281
{
83-
$changes = [];
8482
foreach ($config as $patch) {
8583
if (!isset($patch['file'])) {
8684
$this->write(sprintf('The "file" key is required for the "add-lines" configurator for recipe "%s". Skipping', $recipe->getName()));
@@ -133,15 +131,12 @@ public function getConfigureFileChanges(Recipe $recipe, $config): array
133131
$target = isset($patch['target']) ? $patch['target'] : null;
134132

135133
$newContents = $this->getPatchedContents($file, $content, $position, $target, $warnIfMissing);
136-
$changes[$file] = $newContents;
134+
$this->fileContents[$file] = $newContents;
137135
}
138-
139-
return $changes;
140136
}
141137

142-
public function getUnconfigureFileChanges(Recipe $recipe, $config): array
138+
public function executeUnconfigure(Recipe $recipe, $config): void
143139
{
144-
$changes = [];
145140
foreach ($config as $patch) {
146141
if (!isset($patch['file'])) {
147142
$this->write(sprintf('The "file" key is required for the "add-lines" configurator for recipe "%s". Skipping', $recipe->getName()));
@@ -165,15 +160,13 @@ public function getUnconfigureFileChanges(Recipe $recipe, $config): array
165160
$value = $patch['content'];
166161

167162
$newContents = $this->getUnPatchedContents($file, $value);
168-
$changes[$file] = $newContents;
163+
$this->fileContents[$file] = $newContents;
169164
}
170-
171-
return $changes;
172165
}
173166

174167
private function getPatchedContents(string $file, string $value, string $position, ?string $target, bool $warnIfMissing): string
175168
{
176-
$fileContents = file_get_contents($file);
169+
$fileContents = $this->readFile($file);
177170

178171
if (false !== strpos($fileContents, $value)) {
179172
return $fileContents; // already includes value, skip
@@ -219,7 +212,7 @@ private function getPatchedContents(string $file, string $value, string $positio
219212

220213
private function getUnPatchedContents(string $file, $value): string
221214
{
222-
$fileContents = file_get_contents($file);
215+
$fileContents = $this->readFile($file);
223216

224217
if (false === strpos($fileContents, $value)) {
225218
return $fileContents; // value already gone!
@@ -232,9 +225,8 @@ private function getUnPatchedContents(string $file, $value): string
232225
}
233226

234227
$position = strpos($fileContents, $value);
235-
$fileContents = substr_replace($fileContents, '', $position, \strlen($value));
236228

237-
return $fileContents;
229+
return substr_replace($fileContents, '', $position, \strlen($value));
238230
}
239231

240232
private function isPackageInstalled($packages): bool
@@ -253,4 +245,26 @@ private function isPackageInstalled($packages): bool
253245

254246
return true;
255247
}
248+
249+
private function relativize(string $path): string
250+
{
251+
$rootDir = $this->options->get('root-dir');
252+
if (0 === strpos($path, $rootDir)) {
253+
$path = substr($path, \strlen($rootDir) + 1);
254+
}
255+
256+
return ltrim($path, '/\\');
257+
}
258+
259+
private function readFile(string $file): string
260+
{
261+
if (isset($this->fileContents[$file])) {
262+
return $this->fileContents[$file];
263+
}
264+
265+
$fileContents = file_get_contents($file);
266+
$this->fileContents[$file] = $fileContents;
267+
268+
return $fileContents;
269+
}
256270
}

‎tests/Configurator/AddLinesConfiguratorTest.php

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,8 @@ public function getUnconfigureTests()
409409
*/
410410
public function testUpdate(array $originalFiles, array $originalConfig, array $newConfig, array $expectedFiles)
411411
{
412-
foreach ($originalFiles as $filename => $contents) {
413-
$this->saveFile($filename, $contents);
412+
foreach ($originalFiles as $filename => $originalContents) {
413+
$this->saveFile($filename, $originalContents);
414414
}
415415

416416
$composer = $this->createComposerMockWithPackagesInstalled([
@@ -423,28 +423,30 @@ public function testUpdate(array $originalFiles, array $originalConfig, array $n
423423
$recipeUpdate = new RecipeUpdate($recipe, $recipe, $lock, FLEX_TEST_DIR);
424424
$configurator->update($recipeUpdate, $originalConfig, $newConfig);
425425

426-
foreach ($expectedFiles as $filename => $contents) {
427-
$this->assertSame($contents, $this->readFile($filename));
426+
$this->assertCount(\count($expectedFiles), $recipeUpdate->getNewFiles());
427+
foreach ($expectedFiles as $filename => $expectedContents) {
428+
$this->assertSame($this->readFile($filename), $recipeUpdate->getOriginalFiles()[$filename]);
429+
$this->assertSame($expectedContents, $recipeUpdate->getNewFiles()[$filename]);
428430
}
429431
}
430432

431433
public function getUpdateTests()
432434
{
433-
$appJs = <<<EOF
435+
$appJsOriginal = <<<EOF
434436
import * as Turbo from '@hotwired/turbo';
435437
import './bootstrap';
436438
437439
console.log(Turbo);
438440
EOF;
439441

440-
$bootstrapJs = <<<EOF
442+
$bootstrapJsOriginal = <<<EOF
441443
console.log('bootstrap.js');
442444
443445
console.log('on the bottom');
444446
EOF;
445447

446448
yield 'recipe_changes_patch_contents' => [
447-
['assets/app.js' => $appJs],
449+
['assets/app.js' => $appJsOriginal],
448450
[
449451
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';"],
450452
],
@@ -461,18 +463,18 @@ public function getUpdateTests()
461463
];
462464

463465
yield 'recipe_file_and_value_same_before_and_after' => [
464-
['assets/app.js' => $appJs],
466+
['assets/app.js' => $appJsOriginal],
465467
[
466468
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
467469
],
468470
[
469471
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
470472
],
471-
['assets/app.js' => $appJs],
473+
['assets/app.js' => $appJsOriginal],
472474
];
473475

474476
yield 'different_files_unconfigures_old_and_configures_new' => [
475-
['assets/app.js' => $appJs, 'assets/bootstrap.js' => $bootstrapJs],
477+
['assets/app.js' => $appJsOriginal, 'assets/bootstrap.js' => $bootstrapJsOriginal],
476478
[
477479
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
478480
],
@@ -496,18 +498,18 @@ public function getUpdateTests()
496498
];
497499

498500
yield 'recipe_changes_but_ignored_because_package_not_installed' => [
499-
['assets/app.js' => $appJs],
501+
['assets/app.js' => $appJsOriginal],
500502
[
501503
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/not-installed'],
502504
],
503505
[
504506
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/not-installed'],
505507
],
506-
['assets/app.js' => $appJs],
508+
[], // no changes will come back in the RecipePatch
507509
];
508510

509511
yield 'recipe_changes_are_applied_if_required_package_installed' => [
510-
['assets/app.js' => $appJs],
512+
['assets/app.js' => $appJsOriginal],
511513
[
512514
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/installed-package'],
513515
],

0 commit comments

Comments
 (0)