Use Laravel filesystem interface for assets and avatars (#2729)

* WIP: Use Laravel filesystem interface where possible
* Drop vendorFilesystem
* Support getting URL of cloud-based logo and favicon
* FilesystemAdapter should always be cloud
* Get base avatar URL from filesystem adapter
* Restore deleted getAsset method

Co-authored-by: Alexander Skvortsov <askvortsov1@users.noreply.github.com>

framework/core/src/Api/Controller/DeleteFaviconController.php

@@ -11,8 +11,9 @@ namespace Flarum\Api\Controller;
 
 use Flarum\Http\RequestUtil;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Filesystem;
 use Laminas\Diactoros\Response\EmptyResponse;
-use League\Flysystem\FilesystemInterface;
 use Psr\Http\Message\ServerRequestInterface;
 
 class DeleteFaviconController extends AbstractDeleteController
@@ -23,18 +24,18 @@ class DeleteFaviconController extends AbstractDeleteController
     protected $settings;
 
     /**
-     * @var FilesystemInterface
+     * @var Filesystem
      */
     protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
-     * @param FilesystemInterface $uploadDir
+     * @param Factory $filesystemFactory
      */
-    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
+    public function __construct(SettingsRepositoryInterface $settings, Factory $filesystemFactory)
     {
         $this->settings = $settings;
-        $this->uploadDir = $uploadDir;
+        $this->uploadDir = $filesystemFactory->disk('flarum-assets');
     }
 
     /**
@@ -48,7 +49,7 @@ class DeleteFaviconController extends AbstractDeleteController
 
         $this->settings->set('favicon_path', null);
 
-        if ($this->uploadDir->has($path)) {
+        if ($this->uploadDir->exists($path)) {
             $this->uploadDir->delete($path);
         }
 

framework/core/src/Api/Controller/DeleteLogoController.php

@@ -11,8 +11,9 @@ namespace Flarum\Api\Controller;
 
 use Flarum\Http\RequestUtil;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Filesystem;
 use Laminas\Diactoros\Response\EmptyResponse;
-use League\Flysystem\FilesystemInterface;
 use Psr\Http\Message\ServerRequestInterface;
 
 class DeleteLogoController extends AbstractDeleteController
@@ -23,18 +24,18 @@ class DeleteLogoController extends AbstractDeleteController
     protected $settings;
 
     /**
-     * @var FilesystemInterface
+     * @var Filesystem
      */
     protected $uploadDir;
 
     /**
      * @param SettingsRepositoryInterface $settings
-     * @param FilesystemInterface $uploadDir
+     * @param Factory $filesystemFactory
      */
-    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
+    public function __construct(SettingsRepositoryInterface $settings, Factory $filesystemFactory)
     {
         $this->settings = $settings;
-        $this->uploadDir = $uploadDir;
+        $this->uploadDir = $filesystemFactory->disk('flarum-assets');
     }
 
     /**
@@ -48,7 +49,7 @@ class DeleteLogoController extends AbstractDeleteController
 
         $this->settings->set('logo_path', null);
 
-        if ($this->uploadDir->has($path)) {
+        if ($this->uploadDir->exists($path)) {
             $this->uploadDir->delete($path);
         }
 

framework/core/src/Api/Controller/UploadImageController.php

@@ -11,10 +11,11 @@ namespace Flarum\Api\Controller;
 
 use Flarum\Http\RequestUtil;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Filesystem;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Str;
 use Intervention\Image\Image;
-use League\Flysystem\FilesystemInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use Psr\Http\Message\UploadedFileInterface;
 use Tobscure\JsonApi\Document;
@@ -27,7 +28,7 @@ abstract class UploadImageController extends ShowForumController
     protected $settings;
 
     /**
-     * @var FilesystemInterface
+     * @var Filesystem
      */
     protected $uploadDir;
 
@@ -48,12 +49,12 @@ abstract class UploadImageController extends ShowForumController
 
     /**
      * @param SettingsRepositoryInterface $settings
-     * @param FilesystemInterface $uploadDir
+     * @param Factory $filesystemFactory
      */
-    public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir)
+    public function __construct(SettingsRepositoryInterface $settings, Factory $filesystemFactory)
     {
         $this->settings = $settings;
-        $this->uploadDir = $uploadDir;
+        $this->uploadDir = $filesystemFactory->disk('flarum-assets');
     }
 
     /**
@@ -73,7 +74,7 @@ abstract class UploadImageController extends ShowForumController
 
         $uploadName = $this->filenamePrefix.'-'.Str::lower(Str::random(8)).'.'.$this->fileExtension;
 
-        $this->uploadDir->write($uploadName, $encodedImage);
+        $this->uploadDir->put($uploadName, $encodedImage);
 
         $this->settings->set($this->filePathSettingKey, $uploadName);
 

framework/core/src/Api/Serializer/ForumSerializer.php

@@ -13,6 +13,8 @@ use Flarum\Foundation\Application;
 use Flarum\Foundation\Config;
 use Flarum\Http\UrlGenerator;
 use Flarum\Settings\SettingsRepositoryInterface;
+use Illuminate\Contracts\Filesystem\Cloud;
+use Illuminate\Contracts\Filesystem\Factory;
 
 class ForumSerializer extends AbstractSerializer
 {
@@ -36,14 +38,21 @@ class ForumSerializer extends AbstractSerializer
      */
     protected $url;
 
