Merge branch 'master' into poly-sync

.github/workflows/ci.yml

@@ -103,3 +103,12 @@ jobs:
           author_email: "phpcsfixer@example.com"
           message: Fix code style (php-cs-fixer)
 
+  phpstan:
+    name: Static analysis (PHPStan)
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - name: Install composer dependencies
+        run: composer install
+      - name: Run phpstan
+        run: vendor/bin/phpstan analyse

composer.json

@@ -63,7 +63,8 @@
         "docker-rebuild": "PHP_VERSION=8.1 docker-compose up -d --no-deps --build",
         "docker-m1": "ln -s docker-compose-m1.override.yml docker-compose.override.yml",
         "coverage": "open coverage/phpunit/html/index.html",
-        "phpstan": "vendor/bin/phpstan --pro",
+        "phpstan": "vendor/bin/phpstan",
+        "phpstan-pro": "vendor/bin/phpstan --pro",
         "cs": "php-cs-fixer fix --config=.php-cs-fixer.php",
         "test": "PHP_VERSION=8.1 ./test --no-coverage",
         "test-full": "PHP_VERSION=8.1 ./test"

phpstan.neon

@@ -16,14 +16,17 @@ parameters:
     ignoreErrors:
         - '#Cannot access offset (.*?) on Illuminate\\Contracts\\Foundation\\Application#'
         - '#Cannot access offset (.*?) on Illuminate\\Contracts\\Config\\Repository#'
+        -
+            message: '#Call to an undefined (method|static method) Illuminate\\Database\\Eloquent\\(Model|Builder)#'
+            paths:
+                - src/Commands/CreatePendingTenants.php
+                - src/Commands/ClearPendingTenants.php
+                - src/Database/Concerns/PendingScope.php
+                - src/Database/ParentModelScope.php
         -
             message: '#invalid type Laravel\\Telescope\\IncomingEntry#'
             paths:
                 - src/Features/TelescopeTags.php
-        -
-            message: '#Call to an undefined method Illuminate\\Database\\Eloquent\\Model::getRelationshipToPrimaryModel\(\)#'
-            paths:
-                - src/Database/ParentModelScope.php
         -
             message: '#Parameter \#1 \$key of method Illuminate\\Contracts\\Cache\\Repository::put\(\) expects string#'
             paths:
@@ -44,6 +47,7 @@ parameters:
             message: '#Trying to invoke Closure\|null but it might not be a callable#'
             paths:
                 - src/Database/DatabaseConfig.php
+        - '#Method Stancl\\Tenancy\\Tenancy::cachedResolvers\(\) should return array#'
 
     checkMissingIterableValueType: false
     treatPhpDocTypesAsCertain: false

src/Commands/ClearPendingTenants.php

