From 8376870b0b92a8044a5f0cc5796edf9c223772d8 Mon Sep 17 00:00:00 2001 From: Jeremy Bloomstrom Date: Sun, 7 Dec 2025 19:23:59 -0900 Subject: [PATCH 1/2] fix prefix behavior --- src/CloudinaryStorageAdapter.php | 45 +++--- tests/Unit/CloudinaryStorageAdapterTest.php | 170 ++++++++++++++++++++ 2 files changed, 195 insertions(+), 20 deletions(-) diff --git a/src/CloudinaryStorageAdapter.php b/src/CloudinaryStorageAdapter.php index 3477d5f..0883229 100644 --- a/src/CloudinaryStorageAdapter.php +++ b/src/CloudinaryStorageAdapter.php @@ -12,6 +12,7 @@ use League\MimeTypeDetection\FinfoMimeTypeDetector; use League\MimeTypeDetection\MimeTypeDetector; + class CloudinaryStorageAdapter implements ChecksumProvider, FilesystemAdapter { public function __construct( @@ -195,33 +196,33 @@ public function checksum(string $path, Config $config): string public function prepareResource(string $path): array { - $info = pathinfo($path); - - // Ensure dirname uses forward slashes, regardless of OS - $dirname = str_replace('\\', '/', $info['dirname']); - // Always use forward slash for path construction - $id = $dirname.'/'.$info['filename']; - - $mimeType = $this->mimeTypeDetector->detectMimeTypeFromPath($path); + // Normalize the whole path first so pathinfo treats directories correctly + $normalizedPath = str_replace('\\', '/', $path); - if (strpos($mimeType, 'image/') === 0) { - return [$id, 'image']; - } + $info = pathinfo($normalizedPath); - if (strpos($mimeType, 'video/') === 0) { - return [$id, 'video']; - } + // Build id using forward slashes only + $id = ($info['dirname'] ?? '.').'/'.($info['filename'] ?? ''); - // If a prefix is configured, apply it to the id. When applying a prefix - // strip any leading './' or '/' from the generated id so we don't end up - // with paths like "prefix/./file". + // Apply optional prefix, if necessary; strip leading ./ or / from the id when prefixing if ($this->prefix !== '') { $normalizedId = ltrim($id, './\\/'); - $id = $this->prefix. - ($normalizedId !== '' ? '/'.$normalizedId : ''); + + if (! str_starts_with($normalizedId, $this->prefix)) { + $id = rtrim($this->prefix.'/'.$normalizedId, '/'); + } } - return [$id, 'raw']; + // Detect MIME type from the normalized path + $mimeType = $this->mimeTypeDetector->detectMimeTypeFromPath($normalizedPath) ?? ''; + + $type = match (true) { + str_starts_with($mimeType, 'image/') => 'image', + str_starts_with($mimeType, 'video/') => 'video', + default => 'raw', + }; + + return [$id, $type]; } private function applyPrefixToPath(string $path): string @@ -232,6 +233,10 @@ private function applyPrefixToPath(string $path): string $trimmed = ltrim(str_replace('\\', '/', $path), '\/'); + if (str_starts_with($trimmed, $this->prefix)) { + return $path; + } + return $this->prefix.($trimmed !== '' ? '/'.$trimmed : ''); } } diff --git a/tests/Unit/CloudinaryStorageAdapterTest.php b/tests/Unit/CloudinaryStorageAdapterTest.php index c76bb39..97dbb74 100644 --- a/tests/Unit/CloudinaryStorageAdapterTest.php +++ b/tests/Unit/CloudinaryStorageAdapterTest.php @@ -211,6 +211,176 @@ function createApiResponse(array $data, int $statusCode = 200): ApiResponse $this->adapter->move('Fixtures/source.jpg', 'Fixtures/destination.jpg', new Config); }); +it('uses image resource type for images and builds forward-slash id', function () { + // Given a path with backslashes and nested dirs + $path = 'dir\nested\picture.jpg'; + + // Expect Cloudinary upload called with normalized public_id and image resource type + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'dir/nested/picture' + && $options['resource_type'] === 'image'; + }) + )->willReturn(createApiResponse(['public_id' => 'dir/nested/picture']))->shouldBeCalled(); + + $this->adapter->write($path, 'contents', new Config); +}); + +it('uses video resource type for videos', function () { + $path = 'media/video.mp4'; + + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'media/video' + && $options['resource_type'] === 'video'; + }) + )->willReturn(createApiResponse(['public_id' => 'media/video']))->shouldBeCalled(); + + $this->adapter->write($path, 'contents', new Config); +}); + +it('defaults to raw resource type for non-image/video', function () { + $path = 'docs/file.pdf'; + + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'docs/file' + && $options['resource_type'] === 'raw'; + }) + )->willReturn(createApiResponse(['public_id' => 'docs/file']))->shouldBeCalled(); + + $this->adapter->write($path, 'contents', new Config); +}); + +it('applies prefix and strips leading dot-slash', function () { + // Using the adapter configured with prefix `Fixtures` + $path = './file.jpg'; + + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'Fixtures/file' + && $options['resource_type'] === 'image'; + }) + )->willReturn(createApiResponse(['public_id' => 'Fixtures/file']))->shouldBeCalled(); + + $this->prefixedAdapter->write($path, 'contents', new Config); +}); + +it('applies prefix and strips leading slash', function () { + $path = '/file.jpg'; + + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'Fixtures/file' + && $options['resource_type'] === 'image'; + }) + )->willReturn(createApiResponse(['public_id' => 'Fixtures/file']))->shouldBeCalled(); + + $this->prefixedAdapter->write($path, 'contents', new Config); +}); + +it('applies prefix and normalizes backslashes in nested paths', function () { + $path = 'subdir\\inner\\file.jpg'; + + $this->uploadApi->upload( + Argument::type('string'), + Argument::that(function ($options) { + return $options['public_id'] === 'Fixtures/subdir/inner/file' + && $options['resource_type'] === 'image'; + }) + )->willReturn(createApiResponse(['public_id' => 'Fixtures/subdir/inner/file']))->shouldBeCalled(); + + $this->prefixedAdapter->write($path, 'contents', new Config); +}); + +it('getUrl returns secure url for image and normalizes id', function () { + // No prefix adapter + // Ensure nested path is preserved and extension is removed + $this->adminApi->asset( + Argument::exact('images/photo'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'image') + )->willReturn(createApiResponse([ + 'public_id' => 'images/photo', + 'secure_url' => 'https://cdn.example.com/images/photo.jpg', + ]))->shouldBeCalled(); + + $url = $this->adapter->getUrl('images/photo.jpg'); + expect($url)->toBe('https://cdn.example.com/images/photo.jpg'); +}); + +it('getUrl uses video resource type for videos', function () { + $this->adminApi->asset( + Argument::exact('media/clip'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'video') + )->willReturn(createApiResponse([ + 'public_id' => 'media/clip', + 'secure_url' => 'https://cdn.example.com/media/clip.mp4', + ]))->shouldBeCalled(); + + $url = $this->adapter->getUrl('media/clip.mp4'); + expect($url)->toBe('https://cdn.example.com/media/clip.mp4'); +}); + +it('getUrl applies configured prefix when needed', function () { + // Adapter with prefix = Fixtures + $this->adminApi->asset( + Argument::exact('Fixtures/file'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'image') + )->willReturn(createApiResponse([ + 'public_id' => 'Fixtures/file', + 'secure_url' => 'https://cdn.example.com/Fixtures/file.jpg', + ]))->shouldBeCalled(); + + $url = $this->prefixedAdapter->getUrl('file.jpg'); + expect($url)->toBe('https://cdn.example.com/Fixtures/file.jpg'); +}); + +it('getUrl does not double-apply prefix when path already includes prefix', function () { + // Simulate Laravel passing already-prefixed path to adapter + $this->adminApi->asset( + Argument::exact('Fixtures/file'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'image') + )->willReturn(createApiResponse([ + 'public_id' => 'Fixtures/file', + 'secure_url' => 'https://cdn.example.com/Fixtures/file.jpg', + ]))->shouldBeCalled(); + + $url = $this->prefixedAdapter->getUrl('Fixtures/file.jpg'); + expect($url)->toBe('https://cdn.example.com/Fixtures/file.jpg'); +}); + +it('getUrl does not double-apply prefix when path starts with a leading slash', function () { + // Regression: ensure no `Fixtures/Fixtures` and no leading slash in id + $this->adminApi->asset( + Argument::exact('/Fixtures/file'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'image') + )->willReturn(createApiResponse([ + 'public_id' => 'Fixtures/file', + 'secure_url' => 'https://cdn.example.com/Fixtures/file.jpg', + ]))->shouldBeCalled(); + + $url = $this->prefixedAdapter->getUrl('/Fixtures/file.jpg'); + expect($url)->toBe('https://cdn.example.com/Fixtures/file.jpg'); +}); + +it('getUrl normalizes backslashes in incoming path', function () { + $this->adminApi->asset( + Argument::exact('Fixtures/sub/dir/file'), + Argument::that(fn ($opts) => $opts['resource_type'] === 'image') + )->willReturn(createApiResponse([ + 'public_id' => 'Fixtures/sub/dir/file', + 'secure_url' => 'https://cdn.example.com/Fixtures/sub/dir/file.jpg', + ]))->shouldBeCalled(); + + $url = $this->prefixedAdapter->getUrl('sub\\dir\\file.jpg'); + expect($url)->toBe('https://cdn.example.com/Fixtures/sub/dir/file.jpg'); +}); + it('can calculate checksum', function () { // })->todo(); From 8f968b7cda3aafdaefae2e08eac195bd22bbf95a Mon Sep 17 00:00:00 2001 From: Jeremy Bloomstrom Date: Sat, 13 Dec 2025 08:19:36 -0900 Subject: [PATCH 2/2] remove double prefix, if applicable --- src/CloudinaryStorageAdapter.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/CloudinaryStorageAdapter.php b/src/CloudinaryStorageAdapter.php index 0883229..5bbdfaf 100644 --- a/src/CloudinaryStorageAdapter.php +++ b/src/CloudinaryStorageAdapter.php @@ -12,7 +12,6 @@ use League\MimeTypeDetection\FinfoMimeTypeDetector; use League\MimeTypeDetection\MimeTypeDetector; - class CloudinaryStorageAdapter implements ChecksumProvider, FilesystemAdapter { public function __construct( @@ -211,6 +210,11 @@ public function prepareResource(string $path): array if (! str_starts_with($normalizedId, $this->prefix)) { $id = rtrim($this->prefix.'/'.$normalizedId, '/'); } + + // Remove the double prefix if it exists + if (str_contains($id, $this->prefix.'/'.$this->prefix)) { + $id = str_replace($this->prefix.'/'.$this->prefix, $this->prefix, $id); + } } // Detect MIME type from the normalized path