From 4fea0ebdeeab5890ee13bdb8aa402e493c5de0a1 Mon Sep 17 00:00:00 2001
From: Alexander Skvortsov <38059171+askvortsov1@users.noreply.github.com>
Date: Mon, 19 Apr 2021 16:25:08 -0400
Subject: [PATCH] Filesystem Extender and Tests (#2732)

---
 framework/core/src/Extend/Filesystem.php      |  85 ++++++++++
 .../core/src/Extension/ExtensionManager.php   |  28 ++--
 .../core/src/Filesystem/DriverInterface.php   |  39 +++++
 .../core/src/Filesystem/FilesystemManager.php |  83 ++++++++++
 .../Filesystem/FilesystemServiceProvider.php  |  63 +++++++
 .../core/src/Foundation/InstalledSite.php     |  18 +-
 .../integration/extenders/FilesystemTest.php  | 156 ++++++++++++++++++
 7 files changed, 442 insertions(+), 30 deletions(-)
 create mode 100644 framework/core/src/Extend/Filesystem.php
 create mode 100644 framework/core/src/Filesystem/DriverInterface.php
 create mode 100644 framework/core/src/Filesystem/FilesystemManager.php
 create mode 100644 framework/core/src/Filesystem/FilesystemServiceProvider.php
 create mode 100644 framework/core/tests/integration/extenders/FilesystemTest.php

diff --git a/framework/core/src/Extend/Filesystem.php b/framework/core/src/Extend/Filesystem.php
new file mode 100644
index 000000000..d3c75bbb4
--- /dev/null
+++ b/framework/core/src/Extend/Filesystem.php
@@ -0,0 +1,85 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Extend;
+
+use Flarum\Extension\Extension;
+use Flarum\Foundation\ContainerUtil;
+use Illuminate\Contracts\Container\Container;
+
+class Filesystem implements ExtenderInterface
+{
+    private $disks = [];
+    private $drivers = [];
+
+    /**
+     * Declare a new filesystem disk.
+     * Disks represent storage locations, and are backed by storage drivers.
+     * Flarum core uses disks for storing assets and avatars.
+     *
+     * By default, the "local" driver will be used for disks.
+     * The "local" driver represents the filesystem where your Flarum installation is running.
+     *
+     * To declare a new disk, you must provide default configuration a "local" driver.
+     *
+     * @param string $name: The name of the disk
+     * @param string|callable $callback: A callback or invokable class name with parameters:
+     *                           - \Flarum\Foundation\Paths $paths
+     *                           - \Flarum\Http\UrlGenerator $url
+     *                         which returns a Laravel disk config array.
+     *                         The `driver` key is not necessary for this array, and will be ignored.
+     *
+     * @example
+     * ```
+     * ->disk('flarum-uploads', function (Paths $paths, UrlGenerator $url) {
+     *       return [
+     *          'root'   => "$paths->public/assets/uploads",
+     *          'url'    => $url->to('forum')->path('assets/uploads')
+     *       ];
+     *   });
+     * ```
+     *
+     * @see https://laravel.com/docs/8.x/filesystem#configuration
+     */
+    public function disk(string $name, $callback)
+    {
+        $this->disks[$name] = $callback;
+
+        return $this;
+    }
+
+    /**
+     * Register a new filesystem driver.
+     * Drivers must implement `\Flarum\Filesystem\DriverInterface`.
+     *
+     * @param string $name: The name of the driver
+     * @param string $driverClass: The ::class attribute of the driver.
+     */
+    public function driver(string $name, string $driverClass)
+    {
+        $this->drivers[$name] = $driverClass;
+
+        return $this;
+    }
+
+    public function extend(Container $container, Extension $extension = null)
+    {
+        $container->extend('flarum.filesystem.disks', function ($existingDisks) use ($container) {
+            foreach ($this->disks as $name => $disk) {
+                $existingDisks[$name] = ContainerUtil::wrapCallback($disk, $container);
+            }
+
+            return $existingDisks;
+        });
+
+        $container->extend('flarum.filesystem.drivers', function ($existingDrivers) {
+            return array_merge($existingDrivers, $this->drivers);
+        });
+    }
+}
diff --git a/framework/core/src/Extension/ExtensionManager.php b/framework/core/src/Extension/ExtensionManager.php
index 50f2b127a..8ff03c905 100644
--- a/framework/core/src/Extension/ExtensionManager.php
+++ b/framework/core/src/Extension/ExtensionManager.php
@@ -19,8 +19,7 @@ use Flarum\Foundation\Paths;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Illuminate\Contracts\Container\Container;
 use Illuminate\Contracts\Events\Dispatcher;
-use Illuminate\Contracts\Filesystem\Cloud as FilesystemInterface;
-use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Cloud;
 use Illuminate\Database\ConnectionInterface;
 use Illuminate\Database\Schema\Builder;
 use Illuminate\Filesystem\Filesystem;
@@ -50,11 +49,6 @@ class ExtensionManager
      */
     protected $filesystem;
 
-    /**
-     * @var FilesystemInterface
-     */
-    protected $assetsFilesystem;
-
     /**
      * @var Collection|null
      */
@@ -66,8 +60,7 @@ class ExtensionManager
         Container $container,
         Migrator $migrator,
         Dispatcher $dispatcher,
-        Filesystem $filesystem,
-        Factory $filesystemFactory
+        Filesystem $filesystem
     ) {
         $this->config = $config;
         $this->paths = $paths;
@@ -75,7 +68,6 @@ class ExtensionManager
         $this->migrator = $migrator;
         $this->dispatcher = $dispatcher;
         $this->filesystem = $filesystem;
-        $this->assetsFilesystem = $filesystemFactory->disk('flarum-assets');
     }
 
     /**
@@ -261,7 +253,7 @@ class ExtensionManager
      */
     protected function publishAssets(Extension $extension)
     {
-        $extension->copyAssetsTo($this->assetsFilesystem);
+        $extension->copyAssetsTo($this->getAssetsFilesystem());
     }
 
     /**
@@ -271,7 +263,7 @@ class ExtensionManager
      */
     protected function unpublishAssets(Extension $extension)
     {
-        $this->assetsFilesystem->deleteDirectory('extensions/'.$extension->getId());
+        $this->getAssetsFilesystem()->deleteDirectory('extensions/'.$extension->getId());
     }
 
     /**
@@ -283,7 +275,17 @@ class ExtensionManager
      */
     public function getAsset(Extension $extension, $path)
     {
-        return $this->assetsFilesystem->url($extension->getId()."/$path");
+        return $this->getAssetsFilesystem()->url($extension->getId()."/$path");
+    }
+
+    /**
+     * Get an instance of the assets filesystem.
+     * This is resolved dynamically because Flarum's filesystem configuration
+     * might not be booted yet when the ExtensionManager singleton initializes.
+     */
+    protected function getAssetsFilesystem(): Cloud
+    {
+        return resolve('filesystem')->disk('flarum-assets');
     }
 
     /**
diff --git a/framework/core/src/Filesystem/DriverInterface.php b/framework/core/src/Filesystem/DriverInterface.php
new file mode 100644
index 000000000..4d4a7415f
--- /dev/null
+++ b/framework/core/src/Filesystem/DriverInterface.php
@@ -0,0 +1,39 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Filesystem;
+
+use Flarum\Foundation\Config;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Filesystem\Cloud;
+
+interface DriverInterface
+{
+    /**
+     * Construct a Laravel Cloud filesystem for this filesystem driver.
+     * Settings and configuration can either be pulled from the Flarum settings repository
+     * or the config.php file.
+     *
+     * Typically, this is done by wrapping a Flysystem adapter in Laravel's
+     * `Illuminate\Filesystem\FilesystemAdapter` class.
+     * You should ensure that the Flysystem adapter you use has a `getUrl` method.
+     * If it doesn't, you should create a subclass implementing that method.
+     * Otherwise, this driver won't work for public-facing disks
+     * like `flarum-assets` or `flarum-avatars`.
+     *
+     * @param string $diskName: The name of a disk this driver is being used for.
+     *                   This is generally used to locate disk-specific settings.
+     * @param SettingsRepositoryInterface $settings: An instance of the Flarum settings repository.
+     * @param Config $config: An instance of the wrapper class around `config.php`.
+     * @param array $localConfig: The configuration array that would have been used
+     *                            if this disk were using the 'local' filesystem driver.
+     *                            Some of these settings might be useful (e.g. visibility, )
+     */
+    public function build(string $diskName, SettingsRepositoryInterface $settings, Config $config, array $localConfig): Cloud;
+}
diff --git a/framework/core/src/Filesystem/FilesystemManager.php b/framework/core/src/Filesystem/FilesystemManager.php
new file mode 100644
index 000000000..c31fa2e7b
--- /dev/null
+++ b/framework/core/src/Filesystem/FilesystemManager.php
@@ -0,0 +1,83 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Filesystem;
+
+use Flarum\Foundation\Config;
+use Flarum\Foundation\Paths;
+use Flarum\Http\UrlGenerator;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Container\Container;
+use Illuminate\Contracts\Filesystem\Filesystem;
+use Illuminate\Filesystem\FilesystemManager as LaravelFilesystemManager;
+use Illuminate\Support\Arr;
+use InvalidArgumentException;
+
+class FilesystemManager extends LaravelFilesystemManager
+{
+    protected $diskLocalConfig = [];
+    protected $drivers = [];
+
+    public function __construct(Container $app, array $diskLocalConfig, array $drivers)
+    {
+        parent::__construct($app);
+
+        $this->diskLocalConfig = $diskLocalConfig;
+        $this->drivers = $drivers;
+    }
+
+    /**
+     * @inheritDoc
+     */
+    protected function resolve($name): Filesystem
+    {
+        $driver = $this->getDriver($name);
+
+        $localConfig = $this->getLocalConfig($name);
+
+        if (empty($localConfig)) {
+            throw new InvalidArgumentException("Disk [{$name}] has not been declared. Use the Filesystem extender to do this.");
+        }
+
+        if ($driver === 'local') {
+            return $this->createLocalDriver($localConfig);
+        }
+
+        $settings = $this->app->make(SettingsRepositoryInterface::class);
+        $config = $this->app->make(Config::class);
+
+        return $driver->build($name, $settings, $config, $localConfig);
+    }
+
+    /**
+     * @return string|DriverInterface
+     */
+    protected function getDriver(string $name)
+    {
+        $config = $this->app->make(Config::class);
+        $settings = $this->app->make(SettingsRepositoryInterface::class);
+
+        $key = "disk_driver.$name";
+        $configuredDriver = Arr::get($config, $key, $settings->get($key, 'local'));
+
+        return Arr::get($this->drivers, $configuredDriver, 'local');
+    }
+
+    protected function getLocalConfig(string $name): array
+    {
+        if (! array_key_exists($name, $this->diskLocalConfig)) {
+            return [];
+        }
+
+        $paths = $this->app->make(Paths::class);
+        $url = $this->app->make(UrlGenerator::class);
+
+        return $this->diskLocalConfig[$name]($paths, $url);
+    }
+}
diff --git a/framework/core/src/Filesystem/FilesystemServiceProvider.php b/framework/core/src/Filesystem/FilesystemServiceProvider.php
new file mode 100644
index 000000000..9583d4ceb
--- /dev/null
+++ b/framework/core/src/Filesystem/FilesystemServiceProvider.php
@@ -0,0 +1,63 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Filesystem;
+
+use Flarum\Foundation\AbstractServiceProvider;
+use Flarum\Foundation\Paths;
+use Flarum\Http\UrlGenerator;
+use Illuminate\Filesystem\Filesystem;
+
+class FilesystemServiceProvider extends AbstractServiceProvider
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function register()
+    {
+        $this->container->singleton('files', function () {
+            return new Filesystem;
+        });
+
+        $this->container->singleton('flarum.filesystem.disks', function () {
+            return [
+                'flarum-assets' => function (Paths $paths, UrlGenerator $url) {
+                    return [
+                        'root'   => "$paths->public/assets",
+                        'url'    => $url->to('forum')->path('assets')
+                    ];
+                },
+                'flarum-avatars' => function (Paths $paths, UrlGenerator $url) {
+                    return [
+                        'root'   => "$paths->public/assets/avatars",
+                        'url'    => $url->to('forum')->path('assets/avatars')
+                    ];
+                },
+            ];
+        });
+
+        $this->container->singleton('flarum.filesystem.drivers', function () {
+            return [];
+        });
+
+        $this->container->singleton('flarum.filesystem.resolved_drivers', function () {
+            return array_map(function ($driverClass) {
+                return $this->container->make($driverClass);
+            }, $this->container->make('flarum.filesystem.drivers'));
+        });
+
+        $this->container->singleton('filesystem', function () {
+            return new FilesystemManager(
+                $this->container,
+                $this->container->make('flarum.filesystem.disks'),
+                $this->container->make('flarum.filesystem.resolved_drivers')
+            );
+        });
+    }
+}
diff --git a/framework/core/src/Foundation/InstalledSite.php b/framework/core/src/Foundation/InstalledSite.php
index ec0bd8f51..e7f572e4f 100644
--- a/framework/core/src/Foundation/InstalledSite.php
+++ b/framework/core/src/Foundation/InstalledSite.php
@@ -16,6 +16,7 @@ use Flarum\Console\ConsoleServiceProvider;
 use Flarum\Database\DatabaseServiceProvider;
 use Flarum\Discussion\DiscussionServiceProvider;
 use Flarum\Extension\ExtensionServiceProvider;
