Stream downloads instead of redirecting (#145)

* feat: download service serves responses directly now * refactor: make DownloadService stream directly * tweak: set up some real caching (default 10 days) * config: use app.aspirecloud.download.base * fix: move /download routes from web.php to api.php
aspirepress · Jan 27, 2025 · 025e93e · 025e93e
1 parent 179cdc1
commit 025e93e
Show file tree

Hide file tree

Showing 14 changed files with 245 additions and 80 deletions.
diff --git a/.env.example b/.env.example
@@ -77,3 +77,6 @@ MAILGUN_SECRET=
 
 # AspirePress Related Configuration
 API_AUTHENTICATION_ENABLED=true
+DOWNLOAD_BASE=https://api.aspiredev.org/download/
+# DOWNLOAD_BASE=https://fastly.api.aspiredev.org/download/
+DOWNLOAD_CACHE_SECONDS=864000
diff --git a/app/Contracts/Downloads/Downloader.php b/app/Contracts/Downloads/Downloader.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Contracts\Downloads;
+
+use App\Enums\AssetType;
+use Symfony\Component\HttpFoundation\Response;
+
+interface Downloader
+{
+    public function download(AssetType $type, string $slug, string $file, ?string $revision = null): Response;
+}
diff --git a/app/Enums/AssetType.php b/app/Enums/AssetType.php
@@ -12,16 +12,32 @@ enum AssetType: string
     case PLUGIN_GP_ICON = 'plugin-gp-icon'; // geopattern-icon only -- other icons are treated as screenshots
     case THEME_SCREENSHOT = 'theme-screenshot';
 
-    public function isZip(): bool
-    {
-        return in_array($this, [self::CORE, self::PLUGIN, self::THEME]);
-    }
-
-    public function isAsset(): bool
+    public function isImage(): bool
     {
         return in_array(
             $this,
             [self::PLUGIN_SCREENSHOT, self::PLUGIN_BANNER, self::PLUGIN_GP_ICON, self::THEME_SCREENSHOT],
         );
     }
+
+    public function buildUpstreamUrl(string $slug, string $file, ?string $revision): string
+    {
+        $baseUrl = match ($this) {
+            self::CORE => 'https://wordpress.org/',
+            self::PLUGIN => 'https://downloads.wordpress.org/plugin/',
+            self::THEME => 'https://downloads.wordpress.org/theme/',
+            self::PLUGIN_SCREENSHOT,
+            self::PLUGIN_BANNER => "https://ps.w.org/$slug/assets/",
+            self::PLUGIN_GP_ICON => "https://s.w.org/plugins/geopattern-icon/",
+            self::THEME_SCREENSHOT => "https://ts.w.org/wp-content/themes/$slug/",
+        };
+
+        $url = $baseUrl . $file;
+
+        if ($revision && $this->isImage()) {
+            $url .= "?rev={$revision}";
+        }
+
+        return $url;
+    }
 }
diff --git a/app/Models/WpOrg/Plugin.php b/app/Models/WpOrg/Plugin.php
@@ -234,7 +234,7 @@ public static function rewriteMetadata(array $metadata): array
 
     private static function rewriteDotOrgUrl(string $url): string
     {
-        $base = config('app.url') . '/download/';
+        $base = config('app.aspirecloud.download.base');
 
         // https://downloads.wordpress.org/plugin/elementor.3.26.5.zip
         // => /download/plugin/elementor.3.26.5.zip

diff --git a/app/Models/WpOrg/Theme.php b/app/Models/WpOrg/Theme.php
@@ -182,7 +182,7 @@ public static function rewriteMetadata(array $metadata): array
             return $metadata;
         }
 
-        $base = config('app.url') . '/download/';
+        $base = config('app.aspirecloud.download.base');
         $rewrite = fn(string $url) => \Safe\preg_replace('#https?://.*?/#i', $base, $url);
 
         $download_link = $rewrite($metadata['download_link'] ?? '');

diff --git a/app/Services/Downloads/DownloadService.php b/app/Services/Downloads/DownloadService.php
@@ -2,22 +2,18 @@
 
 namespace App\Services\Downloads;
 
+use App\Contracts\Downloads\Downloader;
 use App\Enums\AssetType;
 use App\Events\AssetCacheHit;
 use App\Events\AssetCacheMissed;
 use App\Models\WpOrg\Asset;
+use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Storage;
 use Symfony\Component\HttpFoundation\Response;
 
