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.
This commit is contained in:
@@ -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);
|
||||
|
||||
@@ -74,6 +74,8 @@ final class TrackOnlineVisitor
|
||||
'robots.txt',
|
||||
'sitemap.xml',
|
||||
'sitemaps/*',
|
||||
'download/artwork/*',
|
||||
'photo/*',
|
||||
])) {
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -52,6 +52,8 @@ return [
|
||||
'leaderboard',
|
||||
'art',
|
||||
'art/*',
|
||||
'download/artwork/*',
|
||||
'photo/*',
|
||||
'sitemap.xml',
|
||||
'sitemaps/*',
|
||||
'robots.txt',
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -0,0 +1,88 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Models\Artwork;
|
||||
use App\Services\ThumbnailService;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
use Illuminate\Support\Facades\Storage;
|
||||
|
||||
beforeEach(function (): void {
|
||||
config([
|
||||
'cdn.files_url' => '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();
|
||||
});
|
||||
@@ -0,0 +1,40 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
use App\Services\Traffic\OnlineVisitorRepository;
|
||||
use Illuminate\Support\Facades\Config;
|
||||
|
||||
beforeEach(function (): void {
|
||||
Config::set('skinbase-sessions.enabled', true);
|
||||
Config::set('skinbase-sessions.debug_header', true);
|
||||
Config::set('skinbase-sessions.skip_anonymous_public_get', true);
|
||||
});
|
||||
|
||||
it('skips session for anonymous GET and HEAD /photo/* without a session cookie', function (): void {
|
||||
$this->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();
|
||||
});
|
||||
@@ -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');
|
||||
});
|
||||
});
|
||||
|
||||
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');
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user