Filesystem Extender and Tests

[askvortsov1]

Followup to #2729
Part of #1783
Part of flarum/issue-archive#121

Changes proposed in this pull request:

• Use our own subclass of Laravel's filesystem manager, which gives us control over disk resolution, and most important, creating drivers for disks.
• Add a filesystem extender that allows:
  • Declaring a new disk (see docblock for explanation)
  • Registering a filesystem driver

Currently, drivers for disks are determined via the following process:

1. If a disk_driver.${DISK_NAME} setting key exists, proceed to step 2. Else return 'local'
2. If a driver with the name returned by step 1 exists, use that. Else, use 'local'

This gives us safety in case the extension providing the filesystem driver being used is removed.

This PR does not attempt to introduce a UI for:

• Selecting drivers for disks
• Selecting settings for disk drivers

We might want to eventually add that though.

Reviewers should focus on:
Do we want to use config.php or settings? Right now, this PR selects the driver for each disk from settings. It passes both settings and config to drivers, so they can choose which they prefer. Should we also support setting drivers for disks in config?

I also put together a proof of concept extension for Azure at https://github.com/askvortsov1/flarum-azure-poc.

Confirmed

• Frontend changes: tested on a local Flarum installation.
• Backend changes: tests are green (run composer test).

[tankerkiller125]

I think I'd prefer config.php, however I may have a bias towards that since Laravel uses a filesystem.php config file. However if a proper UI can be introduced I think that could work just as well, especially given that Flarum can't operate without a database anyways.

[askvortsov1]

I think I'd prefer config.php, however I may have a bias towards that since Laravel uses a filesystem.php config file. However if a proper UI can be introduced I think that could work just as well, especially given that Flarum can't operate without a database anyways.

For mapping disks to drivers, we could support both: first check settings, then config.php, and if none have a value, default to local?

[tankerkiller125]

That works too

[luceos]

I don't understand why we are extending the Laravel filesystem manager only to return a Flysystem Filesystem afterwards (see DriverInterface).

Other than that experience will tell whether this helps and callables might be hard to use as they don't have type hinting, but that was pointed out by @clarkwinkelmann in the past as well.

[luceos]

I dislike disk_driver it strokes against the Laravel naming convention and the manager elsewhere uses filesystems too. Can we use filesystem or the plural version?

[askvortsov1]

In the Laravel config approach, there is an array of "disks", with each "disk" having a "driver" field. In this particular place, that is not inaccurate.

We could replace the 'flarum.filesystem.drivers' binding key with 'flarum.filesystem.filesystems'?

[SychO9]

I do agree that disk_driver is more appropriate here.

[askvortsov1]

I don't understand why we are extending the Laravel filesystem manager only to return a Flysystem Filesystem afterwards (see DriverInterface).

We are extending (and overriding) the filesystem manager so that we have control over:

• How new disks are declared
• How disks are mapped to drivers
• How available drivers are registered
• How config for drivers is obtained

That being said, I think you have a good point that it's a bit odd that the output is a Flysystem interface. I did this because the vast majority of drivers will probably use flysystem so we might as well wrap in FilesystemAdapter for them, but that's not too clean. We could probably require the output to match the laravel Cloud interface.

[SychO9]

Code looks good!

Just have a few minor nitpicks.