-class DownloadService
+class DownloadService implements Downloader
 {
-    public const int TEMPORARY_URL_EXPIRE_MINS = 5;
-
-    /**
-     * Get the file download response. If the asset exists locally, return a redirect url to it.
-     * Otherwise, redirect to WordPress.org and queue a job to download it for future requests.
-     */
     public function download(AssetType $type, string $slug, string $file, ?string $revision = null): Response
     {
         Log::debug("DOWNLOAD", compact("type", "slug", "file", "revision"));
@@ -39,40 +35,19 @@ public function download(AssetType $type, string $slug, string $file, ?string $r
             // TODO: handle case where asset exists but local path does not (DownloadAssetJob always creates a new Asset)
             event(new AssetCacheHit($asset));
             Log::debug("Serving existing asset", ["asset" => $asset]);
-            return $this->response(
-                Storage::temporaryUrl($asset->local_path, now()->addMinutes(self::TEMPORARY_URL_EXPIRE_MINS)),
+            $stream = Storage::disk('s3')->getDriver()->readStream($asset->local_path);
+            return response()->stream(
+                fn() => fpassthru($stream),
+                headers: ['Content-Type' => 'application/octet-stream'],
             );
         }
 
-        $upstreamUrl = self::buildUpstreamUrl($type, $slug, $file, $revision);
+        $upstreamUrl = $type->buildUpstreamUrl($slug, $file, $revision);
 
         event(new AssetCacheMissed(type: $type, slug: $slug, file: $file, upstreamUrl: $upstreamUrl, revision: $revision));
-        return $this->response($upstreamUrl);
-    }
-
-    public static function buildUpstreamUrl(AssetType $type, string $slug, string $file, ?string $revision): string
-    {
-        $baseUrl = match ($type) {
-            AssetType::CORE => 'https://wordpress.org/',
-            AssetType::PLUGIN => 'https://downloads.wordpress.org/plugin/',
-            AssetType::THEME => 'https://downloads.wordpress.org/theme/',
-            AssetType::PLUGIN_SCREENSHOT,
-            AssetType::PLUGIN_BANNER => "https://ps.w.org/$slug/assets/",
-            AssetType::PLUGIN_GP_ICON => "https://s.w.org/plugins/geopattern-icon/",
-            AssetType::THEME_SCREENSHOT => "https://ts.w.org/wp-content/themes/$slug/",
-        };
-
-        $url = $baseUrl . $file;
 
-        if ($revision && $type->isAsset()) {
-            $url .= "?rev={$revision}";
-        }
-
-        return $url;
-    }
-
-    private function response(string $url): Response
-    {
-        return redirect()->away($url);
+        // TODO: use a real client.  Plugins are small enough we can get away with this for now.
+        $response = Http::withHeaders(['User-Agent' => 'AspireCloud'])->get($upstreamUrl);
+        return new Response($response->body(), $response->status(), $response->headers());
     }
 }
diff --git a/app/Services/Downloads/RedirectingDownloadService.php b/app/Services/Downloads/RedirectingDownloadService.php
@@ -0,0 +1,60 @@
+<?php
+
+namespace App\Services\Downloads;
+
+use App\Contracts\Downloads\Downloader;
+use App\Enums\AssetType;
+use App\Events\AssetCacheHit;
+use App\Events\AssetCacheMissed;
+use App\Models\WpOrg\Asset;
+use Illuminate\Http\RedirectResponse;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\Storage;
+
+class RedirectingDownloadService implements Downloader
+{
+    public const int TEMPORARY_URL_EXPIRE_MINS = 5;
+
+    /**
+     * Get the file download response. If the asset exists locally, return a redirect url to it.
+     * Otherwise, redirect to WordPress.org and queue a job to download it for future requests.
+     */
+    public function download(AssetType $type, string $slug, string $file, ?string $revision = null): RedirectResponse
+    {
+        Log::debug("DOWNLOAD", compact("type", "slug", "file", "revision"));
+
+        if ($revision === 'head') {
+            // head is there to have something in the url, but it behaves the same as not passing it
+            $revision = null;
+        }
+        // Check if we have it locally
+        $asset = Asset::query()
+            ->where('asset_type', $type->value)
+            ->where('slug', $slug)
+            ->where('local_path', 'LIKE', "%{$file}")
+            ->when($revision, fn($q) => $q->where('revision', $revision))
+            ->orderBy('revision', 'desc')
+            ->first();
+
+        if ($asset && Storage::exists($asset->local_path)) {
+            // TODO: handle case where asset exists but local path does not (DownloadAssetJob always creates a new Asset)
+            event(new AssetCacheHit($asset));
+            Log::debug("Serving existing asset", ["asset" => $asset]);
+            return $this->response(
+                Storage::temporaryUrl($asset->local_path, now()->addMinutes(self::TEMPORARY_URL_EXPIRE_MINS)),
+            );
+        }
+
+        $upstreamUrl = $type->buildUpstreamUrl($slug, $file, $revision);
+
+        event(
+            new AssetCacheMissed(type: $type, slug: $slug, file: $file, upstreamUrl: $upstreamUrl, revision: $revision),
+        );
+        return $this->response($upstreamUrl);
+    }
+
+    private function response(string $url): RedirectResponse
+    {
+        return redirect()->away($url);
+    }
+}
diff --git a/config/app.php b/config/app.php
@@ -29,5 +29,9 @@
 
     'aspirecloud' => [
         'api_authentication_enable' => env('API_AUTHENTICATION_ENABLED', false),
+        'download' => [
+            'base' => env('DOWNLOAD_BASE', env('APP_URL') . '/download/'), # must have a trailing slash!
+            'cache_seconds' => env('DOWNLOAD_CACHE_SECONDS', 60 * 60 * 24 * 10),
+        ],
     ],
 ];