+use Flarum\Filesystem\FilesystemServiceProvider;
 use Flarum\Filter\FilterServiceProvider;
 use Flarum\Formatter\FormatterServiceProvider;
 use Flarum\Forum\ForumServiceProvider;
@@ -39,7 +40,6 @@ use Illuminate\Contracts\Cache\Repository;
 use Illuminate\Contracts\Cache\Store;
 use Illuminate\Contracts\Container\Container;
 use Illuminate\Filesystem\Filesystem;
-use Illuminate\Filesystem\FilesystemServiceProvider;
 use Illuminate\Hashing\HashServiceProvider;
 use Illuminate\Validation\ValidationServiceProvider;
 use Illuminate\View\ViewServiceProvider;
@@ -166,22 +166,6 @@ class InstalledSite implements SiteInterface
             'mail' => [
                 'driver' => 'mail',
             ],
-            'filesystems' => [
-                'default' => 'local',
-                'cloud' => 's3',
-                'disks' => [
-                    'flarum-assets' => [
-                        'driver' => 'local',
-                        'root'   => $this->paths->public.'/assets',
-                        'url'    => $app->url('assets')
-                    ],
-                    'flarum-avatars' => [
-                        'driver' => 'local',
-                        'root'   => $this->paths->public.'/assets/avatars',
-                        'url'    => $app->url('assets/avatars')
-                    ]
-                ]
-            ],
             'session' => [
                 'lifetime' => 120,
                 'files' => $this->paths->storage.'/sessions',
diff --git a/framework/core/tests/integration/extenders/FilesystemTest.php b/framework/core/tests/integration/extenders/FilesystemTest.php
new file mode 100644
index 000000000..78482a26c
--- /dev/null
+++ b/framework/core/tests/integration/extenders/FilesystemTest.php
@@ -0,0 +1,156 @@
+<?php
+
+/*
+ * This file is part of Flarum.
+ *
+ * For detailed copyright and license information, please view the
+ * LICENSE file that was distributed with this source code.
+ */
+
+namespace Flarum\Tests\integration\extenders;
+
+use Flarum\Extend;
+use Flarum\Filesystem\DriverInterface;
+use Flarum\Foundation\Config;
+use Flarum\Foundation\Paths;
+use Flarum\Http\UrlGenerator;
+use Flarum\Settings\SettingsRepositoryInterface;
+use Flarum\Testing\integration\RetrievesAuthorizedUsers;
+use Flarum\Testing\integration\TestCase;
+use Illuminate\Contracts\Filesystem\Cloud;
+use Illuminate\Filesystem\FilesystemAdapter;
+use InvalidArgumentException;
+use League\Flysystem\Adapter\Local;
+use League\Flysystem\Adapter\NullAdapter;
+use League\Flysystem\Filesystem as LeagueFilesystem;
+
+class FilesystemTest extends TestCase
+{
+    use RetrievesAuthorizedUsers;
+
+    /**
+     * @test
+     */
+    public function custom_disk_doesnt_exist_by_default()
+    {
+        $this->expectException(InvalidArgumentException::class);
+        $this->app()->getContainer()->make('filesystem')->disk('flarum-uploads');
+    }
+
+    /**
+     * @test
+     */
+    public function custom_disk_exists_if_added_and_uses_local_adapter_by_default()
+    {
+        $this->extend((new Extend\Filesystem)->disk('flarum-uploads', function (Paths $paths, UrlGenerator $url) {
+            return [
+                'root' => "$paths->public/assets/uploads",
+                'url'  => $url->to('forum')->path('assets/uploads')
+            ];
+        }));
+
+        $uploadsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-uploads');
+
+        $this->assertEquals(get_class($uploadsDisk->getDriver()->getAdapter()), Local::class);
+    }
+
+    /**
+     * @test
+     */
+    public function custom_disk_exists_if_added_via_invokable_class_and_uses_local_adapter_by_default()
+    {
+        $this->extend((new Extend\Filesystem)->disk('flarum-uploads', UploadsDisk::class));
+
+        $uploadsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-uploads');
+
+        $this->assertEquals(get_class($uploadsDisk->getDriver()->getAdapter()), Local::class);
+    }
+
+    /**
+     * @test
+     */
+    public function disk_uses_local_adapter_if_configured_adapter_unavailable()
+    {
+        $this->app()->getContainer()->make(SettingsRepositoryInterface::class)->set('disk_driver.flarum-assets', 'nonexistent_driver');
+
+        $assetsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets');
+
+        $this->assertEquals(get_class($assetsDisk->getDriver()->getAdapter()), Local::class);
+    }
+
+    /**
+     * @test
+     */
+    public function disk_uses_local_adapter_if_configured_adapter_from_config_file_unavailable()
+    {
+        $this->overrideConfigWithDiskDriver();
+
+        $assetsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets');
+
+        $this->assertEquals(get_class($assetsDisk->getDriver()->getAdapter()), Local::class);
+    }
+
+    /**
+     * @test
+     */
+    public function disk_uses_custom_adapter_if_configured_and_available()
+    {
+        $this->extend(
+            (new Extend\Filesystem)->driver('null', NullFilesystemDriver::class)
+        );
+
+        $this->app()->getContainer()->make(SettingsRepositoryInterface::class)->set('disk_driver.flarum-assets', 'null');
+
+        $assetsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets');
+
+        $this->assertEquals(get_class($assetsDisk->getDriver()->getAdapter()), NullAdapter::class);
+    }
+
+    /**
+     * @test
+     */
+    public function disk_uses_custom_adapter_from_config_file_if_configured_and_available()
+    {
+        $this->extend(
+            (new Extend\Filesystem)->driver('null', NullFilesystemDriver::class)
+        );
+
+        $this->overrideConfigWithDiskDriver();
+
+        $assetsDisk = $this->app()->getContainer()->make('filesystem')->disk('flarum-assets');
+
+        $this->assertEquals(get_class($assetsDisk->getDriver()->getAdapter()), NullAdapter::class);
+    }
+
+    protected function overrideConfigWithDiskDriver()
+    {
+        $tmp = $this->tmpDir();
+        $configArr = include "$tmp/config.php";
+        $configArr = array_merge($configArr, ['disk_driver' => [
+            'flarum-assets' => 'null'
+        ]]);
+
+        $config = new Config($configArr);
+
+        $this->app()->getContainer()->instance('flarum.config', $config);
+    }
+}
+
+class NullFilesystemDriver implements DriverInterface
+{
+    public function build(string $diskName, SettingsRepositoryInterface $settings, Config $config, array $localConfig): Cloud
+    {
+        return new FilesystemAdapter(new LeagueFilesystem(new NullAdapter()));
+    }
+}
+
+class UploadsDisk
+{
+    public function __invoke(Paths $paths, UrlGenerator $url)
+    {
+        return [
+            'root' => "$paths->public/assets/uploads",
+            'url'  => $url->to('forum')->path('assets/uploads')
+        ];
+    }
+}