From 2723124fe2f81556e98ef467f9d6e590f6b6e580 Mon Sep 17 00:00:00 2001 From: Pauline Vos Date: Mon, 5 Oct 2026 21:26:45 +0200 Subject: [PATCH] Sync plugin listing README from listed release Instead of syncing the README from the default branch, it should be synced from the commit hash of the release that it's listed with. For example: a README change may have merged into `main` before a new release was tagged. This means the README may include documentation for changes that have not yet been released. --- app/Services/PluginSyncService.php | 74 ++++++++-- .../CustomerPluginReviewChecksTest.php | 15 +- .../Customer/PluginStatusTransitionsTest.php | 2 + tests/Feature/PluginSyncServiceTest.php | 135 ++++++++++++++++++ 4 files changed, 209 insertions(+), 17 deletions(-) diff --git a/app/Services/PluginSyncService.php b/app/Services/PluginSyncService.php index a6bc4dc7b..710ab4cb2 100644 --- a/app/Services/PluginSyncService.php +++ b/app/Services/PluginSyncService.php @@ -38,10 +38,32 @@ public function sync(Plugin $plugin): bool 'has_token' => $token !== null, ]); - $readme = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'README.md', $token); - $license = $this->fetchLicenseFile($repo['owner'], $repo['repo'], $token); - $composerJson = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'composer.json', $token); - $nativephpJson = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'nativephp.json', $token); + try { + $latestTag = $this->fetchLatestTag($repo['owner'], $repo['repo'], $token); + } catch (\Exception $e) { + Log::warning('[PluginSync] Could not discover latest release', [ + 'plugin_id' => $plugin->id, + 'error' => $e->getMessage(), + ]); + + return false; + } + $commitSha = null; + + if ($latestTag !== null) { + $commitSha = $this->fetchCommitSha($repo['owner'], $repo['repo'], $latestTag, $token); + + if ($commitSha === null) { + Log::warning('[PluginSync] Could not resolve release commit', ['plugin_id' => $plugin->id, 'tag' => $latestTag]); + + return false; + } + } + + $readme = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'README.md', $token, $commitSha); + $license = $this->fetchLicenseFile($repo['owner'], $repo['repo'], $token, $commitSha); + $composerJson = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'composer.json', $token, $commitSha); + $nativephpJson = $this->fetchFileFromGitHub($repo['owner'], $repo['repo'], 'nativephp.json', $token, $commitSha); Log::info('[PluginSync] Fetch results', [ 'plugin_id' => $plugin->id, @@ -106,8 +128,6 @@ public function sync(Plugin $plugin): bool $updateData['license_html'] = CommonMark::convertToHtml($license); } - // Fetch the latest tag/release - $latestTag = $this->fetchLatestTag($repo['owner'], $repo['repo'], $token); if ($latestTag) { $updateData['latest_version'] = ltrim($latestTag, 'v'); } @@ -146,6 +166,10 @@ public function fetchLatestTag(string $owner, string $repo, ?string $token): ?st return $response->json('tag_name'); } + if (! $response->notFound()) { + $response->throw(); + } + // Fall back to tags if no releases exist $tagsResponse = Http::timeout(10) ->when($token, fn ($http) => $http->withToken($token)) @@ -153,17 +177,41 @@ public function fetchLatestTag(string $owner, string $repo, ?string $token): ?st 'per_page' => 1, ]); + if (! $tagsResponse->notFound()) { + $tagsResponse->throw(); + } + if ($tagsResponse->successful() && count($tagsResponse->json()) > 0) { return $tagsResponse->json()[0]['name']; } } catch (\Exception $e) { Log::warning("Failed to fetch latest tag for {$owner}/{$repo}: {$e->getMessage()}"); + + throw $e; + } + + return null; + } + + protected function fetchCommitSha(string $owner, string $repo, string $tag, ?string $token): ?string + { + try { + $reference = rawurlencode($tag); + $response = Http::timeout(10) + ->when($token, fn ($http) => $http->withToken($token)) + ->get("https://api.github.com/repos/{$owner}/{$repo}/commits/{$reference}"); + + if ($response->successful() && filled($response->json('sha'))) { + return $response->json('sha'); + } + } catch (\Exception $e) { + Log::warning("Failed to resolve release commit for {$owner}/{$repo}: {$e->getMessage()}"); } return null; } - protected function fetchFileFromGitHub(string $owner, string $repo, string $path, ?string $token): ?string + protected function fetchFileFromGitHub(string $owner, string $repo, string $path, ?string $token, ?string $commitSha = null): ?string { try { $request = Http::timeout(10); @@ -172,7 +220,10 @@ protected function fetchFileFromGitHub(string $owner, string $repo, string $path $request = $request->withToken($token); } - $response = $request->get("https://api.github.com/repos/{$owner}/{$repo}/contents/{$path}"); + $response = $request->get( + "https://api.github.com/repos/{$owner}/{$repo}/contents/{$path}", + $commitSha !== null ? ['ref' => $commitSha] : [], + ); if ($response->successful()) { $data = $response->json(); @@ -182,7 +233,8 @@ protected function fetchFileFromGitHub(string $owner, string $repo, string $path } } - $baseUrl = "https://raw.githubusercontent.com/{$owner}/{$repo}/main"; + $reference = $commitSha ?? 'main'; + $baseUrl = "https://raw.githubusercontent.com/{$owner}/{$repo}/{$reference}"; $fallbackResponse = Http::timeout(10)->get("{$baseUrl}/{$path}"); if ($fallbackResponse->successful()) { @@ -205,13 +257,13 @@ protected function extractAndroidVersion(array $nativephpData): ?string return $nativephpData['android']['min_version'] ?? null; } - protected function fetchLicenseFile(string $owner, string $repo, ?string $token): ?string + protected function fetchLicenseFile(string $owner, string $repo, ?string $token, ?string $commitSha = null): ?string { // Try common license file names $licenseFiles = ['LICENSE.md', 'LICENSE', 'LICENSE.txt', 'license.md', 'license', 'license.txt']; foreach ($licenseFiles as $filename) { - $content = $this->fetchFileFromGitHub($owner, $repo, $filename, $token); + $content = $this->fetchFileFromGitHub($owner, $repo, $filename, $token, $commitSha); if ($content) { return $content; diff --git a/tests/Feature/CustomerPluginReviewChecksTest.php b/tests/Feature/CustomerPluginReviewChecksTest.php index fc98ad5b1..4801bf15e 100644 --- a/tests/Feature/CustomerPluginReviewChecksTest.php +++ b/tests/Feature/CustomerPluginReviewChecksTest.php @@ -34,7 +34,7 @@ private function fakeGitHubForCreateAndSubmit(string $repoSlug): void Http::fake([ // PluginSyncService calls - "{$base}/contents/README.md" => Http::response([ + "{$base}/contents/README.md*" => Http::response([ 'content' => base64_encode('# Test Plugin'), 'encoding' => 'base64', ]), @@ -42,8 +42,9 @@ private function fakeGitHubForCreateAndSubmit(string $repoSlug): void 'content' => base64_encode($composerJson), 'encoding' => 'base64', ]), - "{$base}/contents/nativephp.json" => Http::response([], 404), + "{$base}/contents/nativephp.json*" => Http::response([], 404), "{$base}/contents/LICENSE*" => Http::response([], 404), + "{$base}/commits/v1.0.0" => Http::response(['sha' => str_repeat('a', 40)]), "{$base}/releases/latest" => Http::response(['tag_name' => 'v1.0.0']), "{$base}/tags*" => Http::response([]), "https://raw.githubusercontent.com/{$repoSlug}/*" => Http::response('', 404), @@ -168,7 +169,7 @@ public function plugin_submitted_email_includes_failing_optional_checks(): void ]); Http::fake([ - "{$base}/contents/README.md" => Http::response([ + "{$base}/contents/README.md*" => Http::response([ 'content' => base64_encode('# Bare Plugin'), 'encoding' => 'base64', ]), @@ -176,8 +177,9 @@ public function plugin_submitted_email_includes_failing_optional_checks(): void 'content' => base64_encode($composerJson), 'encoding' => 'base64', ]), - "{$base}/contents/nativephp.json" => Http::response([], 404), + "{$base}/contents/nativephp.json*" => Http::response([], 404), "{$base}/contents/LICENSE*" => Http::response([], 404), + "{$base}/commits/v1.0.0" => Http::response(['sha' => str_repeat('a', 40)]), "{$base}/releases/latest" => Http::response(['tag_name' => 'v1.0.0']), "{$base}/tags*" => Http::response([['name' => 'v1.0.0']]), "https://raw.githubusercontent.com/{$repoSlug}/*" => Http::response('', 404), @@ -223,12 +225,13 @@ public function plugin_submitted_email_includes_failing_optional_checks(): void 'content' => base64_encode($composerJson), 'encoding' => 'base64', ]), - "{$base}/contents/README.md" => Http::response([ + "{$base}/contents/README.md*" => Http::response([ 'content' => base64_encode('# Bare Plugin'), 'encoding' => 'base64', ]), - "{$base}/contents/nativephp.json" => Http::response([], 404), + "{$base}/contents/nativephp.json*" => Http::response([], 404), "{$base}/contents/LICENSE*" => Http::response([], 404), + "{$base}/commits/v1.0.0" => Http::response(['sha' => str_repeat('a', 40)]), "{$base}/releases/latest" => Http::response(['tag_name' => 'v1.0.0']), "{$base}/tags*" => Http::response([['name' => 'v1.0.0']]), "{$base}/hooks" => function ($request) { diff --git a/tests/Feature/Livewire/Customer/PluginStatusTransitionsTest.php b/tests/Feature/Livewire/Customer/PluginStatusTransitionsTest.php index 2abbd9ce4..348f65363 100644 --- a/tests/Feature/Livewire/Customer/PluginStatusTransitionsTest.php +++ b/tests/Feature/Livewire/Customer/PluginStatusTransitionsTest.php @@ -73,6 +73,7 @@ private function fakeGitHubForSubmission(Plugin $plugin, bool $passingChecks = t "{$base}/contents/LICENSE*" => $passingChecks ? Http::response(['name' => 'LICENSE', 'type' => 'file'], 200) : Http::response([], 404), + "{$base}/commits/v1.0.0" => Http::response(['sha' => str_repeat('a', 40)]), "{$base}/releases/latest" => $passingChecks ? Http::response(['tag_name' => 'v1.0.0'], 200) : Http::response([], 404), @@ -566,6 +567,7 @@ public function test_preflight_detects_manually_installed_webhook(): void 'encoding' => 'base64', ]), "{$base}/contents/LICENSE*" => Http::response(['name' => 'LICENSE', 'type' => 'file'], 200), + "{$base}/commits/v1.0.0" => Http::response(['sha' => str_repeat('a', 40)]), "{$base}/releases/latest" => Http::response(['tag_name' => 'v1.0.0'], 200), "{$base}/tags*" => Http::response([['name' => 'v1.0.0']]), "{$base}/readme" => Http::response([ diff --git a/tests/Feature/PluginSyncServiceTest.php b/tests/Feature/PluginSyncServiceTest.php index 3332b6feb..5c23c5cf8 100644 --- a/tests/Feature/PluginSyncServiceTest.php +++ b/tests/Feature/PluginSyncServiceTest.php @@ -8,6 +8,7 @@ use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Http\Client\Request; use Illuminate\Support\Facades\Http; +use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Queue; use Tests\TestCase; @@ -22,6 +23,140 @@ protected function setUp(): void Queue::fake(); } + public function test_sync_reads_all_listing_content_at_the_release_commit(): void + { + $this->assertSyncUsesReleaseCommit(true); + } + + public function test_sync_uses_a_tag_commit_when_no_release_exists(): void + { + $this->assertSyncUsesReleaseCommit(false); + } + + private function assertSyncUsesReleaseCommit(bool $hasRelease): void + { + $base = 'https://api.github.com/repos/acme/test-plugin'; + $commitSha = str_repeat('a', 40); + $tag = 'release/v1.2.3'; + $files = [ + 'README.md' => '# Released documentation', + 'composer.json' => json_encode(['name' => 'acme/test-plugin', 'description' => 'Released description', 'require' => ['nativephp/mobile' => '^3.0']]), + 'nativephp.json' => json_encode(['ios' => ['min_version' => '16.0'], 'android' => ['min_version' => '28']]), + 'LICENSE.md' => 'Released license', + ]; + + Http::fake([ + "{$base}/releases/latest" => Http::response($hasRelease ? ['tag_name' => $tag, 'target_commitish' => 'main'] : [], $hasRelease ? 200 : 404), + "{$base}/tags*" => Http::response([['name' => $tag]]), + "{$base}/commits/".rawurlencode($tag) => Http::response(['sha' => $commitSha]), + "{$base}/contents/*" => function (Request $request) use ($files, $commitSha) { + $this->assertSame($commitSha, $request['ref']); + $path = basename(parse_url($request->url(), PHP_URL_PATH)); + + return Http::response(['content' => base64_encode($files[$path])]); + }, + '*' => Http::response([], 404), + ]); + + $plugin = Plugin::factory()->create(['name' => 'acme/test-plugin', 'repository_url' => 'https://github.com/acme/test-plugin']); + + $this->assertTrue((new PluginSyncService)->sync($plugin)); + $plugin->refresh(); + $this->assertStringContainsString('Released documentation', $plugin->readme_html); + $this->assertStringContainsString('Released license', $plugin->license_html); + $this->assertSame('Released description', $plugin->description); + $this->assertSame('16.0', $plugin->ios_version); + $this->assertSame('28', $plugin->android_version); + $this->assertSame('^3.0', $plugin->mobile_min_version); + $this->assertSame('release/v1.2.3', $plugin->latest_version); + } + + public function test_raw_fallback_uses_the_release_commit_and_never_reads_main(): void + { + $base = 'https://api.github.com/repos/acme/test-plugin'; + $commitSha = str_repeat('b', 40); + + Http::fake([ + "{$base}/releases/latest" => Http::response(['tag_name' => 'v1.2.3']), + "{$base}/commits/v1.2.3" => Http::response(['sha' => $commitSha]), + "https://raw.githubusercontent.com/acme/test-plugin/{$commitSha}/composer.json" => Http::response(json_encode(['name' => 'acme/test-plugin'])), + "https://raw.githubusercontent.com/acme/test-plugin/{$commitSha}/README.md" => Http::response('# Released fallback'), + '*' => Http::response([], 404), + ]); + + $plugin = Plugin::factory()->create([ + 'name' => 'acme/test-plugin', + 'repository_url' => 'https://github.com/acme/test-plugin', + 'license_html' => 'Existing license', + ]); + + $this->assertTrue((new PluginSyncService)->sync($plugin)); + $this->assertSame('1.2.3', $plugin->fresh()->latest_version); + $this->assertStringContainsString('Released fallback', $plugin->fresh()->readme_html); + $this->assertSame('Existing license', $plugin->fresh()->license_html); + Http::assertNotSent(fn (Request $request): bool => str_contains($request->url(), '/main/')); + } + + public function test_sync_leaves_listing_unchanged_when_release_commit_cannot_be_resolved(): void + { + Http::fake([ + '*/releases/latest' => Http::response(['tag_name' => 'v2.0.0']), + '*' => Http::response([], 404), + ]); + $plugin = Plugin::factory()->create([ + 'repository_url' => 'https://github.com/acme/test-plugin', + 'readme_html' => 'Existing documentation', + 'latest_version' => '1.0.0', + 'last_synced_at' => null, + ]); + + $this->assertFalse((new PluginSyncService)->sync($plugin)); + $plugin->refresh(); + $this->assertSame('Existing documentation', $plugin->readme_html); + $this->assertSame('1.0.0', $plugin->latest_version); + $this->assertNull($plugin->last_synced_at); + Http::assertNotSent(fn (Request $request): bool => str_contains($request->url(), '/contents/') || str_contains($request->url(), 'raw.githubusercontent.com')); + Queue::assertNothingPushed(); + } + + public function test_sync_does_not_read_default_branch_when_release_discovery_fails(): void + { + Log::spy(); + + Http::fake(['*' => Http::response([], 403)]); + $plugin = Plugin::factory()->create([ + 'repository_url' => 'https://github.com/acme/test-plugin', + 'readme_html' => 'Existing documentation', + 'last_synced_at' => null, + ]); + + $this->assertFalse((new PluginSyncService)->sync($plugin)); + Log::shouldHaveReceived('warning')->once()->with( + '[PluginSync] Could not discover latest release', + \Mockery::on(fn (array $context): bool => $context['plugin_id'] === $plugin->id + && str_contains($context['error'], '403')), + ); + $this->assertSame('Existing documentation', $plugin->fresh()->readme_html); + $this->assertNull($plugin->fresh()->last_synced_at); + Http::assertNotSent(fn (Request $request): bool => str_contains($request->url(), '/contents/')); + } + + public function test_sync_reads_default_branch_when_repository_has_no_tags_or_releases(): void + { + Http::fake([ + '*/releases/latest' => Http::response([], 404), + '*/tags*' => Http::response([]), + '*/contents/README.md' => Http::response(['content' => base64_encode('# Unreleased documentation')]), + '*/contents/composer.json' => Http::response(['content' => base64_encode(json_encode(['name' => 'acme/test-plugin']))]), + '*' => Http::response([], 404), + ]); + $plugin = Plugin::factory()->create(['repository_url' => 'https://github.com/acme/test-plugin']); + + $this->assertTrue((new PluginSyncService)->sync($plugin)); + $this->assertStringContainsString('Unreleased documentation', $plugin->fresh()->readme_html); + Http::assertNotSent(fn (Request $request): bool => isset($request['ref']) || str_contains($request->url(), '/commits/')); + } + public function test_sync_extracts_mobile_min_version_from_composer_data(): void { $composerJson = json_encode([