diff --git a/src/Assets/Asset.php b/src/Assets/Asset.php index a432150acbb..3018fbf62b9 100644 --- a/src/Assets/Asset.php +++ b/src/Assets/Asset.php @@ -8,6 +8,8 @@ use Illuminate\Support\Carbon; use Illuminate\Support\Facades\Cache; use League\Flysystem\PathTraversalDetected; +use League\Flysystem\UnableToReadFile; +use League\Flysystem\UnableToRetrieveMetadata; use Statamic\Assets\AssetUploader as Uploader; use Statamic\Contracts\Assets\Asset as AssetContract; use Statamic\Contracts\Assets\AssetContainer as AssetContainerContract; @@ -37,6 +39,7 @@ use Statamic\Facades\Blink; use Statamic\Facades\Image; use Statamic\Facades\Path; +use Statamic\Facades\Stache; use Statamic\Facades\URL; use Statamic\Facades\YAML; use Statamic\GraphQL\ResolvesValues; @@ -260,15 +263,31 @@ public function meta($key = null) return $meta; } - return $this->meta = $this->cacheStore()->rememberForever($this->metaCacheKey(), function () { + $meta = $this->cacheStore()->rememberForever($this->metaCacheKey(), function () { if ($contents = $this->disk()->get($path = $this->metaPath())) { return YAML::file($path)->parse($contents); } - $this->writeMeta($meta = $this->generateMeta()); + $meta = $this->generateMeta(); + + if ($this->hasGeneratedMeta($meta)) { + $this->writeMeta($meta); + } return $meta; }); + + $generated = $this->hasGeneratedMeta($meta); + + if (! $generated) { + $this->cacheStore()->forget($this->metaCacheKey()); + } + + if ($generated || ! $this->exists()) { + $this->meta = $meta; + } + + return $meta; } private function metaValue($key) @@ -281,7 +300,11 @@ private function metaValue($key) $this->cacheStore()->forget($this->metaCacheKey()); - $this->writeMeta($meta = $this->generateMeta()); + $meta = $this->generateMeta(); + + if ($this->hasGeneratedMeta($meta)) { + $this->writeMeta($meta); + } return Arr::get($meta, $key); } @@ -291,21 +314,51 @@ public function generateMeta() $meta = ['data' => $this->data->all()]; if ($this->exists()) { - $attributes = Attributes::asset($this)->get(); - - $meta = array_merge($meta, [ - 'size' => $this->disk()->size($this->path()), - 'last_modified' => $this->disk()->lastModified($this->path()), - 'width' => Arr::get($attributes, 'width'), - 'height' => Arr::get($attributes, 'height'), - 'mime_type' => $this->disk()->mimeType($this->path()), - 'duration' => Arr::get($attributes, 'duration'), - ]); + try { + $attributes = Attributes::asset($this)->get(); + + $meta = array_merge($meta, [ + 'size' => $this->disk()->size($this->path()), + 'last_modified' => $this->disk()->lastModified($this->path()), + 'width' => Arr::get($attributes, 'width'), + 'height' => Arr::get($attributes, 'height'), + 'mime_type' => $this->disk()->mimeType($this->path()), + 'duration' => Arr::get($attributes, 'duration'), + ]); + } catch (UnableToRetrieveMetadata|UnableToReadFile) { + if (! $this->disk()->exists($this->path())) { + $this->forgetMissingFile(); + } + } } return $meta; } + private function hasGeneratedMeta(array $meta): bool + { + return isset($meta['size']); + } + + /** + * The cached directory listing still names this path, but the file is gone. + * Drop the path from the listing and, when assets are indexed in the Stache, + * from that index. Stored asset data stays put: this does not go through the + * asset repository, whose eloquent implementation deletes the meta row. + */ + public function forgetMissingFile(): void + { + $this->meta = null; + $this->cacheStore()->forget($this->metaCacheKey()); + + $this->container()->contents()->forget($this->path())->save(); + + // The eloquent driver keeps asset data in the database and excludes this store. + if (config('statamic.eloquent-driver.assets.driver') !== 'eloquent') { + Stache::store('assets::'.$this->containerHandle())->delete($this); + } + } + public function metaPath() { $path = dirname($this->path()).'/.meta/'.$this->basename().'.yaml'; @@ -320,6 +373,10 @@ protected function metaExists() public function writeMeta($meta) { + if (! $this->exists()) { + return; + } + $meta['data'] = Arr::removeNullValues($meta['data']); $contents = YAML::dump($meta); diff --git a/src/Assets/AssetContainer.php b/src/Assets/AssetContainer.php index 39a733477d6..a6c0e1b7657 100644 --- a/src/Assets/AssetContainer.php +++ b/src/Assets/AssetContainer.php @@ -478,7 +478,13 @@ public function asset($path) return null; } - return $asset->hydrate()->syncOriginal(); + $asset->hydrate()->syncOriginal(); + + if (! $asset->exists()) { + return null; + } + + return $asset; } /** diff --git a/src/Imaging/ImageGenerator.php b/src/Imaging/ImageGenerator.php index 99116c6fd06..30fe8edb70a 100644 --- a/src/Imaging/ImageGenerator.php +++ b/src/Imaging/ImageGenerator.php @@ -90,6 +90,7 @@ public function generateByPath($path, array $params) private function doGenerateByPath($path, array $params, $sourceFilesystemRoot = null) { + $this->asset = null; $this->path = $path; $this->setParams($params); @@ -116,6 +117,7 @@ public function generateByUrl($url, array $params) private function doGenerateByUrl($url, array $params) { + $this->asset = null; $this->skip_validation = true; $this->setParams($params); @@ -318,6 +320,10 @@ private function generate($image) try { $path = $this->server->makeImage($image, $this->params); } catch (GlideFileNotFoundException $e) { + if (! $this->asset?->disk()?->exists($this->asset->path())) { + $this->asset?->forgetMissingFile(); + } + throw UnableToReadFile::fromLocation($image); } diff --git a/tests/Assets/AssetContainerTest.php b/tests/Assets/AssetContainerTest.php index 21256960450..65a62a32eb7 100644 --- a/tests/Assets/AssetContainerTest.php +++ b/tests/Assets/AssetContainerTest.php @@ -5,6 +5,7 @@ use Facades\Statamic\Fields\BlueprintRepository; use Illuminate\Cache\Events\CacheHit; use Illuminate\Contracts\Support\Arrayable; +use Illuminate\Filesystem\FilesystemAdapter; use Illuminate\Support\Carbon; use Illuminate\Support\Collection; use Illuminate\Support\Facades\Cache; @@ -15,12 +16,16 @@ use League\Flysystem\DirectoryListing; use League\Flysystem\FileAttributes; use League\Flysystem\PathTraversalDetected; +use League\Flysystem\UnableToRetrieveMetadata; +use Mockery; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Test; use Statamic\Assets\Asset; use Statamic\Assets\AssetContainer; +use Statamic\Assets\AssetRepository; use Statamic\Contracts\Assets\Asset as AssetContract; use Statamic\Contracts\Assets\AssetFolder; +use Statamic\Contracts\Assets\AssetRepository as AssetRepositoryContract; use Statamic\Events\AssetContainerCreated; use Statamic\Events\AssetContainerCreating; use Statamic\Events\AssetContainerDeleted; @@ -816,6 +821,96 @@ public function it_gets_an_asset_with_data() $this->assertNull($container->asset('non-existent.txt')); } + #[Test] + public function it_does_not_return_an_asset_when_a_listed_file_is_missing() + { + [$container, $missing] = $this->containerWithStaleListedFile(); + + $this->assertNull($container->asset($missing)); + $this->assertMissingPathWasDropped($container, $missing); + + $kept = $container->asset('keep.txt'); + + $this->assertInstanceOf(AssetContract::class, $kept); + $this->assertSame('keep.txt', $kept->path()); + } + + #[Test] + public function it_does_not_throw_when_reading_meta_for_a_missing_listed_file() + { + [$container, $missing] = $this->containerWithStaleListedFile(); + + $asset = $container->makeAsset($missing); + + $this->assertSame(['data' => []], $asset->meta()); + $this->assertFalse(Storage::disk('stale_assets')->exists('social_images/collection-events/.meta/missing.png.yaml')); + $this->assertMissingPathWasDropped($container, $missing); + } + + #[Test] + public function it_keeps_a_listed_file_when_metadata_cannot_be_read() + { + $path = 'notes.txt'; + $filesystem = Mockery::mock(FilesystemAdapter::class); + $filesystem->shouldReceive('get')->andReturn(null); + $filesystem->shouldReceive('size')->andThrow(UnableToRetrieveMetadata::create($path, 'file_size')); + $filesystem->shouldReceive('exists')->once()->andReturn(true); + $filesystem->shouldReceive('put')->never(); + + [$container, $asset] = $this->containerWithListedFile($path, $filesystem); + + $this->assertSame(['data' => []], $asset->meta()); + $this->assertNull(Cache::get($asset->metaCacheKey())); + $this->assertTrue(Cache::get('asset-list-contents-'.$container->handle())->has($path)); + } + + #[Test] + public function it_does_not_drop_a_listed_file_when_the_existence_check_throws() + { + $path = 'notes.txt'; + $filesystem = Mockery::mock(FilesystemAdapter::class); + $filesystem->shouldReceive('get')->andReturn(null); + $filesystem->shouldReceive('size')->andThrow(UnableToRetrieveMetadata::create($path, 'file_size')); + $filesystem->shouldReceive('exists')->andThrow(UnableToRetrieveMetadata::create($path, 'file_size')); + $filesystem->shouldReceive('put')->never(); + + [$container, $asset] = $this->containerWithListedFile($path, $filesystem); + + try { + $asset->meta(); + } catch (UnableToRetrieveMetadata) { + $this->assertNull(Cache::get($asset->metaCacheKey())); + $this->assertTrue(Cache::get('asset-list-contents-'.$container->handle())->has($path)); + + return; + } + + $this->fail('Exception was not thrown.'); + } + + #[Test] + public function it_keeps_stored_asset_data_when_dropping_a_missing_file() + { + $repository = $this->partialMock(AssetRepository::class); + $repository->shouldNotReceive('delete'); + $this->instance(AssetRepositoryContract::class, $repository); + + [$container, $missing] = $this->containerWithStaleListedFile(); + + Storage::disk('stale_assets')->put( + 'social_images/collection-events/.meta/missing.png.yaml', + "data:\n alt: 'Event poster'\n" + ); + + $container->makeAsset($missing)->generateMeta(); + + $this->assertMissingPathWasDropped($container, $missing); + $this->assertSame( + "data:\n alt: 'Event poster'\n", + Storage::disk('stale_assets')->get('social_images/collection-events/.meta/missing.png.yaml') + ); + } + #[Test] public function it_makes_an_asset_at_given_path() { @@ -1100,4 +1195,72 @@ private function containerWithDisk($fixture = 'container') return $container; } + + private function containerWithListedFile(string $path, FilesystemAdapter $filesystem): array + { + Blink::flush(); + + $handle = 'listed_'.md5($path.microtime()); + + Cache::put('asset-list-contents-'.$handle, collect([ + $path => [ + 'type' => 'file', + 'path' => $path, + 'basename' => basename($path), + 'filename' => pathinfo($path, PATHINFO_FILENAME), + 'dirname' => '', + 'extension' => pathinfo($path, PATHINFO_EXTENSION), + ], + ])); + + $container = (new AssetContainer)->handle($handle)->disk($filesystem); + + Facades\AssetContainer::shouldReceive('findByHandle')->with($handle)->andReturn($container); + + return [$container, $container->makeAsset($path)]; + } + + private function containerWithStaleListedFile(): array + { + Blink::flush(); + + Storage::fake('stale_assets'); + Storage::disk('stale_assets')->put('keep.txt', 'still here'); + + $missing = 'social_images/collection-events/missing.png'; + $handle = 'stale_'.md5($missing.microtime()); + + Cache::put('asset-list-contents-'.$handle, collect([ + 'keep.txt' => [ + 'type' => 'file', + 'path' => 'keep.txt', + 'basename' => 'keep.txt', + 'filename' => 'keep', + 'dirname' => '', + 'extension' => 'txt', + ], + $missing => [ + 'type' => 'file', + 'path' => $missing, + 'basename' => 'missing.png', + 'filename' => 'missing', + 'dirname' => 'social_images/collection-events', + 'extension' => 'png', + ], + ])); + + $container = (new AssetContainer)->handle($handle)->disk('stale_assets'); + + Facades\AssetContainer::shouldReceive('findByHandle')->with($handle)->andReturn($container); + + return [$container, $missing]; + } + + private function assertMissingPathWasDropped($container, string $missing): void + { + $listed = Cache::get('asset-list-contents-'.$container->handle()); + + $this->assertFalse($listed->has($missing)); + $this->assertTrue($listed->has('keep.txt')); + } } diff --git a/tests/Feature/Assets/ImageThumbnailTest.php b/tests/Feature/Assets/ImageThumbnailTest.php index ba1c65a5575..c73f270cabf 100644 --- a/tests/Feature/Assets/ImageThumbnailTest.php +++ b/tests/Feature/Assets/ImageThumbnailTest.php @@ -3,9 +3,17 @@ namespace Tests\Feature\Assets; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Cache; +use Illuminate\Support\Facades\Storage; use PHPUnit\Framework\Attributes\Test; +use Statamic\Assets\AssetRepository; +use Statamic\Contracts\Assets\AssetRepository as AssetRepositoryContract; use Statamic\Facades\AssetContainer; +use Statamic\Facades\Blink; +use Statamic\Facades\File; +use Statamic\Facades\Glide; use Statamic\Facades\User; +use Statamic\Facades\YAML; use Tests\FakesRoles; use Tests\PreventSavingStacheItemsToDisk; use Tests\TestCase; @@ -25,6 +33,9 @@ public function setUp(): void 'driver' => 'local', 'root' => $this->tempDir = __DIR__.'/tmp', ]]); + + Glide::cacheStore()->flush(); + File::delete(storage_path('statamic/glide')); } public function tearDown(): void @@ -81,4 +92,58 @@ public function it_denies_access_without_permission_to_view_asset() ->getJson('/cp/thumbnails/'.base64_encode('test::one.png')) ->assertForbidden(); } + + #[Test] + public function it_drops_a_missing_file_from_the_browser_after_the_thumbnail_read_fails() + { + $container = AssetContainer::make('test')->disk('test')->save(); + $asset = $container + ->makeAsset('one.png') + ->upload(UploadedFile::fake()->image('one.png', 30, 60)); + $asset->data(['alt' => 'Event poster'])->save(); + $container + ->makeAsset('keep.png') + ->upload(UploadedFile::fake()->image('keep.png', 30, 60)); + + $cached = Cache::get($asset->metaCacheKey()); + + $this->assertIsArray($cached); + $this->assertArrayHasKey('size', $cached); + $this->assertArrayHasKey('mime_type', $cached); + + Storage::disk('test')->delete('one.png'); + + $repository = $this->partialMock(AssetRepository::class); + $repository->shouldNotReceive('delete'); + $this->instance(AssetRepositoryContract::class, $repository); + + $this->setTestRoles(['test' => ['access cp', 'view test assets']]); + $user = User::make()->assignRole('test')->save(); + + $this + ->actingAs($user) + ->getJson('/cp/assets/browse/folders/test/') + ->assertSuccessful() + ->assertJsonFragment(['path' => 'one.png']) + ->assertJsonFragment(['path' => 'keep.png']); + + $this + ->actingAs($user) + ->getJson('/cp/thumbnails/'.base64_encode('test::one.png')) + ->assertNotFound(); + + Blink::flush(); + + $this + ->actingAs($user) + ->getJson('/cp/assets/browse/folders/test/') + ->assertSuccessful() + ->assertJsonMissing(['path' => 'one.png']) + ->assertJsonFragment(['path' => 'keep.png']); + + $this->assertSame( + 'Event poster', + YAML::parse(Storage::disk('test')->get('.meta/one.png.yaml'))['data']['alt'] + ); + } }