From 7106f69b15dbaccc3c0ec0943a41043f71fd938c Mon Sep 17 00:00:00 2001 From: James Titcumb Date: Mon, 27 Jan 2025 12:00:20 +0000 Subject: [PATCH] Improvements to ensuring pie.json exists --- src/Command/RepositoryAddCommand.php | 13 ++++--- src/Command/RepositoryRemoveCommand.php | 13 ++++--- .../ComposerIntegrationHandler.php | 2 +- .../PieComposerFactory.php | 11 +----- src/ComposerIntegration/PieJsonEditor.php | 38 +++++++++++++++---- .../ComposerIntegration/PieJsonEditorTest.php | 11 +++--- 6 files changed, 55 insertions(+), 33 deletions(-) diff --git a/src/Command/RepositoryAddCommand.php b/src/Command/RepositoryAddCommand.php index 5128041..57e60be 100644 --- a/src/Command/RepositoryAddCommand.php +++ b/src/Command/RepositoryAddCommand.php @@ -7,7 +7,6 @@ use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieJsonEditor; -use Php\Pie\Platform; use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -56,8 +55,8 @@ public function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { - $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); - $pieJsonFilename = Platform::getPieJsonFilename($targetPlatform); + $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); + $pieJsonEditor = PieJsonEditor::fromTargetPlatform($targetPlatform); $type = (string) $input->getArgument(self::ARG_TYPE); /** @psalm-var 'vcs'|'path'|'composer' $type */ @@ -71,11 +70,15 @@ public function execute(InputInterface $input, OutputInterface $output): int if ($type === 'composer' && str_contains($url, 'packagist.org')) { // "adding packagist" is really just removing an exclusion - (new PieJsonEditor($pieJsonFilename))->removeRepository('packagist.org'); + $pieJsonEditor + ->ensureExists() + ->removeRepository('packagist.org'); } else { Assert::stringNotEmpty($url, 'Could not resolve ' . $originalUrl . ' to a real path'); - (new PieJsonEditor($pieJsonFilename))->addRepository($type, $url); + $pieJsonEditor + ->ensureExists() + ->addRepository($type, $url); } CommandHelper::listRepositories( diff --git a/src/Command/RepositoryRemoveCommand.php b/src/Command/RepositoryRemoveCommand.php index 69e7bac..d12f62f 100644 --- a/src/Command/RepositoryRemoveCommand.php +++ b/src/Command/RepositoryRemoveCommand.php @@ -7,7 +7,6 @@ use Php\Pie\ComposerIntegration\PieComposerFactory; use Php\Pie\ComposerIntegration\PieComposerRequest; use Php\Pie\ComposerIntegration\PieJsonEditor; -use Php\Pie\Platform; use Psr\Container\ContainerInterface; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; @@ -47,17 +46,21 @@ public function configure(): void public function execute(InputInterface $input, OutputInterface $output): int { - $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); - $pieJsonFilename = Platform::getPieJsonFilename($targetPlatform); + $targetPlatform = CommandHelper::determineTargetPlatformFromInputs($input, $output); + $pieJsonEditor = PieJsonEditor::fromTargetPlatform($targetPlatform); $url = (string) $input->getArgument(self::ARG_URL); Assert::stringNotEmpty($url); if (str_contains($url, 'packagist.org')) { // "removing packagist" is really just adding an exclusion - (new PieJsonEditor($pieJsonFilename))->excludePackagistOrg(); + $pieJsonEditor + ->ensureExists() + ->excludePackagistOrg(); } else { - (new PieJsonEditor($pieJsonFilename))->removeRepository($url); + $pieJsonEditor + ->ensureExists() + ->removeRepository($url); } CommandHelper::listRepositories( diff --git a/src/ComposerIntegration/ComposerIntegrationHandler.php b/src/ComposerIntegration/ComposerIntegrationHandler.php index 4a03f44..48d93aa 100644 --- a/src/ComposerIntegration/ComposerIntegrationHandler.php +++ b/src/ComposerIntegration/ComposerIntegrationHandler.php @@ -44,7 +44,7 @@ public function __invoke( // Write the new requirement to pie.json; because we later essentially just do a `composer install` using that file $pieComposerJson = Platform::getPieJsonFilename($targetPlatform); - $pieJsonEditor = new PieJsonEditor($pieComposerJson); + $pieJsonEditor = PieJsonEditor::fromTargetPlatform($targetPlatform); $originalPieJsonContent = $pieJsonEditor->addRequire( $requestedPackageAndVersion->package, $recommendedRequireVersion !== '' ? $recommendedRequireVersion : '*', diff --git a/src/ComposerIntegration/PieComposerFactory.php b/src/ComposerIntegration/PieComposerFactory.php index 3b41985..279348d 100644 --- a/src/ComposerIntegration/PieComposerFactory.php +++ b/src/ComposerIntegration/PieComposerFactory.php @@ -16,9 +16,6 @@ use Psr\Container\ContainerInterface; use Webmozart\Assert\Assert; -use function file_exists; -use function mkdir; - /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ class PieComposerFactory extends Factory { @@ -51,15 +48,9 @@ public static function createPieComposer( ContainerInterface $container, PieComposerRequest $composerRequest, ): Composer { - $workDir = Platform::getPieWorkingDirectory($composerRequest->targetPlatform); - - if (! file_exists($workDir)) { - mkdir($workDir, recursive: true); - } - $pieComposer = Platform::getPieJsonFilename($composerRequest->targetPlatform); - (new PieJsonEditor($pieComposer))->ensureExists(); + PieJsonEditor::fromTargetPlatform($composerRequest->targetPlatform)->ensureExists(); $io = $container->get(QuieterConsoleIO::class); $composer = (new PieComposerFactory($container, $composerRequest))->createComposer( diff --git a/src/ComposerIntegration/PieJsonEditor.php b/src/ComposerIntegration/PieJsonEditor.php index 0aa9a1d..973bfbf 100644 --- a/src/ComposerIntegration/PieJsonEditor.php +++ b/src/ComposerIntegration/PieJsonEditor.php @@ -6,11 +6,16 @@ use Composer\Config\JsonConfigSource; use Composer\Json\JsonFile; +use Php\Pie\Platform; +use Php\Pie\Platform\TargetPlatform; +use RuntimeException; use function file_exists; use function file_get_contents; use function file_put_contents; +use function mkdir; use function rtrim; +use function sprintf; use function str_replace; /** @internal This is not public API for PIE, so should not be depended upon unless you accept the risk of BC breaks */ @@ -18,24 +23,43 @@ class PieJsonEditor { public const PACKAGIST_ORG_KEY = 'packagist.org'; - public function __construct(private readonly string $pieJsonFilename) + public function __construct( + private readonly string $pieJsonFilename, + private readonly string $pieWorkingDirectory, + ) { + } + + public static function fromTargetPlatform(TargetPlatform $targetPlatform): self { + return new self( + Platform::getPieJsonFilename($targetPlatform), + Platform::getPieWorkingDirectory($targetPlatform), + ); } /** * Ensure the given `pie.json` exists; if it does not exist, create an * empty but valid `pie.json`. */ - public function ensureExists(): void + public function ensureExists(): self { if (file_exists($this->pieJsonFilename)) { - return; + return $this; } - file_put_contents( - $this->pieJsonFilename, - "{\n}\n", - ); + if (! file_exists($this->pieWorkingDirectory)) { + mkdir($this->pieWorkingDirectory, recursive: true); + } + + if (file_put_contents($this->pieJsonFilename, "{\n}\n") === false) { + throw new RuntimeException(sprintf( + 'Failed to create pie.json in %s (working directory: %s)', + $this->pieJsonFilename, + $this->pieWorkingDirectory, + )); + } + + return $this; } /** diff --git a/test/unit/ComposerIntegration/PieJsonEditorTest.php b/test/unit/ComposerIntegration/PieJsonEditorTest.php index 5b05cd6..29edc04 100644 --- a/test/unit/ComposerIntegration/PieJsonEditorTest.php +++ b/test/unit/ComposerIntegration/PieJsonEditorTest.php @@ -8,6 +8,7 @@ use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; +use function dirname; use function file_get_contents; use function json_decode; use function json_encode; @@ -25,7 +26,7 @@ public function testCreatingPieJson(): void self::assertFileDoesNotExist($testPieJson); - (new PieJsonEditor($testPieJson))->ensureExists(); + (new PieJsonEditor($testPieJson, dirname($testPieJson)))->ensureExists(); self::assertFileExists($testPieJson); self::assertSame( @@ -38,7 +39,7 @@ public function testCanAddRequire(): void { $testPieJson = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_json_test_', true) . '.json'; - $editor = new PieJsonEditor($testPieJson); + $editor = new PieJsonEditor($testPieJson, dirname($testPieJson)); $editor->ensureExists(); $editor->addRequire('foo/bar', '^1.2'); @@ -58,7 +59,7 @@ public function testCanRevert(): void { $testPieJson = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_json_test_', true) . '.json'; - $editor = new PieJsonEditor($testPieJson); + $editor = new PieJsonEditor($testPieJson, dirname($testPieJson)); $editor->ensureExists(); $originalContent = $editor->addRequire('foo/bar', '^1.2'); $editor->revert($originalContent); @@ -72,7 +73,7 @@ public function testCanAddAndRemoveRepositories(): void { $testPieJson = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_json_test_', true) . '.json'; - $editor = new PieJsonEditor($testPieJson); + $editor = new PieJsonEditor($testPieJson, dirname($testPieJson)); $editor->ensureExists(); $originalContent = $editor->addRepository( @@ -141,7 +142,7 @@ public function testCanAddAndRemoveWithTrailingSlash(): void { $testPieJson = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('pie_json_test_', true) . '.json'; - $editor = new PieJsonEditor($testPieJson); + $editor = new PieJsonEditor($testPieJson, dirname($testPieJson)); $editor->ensureExists(); $originalContent = $editor->addRepository(