From a9f092cf4b41caf134d2e1e82aec252390e668a0 Mon Sep 17 00:00:00 2001 From: Franz Liedke Date: Fri, 3 Apr 2020 16:47:55 +0200 Subject: [PATCH] Simplify uploads, inject filesystem instances This avoids injecting the Application god class and assembling default file locations in multiple places. In addition, we no longer use the `MountManager` for these uploads. It only added complexity (by moving tmp files around) and will not be available in the next major release of Flysystem. Note: Passing PSR upload streams to Intervention Image requires an explicit upgrade of the library. (Very likely, users have already updated to the newer versions, as the old constraint allowed it, but we should be explicit for correctness' sake.) --- framework/core/composer.json | 2 +- .../Controller/DeleteFaviconController.php | 19 +++++----- .../Api/Controller/DeleteLogoController.php | 19 +++++----- .../Controller/UploadFaviconController.php | 36 +++++++------------ .../Api/Controller/UploadLogoController.php | 31 ++++++---------- .../src/Settings/SettingsServiceProvider.php | 20 +++++++++++ .../src/User/Command/UploadAvatarHandler.php | 33 +++++------------ 7 files changed, 68 insertions(+), 92 deletions(-) diff --git a/framework/core/composer.json b/framework/core/composer.json index 40bde9c5b..2ecc9ec07 100644 --- a/framework/core/composer.json +++ b/framework/core/composer.json @@ -56,7 +56,7 @@ "illuminate/support": "5.7.*", "illuminate/validation": "5.7.*", "illuminate/view": "5.7.*", - "intervention/image": "^2.3.0", + "intervention/image": "^2.5.0", "laminas/laminas-diactoros": "^1.8.4", "laminas/laminas-httphandlerrunner": "^1.0", "laminas/laminas-stratigility": "^3.0", diff --git a/framework/core/src/Api/Controller/DeleteFaviconController.php b/framework/core/src/Api/Controller/DeleteFaviconController.php index 0ce92feac..172ac3906 100644 --- a/framework/core/src/Api/Controller/DeleteFaviconController.php +++ b/framework/core/src/Api/Controller/DeleteFaviconController.php @@ -9,12 +9,10 @@ namespace Flarum\Api\Controller; -use Flarum\Foundation\Application; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\AssertPermissionTrait; use Laminas\Diactoros\Response\EmptyResponse; -use League\Flysystem\Adapter\Local; -use League\Flysystem\Filesystem; +use League\Flysystem\FilesystemInterface; use Psr\Http\Message\ServerRequestInterface; class DeleteFaviconController extends AbstractDeleteController @@ -27,17 +25,18 @@ class DeleteFaviconController extends AbstractDeleteController protected $settings; /** - * @var Application + * @var FilesystemInterface */ - protected $app; + protected $uploadDir; /** * @param SettingsRepositoryInterface $settings + * @param FilesystemInterface $uploadDir */ - public function __construct(SettingsRepositoryInterface $settings, Application $app) + public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir) { $this->settings = $settings; - $this->app = $app; + $this->uploadDir = $uploadDir; } /** @@ -51,10 +50,8 @@ class DeleteFaviconController extends AbstractDeleteController $this->settings->set('favicon_path', null); - $uploadDir = new Filesystem(new Local($this->app->publicPath().'/assets')); - - if ($uploadDir->has($path)) { - $uploadDir->delete($path); + if ($this->uploadDir->has($path)) { + $this->uploadDir->delete($path); } return new EmptyResponse(204); diff --git a/framework/core/src/Api/Controller/DeleteLogoController.php b/framework/core/src/Api/Controller/DeleteLogoController.php index fb097ff48..34e630445 100644 --- a/framework/core/src/Api/Controller/DeleteLogoController.php +++ b/framework/core/src/Api/Controller/DeleteLogoController.php @@ -9,12 +9,10 @@ namespace Flarum\Api\Controller; -use Flarum\Foundation\Application; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\AssertPermissionTrait; use Laminas\Diactoros\Response\EmptyResponse; -use League\Flysystem\Adapter\Local; -use League\Flysystem\Filesystem; +use League\Flysystem\FilesystemInterface; use Psr\Http\Message\ServerRequestInterface; class DeleteLogoController extends AbstractDeleteController @@ -27,17 +25,18 @@ class DeleteLogoController extends AbstractDeleteController protected $settings; /** - * @var Application + * @var FilesystemInterface */ - protected $app; + protected $uploadDir; /** * @param SettingsRepositoryInterface $settings + * @param FilesystemInterface $uploadDir */ - public function __construct(SettingsRepositoryInterface $settings, Application $app) + public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir) { $this->settings = $settings; - $this->app = $app; + $this->uploadDir = $uploadDir; } /** @@ -51,10 +50,8 @@ class DeleteLogoController extends AbstractDeleteController $this->settings->set('logo_path', null); - $uploadDir = new Filesystem(new Local($this->app->publicPath().'/assets')); - - if ($uploadDir->has($path)) { - $uploadDir->delete($path); + if ($this->uploadDir->has($path)) { + $this->uploadDir->delete($path); } return new EmptyResponse(204); diff --git a/framework/core/src/Api/Controller/UploadFaviconController.php b/framework/core/src/Api/Controller/UploadFaviconController.php index 40e2ac056..f8d789f50 100644 --- a/framework/core/src/Api/Controller/UploadFaviconController.php +++ b/framework/core/src/Api/Controller/UploadFaviconController.php @@ -9,15 +9,12 @@ namespace Flarum\Api\Controller; -use Flarum\Foundation\Application; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\AssertPermissionTrait; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Intervention\Image\ImageManager; -use League\Flysystem\Adapter\Local; -use League\Flysystem\Filesystem; -use League\Flysystem\MountManager; +use League\Flysystem\FilesystemInterface; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -31,17 +28,18 @@ class UploadFaviconController extends ShowForumController protected $settings; /** - * @var Application + * @var FilesystemInterface */ - protected $app; + protected $uploadDir; /** * @param SettingsRepositoryInterface $settings + * @param FilesystemInterface $uploadDir */ - public function __construct(SettingsRepositoryInterface $settings, Application $app) + public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir) { $this->settings = $settings; - $this->app = $app; + $this->uploadDir = $uploadDir; } /** @@ -52,36 +50,28 @@ class UploadFaviconController extends ShowForumController $this->assertAdmin($request->getAttribute('actor')); $file = Arr::get($request->getUploadedFiles(), 'favicon'); - - $tmpFile = tempnam($this->app->storagePath().'/tmp', 'favicon'); - $file->moveTo($tmpFile); - $extension = pathinfo($file->getClientFilename(), PATHINFO_EXTENSION); - if ($extension !== 'ico') { + if ($extension === 'ico') { + $image = $file->getStream(); + } else { $manager = new ImageManager; - $encodedImage = $manager->make($tmpFile)->resize(64, 64, function ($constraint) { + $image = $manager->make($file->getStream())->resize(64, 64, function ($constraint) { $constraint->aspectRatio(); $constraint->upsize(); })->encode('png'); - file_put_contents($tmpFile, $encodedImage); $extension = 'png'; } - $mount = new MountManager([ - 'source' => new Filesystem(new Local(pathinfo($tmpFile, PATHINFO_DIRNAME))), - 'target' => new Filesystem(new Local($this->app->publicPath().'/assets')), - ]); - - if (($path = $this->settings->get('favicon_path')) && $mount->has($file = "target://$path")) { - $mount->delete($file); + if (($path = $this->settings->get('favicon_path')) && $this->uploadDir->has($path)) { + $this->uploadDir->delete($path); } $uploadName = 'favicon-'.Str::lower(Str::random(8)).'.'.$extension; - $mount->move('source://'.pathinfo($tmpFile, PATHINFO_BASENAME), "target://$uploadName"); + $this->uploadDir->write($uploadName, $image); $this->settings->set('favicon_path', $uploadName); diff --git a/framework/core/src/Api/Controller/UploadLogoController.php b/framework/core/src/Api/Controller/UploadLogoController.php index 554e50734..ce117b496 100644 --- a/framework/core/src/Api/Controller/UploadLogoController.php +++ b/framework/core/src/Api/Controller/UploadLogoController.php @@ -9,15 +9,12 @@ namespace Flarum\Api\Controller; -use Flarum\Foundation\Application; use Flarum\Settings\SettingsRepositoryInterface; use Flarum\User\AssertPermissionTrait; use Illuminate\Support\Arr; use Illuminate\Support\Str; use Intervention\Image\ImageManager; -use League\Flysystem\Adapter\Local; -use League\Flysystem\Filesystem; -use League\Flysystem\MountManager; +use League\Flysystem\FilesystemInterface; use Psr\Http\Message\ServerRequestInterface; use Tobscure\JsonApi\Document; @@ -31,17 +28,18 @@ class UploadLogoController extends ShowForumController protected $settings; /** - * @var Application + * @var FilesystemInterface */ - protected $app; + protected $uploadDir; /** * @param SettingsRepositoryInterface $settings + * @param FilesystemInterface $uploadDir */ - public function __construct(SettingsRepositoryInterface $settings, Application $app) + public function __construct(SettingsRepositoryInterface $settings, FilesystemInterface $uploadDir) { $this->settings = $settings; - $this->app = $app; + $this->uploadDir = $uploadDir; } /** @@ -53,28 +51,19 @@ class UploadLogoController extends ShowForumController $file = Arr::get($request->getUploadedFiles(), 'logo'); - $tmpFile = tempnam($this->app->storagePath().'/tmp', 'logo'); - $file->moveTo($tmpFile); - $manager = new ImageManager; - $encodedImage = $manager->make($tmpFile)->heighten(60, function ($constraint) { + $encodedImage = $manager->make($file->getStream())->heighten(60, function ($constraint) { $constraint->upsize(); })->encode('png'); - file_put_contents($tmpFile, $encodedImage); - $mount = new MountManager([ - 'source' => new Filesystem(new Local(pathinfo($tmpFile, PATHINFO_DIRNAME))), - 'target' => new Filesystem(new Local($this->app->publicPath().'/assets')), - ]); - - if (($path = $this->settings->get('logo_path')) && $mount->has($file = "target://$path")) { - $mount->delete($file); + if (($path = $this->settings->get('logo_path')) && $this->uploadDir->has($path)) { + $this->uploadDir->delete($path); } $uploadName = 'logo-'.Str::lower(Str::random(8)).'.png'; - $mount->move('source://'.pathinfo($tmpFile, PATHINFO_BASENAME), "target://$uploadName"); + $this->uploadDir->write($uploadName, $encodedImage); $this->settings->set('logo_path', $uploadName); diff --git a/framework/core/src/Settings/SettingsServiceProvider.php b/framework/core/src/Settings/SettingsServiceProvider.php index 852d23e59..2aa7ade40 100644 --- a/framework/core/src/Settings/SettingsServiceProvider.php +++ b/framework/core/src/Settings/SettingsServiceProvider.php @@ -9,8 +9,15 @@ 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 { @@ -28,5 +35,18 @@ class SettingsServiceProvider extends AbstractServiceProvider }); $this->app->alias(SettingsRepositoryInterface::class, 'flarum.settings'); + + $assets = function (Container $app) { + return $app->make(Factory::class)->disk('flarum-assets')->getDriver(); + }; + + $this->app->when([ + DeleteFaviconController::class, + DeleteLogoController::class, + UploadFaviconController::class, + UploadLogoController::class, + ]) + ->needs(FilesystemInterface::class) + ->give($assets); } } diff --git a/framework/core/src/User/Command/UploadAvatarHandler.php b/framework/core/src/User/Command/UploadAvatarHandler.php index 1803c4cc5..7700becf1 100644 --- a/framework/core/src/User/Command/UploadAvatarHandler.php +++ b/framework/core/src/User/Command/UploadAvatarHandler.php @@ -9,7 +9,6 @@ namespace Flarum\User\Command; -use Flarum\Foundation\Application; use Flarum\Foundation\DispatchEventsTrait; use Flarum\User\AssertPermissionTrait; use Flarum\User\AvatarUploader; @@ -29,11 +28,6 @@ class UploadAvatarHandler */ protected $users; - /** - * @var Application - */ - protected $app; - /** * @var AvatarUploader */ @@ -47,15 +41,13 @@ class UploadAvatarHandler /** * @param Dispatcher $events * @param UserRepository $users - * @param Application $app * @param AvatarUploader $uploader * @param AvatarValidator $validator */ - public function __construct(Dispatcher $events, UserRepository $users, Application $app, AvatarUploader $uploader, AvatarValidator $validator) + public function __construct(Dispatcher $events, UserRepository $users, AvatarUploader $uploader, AvatarValidator $validator) { $this->events = $events; $this->users = $users; - $this->app = $app; $this->uploader = $uploader; $this->validator = $validator; } @@ -76,26 +68,17 @@ class UploadAvatarHandler $this->assertCan($actor, 'edit', $user); } - $file = $command->file; + $this->validator->assertValid(['avatar' => $command->file]); - $tmpFile = tempnam($this->app->storagePath().'/tmp', 'avatar'); - $file->moveTo($tmpFile); + $image = (new ImageManager)->make($command->file->getStream()); - try { - $this->validator->assertValid(['avatar' => $file]); + $this->events->dispatch( + new AvatarSaving($user, $actor, $image) + ); - $image = (new ImageManager)->make($tmpFile); + $this->uploader->upload($user, $image); - $this->events->dispatch( - new AvatarSaving($user, $actor, $image) - ); - - $this->uploader->upload($user, $image); - - $user->save(); - } finally { - @unlink($tmpFile); - } + $user->save(); return $user; }