From 8c478d0a59a4465b9d4e2930750ccec87b8958a6 Mon Sep 17 00:00:00 2001 From: Laurens Kuiper <104916503+lwekuiper@users.noreply.github.com> Date: Mon, 30 Mar 2026 16:12:29 +0200 Subject: [PATCH 1/3] [7.x] Add $unique argument to Asset::move() (#14364) --- src/Assets/Asset.php | 27 +++++---------------------- tests/Assets/AssetTest.php | 8 ++++---- 2 files changed, 9 insertions(+), 26 deletions(-) diff --git a/src/Assets/Asset.php b/src/Assets/Asset.php index f820b780c9b..0cf9297484e 100644 --- a/src/Assets/Asset.php +++ b/src/Assets/Asset.php @@ -745,11 +745,7 @@ public function containerHandle() */ public function rename($filename, $unique = false) { - if ($unique) { - return $this->moveUnique($this->folder(), $filename); - } - - return $this->move($this->folder(), $filename); + return $this->move($this->folder(), $filename, $unique); } /** @@ -757,11 +753,13 @@ public function rename($filename, $unique = false) * * @param string $folder The folder relative to the container. * @param string|null $filename The new filename, if renaming. + * @param bool $unique Whether to ensure the filename is unique. * @return $this */ - public function move($folder, $filename = null) + public function move($folder, $filename = null, $unique = false) { $filename = Uploader::getSafeFilename($filename ?: $this->filename()); + $filename = $unique ? $this->ensureUniqueFilename($folder, $filename) : $filename; $oldPath = $this->path(); $oldMetaPath = $this->metaPath(); $newPath = Str::removeLeft(Path::tidy($folder.'/'.$filename.'.'.pathinfo($oldPath, PATHINFO_EXTENSION)), '/'); @@ -780,22 +778,7 @@ public function move($folder, $filename = null) return $this; } - /** - * Move the asset to a different location with a unique filename. - * - * @param string $folder The folder relative to the container. - * @param string|null $filename The new filename, if renaming. - * @return $this - */ - public function moveUnique($folder, $filename = null) - { - $filename = Uploader::getSafeFilename($filename ?: $this->filename()); - $filename = $this->ensureUniqueFilename($folder, $filename); - - return $this->move($folder, $filename); - } - - public function moveQuietly($folder, $filename = null) + public function moveQuietly($folder, $filename = null, $unique = false) { $this->withEvents = false; diff --git a/tests/Assets/AssetTest.php b/tests/Assets/AssetTest.php index 29c1cc60645..060ecd0c9db 100644 --- a/tests/Assets/AssetTest.php +++ b/tests/Assets/AssetTest.php @@ -1261,7 +1261,7 @@ public function it_doesnt_lowercase_moved_files_when_configured() } #[Test] - public function it_can_be_moved_uniquely_to_another_folder_when_conflict_exists() + public function it_can_be_moved_to_another_folder_with_a_unique_filename_when_conflict_exists() { Storage::fake('local'); $disk = Storage::disk('local'); @@ -1274,7 +1274,7 @@ public function it_can_be_moved_uniquely_to_another_folder_when_conflict_exists( $asset = $container->makeAsset('old/asset.txt')->data(['foo' => 'bar']); $asset->save(); - $return = $asset->moveUnique('new'); + $return = $asset->move('new', null, true); $this->assertEquals($asset, $return); $disk->assertMissing('old/asset.txt'); @@ -1283,7 +1283,7 @@ public function it_can_be_moved_uniquely_to_another_folder_when_conflict_exists( } #[Test] - public function it_can_be_moved_uniquely_to_another_folder_without_renaming_when_no_conflict() + public function it_can_be_moved_to_another_folder_with_a_unique_filename_without_renaming_when_no_conflict() { Storage::fake('local'); $disk = Storage::disk('local'); @@ -1294,7 +1294,7 @@ public function it_can_be_moved_uniquely_to_another_folder_without_renaming_when $asset = $container->makeAsset('old/asset.txt')->data(['foo' => 'bar']); $asset->save(); - $return = $asset->moveUnique('new'); + $return = $asset->move('new', null, true); $this->assertEquals($asset, $return); $disk->assertMissing('old/asset.txt'); From c94f024ac1c37cc2251c49ffa78a99b6375dc703 Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Fri, 11 Sep 2026 16:08:06 +0200 Subject: [PATCH 2/3] Guard ImageGenerator::generateByAsset() against a null asset (#15359) When an asset URL resolves to null (e.g. a repository that can't find the asset by that URL, such as statamic/eloquent-driver#609), generateByAsset() dereferenced it directly, throwing an uncaught Error rather than the Exception the glide tag already knows how to catch and skip. Return '' early instead, matching the existing skip convention used elsewhere in this method. --- src/Imaging/ImageGenerator.php | 6 +++++- tests/Imaging/ImageGeneratorTest.php | 6 ++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Imaging/ImageGenerator.php b/src/Imaging/ImageGenerator.php index c419010fc51..f34175163aa 100644 --- a/src/Imaging/ImageGenerator.php +++ b/src/Imaging/ImageGenerator.php @@ -150,11 +150,15 @@ public function generateVideoThumbnail($asset, array $params) /** * Generate a manipulated image by an asset. * - * @param \Statamic\Contracts\Assets\Asset $asset + * @param \Statamic\Contracts\Assets\Asset|null $asset * @return mixed */ public function generateByAsset($asset, array $params) { + if (! $asset) { + return ''; + } + if (ThumbnailExtractor::enabled() && $asset->isVideo()) { return $this->generateVideoThumbnail($asset, $params); } diff --git a/tests/Imaging/ImageGeneratorTest.php b/tests/Imaging/ImageGeneratorTest.php index f774a2af27b..2446da94b7c 100644 --- a/tests/Imaging/ImageGeneratorTest.php +++ b/tests/Imaging/ImageGeneratorTest.php @@ -98,6 +98,12 @@ public function it_generates_an_image_by_asset() Event::assertDispatchedTimes(GlideImageGenerated::class, 1); } + #[Test] + public function it_does_not_generate_an_image_for_a_missing_asset() + { + $this->assertSame('', $this->makeGenerator()->generateByAsset(null, ['w' => 100])); + } + #[Test] public function it_throws_unable_to_read_file_when_asset_is_not_a_valid_image() { From 7536259d5610caf1a8aa4230a7b1a5c3f00603f0 Mon Sep 17 00:00:00 2001 From: wakqasahmed Date: Mon, 14 Sep 2026 21:37:32 +0200 Subject: [PATCH 3/3] Log the missing item, not a flat message, when an asset can't be resolved Log::error() inside ImageGenerator::generateByAsset()'s null guard has no way to know which item the caller was resolving, so every occurrence logged an identical, context-free message. Throw instead from Glide::generateImage() (where $item is still in scope) so the tag's existing catch (\Exception $e) { Log::error($e->getMessage()); } in generate() logs a message that includes the offending item. Per @jasonvarga's follow-up review on #15447. --- src/Tags/Glide.php | 13 ++++++++++++- tests/Tags/GlideTest.php | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/Tags/Glide.php b/src/Tags/Glide.php index 8b3bcd861d8..705f9aeaecf 100644 --- a/src/Tags/Glide.php +++ b/src/Tags/Glide.php @@ -177,7 +177,18 @@ private function generateImage($item) : $this->getGenerator()->generateByPath($item, $params); } - return $this->getGenerator()->generateByAsset(Asset::find($item), $params); + $asset = Asset::find($item); + + if (! $asset) { + // Thrown (rather than logged here directly) so the calling closure's + // existing catch (\Exception $e) { Log::error($e->getMessage()); } + // in generate() logs it with the identifying $item, instead of the + // flat, context-free message generateByAsset()'s own null-asset + // guard would otherwise produce. + throw new \Exception('Cannot generate an image for a missing asset: '.(is_string($item) ? $item : json_encode($item))); + } + + return $this->getGenerator()->generateByAsset($asset, $params); } /** diff --git a/tests/Tags/GlideTest.php b/tests/Tags/GlideTest.php index 920aa24a253..1df330dce63 100644 --- a/tests/Tags/GlideTest.php +++ b/tests/Tags/GlideTest.php @@ -3,6 +3,7 @@ namespace Tests\Tags; use Illuminate\Http\UploadedFile; +use Illuminate\Support\Facades\Log; use Orchestra\Testbench\Attributes\DefineEnvironment; use PHPUnit\Framework\Attributes\Test; use Statamic\Facades\File; @@ -11,6 +12,25 @@ class GlideTest extends TestCase { + #[Test] + /** + * https://github.com/statamic/cms/pull/15447 + */ + public function it_logs_the_item_when_the_asset_cannot_be_resolved() + { + Log::shouldReceive('error') + ->once() + ->with(\Mockery::pattern('/Cannot generate an image for a missing asset.*nonexistent\.jpg/')); + + $result = (string) Parse::template( + '{{ glide:foo width="100" }}', + ['foo' => 'nonexistent.jpg'], + trusted: true + ); + + $this->assertSame('', $result); + } + #[Test] #[DefineEnvironment('relativeRouteUrl')] public function it_outputs_a_relative_url_by_default_when_the_glide_route_is_relative()