+    /**
+     * @var Cloud
+     */
+    protected $assetsFilesystem;
+
     /**
      * @param Config $config
+     * @param Factory $filesystemFactory
      * @param SettingsRepositoryInterface $settings
      * @param UrlGenerator $url
      */
-    public function __construct(Config $config, SettingsRepositoryInterface $settings, UrlGenerator $url)
+    public function __construct(Config $config, Factory $filesystemFactory, SettingsRepositoryInterface $settings, UrlGenerator $url)
     {
         $this->config = $config;
+        $this->assetsFilesystem = $filesystemFactory->disk('flarum-assets');
         $this->settings = $settings;
         $this->url = $url;
     }
@@ -107,7 +116,7 @@ class ForumSerializer extends AbstractSerializer
     {
         $logoPath = $this->settings->get('logo_path');
 
-        return $logoPath ? $this->url->to('forum')->path('assets/'.$logoPath) : null;
+        return $logoPath ? $this->getAssetUrl($logoPath) : null;
     }
 
     /**
@@ -117,6 +126,11 @@ class ForumSerializer extends AbstractSerializer
     {
         $faviconPath = $this->settings->get('favicon_path');
 
-        return $faviconPath ? $this->url->to('forum')->path('assets/'.$faviconPath) : null;
+        return $faviconPath ? $this->getAssetUrl($faviconPath) : null;
+    }
+
+    public function getAssetUrl($assetPath): string
+    {
+        return $this->assetsFilesystem->url($assetPath);
     }
 }

framework/core/src/Extension/Extension.php

@@ -12,15 +12,12 @@ namespace Flarum\Extension;
 use Flarum\Database\Migrator;
 use Flarum\Extend\LifecycleInterface;
 use Illuminate\Contracts\Container\Container;
+use Illuminate\Contracts\Filesystem\Filesystem as FilesystemInterface;
 use Illuminate\Contracts\Support\Arrayable;
+use Illuminate\Filesystem\Filesystem;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
-use League\Flysystem\Adapter\Local;
-use League\Flysystem\Filesystem;
-use League\Flysystem\FilesystemInterface;
-use League\Flysystem\MountManager;
-use League\Flysystem\Plugin\ListFiles;
 
 /**
  * @property string $name
@@ -428,16 +425,13 @@ class Extension implements Arrayable
             return;
         }
 
-        $mount = new MountManager([
+        $source = new Filesystem();
-            'source' => $source = new Filesystem(new Local($this->getPath().'/assets')),
-            'target' => $target,
-        ]);
 
-        $source->addPlugin(new ListFiles);
+        $assetFiles = $source->allFiles("$this->path/assets");
-        $assetFiles = $source->listFiles('/', true);
 
-        foreach ($assetFiles as $file) {
+        foreach ($assetFiles as $fullPath) {
-            $mount->copy("source://$file[path]", "target://extensions/$this->id/$file[path]");
+            $relPath = substr($fullPath, strlen("$this->path/assets"));
+            $target->put("extensions/$this->id/$relPath", $source->get($fullPath));
         }
     }
 

framework/core/src/Extension/ExtensionManager.php

@@ -19,6 +19,8 @@ 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\Database\ConnectionInterface;
 use Illuminate\Database\Schema\Builder;
 use Illuminate\Filesystem\Filesystem;
@@ -48,6 +50,11 @@ class ExtensionManager
      */
     protected $filesystem;
 
+    /**
+     * @var FilesystemInterface
+     */
+    protected $assetsFilesystem;
+
     /**
      * @var Collection|null
      */
@@ -59,7 +66,8 @@ class ExtensionManager
         Container $container,
         Migrator $migrator,
         Dispatcher $dispatcher,
-        Filesystem $filesystem
+        Filesystem $filesystem,
+        Factory $filesystemFactory
     ) {
         $this->config = $config;
         $this->paths = $paths;
@@ -67,6 +75,7 @@ class ExtensionManager
         $this->migrator = $migrator;
         $this->dispatcher = $dispatcher;
         $this->filesystem = $filesystem;
+        $this->assetsFilesystem = $filesystemFactory->disk('flarum-assets');
     }
 
     /**
@@ -252,12 +261,7 @@ class ExtensionManager
      */
     protected function publishAssets(Extension $extension)
     {
-        if ($extension->hasAssets()) {
+        $extension->copyAssetsTo($this->assetsFilesystem);
-            $this->filesystem->copyDirectory(
-                $extension->getPath().'/assets',
-                $this->paths->public.'/assets/extensions/'.$extension->getId()
-            );
-        }
     }
 
     /**
@@ -267,7 +271,7 @@ class ExtensionManager
      */
     protected function unpublishAssets(Extension $extension)
     {
-        $this->filesystem->deleteDirectory($this->paths->public.'/assets/extensions/'.$extension->getId());
+        $this->assetsFilesystem->deleteDirectory('extensions/'.$extension->getId());
     }
 
     /**
@@ -279,7 +283,7 @@ class ExtensionManager
      */
     public function getAsset(Extension $extension, $path)
     {
-        return $this->paths->public.'/assets/extensions/'.$extension->getId().$path;
+        return $this->assetsFilesystem->url($extension->getId()."/$path");
     }
 
     /**

framework/core/src/Foundation/InstalledSite.php

@@ -177,7 +177,8 @@ class InstalledSite implements SiteInterface
                     ],
                     'flarum-avatars' => [
                         'driver' => 'local',
-                        'root'   => $this->paths->public.'/assets/avatars'
+                        'root'   => $this->paths->public.'/assets/avatars',
+                        'url'    => $app->url('assets/avatars')
                     ]
                 ]
             ],

framework/core/src/Install/Steps/EnableBundledExtensions.php

@@ -15,6 +15,7 @@ use Flarum\Extension\Extension;
 use Flarum\Install\Step;
 use Flarum\Settings\DatabaseSettingsRepository;
 use Illuminate\Database\ConnectionInterface;
+use Illuminate\Filesystem\FilesystemAdapter;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
 use League\Flysystem\Adapter\Local;
@@ -79,7 +80,7 @@ class EnableBundledExtensions implements Step
         foreach ($extensions as $extension) {
             $extension->migrate($this->getMigrator());
             $extension->copyAssetsTo(
-                new Filesystem(new Local($this->assetPath))
+                new FilesystemAdapter(new Filesystem(new Local($this->assetPath)))
             );
         }
 

framework/core/src/Settings/SettingsServiceProvider.php

@@ -9,15 +9,8 @@
 
 namespace Flarum\Settings;
 
-use Flarum\Api\Controller\DeleteFaviconController;
-use Flarum\Api\Controller\DeleteLogoController;
-use Flarum\Api\Controller\UploadFaviconController;
-use Flarum\Api\Controller\UploadLogoController;
 use Flarum\Foundation\AbstractServiceProvider;
-use Illuminate\Contracts\Container\Container;
-use Illuminate\Contracts\Filesystem\Factory;
 use Illuminate\Database\ConnectionInterface;
-use League\Flysystem\FilesystemInterface;
 
 class SettingsServiceProvider extends AbstractServiceProvider
 {
@@ -35,18 +28,5 @@ class SettingsServiceProvider extends AbstractServiceProvider
         });
 
         $this->container->alias(SettingsRepositoryInterface::class, 'flarum.settings');
-
-        $assets = function (Container $container) {
-            return $container->make(Factory::class)->disk('flarum-assets')->getDriver();
-        };
-
-        $this->container->when([
-            DeleteFaviconController::class,
-            DeleteLogoController::class,
-            UploadFaviconController::class,
-            UploadLogoController::class,
-        ])
-            ->needs(FilesystemInterface::class)
-            ->give($assets);
     }
 }

framework/core/src/User/AvatarUploader.php

@@ -9,17 +9,21 @@
 
 namespace Flarum\User;
 
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Filesystem;
 use Illuminate\Support\Str;
 use Intervention\Image\Image;
-use League\Flysystem\FilesystemInterface;
 
 class AvatarUploader
 {
+    /**
+     * @var Filesystem
+     */
     protected $uploadDir;
 
