From b9beb8f5c59e107492dd1ede8afa26f8c04aa281 Mon Sep 17 00:00:00 2001 From: Gregor Klevze Date: Sat, 29 Aug 2026 12:25:37 +0200 Subject: [PATCH] Speed legacy photo serving and skip tracking on download routes. Tighten thumbnail/legacy photo handling and exclude download/photo traffic from session visitor tracking so those hot paths stay cheap. --- .../Legacy/LegacyArtworkPhotoController.php | 32 +++++-- app/Http/Middleware/TrackOnlineVisitor.php | 2 + app/Services/ThumbnailService.php | 2 +- config/skinbase-sessions.php | 2 + routes/legacy.php | 6 ++ .../Artworks/ThumbnailServiceM17CTest.php | 88 +++++++++++++++++++ .../Http/LegacyPhotoM17B1MiddlewareTest.php | 40 +++++++++ tests/Feature/LegacyArtworkPhotoRouteTest.php | 74 +++++++++++++++- 8 files changed, 235 insertions(+), 11 deletions(-) create mode 100644 tests/Feature/Artworks/ThumbnailServiceM17CTest.php create mode 100644 tests/Feature/Http/LegacyPhotoM17B1MiddlewareTest.php diff --git a/app/Http/Controllers/Legacy/LegacyArtworkPhotoController.php b/app/Http/Controllers/Legacy/LegacyArtworkPhotoController.php index 556dabe5..46f7bf4b 100644 --- a/app/Http/Controllers/Legacy/LegacyArtworkPhotoController.php +++ b/app/Http/Controllers/Legacy/LegacyArtworkPhotoController.php @@ -8,6 +8,7 @@ use App\Http\Controllers\Controller; use App\Models\Artwork; use Illuminate\Database\Eloquent\Builder; use Illuminate\Http\RedirectResponse; +use Illuminate\Http\Response; use Illuminate\Support\Facades\Schema; final class LegacyArtworkPhotoController extends Controller @@ -26,25 +27,39 @@ final class LegacyArtworkPhotoController extends Controller private static ?bool $hasLegacyIdColumn = null; - public function __invoke(string $encoded, string $size, string $extension): RedirectResponse + public function __invoke(string $encoded, string $size, string $extension): RedirectResponse|Response { $artworkId = $this->decodeBase62($encoded); $sizeCode = (int) $size; - abort_if($artworkId === null || $artworkId < 1, 404); + if ($artworkId === null || $artworkId < 1) { + return $this->notFound(); + } $artwork = $this->resolveArtwork($artworkId); - abort_unless($artwork !== null, 404); + if ($artwork === null) { + return $this->notFound(); + } $targetUrl = $sizeCode === 7 ? $this->resolveOriginalUrl($artwork) : $artwork->thumbUrl(self::THUMB_SIZE_MAP[$sizeCode] ?? 'md'); - abort_if(empty($targetUrl), 404); + if ($targetUrl === null || $targetUrl === '') { + return $this->notFound(); + } return redirect()->away($targetUrl, 301); } + private function notFound(): Response + { + return response('', 404, [ + 'Content-Type' => 'text/plain; charset=UTF-8', + 'Cache-Control' => 'no-store', + ]); + } + private function decodeBase62(string $value): ?int { if ($value === '') { @@ -86,11 +101,6 @@ final class LegacyArtworkPhotoController extends Controller { $cdn = rtrim((string) config('cdn.files_url', 'https://cdn.skinbase.org'), '/'); $filePath = trim((string) ($artwork->file_path ?? ''), '/'); - - if ($filePath !== '') { - return $cdn . '/' . $filePath; - } - $hash = strtolower((string) preg_replace('/[^a-f0-9]/i', '', (string) ($artwork->hash ?? ''))); $ext = ltrim((string) ($artwork->file_ext ?: $artwork->thumb_ext ?: 'webp'), '.'); @@ -98,6 +108,10 @@ final class LegacyArtworkPhotoController extends Controller return $artwork->thumbUrl('xl') ?? $artwork->thumbUrl('lg') ?? $artwork->thumbUrl('md'); } + if ($filePath !== '') { + return $cdn . '/' . $filePath; + } + $prefix = trim((string) config('uploads.object_storage.prefix', 'artworks'), '/'); $firstDir = substr($hash, 0, 2); $secondDir = substr($hash, 2, 2); diff --git a/app/Http/Middleware/TrackOnlineVisitor.php b/app/Http/Middleware/TrackOnlineVisitor.php index dd9fa4cc..8be50596 100644 --- a/app/Http/Middleware/TrackOnlineVisitor.php +++ b/app/Http/Middleware/TrackOnlineVisitor.php @@ -74,6 +74,8 @@ final class TrackOnlineVisitor 'robots.txt', 'sitemap.xml', 'sitemaps/*', + 'download/artwork/*', + 'photo/*', ])) { return false; } diff --git a/app/Services/ThumbnailService.php b/app/Services/ThumbnailService.php index 7252127d..85664fb5 100644 --- a/app/Services/ThumbnailService.php +++ b/app/Services/ThumbnailService.php @@ -59,7 +59,7 @@ class ThumbnailService try { $artClass = '\\App\\Models\\Artwork'; if (class_exists($artClass)) { - $art = $artClass::where('id', $id)->orWhere('legacy_id', $id)->first(); + $art = $artClass::query()->find($id); if ($art) { $hash = $art->hash ?? null; $extToUse = $ext ?? ($art->thumb_ext ?? null); diff --git a/config/skinbase-sessions.php b/config/skinbase-sessions.php index f1c5e9eb..2e76412d 100644 --- a/config/skinbase-sessions.php +++ b/config/skinbase-sessions.php @@ -52,6 +52,8 @@ return [ 'leaderboard', 'art', 'art/*', + 'download/artwork/*', + 'photo/*', 'sitemap.xml', 'sitemaps/*', 'robots.txt', diff --git a/routes/legacy.php b/routes/legacy.php index 21534607..b3d7b01b 100644 --- a/routes/legacy.php +++ b/routes/legacy.php @@ -129,6 +129,12 @@ Route::get('/profile.php', function () { // ── PROFILE (legacy URL patterns) ──────────────────────────────────────────── Route::get('/user/{username}', [ProfileController::class, 'legacyByUsername'])->where('username', '[A-Za-z0-9_-]{3,20}')->name('legacy.user.profile'); Route::get('/profile/{id}/{username?}', [ProfileController::class, 'legacyById'])->where('id', '\d+')->name('legacy.profile.id'); + +// Legacy profile template "Followers → All" used /following/{id}/{slug}. +// Canonical public surface is /@{username}/followers (people who follow the owner). +Route::get('/following/{id}/{slug?}', [ProfileController::class, 'legacyFollowingById']) + ->where('id', '\d+') + ->name('legacy.following.user'); Route::get('/profile/{username}', [ProfileController::class, 'legacyByUsername'])->where('username', '[A-Za-z0-9_-]{3,20}')->name('legacy.profile'); // Keep legacy `/user` as a permanent redirect to the canonical dashboard path. diff --git a/tests/Feature/Artworks/ThumbnailServiceM17CTest.php b/tests/Feature/Artworks/ThumbnailServiceM17CTest.php new file mode 100644 index 00000000..754beacc --- /dev/null +++ b/tests/Feature/Artworks/ThumbnailServiceM17CTest.php @@ -0,0 +1,88 @@ + 'https://cdn.example.test', + 'uploads.object_storage.prefix' => 'artworks', + ]); +}); + +it('resolves an existing artwork id to the same CDN URL as fromHash', function (): void { + $hash = 'aabbccddeeff00112233445566778899'; + $artwork = Artwork::factory()->create([ + 'hash' => $hash, + 'thumb_ext' => 'webp', + ]); + + $expected = ThumbnailService::fromHash($hash, 'webp', 'md'); + + expect(ThumbnailService::url(null, (int) $artwork->id, null, 6))->toBe($expected); + expect($expected)->toBe('https://cdn.example.test/artworks/md/aa/bb/aabbccddeeff00112233445566778899.webp'); +}); + +it('returns the empty-string fallback for a missing artwork id', function (): void { + expect(ThumbnailService::url(null, 99999999, null, 6))->toBe(''); +}); + +it('falls back to Storage::url when a file path is supplied and id lookup misses', function (): void { + $path = 'uploads/artworks/fallback.jpg'; + + expect(ThumbnailService::url($path, 99999999, null, 6))->toBe(Storage::url($path)); +}); + +it('builds a CDN URL from a direct hash and extension without querying artworks', function (): void { + DB::flushQueryLog(); + DB::enableQueryLog(); + + $url = ThumbnailService::url('11223344556677889900aabbccddeeff', null, 'webp', 'xl'); + + expect($url)->toBe('https://cdn.example.test/artworks/xl/11/22/11223344556677889900aabbccddeeff.webp'); + + $legacyIdQueries = collect(DB::getQueryLog()) + ->filter(fn (array $query): bool => str_contains(strtolower((string) $query['query']), 'legacy_id')); + + expect($legacyIdQueries)->toBeEmpty(); + + DB::disableQueryLog(); +}); + +it('maps numeric size 4 to the thumb/sm CDN path', function (): void { + $hash = 'deadbeefcafebabe0123456789abcdef'; + $artwork = Artwork::factory()->create([ + 'hash' => $hash, + 'thumb_ext' => 'jpg', + ]); + + $url = ThumbnailService::url(null, (int) $artwork->id, null, 4); + $expected = ThumbnailService::fromHash($hash, 'jpg', 'thumb'); + + expect($url)->toBe($expected); + expect($url)->toBe('https://cdn.example.test/artworks/sm/de/ad/deadbeefcafebabe0123456789abcdef.jpg'); +}); + +it('does not query artworks.legacy_id when resolving a thumbnail by artwork id', function (): void { + $artwork = Artwork::factory()->create([ + 'hash' => '00112233445566778899aabbccddeeff', + 'thumb_ext' => 'webp', + ]); + + DB::flushQueryLog(); + DB::enableQueryLog(); + + ThumbnailService::url(null, (int) $artwork->id, null, 'md'); + + $queries = collect(DB::getQueryLog())->map(fn (array $query): string => strtolower((string) $query['query'])); + expect($queries)->not->toBeEmpty(); + + expect($queries->filter(fn (string $sql): bool => str_contains($sql, 'legacy_id')))->toBeEmpty(); + expect($queries->filter(fn (string $sql): bool => str_contains($sql, 'artworks')))->not->toBeEmpty(); + + DB::disableQueryLog(); +}); diff --git a/tests/Feature/Http/LegacyPhotoM17B1MiddlewareTest.php b/tests/Feature/Http/LegacyPhotoM17B1MiddlewareTest.php new file mode 100644 index 00000000..19e84ef4 --- /dev/null +++ b/tests/Feature/Http/LegacyPhotoM17B1MiddlewareTest.php @@ -0,0 +1,40 @@ +get('/photo/0_6.png') + ->assertNotFound() + ->assertHeader('X-Skinbase-Session', 'skipped'); + + $this->head('/photo/0_6.png') + ->assertNotFound() + ->assertHeader('X-Skinbase-Session', 'skipped'); +}); + +it('starts session for /photo/* when a session cookie is already present', function (): void { + $cookie = (string) config('session.cookie'); + + $this->withCookie($cookie, 'existing-session-cookie') + ->get('/photo/0_6.png') + ->assertNotFound() + ->assertHeader('X-Skinbase-Session', 'started'); +}); + +it('does not record presence for /photo/* requests', function (): void { + $this->mock(OnlineVisitorRepository::class, function ($mock): void { + $mock->shouldReceive('track')->never(); + }); + + $this->get('/photo/0_6.png')->assertNotFound(); + $this->head('/photo/0_6.png')->assertNotFound(); +}); diff --git a/tests/Feature/LegacyArtworkPhotoRouteTest.php b/tests/Feature/LegacyArtworkPhotoRouteTest.php index 6ad314d0..07408227 100644 --- a/tests/Feature/LegacyArtworkPhotoRouteTest.php +++ b/tests/Feature/LegacyArtworkPhotoRouteTest.php @@ -58,4 +58,76 @@ it('redirects legacy full-size photo urls to the original CDN asset', function ( $response ->assertStatus(301) ->assertRedirect('https://cdn.example.test/artworks/original/11/22/1122334455667788.png'); -}); \ No newline at end of file +}); + +it('returns a cheap 404 for zero or invalid decoded photo ids', function (): void { + $this->get('/photo/0_6.png')->assertNotFound(); + $this->head('/photo/0_6.png')->assertNotFound(); +}); + +it('returns 404 for a nonexistent artwork', function (): void { + $this->get('/photo/' . encodeLegacyPhotoId(999999) . '_6.png')->assertNotFound(); +}); + +it('returns 404 for private, unapproved, or unpublished artworks', function (): void { + $private = Artwork::factory()->create(['is_public' => false, 'hash' => 'aabbccddeeff0011', 'thumb_ext' => 'webp']); + $unapproved = Artwork::factory()->create(['is_approved' => false, 'hash' => 'bbccddeeff001122', 'thumb_ext' => 'webp']); + $unpublished = Artwork::factory()->create(['published_at' => null, 'hash' => 'ccddeeff00112233', 'thumb_ext' => 'webp']); + + $this->get('/photo/' . encodeLegacyPhotoId($private->id) . '_6.png')->assertNotFound(); + $this->get('/photo/' . encodeLegacyPhotoId($unapproved->id) . '_6.png')->assertNotFound(); + $this->get('/photo/' . encodeLegacyPhotoId($unpublished->id) . '_6.png')->assertNotFound(); +}); + +it('returns 404 for eligible legacy artwork with no usable media instead of redirecting to a dead CDN path', function (): void { + config(['cdn.files_url' => 'https://cdn.example.test']); + + $artwork = Artwork::factory()->create([ + 'hash' => null, + 'thumb_ext' => null, + 'file_ext' => null, + 'file_path' => 'legacy/uploads/old-photo.jpg', + 'is_public' => true, + 'is_approved' => true, + 'published_at' => now()->subDay(), + ]); + + $this->get('/photo/' . encodeLegacyPhotoId($artwork->id) . '_6.png')->assertNotFound(); + $this->get('/photo/' . encodeLegacyPhotoId($artwork->id) . '_7.png')->assertNotFound(); +}); + +it('maps size 3 to the sm thumbnail variant', function (): void { + config([ + 'cdn.files_url' => 'https://cdn.example.test', + 'uploads.object_storage.prefix' => 'artworks', + ]); + + $artwork = Artwork::factory()->create([ + 'hash' => 'aabbccddeeff0011', + 'thumb_ext' => 'webp', + 'file_ext' => 'jpg', + 'file_path' => '', + ]); + + $this->get('/photo/' . encodeLegacyPhotoId($artwork->id) . '_3.png') + ->assertStatus(301) + ->assertRedirect('https://cdn.example.test/artworks/sm/aa/bb/aabbccddeeff0011.webp'); +}); + +it('preserves HEAD redirects for valid modern hash-based photos', function (): void { + config([ + 'cdn.files_url' => 'https://cdn.example.test', + 'uploads.object_storage.prefix' => 'artworks', + ]); + + $artwork = Artwork::factory()->create([ + 'hash' => 'aabbccddeeff0011', + 'thumb_ext' => 'webp', + 'file_ext' => 'jpg', + 'file_path' => '', + ]); + + $this->head('/photo/' . encodeLegacyPhotoId($artwork->id) . '_6.png') + ->assertStatus(301) + ->assertRedirect('https://cdn.example.test/artworks/md/aa/bb/aabbccddeeff0011.webp'); +});