From 22fc843ce3cbf23cd369d860bdefa39b88a0a269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 8 Feb 2019 22:44:46 +0100 Subject: [PATCH] Fix a lot of bugs, fix #23 --- README.md | 35 +++++++++++++++++++--- src/Controllers/TenantAssetsController.php | 2 +- src/Traits/BootstrapsTenancy.php | 19 +++++++----- src/config/tenancy.php | 6 ++++ src/routes.php | 2 +- tests/BootstrapsTenancyTest.php | 9 +++++- tests/HttpKernel.php | 12 ++++---- tests/ReidentificationTest.php | 9 +++++- tests/TenantAssetTest.php | 15 ++++++++++ tests/TestCase.php | 2 +- 10 files changed, 88 insertions(+), 23 deletions(-) create mode 100644 tests/TenantAssetTest.php diff --git a/README.md b/README.md index f3ecda3a..20ee41d1 100644 --- a/README.md +++ b/README.md @@ -104,9 +104,9 @@ Filesystem paths will be suffixed with: config('tenancy.filesystem.suffix_base') . $uuid ``` -These changes will only apply for disks listen in `disks`. +These changes will only apply for disks listed in `disks`. -You can see an example in the [Filesystem](#filesystemstorage) section of the documentation. +You can see an example in the [Filesystem](#filesystemstorage) section of the documentation The `filesystem.root_override` section is explained there as well. # Usage @@ -321,12 +321,24 @@ Assuming the following tenancy config: // Disks which should be suffixed with the prefix_base + tenant UUID. 'disks' => [ 'local', + // 'public', // 's3', ], + 'root_override' => [ + // Disks whose roots should be overriden after storage_path() is suffixed. + 'local' => '%storage_path%/app/', + 'public' => '%storage_path%/app/public/', + ], ], ``` -The `local` filesystem driver will be suffixed with a directory named `tenant` + the tenant UUID. +1. The `storage_path()` will be suffixed with a directory named `tenant` + the tenant UUID. +2. The `local` disk's root will be `storage_path('app')` (which is equivalen to `storage_path() . '/app/'`). + By default, all disks' roots are suffixed with `tenant` + the tenant UUID. This works for s3 and similar disks. But for local disks, this results in unwanted behavior. The default root for this disk is `storage_path('app')`: + https://github.com/laravel/laravel/blob/2a1f3761e89df690190e9f50a6b4ac5ebb8b35a3/config/filesystems.php#L46-L49 + However, this configration file was loaded *before* tenancy was initialized. This means that if we simply suffix this disk's root, we get `/path_to_your_application/storage/app/tenant1e22e620-1cb8-11e9-93b6-8d1b78ac0bcd/`. That's not what we want. We want `/path_to_your_application/storage/tenant1e22e620-1cb8-11e9-93b6-8d1b78ac0bcd/app/`. + + This is what the override section of the config is for. `%storage_path%` gets replaced by `storage_path()` *after* tenancy is initialized. **The roots of disks listed in the `root_override` section of the config will be replaced according it. All other disks will be simply suffixed with `tenant` + the tenant UUID.** ```php >>> Storage::disk('local')->getAdapter()->getPathPrefix() @@ -337,7 +349,7 @@ The `local` filesystem driver will be suffixed with a directory named `tenant` + "domain" => "localhost", ] >>> Storage::disk('local')->getAdapter()->getPathPrefix() -=> "/var/www/laravel/multitenancy/storage/app/tenantdbe0b330-1a6e-11e9-b4c3-354da4b4f339/" +=> "/var/www/laravel/multitenancy/storage/tenantdbe0b330-1a6e-11e9-b4c3-354da4b4f339/app/" ``` `storage_path()` will also be suffixed in the same way. Note that this means that each tenant will have their own storage directory. @@ -357,6 +369,21 @@ You need to make this change to your code: + tenant_asset("images/products/$product_id.png"); ``` +Note that all (public) tenant assets have to be in the `app/public/` subdirectory of the tenant's storage directory, as shown in the image above. + +This is what the backend of `tenant_asset()` returns: +```php +// TenantAssetsController +return response()->file(storage_path('app/public/' . $path)); +``` + +With default filesystem configuration, these two commands are equivalent: + +```php +Storage::disk('public')->put($filename, $data); +Storage::disk('local')->put("public/$filename", $data); +``` + ## Artisan commands ``` diff --git a/src/Controllers/TenantAssetsController.php b/src/Controllers/TenantAssetsController.php index a5db2fea..2d8db007 100644 --- a/src/Controllers/TenantAssetsController.php +++ b/src/Controllers/TenantAssetsController.php @@ -4,7 +4,7 @@ namespace Stancl\Tenancy\Controllers; use Illuminate\Routing\Controller; -class TenantAssetController extends Controller +class TenantAssetsController extends Controller { public function __construct() { diff --git a/src/Traits/BootstrapsTenancy.php b/src/Traits/BootstrapsTenancy.php index 3ce41bd3..afe29aeb 100644 --- a/src/Traits/BootstrapsTenancy.php +++ b/src/Traits/BootstrapsTenancy.php @@ -4,6 +4,7 @@ namespace Stancl\Tenancy\Traits; use Stancl\Tenancy\CacheManager; use Illuminate\Support\Facades\Redis; +use Illuminate\Support\Facades\Storage; trait BootstrapsTenancy { @@ -47,20 +48,22 @@ trait BootstrapsTenancy $suffix = $this->app['config']['tenancy.filesystem.suffix_base'] . tenant('uuid'); + // storage_path() + $this->app->useStoragePath($old['storage_path'] . "/{$suffix}"); + // Storage facade foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) { - $root = $this->app['config']["filesystems.disks.{$disk}.root"]; - - \Storage::disk($disk)->getAdapter()->setPathPrefix( - $root . "/{$suffix}" - ); + if ($root = str_replace('%storage_path%', storage_path(), $this->app['config']["tenancy.filesystem.root_override.{$disk}"])) { + Storage::disk($disk)->getAdapter()->setPathPrefix($root); + } else { + $root = $this->app['config']["filesystems.disks.{$disk}.root"]; + + Storage::disk($disk)->getAdapter()->setPathPrefix($root . "/{$suffix}"); + } $old['storage_disks'][$disk] = $root; } - // storage_path() - $this->app->useStoragePath($old['storage_path'] . "/{$suffix}"); - $this->oldStoragePaths = $old; } } diff --git a/src/config/tenancy.php b/src/config/tenancy.php index 90d139e2..f1d9207f 100644 --- a/src/config/tenancy.php +++ b/src/config/tenancy.php @@ -26,8 +26,14 @@ return [ // Disks which should be suffixed with the suffix_base + tenant UUID. 'disks' => [ 'local', + 'public', // 's3', ], + 'root_override' => [ + // Disks whose roots should be overriden after storage_path() is suffixed. + 'local' => '%storage_path%/app/', + 'public' => '%storage_path%/app/public/', + ], ], 'database_managers' => [ 'sqlite' => 'Stancl\Tenancy\TenantDatabaseManagers\SQLiteDatabaseManager', diff --git a/src/routes.php b/src/routes.php index a51a8871..861368d1 100644 --- a/src/routes.php +++ b/src/routes.php @@ -1,5 +1,5 @@ where('path', '(.*)') ->name('stancl.tenancy.asset'); diff --git a/tests/BootstrapsTenancyTest.php b/tests/BootstrapsTenancyTest.php index e407858c..902ca712 100644 --- a/tests/BootstrapsTenancyTest.php +++ b/tests/BootstrapsTenancyTest.php @@ -47,7 +47,14 @@ class BootstrapsTenancyTest extends TestCase foreach (config('tenancy.filesystem.disks') as $disk) { $suffix = config('tenancy.filesystem.suffix_base') . tenant('uuid'); $current_path_prefix = \Storage::disk($disk)->getAdapter()->getPathPrefix(); - $this->assertSame($old_storage_facade_roots[$disk] . "/$suffix/", $current_path_prefix); + + if ($override = config("tenancy.filesystem.root_override.{$disk}")) { + $correct_path_prefix = str_replace("%storage_path%", storage_path(), $override); + } else { + $correct_path_prefix = $old_storage_facade_roots[$disk] . "/$suffix/"; + } + + $this->assertSame($correct_path_prefix, $current_path_prefix); } } diff --git a/tests/HttpKernel.php b/tests/HttpKernel.php index 120e6a69..18675c48 100644 --- a/tests/HttpKernel.php +++ b/tests/HttpKernel.php @@ -16,9 +16,9 @@ class HttpKernel extends Kernel protected $middleware = [ \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class, \Illuminate\Foundation\Http\Middleware\ValidatePostSize::class, - \App\Http\Middleware\TrimStrings::class, + \Orchestra\Testbench\Http\Middleware\TrimStrings::class, \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull::class, - \App\Http\Middleware\TrustProxies::class, + // \Orchestra\Testbench\Http\Middleware\TrustProxies::class, ]; /** @@ -28,12 +28,12 @@ class HttpKernel extends Kernel */ protected $middlewareGroups = [ 'web' => [ - \App\Http\Middleware\EncryptCookies::class, + \Orchestra\Testbench\Http\Middleware\EncryptCookies::class, \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class, \Illuminate\Session\Middleware\StartSession::class, // \Illuminate\Session\Middleware\AuthenticateSession::class, \Illuminate\View\Middleware\ShareErrorsFromSession::class, - \App\Http\Middleware\VerifyCsrfToken::class, + \Orchestra\Testbench\Http\Middleware\VerifyCsrfToken::class, \Illuminate\Routing\Middleware\SubstituteBindings::class, ], @@ -55,12 +55,12 @@ class HttpKernel extends Kernel * @var array */ protected $routeMiddleware = [ - 'auth' => \App\Http\Middleware\Authenticate::class, + 'auth' => \Orchestra\Testbench\Http\Middleware\Authenticate::class, 'auth.basic' => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class, 'bindings' => \Illuminate\Routing\Middleware\SubstituteBindings::class, 'cache.headers' => \Illuminate\Http\Middleware\SetCacheHeaders::class, 'can' => \Illuminate\Auth\Middleware\Authorize::class, - 'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class, + 'guest' => \Orchestra\Testbench\Http\Middleware\RedirectIfAuthenticated::class, 'signed' => \Illuminate\Routing\Middleware\ValidateSignature::class, 'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class, 'verified' => \Illuminate\Auth\Middleware\EnsureEmailIsVerified::class, diff --git a/tests/ReidentificationTest.php b/tests/ReidentificationTest.php index 616d04f3..83df49ae 100644 --- a/tests/ReidentificationTest.php +++ b/tests/ReidentificationTest.php @@ -25,7 +25,14 @@ class ReidentificationTest extends TestCase foreach (config('tenancy.filesystem.disks') as $disk) { $suffix = config('tenancy.filesystem.suffix_base') . tenant('uuid'); $current_path_prefix = \Storage::disk($disk)->getAdapter()->getPathPrefix(); - $this->assertSame($originals[$disk] . "/$suffix/", $current_path_prefix); + + if ($override = config("tenancy.filesystem.root_override.{$disk}")) { + $correct_path_prefix = str_replace("%storage_path%", storage_path(), $override); + } else { + $correct_path_prefix = $originals[$disk] . "/$suffix/"; + } + + $this->assertSame($correct_path_prefix, $current_path_prefix); } } diff --git a/tests/TenantAssetTest.php b/tests/TenantAssetTest.php new file mode 100644 index 00000000..800071a7 --- /dev/null +++ b/tests/TenantAssetTest.php @@ -0,0 +1,15 @@ +markTestIncomplete(); + $filename = 'testfile' . $this->randomString(10); + \Storage::disk('public')->put($filename, 'bar'); + $this->get(tenant_asset($filename))->assertSee('bar'); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index f28335ff..1e214f14 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -4,7 +4,7 @@ namespace Stancl\Tenancy\Tests; use Illuminate\Support\Facades\Redis; -class TestCase extends \Orchestra\Testbench\TestCase +abstract class TestCase extends \Orchestra\Testbench\TestCase { public $autoCreateTenant = true; public $autoInitTenancy = true;