Clean up usages / deprecate path helpers (#2155)

* Write source map without creating temp file

Less I/O, and one less place where we access the global path helpers.

* Drop useless app_path() helper

This was probably taken straight from Laravel. There is no equivalent
concept in Flarum, so this should be safe to remove.

* Deprecate global path helpers

Developers using these helpers can inject the `Paths` class instead.

* Stop storing paths as strings in container

* Avoid using path helpers from Application class

* Deprecate path helpers from Application class

* Avoid using public_path() in prerequisite check

a) The comparison was already outdated, as a different path was passed.
b) We're trying to get rid of these global helpers.

src/Formatter/FormatterServiceProvider.php

@@ -10,6 +10,7 @@
 namespace Flarum\Formatter;
 
 use Flarum\Foundation\AbstractServiceProvider;
+use Flarum\Foundation\Paths;
 use Illuminate\Cache\Repository;
 use Illuminate\Contracts\Container\Container;
 
@@ -24,7 +25,7 @@ class FormatterServiceProvider extends AbstractServiceProvider
             return new Formatter(
                 new Repository($container->make('cache.filestore')),
                 $container->make('events'),
-                $this->app['flarum']->storagePath().'/formatter'
+                $this->app[Paths::class]->storage.'/formatter'
             );
         });
 

src/Foundation/Application.php

@@ -86,8 +86,6 @@ class Application
         $this->registerBaseBindings();
         $this->registerBaseServiceProviders();
         $this->registerCoreContainerAliases();
-
-        $this->bindPathsInContainer();
     }
 
     /**
@@ -161,22 +159,11 @@ class Application
         $this->register(new EventServiceProvider($this->container));
     }
 
-    /**
-     * Bind all of the application paths in the container.
-     *
-     * @return void
-     */
-    protected function bindPathsInContainer()
-    {
-        foreach (['base', 'public', 'storage', 'vendor'] as $path) {
-            $this->container->instance('path.'.$path, $this->paths->$path);
-        }
-    }
-
     /**
      * Get the base path of the Laravel installation.
      *
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     public function basePath()
     {
@@ -187,6 +174,7 @@ class Application
      * Get the path to the public / web directory.
      *
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     public function publicPath()
     {
@@ -197,6 +185,7 @@ class Application
      * Get the path to the storage directory.
      *
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     public function storagePath()
     {
@@ -207,6 +196,7 @@ class Application
      * Get the path to the vendor directory where dependencies are installed.
      *
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     public function vendorPath()
     {

src/Frontend/Compiler/JsCompiler.php

@@ -50,16 +50,12 @@ class JsCompiler extends RevisionCompiler
         }
 
         // Add a comment to the end of our file to point to the sourcemap
-        // we just constructed. We will then write the JS file, save the
+        // we just constructed. We will then store the JS file and the map
-        // map to a temporary location, and then move it to the asset dir.
+        // in our asset directory.
         $output[] = '//# sourceMappingURL='.$this->assetsDir->url($mapFile);
 
         $this->assetsDir->put($file, implode("\n", $output));
-
+        $this->assetsDir->put($mapFile, json_encode($map, JSON_UNESCAPED_SLASHES));
-        $mapTemp = @tempnam(storage_path('tmp'), $mapFile);
-        $map->save($mapTemp);
-        $this->assetsDir->put($mapFile, file_get_contents($mapTemp));
-        @unlink($mapTemp);
 
         return true;
     }

src/Frontend/FrontendServiceProvider.php

@@ -10,6 +10,7 @@
 namespace Flarum\Frontend;
 
 use Flarum\Foundation\AbstractServiceProvider;
+use Flarum\Foundation\Paths;
 use Flarum\Frontend\Compiler\Source\SourceCollector;
 use Flarum\Http\UrlGenerator;
 use Flarum\Settings\SettingsRepositoryInterface;
@@ -21,14 +22,16 @@ class FrontendServiceProvider extends AbstractServiceProvider
     {
         $this->app->singleton('flarum.assets.factory', function () {
             return function (string $name) {
+                $paths = $this->app[Paths::class];
+
                 $assets = new Assets(
                     $name,
                     $this->app->make('filesystem')->disk('flarum-assets'),
-                    $this->app['flarum']->storagePath()
+                    $paths->storage
                 );
 
                 $assets->setLessImportDirs([
-                    $this->app['flarum']->vendorPath().'/components/font-awesome/less' => ''
+                    $paths->vendor.'/components/font-awesome/less' => ''
                 ]);
 
                 $assets->css([$this, 'addBaseCss']);

src/Install/InstallServiceProvider.php

@@ -23,15 +23,6 @@ class InstallServiceProvider extends AbstractServiceProvider
         $this->app->singleton('flarum.install.routes', function () {
             return new RouteCollection;
         });
-
-        $this->app->singleton(Installation::class, function () {
-            return new Installation(
-                $this->app->basePath(),
-                $this->app->publicPath(),
-                $this->app->storagePath(),
-                $this->app->vendorPath()
-            );
-        });
     }
 
     /**

src/Install/Installation.php

@@ -9,12 +9,14 @@
 
 namespace Flarum\Install;
 
+use Flarum\Foundation\Paths;
+
 class Installation
 {
-    private $basePath;
+    /**
-    private $publicPath;
+     * @var Paths
-    private $storagePath;
+     */
-    private $vendorPath;
+    private $paths;
 
     private $configPath;
     private $debug = false;
@@ -34,12 +36,9 @@ class Installation
     /** @var \Illuminate\Database\ConnectionInterface */
     private $db;
 
-    public function __construct($basePath, $publicPath, $storagePath, $vendorPath)
+    public function __construct(Paths $paths)
     {
-        $this->basePath = $basePath;
+        $this->paths = $paths;
-        $this->publicPath = $publicPath;
-        $this->storagePath = $storagePath;
-        $this->vendorPath = $vendorPath;
     }
 
     public function configPath($path)
@@ -98,9 +97,9 @@ class Installation
                 'tokenizer',
             ]),
             new Prerequisite\WritablePaths([
-                $this->basePath,
+                $this->paths->base,
-                $this->getAssetPath(),
+                $this->getAssetPath().'/*',
-                $this->storagePath,
+                $this->paths->storage,
             ])
         );
     }
@@ -140,11 +139,11 @@ class Installation
         });
 
         $pipeline->pipe(function () {
-            return new Steps\PublishAssets($this->vendorPath, $this->getAssetPath());
+            return new Steps\PublishAssets($this->paths->vendor, $this->getAssetPath());
         });
 
         $pipeline->pipe(function () {
-            return new Steps\EnableBundledExtensions($this->db, $this->vendorPath, $this->getAssetPath());
+            return new Steps\EnableBundledExtensions($this->db, $this->paths->vendor, $this->getAssetPath());
         });
 
         return $pipeline;
@@ -152,12 +151,12 @@ class Installation
 
     private function getConfigPath()
     {
-        return $this->basePath.'/'.($this->configPath ?? 'config.php');
+        return $this->paths->base.'/'.($this->configPath ?? 'config.php');
     }
 
     private function getAssetPath()
     {
-        return "$this->publicPath/assets";
+        return $this->paths->public.'/assets';
     }
 
     private function getMigrationPath()

src/Install/Prerequisite/WritablePaths.php

@@ -10,14 +10,20 @@
 namespace Flarum\Install\Prerequisite;
 
 use Illuminate\Support\Collection;
