From 79e3d53b06f33aa6e30ee4454177e1d798918704 Mon Sep 17 00:00:00 2001 From: Erik Gaal Date: Tue, 8 Mar 2022 01:50:25 +0100 Subject: [PATCH] [3.x] Compatibility with Laravel 9 (#802) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Test on Laravel 9 * Don't extend final Kernel class * Make FilesystemTenancyBootstrapper compatible with Flysystem v3 Co-authored-by: George * Update tenant maintenance mode te be in line with Laravel * Exclude PHP 7.4 <> L9 combination from testing * add root_override-related assertions * getPrefix -> getPathPrefix * handle / inconsistency in s3 prefix * Refactor Storage facade changes Co-authored-by: George Co-authored-by: Samuel Ć tancl --- .github/workflows/ci.yml | 5 +- composer.json | 8 +-- .../FilesystemTenancyBootstrapper.php | 31 ++++----- .../CheckTenantForMaintenanceMode.php | 9 ++- tests/BootstrapperTest.php | 69 +++++++++++-------- tests/Etc/ConsoleKernel.php | 2 +- tests/MaintenanceModeTest.php | 4 +- tests/TestCase.php | 1 + 8 files changed, 74 insertions(+), 55 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f7b64d2d..1303061a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,10 @@ jobs: strategy: matrix: php: ["7.4", "8.0"] - laravel: ["^6.0", "^8.0"] + laravel: ["^6.0", "^8.0", "^9.0"] + exclude: + - laravel: "^9.0" + php: "7.4" steps: - uses: actions/checkout@v2 diff --git a/composer.json b/composer.json index bf66e1f2..88bfea29 100644 --- a/composer.json +++ b/composer.json @@ -11,16 +11,16 @@ ], "require": { "ext-json": "*", - "illuminate/support": "^6.0|^7.0|^8.0", + "illuminate/support": "^6.0|^7.0|^8.0|^9.0", "facade/ignition-contracts": "^1.0", "ramsey/uuid": "^3.7|^4.0", "stancl/jobpipeline": "^1.0", "stancl/virtualcolumn": "^1.0" }, "require-dev": { - "laravel/framework": "^6.0|^7.0|^8.0", - "orchestra/testbench-browser-kit": "^4.0|^5.0|^6.0", - "league/flysystem-aws-s3-v3": "~1.0", + "laravel/framework": "^6.0|^7.0|^8.0|^9.0", + "orchestra/testbench": "^4.0|^5.0|^6.0|^7.0", + "league/flysystem-aws-s3-v3": "^1.0|^3.0", "doctrine/dbal": "^2.10", "spatie/valuestore": "^1.2.5" }, diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index d5ae2d50..418be93f 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -54,20 +54,20 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper } // Storage facade - foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) { - /** @var FilesystemAdapter $filesystemDisk */ - $filesystemDisk = Storage::disk($disk); - $this->originalPaths['disks'][$disk] = $filesystemDisk->getAdapter()->getPathPrefix(); + Storage::forgetDisk($this->app['config']['tenancy.filesystem.disks']); - if ($root = str_replace( + foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) { + $originalRoot = $this->app['config']["filesystems.disks.{$disk}.root"]; + $this->originalPaths['disks'][$disk] = $originalRoot; + + $finalPrefix = str_replace( '%storage_path%', storage_path(), - $this->app['config']["tenancy.filesystem.root_override.{$disk}"] ?? '' - )) { - $filesystemDisk->getAdapter()->setPathPrefix($finalPrefix = $root); - } else { - $root = $this->app['config']["filesystems.disks.{$disk}.root"]; - $filesystemDisk->getAdapter()->setPathPrefix($finalPrefix = $root . "/{$suffix}"); + $this->app['config']["tenancy.filesystem.root_override.{$disk}"] ?? '', + ); + + if (! $finalPrefix) { + $finalPrefix = $originalRoot . '/'. $suffix; } $this->app['config']["filesystems.disks.{$disk}.root"] = $finalPrefix; @@ -84,14 +84,9 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']); // Storage facade + Storage::forgetDisk($this->app['config']['tenancy.filesystem.disks']); foreach ($this->app['config']['tenancy.filesystem.disks'] as $disk) { - /** @var FilesystemAdapter $filesystemDisk */ - $filesystemDisk = Storage::disk($disk); - - $root = $this->originalPaths['disks'][$disk]; - - $filesystemDisk->getAdapter()->setPathPrefix($root); - $this->app['config']["filesystems.disks.{$disk}.root"] = $root; + $this->app['config']["filesystems.disks.{$disk}.root"] = $this->originalPaths['disks'][$disk]; } } } diff --git a/src/Middleware/CheckTenantForMaintenanceMode.php b/src/Middleware/CheckTenantForMaintenanceMode.php index 5554663f..8e29a31e 100644 --- a/src/Middleware/CheckTenantForMaintenanceMode.php +++ b/src/Middleware/CheckTenantForMaintenanceMode.php @@ -5,7 +5,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Middleware; use Closure; -use Illuminate\Foundation\Http\Exceptions\MaintenanceModeException; +use Symfony\Component\HttpKernel\Exception\HttpException; use Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode; use Stancl\Tenancy\Exceptions\TenancyNotInitializedException; use Symfony\Component\HttpFoundation\IpUtils; @@ -29,7 +29,12 @@ class CheckTenantForMaintenanceMode extends CheckForMaintenanceMode return $next($request); } - throw new MaintenanceModeException($data['time'], $data['retry'], $data['message']); + throw new HttpException( + 503, + 'Service Unavailable', + null, + isset($data['retry']) ? ['Retry-After' => $data['retry']] : [] + ); } return $next($request); diff --git a/tests/BootstrapperTest.php b/tests/BootstrapperTest.php index 1b0c880d..29aa7dc9 100644 --- a/tests/BootstrapperTest.php +++ b/tests/BootstrapperTest.php @@ -4,23 +4,27 @@ declare(strict_types=1); namespace Stancl\Tenancy\Tests; -use Illuminate\Support\Facades\Cache; +use Illuminate\Filesystem\FilesystemAdapter; +use ReflectionObject; +use ReflectionProperty; +use Illuminate\Support\Str; use Illuminate\Support\Facades\DB; +use Stancl\JobPipeline\JobPipeline; +use Stancl\Tenancy\Tests\Etc\Tenant; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Redis; use Illuminate\Support\Facades\Storage; -use Stancl\JobPipeline\JobPipeline; -use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper; -use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; -use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; -use Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper; use Stancl\Tenancy\Events\TenancyEnded; -use Stancl\Tenancy\Events\TenancyInitialized; -use Stancl\Tenancy\Events\TenantCreated; use Stancl\Tenancy\Jobs\CreateDatabase; +use Stancl\Tenancy\Events\TenantCreated; +use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Listeners\BootstrapTenancy; use Stancl\Tenancy\Listeners\RevertToCentralContext; -use Stancl\Tenancy\Tests\Etc\Tenant; +use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper; +use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; class BootstrapperTest extends TestCase { @@ -165,6 +169,7 @@ class BootstrapperTest extends TestCase $tenant2 = Tenant::create(); tenancy()->initialize($tenant1); + Storage::disk('public')->put('foo', 'bar'); $this->assertSame('bar', Storage::disk('public')->get('foo')); @@ -184,30 +189,38 @@ class BootstrapperTest extends TestCase $this->assertFalse(Storage::disk('public')->exists('foo')); $this->assertFalse(Storage::disk('public')->exists('abc')); + $expected_storage_path = $old_storage_path . '/tenant' . tenant('id'); // /tenant = suffix base + + // Check that disk prefixes respect the root_override logic + $this->assertSame($expected_storage_path . '/app/', $this->getDiskPrefix('local')); + $this->assertSame($expected_storage_path . '/app/public/', $this->getDiskPrefix('public')); + $this->assertSame('tenant' . tenant('id') . '/', ltrim($this->getDiskPrefix('s3'), '/')); + // Check suffixing logic $new_storage_path = storage_path(); - $this->assertEquals($old_storage_path . '/' . config('tenancy.filesystem.suffix_base') . tenant('id'), $new_storage_path); + $this->assertEquals($expected_storage_path, $new_storage_path); + } - foreach (config('tenancy.filesystem.disks') as $disk) { - $suffix = config('tenancy.filesystem.suffix_base') . tenant('id'); + protected function getDiskPrefix(string $disk): string + { + /** @var FilesystemAdapter $disk */ + $disk = Storage::disk($disk); + $adapter = $disk->getAdapter(); - /** @var FilesystemAdapter $filesystemDisk */ - $filesystemDisk = Storage::disk($disk); - - $current_path_prefix = $filesystemDisk->getAdapter()->getPathPrefix(); - - if ($override = config("tenancy.filesystem.root_override.{$disk}")) { - $correct_path_prefix = str_replace('%storage_path%', storage_path(), $override); - } else { - if ($base = $old_storage_facade_roots[$disk]) { - $correct_path_prefix = $base . "/$suffix/"; - } else { - $correct_path_prefix = "$suffix/"; - } - } - - $this->assertSame($correct_path_prefix, $current_path_prefix); + if (! Str::startsWith(app()->version(), '9.')) { + return $adapter->getPathPrefix(); } + + $prefixer = (new ReflectionObject($adapter))->getProperty('prefixer'); + $prefixer->setAccessible(true); + + // reflection -> instance + $prefixer = $prefixer->getValue($adapter); + + $prefix = (new ReflectionProperty($prefixer, 'prefix')); + $prefix->setAccessible(true); + + return $prefix->getValue($prefixer); } // for queues see QueueTest diff --git a/tests/Etc/ConsoleKernel.php b/tests/Etc/ConsoleKernel.php index 1bc66365..9d37d3c6 100644 --- a/tests/Etc/ConsoleKernel.php +++ b/tests/Etc/ConsoleKernel.php @@ -4,7 +4,7 @@ declare(strict_types=1); namespace Stancl\Tenancy\Tests\Etc; -use Orchestra\Testbench\Console\Kernel; +use Orchestra\Testbench\Foundation\Console\Kernel; class ConsoleKernel extends Kernel { diff --git a/tests/MaintenanceModeTest.php b/tests/MaintenanceModeTest.php index a8ecb064..4a8d8d0c 100644 --- a/tests/MaintenanceModeTest.php +++ b/tests/MaintenanceModeTest.php @@ -4,12 +4,14 @@ declare(strict_types=1); namespace Stancl\Tenancy\Tests; +use Symfony\Component\HttpKernel\Exception\HttpException; use Illuminate\Foundation\Http\Exceptions\MaintenanceModeException; use Illuminate\Support\Facades\Route; use Stancl\Tenancy\Database\Concerns\MaintenanceMode; use Stancl\Tenancy\Middleware\CheckTenantForMaintenanceMode; use Stancl\Tenancy\Middleware\InitializeTenancyByDomain; use Stancl\Tenancy\Tests\Etc\Tenant; +use Symfony\Component\HttpKernel\Exception\ServiceUnavailableHttpException; class MaintenanceModeTest extends TestCase { @@ -32,7 +34,7 @@ class MaintenanceModeTest extends TestCase $tenant->putDownForMaintenance(); - $this->expectException(MaintenanceModeException::class); + $this->expectException(HttpException::class); $this->withoutExceptionHandling() ->get('http://acme.localhost/foo'); } diff --git a/tests/TestCase.php b/tests/TestCase.php index d3e42ea1..cea669a1 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -87,6 +87,7 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase 'public', 's3', ], + 'filesystems.disks.s3.bucket' => 'foo', 'tenancy.redis.tenancy' => env('TENANCY_TEST_REDIS_TENANCY', true), 'database.redis.client' => env('TENANCY_TEST_REDIS_CLIENT', 'phpredis'), 'tenancy.redis.prefixed_connections' => ['default'],