Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flysystem problem for Laravel 9 #64

Closed
chrisschale opened this issue Mar 28, 2022 · 16 comments
Closed

Flysystem problem for Laravel 9 #64

chrisschale opened this issue Mar 28, 2022 · 16 comments

Comments

@chrisschale
Copy link

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
Copy link

Same problem here

@tobz-nz
Copy link

tobz-nz commented Apr 4, 2022

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
Copy link

Can someone Please fix this?

@phuclh
Copy link

phuclh commented Apr 20, 2022

I have the same issue

@unicodeveloper
Copy link
Collaborator

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

@phuclh
Copy link

phuclh commented Apr 21, 2022

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
Copy link

@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
Copy link
Collaborator

@rushi7997 please submit a PR.

@phuclh
Copy link

phuclh commented Apr 28, 2022

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

@unicodeveloper
Copy link
Collaborator

unicodeveloper commented Apr 28, 2022 via email

@brandon14
Copy link

@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
Copy link

rushi7997 commented May 2, 2022

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
Copy link

phuclh commented May 14, 2022

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
Copy link
Collaborator

@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
Copy link

@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
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants