From 942d79cbd7da38337f4cd03577a738eb29cd2d96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 8 Nov 2022 13:34:04 +0100 Subject: [PATCH 1/3] resolve all phpstan issues --- phpstan.neon | 12 ++++-- src/Commands/ClearPendingTenants.php | 20 +++------ src/Commands/CreatePendingTenants.php | 23 ++-------- src/Commands/Install.php | 1 + src/Commands/MigrateFresh.php | 4 +- src/Concerns/DealsWithMigrations.php | 3 ++ src/Database/Concerns/HasPending.php | 43 +++++++++---------- src/Jobs/ClearPendingTenants.php | 7 +-- src/Jobs/CreatePendingTenants.php | 7 +-- src/Middleware/InitializeTenancyByPath.php | 2 - .../InitializeTenancyByRequestData.php | 19 ++++---- src/Tenancy.php | 4 +- 12 files changed, 57 insertions(+), 88 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 0567d5ff..a6bce96d 100644 --- a/phpstan.neon +++ b/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 diff --git a/src/Commands/ClearPendingTenants.php b/src/Commands/ClearPendingTenants.php index 18d9fa42..19d31195 100644 --- a/src/Commands/ClearPendingTenants.php +++ b/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.'; - /** - * Execute the console command. - */ - public function handle() + public function handle(): int { $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; } } diff --git a/src/Commands/CreatePendingTenants.php b/src/Commands/CreatePendingTenants.php index 88202093..7b2c7934 100644 --- a/src/Commands/CreatePendingTenants.php +++ b/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.'; - /** - * Execute the console command. - */ - public function handle() + public function handle(): int { $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. - */ - private function getPendingTenantCount(): int + /** Calculate the number of currently available pending tenants. */ + protected function getPendingTenantCount(): int { return tenancy() ->query() diff --git a/src/Commands/Install.php b/src/Commands/Install.php index 77c96588..c7041a72 100644 --- a/src/Commands/Install.php +++ b/src/Commands/Install.php @@ -117,6 +117,7 @@ class Install extends Command $this->newLine(); } } else { + /** @var string $warning */ $this->components->warn($warning); } } diff --git a/src/Commands/MigrateFresh.php b/src/Commands/MigrateFresh.php index 45a93115..7df75fb0 100644 --- a/src/Commands/MigrateFresh.php +++ b/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; diff --git a/src/Concerns/DealsWithMigrations.php b/src/Concerns/DealsWithMigrations.php index 3129c68d..3a757271 100644 --- a/src/Concerns/DealsWithMigrations.php +++ b/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 diff --git a/src/Database/Concerns/HasPending.php b/src/Database/Concerns/HasPending.php index 3fa9399d..4d72486f 100644 --- a/src/Database/Concerns/HasPending.php +++ b/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 has pending trait for a model. - * - * @return void - */ - public static function bootHasPending() + /** Boot the trait. */ + public static function bootHasPending(): void { static::addGlobalScope(new PendingScope()); } - /** - * Initialize the has pending trait for an instance. - * - * @return void - */ - public function initializeHasPending() + /** Initialize the trait. */ + public function initializeHasPending(): void { $this->casts['pending_since'] = 'timestamp'; } - /** - * Determine if the model instance is in a pending state. - * - * @return bool - */ - public function pending() + /** Determine if the model instance is in a pending state. */ + public function pending(): bool { return ! is_null($this->pending_since); } - /** Create a pending tenant. */ - public static function createPending($attributes = []): Tenant + /** + * Create a pending tenant. + * + * @param array $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)); diff --git a/src/Jobs/ClearPendingTenants.php b/src/Jobs/ClearPendingTenants.php index 7cd78495..773e3e93 100644 --- a/src/Jobs/ClearPendingTenants.php +++ b/src/Jobs/ClearPendingTenants.php @@ -16,12 +16,7 @@ class ClearPendingTenants implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; - /** - * Execute the job. - * - * @return void - */ - public function handle() + public function handle(): void { Artisan::call(ClearPendingTenantsCommand::class); } diff --git a/src/Jobs/CreatePendingTenants.php b/src/Jobs/CreatePendingTenants.php index 8f3da218..81199761 100644 --- a/src/Jobs/CreatePendingTenants.php +++ b/src/Jobs/CreatePendingTenants.php @@ -16,12 +16,7 @@ class CreatePendingTenants implements ShouldQueue { use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; - /** - * Execute the job. - * - * @return void - */ - public function handle() + public function handle(): void { Artisan::call(CreatePendingTenantsCommand::class); } diff --git a/src/Middleware/InitializeTenancyByPath.php b/src/Middleware/InitializeTenancyByPath.php index 3e484f87..e73605e3 100644 --- a/src/Middleware/InitializeTenancyByPath.php +++ b/src/Middleware/InitializeTenancyByPath.php @@ -45,8 +45,6 @@ class InitializeTenancyByPath extends IdentificationMiddleware } else { throw new RouteIsMissingTenantParameterException; } - - return $next($request); } protected function setDefaultTenantForRouteParametersWhenTenancyIsInitialized(): void diff --git a/src/Middleware/InitializeTenancyByRequestData.php b/src/Middleware/InitializeTenancyByRequestData.php index ca29f3d7..925907f0 100644 --- a/src/Middleware/InitializeTenancyByRequestData.php +++ b/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)) { - return $request->get(static::$queryParameter); - } - - if (static::$cookie && $request->hasCookie(static::$cookie)) { - return $request->cookie(static::$cookie); - } - - return null; + /** @var ?string $payload */ + return $payload; } } diff --git a/src/Tenancy.php b/src/Tenancy.php index e95e0059..5b30e3e0 100644 --- a/src/Tenancy.php +++ b/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)); From 99dd862b20bf643b70fb97388f7d9f309d3832a6 Mon Sep 17 00:00:00 2001 From: Abrar Ahmad Date: Tue, 8 Nov 2022 17:47:24 +0500 Subject: [PATCH 2/3] [4.x] [WIP] Add phpstan to CI (#928) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add phpstan * resolve phpstan issue from CI Co-authored-by: Samuel Štancl --- .github/workflows/ci.yml | 9 +++++++++ composer.json | 3 ++- src/Resolvers/PathTenantResolver.php | 5 ++++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26de6a18..724aed35 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/composer.json b/composer.json index 587bbb06..68f16f25 100644 --- a/composer.json +++ b/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" diff --git a/src/Resolvers/PathTenantResolver.php b/src/Resolvers/PathTenantResolver.php index 1359e9c1..090ea365 100644 --- a/src/Resolvers/PathTenantResolver.php +++ b/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)) { From ea3e44576fc2ca8939708da73d58302203036a80 Mon Sep 17 00:00:00 2001 From: Abrar Ahmad Date: Wed, 9 Nov 2022 17:00:54 +0500 Subject: [PATCH 3/3] [4.x] Resource syncing improvements (#992) * Update TenantSyncingTest.php * Update ResourceSyncingTest.php * rename UserTenant to ResourceTenant * Revert "rename UserTenant to ResourceTenant" This reverts commit f9ba778e1b6da7f19a191e68b07248b4dee1ddc3. * rename TenantUser class * return style * Update ResourceSyncingTest.php * revert return style * Update ResourceSyncingTest.php --- tests/ResourceSyncingTest.php | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index e1586bc1..811b8d1a 100644 --- a/tests/ResourceSyncingTest.php +++ b/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([ @@ -299,7 +299,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([ @@ -311,7 +311,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 @@ -319,7 +319,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(); + $resourceUser = TenantUserProvidingMixture::first(); expect($resourceUser)->not()->toBeNull(); $resourceUser = $resourceUser->toArray(); $centralUser = $centralUser->withoutRelations()->toArray(); @@ -332,7 +332,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', @@ -829,6 +829,7 @@ function migrateUsersTableForTenants(): void ])->assertExitCode(0); } +// Tenant model used for resource syncing setup class ResourceTenant extends Tenant { public function users() @@ -885,6 +886,7 @@ class CentralUser extends Model implements SyncMaster } } +// Tenant users class ResourceUser extends Model implements Syncable { use ResourceSyncing; @@ -922,7 +924,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 { @@ -939,7 +941,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 { @@ -1004,7 +1006,7 @@ class CentralUserProvidingMixture extends CentralUser } } -class ResourceUserProvidingMixture extends ResourceUser +class TenantUserProvidingMixture extends ResourceUser { public function getSyncedCreationAttributes(): array {