Merge pull request #251 from tighten/feat/throw-exception-if-packagis…

…t-fails Test the `stats:fetch` command, and report if Packagist download count fails
tighten · Nov 10, 2023 · 238b050 · 238b050
2 parents 275d20a + 27adb0c
commit 238b050
Show file tree

Hide file tree

Showing 3 changed files with 136 additions and 2 deletions.
diff --git a/app/Console/Commands/FetchProjectStats.php b/app/Console/Commands/FetchProjectStats.php
@@ -54,8 +54,11 @@ public function fetchOneProject(Project $project)
 
         // Fetch download counts (if applicable)
         $packagist = Package::fromProject($project);
-        $project->downloads_total = $packagist->totalDownloads;
-        $project->downloads_last_30_days = $packagist->monthlyDownloads;
+
+        if ($packagist->requestOk) {
+            $project->downloads_total = $packagist->totalDownloads;
+            $project->downloads_last_30_days = $packagist->monthlyDownloads;
+        }
 
         $project->is_hidden = $githubProject->isArchived() || $project->is_hidden;
 

diff --git a/app/Remotes/Packagist/Package.php b/app/Remotes/Packagist/Package.php
@@ -14,6 +14,8 @@ class Package
 
     public $totalDownloads = 0;
 
+    public $requestOk = false;
+
     protected $url;
 
     public function __construct($namespace, $name)
@@ -32,7 +34,13 @@ protected function fetchDownloads()
     {
         $response = Http::get($this->url);
 
+        report_if(
+            $response->serverError(),
+            "HTTP Error: Was unable to fetch from packagist {$this->url}"
+        );
+
         if ($response->ok()) {
+            $this->requestOk = true;
             $this->downloadsData = Arr::get($response->json(), 'package.downloads', 0);
             $this->monthlyDownloads = $this->downloadsData['monthly'];
             $this->totalDownloads = $this->downloadsData['total'];

diff --git a/tests/Feature/FetchProjectStatsTest.php b/tests/Feature/FetchProjectStatsTest.php
@@ -0,0 +1,123 @@
+<?php
+
+namespace Tests\Feature;
+
+use App\Models\Project;
+use Github\Api\Issue;
+use Github\Api\PullRequest;
+use Github\Api\Repo;
+use GrahamCampbell\GitHub\Facades\GitHub as GitHubClient;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Cache;
+use Illuminate\Support\Facades\Http;
+use Mockery;
+use Tests\TestCase;
+
+class FetchProjectStatsTest extends TestCase
+{
+    use RefreshDatabase;
+
+    /** @test */
+    public function fetch_stats_and_persist_successfully(): void
+    {
+        Http::preventStrayRequests();
+
+        Http::fake([
+            'packagist.org/packages/tighten/existing_package.json' => Http::response([
+                'package' => [
+                    'downloads' => [
+                        'total' => 1000,
+                        'monthly' => 100,
+                    ],
+                ],
+            ]),
+        ]);
+
+        $this->mockGithubClient('tighten', 'existing_package');
+
+        $project = Project::factory()->create([
+            'namespace' => 'tighten',
+            'name' => 'existing_package',
+            'packagist_name' => null,
+        ]);
+
+        $this->artisan('stats:fetch')->assertSuccessful();
+        $project->refresh();
+
+        $this->assertEquals($project->downloads_total, 1000);
+        $this->assertEquals($project->downloads_last_30_days, 100);
+
+        $this->assertEquals($project->issues_count, 0);
+        $this->assertEquals($project->pull_requests_count, 0);
+        $this->assertEquals($project->issues, collect([]));
+        $this->assertEquals($project->pull_requests, collect([]));
+
+        $this->assertEquals($project->is_hidden, false);
+        $this->assertNotEmpty(Cache::get('projects'));
+    }
+
+    /** @test */
+    public function failed_stats_fetch_does_not_overwrite_packagist_stats(): void
+    {
+        Http::preventStrayRequests();
+
+        Http::fake([
+            'packagist.org/packages/tighten/404_package.json' => Http::response([], 404),
+        ]);
+
+        $this->mockGithubClient('tighten', '404_package');
+
+        $project = Project::factory()->create([
+            'namespace' => 'tighten',
+            'name' => '404_package',
+            'packagist_name' => null,
+            'downloads_total' => 100,
+            'downloads_last_30_days' => 10,
+        ]);
+
+        $this->artisan('stats:fetch')->assertSuccessful();
+
+        $project->refresh();
+
+        $this->assertEquals($project->downloads_total, 100);
+        $this->assertEquals($project->downloads_last_30_days, 10);
+
+        $this->assertEquals($project->issues_count, 0);
+        $this->assertEquals($project->pull_requests_count, 0);
+        $this->assertEquals($project->issues, collect([]));
+        $this->assertEquals($project->pull_requests, collect([]));
+
+        $this->assertEquals($project->is_hidden, false);
+        $this->assertNotEmpty(Cache::get('projects'));
+    }
+
+    public function mockGithubClient($namespace, $repo): void
+    {
+        $issuesMock = Mockery::mock(Issue::class);
+        $issuesMock->shouldReceive('all')
+            ->with($namespace, $repo)
+            ->once()
+            ->andReturn([]);
+        GitHubClient::shouldReceive('issues')
+            ->once()
+            ->andReturn($issuesMock);
+
+        $pullRequestsMock = Mockery::mock(PullRequest::class);
+        $pullRequestsMock->shouldReceive('all')
+            ->with($namespace, $repo)
+            ->once()
+            ->andReturn([]);
+        GitHubClient::shouldReceive('pullRequests')
+            ->once()
+            ->andReturn($pullRequestsMock);
+
+        $reposMock = Mockery::mock(Repo::class);
+        $reposMock->shouldReceive('show')
+            ->with($namespace, $repo)
+            ->once()
+            ->andReturn(['archived' => false]);
+        GitHubClient::shouldReceive('repo')
+            ->once()
+            ->andReturn($reposMock);
+    }
+}