-    public function __construct(FilesystemInterface $uploadDir)
+    public function __construct(Factory $filesystemFactory)
     {
-        $this->uploadDir = $uploadDir;
+        $this->uploadDir = $filesystemFactory->disk('flarum-avatars');
     }
 
     /**
@@ -52,7 +56,7 @@ class AvatarUploader
         $avatarPath = $user->getRawOriginal('avatar_url');
 
         $user->afterSave(function () use ($avatarPath) {
-            if ($this->uploadDir->has($avatarPath)) {
+            if ($this->uploadDir->exists($avatarPath)) {
                 $this->uploadDir->delete($avatarPath);
             }
         });

framework/core/src/User/User.php

@@ -18,7 +18,6 @@ use Flarum\Foundation\EventGeneratorTrait;
 use Flarum\Group\Group;
 use Flarum\Group\Permission;
 use Flarum\Http\AccessToken;
-use Flarum\Http\UrlGenerator;
 use Flarum\Notification\Notification;
 use Flarum\Post\Post;
 use Flarum\User\DisplayName\DriverInterface;
@@ -32,6 +31,7 @@ use Flarum\User\Event\Registered;
 use Flarum\User\Event\Renamed;
 use Flarum\User\Exception\NotAuthenticatedException;
 use Flarum\User\Exception\PermissionDeniedException;
+use Illuminate\Contracts\Filesystem\Factory;
 use Illuminate\Contracts\Hashing\Hasher;
 use Illuminate\Support\Arr;
 
@@ -312,7 +312,7 @@ class User extends AbstractModel
     public function getAvatarUrlAttribute(string $value = null)
     {
         if ($value && strpos($value, '://') === false) {
-            return app(UrlGenerator::class)->to('forum')->path('assets/avatars/'.$value);
+            return resolve(Factory::class)->disk('flarum-avatars')->url($value);
         }
 
         return $value;

framework/core/src/User/UserServiceProvider.php

@@ -24,10 +24,7 @@ use Flarum\User\DisplayName\UsernameDriver;
 use Flarum\User\Event\EmailChangeRequested;
 use Flarum\User\Event\Registered;
 use Flarum\User\Event\Saving;
-use Illuminate\Contracts\Container\Container;
-use Illuminate\Contracts\Filesystem\Factory;
 use Illuminate\Support\Arr;
-use League\Flysystem\FilesystemInterface;
 
 class UserServiceProvider extends AbstractServiceProvider
 {
@@ -36,7 +33,6 @@ class UserServiceProvider extends AbstractServiceProvider
      */
     public function register()
     {
-        $this->registerAvatarsFilesystem();
         $this->registerDisplayNameDrivers();
         $this->registerPasswordCheckers();
 
@@ -78,17 +74,6 @@ class UserServiceProvider extends AbstractServiceProvider
         $this->container->alias('flarum.user.display_name.driver', DriverInterface::class);
     }
 
