Skip to content

Unable to specify asset type for assets lookup #129

@AndikanGabriel

Description

@AndikanGabriel

@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.

$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:

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

  1. Please provide guidance on integrating the resource_type parameter into the existing storage driver methods.
  2. Is there a planned update to support multiple resource types and include missing media extensions like mp3?
  3. Is there a better approach to lookup asset details for a video resource type using the Storage facade?
  4. 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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions