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

[9.x] Flysystem v2 #33612

Merged
merged 7 commits into from
Nov 30, 2020
Merged

[9.x] Flysystem v2 #33612

merged 7 commits into from
Nov 30, 2020

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Jul 21, 2020

Initial attempt at migrating to (currently unreleased) Flysystem v2. Might be a little too soon for Laravel 8. In draft until Flysystem v2 has been released.

Breaking Changes

  • Any write operations (like put, write, writeStream, etc) now overwrites by default. Checking for existing files needs to be done in user land from now on.
  • writeStream no longer throws a FileExistsException. Since this exception is nowhere used anymore in the framework, it's been removed.
  • Similarly, get & readStream no longer throw a FileNotFoundException exception when a file exists. This is because the underlying Flysystem integration no longer distinguishes this specific failure from any other read failure. Both methods will return null when they cannot read a file.
  • delete returns true now even if the file wasn't found.
  • Using custom filesystem creators in a service provider now requires them to return an instance of Illuminate\Contracts\Filesystem\Filesystem rather than League\Flysystem\FilesystemInterface like before. See [9.x] Flysystem v2 #33612 (comment)
  • The FTP driver now ships as a separate league package and needs to be explicitly required through Composer for it to be used.
  • The concept of the cached adapter has been removed from Flysystem v2 and thus also from Laravel.
  • Illuminate\Filesystem\FilesystemAdapter's constructor now requires an extra $adapter argument and an optional $options argument.

@driesvints driesvints marked this pull request as draft July 21, 2020 19:28
}

return $driver;
return $this->customCreators[$config['driver']]($this->app, $config);
Copy link
Member Author

@driesvints driesvints Jul 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes require custom creators to return an instance of Illuminate\Contracts\Filesystem\Filesystem rather than an instance of League\Flysystem\FilesystemInterface. The reason for that is that creating a FilesystemAdapter instance is now much more complex than what the previous adapt method did (see a bit further below).

@driesvints
Copy link
Member Author

driesvints commented Aug 4, 2020

Feel free to ignore the failing "prefer-lowest" tests as they test the old alpha.1 which get pulled in because of composer's "prefer-lowest" flag. The most recent version works.

Fixed.

composer.json Outdated Show resolved Hide resolved
@driesvints driesvints changed the title [8.x] Flysystem v2 [9.x] Flysystem v2 Aug 11, 2020
@driesvints driesvints force-pushed the flysystem-v2 branch 2 times, most recently from 8f679ed to ee36968 Compare August 27, 2020 15:05
@driesvints driesvints changed the base branch from master to 8.x September 28, 2020 15:09
@driesvints driesvints changed the base branch from 8.x to master September 28, 2020 15:09
@driesvints driesvints marked this pull request as ready for review November 27, 2020 16:48
composer.json Outdated Show resolved Hide resolved
@taylorotwell taylorotwell merged commit 6095161 into master Nov 30, 2020
@taylorotwell taylorotwell deleted the flysystem-v2 branch November 30, 2020 16:23
taylorotwell pushed a commit that referenced this pull request Mar 1, 2021
…ound (#36407)

* [9.x] Fixing the Error : Class "League\Flysystem\Adapter\Local" not found

This is related to the new Flysystem v2 (Check the PR #33612)

* Update VendorPublishCommand.php
@taylorotwell
Copy link
Member

taylorotwell commented Jan 5, 2022

I need quite a bit more info on:

Illuminate\Filesystem\FilesystemAdapter's constructor now requires an extra $adapter argument and an optional $options argument.

I have to document more info than this - what should I include specifically? Can you give me a before and after example of something that might affect user land code?

Using custom filesystem creators in a service provider now requires them to return an instance of Illuminate\Contracts\Filesystem\Filesystem rather than League\Flysystem\FilesystemInterface like before. See [9.x] Flysystem v2 #33612 (comment)

Does this mean all custom disk implementations are now broken? If so, can you provide a before and after example of how to fix a hypothetical custom disk so that I can properly document the process in the upgrade guide. Otherwise, users will have nothing to go on if all I mention is the sentence above. In other words, can you give me an example of how to define a custom disk in Laravel 8.x (there is one in docs) and then an example of how to do it in Laravel 9.x?

@driesvints
Copy link
Member Author

driesvints commented Jan 6, 2022

I have to document more info than this - what should I include specifically? Can you give me a before and after example of something that might affect user land code?

No real userland code is affected here, only when people are instantiating their own filesystem adapters or packages adding new drivers.

use Illuminate\Filesystem\FilesystemAdapter;

$flysystem = new Flysystem($adapter, $config);

// Before...
$filesystem = new FilesystemAdapter($flysystem);

// After...
return new FilesystemAdapter($flysystem, $adapter, $config);

Does this mean all custom disk implementations are now broken? If so, can you provide a before and after example of how to fix a hypothetical custom disk so that I can properly document the process in the upgrade guide. Otherwise, users will have nothing to go on if all I mention is the sentence above. In other words, can you give me an example of how to define a custom disk in Laravel 8.x (there is one in docs) and then an example of how to do it in Laravel 9.x?

Yes, custom disks will need to update their implementation.

// Before...
use Illuminate\Support\Facades\Storage;
use League\Flysystem\Filesystem;
use Spatie\Dropbox\Client as DropboxClient;
use Spatie\FlysystemDropbox\DropboxAdapter;

Storage::extend('dropbox', function ($app, $config) {
    $client = new DropboxClient(
        $config['authorization_token']
    );

    return new Filesystem(new DropboxAdapter($client));
});

// After...
use Illuminate\Filesystem\FilesystemAdapter;
use Illuminate\Support\Facades\Storage;
use League\Flysystem\Filesystem as Flysystem;
use Spatie\Dropbox\Client as DropboxClient;
use Spatie\FlysystemDropbox\DropboxAdapter;

Storage::extend('dropbox', function ($app, $config) {
    $client = new DropboxClient(
        $config['authorization_token']
    );

    $adapter = new DropboxAdapter($client);
    $flysystem = new Flysystem($adapter, $config);

    return new FilesystemAdapter($flysystem, $adapter, $config);
});

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

Successfully merging this pull request may close these issues.

4 participants