cloudinary-community / cloudinary-laravel

Laravel SDK for Cloudinary
MIT License
255 stars 71 forks source link

Flysystem problem for Laravel 9 #64

Closed chrisschale closed 1 year ago

chrisschale commented 2 years ago

Hey,

in version 1.0.5 the CloudinaryAdapter still uses the NotSupportingVisibilityTrait which doesn't exist anymore in Flysystem 3. The latter is used in Laravel 9.

Best regards, Chris

lukdiekm commented 2 years ago

Same problem here

tobz-nz commented 2 years ago

This issue still remains.

55 only enabled it to be installed on Laravel 9, but did not ensure full comparability

Related: #57 and #59

rushi7997 commented 2 years ago

Can someone Please fix this?

phuclh commented 2 years ago

I have the same issue

unicodeveloper commented 2 years ago

@phuclh what's the issue you experience here? Can you be more detailed about it so I can fix it ASAP.

phuclh commented 2 years ago

in version 1.0.5 the CloudinaryAdapter still uses the NotSupportingVisibilityTrait which doesn't exist anymore in Flysystem 3. The latter is used in Laravel 9.

In version 1.0.5, the CloudinaryAdapter still uses the NotSupportingVisibilityTrait which doesn't exist anymore in Flysystem 3.

rushi7997 commented 2 years ago

@unicodeveloper, can you please fix this issue? I am migrating my application to Laravel 9, and I am stuck because of this package. Thanks

unicodeveloper commented 2 years ago

@rushi7997 please submit a PR.

phuclh commented 2 years ago

Hi @unicodeveloper , do you or Cloundinary team have any plans to fix this issue?

unicodeveloper commented 2 years ago

Please send in a PR

On Thu, Apr 28, 2022 at 4:57 AM phuclh @.***> wrote:

Hi @unicodeveloper https://github.com/unicodeveloper , do you or Cloundinary team have any plans to fix this issue?

— Reply to this email directly, view it on GitHub https://github.com/cloudinary-labs/cloudinary-laravel/issues/64#issuecomment-1111655666, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAWPNUM4OPT4XWJWE2S2U5TVHHV7RANCNFSM5R3AKGVA . You are receiving this because you were mentioned.Message ID: @.***>

-- Best Regards,

Prosper Otemuyiwa Co-founder, Eden @unicodeveloper https://twitter.com/unicodeveloper | prosper.otemuyiwa1 ( Skype )

brandon14 commented 2 years ago

@unicodeveloper It seems that the adapter shipped with this package was written for a 1.x flysystem adapter, so in order to truly support Laravel 9.x, it will likely involve rewriting the CloudinaryAdapter to be compatible with flysystem 3.x.

I am not sure if a flysystem adapter supports both 1.x and 3.x is possible because I haven't looked to much into the documentation for flysystem, but the notable change sin 2.x and 3.x can be found here: https://flysystem.thephpleague.com/docs/what-is-new/

rushi7997 commented 2 years ago

According to the documentation here

Every adapter must implement the League\Flysystem\FilesystemAdapter interface. This interface defines all the required methods and lists which exceptions should be thrown in case of failure.

we have to rewrite CloudinaryAdapter implementing FilesystemAdapter from flysystem 3.x

the current version implements the interface AdapterInterface which is no longer available in flysystem 3.x

phuclh commented 2 years ago

I re-wrote the CloudinaryAdapter that implements Flysystem 3.x here: https://github.com/phuclh/flysystem-cloudinary. This package is just flysystem driver. It doesn't have all features like this package but it works with Laravel 9

unicodeveloper commented 1 year ago

@rushi7997 @brandon14 @phuclh I just tagged a v2.0.0 that solves this issue. Everyone on Laravel 9 can use the package without issues now.

brandon14 commented 1 year ago

@unicodeveloper I think there still may be some issues with this in applications that stick strictly to the Flysystem contracts. The listContents() method, should return an iterable, but the elements must be instances of StorageAttributes, not plain arrays.

So in something like Statamic for example, if using this as an asset driver, it breaks because the Statamic code expects the listContents() of a 3.x Flysystem adapter to adhere to the contact defined for Flysystem 3.x.

/**
     * @return iterable<StorageAttributes>
     *
     * @throws FilesystemException
     */
    public function listContents(string $path, bool $deep): iterable;

I don't care to submit a PR targeting the 2.x branch of this package to change the return of that method to return an iterable of StorageAttributes to solve this issue.

unicodeveloper commented 1 year ago

@brandon14 you have a point. I missed this. Please can you submit a PR targeting the dev-v2 branch? I'll really appreciate.