diff --git a/phpunit.xml b/phpunit.xml
@@ -23,11 +23,13 @@
         <env name="BCRYPT_ROUNDS" value="4"/>
         <env name="CACHE_STORE" value="array"/>
         <env name="DB_CONNECTION" value="test"/>
+        <env name="DB_DATABASE" value="aspirecloud_testing"/>
         <env name="DB_HOST" value="db.aspiredev.org"/>
+        <env name="DB_PASSWORD" value="password"/>
         <env name="DB_PORT" value="5432"/>
-        <env name="DB_DATABASE" value="aspirecloud_testing"/>
         <env name="DB_USERNAME" value="postgres"/>
-        <env name="DB_PASSWORD" value="password"/>
+        <env name="LOG_CHANNEL" value="null"/>
+        <env name="LOG_LEVEL" value="debug"/>
         <env name="MAIL_MAILER" value="array"/>
         <env name="PULSE_ENABLED" value="false"/>
         <env name="QUEUE_CONNECTION" value="sync"/>

diff --git a/routes/api.php b/routes/api.php
@@ -67,3 +67,4 @@
 // Route::any('{path}', CatchAllController::class)->where('path', '.*');
 
 require __DIR__ . '/inc/admin-api.php';
+require __DIR__ . '/inc/download.php';
diff --git a/routes/inc/download.php b/routes/inc/download.php
@@ -10,8 +10,9 @@
 use Illuminate\Support\Facades\Route;
 
 $auth_middleware = config('app.aspirecloud.api_authentication_enable') ? ['auth:sanctum'] : [];
+$cache_seconds = config('app.aspirecloud.download.cache_seconds');
 $middleware = [
-    'cache.headers:public;max_age=60', // cache 302 redirects for 1 minute while we fetch it
+    "cache.headers:public;max_age=$cache_seconds", // we're streaming responses, so no etags
     ...$auth_middleware,
 ];
 

diff --git a/routes/web.php b/routes/web.php
@@ -27,4 +27,3 @@
 });
 
 require __DIR__ . '/inc/admin-web.php';
