Skip to content

Commit 9c402af

Browse files
fabpotfabpot
committed
Merge branch '1.x' into 2.x
* 1.x: Fixing bug where update changes were made immediately vs reported to RecipeUpdate Avoid exploding on conflict of controllers.json Internal: moving most of method to a private method, no changes Moving guts of method to a private method - no real change Internal: changing private functions to return a string
2 parents 347248e + a2554c7 commit 9c402af

File tree

3 files changed

+110
-63
lines changed

3 files changed

+110
-63
lines changed

‎src/Configurator/AddLinesConfigurator.php

Lines changed: 89 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,61 @@ 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
36+
{
37+
$this->fileContents = [];
38+
$this->executeConfigure($recipe, $config);
39+
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);
43+
}
44+
}
45+
46+
public function unconfigure(Recipe $recipe, $config, Lock $lock): void
47+
{
48+
$this->fileContents = [];
49+
$this->executeUnconfigure($recipe, $config);
50+
51+
foreach ($this->fileContents as $file => $change) {
52+
$this->write(sprintf('[add-lines] Reverting file "%s"', $this->relativize($file)));
53+
file_put_contents($file, $change);
54+
}
55+
}
56+
57+
public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array $newConfig): void
58+
{
59+
// manually check for "requires", as unconfigure ignores it
60+
$originalConfig = array_filter($originalConfig, function ($item) {
61+
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
62+
});
63+
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);
78+
}
79+
80+
public function executeConfigure(Recipe $recipe, $config): void
2781
{
2882
foreach ($config as $patch) {
2983
if (!isset($patch['file'])) {
@@ -57,8 +111,6 @@ public function configure(Recipe $recipe, $config, Lock $lock, array $options =
57111
continue;
58112
}
59113

60-
$this->write(sprintf('Patching file "%s"', $patch['file']));
61-
62114
if (!isset($patch['position'])) {
63115
$this->write(sprintf('The "position" key is required for the "add-lines" configurator for recipe "%s". Skipping', $recipe->getName()));
64116

@@ -78,11 +130,12 @@ public function configure(Recipe $recipe, $config, Lock $lock, array $options =
78130
}
79131
$target = isset($patch['target']) ? $patch['target'] : null;
80132

81-
$this->patchFile($file, $content, $position, $target, $warnIfMissing);
133+
$newContents = $this->getPatchedContents($file, $content, $position, $target, $warnIfMissing);
134+
$this->fileContents[$file] = $newContents;
82135
}
83136
}
84137

85-
public function unconfigure(Recipe $recipe, $config, Lock $lock): void
138+
public function executeUnconfigure(Recipe $recipe, $config): void
86139
{
87140
foreach ($config as $patch) {
88141
if (!isset($patch['file'])) {
@@ -106,51 +159,17 @@ public function unconfigure(Recipe $recipe, $config, Lock $lock): void
106159
}
107160
$value = $patch['content'];
108161

109-
$this->unPatchFile($file, $value);
162+
$newContents = $this->getUnPatchedContents($file, $value);
163+
$this->fileContents[$file] = $newContents;
110164
}
111165
}
112166

113-
public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array $newConfig): void
167+
private function getPatchedContents(string $file, string $value, string $position, ?string $target, bool $warnIfMissing): string
114168
{
115-
$originalConfig = array_filter($originalConfig, function ($item) {
116-
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
117-
});
118-
$newConfig = array_filter($newConfig, function ($item) {
119-
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
120-
});
121-
122-
$filterDuplicates = function (array $sourceConfig, array $comparisonConfig) {
123-
$filtered = [];
124-
foreach ($sourceConfig as $sourceItem) {
125-
$found = false;
126-
foreach ($comparisonConfig as $comparisonItem) {
127-
if ($sourceItem['file'] === $comparisonItem['file'] && $sourceItem['content'] === $comparisonItem['content']) {
128-
$found = true;
129-
break;
130-
}
131-
}
132-
if (!$found) {
133-
$filtered[] = $sourceItem;
134-
}
135-
}
136-
137-
return $filtered;
138-
};
139-
140-
// remove any config where the file+value is the same before & after
141-
$filteredOriginalConfig = $filterDuplicates($originalConfig, $newConfig);
142-
$filteredNewConfig = $filterDuplicates($newConfig, $originalConfig);
143-
144-
$this->unconfigure($recipeUpdate->getOriginalRecipe(), $filteredOriginalConfig, $recipeUpdate->getLock());
145-
$this->configure($recipeUpdate->getNewRecipe(), $filteredNewConfig, $recipeUpdate->getLock());
146-
}
147-
148-
private function patchFile(string $file, string $value, string $position, ?string $target, bool $warnIfMissing)
149-
{
150-
$fileContents = file_get_contents($file);
169+
$fileContents = $this->readFile($file);
151170

152171
if (false !== strpos($fileContents, $value)) {
153-
return; // already includes value, skip
172+
return $fileContents; // already includes value, skip
154173
}
155174

156175
switch ($position) {
@@ -188,15 +207,15 @@ private function patchFile(string $file, string $value, string $position, ?strin
188207
break;
189208
}
190209

191-
file_put_contents($file, $fileContents);
210+
return $fileContents;
192211
}
193212

194-
private function unPatchFile(string $file, $value)
213+
private function getUnPatchedContents(string $file, $value): string
195214
{
196-
$fileContents = file_get_contents($file);
215+
$fileContents = $this->readFile($file);
197216

198217
if (false === strpos($fileContents, $value)) {
199-
return; // value already gone!
218+
return $fileContents; // value already gone!
200219
}
201220

202221
if (false !== strpos($fileContents, "\n".$value)) {
@@ -206,9 +225,8 @@ private function unPatchFile(string $file, $value)
206225
}
207226

208227
$position = strpos($fileContents, $value);
209-
$fileContents = substr_replace($fileContents, '', $position, \strlen($value));
210228

211-
file_put_contents($file, $fileContents);
229+
return substr_replace($fileContents, '', $position, \strlen($value));
212230
}
213231

214232
private function isPackageInstalled($packages): bool
@@ -227,4 +245,26 @@ private function isPackageInstalled($packages): bool
227245

228246
return true;
229247
}
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+
}
230270
}

‎src/PackageJsonSynchronizer.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,12 @@ private function updateControllersJsonFile(array $phpPackages)
296296
return;
297297
}
298298

299-
$previousControllersJson = (new JsonFile($controllersJsonPath))->read();
299+
try {
300+
$previousControllersJson = (new JsonFile($controllersJsonPath))->read();
301+
} catch (ParsingException $e) {
302+
// if controllers.json is invalid (possible during a recipe upgrade), we can't update the file
303+
return;
304+
}
300305
$newControllersJson = [
301306
'controllers' => [],
302307
'entrypoints' => $previousControllersJson['entrypoints'],

‎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)