@@ -9,27 +9,14 @@ use Illuminate\Database\Eloquent\Builder;
 
 class ClearPendingTenants extends Command
 {
-    /**
-     * The name and signature of the console command.
-     *
-     * @var string
-     */
     protected $signature = 'tenants:pending-clear
                             {--all : Override the default settings and deletes all pending tenants}
                             {--older-than-days= : Deletes all pending tenants older than the amount of days}
                             {--older-than-hours= : Deletes all pending tenants older than the amount of hours}';
 
-    /**
-     * The console command description.
-     *
-     * @var string
-     */
     protected $description = 'Remove pending tenants.';
 
-    /**
+    public function handle(): int
-     * Execute the console command.
-     */
-    public function handle()
     {
         $this->info('Removing pending tenants.');
 
@@ -39,7 +26,10 @@ class ClearPendingTenants extends Command
 
         // Skip the time constraints if the 'all' option is given
         if (! $this->option('all')) {
+            /** @var ?int $olderThanDays */
             $olderThanDays = $this->option('older-than-days');
+
+            /** @var ?int $olderThanHours */
             $olderThanHours = $this->option('older-than-hours');
 
             if ($olderThanDays && $olderThanHours) {
@@ -70,5 +60,7 @@ class ClearPendingTenants extends Command
             ->count();
 
         $this->info($deletedTenantCount . ' pending ' . str('tenant')->plural($deletedTenantCount) . ' deleted.');
+
+        return 0;
     }
 }

src/Commands/CreatePendingTenants.php

@@ -8,24 +8,11 @@ use Illuminate\Console\Command;
 
 class CreatePendingTenants extends Command
 {
-    /**
-     * The name and signature of the console command.
-     *
-     * @var string
-     */
     protected $signature = 'tenants:pending-create {--count= : The number of pending tenants to be created}';
 
-    /**
-     * The console command description.
-     *
-     * @var string
-     */
     protected $description = 'Create pending tenants.';
 
-    /**
+    public function handle(): int
-     * Execute the console command.
-     */
-    public function handle()
     {
         $this->info('Creating pending tenants.');
 
@@ -46,13 +33,11 @@ class CreatePendingTenants extends Command
         $this->info($createdCount . ' ' . str('tenant')->plural($createdCount) . ' created.');
         $this->info($maxPendingTenantCount . ' ' . str('tenant')->plural($maxPendingTenantCount) . ' ready to be used.');
 
-        return 1;
+        return 0;
     }
 
-    /**
+    /** Calculate the number of currently available pending tenants. */
-     * Calculate the number of currently available pending tenants.
+    protected function getPendingTenantCount(): int
-     */
-    private function getPendingTenantCount(): int
     {
         return tenancy()
             ->query()

src/Commands/Install.php

@@ -117,6 +117,7 @@ class Install extends Command
                 $this->newLine();
             }
         } else {
+            /** @var string $warning */
             $this->components->warn($warning);
         }
     }

src/Commands/MigrateFresh.php

@@ -4,12 +4,12 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Commands;
 
-use Illuminate\Console\Command;
+use Illuminate\Database\Console\Migrations\BaseCommand;
 use Stancl\Tenancy\Concerns\DealsWithMigrations;
 use Stancl\Tenancy\Concerns\HasTenantOptions;
 use Symfony\Component\Console\Input\InputOption;
 
-class MigrateFresh extends Command
+class MigrateFresh extends BaseCommand
 {
     use HasTenantOptions, DealsWithMigrations;
 

src/Concerns/DealsWithMigrations.php

@@ -4,6 +4,9 @@ declare(strict_types=1);
 
 namespace Stancl\Tenancy\Concerns;
 
+/**
+ * @mixin \Illuminate\Database\Console\Migrations\BaseCommand
+ */
 trait DealsWithMigrations
 {
     protected function getMigrationPaths(): array

src/Database/Concerns/HasPending.php

@@ -5,6 +5,7 @@ declare(strict_types=1);
 namespace Stancl\Tenancy\Database\Concerns;
 
 use Carbon\Carbon;
+use Illuminate\Database\Eloquent\Model;
 use Stancl\Tenancy\Contracts\Tenant;
 use Stancl\Tenancy\Events\CreatingPendingTenant;
 use Stancl\Tenancy\Events\PendingTenantCreated;
@@ -14,7 +15,7 @@ use Stancl\Tenancy\Events\PullingPendingTenant;
 // todo consider adding a method that sets pending_since to null — to flag tenants as not-pending
 
 /**
- * @property Carbon $pending_since
+ * @property ?Carbon $pending_since
  *
  * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder withPending(bool $withPending = true)
  * @method static static|\Illuminate\Database\Eloquent\Builder|\Illuminate\Database\Query\Builder onlyPending()
@@ -22,38 +23,30 @@ use Stancl\Tenancy\Events\PullingPendingTenant;
  */
 trait HasPending
 {
-    /**
+    /** Boot the trait. */
-     * Boot the has pending trait for a model.
+    public static function bootHasPending(): void
-     *
-     * @return void
-     */
-    public static function bootHasPending()
     {
         static::addGlobalScope(new PendingScope());
     }
 
-    /**
+    /** Initialize the trait. */
-     * Initialize the has pending trait for an instance.
+    public function initializeHasPending(): void
-     *
-     * @return void
-     */
-    public function initializeHasPending()
     {
         $this->casts['pending_since'] = 'timestamp';
     }
 
-    /**
+    /** Determine if the model instance is in a pending state. */
-     * Determine if the model instance is in a pending state.
+    public function pending(): bool
-     *
-     * @return bool
-     */
-    public function pending()
     {
         return ! is_null($this->pending_since);
     }
 
-    /** Create a pending tenant. */
+    /**
-    public static function createPending($attributes = []): Tenant
+     * Create a pending tenant.
+     *
+     * @param array<string, mixed> $attributes
+     */
+    public static function createPending(array $attributes = []): Model&Tenant
     {
         $tenant = static::create($attributes);
 
@@ -71,9 +64,12 @@ trait HasPending
     }
 
     /** Pull a pending tenant. */
-    public static function pullPending(): Tenant
+    public static function pullPending(): Model&Tenant
     {
-        return static::pullPendingFromPool(true);
+        /** @var Model&Tenant $pendingTenant */
+        $pendingTenant = static::pullPendingFromPool(true);
+
+        return $pendingTenant;
     }
 
     /** Try to pull a tenant from the pool of pending tenants. */
@@ -88,6 +84,7 @@ trait HasPending
         }
 
         // A pending tenant is surely available at this point
+        /** @var Model&Tenant $tenant */
         $tenant = static::onlyPending()->first();
 
         event(new PullingPendingTenant($tenant));

src/Jobs/ClearPendingTenants.php

@@ -16,12 +16,7 @@ class ClearPendingTenants implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
-    /**
+    public function handle(): void
-     * Execute the job.
-     *
-     * @return void
-     */
-    public function handle()
     {
         Artisan::call(ClearPendingTenantsCommand::class);
     }

src/Jobs/CreatePendingTenants.php

@@ -16,12 +16,7 @@ class CreatePendingTenants implements ShouldQueue
 {
     use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;
 
-    /**
+    public function handle(): void
-     * Execute the job.
-     *
-     * @return void
-     */
-    public function handle()
     {
         Artisan::call(CreatePendingTenantsCommand::class);
     }

src/Middleware/InitializeTenancyByPath.php

@@ -45,8 +45,6 @@ class InitializeTenancyByPath extends IdentificationMiddleware
         } else {
             throw new RouteIsMissingTenantParameterException;
         }
-
-        return $next($request);
     }
 
     protected function setDefaultTenantForRouteParametersWhenTenancyIsInitialized(): void

src/Middleware/InitializeTenancyByRequestData.php

@@ -34,18 +34,17 @@ class InitializeTenancyByRequestData extends IdentificationMiddleware
 
     protected function getPayload(Request $request): ?string
     {
+        $payload = null;
+
         if (static::$header && $request->hasHeader(static::$header)) {
-            return $request->header(static::$header);
+            $payload = $request->header(static::$header);
+        } elseif (static::$queryParameter && $request->has(static::$queryParameter)) {
+            $payload = $request->get(static::$queryParameter);
+        } elseif (static::$cookie && $request->hasCookie(static::$cookie)) {
+            $payload = $request->cookie(static::$cookie);
         }
 
-        if (static::$queryParameter && $request->has(static::$queryParameter)) {
+        /** @var ?string $payload */
-            return $request->get(static::$queryParameter);
+        return $payload;
-        }
-
-        if (static::$cookie && $request->hasCookie(static::$cookie)) {
-            return $request->cookie(static::$cookie);
-        }
-
-        return null;
     }
 }

src/Resolvers/PathTenantResolver.php

@@ -15,7 +15,10 @@ class PathTenantResolver extends Contracts\CachedTenantResolver
         /** @var Route $route */
         $route = $args[0];
 
-        if ($id = (string) $route->parameter(static::tenantParameterName())) {
+        /** @var string $id */
+        $id = $route->parameter(static::tenantParameterName());
+
+        if ($id) {
             $route->forgetParameter(static::tenantParameterName());
 
             if ($tenant = tenancy()->find($id)) {

src/Tenancy.php

@@ -42,8 +42,7 @@ class Tenancy
             }
         }
 
-        // todo1 for phpstan this should be $this->tenant?, but first I want to clean up the $initialized logic and explore removing the property
+        if ($this->initialized && $this->tenant?->getTenantKey() === $tenant->getTenantKey()) {
-        if ($this->initialized && $this->tenant->getTenantKey() === $tenant->getTenantKey()) {
             return;
         }
 
@@ -52,6 +51,7 @@ class Tenancy
             $this->end();
         }
 
+        /** @var Tenant&Model $tenant */
         $this->tenant = $tenant;
 
         event(new Events\InitializingTenancy($this));

tests/ResourceSyncingTest.php

@@ -130,7 +130,7 @@ test('only the synced columns are updated in the central db', function () {
 // This tests attribute list on the central side, and default values on the tenant side
 // Those two don't depend on each other, we're just testing having each option on each side
 // using tests that combine the two, to avoid having an excessively long and complex test suite
-test('sync resource creation works when central model provides attributes and resource model provides default values', function () {
+test('sync resource creation works when central model provides attributes and tenant model provides default values', function () {
     [$tenant1, $tenant2] = createTenantsAndRunMigrations();
 
     addExtraColumnToCentralDB();
@@ -145,14 +145,14 @@ test('sync resource creation works when central model provides attributes and re
     ]);
 
     $tenant1->run(function () {
-        expect(ResourceUserProvidingDefaultValues::all())->toHaveCount(0);
+        expect(TenantUserProvidingDefaultValues::all())->toHaveCount(0);
     });
 
     // When central model provides the list of attributes, resource model will be created from the provided list of attributes' values
     $centralUser->tenants()->attach('t1');
 
     $tenant1->run(function () {
-        $resourceUser = ResourceUserProvidingDefaultValues::all();
+        $resourceUser = TenantUserProvidingDefaultValues::all();
         expect($resourceUser)->toHaveCount(1);
         expect($resourceUser->first()->global_id)->toBe('acme');
         expect($resourceUser->first()->email)->toBe('john@localhost');
@@ -163,7 +163,7 @@ test('sync resource creation works when central model provides attributes and re
     tenancy()->initialize($tenant2);
 
     // When resource model provides the list of default values, central model will be created from the provided list of default values
-    ResourceUserProvidingDefaultValues::create([
+    TenantUserProvidingDefaultValues::create([
         'global_id' => 'asdf',
         'name' => 'John Doe',
         'email' => 'john@localhost',
@@ -186,7 +186,7 @@ test('sync resource creation works when central model provides attributes and re
 // This tests default values on the central side, and attribute list on the tenant side
 // Those two don't depend on each other, we're just testing having each option on each side
 // using tests that combine the two, to avoid having an excessively long and complex test suite
-test('sync resource creation works when central model provides default values and resource model provides attributes', function () {
+test('sync resource creation works when central model provides default values and tenant model provides attributes', function () {
     [$tenant1, $tenant2] = createTenantsAndRunMigrations();
 
     addExtraColumnToCentralDB();
@@ -201,7 +201,7 @@ test('sync resource creation works when central model provides default values an
     ]);
 
     $tenant1->run(function () {
-        expect(ResourceUserProvidingDefaultValues::all())->toHaveCount(0);
+        expect(TenantUserProvidingDefaultValues::all())->toHaveCount(0);
     });
 
     // When central model provides the list of default values, resource model will be created from the provided list of default values
@@ -209,7 +209,7 @@ test('sync resource creation works when central model provides default values an
 
     $tenant1->run(function () {
         // Assert resource user was created using the list of default values
-        $resourceUser = ResourceUserProvidingDefaultValues::first();
+        $resourceUser = TenantUserProvidingDefaultValues::first();
         expect($resourceUser)->not()->toBeNull();
         expect($resourceUser->global_id)->toBe('acme');
         expect($resourceUser->email)->toBe('default@localhost');
@@ -220,7 +220,7 @@ test('sync resource creation works when central model provides default values an
     tenancy()->initialize($tenant2);
 
     // When resource model provides the list of attributes, central model will be created from the provided list of attributes' values
-    ResourceUserProvidingAttributeNames::create([
+    TenantUserProvidingAttributeNames::create([
         'global_id' => 'asdf',
         'name' => 'John Doe',
         'email' => 'john@localhost',
@@ -241,7 +241,7 @@ test('sync resource creation works when central model provides default values an
 // This tests mixed attribute list/defaults on the central side, and no specified attributes on the tenant side
 // Those two don't depend on each other, we're just testing having each option on each side
 // using tests that combine the two, to avoid having an excessively long and complex test suite
-test('sync resource creation works when central model provides mixture and resource model provides nothing', function () {
+test('sync resource creation works when central model provides mixture and tenant model provides nothing', function () {
     [$tenant1, $tenant2] = createTenantsAndRunMigrations();
 
     $centralUser = CentralUserProvidingMixture::create([
@@ -295,7 +295,7 @@ test('sync resource creation works when central model provides mixture and resou
 // This tests no specified attributes on the central side, and mixed attribute list/defaults on the tenant side
 // Those two don't depend on each other, we're just testing having each option on each side
 // using tests that combine the two, to avoid having an excessively long and complex test suite
-test('sync resource creation works when central model provides nothing and resource model provides mixture', function () {
+test('sync resource creation works when central model provides nothing and tenant model provides mixture', function () {
     [$tenant1, $tenant2] = createTenantsAndRunMigrations();
 
     $centralUser = CentralUser::create([
@@ -307,7 +307,7 @@ test('sync resource creation works when central model provides nothing and resou
     ]);
 
     $tenant1->run(function () {
-        expect(ResourceUserProvidingMixture::all())->toHaveCount(0);
+        expect(TenantUserProvidingMixture::all())->toHaveCount(0);
     });
 
     // When central model provides nothing/null, the resource model will be created as a 1:1 copy of central model
@@ -315,7 +315,7 @@ test('sync resource creation works when central model provides nothing and resou
 
     expect($centralUser->getSyncedCreationAttributes())->toBeNull();
     $tenant1->run(function () use ($centralUser) {
-        $resourceUser = ResourceUserProvidingMixture::first()->only(['name', 'email', 'password', 'role']);
+        $resourceUser = TenantUserProvidingMixture::first()->only(['name', 'email', 'password', 'role']);
         $centralUser = $centralUser->only(['name', 'email', 'password', 'role']);
 
         expect($resourceUser)->toBe($centralUser);
@@ -324,7 +324,7 @@ test('sync resource creation works when central model provides nothing and resou
     tenancy()->initialize($tenant2);
 
     // When resource model provides the list of a mixture (attributes and default values), central model will be created from the provided list of mixture (attributes and default values)
-    ResourceUserProvidingMixture::create([
+    TenantUserProvidingMixture::create([
         'global_id' => 'absd',
         'name' => 'John Doe',
         'email' => 'john@localhost',
@@ -821,6 +821,7 @@ function migrateUsersTableForTenants(): void
     ])->assertExitCode(0);
 }
 
+// Tenant model used for resource syncing setup
 class ResourceTenant extends Tenant
 {
     public function users(): BelongsToMany
@@ -876,6 +877,7 @@ class CentralUser extends Model implements SyncMaster
     }
 }
 
+// Tenant users
 class ResourceUser extends Model implements Syncable
 {
     use ResourceSyncing;
@@ -913,7 +915,7 @@ class ResourceUser extends Model implements Syncable
 }
 
 // override method in ResourceUser class to return default attribute values
-class ResourceUserProvidingDefaultValues extends ResourceUser
+class TenantUserProvidingDefaultValues extends ResourceUser
 {
     public function getSyncedCreationAttributes(): array
     {
@@ -930,7 +932,7 @@ class ResourceUserProvidingDefaultValues extends ResourceUser
 }
 
 // override method in ResourceUser class to return attribute names
-class ResourceUserProvidingAttributeNames extends ResourceUser
+class TenantUserProvidingAttributeNames extends ResourceUser
 {
     public function getSyncedCreationAttributes(): array
     {
@@ -995,7 +997,7 @@ class CentralUserProvidingMixture extends CentralUser
     }
 }
 
-class ResourceUserProvidingMixture extends ResourceUser
+class TenantUserProvidingMixture extends ResourceUser
 {
     public function getSyncedCreationAttributes(): array
     {