Skip to content

Commit

Permalink
Fixing bug where update changes were made immediately vs reported to …
Browse files Browse the repository at this point in the history 
…RecipeUpdate
  • Loading branch information
@weaverryan
weaverryan committed Aug 2, 2023
1 parent 7e8967a commit afd6260
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 61 deletions.
110 changes: 62 additions & 48 deletions src/Configurator/AddLinesConfigurator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,64 +23,62 @@ class AddLinesConfigurator extends AbstractConfigurator
self::POSITION_AFTER_TARGET,
];

/**
* Holds file contents for files that have been loaded.
* This allows us to "change" the contents of a file multiple
* times before we actually write it out.
*
* @var string[]
*/
private $fileContents = [];

public function configure(Recipe $recipe, $config, Lock $lock, array $options = []): void
{
$changes = $this->getConfigureFileChanges($recipe, $config);
$this->fileContents = [];
$this->executeConfigure($recipe, $config);

foreach ($changes as $file => $change) {
$this->write(sprintf('[add-lines] Patching file "%s"', $file));
file_put_contents($file, $change);
foreach ($this->fileContents as $file => $contents) {
$this->write(sprintf('[add-lines] Patching file "%s"', $this->relativize($file)));
file_put_contents($file, $contents);
}
}

public function unconfigure(Recipe $recipe, $config, Lock $lock): void
{
$changes = $this->getUnconfigureFileChanges($recipe, $config);
$this->fileContents = [];
$this->executeUnconfigure($recipe, $config);

foreach ($changes as $file => $change) {
$this->write(sprintf('[add-lines] Reverting file "%s"', $file));
foreach ($this->fileContents as $file => $change) {
$this->write(sprintf('[add-lines] Reverting file "%s"', $this->relativize($file)));
file_put_contents($file, $change);
}
}

public function update(RecipeUpdate $recipeUpdate, array $originalConfig, array $newConfig): void
{
// manually check for "requires", as unconfigure ignores it
$originalConfig = array_filter($originalConfig, function ($item) {
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
});
$newConfig = array_filter($newConfig, function ($item) {
return !isset($item['requires']) || $this->isPackageInstalled($item['requires']);
});

$filterDuplicates = function (array $sourceConfig, array $comparisonConfig) {
$filtered = [];
foreach ($sourceConfig as $sourceItem) {
$found = false;
foreach ($comparisonConfig as $comparisonItem) {
if ($sourceItem['file'] === $comparisonItem['file'] && $sourceItem['content'] === $comparisonItem['content']) {
$found = true;
break;
}
}
if (!$found) {
$filtered[] = $sourceItem;
}
}

return $filtered;
};

// remove any config where the file+value is the same before & after
$filteredOriginalConfig = $filterDuplicates($originalConfig, $newConfig);
$filteredNewConfig = $filterDuplicates($newConfig, $originalConfig);

$this->unconfigure($recipeUpdate->getOriginalRecipe(), $filteredOriginalConfig, $recipeUpdate->getLock());
$this->configure($recipeUpdate->getNewRecipe(), $filteredNewConfig, $recipeUpdate->getLock());
// reset the file content cache
$this->fileContents = [];
$this->executeUnconfigure($recipeUpdate->getOriginalRecipe(), $originalConfig);
$this->executeConfigure($recipeUpdate->getNewRecipe(), $newConfig);
$newFiles = [];
$originalFiles = [];
foreach ($this->fileContents as $file => $contents) {
// set the original file to the current contents
$originalFiles[$this->relativize($file)] = file_get_contents($file);
// and the new file where the old recipe was unconfigured, and the new configured
$newFiles[$this->relativize($file)] = $contents;
}
$recipeUpdate->addOriginalFiles($originalFiles);
$recipeUpdate->addNewFiles($newFiles);
}

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

$newContents = $this->getPatchedContents($file, $content, $position, $target, $warnIfMissing);
$changes[$file] = $newContents;
$this->fileContents[$file] = $newContents;
}

return $changes;
}

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

$newContents = $this->getUnPatchedContents($file, $value);
$changes[$file] = $newContents;
$this->fileContents[$file] = $newContents;
}

return $changes;
}

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

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

private function getUnPatchedContents(string $file, $value): string
{
$fileContents = file_get_contents($file);
$fileContents = $this->readFile($file);

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

$position = strpos($fileContents, $value);
$fileContents = substr_replace($fileContents, '', $position, \strlen($value));

return $fileContents;
return substr_replace($fileContents, '', $position, \strlen($value));
}

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

return true;
}

private function relativize(string $path): string
{
$rootDir = $this->options->get('root-dir');
if (0 === strpos($path, $rootDir)) {
$path = substr($path, \strlen($rootDir) + 1);
}

return ltrim($path, '/\\');
}

private function readFile(string $file): string
{
if (isset($this->fileContents[$file])) {
return $this->fileContents[$file];
}

$fileContents = file_get_contents($file);
$this->fileContents[$file] = $fileContents;

return $fileContents;
}
}
28 changes: 15 additions & 13 deletions tests/Configurator/AddLinesConfiguratorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -409,8 +409,8 @@ public function getUnconfigureTests()
*/
public function testUpdate(array $originalFiles, array $originalConfig, array $newConfig, array $expectedFiles)
{
foreach ($originalFiles as $filename => $contents) {
$this->saveFile($filename, $contents);
foreach ($originalFiles as $filename => $originalContents) {
$this->saveFile($filename, $originalContents);
}

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

foreach ($expectedFiles as $filename => $contents) {
$this->assertSame($contents, $this->readFile($filename));
$this->assertCount(\count($expectedFiles), $recipeUpdate->getNewFiles());
foreach ($expectedFiles as $filename => $expectedContents) {
$this->assertSame($this->readFile($filename), $recipeUpdate->getOriginalFiles()[$filename]);
$this->assertSame($expectedContents, $recipeUpdate->getNewFiles()[$filename]);
}
}

public function getUpdateTests()
{
$appJs = <<<EOF
$appJsOriginal = <<<EOF
import * as Turbo from '@hotwired/turbo';
import './bootstrap';
console.log(Turbo);
EOF;

$bootstrapJs = <<<EOF
$bootstrapJsOriginal = <<<EOF
console.log('bootstrap.js');
console.log('on the bottom');
EOF;

yield 'recipe_changes_patch_contents' => [
['assets/app.js' => $appJs],
['assets/app.js' => $appJsOriginal],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';"],
],
Expand All @@ -461,18 +463,18 @@ public function getUpdateTests()
];

yield 'recipe_file_and_value_same_before_and_after' => [
['assets/app.js' => $appJs],
['assets/app.js' => $appJsOriginal],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import * as Turbo from '@hotwired/turbo';"],
],
['assets/app.js' => $appJs],
['assets/app.js' => $appJsOriginal],
];

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

yield 'recipe_changes_but_ignored_because_package_not_installed' => [
['assets/app.js' => $appJs],
['assets/app.js' => $appJsOriginal],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/not-installed'],
],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './stimulus_bootstrap';", 'requires' => 'symfony/not-installed'],
],
['assets/app.js' => $appJs],
[], // no changes will come back in the RecipePatch
];

yield 'recipe_changes_are_applied_if_required_package_installed' => [
['assets/app.js' => $appJs],
['assets/app.js' => $appJsOriginal],
[
['file' => 'assets/app.js', 'position' => 'top', 'content' => "import './bootstrap';", 'requires' => 'symfony/installed-package'],
],
Expand Down

0 comments on commit afd6260

Please sign in to comment.