Misc test fixes (#44)

* Add suffix_storage_path test

* Get filesystem bootstrapper coverage to 100%

* Delete enabling DB bootstrapper in TestCase

* Complete most of test todos

* Complete last tests todo

* Fix docblock

* add todo

---------

Co-authored-by: lukinovec <lukinovec@gmail.com>

phpunit.xml

@@ -15,7 +15,7 @@
     <env name="APP_ENV" value="testing"/>
     <env name="APP_KEY" value="base64:uYVmTs9lrQbXWfHgSSiG0VZMjc2KG/fBbjV1i1JDVos="/>
     <env name="BCRYPT_ROUNDS" value="4"/>
-    <env name="CACHE_DRIVER" value="redis"/>
+    <env name="CACHE_STORE" value="redis"/>
     <env name="MAIL_DRIVER" value="array"/>
     <env name="QUEUE_CONNECTION" value="sync"/>
     <env name="SESSION_DRIVER" value="array"/>

src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php

@@ -34,7 +34,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper
      *
      * For example:
      *
-     * FortifyRouteTenancyBootstrapper::$fortifyRedirectMap = [
+     * FortifyRouteBootstrapper::$fortifyRedirectMap = [
      *     // On logout, redirect the user to the "bye" route in the central app
      *     'logout' => [
      *         'route_name' => 'bye',
@@ -46,7 +46,7 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper
      *        'route_name' => 'welcome',
      *        'context' => Context::TENANT,
      *     ],
-     *];
+     * ];
      */
     public static array $fortifyRedirectMap = [];
 

tests/AutomaticModeTest.php

@@ -15,12 +15,18 @@ beforeEach(function () {
     Event::listen(TenancyEnded::class, RevertToCentralContext::class);
 });
 
-test('context is switched when tenancy is initialized', function () {
-    contextIsSwitchedWhenTenancyInitialized();
-});
+test('context is switched to tenant when initializing tenancy and reverted when ending tenancy', function () {
+    config(['tenancy.bootstrappers' => [
+        MyBootstrapper::class,
+    ]]);
 
-test('context is reverted when tenancy is ended', function () {
-    contextIsSwitchedWhenTenancyInitialized();
+    $tenant = Tenant::create([
+        'id' => 'acme',
+    ]);
+
+    tenancy()->initialize($tenant);
+
+    expect(app('tenancy_initialized_for_tenant'))->toBe('acme');
 
     tenancy()->end();
 
@@ -95,22 +101,6 @@ test('central helper doesnt change tenancy state when called in central context'
     expect(tenant())->toBeNull();
 });
 
-// todo@tests
-function contextIsSwitchedWhenTenancyInitialized()
-{
-    config(['tenancy.bootstrappers' => [
-        MyBootstrapper::class,
-    ]]);
-
-    $tenant = Tenant::create([
-        'id' => 'acme',
-    ]);
-
-    tenancy()->initialize($tenant);
-
-    expect(app('tenancy_initialized_for_tenant'))->toBe('acme');
-}
-
 class MyBootstrapper implements TenancyBootstrapper
 {
     public function bootstrap(\Stancl\Tenancy\Contracts\Tenant $tenant): void

tests/BootstrapperTest.php

@@ -319,6 +319,41 @@ test('files can get fetched using the storage url', function() {
     expect(file_get_contents(public_path($url)))->toBe($centralFileContent);
 });
 
+test('storage_path helper does not change if suffix_storage_path is off', function() {
+    $originalStoragePath = storage_path();
+
+    // todo@tests https://github.com/tenancy-for-laravel/v4/pull/44#issue-2228530362
+
+    config([
+        'tenancy.bootstrappers' => [FilesystemTenancyBootstrapper::class],
+        'tenancy.filesystem.suffix_storage_path' => false,
+    ]);
+
+    tenancy()->initialize(Tenant::create());
+
+    $this->assertEquals($originalStoragePath, storage_path());
+});
+
+test('links to storage disks with a configured root are suffixed if not overridden', function() {
+    config([
+        'filesystems.disks.public.root' => 'http://sample-s3-url.com/my-app',
+        'tenancy.bootstrappers' => [
+            FilesystemTenancyBootstrapper::class,
+        ],
+        'tenancy.filesystem.root_override.public' => null,
+        'tenancy.filesystem.url_override.public' => null,
+    ]);
+
+    $tenant = Tenant::create();
+
+    $expectedStoragePath = storage_path() . '/tenant' . $tenant->getTenantKey(); // /tenant = suffix base
+
+    tenancy()->initialize($tenant);
+
+    // Check suffixing logic
+    expect(storage_path())->toEqual($expectedStoragePath);
+});
+
 test('create and delete storage symlinks jobs work', function() {
     Event::listen(
         TenantCreated::class,

tests/CommandsTest.php

@@ -232,18 +232,43 @@ test('seed command works', function () {
     });
 });
 
-test('database connection is switched to default', function () {
-    databaseConnectionSwitchedToDefault();
-});
+test('database connection is switched to default after running commands', function (bool $initializeTenancy) {
+    $tenant = Tenant::create();
 
-test('database connection is switched to default when tenancy has been initialized', function () {
-    tenancy()->initialize(Tenant::create());
+    if ($initializeTenancy) {
+        tenancy()->initialize($tenant);
+    }
 
-    databaseConnectionSwitchedToDefault();
-});
+    $originalDBName = DB::connection()->getDatabaseName();
+
+    Artisan::call('tenants:migrate');
+    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
+
+    Artisan::call('tenants:seed', ['--class' => ExampleSeeder::class]);
+    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
+
+    Artisan::call('tenants:rollback');
+    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
+
+    Artisan::call('tenants:migrate', ['--tenants' => [$tenant->getTenantKey()]]);
+
+    pest()->artisan("tenants:run --tenants={$tenant->getTenantKey()} 'foo foo --b=bar --c=xyz'");
+
+    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
+})->with([
+    'tenancy initialized' => true,
+    'tenancy not initialized' => false,
+]);
 
 test('run command works', function () {
-    runCommandWorks();
+    $id = Tenant::create()->getTenantKey();
+
+    Artisan::call('tenants:migrate', ['--tenants' => [$id]]);
+
+    pest()->artisan("tenants:run --tenants=$id 'foo foo --b=bar --c=xyz'")
+        ->expectsOutput("User's name is Test user")
+        ->expectsOutput('foo')
+        ->expectsOutput('xyz');
 });
 
 test('install command works', function () {
@@ -404,35 +429,3 @@ test('migrate fresh command only deletes tenant databases if drop_tenant_databas
         expect($tenantHasDatabase($tenant))->toBe($shouldHaveDBAfterMigrateFresh);
     }
 })->with([true, false]);
-
-// todo@tests
-function runCommandWorks(): void
-{
-    $id = Tenant::create()->getTenantKey();
-
-    Artisan::call('tenants:migrate', ['--tenants' => [$id]]);
-
-    pest()->artisan("tenants:run --tenants=$id 'foo foo --b=bar --c=xyz' ")
-        ->expectsOutput("User's name is Test user")
-        ->expectsOutput('foo')
-        ->expectsOutput('xyz');
-}
-
-// todo@tests
-function databaseConnectionSwitchedToDefault()
-{
-    $originalDBName = DB::connection()->getDatabaseName();
-
-    Artisan::call('tenants:migrate');
-    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
-
-    Artisan::call('tenants:seed', ['--class' => ExampleSeeder::class]);
-    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
-
-    Artisan::call('tenants:rollback');
-    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
-
-    runCommandWorks();
-
-    expect(DB::connection()->getDatabaseName())->toBe($originalDBName);
-}

tests/SingleDatabaseTenancyTest.php

@@ -35,30 +35,55 @@ beforeEach(function () {
 });
 
 test('primary models are scoped to the current tenant', function () {
-    primaryModelsScopedToCurrentTenant();
-});
+    // acme context
+    tenancy()->initialize($acme = Tenant::create([
+        'id' => 'acme',
+    ]));
 
-test('primary models are not scoped in the central context', function () {
-    primaryModelsScopedToCurrentTenant();
+    $post = Post::create(['text' => 'Foo']);
 
+    expect($post->tenant_id)->toBe('acme');
+    expect($post->tenant->id)->toBe('acme');
+
+    $post = Post::first();
+
+    expect($post->tenant_id)->toBe('acme');
+    expect($post->tenant->id)->toBe('acme');
+
+    // ======================================
+    // foobar context
+    tenancy()->initialize(Tenant::create([
+        'id' => 'foobar',
+    ]));
+
+    $post = Post::create(['text' => 'Bar']);
+
+    expect($post->tenant_id)->toBe('foobar');
+    expect($post->tenant->id)->toBe('foobar');
+
+    $post = Post::first();
+
+    expect($post->tenant_id)->toBe('foobar');
+    expect($post->tenant->id)->toBe('foobar');
+
+    // ======================================
+    // acme context again
+
+    tenancy()->initialize($acme);
+
+    $post = Post::first();
+    expect($post->tenant_id)->toBe('acme');
+    expect($post->tenant->id)->toBe('acme');
+
+    // Assert foobar models are inaccessible in acme context
+    expect(Post::count())->toBe(1);
+
+    // Primary models are not scoped in the central context
     tenancy()->end();
 
     expect(Post::count())->toBe(2);
 });
 
-test('secondary models are scoped to the current tenant when accessed via primary model', function () {
-    secondaryModelsAreScopedToCurrentTenant();
-});
-
-test('secondary models are not scoped to the current tenant when accessed directly', function () {
-    secondaryModelsAreScopedToCurrentTenant();
-
-    // We're in acme context
-    expect(tenant('id'))->toBe('acme');
-
-    expect(Comment::count())->toBe(2);
-});
-
 test('secondary models ARE scoped to the current tenant when accessed directly and parent relationship trait is used', function () {
     $acme = Tenant::create([
         'id' => 'acme',
@@ -91,9 +116,37 @@ test('secondary models ARE scoped to the current tenant when accessed directly a
     expect(ScopedComment::count())->toBe(2);
 });
 
-test('secondary models are not scoped in the central context', function () {
-    secondaryModelsAreScopedToCurrentTenant();
+test('secondary models are scoped correctly', function () {
+    // Secondary models are scoped to the current tenant when accessed via primary model
+    // acme context
+    tenancy()->initialize($acme = Tenant::create([
+        'id' => 'acme',
+    ]));
 
+    $post = Post::create(['text' => 'Foo']);
+    $post->comments()->create(['text' => 'Comment text']);
+
+    // ================
+    // foobar context
+    tenancy()->initialize(Tenant::create([
+        'id' => 'foobar',
+    ]));
+
+    $post = Post::create(['text' => 'Bar']);
+    $post->comments()->create(['text' => 'Comment text 2']);
+
+    // ================
+    // acme context again
+    tenancy()->initialize($acme);
+    expect(Post::count())->toBe(1);
+    expect(Post::first()->comments->count())->toBe(1);
+
+    // Secondary models are not scoped to the current tenant when accessed directly
+    expect(tenant('id'))->toBe('acme');
+
+    expect(Comment::count())->toBe(2);
+
+    // secondary models are not scoped in the central context
     tenancy()->end();
 
     expect(Comment::count())->toBe(2);
@@ -225,80 +278,6 @@ test('the model returned by the tenant helper has unique and exists validation r
     expect($existsFails)->toBeFalse();
 });
 
-// todo@tests
-function primaryModelsScopedToCurrentTenant()
-{
-    // acme context
-    tenancy()->initialize($acme = Tenant::create([
-        'id' => 'acme',
-    ]));
-
-    $post = Post::create(['text' => 'Foo']);
-
-    expect($post->tenant_id)->toBe('acme');
-    expect($post->tenant->id)->toBe('acme');
-
-    $post = Post::first();
-
-    expect($post->tenant_id)->toBe('acme');
-    expect($post->tenant->id)->toBe('acme');
-
-    // ======================================
-    // foobar context
-    tenancy()->initialize($foobar = Tenant::create([
-        'id' => 'foobar',
-    ]));
-
-    $post = Post::create(['text' => 'Bar']);
-
-    expect($post->tenant_id)->toBe('foobar');
-    expect($post->tenant->id)->toBe('foobar');
-
-    $post = Post::first();
-
-    expect($post->tenant_id)->toBe('foobar');
-    expect($post->tenant->id)->toBe('foobar');
-
-    // ======================================
-    // acme context again
-
-    tenancy()->initialize($acme);
-
-    $post = Post::first();
-    expect($post->tenant_id)->toBe('acme');
-    expect($post->tenant->id)->toBe('acme');
-
-    // Assert foobar models are inaccessible in acme context
-    expect(Post::count())->toBe(1);
-}
-
-// todo@tests
-function secondaryModelsAreScopedToCurrentTenant()
-{
-    // acme context
-    tenancy()->initialize($acme = Tenant::create([
-        'id' => 'acme',
-    ]));
-
-    $post = Post::create(['text' => 'Foo']);
-    $post->comments()->create(['text' => 'Comment text']);
-
-    // ================
-    // foobar context
-    tenancy()->initialize($foobar = Tenant::create([
-        'id' => 'foobar',
-    ]));
-
-    $post = Post::create(['text' => 'Bar']);
-    $post->comments()->create(['text' => 'Comment text 2']);
-
-    // ================
-    // acme context again
-    tenancy()->initialize($acme);
-    expect(Post::count())->toBe(1);
-    expect(Post::first()->comments->count())->toBe(1);
-}
-
 class Tenant extends TestTenant
 {
     use HasScopedValidationRules;

tests/TestCase.php

@@ -18,9 +18,7 @@ use Stancl\Tenancy\Bootstrappers\RootUrlBootstrapper;
 use Stancl\Tenancy\Bootstrappers\MailConfigBootstrapper;
 use Stancl\Tenancy\Bootstrappers\RedisTenancyBootstrapper;
 use Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper;
-use Stancl\Tenancy\Bootstrappers\DatabaseTenancyBootstrapper;
 use Stancl\Tenancy\Bootstrappers\BroadcastingConfigBootstrapper;
-use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper;
 use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper;
 
 abstract class TestCase extends \Orchestra\Testbench\TestCase