+use Illuminate\Support\Str;
 
 class WritablePaths implements PrerequisiteInterface
 {
-    protected $paths;
+    /**
+     * @var Collection
+     */
+    private $paths;
+
+    private $wildcards = [];
 
     public function __construct(array $paths)
     {
-        $this->paths = $paths;
+        $this->paths = $this->normalize($paths);
     }
 
     public function problems(): Collection
@@ -28,7 +34,7 @@ class WritablePaths implements PrerequisiteInterface
 
     private function getMissingPaths(): Collection
     {
-        return (new Collection($this->paths))
+        return $this->paths
             ->reject(function ($path) {
                 return file_exists($path);
             })->map(function ($path) {
@@ -41,13 +47,13 @@ class WritablePaths implements PrerequisiteInterface
 
     private function getNonWritablePaths(): Collection
     {
-        return (new Collection($this->paths))
+        return $this->paths
             ->filter(function ($path) {
                 return file_exists($path) && ! is_writable($path);
-            })->map(function ($path) {
+            })->map(function ($path, $index) {
                 return [
                     'message' => 'The '.$this->getAbsolutePath($path).' directory is not writable.',
-                    'detail' => 'Please chmod this directory'.($path !== public_path() ? ' and its contents' : '').' to 0775.'
+                    'detail' => 'Please chmod this directory'.(in_array($index, $this->wildcards) ? ' and its contents' : '').' to 0775.'
                 ];
             });
     }
@@ -71,4 +77,17 @@ class WritablePaths implements PrerequisiteInterface
 
         return (substr($path, 0, 1) == '/' ? '/' : '').implode(DIRECTORY_SEPARATOR, $absolutes);
     }
+
+    private function normalize(array $paths): Collection
+    {
+        return (new Collection($paths))
+            ->map(function ($path, $index) {
+                if (Str::endsWith($path, '/*')) {
+                    $this->wildcards[] = $index;
+                    $path = substr($path, 0, -2);
+                }
+
+                return $path;
+            });
+    }
 }

src/Locale/LocaleServiceProvider.php

@@ -11,6 +11,7 @@ namespace Flarum\Locale;
 
 use Flarum\Foundation\AbstractServiceProvider;
 use Flarum\Foundation\Event\ClearingCache;
+use Flarum\Foundation\Paths;
 use Flarum\Settings\SettingsRepositoryInterface;
 use Illuminate\Contracts\Events\Dispatcher;
 use Illuminate\Contracts\Translation\Translator as TranslatorContract;
@@ -73,6 +74,6 @@ class LocaleServiceProvider extends AbstractServiceProvider
 
     private function getCacheDir(): string
     {
-        return $this->app['flarum']->storagePath().'/locale';
+        return $this->app[Paths::class]->storage.'/locale';
     }
 }

src/Queue/QueueServiceProvider.php

@@ -13,6 +13,7 @@ use Flarum\Console\Event\Configuring;
 use Flarum\Foundation\AbstractServiceProvider;
 use Flarum\Foundation\ErrorHandling\Registry;
 use Flarum\Foundation\ErrorHandling\Reporter;
+use Flarum\Foundation\Paths;
 use Illuminate\Contracts\Debug\ExceptionHandler as ExceptionHandling;
 use Illuminate\Contracts\Queue\Factory;
 use Illuminate\Contracts\Queue\Queue;
@@ -70,7 +71,7 @@ class QueueServiceProvider extends AbstractServiceProvider
         // Override the Laravel native Listener, so that we can ignore the environment
         // option and force the binary to flarum.
         $this->app->singleton(QueueListener::class, function ($app) {
-            return new Listener($app->basePath());
+            return new Listener($app[Paths::class]->base);
         });
 
         // Bind a simple cache manager that returns the cache store.

src/helpers.php

@@ -7,6 +7,7 @@
  * LICENSE file that was distributed with this source code.
  */
 
+use Flarum\Foundation\Paths;
 use Illuminate\Container\Container;
 
 if (! function_exists('app')) {
@@ -27,29 +28,17 @@ if (! function_exists('app')) {
     }
 }
 
-if (! function_exists('app_path')) {
-    /**
-     * Get the path to the application folder.
-     *
-     * @param  string  $path
-     * @return string
-     */
-    function app_path($path = '')
-    {
-        return app('path').($path ? DIRECTORY_SEPARATOR.$path : $path);
-    }
-}
-
 if (! function_exists('base_path')) {
     /**
      * Get the path to the base of the install.
      *
      * @param  string  $path
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     function base_path($path = '')
     {
-        return app()->basePath().($path ? DIRECTORY_SEPARATOR.$path : $path);
+        return app(Paths::class)->base.($path ? DIRECTORY_SEPARATOR.$path : $path);
     }
 }
 
@@ -59,10 +48,11 @@ if (! function_exists('public_path')) {
      *
      * @param  string  $path
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     function public_path($path = '')
     {
-        return app()->publicPath().($path ? DIRECTORY_SEPARATOR.$path : $path);
+        return app(Paths::class)->public.($path ? DIRECTORY_SEPARATOR.$path : $path);
     }
 }
 
@@ -72,10 +62,11 @@ if (! function_exists('storage_path')) {
      *
      * @param  string  $path
      * @return string
+     * @deprecated Will be removed in Beta.15.
      */
     function storage_path($path = '')
     {
-        return app('path.storage').($path ? DIRECTORY_SEPARATOR.$path : $path);
+        return app(Paths::class)->storage.($path ? DIRECTORY_SEPARATOR.$path : $path);
     }
 }
 

tests/integration/setup.php

@@ -7,6 +7,7 @@
  * LICENSE file that was distributed with this source code.
  */
 
+use Flarum\Foundation\Paths;
 use Flarum\Install\AdminUser;
 use Flarum\Install\BaseUrl;
 use Flarum\Install\DatabaseConfig;
@@ -38,10 +39,12 @@ echo "\nOff we go...\n";
  */
 
 $installation = new Installation(
-    __DIR__.'/tmp',
+    new Paths([
-    __DIR__.'/tmp/public',
+        'base' => __DIR__.'/tmp',
-    __DIR__.'/tmp/storage',
+        'public' => __DIR__.'/tmp/public',
-    __DIR__.'/../../vendor'
+        'storage' => __DIR__.'/tmp/storage',
+        'vendor' => __DIR__.'/../../vendor',
+    ])
 );
 
 $pipeline = $installation

tests/unit/Install/Prerequisite/WritablePathsTest.php

@@ -0,0 +1,71 @@
+<?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\unit\Install\Prerequisite;
+
+use Flarum\Install\Prerequisite\WritablePaths;
+use Flarum\Tests\unit\TestCase;
+
+class WritablePathsTest extends TestCase
+{
+    public function test_no_problems_when_all_directories_are_writable()
+    {
+        $writable = new WritablePaths([
+            __DIR__.'/../../../fixtures/writable_paths/writable',
+        ]);
+
+        $this->assertCount(0, $writable->problems());
+    }
+
+    public function test_paths_can_be_given_with_wildcard()
+    {
+        $writable = new WritablePaths([
+            __DIR__.'/../../../fixtures/writable_paths/writable/*',
+        ]);
+
+        $this->assertCount(0, $writable->problems());
+    }
+
+    public function test_problems_when_one_path_is_missing()
+    {
+        $writable = new WritablePaths([
+            __DIR__.'/../../../fixtures/writable_paths/missing',
+            __DIR__.'/../../../fixtures/writable_paths/writable',
+        ]);
+
+        $problems = $writable->problems();
+        $this->assertCount(1, $problems);
+        $this->assertRegExp(
+            "%^The .+/tests/fixtures/writable_paths/missing directory doesn't exist$%",
+            $problems[0]['message']
+        );
+        $this->assertEquals(
+            'This directory is necessary for the installation. Please create the folder.',
+            $problems[0]['detail']
+        );
+    }
+
+    public function test_problem_details_filter_out_wildcard()
+    {
+        $writable = new WritablePaths([
+            __DIR__.'/../../../fixtures/writable_paths/missing/*',
+        ]);
+
+        $problems = $writable->problems();
+        $this->assertCount(1, $problems);
+        $this->assertRegExp(
+            "%^The .+/tests/fixtures/writable_paths/missing directory doesn't exist$%",
+            $problems[0]['message']
+        );
+        $this->assertEquals(
+            'This directory is necessary for the installation. Please create the folder.',
+            $problems[0]['detail']
+        );
+    }
+}