-
Notifications
You must be signed in to change notification settings - Fork 78
Description
@joshmanders the current implementation of the asset lookup function in the storage driver defaults to the image
resource type and does not provide an option to specify other resource types, such as video
. This limitation prevents one from retrieving details for video assets using the Storage
facade.
Steps to reproduce
1. Upload a video or audio file
Storage::upload($file)
returns void, I'll use the Cloudinary API directly:
$publicId = \Cloudinary::uploadFile($file)->getPublicId()
2. Check if uploaded file exists using the public id
Storage::disk('cloudinary')->fileExists($publicId)
Expected behaviour
The fileExists
method should return true for files that have been successfully uploaded and exist on Cloudinary's server, regardless of whether the file is an image or video.
Actual behaviour
The fileExists
method returns false for files that are confirmed to exist on the server. This is likely because the current implementation defaults to checking only image resources, ignoring other types such as video.
Debugging
Upon examining the Cloudinary PHP SDK, specifically the method responsible for setting the asset type, and the Cloudinary Laravel Adapter, it is clear that the package defaults to image and lacks an interface to specify other resource types.
cloudinary-laravel/src/CloudinaryAdapter.php
Line 250 in 434bb1f
$this->adminApi()->asset($this->preparePublicId($path)); |
Observe in L250, that there's no interface to specify the resource type. Additionally, I noticed that the media extension property is missing mp3 support: Reference.
My work around
To address this issue, I extended the Cloudinary storage driver and updated the media extension property. For each method relying on the assets
api, I modified the code to first check for image
resources and then fall back to video
resources if the initial check fails. Here is the affected code snippet:
cloudinary-laravel/src/CloudinaryAdapter.php
Lines 277 to 282 in 434bb1f
public function read(string $path): string | |
{ | |
$resource = (array)$this->adminApi()->asset($this->preparePublicId($path)); | |
return file_get_contents($resource['secure_url']); | |
} |
Here is the updated code snippet:
public function read(string $path): string
{
try {
$resource = (array) $this->adminApi()->asset($this->preparePublicId($path));
} catch (Exception) {
$resource = (array) $this->adminApi()->asset(
$this->preparePublicId($path), ['resource_type' => 'video']
);
}
return file_get_contents($resource['secure_url']);
}
This modification ensures that the asset API can handle video and other resource types gracefully.
Additionally, considering Cloudinary supports three resource types, the code could be further enhanced to handle image, video, and raw resources:
foreach (['image', 'video', 'raw'] as $type) {
try {
$this->adminApi()->asset(
$this->preparePublicId($path), ['resource_type' => $type]
);
return true;
} catch (Exception) {
}
}
return false;
Request
- Please provide guidance on integrating the
resource_type
parameter into the existing storage driver methods. - Is there a planned update to support multiple resource types and include missing media extensions like
mp3
? - Is there a better approach to lookup asset details for a
video
resource type using theStorage
facade? - Does my modification align with the package's code standard? If so, would a PR be welcomed?
Thank you for your attention to this request. Improving support for various resource types will significantly enhance the flexibility and functionality of the package.