-require __DIR__ . '/inc/download.php';
diff --git a/tests/Feature/Download/DownloadRedirectsTest.php b/tests/Feature/Download/DownloadRedirectsTest.php
@@ -0,0 +1,102 @@
+<?php
+
+use App\Enums\AssetType;
+use App\Jobs\DownloadAssetJob;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\Queue;
+use Illuminate\Support\Facades\Storage;
+
+beforeEach(function () {
+    Storage::fake('local');
+    Queue::fake();
+    Http::fake();
+});
+
+
+
+describe('Download Routes', function () {
+    $getJob = function (): DownloadAssetJob {
+        $jobs = Queue::pushed(DownloadAssetJob::class);
+        expect($jobs)->toHaveCount(1);
+        return $jobs->first();
+    };
+
+    it('handles WordPress core download requests', function () use ($getJob) {
+        $response = $this->get('/download/wordpress-6.4.2.zip');
+
+        expect($response->status())->toBe(302);
+        /** @noinspection PhpUndefinedMethodInspection */
+        expect($response->getTargetUrl())->toBe('https://wordpress.org/wordpress-6.4.2.zip');
+
+        $job = $getJob();
+        expect($job->type)
+            ->toBe(AssetType::CORE)
+            ->and($job->file)->toBe('wordpress-6.4.2.zip')
+            ->and($job->slug)->toBe('wordpress')
+            ->and($job->upstreamUrl)->toBe('https://wordpress.org/wordpress-6.4.2.zip')
+            ->and($job->revision)->toBeNull();
+    });
+
+    it('handles plugin download requests', function () use ($getJob) {
+        $response = $this->get('/download/plugin/test-plugin.1.0.0.zip');
+
+        expect($response->status())->toBe(302);
+        /** @noinspection PhpUndefinedMethodInspection */
+        expect($response->getTargetUrl())->toBe('https://downloads.wordpress.org/plugin/test-plugin.1.0.0.zip');
+
+        $job = $getJob();
+        expect($job->type)
+            ->toBe(AssetType::PLUGIN)
+            ->and($job->file)->toBe('test-plugin.1.0.0.zip')
+            ->and($job->slug)->toBe('test-plugin')
+            ->and($job->upstreamUrl)->toBe('https://downloads.wordpress.org/plugin/test-plugin.1.0.0.zip')
+            ->and($job->revision)->toBeNull();
+    });
+
+    it('handles theme download requests', function () use ($getJob) {
+        $response = $this->get('/download/theme/test-theme.1.0.0.zip');
+
+        expect($response->status())->toBe(302);
+        /** @noinspection PhpUndefinedMethodInspection */
+        expect($response->getTargetUrl())->toBe('https://downloads.wordpress.org/theme/test-theme.1.0.0.zip');
+
+        $job = $getJob();
+        expect($job->type)
+            ->toBe(AssetType::THEME)
+            ->and($job->file)->toBe('test-theme.1.0.0.zip')
+            ->and($job->slug)->toBe('test-theme')
+            ->and($job->upstreamUrl)->toBe('https://downloads.wordpress.org/theme/test-theme.1.0.0.zip')
+            ->and($job->revision)->toBeNull();
+    });
+
+    it('handles plugin asset download requests', function () use ($getJob) {
+        $response = $this->get('/download/assets/plugin/test-plugin/head/screenshot-1.png');
+        expect($response->status())->toBe(302);
+        /** @noinspection PhpUndefinedMethodInspection */
+        expect($response->getTargetUrl())->toBe('https://ps.w.org/test-plugin/assets/screenshot-1.png');
+
+        $job = $getJob();
+        expect($job->type)
+            ->toBe(AssetType::PLUGIN_SCREENSHOT)
+            ->and($job->file)->toBe('screenshot-1.png')
+            ->and($job->slug)->toBe('test-plugin')
+            ->and($job->upstreamUrl)->toBe('https://ps.w.org/test-plugin/assets/screenshot-1.png')
+            ->and($job->revision)->toBeNull();
+    });
+
+    it('handles asset download requests with revision', function () use ($getJob) {
+        $response = $this->get('/download/assets/plugin/test-plugin/3164133/banner-1544x500.png');
+
+        expect($response->status())->toBe(302);
+        /** @noinspection PhpUndefinedMethodInspection */
+        expect($response->getTargetUrl())->toBe('https://ps.w.org/test-plugin/assets/banner-1544x500.png?rev=3164133');
+
+        $job = $getJob();
+        expect($job->type)
+            ->toBe(AssetType::PLUGIN_BANNER)
+            ->and($job->file)->toBe('banner-1544x500.png')
+            ->and($job->slug)->toBe('test-plugin')
+            ->and($job->upstreamUrl)->toBe('https://ps.w.org/test-plugin/assets/banner-1544x500.png?rev=3164133')
+            ->and($job->revision)->toBe('3164133');
+    });
+})->todo("turn these into tests of AssetType::buildUpstreamUrl");
Original file line number	Diff line number	Diff line change
Expand Up		@@ -67,3 +67,4 @@
		// Route::any('{path}', CatchAllController::class)->where('path', '.*');

		require __DIR__ . '/inc/admin-api.php';
		require __DIR__ . '/inc/download.php';
Original file line number	Diff line number	Diff line change
Expand Up		@@ -27,4 +27,3 @@
		});

		require __DIR__ . '/inc/admin-web.php';
		require __DIR__ . '/inc/download.php';