From 455b514d50128319140c7c2d5fa559b69c5e6dd7 Mon Sep 17 00:00:00 2001 From: test Date: Sun, 20 Sep 2026 14:49:19 +0200 Subject: [PATCH] Track original artwork file availability more accurately. Resolve local and object-storage originals, persist download status, and audit missing files without guessing from a single path. --- .../AuditArtworkDownloadFilesCommand.php | 86 ++++++++-- .../Controllers/ArtworkDownloadController.php | 83 ++++++++-- app/Services/ArtworkOriginalFileLocator.php | 133 ++++++++++++++- ...ownload_availability_to_artworks_table.php | 25 +++ tests/Feature/ArtworkDownloadTest.php | 84 +++++++++- .../ArtworkOriginalFileLocatorTest.php | 153 ++++++++++++++++++ .../AuditArtworkDownloadFilesCommandTest.php | 73 +++++++++ 7 files changed, 599 insertions(+), 38 deletions(-) create mode 100644 database/migrations/2026_09_05_000001_add_download_availability_to_artworks_table.php create mode 100644 tests/Feature/ArtworkOriginalFileLocatorTest.php create mode 100644 tests/Feature/AuditArtworkDownloadFilesCommandTest.php diff --git a/app/Console/Commands/AuditArtworkDownloadFilesCommand.php b/app/Console/Commands/AuditArtworkDownloadFilesCommand.php index 9e5d3a82..523e0cda 100644 --- a/app/Console/Commands/AuditArtworkDownloadFilesCommand.php +++ b/app/Console/Commands/AuditArtworkDownloadFilesCommand.php @@ -18,6 +18,9 @@ final class AuditArtworkDownloadFilesCommand extends Command {--id= : Audit only this artwork ID} {--limit= : Stop after processing this many artworks} {--chunk=500 : Number of artworks to scan per batch} + {--status= : Only inspect records with this persisted download status} + {--missing-only : Only report unresolved records} + {--mark-status : Persist the resolved availability status (explicit write)} {--restore-missing : Copy missing local originals from object storage when available}'; protected $description = 'Scan artworks in descending ID order and report missing local download files with full URLs.'; @@ -28,6 +31,8 @@ final class AuditArtworkDownloadFilesCommand extends Command $limit = $this->option('limit') !== null ? max(1, (int) $this->option('limit')) : null; $chunkSize = max(1, min((int) $this->option('chunk'), 2000)); $restoreMissing = (bool) $this->option('restore-missing'); + $markStatus = (bool) $this->option('mark-status'); + $statusFilter = $this->option('status'); $this->info(sprintf( 'Starting download file audit. order=desc include_trashed=yes chunk=%d limit=%s restore_missing=%s', @@ -41,10 +46,11 @@ final class AuditArtworkDownloadFilesCommand extends Command $unresolved = 0; $restored = 0; $restoreFailed = 0; + $classifications = []; $lastSeenId = null; do { - $artworks = $this->nextChunk($artworkId, $chunkSize, $lastSeenId); + $artworks = $this->nextChunk($artworkId, $chunkSize, $lastSeenId, is_string($statusFilter) ? $statusFilter : null); if ($artworks->isEmpty()) { break; } @@ -54,41 +60,59 @@ final class AuditArtworkDownloadFilesCommand extends Command break 2; } - $localPath = $locator->resolveLocalPath($artwork); - $missingReason = null; - - if ($localPath === '') { - $missingReason = 'unresolved_local_path'; + $resolution = $locator->resolve($artwork); + $classification = match ($resolution['status']) { + 'available' => strtoupper((string) $resolution['source']).'_OK', + 'pending' => 'PENDING', + 'unknown' => 'AMBIGUOUS', + default => 'MISSING_EVERYWHERE', + }; + $classifications[$classification] = ($classifications[$classification] ?? 0) + 1; + $localPath = (string) $resolution['path']; + $missingReason = $resolution['status'] === 'missing' ? 'missing_everywhere' : null; + if ($missingReason !== null) { $unresolved++; - } elseif (! File::isFile($localPath)) { - $missingReason = 'missing_local_file'; + } + + if ($markStatus) { + $this->persistStatus($artwork, $resolution); + } + + if ($missingReason === null && (bool) $this->option('missing-only')) { + $processed++; + + continue; + } + + if ($missingReason === null) { + $this->line(sprintf('Artwork %d %s source=%s', (int) $artwork->id, $classification, (string) $resolution['source'])); } if ($missingReason !== null) { - $objectPath = $locator->resolveObjectPath($artwork); + $objectPath = (string) $resolution['object_key']; $objectUrl = $locator->resolveObjectUrl($artwork); $missing++; $this->warn(sprintf('Artwork %d %s', (int) $artwork->id, $missingReason)); - $this->line(' artwork_url: ' . route('art.show', [ + $this->line(' artwork_url: '.route('art.show', [ 'id' => (int) $artwork->id, 'slug' => (string) ($artwork->slug ?? ''), ])); - $this->line(' download_url: ' . route('art.download', ['id' => (int) $artwork->id])); + $this->line(' download_url: '.route('art.download', ['id' => (int) $artwork->id])); if ($objectPath !== '') { - $this->line(' object_path: ' . $objectPath); + $this->line(' object_path: '.$objectPath); } if ($objectUrl !== null && $objectUrl !== '') { - $this->line(' object_url: ' . $objectUrl); + $this->line(' object_url: '.$objectUrl); } if ($localPath !== '') { - $this->line(' local_path: ' . $localPath); + $this->line(' local_path: '.$localPath); } - if ($restoreMissing && $missingReason === 'missing_local_file' && $localPath !== '') { + if ($restoreMissing && $missingReason === 'missing_everywhere' && $localPath !== '') { $restoreResult = $this->restoreLocalFile($storage, $objectPath, $localPath); if ($restoreResult === 'restored') { @@ -120,6 +144,9 @@ final class AuditArtworkDownloadFilesCommand extends Command $restored, $restoreFailed, )); + foreach ($classifications as $classification => $count) { + $this->line(sprintf('classification=%s count=%d', $classification, $count)); + } return self::SUCCESS; } @@ -127,11 +154,15 @@ final class AuditArtworkDownloadFilesCommand extends Command /** * @return Collection */ - private function nextChunk(?int $artworkId, int $chunkSize, ?int $lastSeenId): Collection + private function nextChunk(?int $artworkId, int $chunkSize, ?int $lastSeenId, ?string $status): Collection { + if ($artworkId !== null && $lastSeenId !== null) { + return collect(); + } + $query = Artwork::query() ->withTrashed() - ->select(['id', 'slug', 'file_path', 'hash', 'file_ext']) + ->select(['id', 'slug', 'file_name', 'file_path', 'hash', 'file_ext']) ->orderByDesc('id'); if ($artworkId !== null) { @@ -140,9 +171,30 @@ final class AuditArtworkDownloadFilesCommand extends Command $query->where('id', '<', $lastSeenId); } + if ($status !== null && $status !== '') { + $query->where('download_status', $status); + } + return $query->limit($chunkSize)->get(); } + /** @param array{status:string,source:?string} $resolution */ + private function persistStatus(Artwork $artwork, array $resolution): void + { + $status = match ($resolution['status']) { + 'available' => 'available', + 'pending' => 'pending', + 'unknown' => 'unknown', + default => 'missing', + }; + + $artwork->forceFill([ + 'download_status' => $status, + 'download_source' => $resolution['source'], + 'download_checked_at' => now(), + ])->saveQuietly(); + } + private function restoreLocalFile(UploadStorageService $storage, string $objectPath, string $localPath): string { if ($objectPath === '') { diff --git a/app/Http/Controllers/ArtworkDownloadController.php b/app/Http/Controllers/ArtworkDownloadController.php index 41069b28..27884075 100644 --- a/app/Http/Controllers/ArtworkDownloadController.php +++ b/app/Http/Controllers/ArtworkDownloadController.php @@ -8,11 +8,13 @@ use App\Models\Artwork; use App\Models\ArtworkDownload; use App\Services\ArtworkOriginalFileLocator; use App\Services\ArtworkStatsService; +use Illuminate\Http\RedirectResponse; use Illuminate\Http\Request; use Illuminate\Http\Response; use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Schema; +use Illuminate\Support\Facades\Storage; use Illuminate\Support\Str; use Symfony\Component\HttpFoundation\BinaryFileResponse; @@ -43,7 +45,7 @@ final class ArtworkDownloadController extends Controller private readonly ArtworkOriginalFileLocator $originalFiles, ) {} - public function __invoke(Request $request, int $id): BinaryFileResponse|Response + public function __invoke(Request $request, int $id): BinaryFileResponse|RedirectResponse|Response { $artwork = Artwork::query()->find($id); @@ -51,23 +53,70 @@ final class ArtworkDownloadController extends Controller return $this->notFound(); } - $filePath = $this->originalFiles->resolveLocalPath($artwork); - $ext = strtolower(ltrim((string) pathinfo($filePath, PATHINFO_EXTENSION), '.')); + $resolution = $this->originalFiles->resolve($artwork); + $filePath = (string) $resolution['path']; + $objectKey = (string) $resolution['object_key']; + $ext = strtolower((string) ($resolution['file_ext'] ?: pathinfo($filePath !== '' ? $filePath : $objectKey, PATHINFO_EXTENSION))); - if ($filePath === '' || ! in_array($ext, self::ALLOWED_EXTENSIONS, true)) { + if (($filePath === '' && $objectKey === '') || ! in_array($ext, self::ALLOWED_EXTENSIONS, true)) { return $this->notFound(); } - if (! File::isFile($filePath)) { + if (! $resolution['exists']) { Log::warning('Artwork original file missing for download.', [ 'artwork_id' => $artwork->id, 'ext' => $ext, - 'resolved_path' => $filePath, + 'download_status' => $artwork->download_status, + 'canonical_local_exists' => $filePath !== '' && File::isFile($filePath), + 'object_exists' => false, + 'resolution_status' => $resolution['status'], ]); return $this->notFound(); } + $downloadName = $this->buildDownloadFilename((string) $artwork->file_name, $ext); + $temporaryUrl = null; + + if ($resolution['source'] === 'object') { + try { + $temporaryUrl = Storage::disk((string) $resolution['disk'])->temporaryUrl( + $objectKey, + now()->addMinutes(5), + [ + 'ResponseContentDisposition' => 'attachment; filename="'.addslashes($downloadName).'"', + ], + ); + } catch (\Throwable $exception) { + Log::warning('Artwork object temporary URL generation failed.', [ + 'artwork_id' => $artwork->id, + 'disk' => $resolution['disk'], + 'object_key' => $objectKey, + 'object_exists' => true, + 'object_size' => $resolution['size'], + 'exception_class' => $exception::class, + 'exception_message' => mb_substr($exception->getMessage(), 0, 300), + ]); + + return $this->notFound(); + } + + if (! is_string($temporaryUrl) || trim($temporaryUrl) === '') { + Log::warning('Artwork object temporary URL generation failed.', [ + 'artwork_id' => $artwork->id, + 'disk' => $resolution['disk'], + 'object_key' => $objectKey, + 'resolution_status' => $resolution['status'], + 'object_exists' => true, + 'object_size' => $resolution['size'], + 'exception_class' => 'InvalidTemporaryUrl', + 'exception_message' => 'Storage adapter did not return a temporary URL.', + ]); + + return $this->notFound(); + } + } + $this->recordDownload($request, $artwork->id); $this->incrementDownloadCountIfAvailable($artwork->id); @@ -80,7 +129,13 @@ final class ArtworkDownloadController extends Controller ]); } - $downloadName = $this->buildDownloadFilename((string) $artwork->file_name, $ext); + if (array_key_exists('download_status', $artwork->getAttributes())) { + $artwork->forceFill([ + 'download_status' => 'available', + 'download_source' => $resolution['source'], + 'download_checked_at' => now(), + ])->saveQuietly(); + } // X-Accel-Redirect is safe only when nginx is explicitly configured to // map the internal URI to the originals root. Otherwise fallback to the @@ -90,11 +145,15 @@ final class ArtworkDownloadController extends Controller return response('', 200, [ 'X-Accel-Redirect' => $accelUri, 'Content-Type' => 'application/octet-stream', - 'Content-Disposition' => 'attachment; filename="' . addslashes($downloadName) . '"', + 'Content-Disposition' => 'attachment; filename="'.addslashes($downloadName).'"', 'X-Content-Type-Options' => 'nosniff', ]); } + if ($resolution['source'] === 'object') { + return redirect()->away($temporaryUrl); + } + return response()->download($filePath, $downloadName); } @@ -124,7 +183,7 @@ final class ArtworkDownloadController extends Controller $normalizedRoot = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $root); $normalizedFilePath = str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $filePath); - $rootPrefix = $normalizedRoot . DIRECTORY_SEPARATOR; + $rootPrefix = $normalizedRoot.DIRECTORY_SEPARATOR; if (! str_starts_with($normalizedFilePath, $rootPrefix)) { Log::warning('Artwork download accel path skipped because file is outside originals root.', [ @@ -140,7 +199,7 @@ final class ArtworkDownloadController extends Controller return null; } - return $accelBase . str_replace(DIRECTORY_SEPARATOR, '/', $relativePath); + return $accelBase.str_replace(DIRECTORY_SEPARATOR, '/', $relativePath); } private function recordDownload(Request $request, int $artworkId): void @@ -195,10 +254,10 @@ final class ArtworkDownloadController extends Controller $brandSuffix = $this->downloadBrandSuffix(); if ($brandSuffix !== '' && ! Str::contains(Str::lower($baseName), Str::lower($brandSuffix))) { - $baseName .= ' (' . $brandSuffix . ')'; + $baseName .= ' ('.$brandSuffix.')'; } - return $baseName . '.' . $ext; + return $baseName.'.'.$ext; } private function downloadBrandSuffix(): string diff --git a/app/Services/ArtworkOriginalFileLocator.php b/app/Services/ArtworkOriginalFileLocator.php index 5b0acc0f..d55890c7 100644 --- a/app/Services/ArtworkOriginalFileLocator.php +++ b/app/Services/ArtworkOriginalFileLocator.php @@ -10,6 +10,12 @@ use Illuminate\Support\Facades\Storage; final class ArtworkOriginalFileLocator { + /** @var list */ + private const ALLOWED_EXTENSIONS = [ + 'jpg', 'jpeg', 'png', 'gif', 'webp', 'bmp', 'tiff', + 'zip', 'rar', '7z', 'tar', 'gz', + ]; + public function __construct( private readonly UploadStorageService $storage, ) {} @@ -27,7 +33,7 @@ final class ArtworkOriginalFileLocator } $hash = strtolower((string) $artwork->hash); - $ext = strtolower(ltrim((string) $artwork->file_ext, '.')); + $ext = $this->safeExtension($artwork); if (! $this->isValidHash($hash) || $ext === '') { return ''; @@ -41,17 +47,136 @@ final class ArtworkOriginalFileLocator . DIRECTORY_SEPARATOR . $hash . '.' . $ext; } + /** @return array{status:string,source:?string,disk:?string,path:string,object_key:string,exists:bool,size:?int,file_name:string,file_ext:string} */ + public function resolve(Artwork $artwork): array + { + $fileName = trim((string) ($artwork->file_name ?? '')); + $extension = $this->safeExtension($artwork); + $localPath = $this->resolveLocalPath($artwork); + + if ($localPath !== '' && is_file($localPath)) { + return $this->result('available', 'local', null, $localPath, '', true, (int) filesize($localPath), $fileName, $extension); + } + + if ($this->isTruePending($artwork)) { + return $this->result('pending', null, null, $localPath, '', false, null, $fileName, $extension); + } + + $objectKey = $this->resolveObjectPath($artwork); + if ($objectKey !== '') { + try { + $diskName = $this->storage->objectDiskName(); + $disk = Storage::disk($diskName); + if ($disk->exists($objectKey)) { + return $this->result('available', 'object', $diskName, '', $objectKey, true, (int) $disk->size($objectKey), $fileName, $extension); + } + } catch (\Throwable) { + // Treat unavailable storage as unresolved; the route logs context. + } + } + + $legacyPath = $this->resolveReadonlyLegacyPath($artwork); + if ($legacyPath !== '' && is_file($legacyPath)) { + return $this->result('available', 'legacy', null, $legacyPath, '', true, (int) filesize($legacyPath), $fileName, $extension); + } + + if ($this->hasIncompleteLegacyMetadata($artwork)) { + return $this->result('unknown', null, null, $localPath, $objectKey, false, null, $fileName, $extension); + } + + return $this->result('missing', null, null, $localPath, $objectKey, false, null, $fileName, $extension); + } + + private function isTruePending(Artwork $artwork): bool + { + return strtolower(trim((string) ($artwork->file_name ?? ''))) === 'pending' + && trim((string) ($artwork->hash ?? '')) === '' + && trim((string) ($artwork->file_ext ?? '')) === '' + && trim((string) ($artwork->file_path ?? '')) === ''; + } + + private function hasIncompleteLegacyMetadata(Artwork $artwork): bool + { + return ! $this->isValidHash(strtolower((string) ($artwork->hash ?? ''))) + || $this->safeExtension($artwork) === ''; + } + + private function resolveReadonlyLegacyPath(Artwork $artwork): string + { + $legacyRoot = rtrim(str_replace(['/', '\\'], DIRECTORY_SEPARATOR, trim((string) config('uploads.legacy_originals_root', ''))), DIRECTORY_SEPARATOR); + $legacyRelative = $this->legacyRelativePath($artwork); + if ($legacyRoot !== '' && $legacyRelative !== '') { + return $legacyRoot . DIRECTORY_SEPARATOR . str_replace(['/', '\\'], DIRECTORY_SEPARATOR, $legacyRelative); + } + + $root = rtrim(str_replace(['/', '\\'], DIRECTORY_SEPARATOR, trim((string) config('uploads.readonly_backup_originals_root', ''))), DIRECTORY_SEPARATOR); + $hash = strtolower((string) $artwork->hash); + $extension = $this->safeExtension($artwork); + if ($root === '' || ! $this->isValidHash($hash) || $extension === '') { + return ''; + } + + return $root . DIRECTORY_SEPARATOR . substr($hash, 0, 2) . DIRECTORY_SEPARATOR . substr($hash, 2, 2) . DIRECTORY_SEPARATOR . $hash . '.' . $extension; + } + + private function legacyRelativePath(Artwork $artwork): string + { + $path = trim((string) ($artwork->file_path ?? ''), '/\\'); + if ($path === '' || ! str_starts_with(strtolower($path), 'legacy/uploads/')) { + return ''; + } + + $relative = substr($path, strlen('legacy/uploads/')); + if ($relative === false || $relative === '' || str_contains($relative, '..')) { + return ''; + } + + return 'legacy/uploads/' . ltrim($relative, '/\\'); + } + + private function safeExtension(Artwork $artwork): string + { + $stored = strtolower(ltrim(trim((string) ($artwork->file_ext ?? '')), '.')); + if (in_array($stored, self::ALLOWED_EXTENSIONS, true)) { + return $stored; + } + + foreach ([(string) ($artwork->file_name ?? ''), (string) ($artwork->file_path ?? '')] as $candidate) { + $inferred = strtolower(ltrim((string) pathinfo($candidate, PATHINFO_EXTENSION), '.')); + if (in_array($inferred, self::ALLOWED_EXTENSIONS, true)) { + return $inferred; + } + } + + return ''; + } + + private function result(string $status, ?string $source, ?string $disk, string $path, string $objectKey, bool $exists, ?int $size, string $fileName, string $extension): array + { + return [ + 'status' => $status, + 'source' => $source, + 'disk' => $disk, + 'path' => $path, + 'object_key' => $objectKey, + 'exists' => $exists, + 'size' => $size, + 'file_name' => $fileName, + 'file_ext' => $extension, + ]; + } + public function resolveObjectPath(Artwork $artwork): string { $relative = trim((string) $artwork->file_path, '/'); $prefix = $this->originalObjectPrefix(); - if ($relative !== '' && str_starts_with($relative, $prefix)) { + if ($relative !== '' && (str_starts_with($relative, $prefix) || str_starts_with(strtolower($relative), 'legacy/uploads/'))) { return $relative; } $hash = strtolower((string) $artwork->hash); - $ext = strtolower(ltrim((string) $artwork->file_ext, '.')); + $ext = $this->safeExtension($artwork); if (! $this->isValidHash($hash) || $ext === '') { return ''; @@ -77,6 +202,6 @@ final class ArtworkOriginalFileLocator private function isValidHash(string $hash): bool { - return $hash !== '' && preg_match('/^[a-f0-9]+$/', $hash) === 1; + return preg_match('/^[a-f0-9]{40}$/', $hash) === 1; } } diff --git a/database/migrations/2026_09_05_000001_add_download_availability_to_artworks_table.php b/database/migrations/2026_09_05_000001_add_download_availability_to_artworks_table.php new file mode 100644 index 00000000..3219a1f3 --- /dev/null +++ b/database/migrations/2026_09_05_000001_add_download_availability_to_artworks_table.php @@ -0,0 +1,25 @@ +string('download_status', 16)->default('unknown')->after('file_path'); + $table->string('download_source', 16)->nullable()->after('download_status'); + $table->timestamp('download_checked_at')->nullable()->after('download_source'); + }); + } + + public function down(): void + { + Schema::table('artworks', function (Blueprint $table): void { + $table->dropColumn(['download_status', 'download_source', 'download_checked_at']); + }); + } +}; diff --git a/tests/Feature/ArtworkDownloadTest.php b/tests/Feature/ArtworkDownloadTest.php index 0e23775b..fea4933c 100644 --- a/tests/Feature/ArtworkDownloadTest.php +++ b/tests/Feature/ArtworkDownloadTest.php @@ -6,6 +6,7 @@ use App\Models\Artwork; use App\Models\User; use Illuminate\Support\Facades\File; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Storage; beforeEach(function () { $root = storage_path('framework/testing/artwork-downloads'); @@ -44,7 +45,7 @@ function makeOriginalFile(string $hash, string $ext, string $content = 'test-ima } it('downloads an existing artwork file', function () { - $hash = 'a9f3e6c1b8'; + $hash = 'a9f3e6c1b8a9f3e6c1b8a9f3e6c1b8a9f3e6c1b8'; $ext = 'png'; makeOriginalFile($hash, $ext); @@ -61,7 +62,7 @@ it('downloads an existing artwork file', function () { }); it('forces the download filename using file_name and extension', function () { - $hash = 'b7c4d1e2f3'; + $hash = 'b7c4d1e2f3b7c4d1e2f3b7c4d1e2f3b7c4d1e2f3'; $ext = 'jpg'; makeOriginalFile($hash, $ext); @@ -77,6 +78,79 @@ it('forces the download filename using file_name and extension', function () { $response->assertDownload('My Original Name (skinbase.org).jpg'); }); +it('downloads an object-only original without restoring it locally', function () { + config(['uploads.object_storage.disk' => 's3']); + + $hash = '1234567890abcdef1234567890abcdef12345678'; + $key = "artworks/original/12/34/{$hash}.jpg"; + $disk = Mockery::mock(); + $disk->shouldReceive('exists')->once()->with($key)->andReturnTrue(); + $disk->shouldReceive('size')->once()->with($key)->andReturn(15); + $disk->shouldReceive('temporaryUrl') + ->once() + ->withArgs(fn (string $path, \DateTimeInterface $expires, array $options): bool => + $path === $key + && $expires > now() + && str_contains((string) ($options['ResponseContentDisposition'] ?? ''), 'Object Original') + ) + ->andReturn('https://object.example.test/signed-object'); + $disk->shouldNotReceive('readStream'); + $disk->shouldNotReceive('get'); + Storage::shouldReceive('disk')->with('s3')->andReturn($disk); + + $artwork = Artwork::factory()->create([ + 'file_name' => 'Object Original', + 'file_path' => '', + 'hash' => $hash, + 'file_ext' => 'jpg', + ]); + + $response = $this->get("/download/artwork/{$artwork->id}"); + + $response->assertRedirect('https://object.example.test/signed-object'); + expect($artwork->fresh()->download_status)->toBe('available') + ->and($artwork->fresh()->download_source)->toBe('object') + ->and(DB::table('artwork_downloads')->where('artwork_id', $artwork->id)->count())->toBe(1); +}); + +it('does not count an object download when signing fails', function () { + config(['uploads.object_storage.disk' => 's3']); + $hash = 'abcdefabcdefabcdefabcdefabcdefabcdefabcd'; + $key = "artworks/original/ab/cd/{$hash}.jpg"; + $disk = Mockery::mock(); + $disk->shouldReceive('exists')->once()->with($key)->andReturnTrue(); + $disk->shouldReceive('size')->once()->with($key)->andReturn(15); + $disk->shouldReceive('temporaryUrl')->once()->andThrow(new RuntimeException('signing unavailable')); + Storage::shouldReceive('disk')->with('s3')->andReturn($disk); + + $artwork = Artwork::factory()->create([ + 'file_name' => 'Signing Failure', + 'file_path' => '', + 'hash' => $hash, + 'file_ext' => 'jpg', + ]); + $this->mock(\App\Services\ArtworkStatsService::class, function ($mock): void { + $mock->shouldReceive('incrementDownloads')->never(); + }); + + $this->get("/download/artwork/{$artwork->id}")->assertNotFound(); +}); + +it('classifies incomplete original metadata as pending', function () { + $artwork = Artwork::factory()->create([ + 'file_name' => 'pending', + 'file_path' => '', + 'hash' => null, + 'file_ext' => null, + ]); + + expect(app(\App\Services\ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'pending', + 'source' => null, + 'exists' => false, + ]); +}); + it('returns 404 for a missing artwork', function () { $this->get('/download/artwork/999999')->assertNotFound(); }); @@ -91,7 +165,7 @@ it('returns 404 when the original file is missing', function () { }); it('logs download metadata with user and request context', function () { - $hash = 'd4e5f6a7b8'; + $hash = 'd4e5f6a7b8d4e5f6a7b8d4e5f6a7b8d4e5f6a7b8'; $ext = 'gif'; makeOriginalFile($hash, $ext); @@ -119,7 +193,7 @@ it('logs download metadata with user and request context', function () { }); it('logs guest download with null user_id', function () { - $hash = 'e1f2a3b4c5'; + $hash = 'e1f2a3b4c5e1f2a3b4c5e1f2a3b4c5e1f2a3b4c5'; $ext = 'png'; makeOriginalFile($hash, $ext); @@ -137,7 +211,7 @@ it('logs guest download with null user_id', function () { }); it('increments artwork_stats downloads on the real download route', function () { - $hash = 'f1e2d3c4b5'; + $hash = 'f1e2d3c4b5f1e2d3c4b5f1e2d3c4b5f1e2d3c4b5'; $ext = 'png'; makeOriginalFile($hash, $ext); diff --git a/tests/Feature/ArtworkOriginalFileLocatorTest.php b/tests/Feature/ArtworkOriginalFileLocatorTest.php new file mode 100644 index 00000000..b10c5c25 --- /dev/null +++ b/tests/Feature/ArtworkOriginalFileLocatorTest.php @@ -0,0 +1,153 @@ + $root, + 'uploads.readonly_backup_originals_root' => storage_path('framework/testing/artwork-readonly-backup'), + 'uploads.legacy_originals_root' => $legacyRoot, + 'uploads.object_storage.disk' => 's3', + ]); + File::deleteDirectory($root); + File::deleteDirectory($legacyRoot); + File::deleteDirectory(storage_path('framework/testing/artwork-readonly-backup')); + File::makeDirectory($root, 0755, true); + File::makeDirectory($legacyRoot, 0755, true); + Storage::fake('s3'); +}); + +afterEach(function (): void { + File::deleteDirectory(storage_path('framework/testing/artwork-originals')); + File::deleteDirectory(storage_path('framework/testing/artwork-legacy')); + File::deleteDirectory(storage_path('framework/testing/artwork-readonly-backup')); +}); + +function locatorLocalPath(string $hash, string $extension): string +{ + return storage_path("framework/testing/artwork-originals/{$hash[0]}{$hash[1]}/{$hash[2]}{$hash[3]}/{$hash}.{$extension}"); +} + +it('infers a missing extension and resolves a hashed local original', function (): void { + $hash = str_repeat('a', 40); + File::ensureDirectoryExists(dirname(locatorLocalPath($hash, 'jpg'))); + File::put(locatorLocalPath($hash, 'jpg'), 'local'); + $artwork = Artwork::factory()->create(['hash' => $hash, 'file_ext' => null, 'file_name' => 'Something.JPG', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'available', 'source' => 'local', 'file_ext' => 'jpg', + ]); +}); + +it('infers a missing extension and resolves a hashed object original', function (): void { + $hash = str_repeat('b', 40); + Storage::disk('s3')->put("artworks/original/bb/bb/{$hash}.png", 'object'); + $artwork = Artwork::factory()->create(['hash' => $hash, 'file_ext' => null, 'file_name' => 'Image.PNG', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'available', 'source' => 'object', 'file_ext' => 'png', + ]); +}); + +it('infers a missing extension and resolves a hashed readonly backup original', function (): void { + $hash = str_repeat('d', 40); + $root = (string) config('uploads.readonly_backup_originals_root'); + $path = $root . DIRECTORY_SEPARATOR . 'dd' . DIRECTORY_SEPARATOR . 'dd' . DIRECTORY_SEPARATOR . $hash . '.jpeg'; + File::ensureDirectoryExists(dirname($path)); + File::put($path, 'backup'); + $artwork = Artwork::factory()->create(['hash' => $hash, 'file_ext' => null, 'file_name' => 'backup.JPEG', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'available', 'source' => 'legacy', 'file_ext' => 'jpeg', + ]); +}); + +it('resolves a hashless legacy upload from the authoritative legacy filesystem root', function (): void { + $relative = 'legacy/uploads/old-image.jpg'; + $path = storage_path("framework/testing/artwork-legacy/{$relative}"); + File::ensureDirectoryExists(dirname($path)); + File::put($path, 'legacy'); + $artwork = Artwork::factory()->create(['hash' => null, 'file_ext' => null, 'file_name' => 'old-image.jpg', 'file_path' => $relative]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'available', 'source' => 'legacy', 'file_ext' => 'jpg', + ]); +}); + +it('resolves a hashless legacy upload from the authoritative object disk', function (): void { + $relative = 'legacy/uploads/old-image.jpg'; + Storage::disk('s3')->put($relative, 'legacy'); + $artwork = Artwork::factory()->create(['hash' => null, 'file_ext' => null, 'file_name' => 'old-image.jpg', 'file_path' => $relative]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'available', 'source' => 'object', 'object_key' => $relative, 'file_ext' => 'jpg', + ]); +}); + +it('does not use the canonical readonly backup root as a legacy filename root', function (): void { + config(['uploads.legacy_originals_root' => '']); + $relative = 'legacy/uploads/not-migrated.jpg'; + $path = storage_path("framework/testing/artwork-readonly-backup/{$relative}"); + File::ensureDirectoryExists(dirname($path)); + File::put($path, 'not authoritative'); + $artwork = Artwork::factory()->create(['hash' => null, 'file_ext' => null, 'file_name' => 'not-migrated.jpg', 'file_path' => $relative]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork)['status'])->toBe('unknown'); +}); + +it('keeps a genuine pending placeholder pending', function (): void { + $artwork = Artwork::factory()->create(['file_name' => 'pending', 'file_path' => '', 'hash' => null, 'file_ext' => null]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork)['status'])->toBe('pending'); +}); + +it('classifies unresolved incomplete legacy metadata as unknown', function (): void { + $artwork = Artwork::factory()->create(['file_name' => 'old-image.jpg', 'file_path' => 'legacy/uploads/old-image.jpg', 'hash' => null, 'file_ext' => null]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork)['status'])->toBe('unknown'); +}); + +it('does not infer unsupported or executable extensions', function (): void { + $artwork = Artwork::factory()->create(['file_name' => 'CIH.php', 'file_path' => 'legacy/uploads/CIH.php', 'hash' => null, 'file_ext' => null]); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork)['status'])->toBe('unknown'); +}); + +it('keeps complete metadata missing everywhere as missing', function (): void { + $hash = str_repeat('c', 40); + $artwork = Artwork::factory()->create(['hash' => $hash, 'file_ext' => 'jpg', 'file_name' => 'missing.jpg', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork)['status'])->toBe('missing'); +}); + +it('keeps a valid SHA-1 BMP original missing when all sources are absent', function (): void { + $hash = str_repeat('e', 40); + $artwork = Artwork::factory()->create(['hash' => $hash, 'file_ext' => null, 'file_name' => 'preview.bmp', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'missing', 'file_ext' => 'bmp', + ]); +}); + +it('classifies a short hash as unknown and does not construct a canonical path', function (): void { + $artwork = Artwork::factory()->create(['hash' => 'ddeeff112233', 'file_ext' => null, 'file_name' => 'rose-closeup.jpg', 'file_path' => 'uploads/artworks/image.jpg']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'unknown', 'path' => '', 'object_key' => '', + ]); +}); + +it('classifies a non-hex 40-character hash as unknown', function (): void { + $artwork = Artwork::factory()->create(['hash' => str_repeat('g', 40), 'file_ext' => 'jpg', 'file_name' => 'invalid.jpg', 'file_path' => '']); + + expect(app(ArtworkOriginalFileLocator::class)->resolve($artwork))->toMatchArray([ + 'status' => 'unknown', 'path' => '', 'object_key' => '', + ]); +}); diff --git a/tests/Feature/AuditArtworkDownloadFilesCommandTest.php b/tests/Feature/AuditArtworkDownloadFilesCommandTest.php new file mode 100644 index 00000000..938ec60d --- /dev/null +++ b/tests/Feature/AuditArtworkDownloadFilesCommandTest.php @@ -0,0 +1,73 @@ + 's3']); + $hash = $status === 'pending' + ? null + : str_repeat($source === 'local' ? 'a' : ($source === 'object' ? 'b' : 'c'), 40); + $ext = $status === 'pending' ? null : 'jpg'; + + if ($source === 'local') { + $root = (string) config('uploads.local_originals_root'); + $path = $root . DIRECTORY_SEPARATOR . 'aa' . DIRECTORY_SEPARATOR . 'aa' . DIRECTORY_SEPARATOR . $hash . '.jpg'; + File::ensureDirectoryExists(dirname($path)); + File::put($path, 'local'); + } elseif ($source === 'object') { + Storage::disk('s3')->put("artworks/original/bb/bb/{$hash}.jpg", 'object'); + } + + $artwork = Artwork::factory()->create([ + 'file_name' => $status === 'pending' ? 'pending' : 'audit-' . $status, + 'hash' => $hash, + 'file_ext' => $ext, + 'file_path' => '', + ]); + + $exit = Artisan::call('artworks:audit-download-files', [ + '--id' => $artwork->id, + '--chunk' => 1, + ]); + $output = Artisan::output(); + + expect($exit)->toBe(Command::SUCCESS) + ->and($output)->toContain("classification={$expected} count=1") + ->and($output)->toContain('processed=1'); +})->with([ + ['available', 'local', 'LOCAL_OK'], + ['available', 'object', 'OBJECT_OK'], + ['pending', null, 'PENDING'], + ['missing', null, 'MISSING_EVERYWHERE'], +]); + +it('marks an explicit id once when requested', function (): void { + Storage::fake('s3'); + config(['uploads.object_storage.disk' => 's3']); + $hash = str_repeat('cd', 20); + Storage::disk('s3')->put("artworks/original/cd/cd/{$hash}.jpg", 'object'); + + $artwork = Artwork::factory()->create([ + 'hash' => $hash, + 'file_ext' => 'jpg', + 'file_path' => '', + ]); + + expect(Artisan::call('artworks:audit-download-files', [ + '--id' => $artwork->id, + '--chunk' => 1, + '--mark-status' => true, + ]))->toBe(Command::SUCCESS); + + $artwork->refresh(); + expect($artwork->download_status)->toBe('available') + ->and($artwork->download_source)->toBe('object') + ->and(Artisan::output())->toContain('processed=1'); +});