From a61724df1b05e7c57eb90409ba6e977950e6d1f8 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:45:03 +0000 Subject: [PATCH 01/10] Fix constrained pivot mutation ownership Retain every relation-owned pivot predicate, including between clauses, and replay them as one grouped condition so boolean-or scopes cannot escape the relation identity during explicit pivot writes. Carry those constraints onto hydrated Pivot and MorphPivot instances, preserve primary-key authority, and route current-pivot hydration through the relation's canonical constructor. Explicit custom-pivot attributes now use coroutine-safe forceFill semantics so casts, mutators, timestamps, and model events run without applying request mass-assignment policy. Add counterfactual coverage for grouped scope replay, range predicates, cross-parent isolation, primary-key behavior, guarded custom attributes, morph identity, and stock/custom pivot events. --- .../src/Eloquent/Relations/BelongsToMany.php | 7 + .../Eloquent/Relations/Concerns/AsPivot.php | 71 ++++- .../Concerns/InteractsWithPivotTable.php | 65 ++-- .../src/Eloquent/Relations/MorphToMany.php | 24 +- .../BelongsToManyPivotEventsTest.php | 297 ++++++++++++++++++ .../Relations/MorphToManyPivotEventsTest.php | 25 +- ...000000_create_pivot_events_test_tables.php | 27 +- 7 files changed, 472 insertions(+), 44 deletions(-) diff --git a/src/database/src/Eloquent/Relations/BelongsToMany.php b/src/database/src/Eloquent/Relations/BelongsToMany.php index 0918398f7..ce3c70dee 100644 --- a/src/database/src/Eloquent/Relations/BelongsToMany.php +++ b/src/database/src/Eloquent/Relations/BelongsToMany.php @@ -89,6 +89,11 @@ class BelongsToMany extends Relation */ protected array $pivotWhereNulls = []; + /** + * Any pivot table restrictions for whereBetween clauses. + */ + protected array $pivotWhereBetweens = []; + /** * The default values for the pivot columns. */ @@ -357,6 +362,8 @@ public function wherePivot(mixed $column, mixed $operator = null, mixed $value = */ public function wherePivotBetween(mixed $column, array $values, string $boolean = 'and', bool $not = false): static { + $this->pivotWhereBetweens[] = func_get_args(); + return $this->whereBetween($this->qualifyPivotColumn($column), $values, $boolean, $not); } diff --git a/src/database/src/Eloquent/Relations/Concerns/AsPivot.php b/src/database/src/Eloquent/Relations/Concerns/AsPivot.php index 26f2ded08..536373f4d 100644 --- a/src/database/src/Eloquent/Relations/Concerns/AsPivot.php +++ b/src/database/src/Eloquent/Relations/Concerns/AsPivot.php @@ -31,6 +31,18 @@ trait AsPivot */ protected string $relatedKey; + /** + * The relation-owned predicates that identify this pivot row. + * + * @var null|array{ + * wheres: array>, + * whereIns: array>, + * whereNulls: array>, + * whereBetweens: array> + * } + */ + protected ?array $pivotConstraints = null; + /** * Create a new pivot model instance. */ @@ -89,7 +101,9 @@ protected function setKeysForSelectQuery(Builder $query): Builder $query->where($this->foreignKey, $this->getPivotKeyForQuery($this->foreignKey)); - return $query->where($this->relatedKey, $this->getPivotKeyForQuery($this->relatedKey)); + $query->where($this->relatedKey, $this->getPivotKeyForQuery($this->relatedKey)); + + return $this->applyPivotConstraints($query); } /** @@ -135,10 +149,43 @@ public function delete(): int */ protected function getDeleteQuery(): Builder { - return $this->newQueryWithoutRelationships()->where([ + $query = $this->newQueryWithoutRelationships()->where([ $this->foreignKey => $this->getPivotKeyForQuery($this->foreignKey), $this->relatedKey => $this->getPivotKeyForQuery($this->relatedKey), ]); + + return $this->applyPivotConstraints($query); + } + + /** + * Apply the relation-owned predicates to a pivot identity query. + * + * @param \Hypervel\Database\Eloquent\Builder $query + * @return \Hypervel\Database\Eloquent\Builder + */ + protected function applyPivotConstraints(Builder $query): Builder + { + if ($this->pivotConstraints === null) { + return $query; + } + + return $query->where(function (Builder $query): void { + foreach ($this->pivotConstraints['wheres'] as $arguments) { + $query->where(...$arguments); + } + + foreach ($this->pivotConstraints['whereIns'] as $arguments) { + $query->whereIn(...$arguments); + } + + foreach ($this->pivotConstraints['whereNulls'] as $arguments) { + $query->whereNull(...$arguments); + } + + foreach ($this->pivotConstraints['whereBetweens'] as $arguments) { + $query->whereBetween(...$arguments); + } + }); } /** @@ -211,6 +258,26 @@ public function setPivotKeys(string $foreignKey, string $relatedKey): static return $this; } + /** + * Set the relation-owned predicates for the pivot model. + * + * @param array> $wheres + * @param array> $whereIns + * @param array> $whereNulls + * @param array> $whereBetweens + * @return $this + */ + public function setPivotConstraints( + array $wheres, + array $whereIns, + array $whereNulls, + array $whereBetweens, + ): static { + $this->pivotConstraints = compact('wheres', 'whereIns', 'whereNulls', 'whereBetweens'); + + return $this; + } + /** * Set the related model of the relationship. * diff --git a/src/database/src/Eloquent/Relations/Concerns/InteractsWithPivotTable.php b/src/database/src/Eloquent/Relations/Concerns/InteractsWithPivotTable.php index 178738fd3..67567b415 100644 --- a/src/database/src/Eloquent/Relations/Concerns/InteractsWithPivotTable.php +++ b/src/database/src/Eloquent/Relations/Concerns/InteractsWithPivotTable.php @@ -276,7 +276,7 @@ protected function updateExistingPivotUsingCustomClass(mixed $id, array $attribu { $pivot = $this->getCurrentlyAttachedPivotsForIds($id)->first(); - $updated = $pivot ? $pivot->fill($attributes)->isDirty() : false; + $updated = $pivot ? $pivot->forceFill($attributes)->isDirty() : false; if ($updated) { $pivot->save(); @@ -520,15 +520,7 @@ protected function getCurrentlyAttachedPivotsForIds(mixed $ids = null): BaseColl $this->parseIds($ids) )) ->get() - ->map(function ($record) { - $class = $this->using ?: Pivot::class; - - $pivot = $class::fromRawAttributes($this->parent, (array) $record, $this->getTable(), true); - - return $pivot - ->setPivotKeys($this->foreignPivotKey, $this->relatedPivotKey) - ->setRelatedModel($this->related); - }); + ->map(fn ($record) => $this->newExistingPivot((array) $record)); } /** @@ -538,6 +530,7 @@ public function newPivot(array $attributes = [], bool $exists = false): Model { $attributes = array_merge(array_column($this->pivotValues, 'value', 'column'), $attributes); + /** @var Pivot $pivot */ $pivot = $this->related->newPivot( $this->parent, $attributes, @@ -546,9 +539,20 @@ public function newPivot(array $attributes = [], bool $exists = false): Model $this->using ); - return $pivot - ->setPivotKeys($this->foreignPivotKey, $this->relatedPivotKey) // @phpstan-ignore method.notFound (AsPivot trait provides setPivotKeys) + $pivot = $pivot + ->setPivotKeys($this->foreignPivotKey, $this->relatedPivotKey) ->setRelatedModel($this->related); + + if ($this->hasPivotConstraints()) { + $pivot->setPivotConstraints( + wheres: $this->pivotWheres, + whereIns: $this->pivotWhereIns, + whereNulls: $this->pivotWhereNulls, + whereBetweens: $this->pivotWhereBetweens, + ); + } + + return $pivot; } /** @@ -582,21 +586,40 @@ public function newPivotQuery(): QueryBuilder { $query = $this->newPivotStatement(); - foreach ($this->pivotWheres as $arguments) { - $query->where(...$arguments); - } + if ($this->hasPivotConstraints()) { + $query->where(function (QueryBuilder $query): void { + foreach ($this->pivotWheres as $arguments) { + $query->where(...$arguments); + } - foreach ($this->pivotWhereIns as $arguments) { - $query->whereIn(...$arguments); - } + foreach ($this->pivotWhereIns as $arguments) { + $query->whereIn(...$arguments); + } + + foreach ($this->pivotWhereNulls as $arguments) { + $query->whereNull(...$arguments); + } - foreach ($this->pivotWhereNulls as $arguments) { - $query->whereNull(...$arguments); + foreach ($this->pivotWhereBetweens as $arguments) { + $query->whereBetween(...$arguments); + } + }); } return $query->where($this->getQualifiedForeignPivotKeyName(), $this->parent->{$this->parentKey}); } + /** + * Determine whether the relation has pivot constraints. + */ + protected function hasPivotConstraints(): bool + { + return $this->pivotWheres !== [] + || $this->pivotWhereIns !== [] + || $this->pivotWhereNulls !== [] + || $this->pivotWhereBetweens !== []; + } + /** * Set the columns on the pivot table to retrieve. * @@ -669,7 +692,7 @@ protected function castKey(mixed $key): mixed protected function castAttributes(array $attributes): array { return $this->using - ? $this->newPivot()->fill($attributes)->getAttributes() + ? $this->newPivot()->forceFill($attributes)->getAttributes() : $attributes; } diff --git a/src/database/src/Eloquent/Relations/MorphToMany.php b/src/database/src/Eloquent/Relations/MorphToMany.php index 95a17aa1d..e1aa6927b 100644 --- a/src/database/src/Eloquent/Relations/MorphToMany.php +++ b/src/database/src/Eloquent/Relations/MorphToMany.php @@ -114,21 +114,6 @@ public function getRelationExistenceQuery(Builder $query, Builder $parentQuery, ); } - /** - * Get the pivot models that are currently attached, filtered by related model keys. - * - * @return \Hypervel\Support\Collection - */ - protected function getCurrentlyAttachedPivotsForIds(mixed $ids = null): Collection - { - return parent::getCurrentlyAttachedPivotsForIds($ids)->map(function ($record) { - return $record instanceof MorphPivot - ? $record->setMorphType($this->morphType) - ->setMorphClass($this->morphClass) - : $record; - }); - } - /** * Create a new query builder for the pivot table. */ @@ -157,6 +142,15 @@ public function newPivot(array $attributes = [], bool $exists = false): Model ->setMorphType($this->morphType) ->setMorphClass($this->morphClass); + if ($this->hasPivotConstraints()) { + $pivot->setPivotConstraints( + wheres: $this->pivotWheres, + whereIns: $this->pivotWhereIns, + whereNulls: $this->pivotWhereNulls, + whereBetweens: $this->pivotWhereBetweens, + ); + } + return $pivot; } diff --git a/tests/Database/Eloquent/Relations/BelongsToManyPivotEventsTest.php b/tests/Database/Eloquent/Relations/BelongsToManyPivotEventsTest.php index e3d531a28..cf269e67c 100644 --- a/tests/Database/Eloquent/Relations/BelongsToManyPivotEventsTest.php +++ b/tests/Database/Eloquent/Relations/BelongsToManyPivotEventsTest.php @@ -8,7 +8,10 @@ use Hypervel\Database\Eloquent\Relations\BelongsToMany; use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Foundation\Testing\RefreshDatabase; +use Hypervel\Support\ClassInvoker; +use Hypervel\Support\Facades\DB; use Hypervel\Testbench\TestCase; +use PHPUnit\Framework\Attributes\DataProvider; /** * Tests that pivot model events fire when using a custom pivot class via ->using(). @@ -145,6 +148,88 @@ public function testDetachWithoutCustomPivotDoesNotFireEvents(): void $this->assertEquals([], PivotEventsTestCollaborator::$eventsCalled); } + public function testStockDetachKeepsPivotOrPredicatesInsideTheParentIdentity(): void + { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $otherUser = PivotEventsTestUser::forceCreate(['name' => 'Other User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesWithoutPivot()->attach($role->id, ['is_active' => true]); + $otherUser->rolesWithoutPivot()->attach($role->id, ['is_active' => false]); + + $deleted = $user->rolesWithBooleanScope()->detach($role->id); + + $this->assertSame(1, $deleted); + $this->assertDatabaseMissing('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $otherUser->id, + 'role_id' => $role->id, + ]); + } + + public function testCustomDetachKeepsPivotOrPredicatesInsideTheParentIdentity(): void + { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $otherUser = PivotEventsTestUser::forceCreate(['name' => 'Other User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesWithoutPivot()->attach($role->id, ['is_active' => true]); + $otherUser->rolesWithoutPivot()->attach($role->id, ['is_active' => false]); + + $deleted = $user->rolesWithCustomBooleanScope()->detach($role->id); + + $this->assertSame(1, $deleted); + $this->assertSame(['deleting', 'deleted'], PivotEventsTestCollaborator::$eventsCalled); + $this->assertDatabaseMissing('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $otherUser->id, + 'role_id' => $role->id, + ]); + } + + #[DataProvider('pivotRangeConstraintProvider')] + public function testPivotRangeConstraintsApplyToDestructiveQueries( + string $method, + int $deletedPriority, + int $retainedPriority, + ): void { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $deletedRole = PivotEventsTestRole::forceCreate(['name' => 'Deleted']); + $retainedRole = PivotEventsTestRole::forceCreate(['name' => 'Retained']); + + $user->rolesWithoutPivot()->attach($deletedRole->id, ['priority' => $deletedPriority]); + $user->rolesWithoutPivot()->attach($retainedRole->id, ['priority' => $retainedPriority]); + + $relation = $user->rolesWithoutPivot(); + $relation->{$method}('priority', [1, 10]); + + $this->assertSame(1, $relation->detach()); + $this->assertDatabaseMissing('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $deletedRole->id, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $retainedRole->id, + ]); + } + + public static function pivotRangeConstraintProvider(): array + { + return [ + 'where between' => ['wherePivotBetween', 5, 20], + 'or where between' => ['orWherePivotBetween', 5, 20], + 'where not between' => ['wherePivotNotBetween', 20, 5], + 'or where not between' => ['orWherePivotNotBetween', 20, 5], + ]; + } + // ========================================================================= // Tests for updateExistingPivot() // ========================================================================= @@ -191,6 +276,155 @@ public function testUpdateExistingPivotWithoutCustomPivotDoesNotFireEvents(): vo $this->assertEquals([], PivotEventsTestCollaborator::$eventsCalled); } + public function testCustomPivotWritesBypassMassAssignmentFilteringInStrictMode(): void + { + Model::shouldBeStrict(); + + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesWithPivot()->attach($role->id, ['is_active' => false]); + + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'is_active' => false, + ]); + + PivotEventsTestCollaborator::$eventsCalled = []; + + $this->assertSame(1, $user->rolesWithPivot()->updateExistingPivot( + $role->id, + ['is_active' => true], + )); + $this->assertSame( + ['saving', 'updating', 'updated', 'saved'], + PivotEventsTestCollaborator::$eventsCalled, + ); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'is_active' => true, + ]); + } + + public function testHydratedStockPivotSaveAndDeleteRetainRelationConstraints(): void + { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesInScopeOne()->attach($role->id, ['is_active' => true]); + DB::table('pivot_events_role_user')->insert([ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + 'is_active' => true, + ]); + + $pivot = $user->rolesInScopeOne()->firstOrFail()->pivot; + $pivot->is_active = false; + + $this->assertTrue($pivot->save()); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 1, + 'is_active' => false, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + 'is_active' => true, + ]); + + $this->assertSame(1, $pivot->delete()); + $this->assertDatabaseMissing('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 1, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + ]); + } + + public function testCustomPivotUpdateAndDetachRetainRelationConstraints(): void + { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesWithScopedPivot()->attach($role->id, ['is_active' => false]); + DB::table('pivot_events_role_user')->insert([ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + 'is_active' => false, + ]); + + PivotEventsTestCollaborator::$eventsCalled = []; + + $this->assertSame(1, $user->rolesWithScopedPivot()->updateExistingPivot( + $role->id, + ['is_active' => true], + )); + $this->assertSame( + ['saving', 'updating', 'updated', 'saved'], + PivotEventsTestCollaborator::$eventsCalled, + ); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 1, + 'is_active' => true, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + 'is_active' => false, + ]); + + PivotEventsTestCollaborator::$eventsCalled = []; + + $this->assertSame(1, $user->rolesWithScopedPivot()->detach($role->id)); + $this->assertSame(['deleting', 'deleted'], PivotEventsTestCollaborator::$eventsCalled); + $this->assertDatabaseMissing('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 1, + ]); + $this->assertDatabaseHas('pivot_events_role_user', [ + 'user_id' => $user->id, + 'role_id' => $role->id, + 'scope_id' => 2, + ]); + } + + public function testPrimaryKeyPivotKeepsNativeIdentityWhenAConstraintColumnChanges(): void + { + $user = PivotEventsTestUser::forceCreate(['name' => 'Test User']); + $role = PivotEventsTestRole::forceCreate(['name' => 'Admin']); + + $user->rolesWithKeyedPivot()->attach($role->id, ['is_active' => true]); + + $pivot = $user->rolesWithKeyedPivot()->firstOrFail()->pivot; + $query = $pivot->newQueryWithoutRelationships(); + (new ClassInvoker($pivot))->setKeysForSaveQuery($query); + + $this->assertSame('select * from "pivot_events_role_user_ids" where "id" = ?', $query->toSql()); + + $pivot->scope_id = 2; + + $this->assertTrue($pivot->save()); + $this->assertDatabaseHas('pivot_events_role_user_ids', [ + 'id' => $pivot->id, + 'scope_id' => 2, + ]); + } + // ========================================================================= // Tests for sync() // ========================================================================= @@ -303,6 +537,58 @@ public function rolesWithoutPivot(): BelongsToMany 'role_id' )->withPivot('is_active')->withTimestamps(); } + + /** + * @return BelongsToMany + */ + public function rolesWithBooleanScope(): BelongsToMany + { + return $this->rolesWithoutPivot() + ->wherePivot('is_active', true) + ->orWherePivot('is_active', false); + } + + /** + * @return BelongsToMany + */ + public function rolesWithCustomBooleanScope(): BelongsToMany + { + return $this->rolesWithBooleanScope()->using(PivotEventsTestCollaborator::class); + } + + /** + * @return BelongsToMany + */ + public function rolesInScopeOne(): BelongsToMany + { + return $this->rolesWithoutPivot() + ->withPivot('scope_id') + ->withPivotValue('scope_id', 1); + } + + /** + * @return BelongsToMany + */ + public function rolesWithScopedPivot(): BelongsToMany + { + return $this->rolesInScopeOne()->using(PivotEventsTestCollaborator::class); + } + + /** + * @return BelongsToMany + */ + public function rolesWithKeyedPivot(): BelongsToMany + { + return $this->belongsToMany( + PivotEventsTestRole::class, + 'pivot_events_role_user_ids', + 'user_id', + 'role_id', + )->using(PivotEventsKeyedTestCollaborator::class) + ->withPivot(['id', 'scope_id', 'is_active']) + ->withPivotValue('scope_id', 1) + ->withTimestamps(); + } } class PivotEventsTestRole extends Model @@ -324,6 +610,8 @@ class PivotEventsTestCollaborator extends Pivot 'is_active' => 'boolean', ]; + protected array $guarded = ['is_active']; + public static array $eventsCalled = []; protected static function boot(): void @@ -363,3 +651,12 @@ protected static function boot(): void }); } } + +class PivotEventsKeyedTestCollaborator extends Pivot +{ + protected ?string $table = 'pivot_events_role_user_ids'; + + public bool $incrementing = true; + + public bool $timestamps = true; +} diff --git a/tests/Database/Eloquent/Relations/MorphToManyPivotEventsTest.php b/tests/Database/Eloquent/Relations/MorphToManyPivotEventsTest.php index 384f24772..308f9b603 100644 --- a/tests/Database/Eloquent/Relations/MorphToManyPivotEventsTest.php +++ b/tests/Database/Eloquent/Relations/MorphToManyPivotEventsTest.php @@ -151,18 +151,33 @@ public function testDetachWithoutCustomMorphPivotDoesNotFireEvents(): void // Tests for updateExistingPivot() // ========================================================================= - public function testUpdateExistingPivotFiresSavingAndSavedEventsWithCustomMorphPivot(): void + public function testUpdateExistingPivotUsesGuardedAttributesAndKeepsTheMorphIdentity(): void { - $post = MorphPivotEventsTestPost::forceCreate(['title' => 'Test Post']); + $post = MorphPivotEventsTestPost::forceCreate(['id' => 100, 'title' => 'Test Post']); + $video = MorphPivotEventsTestVideo::forceCreate(['id' => 100, 'title' => 'Test Video']); $tag = MorphPivotEventsTestTag::forceCreate(['name' => 'PHP']); - $post->tagsWithPivot()->attach($tag->id, ['is_primary' => false]); + + $post->tagsWithPivot()->attach($tag->id, ['is_primary' => true]); + $video->tagsWithPivot()->attach($tag->id, ['is_primary' => true]); MorphPivotEventsTestTaggable::$eventsCalled = []; - $updated = $post->tagsWithPivot()->updateExistingPivot($tag->id, ['is_primary' => true]); + $updated = $post->tagsWithPivot()->updateExistingPivot($tag->id, ['is_primary' => false]); $this->assertSame(1, $updated); $this->assertEquals(['saving', 'updating', 'updated', 'saved'], MorphPivotEventsTestTaggable::$eventsCalled); + $this->assertDatabaseHas('pivot_events_taggables', [ + 'taggable_id' => $post->id, + 'taggable_type' => MorphPivotEventsTestPost::class, + 'tag_id' => $tag->id, + 'is_primary' => false, + ]); + $this->assertDatabaseHas('pivot_events_taggables', [ + 'taggable_id' => $video->id, + 'taggable_type' => MorphPivotEventsTestVideo::class, + 'tag_id' => $tag->id, + 'is_primary' => true, + ]); } public function testUpdateExistingPivotDoesNotFireEventsWhenNotDirty(): void @@ -355,6 +370,8 @@ class MorphPivotEventsTestTaggable extends MorphPivot 'is_primary' => 'boolean', ]; + protected array $guarded = ['is_primary']; + public static array $eventsCalled = []; protected static function boot(): void diff --git a/tests/Database/Eloquent/Relations/migrations/2025_01_01_000000_create_pivot_events_test_tables.php b/tests/Database/Eloquent/Relations/migrations/2025_01_01_000000_create_pivot_events_test_tables.php index 7baad7928..d855ddc9f 100644 --- a/tests/Database/Eloquent/Relations/migrations/2025_01_01_000000_create_pivot_events_test_tables.php +++ b/tests/Database/Eloquent/Relations/migrations/2025_01_01_000000_create_pivot_events_test_tables.php @@ -24,10 +24,31 @@ public function up(): void Schema::create('pivot_events_role_user', function (Blueprint $table) { $table->unsignedBigInteger('user_id'); $table->unsignedBigInteger('role_id'); + $table->unsignedInteger('scope_id')->default(1); + $table->unsignedInteger('priority')->default(0); $table->boolean('is_active')->default(true); $table->timestamps(); - $table->primary(['user_id', 'role_id']); + $table->primary(['user_id', 'role_id', 'scope_id']); + + $table->foreign('user_id') + ->references('id') + ->on('pivot_events_users') + ->onDelete('cascade'); + + $table->foreign('role_id') + ->references('id') + ->on('pivot_events_roles') + ->onDelete('cascade'); + }); + + Schema::create('pivot_events_role_user_ids', function (Blueprint $table) { + $table->id(); + $table->unsignedBigInteger('user_id'); + $table->unsignedBigInteger('role_id'); + $table->unsignedInteger('scope_id')->default(1); + $table->boolean('is_active')->default(true); + $table->timestamps(); $table->foreign('user_id') ->references('id') @@ -63,10 +84,12 @@ public function up(): void $table->unsignedBigInteger('tag_id'); $table->unsignedBigInteger('taggable_id'); $table->string('taggable_type'); + $table->unsignedInteger('scope_id')->default(1); + $table->unsignedInteger('priority')->default(0); $table->boolean('is_primary')->default(false); $table->timestamps(); - $table->primary(['tag_id', 'taggable_id', 'taggable_type']); + $table->primary(['tag_id', 'taggable_id', 'taggable_type', 'scope_id']); $table->foreign('tag_id') ->references('id') From 2a995e5d961d55078542c223df554ea1a73355c5 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:45:22 +0000 Subject: [PATCH 02/10] Honor Permission custom pivot and replacement contracts Resolve saved writes through the public roles and permissions relations, preserve their captured partition/team context, and retain stock set-based mutations while allowing configured custom pivots to run native casts and lifecycle hooks. Deferred assignments now retain their selected pivot class without storing request models or adding coroutine state. Complete warm direct and role-derived pivot metadata, constraints, and orientation; return fresh public via-role pivots so callers cannot mutate cached authorization edges. Replacement events publish the complete pre-operation payload after successful writes, while no-op permission syncs preserve warm caches and role sync mutates only changed edges so retained custom pivot rows survive. Add coverage for custom and partitioned pivots, event ordering and failure paths, warm-pivot save/delete isolation, catalog alias protection, no-op cache retention, exact role-diff writes, deferred assignments, and stock bulk-update retention. --- src/permission/src/PermissionRegistrar.php | 64 +++- src/permission/src/Support/Config.php | 5 + .../src/Traits/BuildsPermissionRelations.php | 2 +- .../Traits/EnforcesPermissionPartition.php | 8 + src/permission/src/Traits/HasPermissions.php | 339 ++++++++++++------ src/permission/src/Traits/HasRoles.php | 168 +++++---- tests/Permission/CustomPivotTest.php | 201 +++++++++++ tests/Permission/Events/EventTest.php | 108 +++++- .../Permission/Events/PartitionEventTest.php | 5 +- tests/Permission/Integration/CacheTest.php | 45 +++ tests/Permission/PartitionCustomPivotTest.php | 241 +++++++++++++ tests/Permission/PartitionRelationsTest.php | 136 +++++++ tests/Permission/PartitionTeamsTest.php | 77 ++++ .../Permission/Traits/HasPermissionsTest.php | 68 ++++ .../Traits/TeamHasPermissionsTest.php | 88 +++++ 15 files changed, 1360 insertions(+), 195 deletions(-) create mode 100644 tests/Permission/CustomPivotTest.php create mode 100644 tests/Permission/PartitionCustomPivotTest.php diff --git a/src/permission/src/PermissionRegistrar.php b/src/permission/src/PermissionRegistrar.php index b8e09a19e..c3f0a139f 100644 --- a/src/permission/src/PermissionRegistrar.php +++ b/src/permission/src/PermissionRegistrar.php @@ -16,6 +16,7 @@ use Hypervel\Contracts\Container\Container; use Hypervel\Database\Eloquent\Collection; use Hypervel\Database\Eloquent\Model; +use Hypervel\Database\Eloquent\Relations\BelongsToMany; use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Permission\Contracts\Permission as PermissionContract; use Hypervel\Permission\Contracts\PermissionsTeamResolver; @@ -88,6 +89,11 @@ class PermissionRegistrar protected ?string $cacheStoreName = null; + /** + * @var array, array>> + */ + protected array $assignmentPivotClasses = []; + /** * @var WeakMap> */ @@ -259,6 +265,7 @@ public function initializeCache(): void $cacheStore = $this->config->string('permission.cache.store', 'default'); $this->cacheStoreName = $cacheStore === 'default' ? null : $cacheStore; + $this->assignmentPivotClasses = []; $this->clearAllPermissionRuntimeState(); $this->validateModelClasses(); $this->validateCacheColumnExclusions(); @@ -1316,6 +1323,25 @@ public function getTeamClass(): ?string return $this->teamClass; } + /** + * Get the pivot class selected by a model's public assignment relation. + * + * @return class-string + */ + public function getAssignmentPivotClass(Model $model, string $relation): string + { + $modelClass = $model::class; + + if (isset($this->assignmentPivotClasses[$modelClass][$relation])) { + return $this->assignmentPivotClasses[$modelClass][$relation]; + } + + /** @var BelongsToMany $assignmentRelation */ + $assignmentRelation = $model->{$relation}(); + + return $this->assignmentPivotClasses[$modelClass][$relation] = $assignmentRelation->getPivotClass(); + } + /** * Set the team model class. * @@ -1464,7 +1490,12 @@ private function getHydratedPermissionCollection(array $permissions, Collection return Collection::make(array_map( function (array $item) use ($permissionInstance, $rolesByKey, $context): Model { $permission = (clone $permissionInstance)->setRawAttributes((array) $item['attributes'], true); - $roles = $this->getHydratedPermissionRoleCollection((array) $item['roles'], $permission, $rolesByKey); + $roles = $this->getHydratedPermissionRoleCollection( + (array) $item['roles'], + $permission, + $rolesByKey, + $context, + ); $permission->setRelation('roles', $roles); $this->markLoadedRelation($permission, 'roles', $roles, $context); @@ -1548,9 +1579,13 @@ private function indexModelOrderByKey(Collection $models): array * * @param array> $roles */ - private function getHydratedPermissionRoleCollection(array $roles, Model $permission, Collection $roleCatalog): Collection - { - return Collection::make(array_values(array_filter(array_map(function (array $item) use ($permission, $roleCatalog): ?Model { + private function getHydratedPermissionRoleCollection( + array $roles, + Model $permission, + Collection $roleCatalog, + PermissionRelationContext $context, + ): Collection { + return Collection::make(array_values(array_filter(array_map(function (array $item) use ($permission, $roleCatalog, $context): ?Model { $roleKey = $item['pivot'][$this->pivotRole] ?? null; $role = $roleKey === null ? null : $roleCatalog->get((string) $roleKey); @@ -1559,12 +1594,29 @@ private function getHydratedPermissionRoleCollection(array $roles, Model $permis } $role = clone $role; - $role->setRelation('pivot', Pivot::fromRawAttributes( + $pivot = Pivot::fromRawAttributes( $permission, (array) $item['pivot'], $this->config->string('permission.table_names.role_has_permissions'), true, - )); + ); + $pivot->setPivotKeys($this->pivotPermission, $this->pivotRole) + ->setRelatedModel($role); + + if ($context->partition) { + $pivot->setPivotConstraints( + wheres: [[ + $context->partition->column, + '=', + $context->partition->value, + ]], + whereIns: [], + whereNulls: [], + whereBetweens: [], + ); + } + + $role->setRelation('pivot', $pivot); return $role; }, $roles)))); diff --git a/src/permission/src/Support/Config.php b/src/permission/src/Support/Config.php index ca0e20541..dbb7e83c1 100644 --- a/src/permission/src/Support/Config.php +++ b/src/permission/src/Support/Config.php @@ -15,6 +15,11 @@ class Config { + // Eloquent derives the morph-type column from the relation name. + public const MORPH_NAME = 'model'; + + public const MORPH_TYPE = self::MORPH_NAME . '_type'; + /** * Get the config repository. */ diff --git a/src/permission/src/Traits/BuildsPermissionRelations.php b/src/permission/src/Traits/BuildsPermissionRelations.php index 7b473091c..85ae5ba47 100644 --- a/src/permission/src/Traits/BuildsPermissionRelations.php +++ b/src/permission/src/Traits/BuildsPermissionRelations.php @@ -95,7 +95,7 @@ protected function permissionMorphToMany( $relation = new PartitionedMorphToMany( $query, $this, - 'model', + Config::MORPH_NAME, $table, $foreignPivotKey, $relatedPivotKey, diff --git a/src/permission/src/Traits/EnforcesPermissionPartition.php b/src/permission/src/Traits/EnforcesPermissionPartition.php index 8c3fe0130..a0a7313ad 100644 --- a/src/permission/src/Traits/EnforcesPermissionPartition.php +++ b/src/permission/src/Traits/EnforcesPermissionPartition.php @@ -28,6 +28,14 @@ protected function initializePermissionPartitionRelation( $this->permissionRelationContext = $context; } + /** + * Get the relation's captured permission context. + */ + public function getPermissionRelationContext(): PermissionRelationContext + { + return $this->permissionRelationContext; + } + /** * Format an attachment record without allowing partition overrides. */ diff --git a/src/permission/src/Traits/HasPermissions.php b/src/permission/src/Traits/HasPermissions.php index 1ab140d30..06d9ac3a5 100644 --- a/src/permission/src/Traits/HasPermissions.php +++ b/src/permission/src/Traits/HasPermissions.php @@ -10,6 +10,7 @@ use Hypervel\Database\Eloquent\MissingAttributeException; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\BelongsToMany; +use Hypervel\Database\Eloquent\Relations\MorphPivot; use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Permission\Contracts\Permission; use Hypervel\Permission\Contracts\Role; @@ -23,6 +24,8 @@ use Hypervel\Permission\Exceptions\WildcardPermissionNotImplementsContract; use Hypervel\Permission\Guard; use Hypervel\Permission\PermissionRegistrar; +use Hypervel\Permission\Relations\PartitionedBelongsToMany; +use Hypervel\Permission\Relations\PartitionedMorphToMany; use Hypervel\Permission\Support\Config; use Hypervel\Permission\Support\PermissionPartition; use Hypervel\Permission\Support\PermissionRelationContext; @@ -42,7 +45,7 @@ trait HasPermissions private ?string $wildcardClass = null; /** - * @var array, pivot: array, context: PermissionRelationContext}> + * @var array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> */ private array $queuedPermissionAssignments = []; @@ -136,7 +139,7 @@ protected static function deleteSubjectAssignments(Model $model, string $table): if ($partitionColumn === null && ! $registrar->teams) { $connection->table($table) ->where($morphKey, $modelKey) - ->where('model_type', $morphType) + ->where(Config::MORPH_TYPE, $morphType) ->delete(); return null; @@ -164,7 +167,7 @@ protected static function deleteSubjectAssignments(Model $model, string $table): $scopes = $connection->table($table) ->select($columns) ->where($morphKey, $modelKey) - ->where('model_type', $morphType) + ->where(Config::MORPH_TYPE, $morphType) ->distinct() ->get(); $contexts = []; @@ -205,7 +208,7 @@ protected static function deleteSubjectAssignments(Model $model, string $table): $connection->table($table) ->where($morphKey, $modelKey) - ->where('model_type', $morphType) + ->where(Config::MORPH_TYPE, $morphType) ->delete(); return $contexts; @@ -331,6 +334,26 @@ protected function permissionAssignmentRelation( )->withPivot('is_denied'); } + /** + * Get the immutable context captured by a permission assignment relation. + */ + protected function permissionRelationContext(BelongsToMany $relation): PermissionRelationContext + { + /** @var PartitionedBelongsToMany|PartitionedMorphToMany $relation */ + return $relation->getPermissionRelationContext(); + } + + /** + * Forget an assignment relation loaded for an earlier context. + */ + protected function forgetStalePermissionRelation(PermissionRegistrar $registrar, string $relation): void + { + if ($this->relationLoaded($relation) + && ! $registrar->loadedRelationIsCurrent($this, $relation)) { + $this->unsetRelation($relation); + } + } + /** * Get cached direct permission assignments for this model. */ @@ -340,10 +363,7 @@ protected function getCachedDirectPermissions(): Collection $registrar = $this->permissionRegistrar(); $context = $this->permissionAssignmentContext($registrar); - if ($model->relationLoaded('permissions') - && ! $registrar->loadedRelationIsCurrent($model, 'permissions')) { - $model->unsetRelation('permissions'); - } + $this->forgetStalePermissionRelation($registrar, 'permissions'); if ($this instanceof Role || $this instanceof Permission || ! $model->exists || $this->relationLoaded('permissions')) { return $this->relationCollection($this, 'permissions'); @@ -379,7 +399,7 @@ protected function getCachedDirectPermissions(): Collection $pivot = [ $registrar->pivotPermission => $permission->getKey(), Config::morphKey() => $model->getKey(), - 'model_type' => $model->getMorphClass(), + Config::MORPH_TYPE => $model->getMorphClass(), 'is_denied' => (bool) $assignment['is_denied'], ]; @@ -392,12 +412,47 @@ protected function getCachedDirectPermissions(): Collection } $permission = clone $permission; - $permission->setRelation('pivot', Pivot::fromRawAttributes( + $morphPivot = MorphPivot::fromRawAttributes( $model, $pivot, Config::modelHasPermissionsTable(), true, - )); + ); + $morphPivot + ->setPivotKeys(Config::morphKey(), $registrar->pivotPermission) + ->setRelatedModel($permission) + ->setMorphType(Config::MORPH_TYPE) + ->setMorphClass($model->getMorphClass()); + + $pivotWheres = []; + $pivotWhereNulls = []; + + if ($context->partition) { + $pivotWheres[] = [ + $context->partition->column, + '=', + $context->partition->value, + ]; + } + + if ($context->teamScoped) { + if ($context->team === null) { + $pivotWhereNulls[] = [$registrar->teamsKey]; + } else { + $pivotWheres[] = [$registrar->teamsKey, '=', $context->team]; + } + } + + if ($pivotWheres !== [] || $pivotWhereNulls !== []) { + $morphPivot->setPivotConstraints( + wheres: $pivotWheres, + whereIns: [], + whereNulls: $pivotWhereNulls, + whereBetweens: [], + ); + } + + $permission->setRelation('pivot', $morphPivot); return $permission; }) @@ -405,6 +460,24 @@ protected function getCachedDirectPermissions(): Collection ->values(); } + /** + * Get direct permissions for a model-returning public API. + */ + protected function directPermissionsForModelResult(): Collection + { + if (! $this->exists) { + return $this->getCachedDirectPermissions(); + } + + $registrar = $this->permissionRegistrar(); + + if ($registrar->getAssignmentPivotClass($this, 'permissions') === Pivot::class) { + return $this->getCachedDirectPermissions(); + } + + return $this->relationCollection($this, 'permissions'); + } + /** * Return allowed direct permissions. */ @@ -762,7 +835,7 @@ public function getPermissionsViaRoles(): Collection */ public function getAllPermissions(): Collection { - $directPermissions = $this->getCachedDirectPermissions(); + $directPermissions = $this->directPermissionsForModelResult(); $viaRolePermissions = $this instanceof Permission ? collect() : $this->getPermissionsViaRolesWithPivots(); @@ -860,10 +933,13 @@ private function attachPermissions(array $permissions, bool $isDenied): static return $this; } - $pivot = $this->permissionAssignmentPivot($isDenied, $context); - if (! $model->exists) { - $this->queuePermissionAssignments($permissions, $pivot, $context); + $this->queuePermissionAssignments( + $permissions, + $this->permissionAssignmentPivot($isDenied, $context), + $context, + $registrar->getAssignmentPivotClass($this, 'permissions'), + ); $this->dispatchPermissionAttachedEvent($permissions); return $this; @@ -871,9 +947,13 @@ private function attachPermissions(array $permissions, bool $isDenied): static $this->requireModelKey($model); + $relation = $this->permissions(); + $context = $this->permissionRelationContext($relation); + $changes = $this->synchronizePermissionAssignments( $isDenied ? [] : $permissions, $isDenied ? $permissions : [], + $relation, $context, false, ); @@ -989,6 +1069,7 @@ protected function readCurrentAssignmentPivots( private function synchronizePermissionAssignments( array $allowed, array $denied, + BelongsToMany $relation, PermissionRelationContext $context, bool $detaching, ): array { @@ -1008,8 +1089,7 @@ private function synchronizePermissionAssignments( ]; } - return $this->getConnection()->transaction(function () use ($context, $desired, $detaching): array { - $relation = $this->permissionAssignmentRelation($context); + return $this->getConnection()->transaction(function () use ($context, $desired, $detaching, $relation): array { $relatedPivotKey = $relation->getRelatedPivotKeyName(); $pivots = $this->readCurrentAssignmentPivots( $relation, @@ -1090,18 +1170,29 @@ private function synchronizePermissionAssignments( ); } - // Permission assignment pivots have no timestamps or custom pivot class, - // so effect changes can be written in two scoped batches. if ($updateAllowed !== []) { - $relation->newPivotQuery() - ->whereIn($relatedPivotKey, $updateAllowed) - ->update(['is_denied' => false]); + // Stock pivots update in bulk; custom pivots retain their casts and model events. + if ($relation->getPivotClass() === Pivot::class) { + $relation->newPivotQuery() + ->whereIn($relatedPivotKey, $updateAllowed) + ->update(['is_denied' => false]); + } else { + foreach ($updateAllowed as $id) { + $relation->updateExistingPivot($id, ['is_denied' => false], false); + } + } } if ($updateDenied !== []) { - $relation->newPivotQuery() - ->whereIn($relatedPivotKey, $updateDenied) - ->update(['is_denied' => true]); + if ($relation->getPivotClass() === Pivot::class) { + $relation->newPivotQuery() + ->whereIn($relatedPivotKey, $updateDenied) + ->update(['is_denied' => true]); + } else { + foreach ($updateDenied as $id) { + $relation->updateExistingPivot($id, ['is_denied' => true], false); + } + } } if ($changes['attached'] !== [] @@ -1119,11 +1210,13 @@ private function synchronizePermissionAssignments( * * @param array $permissions * @param array $pivot + * @param class-string $pivotClass */ protected function attachPermissionAssignments( array $permissions, array $pivot, - ?PermissionRelationContext $context = null, + ?PermissionRelationContext $context, + string $pivotClass, ): void { if ($permissions === []) { return; @@ -1131,6 +1224,10 @@ protected function attachPermissionAssignments( $relation = $this->permissionAssignmentRelation($context); + if ($pivotClass !== Pivot::class) { + $relation->using($pivotClass); + } + $relation->attach($permissions, $pivot, false); $relation->touchIfTouching(); } @@ -1140,54 +1237,32 @@ protected function attachPermissionAssignments( * * @param array $permissions * @param array $pivot + * @param class-string $pivotClass */ protected function queuePermissionAssignments( array $permissions, array $pivot, PermissionRelationContext $context, + string $pivotClass, ): void { $this->queuedPermissionAssignments[] = [ 'permissions' => $permissions, 'pivot' => $pivot, 'context' => $context, + 'pivotClass' => $pivotClass, ]; } /** * Replace queued permission assignments for the given scope. * - * @param array, pivot: array, context: PermissionRelationContext}> $assignments + * @param array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> $assignments */ private function replaceQueuedPermissionAssignments( array $assignments, PermissionRelationContext $context, - ): bool { + ): void { $scopeKey = $context->identity(); - $current = []; - - foreach ($this->queuedPermissionAssignments as $assignment) { - if ($assignment['context']->identity() === $scopeKey) { - $current[] = [ - 'permissions' => $assignment['permissions'], - 'pivot' => $assignment['pivot'], - ]; - } - } - - $replacement = []; - - foreach ($assignments as $assignment) { - if ($assignment['permissions'] !== []) { - $replacement[] = [ - 'permissions' => $assignment['permissions'], - 'pivot' => $assignment['pivot'], - ]; - } - } - - if ($current === $replacement) { - return false; - } $this->queuedPermissionAssignments = array_values(array_filter( $this->queuedPermissionAssignments, @@ -1203,10 +1278,9 @@ private function replaceQueuedPermissionAssignments( $assignment['permissions'], $assignment['pivot'], $assignment['context'], + $assignment['pivotClass'], ); } - - return true; } /** @@ -1217,9 +1291,8 @@ private function replaceQueuedPermissionAssignments( protected function removeQueuedPermissionAssignments( array $permissions, PermissionRelationContext $context, - ): bool { + ): void { $identity = $context->identity(); - $changed = false; $assignments = []; foreach ($this->queuedPermissionAssignments as $assignment) { @@ -1238,19 +1311,13 @@ protected function removeQueuedPermissionAssignments( continue; } - $changed = true; - if ($remainingPermissions !== []) { $assignment['permissions'] = $remainingPermissions; $assignments[] = $assignment; } } - if ($changed) { - $this->queuedPermissionAssignments = $assignments; - } - - return $changed; + $this->queuedPermissionAssignments = $assignments; } /** @@ -1276,7 +1343,7 @@ protected function flushQueuedPermissionAssignments(): void /** * Attach collapsed queued permission assignment batches. * - * @param array, pivot: array, context: PermissionRelationContext}> $assignments + * @param array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> $assignments */ protected function attachQueuedPermissionAssignmentBatches(array $assignments): void { @@ -1285,6 +1352,7 @@ protected function attachQueuedPermissionAssignmentBatches(array $assignments): $assignment['permissions'], $assignment['pivot'], $assignment['context'], + $assignment['pivotClass'], ); } } @@ -1300,7 +1368,7 @@ protected function clearQueuedPermissionAssignments(): void /** * Invalidate the captured contexts of committed permission assignments. * - * @param array, pivot: array, context: PermissionRelationContext}> $assignments + * @param array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> $assignments */ protected function invalidateQueuedPermissionAssignmentContexts(array $assignments): void { @@ -1327,7 +1395,7 @@ protected function invalidateQueuedPermissionAssignmentContexts(array $assignmen /** * Collapse queued permission assignments to their final edge state. * - * @return array, pivot: array, context: PermissionRelationContext}> + * @return array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> */ protected function collapseQueuedPermissionAssignments(): array { @@ -1343,6 +1411,7 @@ protected function collapseQueuedPermissionAssignments(): array 'permission' => $permission, 'pivot' => $assignment['pivot'], 'context' => $assignment['context'], + 'pivotClass' => $assignment['pivotClass'], ]; } } @@ -1353,12 +1422,14 @@ protected function collapseQueuedPermissionAssignments(): array foreach ($collapsed as $assignment) { $pivot = $assignment['pivot']; $batchKey = $assignment['context']->identity() . ':' - . ((bool) $pivot['is_denied'] ? 'denied' : 'allowed'); + . ((bool) $pivot['is_denied'] ? 'denied' : 'allowed') . ':' + . PermissionPartition::encodeCacheSegment($assignment['pivotClass']); $batches[$batchKey] ??= [ 'permissions' => [], 'pivot' => $pivot, 'context' => $assignment['context'], + 'pivotClass' => $assignment['pivotClass'], ]; $batches[$batchKey]['permissions'][] = $assignment['permission']; } @@ -1409,15 +1480,15 @@ public function syncPermissions(...$permissions): static $registrar = $this->permissionRegistrar(); $context = $this->permissionAssignmentContext($registrar); $permissions = $this->collectPermissions($permissions, $context->partition); - $pivot = $this->permissionAssignmentPivot(false, $context); if (! $this->exists) { $this->replaceQueuedPermissionAssignments( [ [ 'permissions' => $permissions, - 'pivot' => $pivot, + 'pivot' => $this->permissionAssignmentPivot(false, $context), 'context' => $context, + 'pivotClass' => $registrar->getAssignmentPivotClass($this, 'permissions'), ], ], $context, @@ -1429,31 +1500,38 @@ public function syncPermissions(...$permissions): static $this->requireModelKey($this); + $relation = $this->permissions(); + $context = $this->permissionRelationContext($relation); + $detachedPermissions = $this->permissionDetachedEventIsListenedFor() + ? $relation->get() + : new Collection; + $changes = $this->synchronizePermissionAssignments( $permissions, [], + $relation, $context, true, ); - if ($changes['attached'] === [] - && $changes['detached'] === [] - && $changes['updated'] === []) { - $this->dispatchPermissionAttachedEvent($permissions); + if ($changes['attached'] !== [] + || $changes['detached'] !== [] + || $changes['updated'] !== []) { + $this->unsetRelation('permissions'); - return $this; + if ($this instanceof Role) { + $registrar->forgetCachedPermissionsFor($context->partition); + } else { + $registrar->forgetModelPermissionCacheFor( + $this, + $context->partition, + $context->team, + ); + } } - $this->unsetRelation('permissions'); - - if ($this instanceof Role) { - $registrar->forgetCachedPermissionsFor($context->partition); - } else { - $registrar->forgetModelPermissionCacheFor( - $this, - $context->partition, - $context->team, - ); + if ($detachedPermissions->isNotEmpty()) { + $this->dispatchPermissionDetachedEvent($detachedPermissions); } $this->dispatchPermissionAttachedEvent($permissions); @@ -1483,21 +1561,23 @@ public function syncPermissionEffects(array|Collection $allowed = [], array|Coll fn (int|string $allowedId): bool => ! in_array($allowedId, $deniedIds, true), )); $permissions = array_merge($allowedIds, $deniedIds); - $allowedPivot = $this->permissionAssignmentPivot(false, $context); - $deniedPivot = $this->permissionAssignmentPivot(true, $context); if (! $this->exists) { + $pivotClass = $registrar->getAssignmentPivotClass($this, 'permissions'); + $this->replaceQueuedPermissionAssignments( [ [ 'permissions' => $allowedIds, - 'pivot' => $allowedPivot, + 'pivot' => $this->permissionAssignmentPivot(false, $context), 'context' => $context, + 'pivotClass' => $pivotClass, ], [ 'permissions' => $deniedIds, - 'pivot' => $deniedPivot, + 'pivot' => $this->permissionAssignmentPivot(true, $context), 'context' => $context, + 'pivotClass' => $pivotClass, ], ], $context, @@ -1509,31 +1589,38 @@ public function syncPermissionEffects(array|Collection $allowed = [], array|Coll $this->requireModelKey($this); + $relation = $this->permissions(); + $context = $this->permissionRelationContext($relation); + $detachedPermissions = $this->permissionDetachedEventIsListenedFor() + ? $relation->get() + : new Collection; + $changes = $this->synchronizePermissionAssignments( $allowedIds, $deniedIds, + $relation, $context, true, ); - if ($changes['attached'] === [] - && $changes['detached'] === [] - && $changes['updated'] === []) { - $this->dispatchPermissionAttachedEvent($permissions); + if ($changes['attached'] !== [] + || $changes['detached'] !== [] + || $changes['updated'] !== []) { + $this->unsetRelation('permissions'); - return $changes; + if ($this instanceof Role) { + $registrar->forgetCachedPermissionsFor($context->partition); + } else { + $registrar->forgetModelPermissionCacheFor( + $this, + $context->partition, + $context->team, + ); + } } - $this->unsetRelation('permissions'); - - if ($this instanceof Role) { - $registrar->forgetCachedPermissionsFor($context->partition); - } else { - $registrar->forgetModelPermissionCacheFor( - $this, - $context->partition, - $context->team, - ); + if ($detachedPermissions->isNotEmpty()) { + $this->dispatchPermissionDetachedEvent($detachedPermissions); } $this->dispatchPermissionAttachedEvent($permissions); @@ -1568,7 +1655,8 @@ public function revokePermissionTo($permission): static $this->requireModelKey($this); - $relation = $this->permissionAssignmentRelation($context); + $relation = $this->permissions(); + $context = $this->permissionRelationContext($relation); $detached = $relation->detach($storedPermission); if ($detached > 0) { @@ -1701,13 +1789,20 @@ protected function loadPermissionsViaRolesWithPivots(): Collection } $roleIds = array_flip($roles->map(fn (Model $role): string => (string) $role->getKey())->all()); + $registrar = $this->permissionRegistrar(); + $partition = $registrar->resolvePartition(); - return $this->permissionRegistrar() + return $registrar ->getPermissions([], false, $this->getPermissionClass()) ->flatMap( fn (Model $permission): Collection => $this->relationCollection($permission, 'roles') ->filter(fn (Model $role): bool => isset($roleIds[(string) $role->getKey()])) - ->map(fn (Model $role): Model => $this->permissionWithRolePivot($permission, $role)) + ->map(fn (Model $role): Model => $this->permissionWithRolePivot( + $permission, + $role, + $registrar, + $partition, + )) ); } @@ -1772,10 +1867,34 @@ protected function permissionComparisonKey(Model $permission): string /** * Clone a permission with the matching role-permission pivot. */ - protected function permissionWithRolePivot(Model $permission, Model $role): Model - { + protected function permissionWithRolePivot( + Model $permission, + Model $role, + PermissionRegistrar $registrar, + ?PermissionPartition $partition, + ): Model { $permission = clone $permission; - $permission->setRelation('pivot', $role->getRelation('pivot')); + /** @var Pivot $cachedPivot */ + $cachedPivot = $role->getRelation('pivot'); + $pivot = Pivot::fromRawAttributes( + $role, + $cachedPivot->getAttributes(), + Config::roleHasPermissionsTable(), + true, + ); + $pivot->setPivotKeys($registrar->pivotRole, $registrar->pivotPermission) + ->setRelatedModel($permission); + + if ($partition) { + $pivot->setPivotConstraints( + wheres: [[$partition->column, '=', $partition->value]], + whereIns: [], + whereNulls: [], + whereBetweens: [], + ); + } + + $permission->setRelation('pivot', $pivot); return $permission; } diff --git a/src/permission/src/Traits/HasRoles.php b/src/permission/src/Traits/HasRoles.php index 9ad273e7c..29266367a 100644 --- a/src/permission/src/Traits/HasRoles.php +++ b/src/permission/src/Traits/HasRoles.php @@ -8,6 +8,7 @@ use Hypervel\Database\Eloquent\Builder; use Hypervel\Database\Eloquent\Model; use Hypervel\Database\Eloquent\Relations\BelongsToMany; +use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Permission\Contracts\Permission; use Hypervel\Permission\Contracts\Role; use Hypervel\Permission\Events\RoleAttachedEvent; @@ -31,7 +32,7 @@ trait HasRoles private ?string $roleClass = null; /** - * @var array, pivot: array, context: PermissionRelationContext}> + * @var array, pivot: array, context: PermissionRelationContext, pivotClass: class-string}> */ private array $queuedRoleAssignments = []; @@ -144,10 +145,7 @@ protected function getCachedRoles(): Collection $registrar = $this->permissionRegistrar(); $context = $this->roleAssignmentContext($registrar); - if ($model->relationLoaded('roles') - && ! $registrar->loadedRelationIsCurrent($model, 'roles')) { - $model->unsetRelation('roles'); - } + $this->forgetStalePermissionRelation($registrar, 'roles'); if ($this instanceof Permission || ! $model->exists || $this->relationLoaded('roles')) { return $this->relationCollection($this, 'roles'); @@ -287,7 +285,7 @@ public function scopeTeam(Builder $query, $teams, bool $without = false): Builde function ($subQuery) use ($pivotTable, $morphKey, $query, $teamsKey, $teamIds, $partition) { $subQuery->from($pivotTable) ->whereColumn($morphKey, $query->getModel()->getQualifiedKeyName()) - ->where('model_type', $query->getModel()->getMorphClass()) + ->where(Config::MORPH_TYPE, $query->getModel()->getMorphClass()) ->whereIn($teamsKey, $teamIds); if ($partition) { @@ -355,10 +353,13 @@ public function assignRole(...$roles): static return $this; } - $pivot = $this->roleAssignmentPivot($context); - if (! $this->exists) { - $this->queueRoleAssignments($roles, $pivot, $context); + $this->queueRoleAssignments( + $roles, + $this->roleAssignmentPivot($context), + $context, + $registrar->getAssignmentPivotClass($this, 'roles'), + ); $this->dispatchRoleAttachedEvent($roles); return $this; @@ -366,8 +367,15 @@ public function assignRole(...$roles): static $this->requireModelKey($this); - $currentRoles = $this->getCachedRoles() - ->map(fn (Model $role): int|string => $role->getKey()) + $relation = $this->roles(); + $context = $this->permissionRelationContext($relation); + $relatedPivotKey = $relation->getRelatedPivotKeyName(); + + $currentRoles = $this->readCurrentAssignmentPivots($relation, [$relatedPivotKey], $roles) + ->map(fn (object $pivot): int|string => $this->normalizeRelatedPivotId( + $relation, + $pivot->{$relatedPivotKey}, + )) ->all(); $attachedRoles = array_values(array_filter( $roles, @@ -380,7 +388,7 @@ public function assignRole(...$roles): static return $this; } - $this->roleAssignmentRelation($context)->attach($attachedRoles, $pivot); + $relation->attach($attachedRoles, $this->roleAssignmentPivot($context)); $this->unsetRelation('roles'); if ($this instanceof Permission) { @@ -403,13 +411,15 @@ public function assignRole(...$roles): static * * @param array $roles * @param array $pivot + * @param class-string $pivotClass */ protected function queueRoleAssignments( array $roles, array $pivot, PermissionRelationContext $context, + string $pivotClass, ): void { - $identity = $context->identity(); + $identity = $context->identity() . ':' . PermissionPartition::encodeCacheSegment($pivotClass); $queuedRoles = $this->queuedRoleAssignments[$identity]['roles'] ?? []; foreach ($roles as $role) { @@ -422,6 +432,7 @@ protected function queueRoleAssignments( 'roles' => $queuedRoles, 'pivot' => $pivot, 'context' => $context, + 'pivotClass' => $pivotClass, ]; } @@ -430,55 +441,46 @@ protected function queueRoleAssignments( * * @param array $roles * @param array $pivot + * @param class-string $pivotClass */ protected function replaceQueuedRoleAssignments( array $roles, array $pivot, PermissionRelationContext $context, - ): bool { - $identity = $context->identity(); + string $pivotClass, + ): void { + $identity = $context->identity() . ':' . PermissionPartition::encodeCacheSegment($pivotClass); if ($roles === []) { - if (! isset($this->queuedRoleAssignments[$identity])) { - return false; - } - unset($this->queuedRoleAssignments[$identity]); - return true; - } - - $current = $this->queuedRoleAssignments[$identity] ?? null; - - if ($current !== null - && $current['roles'] === $roles - && $current['pivot'] === $pivot) { - return false; + return; } $this->queuedRoleAssignments[$identity] = [ 'roles' => $roles, 'pivot' => $pivot, 'context' => $context, + 'pivotClass' => $pivotClass, ]; - - return true; } /** * Remove role assignments queued for a captured context. * * @param array $roles + * @param class-string $pivotClass */ protected function removeQueuedRoleAssignments( array $roles, PermissionRelationContext $context, - ): bool { - $identity = $context->identity(); + string $pivotClass, + ): void { + $identity = $context->identity() . ':' . PermissionPartition::encodeCacheSegment($pivotClass); $assignment = $this->queuedRoleAssignments[$identity] ?? null; if ($assignment === null) { - return false; + return; } $remainingRoles = array_values(array_filter( @@ -487,19 +489,17 @@ protected function removeQueuedRoleAssignments( )); if ($remainingRoles === $assignment['roles']) { - return false; + return; } if ($remainingRoles === []) { unset($this->queuedRoleAssignments[$identity]); - return true; + return; } $assignment['roles'] = $remainingRoles; $this->queuedRoleAssignments[$identity] = $assignment; - - return true; } /** @@ -516,8 +516,13 @@ protected function flushQueuedPermissionAssignments(): void $this->getConnection()->transaction(function () use ($roleAssignments, $permissionAssignments): void { foreach ($roleAssignments as $assignment) { - $this->roleAssignmentRelation($assignment['context']) - ->attach($assignment['roles'], $assignment['pivot']); + $relation = $this->roleAssignmentRelation($assignment['context']); + + if ($assignment['pivotClass'] !== Pivot::class) { + $relation->using($assignment['pivotClass']); + } + + $relation->attach($assignment['roles'], $assignment['pivot']); } $this->attachQueuedPermissionAssignmentBatches($permissionAssignments); @@ -597,7 +602,11 @@ public function removeRole(...$role): static } if (! $this->exists) { - $this->removeQueuedRoleAssignments($roles, $context); + $this->removeQueuedRoleAssignments( + $roles, + $context, + $registrar->getAssignmentPivotClass($this, 'roles'), + ); $this->dispatchRoleDetachedEvent($roles); return $this; @@ -605,7 +614,8 @@ public function removeRole(...$role): static $this->requireModelKey($this); - $relation = $this->roleAssignmentRelation($context); + $relation = $this->roles(); + $context = $this->permissionRelationContext($relation); $detached = $relation->detach($roles); if ($detached > 0) { @@ -663,10 +673,13 @@ public function syncRoles(...$roles): static $roles = $this->collectRoles($roles, $context->partition); if (! $this->exists) { + $pivotClass = $registrar->getAssignmentPivotClass($this, 'roles'); + $this->replaceQueuedRoleAssignments( $roles, $this->roleAssignmentPivot($context), $context, + $pivotClass, ); $this->dispatchRoleAttachedEvent($roles); @@ -675,46 +688,49 @@ public function syncRoles(...$roles): static $this->requireModelKey($this); - $relation = $this->roleAssignmentRelation($context); - $detachedRoles = []; - - if ($this->roleDetachedEventIsListenedFor()) { - $relatedPivotKey = $relation->getRelatedPivotKeyName(); + $relation = $this->roles(); + $context = $this->permissionRelationContext($relation); + $relatedPivotKey = $relation->getRelatedPivotKeyName(); + $currentRoles = $this->readCurrentAssignmentPivots($relation, [$relatedPivotKey]) + ->map(fn (object $pivot): int|string => $this->normalizeRelatedPivotId( + $relation, + $pivot->{$relatedPivotKey}, + )) + ->all(); + $currentRolesByIdentity = $this->indexAssignmentIds($currentRoles); + $desiredRolesByIdentity = $this->indexAssignmentIds($roles); + $rolesToDetach = array_values(array_diff_key($currentRolesByIdentity, $desiredRolesByIdentity)); + $rolesToAttach = array_values(array_diff_key($desiredRolesByIdentity, $currentRolesByIdentity)); + $detachedEventRoles = $this->roleDetachedEventIsListenedFor() ? $currentRoles : []; + + if ($rolesToDetach !== [] || $rolesToAttach !== []) { + $this->getConnection()->transaction(function () use ($context, $relation, $rolesToAttach, $rolesToDetach): void { + if ($rolesToDetach !== []) { + $relation->detach($rolesToDetach, false); + } - foreach ($this->readCurrentAssignmentPivots($relation, [$relatedPivotKey]) as $pivot) { - $detachedRoles[] = $this->normalizeRelatedPivotId( - $relation, - $pivot->{$relatedPivotKey}, - ); - } - } + if ($rolesToAttach !== []) { + $relation->attach($rolesToAttach, $this->roleAssignmentPivot($context), false); + } - $pivot = $this->roleAssignmentPivot($context); + $relation->touchIfTouching(); + }); - $this->getConnection()->transaction(function () use ($pivot, $relation, $roles): void { - $relation->detach(null, false); + $this->unsetRelation('roles'); - if ($roles !== []) { - $relation->attach($roles, $pivot, false); + if ($this instanceof Permission) { + $registrar->forgetCachedPermissionsFor($context->partition); + } else { + $registrar->forgetModelRoleCacheFor( + $this, + $context->partition, + $context->team, + ); } - - $relation->touchIfTouching(); - }); - - $this->unsetRelation('roles'); - - if ($this instanceof Permission) { - $registrar->forgetCachedPermissionsFor($context->partition); - } else { - $registrar->forgetModelRoleCacheFor( - $this, - $context->partition, - $context->team, - ); } - if ($detachedRoles !== []) { - $this->dispatchRoleDetachedEvent($detachedRoles); + if ($detachedEventRoles !== []) { + $this->dispatchRoleDetachedEvent($detachedEventRoles); } $this->dispatchRoleAttachedEvent($roles); @@ -878,7 +894,9 @@ public function hasExactRoles($roles, ?string $guard = null): bool */ public function getDirectPermissions(): Collection { - return $this->allowedDirectPermissions(); + return $this->directPermissionsForModelResult() + ->reject(fn (Model $permission): bool => $this->pivotIsDenied($permission)) + ->values(); } /** diff --git a/tests/Permission/CustomPivotTest.php b/tests/Permission/CustomPivotTest.php new file mode 100644 index 000000000..cb7fe6e71 --- /dev/null +++ b/tests/Permission/CustomPivotTest.php @@ -0,0 +1,201 @@ + 'custom@example.com']); + + $user->givePermissionTo('edit-articles'); + + $this->assertSame(['created'], CustomPermissionPivotTestPermissionPivot::$events); + $this->assertDatabaseHas('model_has_permissions', [ + 'model_test_id' => $user->getKey(), + 'permission_test_id' => $this->testUserPermission->getKey(), + 'is_denied' => false, + ]); + + CustomPermissionPivotTestPermissionPivot::$events = []; + + $user->denyPermissionTo('edit-articles'); + + $this->assertSame(['updated'], CustomPermissionPivotTestPermissionPivot::$events); + $this->assertDatabaseHas('model_has_permissions', [ + 'model_test_id' => $user->getKey(), + 'permission_test_id' => $this->testUserPermission->getKey(), + 'is_denied' => true, + ]); + } + + public function testImmediateRoleWritesUseThePublicCustomPivot(): void + { + $user = CustomPermissionPivotTestUser::create(['email' => 'custom@example.com']); + + $user->assignRole('testRole'); + + $this->assertSame(['created'], CustomPermissionPivotTestRolePivot::$events); + + CustomPermissionPivotTestRolePivot::$events = []; + + $user->syncRoles('testRole2'); + + $this->assertSame(['deleted', 'created'], CustomPermissionPivotTestRolePivot::$events); + $this->assertFalse($user->hasRole('testRole')); + $this->assertTrue($user->hasRole('testRole2')); + } + + public function testRoleSyncPreservesRetainedCustomPivotRows(): void + { + $user = CustomPermissionPivotTestUser::create(['email' => 'custom@example.com']); + $user->assignRole('testRole'); + CustomPermissionPivotTestRolePivot::$events = []; + + $user->syncRoles('testRole'); + + $this->assertSame([], CustomPermissionPivotTestRolePivot::$events); + + $user->syncRoles('testRole', 'testRole2'); + + $this->assertSame(['created'], CustomPermissionPivotTestRolePivot::$events); + $this->assertTrue($user->hasRole('testRole')); + $this->assertTrue($user->hasRole('testRole2')); + } + + public function testModelReturningPermissionApisReuseTheLoadedCustomPivotRelation(): void + { + $user = CustomPermissionPivotTestUser::create(['email' => 'custom@example.com']); + $user->givePermissionTo('edit-articles'); + + DB::enableQueryLog(); + DB::flushQueryLog(); + + $directPermissions = $user->getDirectPermissions(); + $queriesAfterFirstLoad = count(DB::getQueryLog()); + + $this->assertInstanceOf( + CustomPermissionPivotTestPermissionPivot::class, + $directPermissions->firstOrFail()->getRelation('pivot'), + ); + $this->assertSame(['edit-articles'], $user->getDirectPermissions()->pluck('name')->all()); + $this->assertSame($queriesAfterFirstLoad, count(DB::getQueryLog())); + + $this->assertSame( + CustomPermissionPivotTestPermissionPivot::class, + $user->getAllPermissions()->firstOrFail()->getRelation('pivot')::class, + ); + $queriesAfterAllPermissions = count(DB::getQueryLog()); + $this->assertSame(['edit-articles'], $user->getAllPermissions()->pluck('name')->all()); + $this->assertSame($queriesAfterAllPermissions, count(DB::getQueryLog())); + + $this->assertTrue($user->hasDirectPermission('edit-articles')); + $this->assertSame(['edit-articles'], $user->getPermissionNames()->all()); + $this->assertSame($queriesAfterAllPermissions, count(DB::getQueryLog())); + } + + public function testDeferredAssignmentsRetainTheirCustomPivotClasses(): void + { + $user = new CustomPermissionPivotTestUser(['email' => 'deferred@example.com']); + + $user->syncPermissionEffects(allowed: ['edit-articles'], denied: ['edit-news']); + $user->syncRoles('testRole'); + $user->save(); + + $this->assertSame(['created', 'created'], CustomPermissionPivotTestPermissionPivot::$events); + $this->assertSame(['created'], CustomPermissionPivotTestRolePivot::$events); + $this->assertTrue($user->hasDirectPermission('edit-articles')); + $this->assertTrue($user->hasDeniedPermission('edit-news')); + $this->assertTrue($user->hasRole('testRole')); + } +} + +class CustomPermissionPivotTestUser extends UserWithoutHasRoles +{ + use HasRoles { + permissions as protected traitPermissions; + roles as protected traitRoles; + } + + protected string $guard_name = 'web'; + + /** + * @return BelongsToMany + */ + public function permissions(): BelongsToMany + { + return $this->traitPermissions()->using(CustomPermissionPivotTestPermissionPivot::class); + } + + /** + * @return BelongsToMany + */ + public function roles(): BelongsToMany + { + return $this->traitRoles()->using(CustomPermissionPivotTestRolePivot::class); + } +} + +class CustomPermissionPivotTestPermissionPivot extends MorphPivot +{ + protected array $casts = [ + 'is_denied' => 'boolean', + ]; + + protected array $guarded = ['is_denied']; + + public static array $events = []; + + protected static function boot(): void + { + parent::boot(); + + static::created(function (): void { + static::$events[] = 'created'; + }); + static::updated(function (): void { + static::$events[] = 'updated'; + }); + static::deleted(function (): void { + static::$events[] = 'deleted'; + }); + } +} + +class CustomPermissionPivotTestRolePivot extends MorphPivot +{ + public static array $events = []; + + protected static function boot(): void + { + parent::boot(); + + static::created(function (): void { + static::$events[] = 'created'; + }); + static::updated(function (): void { + static::$events[] = 'updated'; + }); + static::deleted(function (): void { + static::$events[] = 'deleted'; + }); + } +} diff --git a/tests/Permission/Events/EventTest.php b/tests/Permission/Events/EventTest.php index 14c6fbfda..9609d6b9f 100644 --- a/tests/Permission/Events/EventTest.php +++ b/tests/Permission/Events/EventTest.php @@ -332,7 +332,7 @@ public function testRoleSyncDispatchesCurrentDetachedAndRequestedAttachedPayload }); } - public function testPermissionSyncDispatchesRequestedAttachedPayloadOnly(): void + public function testPermissionSyncDispatchesCurrentDetachedAndRequestedAttachedPayloads(): void { $this->testUser->givePermissionTo('edit-articles'); $permission = $this->app->make(PermissionContract::class)::findByName('edit-news'); @@ -346,7 +346,111 @@ public function testPermissionSyncDispatchesRequestedAttachedPayloadOnly(): void return $event->model->is($this->testUser) && $event->permissionsOrIds === [$this->testUserPermission->getKey(), $permission->getKey()]; }); - Event::assertNotDispatched(PermissionDetachedEvent::class); + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds->modelKeys() === [$this->testUserPermission->getKey()]; + }); + } + + public function testPermissionSyncReportsSameSetAndEmptyReplacements(): void + { + $this->testUser->givePermissionTo('edit-articles'); + $this->app->make('config')->set('permission.events_enabled', true); + + Event::fake([PermissionAttachedEvent::class, PermissionDetachedEvent::class]); + + $this->testUser->syncPermissions('edit-articles'); + + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds->modelKeys() === [$this->testUserPermission->getKey()]; + }); + Event::assertDispatched(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds === [$this->testUserPermission->getKey()]; + }); + + Event::fake([PermissionAttachedEvent::class, PermissionDetachedEvent::class]); + + $this->testUser->syncPermissions(); + + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds->modelKeys() === [$this->testUserPermission->getKey()]; + }); + Event::assertDispatched(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds === []; + }); + } + + public function testRolePermissionSyncReportsTheCurrentPermissionCollection(): void + { + $this->testUserRole->givePermissionTo('edit-articles'); + $permission = $this->app->make(PermissionContract::class)::findByName('edit-news'); + $this->app->make('config')->set('permission.events_enabled', true); + + Event::fake([PermissionAttachedEvent::class, PermissionDetachedEvent::class]); + + $this->testUserRole->syncPermissions('edit-news'); + + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event): bool { + return $event->model->is($this->testUserRole) + && $event->permissionsOrIds->modelKeys() === [$this->testUserPermission->getKey()]; + }); + Event::assertDispatched(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event) use ($permission): bool { + return $event->model->is($this->testUserRole) + && $event->permissionsOrIds === [$permission->getKey()]; + }); + } + + public function testPermissionEffectSyncReportsThePreOperationCollection(): void + { + $this->testUser->givePermissionTo('edit-articles'); + $permission = $this->app->make(PermissionContract::class)::findByName('edit-news'); + $this->app->make('config')->set('permission.events_enabled', true); + + Event::fake([PermissionAttachedEvent::class, PermissionDetachedEvent::class]); + + $this->testUser->syncPermissionEffects( + allowed: ['edit-news'], + denied: ['edit-articles'], + ); + + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds->modelKeys() === [$this->testUserPermission->getKey()]; + }); + Event::assertDispatched(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event) use ($permission): bool { + return $event->model->is($this->testUser) + && $event->permissionsOrIds === [$permission->getKey(), $this->testUserPermission->getKey()]; + }); + $this->assertTrue($this->testUser->hasDirectPermission($permission)); + $this->assertTrue($this->testUser->hasDeniedPermission($this->testUserPermission)); + } + + public function testPermissionSyncDispatchesAfterCommitAndCacheInvalidationInReplacementOrder(): void + { + $this->testUser->givePermissionTo('edit-articles'); + $this->app->make('config')->set('permission.events_enabled', true); + + $events = []; + + Event::listen(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event) use (&$events): void { + $events[] = 'detached'; + $this->assertSame(['edit-articles'], $event->permissionsOrIds->pluck('name')->all()); + $this->assertFalse($event->model->hasDirectPermission('edit-articles')); + $this->assertTrue($event->model->hasDirectPermission('edit-news')); + }); + Event::listen(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event) use (&$events): void { + $events[] = 'attached'; + $this->assertFalse($event->model->hasDirectPermission('edit-articles')); + $this->assertTrue($event->model->hasDirectPermission('edit-news')); + }); + + $this->testUser->syncPermissions('edit-news'); + + $this->assertSame(['detached', 'attached'], $events); } public function testPermissionEffectUpdateDispatchesTheChangedPermission(): void diff --git a/tests/Permission/Events/PartitionEventTest.php b/tests/Permission/Events/PartitionEventTest.php index 1a04605b9..8f6f93bbf 100644 --- a/tests/Permission/Events/PartitionEventTest.php +++ b/tests/Permission/Events/PartitionEventTest.php @@ -123,7 +123,10 @@ public function testPartitionedNoOpAndSyncEventsPreserveRequestedUuidPayloads(): return $event->model->is($user) && $event->permissionsOrIds === [$edit->getKey(), $publish->getKey()]; }); - Event::assertNotDispatched(PermissionDetachedEvent::class); + Event::assertDispatched(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event) use ($user, $edit): bool { + return $event->model->is($user) + && $event->permissionsOrIds->modelKeys() === [$edit->getKey()]; + }); } /** diff --git a/tests/Permission/Integration/CacheTest.php b/tests/Permission/Integration/CacheTest.php index 94ab1fb99..9ad092ca1 100644 --- a/tests/Permission/Integration/CacheTest.php +++ b/tests/Permission/Integration/CacheTest.php @@ -4,6 +4,7 @@ namespace Hypervel\Tests\Permission\Integration; +use Hypervel\Contracts\Cache\Repository; use Hypervel\Permission\Contracts\Permission; use Hypervel\Permission\Contracts\Role; use Hypervel\Permission\Exceptions\PermissionDoesNotExist; @@ -12,6 +13,7 @@ use Hypervel\Support\Facades\DB; use Hypervel\Tests\Permission\Fixtures\Models\User; use Hypervel\Tests\Permission\TestCase; +use Mockery as m; class CacheTest extends TestCase { @@ -168,6 +170,32 @@ public function testItFlushesTheCacheWhenGivingAPermissionToARole(): void $this->assertQueryCount($this->cacheRunCount); } + public function testNoOpPermissionSyncDoesNotFlushTheCache(): void + { + $this->testUserRole->givePermissionTo($this->testUserPermission); + $this->registrar->getPermissions(); + + $this->testUserRole->syncPermissions($this->testUserPermission); + + $this->resetQueryCount(); + $this->registrar->getPermissions(); + + $this->assertQueryCount(0); + } + + public function testNoOpPermissionEffectSyncDoesNotFlushTheCache(): void + { + $this->testUserRole->givePermissionTo($this->testUserPermission); + $this->registrar->getPermissions(); + + $this->testUserRole->syncPermissionEffects(allowed: [$this->testUserPermission]); + + $this->resetQueryCount(); + $this->registrar->getPermissions(); + + $this->assertQueryCount(0); + } + public function testItUsesTheCacheForHasPermissionTo(): void { $this->testUserRole->givePermissionTo(['edit-articles', 'edit-news', 'Edit News']); @@ -280,6 +308,23 @@ public function testItCanResetTheCacheWithArtisanCommand(): void $this->assertQueryCount($this->cacheRunCount); } + public function testItShowsAnErrorWhenTheCacheExistsButCannotBeFlushed(): void + { + $registrar = m::mock(PermissionRegistrar::class)->makePartial(); + $registrar->cacheKey = $this->registrar->cacheKey; + $registrar->shouldReceive('forgetCachedPermissions')->once()->andReturn(false); + + $cacheRepository = m::mock(Repository::class); + $cacheRepository->shouldReceive('has')->with($registrar->cacheKey)->andReturn(true); + $registrar->shouldReceive('getCacheRepository')->once()->andReturn($cacheRepository); + + $this->app->instance(PermissionRegistrar::class, $registrar); + + Artisan::call('permission:cache-reset'); + + $this->assertStringContainsString('Unable to flush cache.', Artisan::output()); + } + protected function resetQueryCount(): void { DB::flushQueryLog(); diff --git a/tests/Permission/PartitionCustomPivotTest.php b/tests/Permission/PartitionCustomPivotTest.php new file mode 100644 index 000000000..701884d93 --- /dev/null +++ b/tests/Permission/PartitionCustomPivotTest.php @@ -0,0 +1,241 @@ + self::PARTITION_A, 'name' => 'A One']); + $teamA2 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A Two']); + $user = PartitionCustomPivotUser::create(['email' => 'custom@example.com']); + $permissionA = PartitionedPermission::create(['name' => 'articles.edit']); + + setPermissionsTeamId($teamA1); + $user->givePermissionTo($permissionA); + + setPermissionsTeamId($teamA2); + $user->givePermissionTo($permissionA); + + $this->setPartition(self::PARTITION_B); + $teamB = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_B, 'name' => 'B One']); + $permissionB = PartitionedPermission::create(['name' => 'articles.edit']); + setPermissionsTeamId($teamB); + $user->givePermissionTo($permissionB); + + $this->setPartition(self::PARTITION_A); + setPermissionsTeamId($teamA1); + PartitionCustomPermissionPivot::$events = []; + + $user->denyPermissionTo($permissionA); + + $this->assertSame(['updated'], PartitionCustomPermissionPivot::$events); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA1->getKey(), + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => true, + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA2->getKey(), + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => false, + ]); + + PartitionCustomPermissionPivot::$events = []; + $user->revokePermissionTo($permissionA); + + $this->assertSame(['deleted'], PartitionCustomPermissionPivot::$events); + $this->assertDatabaseMissing(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA1->getKey(), + 'permission_test_id' => $permissionA->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA2->getKey(), + 'permission_test_id' => $permissionA->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'team_test_id' => $teamB->getKey(), + 'permission_test_id' => $permissionB->getKey(), + ]); + } + + public function testDeferredCustomPivotAssignmentsRetainTheirCapturedContexts(): void + { + $teamA = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A One']); + $permissionA = PartitionedPermission::create(['name' => 'articles.edit']); + $user = new PartitionCustomPivotUser(['email' => 'deferred@example.com']); + + setPermissionsTeamId($teamA); + $user->givePermissionTo($permissionA); + + $this->setPartition(self::PARTITION_B); + $teamB = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_B, 'name' => 'B One']); + $permissionB = PartitionedPermission::create(['name' => 'articles.edit']); + setPermissionsTeamId($teamB); + $user->denyPermissionTo($permissionB); + + PartitionContext::forget(); + setPermissionsTeamId(null); + $user->save(); + + $this->assertSame(['created', 'created'], PartitionCustomPermissionPivot::$events); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA->getKey(), + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => false, + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'team_test_id' => $teamB->getKey(), + 'permission_test_id' => $permissionB->getKey(), + 'is_denied' => true, + ]); + } + + public function testCustomRoleWritesRetainTheCapturedPartitionAndTeam(): void + { + $teamA1 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A One']); + $teamA2 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A Two']); + $user = PartitionCustomPivotUser::create(['email' => 'roles@example.com']); + + setPermissionsTeamId($teamA1); + $roleA1 = PartitionedRole::create(['name' => 'member']); + $user->assignRole($roleA1); + + setPermissionsTeamId($teamA2); + $roleA2 = PartitionedRole::create(['name' => 'member']); + $user->assignRole($roleA2); + + $this->setPartition(self::PARTITION_B); + $teamB = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_B, 'name' => 'B One']); + setPermissionsTeamId($teamB); + $roleB = PartitionedRole::create(['name' => 'member']); + $user->assignRole($roleB); + + $this->setPartition(self::PARTITION_A); + setPermissionsTeamId($teamA1); + PartitionCustomRolePivot::$events = []; + + $user->removeRole($roleA1); + + $this->assertSame(['deleted'], PartitionCustomRolePivot::$events); + $this->assertDatabaseMissing(Config::modelHasRolesTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA1->getKey(), + 'role_test_id' => $roleA1->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasRolesTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA2->getKey(), + 'role_test_id' => $roleA2->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasRolesTable(), [ + 'workspace_id' => self::PARTITION_B, + 'team_test_id' => $teamB->getKey(), + 'role_test_id' => $roleB->getKey(), + ]); + } +} + +class PartitionCustomPivotUser extends UserWithoutHasRoles +{ + use HasRoles { + permissions as protected traitPermissions; + roles as protected traitRoles; + } + use HasUuids; + + protected ?string $table = 'global_partition_users'; + + protected string $guard_name = 'web'; + + /** + * @return BelongsToMany + */ + public function permissions(): BelongsToMany + { + return $this->traitPermissions()->using(PartitionCustomPermissionPivot::class); + } + + /** + * @return BelongsToMany + */ + public function roles(): BelongsToMany + { + return $this->traitRoles()->using(PartitionCustomRolePivot::class); + } +} + +class PartitionCustomPermissionPivot extends MorphPivot +{ + protected array $casts = [ + 'is_denied' => 'boolean', + ]; + + protected array $guarded = ['is_denied']; + + public static array $events = []; + + protected static function boot(): void + { + parent::boot(); + + static::created(function (): void { + static::$events[] = 'created'; + }); + static::updated(function (): void { + static::$events[] = 'updated'; + }); + static::deleted(function (): void { + static::$events[] = 'deleted'; + }); + } +} + +class PartitionCustomRolePivot extends MorphPivot +{ + public static array $events = []; + + protected static function boot(): void + { + parent::boot(); + + static::created(function (): void { + static::$events[] = 'created'; + }); + static::deleted(function (): void { + static::$events[] = 'deleted'; + }); + } +} diff --git a/tests/Permission/PartitionRelationsTest.php b/tests/Permission/PartitionRelationsTest.php index 7c0550fe9..ba89e1eec 100644 --- a/tests/Permission/PartitionRelationsTest.php +++ b/tests/Permission/PartitionRelationsTest.php @@ -5,6 +5,8 @@ namespace Hypervel\Tests\Permission; use Hypervel\Context\CoroutineContext; +use Hypervel\Database\Eloquent\Relations\MorphPivot; +use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Database\Events\TransactionBeginning; use Hypervel\Database\QueryException; use Hypervel\Permission\Events\PermissionAttachedEvent; @@ -15,6 +17,7 @@ use Hypervel\Permission\PermissionRegistrar; use Hypervel\Permission\Support\Config; use Hypervel\Permission\Support\PermissionPartition; +use Hypervel\Support\ClassInvoker; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Event; use Hypervel\Tests\Permission\Fixtures\Models\GlobalPartitionUser; @@ -52,6 +55,139 @@ public function testGlobalSubjectHasIndependentAssignmentsInEachPartition(): voi $this->assertTrue($user->hasPermissionTo($permissionA)); } + public function testWarmPermissionPivotRetainsItsPartitionIdentity(): void + { + $user = GlobalPartitionUser::create(['email' => 'global@example.com']); + $permissionA = PartitionedPermission::create(['name' => 'articles.edit']); + $user->givePermissionTo($permissionA); + + $warmPermission = $user->getDirectPermissions()->sole(); + $warmPivot = $warmPermission->getRelation('pivot'); + $relationPivot = $user->permissions()->firstOrFail()->getRelation('pivot'); + $registrar = $this->app->make(PermissionRegistrar::class); + + $this->assertInstanceOf(MorphPivot::class, $warmPivot); + $this->assertInstanceOf(MorphPivot::class, $relationPivot); + $this->assertSame(Config::morphKey(), $warmPivot->getForeignKey()); + $this->assertSame($registrar->pivotPermission, $warmPivot->getRelatedKey()); + $this->assertSame(Config::MORPH_TYPE, $warmPivot->getMorphType()); + + $warmQuery = (new ClassInvoker($warmPivot))->getDeleteQuery(); + $relationQuery = (new ClassInvoker($relationPivot))->getDeleteQuery(); + + $this->assertSame($relationQuery->toSql(), $warmQuery->toSql()); + $this->assertSame($relationQuery->getBindings(), $warmQuery->getBindings()); + + $this->setPartition(self::PARTITION_B); + $permissionB = PartitionedPermission::create(['name' => 'articles.edit']); + $user->givePermissionTo($permissionB); + + $warmPivot->setAttribute('is_denied', true); + $this->assertTrue($warmPivot->save()); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => true, + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'permission_test_id' => $permissionB->getKey(), + 'is_denied' => false, + ]); + + $this->assertSame(1, $warmPivot->delete()); + $this->assertDatabaseMissing(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'permission_test_id' => $permissionA->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'permission_test_id' => $permissionB->getKey(), + ]); + } + + public function testWarmRolePermissionPivotsRetainTheirPartitionIdentity(): void + { + $user = GlobalPartitionUser::create(['email' => 'global@example.com']); + $roleA = PartitionedRole::create(['name' => 'member']); + $permissionA = PartitionedPermission::create(['name' => 'articles.edit']); + $deniedPermission = PartitionedPermission::create(['name' => 'articles.delete']); + $roleA->givePermissionTo($permissionA); + $roleA->denyPermissionTo($deniedPermission); + $user->assignRole($roleA); + + $registrar = $this->app->make(PermissionRegistrar::class); + $catalogPermission = $registrar->getPermissions(['name' => 'articles.edit'], true)->firstOrFail(); + $catalogRole = $catalogPermission->getRelation('roles')->sole(); + $catalogPivot = $catalogRole->getRelation('pivot'); + $liveCatalogPivot = $permissionA->roles()->firstOrFail()->getRelation('pivot'); + + $this->assertInstanceOf(Pivot::class, $catalogPivot); + $this->assertInstanceOf(Pivot::class, $liveCatalogPivot); + $this->assertSame($registrar->pivotPermission, $catalogPivot->getForeignKey()); + $this->assertSame($registrar->pivotRole, $catalogPivot->getRelatedKey()); + + $catalogQuery = (new ClassInvoker($catalogPivot))->getDeleteQuery(); + $liveCatalogQuery = (new ClassInvoker($liveCatalogPivot))->getDeleteQuery(); + + $this->assertSame($liveCatalogQuery->toSql(), $catalogQuery->toSql()); + $this->assertSame($liveCatalogQuery->getBindings(), $catalogQuery->getBindings()); + + $viaPermission = $user->getPermissionsViaRoles()->firstWhere('name', 'articles.edit'); + $this->assertInstanceOf(PartitionedPermission::class, $viaPermission); + $viaPivot = $viaPermission->getRelation('pivot'); + $liveViaPivot = $roleA->permissions()->firstOrFail()->getRelation('pivot'); + + $this->assertInstanceOf(Pivot::class, $viaPivot); + $this->assertInstanceOf(Pivot::class, $liveViaPivot); + $this->assertNotSame($catalogPivot, $viaPivot); + $this->assertSame($registrar->pivotRole, $viaPivot->getForeignKey()); + $this->assertSame($registrar->pivotPermission, $viaPivot->getRelatedKey()); + + $viaQuery = (new ClassInvoker($viaPivot))->getDeleteQuery(); + $liveViaQuery = (new ClassInvoker($liveViaPivot))->getDeleteQuery(); + + $this->assertSame($liveViaQuery->toSql(), $viaQuery->toSql()); + $this->assertSame($liveViaQuery->getBindings(), $viaQuery->getBindings()); + + $viaPivot->setAttribute('is_denied', true); + + $this->assertFalse((bool) $catalogPivot->getAttribute('is_denied')); + $this->assertFalse($user->hasDeniedPermissionViaRoles($permissionA)); + + $this->setPartition(self::PARTITION_B); + $roleB = PartitionedRole::create(['name' => 'member']); + $permissionB = PartitionedPermission::create(['name' => 'articles.edit']); + $roleB->givePermissionTo($permissionB); + $user->assignRole($roleB); + + $this->assertTrue($viaPivot->save()); + $this->assertDatabaseHas(Config::roleHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'permission_test_id' => $permissionA->getKey(), + 'role_test_id' => $roleA->getKey(), + 'is_denied' => true, + ]); + $this->assertDatabaseHas(Config::roleHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'permission_test_id' => $permissionB->getKey(), + 'role_test_id' => $roleB->getKey(), + 'is_denied' => false, + ]); + + $this->assertSame(1, $viaPivot->delete()); + $this->assertDatabaseMissing(Config::roleHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'permission_test_id' => $permissionA->getKey(), + 'role_test_id' => $roleA->getKey(), + ]); + $this->assertDatabaseHas(Config::roleHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'permission_test_id' => $permissionB->getKey(), + 'role_test_id' => $roleB->getKey(), + ]); + } + public function testPublicRelationAttachAddsTheCapturedPartition(): void { $user = GlobalPartitionUser::create(['email' => 'global@example.com']); diff --git a/tests/Permission/PartitionTeamsTest.php b/tests/Permission/PartitionTeamsTest.php index ec756c1dd..117563be6 100644 --- a/tests/Permission/PartitionTeamsTest.php +++ b/tests/Permission/PartitionTeamsTest.php @@ -4,8 +4,10 @@ namespace Hypervel\Tests\Permission; +use Hypervel\Database\Eloquent\Relations\MorphPivot; use Hypervel\Permission\PermissionRegistrar; use Hypervel\Permission\Support\Config; +use Hypervel\Support\ClassInvoker; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Schema; use Hypervel\Tests\Permission\Fixtures\Models\GlobalPartitionPermissionsOnlyUser; @@ -86,6 +88,81 @@ public function testPartitionAndTeamRemainIndependentAssignmentDimensions(): voi ]); } + public function testWarmPermissionPivotRetainsItsPartitionAndTeamIdentity(): void + { + $teamA1 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A One']); + $teamA2 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A Two']); + $user = GlobalPartitionUser::create(['email' => 'global@example.com']); + $permissionA = PartitionedPermission::create(['name' => 'articles.edit']); + + setPermissionsTeamId($teamA1); + $user->givePermissionTo($permissionA); + + $warmPermission = $user->getDirectPermissions()->sole(); + $warmPivot = $warmPermission->getRelation('pivot'); + $relationPivot = $user->permissions()->firstOrFail()->getRelation('pivot'); + $registrar = $this->app->make(PermissionRegistrar::class); + + $this->assertInstanceOf(MorphPivot::class, $warmPivot); + $this->assertInstanceOf(MorphPivot::class, $relationPivot); + $this->assertSame(Config::morphKey(), $warmPivot->getForeignKey()); + $this->assertSame($registrar->pivotPermission, $warmPivot->getRelatedKey()); + $this->assertSame(Config::MORPH_TYPE, $warmPivot->getMorphType()); + + $warmQuery = (new ClassInvoker($warmPivot))->getDeleteQuery(); + $relationQuery = (new ClassInvoker($relationPivot))->getDeleteQuery(); + + $this->assertSame($relationQuery->toSql(), $warmQuery->toSql()); + $this->assertSame($relationQuery->getBindings(), $warmQuery->getBindings()); + + setPermissionsTeamId($teamA2); + $user->givePermissionTo($permissionA); + + $this->setPartition(self::PARTITION_B); + $teamB = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_B, 'name' => 'B One']); + $permissionB = PartitionedPermission::create(['name' => 'articles.edit']); + setPermissionsTeamId($teamB); + $user->givePermissionTo($permissionB); + + $warmPivot->setAttribute('is_denied', true); + $this->assertTrue($warmPivot->save()); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA1->getKey(), + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => true, + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA2->getKey(), + 'permission_test_id' => $permissionA->getKey(), + 'is_denied' => false, + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'team_test_id' => $teamB->getKey(), + 'permission_test_id' => $permissionB->getKey(), + 'is_denied' => false, + ]); + + $this->assertSame(1, $warmPivot->delete()); + $this->assertDatabaseMissing(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA1->getKey(), + 'permission_test_id' => $permissionA->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_A, + 'team_test_id' => $teamA2->getKey(), + 'permission_test_id' => $permissionA->getKey(), + ]); + $this->assertDatabaseHas(Config::modelHasPermissionsTable(), [ + 'workspace_id' => self::PARTITION_B, + 'team_test_id' => $teamB->getKey(), + 'permission_test_id' => $permissionB->getKey(), + ]); + } + public function testChangingTeamReloadsLoadedAssignmentRelationsAutomatically(): void { $teamA1 = PartitionWorkspaceTeam::create(['workspace_id' => self::PARTITION_A, 'name' => 'A One']); diff --git a/tests/Permission/Traits/HasPermissionsTest.php b/tests/Permission/Traits/HasPermissionsTest.php index fb10b340e..39a00f383 100644 --- a/tests/Permission/Traits/HasPermissionsTest.php +++ b/tests/Permission/Traits/HasPermissionsTest.php @@ -6,12 +6,15 @@ use Hypervel\Database\Eloquent\MissingAttributeException; use Hypervel\Database\Eloquent\Model; +use Hypervel\Database\Eloquent\Relations\Pivot; use Hypervel\Permission\Contracts\Permission; use Hypervel\Permission\Contracts\Role; use Hypervel\Permission\Events\PermissionAttachedEvent; use Hypervel\Permission\Events\PermissionDetachedEvent; use Hypervel\Permission\Exceptions\GuardDoesNotMatch; use Hypervel\Permission\Exceptions\PermissionDoesNotExist; +use Hypervel\Permission\PermissionRegistrar; +use Hypervel\Support\ClassInvoker; use Hypervel\Support\Facades\DB; use Hypervel\Support\Facades\Event; use Hypervel\Tests\Permission\Fixtures\Models\SoftDeletingUser; @@ -30,6 +33,13 @@ public function testItCanAssignAPermissionToAUser(): void $this->assertTrue($this->testUser->hasPermissionTo($this->testUserPermission)); } + public function testItCanCheckAPermissionByItsId(): void + { + $this->testUser->givePermissionTo($this->testUserPermission); + + $this->assertTrue($this->testUser->hasPermissionTo($this->testUserPermission->getKey())); + } + public function testItCanAssignAPermissionToAUserWithANonDefaultGuard(): void { $testUserPermission = app(Permission::class)->create([ @@ -74,6 +84,14 @@ public function testItCanRevokeAPermissionFromAUser(): void $this->assertFalse($this->testUser->hasPermissionTo($this->testUserPermission)); } + public function testItSilentlyIgnoresValuesThatCannotBeResolvedToAPermission(): void + { + $this->testUser->givePermissionTo(['edit-articles', true]); + + $this->assertTrue($this->testUser->hasPermissionTo('edit-articles')); + $this->assertCount(1, $this->testUser->permissions); + } + public function testItCanAssignAndRemoveAPermissionUsingEnums(): void { $enum = TestRolePermissionsEnum::ViewArticles; @@ -492,6 +510,56 @@ public function testItCanListAllThePermissionsViaRolesOfUser(): void ); } + public function testViaRolePermissionPivotsAreCompleteAndDoNotAliasTheCatalog(): void + { + $this->testUserRole->givePermissionTo('edit-articles'); + $this->testUserRole->denyPermissionTo('edit-news'); + $this->testUser->assignRole('testRole'); + + $registrar = $this->app->make(PermissionRegistrar::class); + $catalogPermission = $registrar->getPermissions(['name' => 'edit-articles'], true)->firstOrFail(); + $catalogRole = $catalogPermission->getRelation('roles')->sole(); + $catalogPivot = $catalogRole->getRelation('pivot'); + $liveCatalogPivot = $this->testUserPermission->roles()->firstOrFail()->getRelation('pivot'); + + $this->assertInstanceOf(Pivot::class, $catalogPivot); + $this->assertInstanceOf(Pivot::class, $liveCatalogPivot); + $this->assertSame($registrar->pivotPermission, $catalogPivot->getForeignKey()); + $this->assertSame($registrar->pivotRole, $catalogPivot->getRelatedKey()); + $this->assertSame( + (new ClassInvoker($liveCatalogPivot))->getDeleteQuery()->toSql(), + (new ClassInvoker($catalogPivot))->getDeleteQuery()->toSql(), + ); + $this->assertSame( + (new ClassInvoker($liveCatalogPivot))->getDeleteQuery()->getBindings(), + (new ClassInvoker($catalogPivot))->getDeleteQuery()->getBindings(), + ); + + $viaPermission = $this->testUser->getPermissionsViaRoles()->firstWhere('name', 'edit-articles'); + $this->assertInstanceOf(Model::class, $viaPermission); + $viaPivot = $viaPermission->getRelation('pivot'); + $liveViaPivot = $this->testUserRole->permissions()->firstOrFail()->getRelation('pivot'); + + $this->assertInstanceOf(Pivot::class, $viaPivot); + $this->assertInstanceOf(Pivot::class, $liveViaPivot); + $this->assertNotSame($catalogPivot, $viaPivot); + $this->assertSame($registrar->pivotRole, $viaPivot->getForeignKey()); + $this->assertSame($registrar->pivotPermission, $viaPivot->getRelatedKey()); + $this->assertSame( + (new ClassInvoker($liveViaPivot))->getDeleteQuery()->toSql(), + (new ClassInvoker($viaPivot))->getDeleteQuery()->toSql(), + ); + $this->assertSame( + (new ClassInvoker($liveViaPivot))->getDeleteQuery()->getBindings(), + (new ClassInvoker($viaPivot))->getDeleteQuery()->getBindings(), + ); + + $viaPivot->setAttribute('is_denied', true); + + $this->assertFalse((bool) $catalogPivot->getAttribute('is_denied')); + $this->assertFalse($this->testUser->hasDeniedPermissionViaRoles('edit-articles')); + } + public function testItCanListAllTheCoupledPermissionsBothDirectlyAndViaRoles(): void { $this->testUser->givePermissionTo('edit-news'); diff --git a/tests/Permission/Traits/TeamHasPermissionsTest.php b/tests/Permission/Traits/TeamHasPermissionsTest.php index 8578d3d5f..f19e60fbc 100644 --- a/tests/Permission/Traits/TeamHasPermissionsTest.php +++ b/tests/Permission/Traits/TeamHasPermissionsTest.php @@ -5,7 +5,15 @@ namespace Hypervel\Tests\Permission\Traits; use Hypervel\Contracts\Foundation\Application as ApplicationContract; +use Hypervel\Database\Eloquent\Model; +use Hypervel\Database\Eloquent\Relations\MorphPivot; +use Hypervel\Permission\Events\PermissionAttachedEvent; +use Hypervel\Permission\Events\PermissionDetachedEvent; +use Hypervel\Permission\PermissionRegistrar; +use Hypervel\Permission\Support\Config; +use Hypervel\Support\ClassInvoker; use Hypervel\Support\Facades\DB; +use Hypervel\Support\Facades\Event; use Hypervel\Tests\Permission\Fixtures\Models\User; class TeamHasPermissionsTest extends HasPermissionsTest @@ -329,4 +337,84 @@ public function testPermissionScopeUsesRoleAssignmentsForCurrentTeamOnly(): void fn (User $user): bool => $user->is($this->testUser), )); } + + public function testPermissionReplacementEventsStayInsideTheCurrentTeam(): void + { + setPermissionsTeamId(1); + $this->testUser->givePermissionTo('edit-articles'); + + setPermissionsTeamId(2); + $this->testUser->givePermissionTo('edit-articles'); + + setPermissionsTeamId(1); + $this->app->make('config')->set('permission.events_enabled', true); + $events = []; + + Event::listen(PermissionDetachedEvent::class, function (PermissionDetachedEvent $event) use (&$events): void { + $events[] = 'detached'; + $this->assertSame(['edit-articles'], $event->permissionsOrIds->pluck('name')->all()); + $this->assertFalse($event->model->hasDirectPermission('edit-articles')); + $this->assertTrue($event->model->hasDirectPermission('edit-news')); + }); + Event::listen(PermissionAttachedEvent::class, function (PermissionAttachedEvent $event) use (&$events): void { + $events[] = 'attached'; + $this->assertFalse($event->model->hasDirectPermission('edit-articles')); + $this->assertTrue($event->model->hasDirectPermission('edit-news')); + }); + + $this->testUser->syncPermissions('edit-news'); + + $this->assertSame(['detached', 'attached'], $events); + + setPermissionsTeamId(2); + $this->assertTrue($this->testUser->hasDirectPermission('edit-articles')); + $this->assertFalse($this->testUser->hasDirectPermission('edit-news')); + } + + public function testWarmPermissionPivotMatchesTheCapturedTeamConstraint(): void + { + setPermissionsTeamId(1); + + $this->assertWarmPermissionPivotMatchesRelationPivot( + $this->testUser, + $this->testUserPermission, + ); + } + + public function testWarmPermissionPivotMatchesTheCapturedGlobalTeamConstraint(): void + { + setPermissionsTeamId(null); + + $this->assertWarmPermissionPivotMatchesRelationPivot( + $this->testUser, + $this->testUserPermission, + ); + } + + /** + * Assert a compact-cache pivot retains the selected relation's identity constraints. + */ + private function assertWarmPermissionPivotMatchesRelationPivot(Model $user, Model $permission): void + { + $registrar = $this->app->make(PermissionRegistrar::class); + $registrar->rememberModelPermissionAssignments($user, fn (): array => [[ + $permission->getKeyName() => $permission->getKey(), + 'is_denied' => false, + ]]); + + $warmPermission = $user->getDirectPermissions()->sole(); + $warmPivot = $warmPermission->getRelation('pivot'); + + $this->assertInstanceOf(MorphPivot::class, $warmPivot); + $this->assertSame(Config::morphKey(), $warmPivot->getForeignKey()); + $this->assertSame($registrar->pivotPermission, $warmPivot->getRelatedKey()); + $this->assertSame(Config::MORPH_TYPE, $warmPivot->getMorphType()); + + $relationPivot = $user->permissions()->newExistingPivot($warmPivot->getAttributes()); + $warmQuery = (new ClassInvoker($warmPivot))->getDeleteQuery(); + $relationQuery = (new ClassInvoker($relationPivot))->getDeleteQuery(); + + $this->assertSame($relationQuery->toSql(), $warmQuery->toSql()); + $this->assertSame($relationQuery->getBindings(), $warmQuery->getBindings()); + } } From 64af1663f5bcda8ed8f3582dff56e82b329ab3e7 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:45:37 +0000 Subject: [PATCH 03/10] Harden Permission commands and package discovery Treat only null and empty team options as absent so the string zero remains a valid team identifier, restore the caller's prior team context after command execution, and report migration publication failures through the command exit status. Declare Permission's direct split-package dependencies, remove runtime class guards for those installed dependencies, and register Permission, Horizon, and Wayfinder providers in the root manifest. Add a repository-wide split-provider discoverability invariant and exact Permission metadata parity coverage. Expand command regressions for disabled teams, zero identifiers, global-role warnings, existing migrations, copy failures, context restoration, and About output. --- composer.json | 3 + src/permission/composer.json | 5 +- .../src/Commands/AssignRoleCommand.php | 6 +- .../src/Commands/CreateRoleCommand.php | 9 +- .../src/Commands/UpgradeForTeamsCommand.php | 15 +-- .../src/PermissionServiceProvider.php | 4 - .../PackageManifestConsistencyTest.php | 25 +++++ tests/Permission/Commands/CommandTest.php | 30 +++--- tests/Permission/Commands/TeamCommandTest.php | 91 +++++++++++++++++++ tests/Permission/PackageMetadataTest.php | 46 ++++++++++ 10 files changed, 206 insertions(+), 28 deletions(-) create mode 100644 tests/Permission/PackageMetadataTest.php diff --git a/composer.json b/composer.json index 149a8c705..f0ab99aac 100644 --- a/composer.json +++ b/composer.json @@ -335,6 +335,7 @@ "Hypervel\\Events\\EventServiceProvider", "Hypervel\\Filesystem\\FilesystemServiceProvider", "Hypervel\\Hashing\\HashingServiceProvider", + "Hypervel\\Horizon\\HorizonServiceProvider", "Hypervel\\Http\\HttpServiceProvider", "Hypervel\\Inertia\\InertiaServiceProvider", "Hypervel\\Jwt\\JwtServiceProvider", @@ -348,6 +349,7 @@ "Hypervel\\Passkeys\\PasskeysServiceProvider", "Hypervel\\Fortify\\FortifyServiceProvider", "Hypervel\\Grpc\\GrpcServiceProvider", + "Hypervel\\Permission\\PermissionServiceProvider", "Hypervel\\Pipeline\\PipelineServiceProvider", "Hypervel\\Queue\\QueueServiceProvider", "Hypervel\\RateLimiter\\RateLimiterServiceProvider", @@ -365,6 +367,7 @@ "Hypervel\\Tinker\\TinkerServiceProvider", "Hypervel\\Translation\\TranslationServiceProvider", "Hypervel\\Validation\\ValidationServiceProvider", + "Hypervel\\Wayfinder\\WayfinderServiceProvider", "Hypervel\\WebSocketServer\\WebSocketServerServiceProvider", "Hypervel\\Scout\\ScoutServiceProvider", "Hypervel\\Telescope\\TelescopeServiceProvider", diff --git a/src/permission/composer.json b/src/permission/composer.json index 5f57f1e66..7de92d3ed 100644 --- a/src/permission/composer.json +++ b/src/permission/composer.json @@ -33,6 +33,7 @@ }, "require": { "php": "^8.4", + "composer-runtime-api": "^2.2", "hypervel/auth": "^0.4", "hypervel/broadcasting": "^0.4", "hypervel/cache": "^0.4", @@ -49,8 +50,10 @@ "hypervel/routing": "^0.4", "hypervel/support": "^0.4", "hypervel/view": "^0.4", + "nesbot/carbon": "^3.13.1", "symfony/console": "^8.1", - "symfony/http-foundation": "^8.1" + "symfony/http-foundation": "^8.1", + "symfony/http-kernel": "^8.1" }, "config": { "sort-packages": true diff --git a/src/permission/src/Commands/AssignRoleCommand.php b/src/permission/src/Commands/AssignRoleCommand.php index efc365d11..6a6de1d80 100644 --- a/src/permission/src/Commands/AssignRoleCommand.php +++ b/src/permission/src/Commands/AssignRoleCommand.php @@ -31,8 +31,10 @@ public function handle(PermissionRegistrar $permissionRegistrar): int $userId = (string) $this->argument('userId'); $guardName = $this->argument('guard'); $userModelClass = $this->argument('userModelNamespace'); + $teamId = $this->option('team-id'); + $hasTeamId = $teamId !== null && $teamId !== ''; - if (! $permissionRegistrar->teams && $this->option('team-id')) { + if (! $permissionRegistrar->teams && $hasTeamId) { $this->warn('Teams feature disabled, argument --team-id has no effect. Either enable it in permissions config file or remove --team-id parameter'); return self::SUCCESS; @@ -65,7 +67,7 @@ public function handle(PermissionRegistrar $permissionRegistrar): int } $teamIdAux = getPermissionsTeamId(); - setPermissionsTeamId($this->option('team-id') ?: null); + setPermissionsTeamId($hasTeamId ? $teamId : null); $roleClass = $permissionRegistrar->getRoleClass(); diff --git a/src/permission/src/Commands/CreateRoleCommand.php b/src/permission/src/Commands/CreateRoleCommand.php index 3e8243300..95995562f 100644 --- a/src/permission/src/Commands/CreateRoleCommand.php +++ b/src/permission/src/Commands/CreateRoleCommand.php @@ -25,7 +25,10 @@ class CreateRoleCommand extends Command */ public function handle(PermissionRegistrar $permissionRegistrar): int { - if (! $permissionRegistrar->teams && $this->option('team-id')) { + $teamId = $this->option('team-id'); + $hasTeamId = $teamId !== null && $teamId !== ''; + + if (! $permissionRegistrar->teams && $hasTeamId) { $this->warn('Teams feature disabled, argument --team-id has no effect. Either enable it in permissions config file or remove --team-id parameter'); return self::SUCCESS; @@ -36,7 +39,7 @@ public function handle(PermissionRegistrar $permissionRegistrar): int $teamIdAux = getPermissionsTeamId(); try { - setPermissionsTeamId($this->option('team-id') ?: null); + setPermissionsTeamId($hasTeamId ? $teamId : null); $role = $roleClass::findOrCreate((string) $this->argument('name'), is_string($guard) ? $guard : null); } finally { @@ -44,7 +47,7 @@ public function handle(PermissionRegistrar $permissionRegistrar): int } $teamsKey = $permissionRegistrar->teamsKey; - if ($permissionRegistrar->teams && $this->option('team-id') && is_null($role->{$teamsKey})) { + if ($permissionRegistrar->teams && $hasTeamId && is_null($role->{$teamsKey})) { $this->warn("Role `{$role->name}` already exists on the global team; argument --team-id has no effect"); } diff --git a/src/permission/src/Commands/UpgradeForTeamsCommand.php b/src/permission/src/Commands/UpgradeForTeamsCommand.php index e6856206e..87e77dd30 100644 --- a/src/permission/src/Commands/UpgradeForTeamsCommand.php +++ b/src/permission/src/Commands/UpgradeForTeamsCommand.php @@ -51,15 +51,17 @@ public function handle(ConfigRepository $config): int $this->line('Creating migration'); - if ($this->createMigration()) { - $this->info('Migration created successfully.'); - } else { + if (! $this->createMigration()) { $this->error( "Couldn't create migration.\n" . 'Check the write permissions within the database/migrations directory.' ); + + return self::FAILURE; } + $this->info('Migration created successfully.'); + $this->line(''); return self::SUCCESS; @@ -72,11 +74,10 @@ protected function createMigration(): bool { try { $migrationStub = __DIR__ . "/../../database/migrations/{$this->migrationSuffix}.stub"; - copy($migrationStub, $this->getMigrationPath()); - return true; - } catch (Throwable $e) { - $this->error($e->getMessage()); + return copy($migrationStub, $this->getMigrationPath()); + } catch (Throwable $throwable) { + $this->error($throwable->getMessage()); return false; } diff --git a/src/permission/src/PermissionServiceProvider.php b/src/permission/src/PermissionServiceProvider.php index 3528c7616..5b5c1b7ee 100644 --- a/src/permission/src/PermissionServiceProvider.php +++ b/src/permission/src/PermissionServiceProvider.php @@ -200,10 +200,6 @@ protected function registerGateHook(): void */ protected function registerAbout(): void { - if (! class_exists(InstalledVersions::class) || ! class_exists(AboutCommand::class)) { - return; - } - $features = [ 'Teams' => 'teams', 'Wildcard Permissions' => 'enable_wildcard_permission', diff --git a/tests/Composer/PackageManifestConsistencyTest.php b/tests/Composer/PackageManifestConsistencyTest.php index 8033e6e99..6ed44ee74 100644 --- a/tests/Composer/PackageManifestConsistencyTest.php +++ b/tests/Composer/PackageManifestConsistencyTest.php @@ -4,6 +4,7 @@ namespace Hypervel\Tests\Composer; +use Hypervel\Support\DefaultProviders; use Hypervel\Tests\TestCase; use JsonException; @@ -51,6 +52,30 @@ public function testSplitPackageRequirementsAreDeclaredConsistentlyInRootManifes } } + /** + * Ensure split package providers remain discoverable from the root package. + * + * @throws JsonException + */ + public function testSplitPackageProvidersAreDiscoverableFromRootPackage(): void + { + $rootComposer = $this->decodeManifest(__DIR__ . '/../../composer.json'); + $rootProviders = $rootComposer['extra']['hypervel']['providers'] ?? []; + $defaultProviders = (new DefaultProviders)->toArray(); + + foreach ($this->splitManifests() as $manifest) { + $composer = $this->decodeManifest($manifest); + + foreach ($composer['extra']['hypervel']['providers'] ?? [] as $provider) { + $this->assertTrue( + in_array($provider, $rootProviders, true) + || in_array($provider, $defaultProviders, true), + "Split package [{$composer['name']}] provider [{$provider}] is not discoverable from the root package.", + ); + } + } + } + /** * Ensure every declared autoload path exists. * diff --git a/tests/Permission/Commands/CommandTest.php b/tests/Permission/Commands/CommandTest.php index f1e2dc0ef..581854b71 100644 --- a/tests/Permission/Commands/CommandTest.php +++ b/tests/Permission/Commands/CommandTest.php @@ -4,9 +4,7 @@ namespace Hypervel\Tests\Permission\Commands; -use Composer\InstalledVersions; use Hypervel\Database\Eloquent\Model; -use Hypervel\Foundation\Console\AboutCommand; use Hypervel\Permission\Models\Permission; use Hypervel\Permission\Models\Role; use Hypervel\Permission\PermissionRegistrar; @@ -156,12 +154,15 @@ public function testItCanSetupTeamsUpgrade(): void } } - public function testItCanRespondToAboutCommandWithDefaultFeatures(): void + public function testSetupTeamsFailsWhenTeamsAreDisabled(): void { - if (! class_exists(InstalledVersions::class) || ! method_exists(AboutCommand::class, 'flushState')) { - $this->markTestSkipped('About command package metadata is unavailable in this environment.'); - } + $this->artisan('permission:setup-teams') + ->expectsOutputToContain('Teams feature is disabled') + ->assertFailed(); + } + public function testItCanRespondToAboutCommandWithDefaultFeatures(): void + { $this->app->make(PermissionRegistrar::class)->initializeCache(); Artisan::call('about'); @@ -172,10 +173,6 @@ public function testItCanRespondToAboutCommandWithDefaultFeatures(): void public function testItCanRespondToAboutCommandWithTeams(): void { - if (! class_exists(InstalledVersions::class) || ! method_exists(AboutCommand::class, 'flushState')) { - $this->markTestSkipped('About command package metadata is unavailable in this environment.'); - } - $this->app->make('config')->set('permission.teams', true); $this->app->make(PermissionRegistrar::class)->initializeCache(); @@ -249,9 +246,20 @@ public function testItWarnsWhenAssigningRoleWithTeamIdButTeamsDisabled(): void 'name' => 'testRole', 'userId' => (string) $user->id, 'userModelNamespace' => User::class, - '--team-id' => 1, + '--team-id' => '0', + ]); + + $this->assertStringContainsString('Teams feature disabled', Artisan::output()); + } + + public function testItWarnsWhenCreatingRoleWithTeamIdButTeamsDisabled(): void + { + Artisan::call('permission:create-role', [ + 'name' => 'zero-team-role', + '--team-id' => '0', ]); $this->assertStringContainsString('Teams feature disabled', Artisan::output()); + $this->assertDatabaseMissing('roles', ['name' => 'zero-team-role']); } } diff --git a/tests/Permission/Commands/TeamCommandTest.php b/tests/Permission/Commands/TeamCommandTest.php index 4dc665ee9..5d3ce5afe 100644 --- a/tests/Permission/Commands/TeamCommandTest.php +++ b/tests/Permission/Commands/TeamCommandTest.php @@ -5,12 +5,14 @@ namespace Hypervel\Tests\Permission\Commands; use Hypervel\Contracts\Foundation\Application as ApplicationContract; +use Hypervel\Permission\Commands\UpgradeForTeamsCommand; use Hypervel\Permission\Models\Role; use Hypervel\Permission\PermissionRegistrar; use Hypervel\Support\Facades\Artisan; use Hypervel\Tests\Permission\Fixtures\Models\Team; use Hypervel\Tests\Permission\Fixtures\Models\User; use Hypervel\Tests\Permission\TestCase; +use Symfony\Component\Console\Tester\CommandTester; class TeamCommandTest extends TestCase { @@ -90,6 +92,54 @@ public function testItRestoresPreviousTeamIdAfterAssigningRole(): void $this->assertSame(5, getPermissionsTeamId()); } + public function testItPreservesZeroAsAnExplicitTeamId(): void + { + $user = User::query()->firstOrFail(); + setPermissionsTeamId(5); + + Artisan::call('permission:create-role', [ + 'name' => 'zero-team-role', + '--team-id' => '0', + ]); + + $this->assertSame(5, getPermissionsTeamId()); + $this->assertDatabaseHas('roles', [ + 'name' => 'zero-team-role', + 'team_test_id' => 0, + ]); + + Artisan::call('permission:assign-role', [ + 'name' => 'zero-team-role', + 'userId' => (string) $user->getKey(), + 'guard' => 'web', + 'userModelNamespace' => User::class, + '--team-id' => '0', + ]); + + $this->assertSame(5, getPermissionsTeamId()); + + setPermissionsTeamId('0'); + + $this->assertTrue($user->fresh()->hasRole('zero-team-role')); + } + + public function testItWarnsWhenATeamIdResolvesAnExistingGlobalRole(): void + { + setPermissionsTeamId(null); + Role::create(['name' => 'global-role']); + + Artisan::call('permission:create-role', [ + 'name' => 'global-role', + '--team-id' => 1, + ]); + + $this->assertStringContainsString( + 'already exists on the global team; argument --team-id has no effect', + Artisan::output(), + ); + $this->assertSame(1, Role::query()->where('name', 'global-role')->count()); + } + public function testItCanCreateTeamsMigration(): void { $before = glob(database_path('migrations/*_add_teams_fields.php')) ?: []; @@ -110,6 +160,36 @@ public function testItCanCreateTeamsMigration(): void } } + public function testItWarnsAboutAnExistingMigrationBeforeADeclinedSetup(): void + { + $migration = database_path('migrations/2020_01_01_000000_add_teams_fields.php'); + touch($migration); + + try { + $before = glob(database_path('migrations/*_add_teams_fields.php')) ?: []; + + $this->artisan('permission:setup-teams') + ->expectsOutputToContain('Setup teams migration already exists') + ->expectsConfirmation('Proceed with the migration creation?', 'no') + ->assertSuccessful(); + + $this->assertSame($before, glob(database_path('migrations/*_add_teams_fields.php')) ?: []); + } finally { + unlink($migration); + } + } + + public function testSetupTeamsFailsWhenTheMigrationCannotBeCopied(): void + { + $command = new InvalidDestinationUpgradeForTeamsCommand; + $command->setHypervel($this->app); + $tester = new CommandTester($command); + $tester->setInputs(['yes']); + + $this->assertSame(UpgradeForTeamsCommand::FAILURE, $tester->execute([])); + $this->assertStringContainsString("Couldn't create migration.", $tester->getDisplay()); + } + public function testItCanShowRolesByTeams(): void { $this->app->make(PermissionRegistrar::class)->initializeCache(); @@ -127,3 +207,14 @@ public function testItCanShowRolesByTeams(): void $this->assertMatchesRegularExpression('/\|\s+\|\s+testRole\s+\|\s+testRole_2\s+\|\s+testRole_Team\s+\|\s+testRole_Team\s+\|/', $output); } } + +class InvalidDestinationUpgradeForTeamsCommand extends UpgradeForTeamsCommand +{ + /** + * Return an existing directory as an invalid copy destination. + */ + protected function getMigrationPath(?string $date = null): string + { + return database_path('migrations'); + } +} diff --git a/tests/Permission/PackageMetadataTest.php b/tests/Permission/PackageMetadataTest.php new file mode 100644 index 000000000..8da614cf3 --- /dev/null +++ b/tests/Permission/PackageMetadataTest.php @@ -0,0 +1,46 @@ +assertSame($rootComposer['require'][$dependency], $composer['require'][$dependency]); + } + + $this->assertSame( + [PermissionServiceProvider::class], + $composer['extra']['hypervel']['providers'], + ); + $this->assertContains( + PermissionServiceProvider::class, + $rootComposer['extra']['hypervel']['providers'], + ); + } +} From b65067912bb505f1a8b81b9152d5e2816ec120c1 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:45:49 +0000 Subject: [PATCH 04/10] Port current Permission regression coverage Cover guard resolution without a provider, LDAP-backed model discovery, missing Passport clients, model-valued team identifiers, missing role IDs, exact pipe-delimited role checks, current-team reverse assignments, invalid wildcard implementations, and blank wildcard subparts. These are supported current Spatie behaviors that Hypervel already implements; the tests make that compatibility executable without adding production seams or compatibility machinery. --- tests/Permission/GuardTest.php | 39 +++++++++++++++++++ .../Integration/PermissionRegistrarTest.php | 9 +++++ tests/Permission/Models/RoleTest.php | 7 ++++ tests/Permission/Traits/HasRolesTest.php | 8 ++++ .../Traits/TeamHasAssignedModelsTest.php | 13 +++++++ .../Traits/WildcardHasPermissionsTest.php | 21 ++++++++++ 6 files changed, 97 insertions(+) diff --git a/tests/Permission/GuardTest.php b/tests/Permission/GuardTest.php index 55fb6b4cb..53bd97fbb 100644 --- a/tests/Permission/GuardTest.php +++ b/tests/Permission/GuardTest.php @@ -4,6 +4,7 @@ namespace Hypervel\Tests\Permission; +use Hypervel\Contracts\Auth\Guard as GuardContract; use Hypervel\Database\Eloquent\MissingAttributeException; use Hypervel\Database\Eloquent\Model; use Hypervel\Permission\Guard; @@ -13,10 +14,48 @@ use Hypervel\Tests\Permission\Fixtures\Models\Admin; use Hypervel\Tests\Permission\Fixtures\Models\User; use Hypervel\Tests\Permission\Fixtures\PassportGuard; +use Mockery as m; use PHPUnit\Framework\Attributes\DataProvider; class GuardTest extends TestCase { + public function testItReturnsNullForAGuardWithoutAProvider(): void + { + $this->app->make('config')->set('auth.guards.no-provider-guard', []); + + $this->assertNull(Guard::getModelForGuard('no-provider-guard')); + } + + public function testItResolvesTheModelForAnLdapProvider(): void + { + $this->app->make('config')->set([ + 'auth.guards.ldap-guard' => ['provider' => 'ldap-provider'], + 'auth.providers.ldap-provider' => [ + 'driver' => 'ldap', + 'database' => ['model' => User::class], + ], + ]); + + $this->assertSame(User::class, Guard::getModelForGuard('ldap-guard')); + } + + public function testItReturnsNullWhenNoPassportGuardIsConfigured(): void + { + $this->assertNull(Guard::getPassportClient('web')); + } + + public function testItReturnsNullWhenThePassportGuardHasNoClientSurface(): void + { + $this->app->make('config')->set( + 'auth.guards.fake-passport', + ['driver' => 'passport', 'provider' => 'users'], + ); + + Auth::shouldReceive('guard')->once()->with('fake-passport')->andReturn(m::mock(GuardContract::class)); + + $this->assertNull(Guard::getPassportClient('web')); + } + public function testZeroGuardNamesRemainStringIdentifiers(): void { $user = new User; diff --git a/tests/Permission/Integration/PermissionRegistrarTest.php b/tests/Permission/Integration/PermissionRegistrarTest.php index a137d2a08..6a564d497 100644 --- a/tests/Permission/Integration/PermissionRegistrarTest.php +++ b/tests/Permission/Integration/PermissionRegistrarTest.php @@ -122,6 +122,15 @@ public function testItCanChangeTeamId(): void $this->assertSame($teamId, $this->app->make(PermissionRegistrar::class)->getPermissionsTeamId()); } + public function testItCanChangeTeamIdUsingAModelInstance(): void + { + $registrar = $this->app->make(PermissionRegistrar::class); + + $registrar->setPermissionsTeamId($this->testUser); + + $this->assertSame($this->testUser->getKey(), $registrar->getPermissionsTeamId()); + } + public function testPermissionLookupUsesGuardExactCatalogIndex(): void { $permissionClass = $this->app->make(PermissionContract::class); diff --git a/tests/Permission/Models/RoleTest.php b/tests/Permission/Models/RoleTest.php index 4d98e1bee..700bcb53f 100644 --- a/tests/Permission/Models/RoleTest.php +++ b/tests/Permission/Models/RoleTest.php @@ -291,6 +291,13 @@ public static function roleNameProvider(): array ]; } + public function testItThrowsAnExceptionWhenARoleWithTheGivenIdDoesNotExist(): void + { + $this->expectException(RoleDoesNotExist::class); + + $this->app->make(Role::class)::findById(456789, 'web'); + } + public function testItBelongsToAGuard(): void { $role = $this->app->make(Role::class)->create(['name' => 'admin', 'guard_name' => 'admin']); diff --git a/tests/Permission/Traits/HasRolesTest.php b/tests/Permission/Traits/HasRolesTest.php index a7cfa30c8..5ee01ae17 100644 --- a/tests/Permission/Traits/HasRolesTest.php +++ b/tests/Permission/Traits/HasRolesTest.php @@ -185,6 +185,14 @@ public function testMalformedQuotedPipeRoleStringDoesNotTrimLeadingQuote(): void $this->assertFalse($this->testUser->hasRole('"admin|editor')); } + public function testItCanCheckExactRolesUsingAPipeDelimitedString(): void + { + $this->testUser->assignRole('testRole', 'testRole2'); + + $this->assertTrue($this->testUser->hasExactRoles('testRole|testRole2')); + $this->assertFalse($this->testUser->hasExactRoles('testRole|testRole2|testRole3')); + } + public function testItCanAssignAndRemoveMultipleRolesAtOnce(): void { $this->testUser->assignRole($this->testUserRole->getKey(), 'testRole2'); diff --git a/tests/Permission/Traits/TeamHasAssignedModelsTest.php b/tests/Permission/Traits/TeamHasAssignedModelsTest.php index f1b33f091..6d7401c79 100644 --- a/tests/Permission/Traits/TeamHasAssignedModelsTest.php +++ b/tests/Permission/Traits/TeamHasAssignedModelsTest.php @@ -24,6 +24,19 @@ protected function setUpInCoroutine(): void $this->setUpTeams(); } + public function testItAppliesTheCurrentTeamIdWhenAssigningModels(): void + { + $user = User::create(['email' => 'team-user@test.com']); + + $this->testUserRole->assignToModels($user); + + $pivot = DB::table(Config::modelHasRolesTable()) + ->where(Config::morphKey(), $user->getKey()) + ->first(); + + $this->assertSame(1, (int) $pivot->team_test_id); + } + public function testItAssignsModelsInCurrentTeamWhenModelAlreadyHasRoleInAnotherTeam(): void { $user = User::create(['email' => 'user1@test.com']); diff --git a/tests/Permission/Traits/WildcardHasPermissionsTest.php b/tests/Permission/Traits/WildcardHasPermissionsTest.php index 0c3b2494f..3508e8df8 100644 --- a/tests/Permission/Traits/WildcardHasPermissionsTest.php +++ b/tests/Permission/Traits/WildcardHasPermissionsTest.php @@ -6,6 +6,7 @@ use Hypervel\Permission\Exceptions\PermissionDoesNotExist; use Hypervel\Permission\Exceptions\WildcardPermissionInvalidArgument; +use Hypervel\Permission\Exceptions\WildcardPermissionNotImplementsContract; use Hypervel\Permission\Exceptions\WildcardPermissionNotProperlyFormatted; use Hypervel\Permission\Models\Permission; use Hypervel\Tests\Permission\Fixtures\Models\TestRolePermissionsEnum; @@ -208,6 +209,26 @@ public function testItThrowsExceptionWhenWildcardPermissionIsNotProperlyFormatte $user->hasPermissionTo('invoices.*'); } + public function testItThrowsExceptionWhenWildcardPermissionClassDoesNotImplementContract(): void + { + $this->app->make('config')->set('permission.wildcard_permission', User::class); + $this->flushPermissionState(); + + $user = User::create(['email' => 'user1@test.com']); + + $this->expectException(WildcardPermissionNotImplementsContract::class); + $user->hasPermissionTo('posts.create'); + } + + public function testItThrowsExceptionWhenACommaSeparatedWildcardSubpartIsBlank(): void + { + $user = User::create(['email' => 'user1@test.com']); + $user->givePermissionTo(Permission::create(['name' => 'articles,,edit'])); + + $this->expectException(WildcardPermissionNotProperlyFormatted::class); + $user->hasPermissionTo('articles.edit'); + } + public function testItCanVerifyPermissionInstancesNotAssignedToUser(): void { $user = User::create(['email' => 'user@test.com']); From e20683bf70c4b074d2988ad8ecd1efe26ea10de9 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:46:10 +0000 Subject: [PATCH 05/10] Declare missing split package dependencies Add the direct Symfony HttpKernel, Carbon, and Composer runtime requirements already used by Broadcasting, Concurrency, Contracts, DI, Notifications, Passkeys, Process, and Telescope. Pin each split manifest to the root constraint with focused metadata tests so subtree packages remain independently installable and dependency drift fails in CI. --- src/broadcasting/composer.json | 3 +- src/concurrency/composer.json | 3 +- src/contracts/composer.json | 4 ++- src/di/composer.json | 1 + src/notifications/composer.json | 3 +- src/passkeys/composer.json | 2 ++ src/process/composer.json | 1 + src/telescope/composer.json | 4 ++- tests/Broadcasting/PackageMetadataTest.php | 1 + tests/Concurrency/PackageMetadataTest.php | 34 +++++++++++++++++++ tests/Contracts/PackageMetadataTest.php | 12 +++++-- tests/Di/PackageMetadataTest.php | 37 +++++++++++++++++++++ tests/Notifications/PackageMetadataTest.php | 1 + tests/Passkeys/PackageMetadataTest.php | 36 ++++++++++++++++++++ tests/Process/PackageMetadataTest.php | 34 +++++++++++++++++++ tests/Telescope/PackageMetadataTest.php | 36 ++++++++++++++++++++ 16 files changed, 204 insertions(+), 8 deletions(-) create mode 100644 tests/Concurrency/PackageMetadataTest.php create mode 100644 tests/Di/PackageMetadataTest.php create mode 100644 tests/Passkeys/PackageMetadataTest.php create mode 100644 tests/Process/PackageMetadataTest.php create mode 100644 tests/Telescope/PackageMetadataTest.php diff --git a/src/broadcasting/composer.json b/src/broadcasting/composer.json index 7f570a935..6eb337be6 100644 --- a/src/broadcasting/composer.json +++ b/src/broadcasting/composer.json @@ -41,7 +41,8 @@ "hypervel/queue": "^0.4", "hypervel/routing": "^0.4", "hypervel/support": "^0.4", - "psr/log": "^3.0" + "psr/log": "^3.0", + "symfony/http-kernel": "^8.1" }, "suggest": { "ext-hash": "Required to use the Ably and Pusher broadcast drivers.", diff --git a/src/concurrency/composer.json b/src/concurrency/composer.json index afd6f907e..5b5fe055a 100644 --- a/src/concurrency/composer.json +++ b/src/concurrency/composer.json @@ -38,7 +38,8 @@ "hypervel/contracts": "^0.4", "hypervel/coroutine": "^0.4", "hypervel/process": "^0.4", - "hypervel/support": "^0.4" + "hypervel/support": "^0.4", + "nesbot/carbon": "^3.13.1" }, "config": { "sort-packages": true diff --git a/src/contracts/composer.json b/src/contracts/composer.json index 25547b934..3af34d163 100644 --- a/src/contracts/composer.json +++ b/src/contracts/composer.json @@ -31,11 +31,13 @@ "require": { "php": "^8.4", "monolog/monolog": "^3.1", + "nesbot/carbon": "^3.13.1", "psr/container": "^2.0.1", "psr/log": "^3.0", "psr/simple-cache": "^3.0", "symfony/console": "^8.1", - "symfony/http-foundation": "^8.1" + "symfony/http-foundation": "^8.1", + "symfony/http-kernel": "^8.1" }, "config": { "sort-packages": true diff --git a/src/di/composer.json b/src/di/composer.json index eb96a2351..7b61f2af3 100644 --- a/src/di/composer.json +++ b/src/di/composer.json @@ -33,6 +33,7 @@ }, "require": { "php": "^8.4", + "composer-runtime-api": "^2.2", "hypervel/collections": "^0.4", "hypervel/container": "^0.4", "hypervel/filesystem": "^0.4", diff --git a/src/notifications/composer.json b/src/notifications/composer.json index 20d134ce3..dc34ff3f1 100644 --- a/src/notifications/composer.json +++ b/src/notifications/composer.json @@ -48,7 +48,8 @@ "hypervel/macroable": "^0.4", "hypervel/mail": "^0.4", "hypervel/queue": "^0.4", - "hypervel/support": "^0.4" + "hypervel/support": "^0.4", + "nesbot/carbon": "^3.13.1" }, "config": { "sort-packages": true diff --git a/src/passkeys/composer.json b/src/passkeys/composer.json index 260f2486f..a393c9ce8 100644 --- a/src/passkeys/composer.json +++ b/src/passkeys/composer.json @@ -32,9 +32,11 @@ "hypervel/session": "^0.4", "hypervel/support": "^0.4", "hypervel/validation": "^0.4", + "nesbot/carbon": "^3.13.1", "paragonie/constant_time_encoding": "^3.1", "symfony/console": "^8.1", "symfony/http-foundation": "^8.1", + "symfony/http-kernel": "^8.1", "symfony/serializer": "^8.1", "web-auth/cose-lib": "^4.5", "web-auth/webauthn-lib": "^5.3" diff --git a/src/process/composer.json b/src/process/composer.json index e805392d7..f22f90db8 100644 --- a/src/process/composer.json +++ b/src/process/composer.json @@ -29,6 +29,7 @@ "hypervel/contracts": "^0.4", "hypervel/macroable": "^0.4", "hypervel/support": "^0.4", + "nesbot/carbon": "^3.13.1", "symfony/process": "^8.1" }, "autoload": { diff --git a/src/telescope/composer.json b/src/telescope/composer.json index dfa7561c4..f653a8ada 100644 --- a/src/telescope/composer.json +++ b/src/telescope/composer.json @@ -47,8 +47,10 @@ "hypervel/server": "^0.4", "hypervel/support": "^0.4", "hypervel/view": "^0.4", + "nesbot/carbon": "^3.13.1", "symfony/console": "^8.1", - "symfony/http-foundation": "^8.1" + "symfony/http-foundation": "^8.1", + "symfony/http-kernel": "^8.1" }, "autoload": { "psr-4": { diff --git a/tests/Broadcasting/PackageMetadataTest.php b/tests/Broadcasting/PackageMetadataTest.php index 092d621db..ff49b5133 100644 --- a/tests/Broadcasting/PackageMetadataTest.php +++ b/tests/Broadcasting/PackageMetadataTest.php @@ -32,6 +32,7 @@ public function testDependenciesAreDeclared(): void $this->assertSame('^0.4', $composer['require']['hypervel/routing']); $this->assertSame('^3.0', $composer['require']['psr/log']); + $this->assertSame($rootComposer['require']['symfony/http-kernel'], $composer['require']['symfony/http-kernel']); $this->assertArrayNotHasKey('hypervel/auth', $composer['require']); $this->assertArrayNotHasKey('hypervel/cache', $composer['require']); diff --git a/tests/Concurrency/PackageMetadataTest.php b/tests/Concurrency/PackageMetadataTest.php new file mode 100644 index 000000000..d1ff3b1f8 --- /dev/null +++ b/tests/Concurrency/PackageMetadataTest.php @@ -0,0 +1,34 @@ +assertSame($rootComposer['require']['nesbot/carbon'], $composer['require']['nesbot/carbon']); + } +} diff --git a/tests/Contracts/PackageMetadataTest.php b/tests/Contracts/PackageMetadataTest.php index e7ee76684..10dac3156 100644 --- a/tests/Contracts/PackageMetadataTest.php +++ b/tests/Contracts/PackageMetadataTest.php @@ -22,16 +22,22 @@ public function testExternalParentInterfaceDependenciesAreDeclared(): void 512, JSON_THROW_ON_ERROR ); + $rootComposer = json_decode( + file_get_contents(__DIR__ . '/../../composer.json'), + true, + 512, + JSON_THROW_ON_ERROR + ); foreach ([ 'monolog/monolog', + 'nesbot/carbon', 'psr/container', 'psr/log', 'psr/simple-cache', + 'symfony/http-kernel', ] as $dependency) { - $this->assertArrayHasKey($dependency, $composer['require']); - $this->assertIsString($composer['require'][$dependency]); - $this->assertNotSame('', trim($composer['require'][$dependency])); + $this->assertSame($rootComposer['require'][$dependency], $composer['require'][$dependency]); } } } diff --git a/tests/Di/PackageMetadataTest.php b/tests/Di/PackageMetadataTest.php new file mode 100644 index 000000000..8fd1ec744 --- /dev/null +++ b/tests/Di/PackageMetadataTest.php @@ -0,0 +1,37 @@ +assertSame( + $rootComposer['require']['composer-runtime-api'], + $composer['require']['composer-runtime-api'], + ); + } +} diff --git a/tests/Notifications/PackageMetadataTest.php b/tests/Notifications/PackageMetadataTest.php index 14af5df51..0b1f2ecc2 100644 --- a/tests/Notifications/PackageMetadataTest.php +++ b/tests/Notifications/PackageMetadataTest.php @@ -31,6 +31,7 @@ public function testDependenciesAndProviderAreDeclared(): void ); $this->assertSame('*', $composer['require']['ext-mbstring']); + $this->assertSame($rootComposer['require']['nesbot/carbon'], $composer['require']['nesbot/carbon']); foreach (['symfony/console', 'hypervel/conditionable', 'hypervel/macroable'] as $dependency) { $this->assertArrayHasKey($dependency, $composer['require']); diff --git a/tests/Passkeys/PackageMetadataTest.php b/tests/Passkeys/PackageMetadataTest.php new file mode 100644 index 000000000..5e905c7ba --- /dev/null +++ b/tests/Passkeys/PackageMetadataTest.php @@ -0,0 +1,36 @@ +assertSame($rootComposer['require'][$dependency], $composer['require'][$dependency]); + } + } +} diff --git a/tests/Process/PackageMetadataTest.php b/tests/Process/PackageMetadataTest.php new file mode 100644 index 000000000..a36b05e0d --- /dev/null +++ b/tests/Process/PackageMetadataTest.php @@ -0,0 +1,34 @@ +assertSame($rootComposer['require']['nesbot/carbon'], $composer['require']['nesbot/carbon']); + } +} diff --git a/tests/Telescope/PackageMetadataTest.php b/tests/Telescope/PackageMetadataTest.php new file mode 100644 index 000000000..63b9bd4e3 --- /dev/null +++ b/tests/Telescope/PackageMetadataTest.php @@ -0,0 +1,36 @@ +assertSame($rootComposer['require'][$dependency], $composer['require'][$dependency]); + } + } +} From 3c39a3b614a67797bc2a7897f3c87896156d1b16 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:46:25 +0000 Subject: [PATCH 06/10] Document Permission replacement and pivot behavior Define permissions and roles in Laravel-style terms, correct the revocation example, and document saved replacement event payloads, listener gating, transaction ordering, and no-op cache behavior. Show the supported trait-alias custom-pivot extension pattern, explain which model-returning APIs load the real relation, preserve the compact authorization cache contract, and state the reverse arbitrary-model boundary and exact performance costs. --- src/boost/docs/permission.md | 46 +++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) diff --git a/src/boost/docs/permission.md b/src/boost/docs/permission.md index 8be7c0da9..86bcf08c0 100644 --- a/src/boost/docs/permission.md +++ b/src/boost/docs/permission.md @@ -49,6 +49,7 @@ - [Wildcard Permissions](#wildcard-permissions) - [Polymorphic Models](#polymorphic-models) - [Custom Models](#custom-models) + - [Custom Pivot Models](#custom-pivot-models) - [UUID and ULID Keys](#uuid-and-ulid-keys) - [Caching](#caching) - [Testing and Seeding](#testing-and-seeding) @@ -60,7 +61,7 @@ ## Introduction -Hypervel's permission package provides role-based access control for Eloquent models. You may create roles and permissions, assign them to users or other models, and check access by role, direct permission, or permission inherited through a role. +Hypervel's permission package provides role-based access control for Eloquent models. A permission represents one ability, such as `edit articles`. A role is a named group of permissions, such as `editor`. You may assign roles and permissions to users or other models, then check access by role, direct permission, or permission inherited through a role. The package is based on Spatie's `laravel-permission` package and adapted for Hypervel. It also supports denied permissions, which explicitly reject an ability even when the model receives the same permission directly or through a role. @@ -578,7 +579,7 @@ You may remove permissions from a model: ```php $user->revokePermissionTo('edit articles'); -$user->revokePermissionTo('edit articles', 'delete articles'); +$user->revokePermissionTo(['edit articles', 'delete articles']); ``` This removes the assignment edge whether it is currently allowed or denied. @@ -846,7 +847,9 @@ Hypervel\Permission\Events\PermissionDetachedEvent::class; Events are only dispatched when events are enabled and the event dispatcher has listeners for the event class. -Assignment events preserve Spatie's request-oriented payloads. Role attach/detach and Permission attach events contain the collected requested IDs, including already-satisfied or empty requests. `PermissionDetachedEvent` receives the stored Permission model or collection. Role synchronization reports the pre-operation current Role IDs through its detached event and the requested replacement IDs through its attached event; Permission synchronization emits only its requested attached event. +Assignment events preserve Spatie's request-oriented payloads. Role attach and detach events, as well as permission attach events, contain the requested IDs, including already-satisfied or empty requests. A direct permission removal passes the stored Permission model or collection to `PermissionDetachedEvent`. + +Saved permission replacement operations dispatch a complete replacement pair. `PermissionDetachedEvent` receives the direct Permission collection as it existed before the operation, including permissions retained by the replacement. `PermissionAttachedEvent` then receives the requested replacement IDs. A same-set replacement therefore dispatches both events. The detached event is dispatched first, after the transaction succeeds and any affected permission cache has been cleared. A failed transaction dispatches neither event. Unsaved models have no stored collection to detach, so their queued replacement dispatches only the attached event. Assignments made before a subject model is saved are queued on that model and written atomically after save. Their events dispatch synchronously when the assignment method is called, in the caller's established context. The saved callback does not dispatch a duplicate event. @@ -1265,6 +1268,41 @@ After creating custom models, update the permission configuration: ], ``` + +### Custom Pivot Models + +You may use a custom pivot model when permission or role assignments need additional casts, timestamps, events, or other model behavior. Alias the package relationship, then apply your pivot model using Eloquent's `using` method: + +```php +traitPermissions()->using(CustomPermissionPivot::class); + } +} +``` + +You may customize role assignments in the same way by aliasing the `roles` relationship and applying your custom role pivot model. + +When a custom permission pivot is configured, `getDirectPermissions()` and `getAllPermissions()` load the model's relationship so the returned Permission models include your pivot class. Normal authorization checks and `getPermissionNames()` continue using the compact permission cache. + +The reverse `assignToModels`, `removeFromModels`, and `syncModels` methods do not use the assigned model's relationship override. When your custom pivot behavior is required, perform the assignment through the model's `givePermissionTo`, `revokePermissionTo`, `syncPermissions`, `assignRole`, `removeRole`, or `syncRoles` methods. + In an unpartitioned application, models that replace rather than extend the package bases must implement `Hypervel\Permission\Contracts\Role` or `Hypervel\Permission\Contracts\Permission`. Partition-enabled Role and Permission models must extend the package bases so every unscoped Eloquent lifecycle path remains protected. The package's default role and permission models do not use soft deletes, and soft deletes are not recommended for permission models. Roles and permissions are access-control records; deleting one should normally remove its assignments, not leave them waiting to become active again later. @@ -1427,6 +1465,8 @@ Permission checks use cached role and permission data after the first lookup. Mo Warm authorization checks execute no database queries. A cold catalog uses three queries. Enabling row partitioning does not add queries: it adds one bound, indexed predicate to existing SQL and one value to pivot inserts. The partition resolver is an in-memory Context lookup. Exact subject mutations forget exact cache identities, while catalog-wide changes advance only the affected partition's assignment token so older entries expire naturally through the configured TTL. +Saved permission replacements perform one additional relationship query only when assignment events are enabled and a listener is registered for `PermissionDetachedEvent`. When a custom pivot is configured, methods that return Permission models load the relationship once for the model in the current coroutine and reuse it on later calls. Authorization and permission-name checks remain query-free after the permission cache is warm. + If you need to display a model's roles or permissions, eager load the relationships you will render: ```php From 750b111120486caea0bf336d3c2a7db69db79875 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:46:35 +0000 Subject: [PATCH 07/10] Record the completed Permission maintenance Route permission-06 through permission-18, database-29 through database-33, and the related discovery and metadata findings through the core dependency index. Add the final ledger assessment covering lifecycle ownership, custom-pivot semantics, replacement events, no-op cache preservation, differential role sync, performance boundaries, cross-package revalidation, rejected machinery, verification, and independent review status while leaving the later fresh Permission audit optional. --- ...amework-coroutine-state-lifecycle-audit.md | 36 ++++++++++++++++--- ...-coroutine-state-lifecycle-audit-ledger.md | 28 +++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md b/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md index b199d95d3..ca635c004 100644 --- a/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md +++ b/docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md @@ -990,9 +990,9 @@ An exceptionally large shared work unit may receive its own linked detail plan w This compact index routes the completed-work history that must be consulted with the full plan after compaction. Detailed history remains in the [companion ledger](2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md). -- **Active package or work unit:** `inertia`; correctness and SSR lifecycle maintenance is recorded under `Complete Inertia correctness and SSR lifecycle maintenance`; detail plan `2026-08-07-2018-inertia-correctness-ssr-lifecycle-and-current-parity.md`. Current upstream DevTools is the next Inertia work unit. -- **Ledger entries required for the active work:** `Complete Inertia correctness and SSR lifecycle maintenance`. -- **Pending revalidation carried into the active work:** None. Inertia revalidated `support-02`; current upstream DevTools remains separately scoped before the package checklist can be completed. +- **Active package or work unit:** `permission`; targeted correctness, custom-pivot, relation-scope, and metadata maintenance is recorded under `Complete Permission correctness, custom pivots, and relation scope safety`; detail plan `2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md`. The later fresh Permission audit remains open. +- **Ledger entries required for the active work:** `Complete Permission correctness, custom pivots, and relation scope safety`. +- **Pending revalidation carried into the active work:** None. Permission revalidated `support-02`; Database and every corrected metadata sibling were revalidated in the same work unit. Update these three lines when a package starts, completes, or gains a cross-package dependency. Name exact work-unit headings or shared finding IDs from the companion ledger; never use “see recent entries” or require a full-ledger reread. @@ -1053,7 +1053,7 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `queue-11` | `queue` | `events`, `queue`, and `broadcasting` (revalidation complete) | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `queue-11` | | `queue-12` | `bus`, `queue` | `events`, `bus`, `queue`, and `broadcasting` (revalidation complete) | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `queue-12` | | `foundation-01` | `foundation` | `support` and `foundation` (revalidation complete) | `Correct event dispatch, queued-consumer isolation, and queue interoperability`; finding `foundation-01` | -| `support-02` | `support` | `auth` (revalidation complete), `broadcasting` (revalidation complete), `bus` (revalidation complete), `cache` (revalidation complete), `concurrency`, `console` (revalidation complete), `container`, `contracts`, `cookie`, `database` (revalidation complete), `events`, `filesystem` (revalidation complete), `foundation` (revalidation complete), `hashing` (revalidation complete), `horizon` (revalidation complete), `inertia` (revalidation complete), `jwt`, `log`, `mail`, `notifications` (revalidation complete), `permission`, `pipeline`, `queue` (revalidation complete), `redis` (revalidation complete), `reverb` (revalidation complete), `routing` (revalidation complete), `sanctum` (revalidation complete), `scout`, `session` (revalidation complete), `socialite` (revalidation complete), `telescope`, `testbench`; `translation` (revalidation complete); later full remaining consumer audits | `Normalize framework enum identifiers at string boundaries`; finding `support-02`; sibling findings `translation-01` and `reverb-03`; linked detail plan `2026-07-15-0920-framework-enum-identifier-contracts.md` | +| `support-02` | `support` | `auth` (revalidation complete), `broadcasting` (revalidation complete), `bus` (revalidation complete), `cache` (revalidation complete), `concurrency`, `console` (revalidation complete), `container`, `contracts`, `cookie`, `database` (revalidation complete), `events`, `filesystem` (revalidation complete), `foundation` (revalidation complete), `hashing` (revalidation complete), `horizon` (revalidation complete), `inertia` (revalidation complete), `jwt`, `log`, `mail`, `notifications` (revalidation complete), `permission` (revalidation complete), `pipeline`, `queue` (revalidation complete), `redis` (revalidation complete), `reverb` (revalidation complete), `routing` (revalidation complete), `sanctum` (revalidation complete), `scout`, `session` (revalidation complete), `socialite` (revalidation complete), `telescope`, `testbench`; `translation` (revalidation complete); later full remaining consumer audits | `Normalize framework enum identifiers at string boundaries`; finding `support-02`; sibling findings `translation-01` and `reverb-03`; linked detail plan `2026-07-15-0920-framework-enum-identifier-contracts.md` | | `macroable-03` | `macroable` | `cookie`, `log`, and `notifications` (revalidation complete); later full `jwt` audit | `Complete Macroable callable and test-state handling`; finding `macroable-03` | | `auth-01` | `support`, `auth` | `auth` (revalidation complete) | `Correct Support utility boundaries and authentication timing isolation`; finding `auth-01` | | `encryption-03` | `encryption` | `contracts`, `support`, `filesystem`, and `foundation` (revalidation complete) | `Harden encryption rotation, key publication, and global lifecycle state`; finding `encryption-03` | @@ -1146,6 +1146,19 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `permission-03` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Harden Eloquent identity and partial-projection safety`; finding `permission-03` | | `permission-04` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Harden Eloquent identity and partial-projection safety`; finding `permission-04` | | `permission-05` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Harden Eloquent identity and partial-projection safety`; finding `permission-05` | +| `permission-06` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-06` | +| `permission-07` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-07` | +| `permission-08` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-08` | +| `permission-09` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-09` | +| `permission-10` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-10` | +| `permission-11` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-11` | +| `permission-12` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-12` | +| `permission-13` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-13` | +| `permission-14` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-14` | +| `permission-15` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-15` | +| `permission-16` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-16` | +| `permission-17` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-17` | +| `permission-18` | `permission` | `permission` (targeted correction complete); later full `permission` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `permission-18` | | `fortify-02` | `fortify` | `fortify` (revalidation complete) | `Harden Eloquent identity and partial-projection safety`; finding `fortify-02` | | `pagination-01` | `pagination` | `pagination` (revalidation complete) | `Complete Pagination correctness, current parity, and query contracts`; finding `pagination-01` | | `pagination-02` | `pagination` | `pagination` (revalidation complete) | `Complete Pagination correctness, current parity, and query contracts`; finding `pagination-02` | @@ -1171,6 +1184,21 @@ Add one row only for a shared finding or changed lower-level assumption that ano | `database-26` | `database` | `database`, `auth`, and `sanctum` (revalidation complete) | `Complete Sanctum correctness, cache settlement, and current parity`; finding `database-26` | | `database-27` | `database` | `database`, `bus`, `foundation`, `queue`, `events`, `mail`, `notifications`, `broadcasting`, `scout`, and `sanctum` (revalidation complete) | `Complete Sanctum correctness, cache settlement, and current parity`; finding `database-27` | | `database-28` | `database` | `database`, `auth`, and `sanctum` (revalidation complete) | `Complete Sanctum correctness, cache settlement, and current parity`; finding `database-28` | +| `database-29` | `database` | `database` and `permission` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `database-29` | +| `database-30` | `database` | `database` and `permission` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `database-30` | +| `database-31` | `database` | `database` and `permission` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `database-31` | +| `database-32` | `database` | `database` and `permission` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `database-32` | +| `database-33` | `database` | `database` and `permission` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `database-33` | +| `horizon-22` | `horizon` | `horizon` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `horizon-22` | +| `wayfinder-01` | `wayfinder` | `wayfinder` (targeted correction complete); later full `wayfinder` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `wayfinder-01` | +| `broadcasting-17` | `broadcasting` | `broadcasting` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `broadcasting-17` | +| `contracts-12` | `contracts` | `contracts` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `contracts-12` | +| `concurrency-08` | `concurrency` | `concurrency` (targeted correction complete); later full `concurrency` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `concurrency-08` | +| `di-06` | `di` | `di` (targeted correction complete); later full `di` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `di-06` | +| `notifications-21` | `notifications` | `notifications` (revalidation complete) | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `notifications-21` | +| `passkeys-01` | `passkeys` | `passkeys` (targeted correction complete); later full `passkeys` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `passkeys-01` | +| `process-11` | `process` | `process` (targeted correction complete); later full `process` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `process-11` | +| `telescope-04` | `telescope` | `telescope` (targeted correction complete); later full `telescope` audit | `Complete Permission correctness, custom pivots, and relation scope safety`; finding `telescope-04` | | `auth-18` | `auth` | `auth` and `sanctum` (revalidation complete) | `Complete Sanctum correctness, cache settlement, and current parity`; finding `auth-18` | | `api-client-01` | `api-client` | `api-client` (targeted correction complete); later full `api-client` audit | `Complete Pagination correctness, current parity, and query contracts`; finding `api-client-01` | | `database-24` | `database` | `database` and `pagination` (revalidation complete) | `Complete Pagination correctness, current parity, and query contracts`; finding `database-24` | diff --git a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md index 01c35b820..750287678 100644 --- a/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md +++ b/docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md @@ -2015,3 +2015,31 @@ Append package entries in checklist order. Keep each entry compact but complete - **Laravel-facing result:** Supported Fortify names, signatures, named arguments, construction, response, property, and protected extension APIs are compatible or restored. Deliberate Hypervel differences remain the current-guard authority, private typed worker-static configuration, OTPHP/chillerlan implementations, integrated Passkeys, and worker-safe lifecycle boundaries. - **Validation and review:** Focused API, route, installer, and complete Fortify coverage is green. The authoritative `composer fix` gate, final stale-state and caller/callee review, and independent code review passed with no remaining finding. - **Assessment:** The implementation restores current parity and one-source configuration ownership without a compatibility shim, speculative abstraction, hot-path synchronization, or meaningful performance regression. + +### Complete Permission correctness, custom pivots, and relation scope safety + +- **Status and inspected surface:** Implementation, focused validation, the authoritative gate, fresh self-review, and independent code review are complete. This targeted work covered every reported Permission finding plus related Database pivot ownership, package discovery, split metadata, current supported Spatie Permission behavior, public documentation, and carried `permission-01` through `permission-05` and `support-02`. Permission remains open in the package checklist because a later fresh source-wide audit is still optional. The detailed design is recorded in [`2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md`](2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md). + +| Findings | Final decision | +|---|---| +| `permission-06` | Restore saved replacement events with the complete pre-operation detached collection and requested attached IDs, while preserving transaction ordering and listener-gated hydration. | +| `permission-07`, `permission-13` | Honor the documented `using(CustomPivot::class)` relation override, preserve custom casts/events, use real relations only for model-returning APIs, and build complete scope-safe stock `MorphPivot` instances from the warm assignment cache. | +| `permission-08`, `permission-14` | Preserve exact team identifiers including `"0"`, restore prior team context, and return truthful setup-command status when migration publication fails. | +| `permission-09`, `permission-10` | Restore root package discovery and declare Permission's direct split-package runtime dependencies. | +| `permission-11`, `permission-12` | Complete Laravel-style public guidance and port only unique, supported current Spatie regression coverage. | +| `permission-15` | Remove unused queued-mutation comparison results while preserving queue identity and persistence behavior. | +| `permission-16` | Complete cached `Permission::roles` pivots and return fresh upstream-oriented via-role pivots so public accessors and scoped writes work without aliasing coroutine-cached authorization edges. | +| `permission-17` | Preserve warm catalog and model caches when permission replacement produces no attached, detached, or updated edge while retaining unconditional replacement events. | +| `permission-18` | Synchronize role replacements by exact edge difference so retained custom pivots keep application state, timestamps, identity, and hooks. | +| `database-29`–`database-33` | Retain and group every destructive pivot predicate, hydrate pivots through one owner, and apply explicit custom-pivot attributes without model mass-assignment filtering while preserving casts, mutators, timestamps, and events. | +| `horizon-22`, `wayfinder-01`, `broadcasting-17`, `contracts-12`, `concurrency-08`, `di-06`, `notifications-21`, `passkeys-01`, `process-11`, `telescope-04` | Correct direct split dependencies or root discovery and pin every correction with executable manifest coverage. | + +- **Architecture and worker ownership:** Permission catalogs and compact assignment records remain worker-cached; assignment caches, selected relation provenance, team identifiers, and partition values remain coroutine-local. `PermissionRegistrar` owns one bounded instance memo of boot-stable pivot classes keyed only by subject class and relation name; it retains no request models and resets with registrar reconfiguration. Warm authorization remains query-free and does not construct relations. No new static state, subscriber entry, request registry, context slot, lock, or unbounded map was added. +- **Correctness and extension parity:** Saved writes resolve the public `roles()` or `permissions()` relation once and use its immutable captured context throughout comparison, mutation, effect synchronization, and invalidation. Stock pivots retain bulk writes; custom pivots use native per-row Eloquent writes only where their casts and hooks require them. Role replacement mutates only attached and detached edge differences, so retained custom pivots are never recreated. Model-returning permission APIs expose the real custom pivot and reuse the loaded relation, while authorization and permission-name checks retain the compact cache. Warm stock pivots now carry keys, related model, morph identity, and exact team/partition constraints. Cached role-permission edges are structurally complete; model-returning via-role APIs receive fresh `Role::permissions`-oriented pivots instead of aliases into the coroutine catalog. Replacement events publish only after successful mutation and invalidate caches only when stored edges changed. +- **Database ownership:** Relation-owned pivot predicates are retained as bounded descriptors and replayed inside one grouped clause so `or` predicates cannot escape parent identity. Composite-key pivot select/save/delete retains relation scope; real primary keys remain authoritative. `wherePivotBetween()` joins the same destructive replay family. Explicit relation attributes use `forceFill()` only inside the existing custom-pivot cast/update paths, matching stock relation semantics without bypassing casts or events. Permission contains no local query-safety workaround. +- **Cross-package revalidation:** `permission-01` through `permission-05` remain correct under the final mutation and cache paths. `support-02` remains correct across Permission's identifiers. Database is revalidated through constrained stock/custom and morph pivot operations. Horizon, Broadcasting, Contracts, and Notifications metadata is revalidated; Wayfinder, Concurrency, DI, Passkeys, Process, and Telescope receive targeted manifest corrections without claiming their later package-wide audits. +- **Important rejected concerns:** Do not add a relation or model registry, partitioned-relation interface, custom-attribute cache, cache invalidation observer, static pivot memo, generic predicate object, reverse-operation relation discovery, delta-only replacement events, Permission-local raw-query enforcement, metadata import scanner, retry path, Octane compatibility hook, or request-scoped registrar. Do not change Laravel's public relationship `orWhere` semantics or attempt to enforce contracts across raw/eventless escape hatches. +- **Performance and complexity:** Ordinary non-wildcard authorization and name checks gain no query, relation construction, event allocation, lock, yield, serializer, or container lookup. Warm role-edge completion is once-per-coroutine catalog hydration; model-returning via-role APIs and the memoized wildcard-index build create one necessary fresh Pivot beside each existing Permission clone. Saved custom-pivot writes pay native per-row work only when an application explicitly configures a custom class; stock pivots retain set-based operations. Saved `assignRole()` builds its selected relation and performs one scoped pivot read limited to requested IDs, including when all requested roles are already assigned; this is the necessary cost of honoring relation overrides. Saved replacement sync performs its existing scoped comparison read, preserves warm caches on no-op results, and adds one separate hydration read only when events are enabled and a targeted detached listener exists. Database stores small predicate arrays only on constrained explicit pivot instances. Command, metadata, documentation, and test changes add no request-path cost. +- **Laravel/Spatie-facing result:** Supported method names, named arguments, relation overrides, event payloads, command options, pivot events, custom casts, and model-returning APIs remain compatible or are restored. Hypervel's denied permissions, partitioning, immutable relation contexts, compact caches, listener gating, and transactional synchronization remain intact. Reverse arbitrary-model operations retain Spatie's boundary and do not consult a subject relation override. +- **Regression and validation:** Counterfactual coverage proves grouped and range pivot scope, custom pivot force-filled casts/events, primary-key behavior, morph isolation, immediate and deferred permission/role assignment, retained custom role pivots, no-op cache preservation, scope-safe warm direct and role-permission pivots, catalog/result isolation, replacement event ordering and failures, exact team identifiers, publication failures, current supported Spatie cases, root discovery, and exact split dependencies. Formatting, both PHPStan configurations, the complete parallel suite, Testbench package mode, dogfood, Composer validation, and focused Permission/Database/metadata suites passed. PHPUnit reports only the acknowledged upstream ParaTest 7.24 worker-flag deprecation; no Hypervel test or source deprecation remains. +- **Assessment:** Every accepted finding is fixed at its lowest owner without a compatibility shim, speculative abstraction, stale path, unintended Laravel/Spatie API break, meaningful read-path regression, unresolved accepted defect, or TODO. From 05ef82bf2320ebf04b5f3a9afedab76795e45d8e Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 11:46:53 +0000 Subject: [PATCH 08/10] Add the Permission correctness implementation plan Record the agreed Database pivot ownership, Permission custom-pivot and replacement-event contracts, warm-pivot construction, command and metadata corrections, supported upstream coverage, documentation scope, rejected overengineering, and complete verification strategy. The plan includes the core audit's exact anti-overengineering guidance and the final permission-06 through permission-18 design so implementation intent and performance boundaries remain durable across future maintenance. --- ...ension-parity-and-relation-scope-safety.md | 622 ++++++++++++++++++ 1 file changed, 622 insertions(+) create mode 100644 docs/plans/2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md diff --git a/docs/plans/2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md b/docs/plans/2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md new file mode 100644 index 000000000..4cbd42019 --- /dev/null +++ b/docs/plans/2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md @@ -0,0 +1,622 @@ +# Permission Correctness, Extension Parity, and Relation Scope Safety + +Status: implementation, authoritative gate, final self-review, and independent code review complete. + +## Scope and outcome + +Complete the reported Permission maintenance against Hypervel's partitioned, denied-permission, coroutine-safe implementation and current Spatie Permission. Restore replacement-event parity, the documented custom-pivot extension pattern, exact team identifiers, package discovery, split-package metadata, current supported upstream coverage, and concise public documentation. Fix the related Eloquent pivot-scope defects at Database, their lowest owner. + +Preserve Hypervel's existing advantages: compact worker-cached permission assignments, zero-query warm authorization checks, immutable partition/team relation contexts, transactional effect synchronization, listener-gated event work, and exact cache invalidation. This is targeted maintenance, not the later fresh package-wide audit; Permission remains unchecked in the core package checklist. + +References checked for this design: + +- Hypervel baseline 0848a9c05cff9dcf15af0ce2a2b3722bda3981ad; +- every reported Permission source, test, metadata, command, provider, cache, relation, and Boost-documentation surface; +- current Spatie Permission afd24018f68306e8b43ace487c107a59af1be776, including the custom-pivot, sync-event, command, guard, cache, role, permission, and wildcard tests; +- Hypervel BelongsToMany, MorphToMany, InteractsWithPivotTable, Pivot, MorphPivot, their event suites, and the corresponding current Laravel implementations; +- root and split Composer manifests, DefaultProviders, package-discovery tests, and existing package metadata contracts; +- carried support-02 and permission-01 through permission-05. + +## What this audit is not + +The following wording is retained verbatim from the core audit plan. It includes the complete “What this audit is not” section plus principles 7–10 because those principles govern hot-path quality, superseded design, remediation choice, and speculative complexity; principles 1–6 govern the broader audit procedure and remain in the core plan. + +This audit is not permission to add defensive machinery for every imaginable failure. Do not add an abstraction, state machine, retry loop, configurable timeout, registry, mutex, context slot, cache, or compatibility API merely because it sounds robust. + +Complexity must pay for itself with at least one of: + +- a demonstrated failure; +- a complete source trace proving a realistic vulnerable schedule; +- a clear general capability with real consumers and owner approval; +- deletion of greater or riskier complexity elsewhere. + +Typical Laravel lifecycle semantics define the supported contract. A package that intentionally relies on model events, middleware, listeners, transactions, or another documented mechanism is not defective merely because userland can explicitly bypass that mechanism. Do not build a parallel enforcement path for `withoutEvents()`, raw database writes, disabled middleware, direct transport access, or comparable deliberate bypasses unless the public contract explicitly promises behavior through that bypass. + +Underengineering is equally a failure. Fix every verified defect completely at its lowest owning boundary, never with a partial fix or a local patch over a broken shared contract, and always surface meaningful evidence-backed improvements rather than dropping them to avoid effort. Restraint applies to speculative machinery and cosmetic change, not to complete fixes or worthwhile opportunities. + +Do not treat an upstream difference as a bug without tracing it. Do not treat upstream parity as proof of correctness. A real Hypervel defect remains a defect when Laravel, Hyperf, Symfony, or an SDK has the same hole. + +The audit categories are discovery lenses, not boundaries around what may be corrected. Any genuine issue discovered while auditing, implementing, testing, or reviewing must be investigated, assigned to its lowest owning boundary, and taken through the applicable consensus, implementation, validation, review, and approval workflow—even when it is outside the current package, initial taxonomy, or changed diff. Do not dismiss a verified issue as unrelated or defer it merely to preserve package order. This rule applies only after the evidence threshold is met; it does not turn speculative concerns, deliberate bypasses, unsupported use, or contract violations into work. + +#### 7. Preserve hot-path quality + +For every fix, inspect: + +- additional allocations; +- container or facade resolutions; +- locking and atomics; +- hashing and serialization; +- new yields or sleeps; +- retries and polling; +- logging or exception construction; +- retained worker memory; +- cache invalidation and eviction. + +A correctness guard on a cold failure path has a different cost from a new lock or resolver on every request. State the difference explicitly. + +Any proposed change with a measured or source-proven hot-path regression requires explicit owner approval before implementation, even when it fixes a defect. Present the expected frequency and magnitude, the evidence, and the viable alternatives. Do not hide an unavoidable tradeoff inside a general correctness claim. + +Performance improvements must provide a meaningful practical benefit after accounting for code complexity and divergence from upstream. Measure representative behavior where practical. Always surface an evidence-backed opportunity to the owner, but do not implement it without approval; a micro-optimization within measurement noise is neither a reason to diverge nor an actionable finding. + +#### 8. Remove superseded design completely + +When a fix changes the owning model, delete obsolete helpers, callbacks, properties, config keys, comments, tests, and documentation. Do not leave a compatibility path or comment describing behavior that no longer exists. Preserve intentional upstream comments unless the new design makes them incorrect. + +#### 9. Treat remediation patterns as candidates + +The established patterns later in this plan are a vocabulary, not a lookup table. Choose among per-call parameters, immutable values, scoped bindings, cloning, CoroutineContext, factories, explicit ownership, static reset, or resource teardown only after proving the real lifetime and owner. + +#### 10. Reject speculative complexity + +Record low-confidence concerns under rejected or unresolved analysis. Do not implement them. Surface every evidence-backed, meaningful non-defect improvement to the owner with its benefit, cost, and alternatives, then stop for explicit approval. This requirement exists to keep worthwhile opportunities visible, not to discourage finding them. + +## Final design + +### 1. Keep all destructive pivot predicates inside the relation identity + +Database owns three live relation defects: + +- custom-pivot detach/update selects a scoped row, then Pivot::delete() or save() drops the relation predicates; +- newPivotQuery() replays non-leading or predicates without grouping, allowing SQL precedence to escape the parent identity; +- wherePivotBetween(), including its or/not variants, is applied to reads but never recorded for destructive writes. + +BelongsToMany will record a fourth predicate family: + +~~~php +protected array $pivotWhereBetweens = []; + +public function wherePivotBetween( + mixed $column, + array $values, + string $boolean = 'and', + bool $not = false, +): static { + $this->pivotWhereBetweens[] = func_get_args(); + + return $this->whereBetween( + $this->qualifyPivotColumn($column), + $values, + $boolean, + $not, + ); +} +~~~ + +Both newPivotQuery() and hydrated pivot key queries will replay all four families inside one nested where group. Parent, related, primary, and morph identity clauses remain outside that group: + +~~~php +$query->where(function (QueryBuilder $query): void { + foreach ($pivotWheres as $arguments) { + $query->where(...$arguments); + } + + foreach ($pivotWhereIns as $arguments) { + $query->whereIn(...$arguments); + } + + foreach ($pivotWhereNulls as $arguments) { + $query->whereNull(...$arguments); + } + + foreach ($pivotWhereBetweens as $arguments) { + $query->whereBetween(...$arguments); + } +}); +~~~ + +This produces identity AND (recorded predicate OR predicate), never identity OR predicate. + +AsPivot will hold one nullable set of relation predicate descriptors. BelongsToMany::newPivot() and MorphToMany::newPivot() attach it whenever any replayable relation constraint exists, regardless of whether the pivot class is stock or custom: + +~~~php +public function setPivotConstraints( + array $wheres, + array $whereIns, + array $whereNulls, + array $whereBetweens, +): static; + +$pivot->setPivotConstraints( + wheres: $this->pivotWheres, + whereIns: $this->pivotWhereIns, + whereNulls: $this->pivotWhereNulls, + whereBetweens: $this->pivotWhereBetweens, +); +~~~ + +The pivot applies the grouped predicates from the composite foreign/related-key branches of setKeysForSelectQuery(), setKeysForSaveQuery(), and getDeleteQuery(). A pivot table with a real primary key keeps the exact parent save/delete path: its unique key already identifies one row, and appending the old relation scope would incorrectly prevent an intentional constrained-column change. Unconstrained pivots keep null state and add only one predictable null guard on explicit composite-key pivot save/delete/select operations; ordinary relation reads and authorization do not enter this path. MorphPivot's composite-key delete branch must retain the grouped predicates before adding morph identity. + +InteractsWithPivotTable::getCurrentlyAttachedPivotsForIds() will hydrate through newExistingPivot() instead of duplicating fromRawAttributes(), key, and related-model setup. The MorphToMany post-map becomes dead and is removed; newPivot() remains the sole owner of class, keys, related model, morph metadata, and constraint descriptors. + +For stock pivots, this also aligns explicit current-pivot hydration with ordinary relation hydration: both pass raw database rows through newExistingPivot() and the model's attribute/date normalization. Custom pivots remain byte-identical because Model::newPivot() continues constructing a configured using() class through fromRawAttributes(). + +The same shared owner has two mass-assignment defects on explicit custom-pivot writes. updateExistingPivotUsingCustomClass() calls fill(), and castAttributes() does the same while preparing attach, sync, and toggle records. A restrictive guarded/fillable policy can silently discard requested values; a totally guarded pivot or strict model mode instead throws MassAssignmentException. Stock relation writes apply no mass-assignment policy. + +Replace both calls with forceFill(). castAttributes() remains required: a using() class is built through fromRawAttributes(), so this is its only cast-on-write pass. forceFill() runs the same setAttribute() loop and preserves casts, mutators, timestamps, and model events without treating developer-authored relation attributes as request mass assignment: + +~~~php +$updated = $pivot ? $pivot->forceFill($attributes)->isDirty() : false; + +$attributes = $this->newPivot()->forceFill($attributes)->getAttributes(); +~~~ + +This is coroutine-safe in Hypervel: forceFill() uses GuardsAttributes::unguarded(), whose flag is stored in CoroutineContext and restored in finally. It does not open Laravel's process-global unguarded window across sibling requests. + +Do not alter the public read-query rule that an ungrouped orWherePivot() can escape a relationship, matching Laravel's general relationship-orWhere contract. This correction only prevents destructive relation operations and public pivot-instance writes from losing identity while replaying relation-owned constraints. + +Durable findings: database-29 (constrained hydrated/custom pivot writes), database-30 (grouped or replay), database-31 (wherePivotBetween write scope), database-32 (custom-pivot explicit updates bypass mass-assignment filtering), database-33 (custom-pivot attach/sync attributes bypass mass-assignment filtering). + +### 2. Honor Permission's public custom-pivot extension without slowing authorization + +Spatie's supported extension shape aliases the trait relation and appends using(): + +~~~php +use HasPermissions { + permissions as traitPermissions; +} + +public function permissions(): BelongsToMany +{ + return $this->traitPermissions()->using(CustomPermissionPivot::class); +} +~~~ + +Database-33 is a prerequisite for this extension contract: Permission's partitioned attach formatter delegates to the shared castAttributes() path, so guarded custom-pivot attributes must survive there before Permission can honor using() on attachPermissions(), syncPermissions(), assignRole(), and syncRoles(). + +Keep empty and unsaved early returns free of relation construction. Normalize inputs and make pre-write decisions with the ambient assignment context. On a saved path that reaches database comparison or mutation, resolve the public roles()/permissions() relation once at the point it is first needed, then pass that selected relation through the remaining comparison, attach, detach, effect-update, and cache-invalidation work. Its captured PermissionRelationContext becomes authoritative from that boundary onward. Saved assignRole() therefore builds the selected relation and runs one scoped pivot read, limited to the requested IDs, before its already-assigned return; this is required to compare the overridden relation's real rows rather than the compact warm cache. + +~~~php +$relation = $this->permissions(); +$context = $this->permissionRelationContext($relation); +~~~ + +EnforcesPermissionPartition exposes the captured immutable value without allowing mutation: + +~~~php +public function getPermissionRelationContext(): PermissionRelationContext +{ + return $this->permissionRelationContext; +} +~~~ + +HasPermissions owns one reused narrowing helper rather than adding an internal interface or repeating annotations at every operation: + +~~~php +protected function permissionRelationContext(BelongsToMany $relation): PermissionRelationContext +{ + /** @var PartitionedBelongsToMany|PartitionedMorphToMany $relation */ + return $relation->getPermissionRelationContext(); +} +~~~ + +Keep the native parameter typed BelongsToMany and the union as an inline @var inside the helper. Do not promote the union to @param: callers correctly hold the public BelongsToMany return type, and narrowing the parameter would make every call a static-analysis error. HasRoles composes HasPermissions, so both role and permission operations share this helper. + +The context ownership split is exact: + +- attachPermissions(), syncPermissions(), syncPermissionEffects(), revokePermissionTo(), assignRole(), removeRole(), and syncRoles() keep their ambient helper for input normalization, empty-input returns, and unsaved queueing, then switch to the selected relation's context when the saved write path first needs that relation; +- keep permissionAssignmentContext() in getCachedDirectPermissions() and roleAssignmentContext() in getCachedRoles(), including warm reads, because those helpers provide the only fail-closed partition check when no relation is built. + +Neither ambient context helper is removed. Warm authorization and unsaved queueing must not construct a per-operation relation merely to obtain context. Saved no-op replacement and duplicate-assignment paths build the selected relation because its real rows define the result, but they perform no mutation or cache invalidation. On saved write paths, building the supported relation calls ensurePermissionRelationParentMatches(), which performs the same validation before the relation-derived context replaces the ambient value, so no guard is lost. Both values come from the same coroutine-local state during one method invocation. + +The supported override aliases the package relation, so it remains one of the two partitioned classes. An override that replaces the package relation entirely fails fast at the undefined accessor; do not add an instanceof fallback that would silently discard captured-context ownership. getPermissionRelationContext() remains non-nullable: both partitioned relation constructors initialize the trait state before parent::__construct(), so the typed property is always set before the relation can escape construction. This preserves using(), custom casts, timestamps, model events, and one authoritative immutable team/partition context without a discovery relation, interface, or registry. + +Stock Pivot::class keeps existing bulk/set-based writes. Custom classes use native per-row Eloquent operations: + +~~~php +if ($relation->getPivotClass() === Pivot::class) { + $relation->newPivotQuery() + ->whereIn($relatedPivotKey, $updatedIds) + ->update(['is_denied' => $isDenied]); +} else { + foreach ($updatedIds as $id) { + $relation->updateExistingPivot($id, ['is_denied' => $isDenied], false); + } +} +~~~ + +Touch once after the complete mutation. Replace the now-false comment claiming permission assignment pivots have no custom class with a concise explanation of the stock bulk/custom native branch. Database's corrected predicate ownership makes a Permission-local scope guard unnecessary; no such compensation exists to remove. + +syncRoles() reads the current scoped IDs once, indexes current and requested IDs through their collision-safe identities, and detaches or attaches only the set difference. A retained custom pivot row is never deleted and recreated, so application attributes, timestamps, and model identity survive replacement. The detached event still receives the complete pre-operation set, same-set replacements still dispatch the documented pair, and no-op replacements perform no mutation, touch, or cache invalidation. + +Unsaved assignments capture the registrar's memoized pivot class beside PermissionRelationContext; they do not construct a per-operation write relation: + +~~~php +[ + 'permissions' => $permissions, + 'pivot' => $pivot, + 'context' => $context, + 'pivotClass' => $registrar->getAssignmentPivotClass($this, 'permissions'), +] +~~~ + +Queued-batch identity includes the pivot class. Save-time flush rebuilds the protected captured-context relation, reapplies only a non-stock class with using(), and never consults future ambient context. + +PermissionRegistrar::getAssignmentPivotClass() owns a bounded instance memo keyed by subject model class and relation name for boot-stable pivot-class metadata. Its cold resolver builds the actual public relation from the actual model; later unsaved operations only read the memo. Reinitializing registrar model/cache configuration clears this instance memo. It is not static, does not retain request models, and needs no flushState() or subscriber entry. + +The compact direct-assignment cache remains permission ID plus is_denied only. App-owned custom pivot attributes must not enter it because arbitrary pivot saves cannot invalidate that catalog. + +Authorization and name-only APIs stay on getCachedDirectPermissions(): + +- hasDirectPermission(); +- hasDeniedPermission(); +- getPermissionNames(). + +Only public methods that return Permission models/pivots switch to the real relation for a configured custom pivot: + +- getDirectPermissions(); +- the direct leg of getAllPermissions(). + +Keep allowedDirectPermissions() on getCachedDirectPermissions() so getPermissionNames() stays warm and query-free. getDirectPermissions() and getAllPermissions() independently select the memoized stock/custom source, then apply the same is_denied rejection. Do not redirect allowedDirectPermissions() to the public relation. + +relationCollection()/loadMissing performs at most one query per model per coroutine; the Eloquent relation cache makes later calls zero-query. getCachedDirectPermissions() already returns a loaded current relation, so subsequent authorization on the same model uses the same source without another query. + +Reverse arbitrary-model operations assignToModels(), removeFromModels(), and syncModels() retain upstream behavior and do not honor a subject relation override; document that boundary rather than adding a model/relation registry. + +Durable findings: permission-07 and permission-18. + +### 3. Build valid pivots from Permission's warm caches + +The direct-assignment cache currently attaches an unconfigured base Pivot. Public pivot methods can then fail because foreign/related keys and morph metadata are absent. Put the exact identifiers with their existing metadata family in Support\Config: + +~~~php +public const MORPH_NAME = 'model'; +public const MORPH_TYPE = self::MORPH_NAME . '_type'; +~~~ + +Replace the constructor literal inside permissionMorphToMany() with the name constant. Replace all five independent model_type column literals—HasPermissions hard-delete fast path, scope-discovery select, hard-delete transaction, warm hydration, and HasRoles scopeTeam subquery—with the type constant. Do not add a new permissionMorphToMany() parameter. Build the stock MorphPivot directly from already-known metadata, with no relation query or larger cache entry: + +~~~php +$morphType = Config::MORPH_TYPE; + +$pivot = MorphPivot::fromRawAttributes( + $model, + $attributes, + Config::modelHasPermissionsTable(), + true, +); + +$pivot->setPivotKeys(Config::morphKey(), $registrar->pivotPermission) + ->setRelatedModel($permission) + ->setMorphType($morphType) + ->setMorphClass($model->getMorphClass()); +~~~ + +The related model is the cloned Permission returned to the caller. Team and partition columns already present in the cached hydration remain attached. Because this synthetic warm pivot does not pass through a relation's newPivot(), also attach exact current-context predicates directly: partition and non-null team values become pivot where descriptors; a global-team null becomes a pivot where-null descriptor. This keeps public save()/delete() inside the cached assignment's scope without building a relation or adding a query: + +~~~php +$pivotWheres = $context->partition + ? [[$context->partition->column, '=', $context->partition->value]] + : []; +$pivotWhereNulls = []; + +if ($context->teamScoped) { + if ($context->team === null) { + $pivotWhereNulls[] = [$registrar->teamsKey]; + } else { + $pivotWheres[] = [$registrar->teamsKey, '=', $context->team]; + } +} + +if ($pivotWheres !== [] || $pivotWhereNulls !== []) { + $pivot->setPivotConstraints( + wheres: $pivotWheres, + whereIns: [], + whereNulls: $pivotWhereNulls, + whereBetweens: [], + ); +} +~~~ + +Keep this bounded construction beside warm pivot hydration; do not add methods to PermissionRelationContext solely for this one consumer. Add load-bearing comparisons of compiled delete-query SQL and bindings for warm-cached and relation-hydrated pivots on the same edge under four real scope shapes: non-null team, global null team, partition only, and team plus partition. Use test-local protected-method access, not a production seam, and do not assert pivotRelated: setRelatedModel() is part of authoritative pivot construction, but that stored property currently has no reader. Cover getForeignKey(), save(), and delete() on a warm stock pivot, including sibling team and partition rows. + +The worker permission catalog has the same structural defect on its role edges. getHydratedPermissionRoleCollection() attaches a Pivot without keys or a related model, so public key accessors and composite-key save/delete fail. Complete that Hypervel-owned Permission::roles pivot with the registrar's permission/role key names, related Role, exists=true, and the already resolved partition descriptor. The role_has_permissions table has no team dimension. + +getPermissionsViaRoles() and the via-role leg of getAllPermissions() must not hand out the catalog's Pivot instance. Besides exposing the opposite Permission::roles key orientation from Spatie's Role::permissions result, sharing the object lets caller mutation alter the coroutine-cached denied edge used by later authorization. Clone the Permission first and build one fresh, upstream-oriented Pivot from the cached edge attributes: + +~~~php +$pivot = Pivot::fromRawAttributes( + $role, + $cachedPivot->getAttributes(), + Config::roleHasPermissionsTable(), + true, +); +$pivot->setPivotKeys($registrar->pivotRole, $registrar->pivotPermission) + ->setRelatedModel($permission); +~~~ + +Resolve the registrar and current partition once in loadPermissionsViaRolesWithPivots(), pass them through the existing mapping call, and attach the single partition descriptor when present. Do not build owning relations per edge and do not extract a generic pivot builder: the direct MorphPivot, catalog Permission::roles Pivot, and public Role::permissions Pivot differ in parent, class, keys, related model, and constraints. + +This result construction is paid only by model-returning via-role APIs and the memoized wildcard-index build, once per edge beside the Permission clone already created there. Ordinary non-wildcard authorization is unchanged; completing catalog pivots adds only setter calls during the existing once-per-coroutine catalog hydration. + +The custom-permission-class branch of PermissionRegistrar::getPermissions() cannot feed permissionWithRolePivot() in supported use: HasPermissions::getPermissionClass() memoizes the registrar's configured class, and registrar reconfiguration is boot/test-only. Do not manufacture coverage for mid-coroutine reconfiguration. + +Durable findings: permission-13 and permission-16. + +### 4. Restore complete Permission replacement events + +For a saved model, syncPermissions() and Hypervel's syncPermissionEffects() use one replacement contract: + +1. capture the current relation context and selected public relation; +2. only when events are enabled and PermissionDetachedEvent has a targeted listener, hydrate the pre-operation Permission collection through that relation; +3. run the existing transaction and effect-aware synchronization; +4. invalidate the exact catalog/model cache after successful commit; +5. dispatch a nonempty detached pre-operation collection, then the requested attached IDs. + +~~~php +$detached = $this->permissionDetachedEventIsListenedFor() + ? $relation->get() + : new Collection; + +$changes = $this->synchronizePermissionAssignments(...); +$this->forgetPermissionAssignmentCache(...); + +if ($detached->isNotEmpty()) { + $this->dispatchPermissionDetachedEvent($detached); +} + +$this->dispatchPermissionAttachedEvent($permissions); +~~~ + +The detached payload describes replacement, not a delta: retained permissions appear, and same-set sync still reports the current collection. A failed transaction dispatches neither event. Unsaved sync remains attached-only because there is no persisted set to detach. + +Reusing the selected relation after get() is deliberate. get() changes only the relation read builder's selected columns; attach, detach, and effect updates create fresh pivot statements through newPivotQuery(). Direct get() also bypasses the relation's getResults() override, so it does not populate or mark the model relation cache. Keep the one extra detached read separate rather than rebuilding the relation and recapturing ambient context. + +Both replacement methods keep the exact synchronization result. They clear loaded relations and invalidate catalog/model caches only when at least one edge is attached, detached, or updated; a same-set replacement still dispatches both events but preserves every warm cache. + +The ordinary path gains no event construction. The only listener-driven extra read is on saved replacement when events are enabled and a targeted detached listener exists; the synchronizer's existing pivot comparison read remains required. + +Durable findings: permission-06 and permission-17. + +### 5. Preserve exact team identifiers and truthful setup status + +CreateRoleCommand and AssignRoleCommand read team-id once and distinguish only null/empty from a supplied value: + +~~~php +$teamId = $this->option('team-id'); +$hasTeamId = $teamId !== null && $teamId !== ''; +~~~ + +Use those values for disabled-team warnings, setPermissionsTeamId(), and the global-role warning. String "0" remains a real team identifier. Preserve and restore the prior team context in finally. CreateRoleCommand continues assigning role permissions after restoration because that edge is not team-scoped. + +UpgradeForTeamsCommand returns the native copy result and fails the command when publication fails: + +~~~php +if (! $this->createMigration()) { + $this->error(...); + + return self::FAILURE; +} + +return self::SUCCESS; +~~~ + +~~~php +try { + return copy($migrationStub, $this->getMigrationPath()); +} catch (Throwable $throwable) { + $this->error($throwable->getMessage()); + + return false; +} +~~~ + +Use a portable invalid destination in the regression. Keep the existing framework warning-to-exception handler; add no temporary global error handler or production test seam. + +Durable findings: permission-08 and permission-14. + +### 6. Remove unused queued-mutation result computation + +The four queued replacement/removal helpers return booleans that no caller reads. Convert them to void and remove comparison-only current/replacement arrays and changed flags. Preserve filtering, no-op early returns, nonempty requeueing, captured context, and pivot-class identity. + +~~~php +private function replaceQueuedPermissionAssignments(...): void +{ + $this->queuedPermissionAssignments = array_values(array_filter(...)); + + foreach ($assignments as $assignment) { + if ($assignment['permissions'] !== []) { + $this->queuePermissionAssignments(...); + } + } +} +~~~ + +Apply the same cleanup to queued roles. Do not alter persistence behavior or add new helper layers. + +Durable finding: permission-15. + +### 7. Make root discovery and split dependencies self-verifying + +Add Permission, Horizon, and Wayfinder providers to the root extra.hypervel.providers list. Do not add optional packages to DefaultProviders. + +Extend tests/Composer/PackageManifestConsistencyTest.php with one repository-wide invariant: + +~~~php +foreach ($this->splitManifests() as $manifest) { + $composer = $this->decodeManifest($manifest); + + foreach ($composer['extra']['hypervel']['providers'] ?? [] as $provider) { + $this->assertTrue( + in_array($provider, $rootProviders, true) + || in_array($provider, $defaultProviders, true), + ); + } +} +~~~ + +Foundation is the expected DefaultProviders-only case. This prevents future split/root discovery drift without duplicating provider lists in package-specific tests. + +Permission split metadata adds the direct dependencies already pinned by the root: + +~~~json +"composer-runtime-api": "^2.2", +"nesbot/carbon": "^3.13.1", +"symfony/http-kernel": "^8.1" +~~~ + +Remove PermissionServiceProvider's class_exists guard around AboutCommand registration and the matching test skips. + +Apply the same exact direct-dependency correction to the verified sibling manifests: + +| Dependency | Split packages | +|---|---| +| symfony/http-kernel ^8.1 | broadcasting, contracts, passkeys, telescope | +| nesbot/carbon ^3.13.1 | concurrency, contracts, notifications, passkeys, process, telescope | +| composer-runtime-api ^2.2 | di | + +Create focused PackageMetadataTest classes for Permission, Concurrency, DI, Passkeys, Process, and Telescope. Extend the existing Broadcasting, Contracts, and Notifications asserted requirement lists with their newly declared dependencies. Assert exact root parity, not merely key presence. Do not add an import scanner. + +Durable findings: permission-09, permission-10, horizon-22, wayfinder-01, broadcasting-17, contracts-12, concurrency-08, di-06, notifications-21, passkeys-01, process-11, telescope-04. + +### 8. Port only supported, unique current-upstream coverage + +Merge the current Spatie tests whose branches remain meaningful in Hypervel: + +- setup-teams disabled, declined, existing migration, and creation failure; +- create-role disabled/global warnings and team "0"; +- Guard with no provider, LDAP provider, no Passport guard, and a guard without a client surface; +- cache reset when forget() fails; +- Model team ID; +- missing Role::findById(); +- Permission ID checks and unsupported mixed assignment input being ignored; +- exact pipe-delimited role parsing; +- invalid wildcard implementation and blank comma subparts. + +Source files: + +- tests/Permission/Commands/CommandTest.php; +- tests/Permission/GuardTest.php; +- tests/Permission/Integration/CacheTest.php; +- tests/Permission/Integration/PermissionRegistrarTest.php; +- tests/Permission/Models/RoleTest.php; +- tests/Permission/Traits/HasAssignedModelsTest.php; +- tests/Permission/Traits/HasPermissionsTest.php; +- tests/Permission/Traits/HasRolesTest.php; +- tests/Permission/Traits/TeamHasPermissionsTest.php; +- tests/Permission/Traits/TeamHasRolesTest.php; +- tests/Permission/Traits/WildcardHasPermissionsTest.php. + +Do not port the unknown-store array-cache fallback, obsolete retry behavior, Laravel Octane reset wiring, or redundant reflection tests. Do not add production seams solely for tests. + +Durable finding: permission-12. + +### 9. Complete Laravel-style Permission documentation + +Update src/boost/docs/permission.md in the surrounding Laravel-docs prose: + +- correct the revoke example to pass an array; +- define a Permission as one ability and a Role as a named permission group; +- describe replacement events, including the pre-operation detached collection, requested attached IDs, same-set behavior, listener gating, and failure ordering; +- show the trait-alias plus using(CustomPivot::class) extension pattern; +- explain that custom-pivot model-returning APIs load the real relation while authorization and permission-name checks retain the compact cache; +- state that reverse arbitrary-model operations do not use a subject's relation override; +- update the performance section with the exact listener-gated replacement read. + +Keep implementation internals out of user documentation. Do not claim scoped removals bypass pivot hooks after Database restores native pivot event behavior. + +Durable finding: permission-11. + +### 10. Update durable audit records without declaring a full audit + +Update the core routing lines to this Permission work unit and the exact carried dependencies. Add one compact ledger section with: + +- permission-06 through permission-18; +- database-29 through database-33 and Database revalidation; +- provider/dependency sibling IDs and completed revalidation; +- support-02 revalidation; +- final API/performance result; +- important rejected concerns. + +Keep Permission unchecked in the core package checklist because this work begins from the completed external finding report rather than a fresh source-wide audit. Do not rewrite historical permission-01 through permission-05. + +## Rejected designs and non-findings + +- No relation registry, custom-pivot attribute cache, cache-invalidation observer, per-request model map, static pivot memo, subscriber entry, or new CoroutineContext slot. +- No partitioned-relation interface; one shared helper owns the only static-analysis narrowing the existing two concrete relation classes need. +- No ordinary authorization query, custom pivot lookup, event construction, or model hydration. +- No cached/public role-edge Pivot alias, relation construction per cached edge, generic pivot builder, or manufactured mid-coroutine permission-class reconfiguration path. +- No using(Pivot::class); stock pivots retain set-based operations. +- No Permission-local raw-query safety path after Database owns predicate retention. +- No delta-only detached event and no ID-only event payload. +- No custom pivot handling for reverse arbitrary-model methods. +- No redesign of Laravel's public relationship orWhere semantics. +- No generic predicate object or lazy query-builder reconstruction. +- No array-cache fallback, retry compatibility path, Octane reset hook, optional-provider DefaultProviders entry, metadata import scanner, or temporary error handler. +- Existing worker/coroutine ownership remains correct: permission catalogs are worker-cached, request assignment caches and relation provenance are coroutine-local, registrar configuration mutators remain boot/test-only, and no native resource lifecycle exists in Permission. + +## Test plan + +Run each changed test file immediately after its coherent source slice. + +### Database relation safety + +1. Run BelongsToManyPivotEventsTest and MorphToManyPivotEventsTest before and after the shared fix; preserve per-row deleting/deleted and saving/saved order. +2. Add cross-parent regressions proving grouped wherePivot + orWherePivot cannot escape parent identity. +3. Prove stock detach/update obey wherePivotBetween and its shared or/not funnel. +4. Prove stock and custom hydrated pivot save/delete retain team/partition/value/range scopes. +5. Prove constrained custom detach/update preserves sibling rows and still fires native pivot events. +6. Prove custom attach/sync and updateExistingPivot() force-fill explicit guarded attributes while preserving casts and model events; include one strict-mode regression covering attach and update. +7. Prove an id-bearing constrained pivot retains primary-key save/delete behavior, including an intentional constrained-column change. +8. Prove the MorphToMany guarded-attribute path changes only the intended morph-scoped row and leaves a sibling morph type untouched. +9. Prove unconstrained stock/custom behavior is unchanged. + +### Permission behavior + +Run targeted PHPStan for src/permission after the first custom-relation/context slice, before proceeding to later Permission changes. This catches accidental promotion of the helper's inline union narrowing to a parameter contract. + +1. Replacement events: saved subject and Role; team and partition scopes; same-set, empty, retained, allowed/denied updates; disabled/no listener; transaction failure; unsaved attached-only; detached-before-attached ordering and fresh post-invalidation reads. +2. Custom pivots: immediate and deferred permission/role assignment; casts, timestamps, events, detaching, guarded denied-effect updates, captured relation context, pivot-class batch identity, retained rows across role replacement, and stock bulk-query retention. +3. Cache contract: custom getDirectPermissions()/getAllPermissions() returns real pivot; the second call and later authorization issue no extra query; getPermissionNames() stays warm zero-query. +4. Stock warm pivot: correct class, keys, morph metadata, warm/relation delete-query SQL and binding parity under non-null team, global null team, partition-only, and combined scopes, plus save/delete without crossing sibling rows. +5. Role-permission pivots: catalog Permission::roles and public via-role Role::permissions orientations match live relation delete SQL/bindings in partitioned and non-partitioned configurations; public key accessors work; mutating the fresh public pivot leaves the catalog edge and hasDeniedPermissionViaRoles() unchanged; scoped save/delete cannot cross partitions. +6. CLI/setup: team "0", disabled/global warnings, prior context restoration, publication success/failure status. +7. Queued helper cleanup: existing unsaved assignment/replacement/removal suites remain green. +8. Port the bounded current-upstream cases listed above. +9. Prove same-set permission and effect replacement preserves the warm Role permission catalog while still dispatching the documented events. + +### Metadata, documentation, and records + +1. Run Composer PackageManifestConsistencyTest and every changed PackageMetadataTest. +2. Run PermissionServiceProvider/Command tests with the installed direct dependencies. +3. Validate root and every changed split manifest with composer validate --strict --no-check-publish. +4. Search for stale class_exists/skip guards, old queued boolean contracts, raw Pivot warm hydration, duplicate MorphToMany post-mapping, and outdated documentation claims. +5. Check plan, ledger, routing, finding IDs, and package checklist consistency. + +### Gates and review + +1. Run the complete Database relation and Permission suites. +2. Run composer fix once at the implementation checkpoint. +3. Perform a fresh caller/callee, transaction/event ordering, public/protected API, partition/team context, coroutine state, hot-path, retained-memory, and overengineering review. +4. Apply review corrections, rerun affected focused tests, and repeat the complete gate only if changes warrant it. + +## Expected final result + +- Laravel/Spatie-facing Permission APIs, named arguments, relation overrides, pivot events, command options, and event payloads remain compatible or are restored. +- Hypervel-specific denied permissions, partitioning, immutable relation contexts, compact caching, and transactional synchronization remain intact. +- Warm authorization and permission-name checks gain no query, allocation loop, lock, yield, serialization, or container lookup. +- Custom-pivot model-returning APIs pay one necessary relation query per model/coroutine, then reuse the loaded relation; custom writes honor every explicit attribute and use native per-row casts and hooks only where requested. +- Warm role-permission pivots are structurally complete; model-returning via-role APIs and the memoized wildcard-index build create one fresh correctly oriented Pivot per returned edge so caller mutation cannot alter cached authorization state. +- No-op permission and role replacements preserve warm caches and retained custom-pivot rows while keeping the documented replacement-event payloads. +- The saved replacement event path adds one hydration query only when its targeted detached listener is active. +- Database adds bounded in-memory predicate descriptor retention only to constrained pivot instances; unconstrained explicit pivot writes add one predictable null guard, outside ordinary authorization and relationship reads. +- No stale helper, duplicate hydration path, workaround, compatibility shim, speculative abstraction, unresolved accepted defect, or TODO remains. From 819bc3477c36b2940c86cb79405d1ef02d2e355a Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:08:20 +0000 Subject: [PATCH 09/10] docs(agents): clarify Composer dependency workflow Require new root package dependencies to use the latest compatible stable release published on Packagist.\n\nDocument that the root Composer lock is intentionally untracked, that local dependencies should be refreshed after dependency changes, and that stale local lock state is not a repository defect. --- AGENTS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 22f383bb2..7d8a6f490 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,7 +129,7 @@ The Working rules and the Avoid overengineering rules apply to all work in this - **Read the source before describing behavior** — never state how code behaves from memory or Laravel assumptions. Hypervel's coroutine runtime breaks many Laravel assumptions; if you haven't read the relevant source, read it first. - **Treat past owner decisions as context, not constraints** — Previous owner approvals and completed plans explain history but do not determine the best design today. Never retain or reject a design merely because it was previously approved; decide from current requirements, code, and evidence. - **Revert failed attempts immediately** — when a fix doesn't work, revert it before trying another approach. Don't leave experimental code in place. -- **Use `composer require` for root dependencies** — the root `composer.json` has a lockfile, so dependency entries go through Composer, never hand-edits. Direct edits are fine for metadata sections no command can write (`autoload`, `replace`, `extra`, `scripts`) and for the sub-package `src/{package}/composer.json` files, which have no lockfile. +- **Check dependency versions before adding them** — Before adding a package dependency to the root `composer.json`, check Packagist for the latest compatible stable version. The root `composer.lock` is intentionally untracked; run `composer update` after adding or merging dependency changes, do not treat an outdated local lock as a repository defect, and never commit it. ### Documentation From 60d1c24ce3880ec909ed5d525f1757af52fdddb3 Mon Sep 17 00:00:00 2001 From: Raj Siva-Rajah <5361908+binaryfire@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:31:11 +0000 Subject: [PATCH 10/10] test(permission): align sync query expectations Update the role replacement query-count regression for the diff-based synchronization path. Initial assignment now asserts one scoped pivot read followed by one bulk insert, without expecting an unnecessary delete.\n\nClarify that detached-event payloads reuse the mandatory role read and require both role and permission pivot-read regressions to pin the SELECT statement type. --- .../Integration/PartitionQueryCountTest.php | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tests/Permission/Integration/PartitionQueryCountTest.php b/tests/Permission/Integration/PartitionQueryCountTest.php index 88f3c1694..30746a3c2 100644 --- a/tests/Permission/Integration/PartitionQueryCountTest.php +++ b/tests/Permission/Integration/PartitionQueryCountTest.php @@ -106,7 +106,7 @@ public function testResolverLookupAndOrdinaryMutationsAddNoDiscoveryQuery(): voi $this->assertSame([], $discoveryQueries); } - public function testRoleSyncUsesOneDeleteAndOneBulkInsertWithoutListeners(): void + public function testRoleSyncUsesOnePivotReadAndOneBulkInsertWithoutListeners(): void { $user = GlobalPartitionUser::create(['email' => 'roles@example.com']); $firstRole = PartitionedRole::create(['name' => 'editor']); @@ -125,12 +125,16 @@ public function testRoleSyncUsesOneDeleteAndOneBulkInsertWithoutListeners(): voi $this->assertContains(self::PARTITION_A, $query['bindings']); } - $this->assertStringContainsString('delete from', strtolower($queries[0]['query'])); - $this->assertStringContainsString('model_has_roles', $queries[0]['query']); + $pivotRead = strtolower($queries[0]['query']); + + $this->assertStringContainsString('select', $pivotRead); + $this->assertStringContainsString('model_has_roles', $pivotRead); + $this->assertStringContainsString('role_test_id', $pivotRead); + $this->assertStringNotContainsString(' join ', $pivotRead); $this->assertStringContainsString('insert into', strtolower($queries[1]['query'])); } - public function testRoleSyncAddsOnePivotOnlyReadForTheDetachedEventPayload(): void + public function testRoleSyncReusesPivotReadForDetachedEventPayload(): void { $user = GlobalPartitionUser::create(['email' => 'role-events@example.com']); $currentRole = PartitionedRole::create(['name' => 'editor']); @@ -178,6 +182,7 @@ public function testPermissionSyncUsesAPivotOnlyReadAndOneBulkInsert(): void $pivotRead = strtolower($queries[0]['query']); + $this->assertStringContainsString('select', $pivotRead); $this->assertStringContainsString('model_has_permissions', $pivotRead); $this->assertStringContainsString('permission_test_id', $pivotRead); $this->assertStringNotContainsString(' join ', $pivotRead);