-    protected function registerAvatarsFilesystem()
-    {
-        $avatarsFilesystem = function (Container $container) {
-            return $container->make(Factory::class)->disk('flarum-avatars')->getDriver();
-        };
-
-        $this->container->when(AvatarUploader::class)
-            ->needs(FilesystemInterface::class)
-            ->give($avatarsFilesystem);
-    }
-
     protected function registerPasswordCheckers()
     {
         $this->container->singleton('flarum.user.password_checkers', function () {

framework/core/tests/unit/User/AvatarUploaderTest.php

@@ -13,15 +13,17 @@ use Flarum\Testing\unit\TestCase;
 use Flarum\User\AvatarUploader;
 use Flarum\User\User;
 use Illuminate\Contracts\Events\Dispatcher;
+use Illuminate\Contracts\Filesystem\Factory;
+use Illuminate\Contracts\Filesystem\Filesystem;
 use Illuminate\Database\Eloquent\Model;
 use Intervention\Image\ImageManagerStatic;
-use League\Flysystem\FilesystemInterface;
 use Mockery as m;
 
 class AvatarUploaderTest extends TestCase
 {
     private $dispatcher;
     private $filesystem;
+    private $filesystemFactory;
     private $uploader;
 
     /**
@@ -33,13 +35,15 @@ class AvatarUploaderTest extends TestCase
         $this->dispatcher->shouldIgnoreMissing();
         Model::setEventDispatcher($this->dispatcher);
 
-        $this->filesystem = m::mock(FilesystemInterface::class);
+        $this->filesystem = m::mock(Filesystem::class);
-        $this->uploader = new AvatarUploader($this->filesystem);
+        $this->filesystemFactory = m::mock(Factory::class);
+        $this->filesystemFactory->shouldReceive('disk')->with('flarum-avatars')->andReturn($this->filesystem);
+        $this->uploader = new AvatarUploader($this->filesystemFactory);
     }
 
     public function test_removing_avatar_removes_file()
     {
-        $this->filesystem->shouldReceive('has')->with('ABCDEFGHabcdefgh.png')->andReturn(true);
+        $this->filesystem->shouldReceive('exists')->with('ABCDEFGHabcdefgh.png')->andReturn(true);
         $this->filesystem->shouldReceive('delete')->with('ABCDEFGHabcdefgh.png')->once();
 
         $user = new User();
@@ -61,7 +65,7 @@ class AvatarUploaderTest extends TestCase
 
     public function test_removing_url_avatar_removes_no_file()
     {
-        $this->filesystem->shouldReceive('has')->with('https://example.com/avatar.png')->andReturn(false)->once();
+        $this->filesystem->shouldReceive('exists')->with('https://example.com/avatar.png')->andReturn(false)->once();
         $this->filesystem->shouldNotReceive('delete');
 
         $user = new User();
@@ -82,7 +86,7 @@ class AvatarUploaderTest extends TestCase
     public function test_changing_avatar_removes_file()
     {
         $this->filesystem->shouldReceive('put')->once();
-        $this->filesystem->shouldReceive('has')->with('ABCDEFGHabcdefgh.png')->andReturn(true);
+        $this->filesystem->shouldReceive('exists')->with('ABCDEFGHabcdefgh.png')->andReturn(true);
         $this->filesystem->shouldReceive('delete')->with('ABCDEFGHabcdefgh.png')->once();
 
         $user = new User();