From 9c685f4e2136c9a36fa64019eed0f290dfb8f5c7 Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Sat, 21 Dec 2024 21:52:32 +0100 Subject: [PATCH 1/7] Prevent creating non-existent models from container --- .../src/Concerns/EvaluatesClosures.php | 6 +- tests/src/Support/EvaluatesClosuresTest.php | 100 ++++++++++++++++++ 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 tests/src/Support/EvaluatesClosuresTest.php diff --git a/packages/support/src/Concerns/EvaluatesClosures.php b/packages/support/src/Concerns/EvaluatesClosures.php index 47bb8439ed3..a0f8f341f92 100644 --- a/packages/support/src/Concerns/EvaluatesClosures.php +++ b/packages/support/src/Concerns/EvaluatesClosures.php @@ -4,6 +4,7 @@ use Closure; use Illuminate\Contracts\Container\BindingResolutionException; +use Illuminate\Database\Eloquent\Model; use ReflectionFunction; use ReflectionNamedType; use ReflectionParameter; @@ -81,7 +82,10 @@ protected function resolveClosureDependencyForEvaluation(ReflectionParameter $pa return $this; } - if (filled($typedParameterClassName)) { + if ( + filled($typedParameterClassName) + && ! is_subclass_of($parameter->getType()->getName(), Model::class) + ) { return app()->make($typedParameterClassName); } diff --git a/tests/src/Support/EvaluatesClosuresTest.php b/tests/src/Support/EvaluatesClosuresTest.php new file mode 100644 index 00000000000..e33feeaead1 --- /dev/null +++ b/tests/src/Support/EvaluatesClosuresTest.php @@ -0,0 +1,100 @@ +evaluate(function (IsEvaluatingClosures $isEvaluatingClosures) { + $this->expectNotToPerformAssertions(); + }); +}); + +it('will instantiate Eloquent Models provided by name', function () { + $isEvaluatingClosures = new IsEvaluatingClosures( + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: true, + shouldResolveDefaultClosureDependencyForEvaluationByType: false, + ); + + $isEvaluatingClosures->evaluate(function (RecordModel $record) { + $this->expectNotToPerformAssertions(); + }); +}); + +it('will not instantiate Eloquent Models not provided by name', function () { + $isEvaluatingClosures = new IsEvaluatingClosures( + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: true, + shouldResolveDefaultClosureDependencyForEvaluationByType: false, + ); + + $this->expectException(BindingResolutionException::class); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModel) { + throw new RuntimeException('Should not be called because named parameter not provided.'); + }); +}); + +it('will instantiate Eloquent Models provided by type', function () { + $isEvaluatingClosures = new IsEvaluatingClosures( + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: false, + shouldResolveDefaultClosureDependencyForEvaluationByType: true, + ); + + $isEvaluatingClosures->evaluate(function (RecordModel $record) use ($recordModel) { + expect($record)->toBe($recordModel); + }); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModelWithDifferentName) use ($recordModel) { + expect($recordModelWithDifferentName)->toBe($recordModel); + }); +}); + +it('will not instantiate empty Models from container', function () { + $isEvaluatingClosures = new IsEvaluatingClosures; + + $this->expectException(BindingResolutionException::class); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModel) { + throw new RuntimeException('Should not be called.'); + }); +}); + +class RecordModel extends Model +{ + // +} + +class IsEvaluatingClosures +{ + public function __construct( + public ?RecordModel $record = null, + public bool $shouldResolveDefaultClosureDependencyForEvaluationByName = false, + public bool $shouldResolveDefaultClosureDependencyForEvaluationByType = false, + ) {} + + use Support\Concerns\EvaluatesClosures; + + protected function resolveDefaultClosureDependencyForEvaluationByName(string $parameterName): array + { + return match (true) { + $this->shouldResolveDefaultClosureDependencyForEvaluationByName && $parameterName === 'record' => [$this->record], + default => [], + }; + } + + protected function resolveDefaultClosureDependencyForEvaluationByType(string $parameterType): array + { + return match (true) { + $this->shouldResolveDefaultClosureDependencyForEvaluationByType && $parameterType === $this->record::class => [$this->record], + default => [], + }; + } +} From cfe8227b672a30b3fb8bacaec849ceedee043a9b Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Sat, 21 Dec 2024 22:01:44 +0100 Subject: [PATCH 2/7] Style --- tests/src/Support/EvaluatesClosuresTest.php | 42 ++++++++++----------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/tests/src/Support/EvaluatesClosuresTest.php b/tests/src/Support/EvaluatesClosuresTest.php index e33feeaead1..5ec2d432394 100644 --- a/tests/src/Support/EvaluatesClosuresTest.php +++ b/tests/src/Support/EvaluatesClosuresTest.php @@ -17,9 +17,9 @@ it('will instantiate Eloquent Models provided by name', function () { $isEvaluatingClosures = new IsEvaluatingClosures( - record: $recordModel = new RecordModel, - shouldResolveDefaultClosureDependencyForEvaluationByName: true, - shouldResolveDefaultClosureDependencyForEvaluationByType: false, + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: true, + shouldResolveDefaultClosureDependencyForEvaluationByType: false, ); $isEvaluatingClosures->evaluate(function (RecordModel $record) { @@ -29,9 +29,9 @@ it('will not instantiate Eloquent Models not provided by name', function () { $isEvaluatingClosures = new IsEvaluatingClosures( - record: $recordModel = new RecordModel, - shouldResolveDefaultClosureDependencyForEvaluationByName: true, - shouldResolveDefaultClosureDependencyForEvaluationByType: false, + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: true, + shouldResolveDefaultClosureDependencyForEvaluationByType: false, ); $this->expectException(BindingResolutionException::class); @@ -42,19 +42,19 @@ }); it('will instantiate Eloquent Models provided by type', function () { - $isEvaluatingClosures = new IsEvaluatingClosures( - record: $recordModel = new RecordModel, - shouldResolveDefaultClosureDependencyForEvaluationByName: false, - shouldResolveDefaultClosureDependencyForEvaluationByType: true, - ); - - $isEvaluatingClosures->evaluate(function (RecordModel $record) use ($recordModel) { - expect($record)->toBe($recordModel); - }); - - $isEvaluatingClosures->evaluate(function (RecordModel $recordModelWithDifferentName) use ($recordModel) { - expect($recordModelWithDifferentName)->toBe($recordModel); - }); + $isEvaluatingClosures = new IsEvaluatingClosures( + record: $recordModel = new RecordModel, + shouldResolveDefaultClosureDependencyForEvaluationByName: false, + shouldResolveDefaultClosureDependencyForEvaluationByType: true, + ); + + $isEvaluatingClosures->evaluate(function (RecordModel $record) use ($recordModel) { + expect($record)->toBe($recordModel); + }); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModelWithDifferentName) use ($recordModel) { + expect($recordModelWithDifferentName)->toBe($recordModel); + }); }); it('will not instantiate empty Models from container', function () { @@ -74,14 +74,14 @@ class RecordModel extends Model class IsEvaluatingClosures { + use Support\Concerns\EvaluatesClosures; + public function __construct( public ?RecordModel $record = null, public bool $shouldResolveDefaultClosureDependencyForEvaluationByName = false, public bool $shouldResolveDefaultClosureDependencyForEvaluationByType = false, ) {} - use Support\Concerns\EvaluatesClosures; - protected function resolveDefaultClosureDependencyForEvaluationByName(string $parameterName): array { return match (true) { From 5085d58421b97ee9540ead49f037e69b322c9bd8 Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Sat, 21 Dec 2024 22:08:09 +0100 Subject: [PATCH 3/7] Fix PHPStan --- packages/support/src/Concerns/EvaluatesClosures.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/support/src/Concerns/EvaluatesClosures.php b/packages/support/src/Concerns/EvaluatesClosures.php index a0f8f341f92..b78c2563dc5 100644 --- a/packages/support/src/Concerns/EvaluatesClosures.php +++ b/packages/support/src/Concerns/EvaluatesClosures.php @@ -84,7 +84,7 @@ protected function resolveClosureDependencyForEvaluation(ReflectionParameter $pa if ( filled($typedParameterClassName) - && ! is_subclass_of($parameter->getType()->getName(), Model::class) + && ! is_subclass_of($typedParameterClassName, Model::class) ) { return app()->make($typedParameterClassName); } From 006d84e8f719a8b044a7706be8b70d6373ece9e9 Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Sat, 21 Dec 2024 22:11:19 +0100 Subject: [PATCH 4/7] Style --- .../PostCategories/PostCategoryResource.php | 12 ++++-------- tests/src/Fixtures/Resources/Posts/PostResource.php | 12 ++++-------- .../Resources/Shop/Products/ProductResource.php | 12 ++++-------- tests/src/Fixtures/Resources/Users/UserResource.php | 12 ++++-------- 4 files changed, 16 insertions(+), 32 deletions(-) diff --git a/tests/src/Fixtures/Resources/PostCategories/PostCategoryResource.php b/tests/src/Fixtures/Resources/PostCategories/PostCategoryResource.php index 150b4eeeaa1..f0b73dec9dc 100644 --- a/tests/src/Fixtures/Resources/PostCategories/PostCategoryResource.php +++ b/tests/src/Fixtures/Resources/PostCategories/PostCategoryResource.php @@ -4,10 +4,6 @@ use Filament\Resources\Resource; use Filament\Tests\Fixtures\Models\PostCategory; -use Filament\Tests\Fixtures\Resources\PostCategories\Pages\CreatePostCategory; -use Filament\Tests\Fixtures\Resources\PostCategories\Pages\EditPostCategory; -use Filament\Tests\Fixtures\Resources\PostCategories\Pages\ListPostCategories; -use Filament\Tests\Fixtures\Resources\PostCategories\Pages\ViewPostCategory; class PostCategoryResource extends Resource { @@ -20,10 +16,10 @@ class PostCategoryResource extends Resource public static function getPages(): array { return [ - 'index' => ListPostCategories::route('/'), - 'create' => CreatePostCategory::route('/create'), - 'view' => ViewPostCategory::route('/{record}'), - 'edit' => EditPostCategory::route('/{record}/edit'), + 'index' => Pages\ListPostCategories::route('/'), + 'create' => Pages\CreatePostCategory::route('/create'), + 'view' => Pages\ViewPostCategory::route('/{record}'), + 'edit' => Pages\EditPostCategory::route('/{record}/edit'), ]; } } diff --git a/tests/src/Fixtures/Resources/Posts/PostResource.php b/tests/src/Fixtures/Resources/Posts/PostResource.php index 0205179d3ca..438c4ebe152 100644 --- a/tests/src/Fixtures/Resources/Posts/PostResource.php +++ b/tests/src/Fixtures/Resources/Posts/PostResource.php @@ -12,10 +12,6 @@ use Filament\Tables; use Filament\Tables\Table; use Filament\Tests\Fixtures\Models\Post; -use Filament\Tests\Fixtures\Resources\Posts\Pages\CreatePost; -use Filament\Tests\Fixtures\Resources\Posts\Pages\EditPost; -use Filament\Tests\Fixtures\Resources\Posts\Pages\ListPosts; -use Filament\Tests\Fixtures\Resources\Posts\Pages\ViewPost; use Illuminate\Database\Eloquent\Builder; class PostResource extends Resource @@ -74,10 +70,10 @@ public static function table(Table $table): Table public static function getPages(): array { return [ - 'index' => ListPosts::route('/'), - 'create' => CreatePost::route('/create'), - 'view' => ViewPost::route('/{record}'), - 'edit' => EditPost::route('/{record}/edit'), + 'index' => Pages\ListPosts::route('/'), + 'create' => Pages\CreatePost::route('/create'), + 'view' => Pages\ViewPost::route('/{record}'), + 'edit' => Pages\EditPost::route('/{record}/edit'), ]; } } diff --git a/tests/src/Fixtures/Resources/Shop/Products/ProductResource.php b/tests/src/Fixtures/Resources/Shop/Products/ProductResource.php index 2b27809ad38..2191694136b 100644 --- a/tests/src/Fixtures/Resources/Shop/Products/ProductResource.php +++ b/tests/src/Fixtures/Resources/Shop/Products/ProductResource.php @@ -4,10 +4,6 @@ use Filament\Resources\Resource; use Filament\Tests\Fixtures\Models\Product; -use Filament\Tests\Fixtures\Resources\Shop\Products\Pages\CreateProduct; -use Filament\Tests\Fixtures\Resources\Shop\Products\Pages\EditProduct; -use Filament\Tests\Fixtures\Resources\Shop\Products\Pages\ListProducts; -use Filament\Tests\Fixtures\Resources\Shop\Products\Pages\ViewProduct; class ProductResource extends Resource { @@ -20,10 +16,10 @@ class ProductResource extends Resource public static function getPages(): array { return [ - 'index' => ListProducts::route('/'), - 'create' => CreateProduct::route('/create'), - 'view' => ViewProduct::route('/{record}'), - 'edit' => EditProduct::route('/{record}/edit'), + 'index' => Pages\ListProducts::route('/'), + 'create' => Pages\CreateProduct::route('/create'), + 'view' => Pages\ViewProduct::route('/{record}'), + 'edit' => Pages\EditProduct::route('/{record}/edit'), ]; } } diff --git a/tests/src/Fixtures/Resources/Users/UserResource.php b/tests/src/Fixtures/Resources/Users/UserResource.php index 820b555f938..069df5e9ab9 100644 --- a/tests/src/Fixtures/Resources/Users/UserResource.php +++ b/tests/src/Fixtures/Resources/Users/UserResource.php @@ -11,10 +11,6 @@ use Filament\Tables; use Filament\Tables\Table; use Filament\Tests\Fixtures\Models\User; -use Filament\Tests\Fixtures\Resources\Users\Pages\CreateUser; -use Filament\Tests\Fixtures\Resources\Users\Pages\EditUser; -use Filament\Tests\Fixtures\Resources\Users\Pages\ListUsers; -use Filament\Tests\Fixtures\Resources\Users\Pages\ViewUser; class UserResource extends Resource { @@ -72,10 +68,10 @@ public static function table(Table $table): Table public static function getPages(): array { return [ - 'index' => ListUsers::route('/'), - 'create' => CreateUser::route('/create'), - 'view' => ViewUser::route('/{record}'), - 'edit' => EditUser::route('/{record}/edit'), + 'index' => Pages\ListUsers::route('/'), + 'create' => Pages\CreateUser::route('/create'), + 'view' => Pages\ViewUser::route('/{record}'), + 'edit' => Pages\EditUser::route('/{record}/edit'), ]; } } From 1bea137330fb2d375dc56f6c3042e1a5b2d4cc02 Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Thu, 26 Dec 2024 17:07:36 +0100 Subject: [PATCH 5/7] Resolve models if bound --- .../src/Concerns/EvaluatesClosures.php | 2 +- tests/src/Support/EvaluatesClosuresTest.php | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/packages/support/src/Concerns/EvaluatesClosures.php b/packages/support/src/Concerns/EvaluatesClosures.php index b78c2563dc5..68332ba4055 100644 --- a/packages/support/src/Concerns/EvaluatesClosures.php +++ b/packages/support/src/Concerns/EvaluatesClosures.php @@ -84,7 +84,7 @@ protected function resolveClosureDependencyForEvaluation(ReflectionParameter $pa if ( filled($typedParameterClassName) - && ! is_subclass_of($typedParameterClassName, Model::class) + && (! is_subclass_of($typedParameterClassName, Model::class) || app()->bound($typedParameterClassName)) ) { return app()->make($typedParameterClassName); } diff --git a/tests/src/Support/EvaluatesClosuresTest.php b/tests/src/Support/EvaluatesClosuresTest.php index 5ec2d432394..62a86ef4b1e 100644 --- a/tests/src/Support/EvaluatesClosuresTest.php +++ b/tests/src/Support/EvaluatesClosuresTest.php @@ -67,6 +67,42 @@ }); }); +it('will instantiate empty Models from container if bound', function () { + $isEvaluatingClosures = new IsEvaluatingClosures; + + $boundRecordModel = new RecordModel; + + app()->bind(RecordModel::class, fn () => $boundRecordModel); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModel) use ($boundRecordModel) { + expect($recordModel)->toBe($boundRecordModel); + }); +}); + +it('will instantiate empty Models from container if bound as singleton', function () { + $isEvaluatingClosures = new IsEvaluatingClosures; + + $boundRecordModel = new RecordModel; + + app()->singleton(RecordModel::class, fn () => $boundRecordModel); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModel) use ($boundRecordModel) { + expect($recordModel)->toBe($boundRecordModel); + }); +}); + +it('will instantiate empty Models from container if bound as scoped', function () { + $isEvaluatingClosures = new IsEvaluatingClosures; + + $boundRecordModel = new RecordModel; + + app()->scoped(RecordModel::class, fn () => $boundRecordModel); + + $isEvaluatingClosures->evaluate(function (RecordModel $recordModel) use ($boundRecordModel) { + expect($recordModel)->toBe($boundRecordModel); + }); +}); + class RecordModel extends Model { // From 8066a6234e1791c8257130cb897071a8d3acf1c2 Mon Sep 17 00:00:00 2001 From: "Ralph J. Smit" <59207045+ralphjsmit@users.noreply.github.com> Date: Thu, 26 Dec 2024 17:11:33 +0100 Subject: [PATCH 6/7] Replace expect no assertion with expectation --- tests/src/Support/EvaluatesClosuresTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/src/Support/EvaluatesClosuresTest.php b/tests/src/Support/EvaluatesClosuresTest.php index 62a86ef4b1e..969176812b0 100644 --- a/tests/src/Support/EvaluatesClosuresTest.php +++ b/tests/src/Support/EvaluatesClosuresTest.php @@ -22,8 +22,8 @@ shouldResolveDefaultClosureDependencyForEvaluationByType: false, ); - $isEvaluatingClosures->evaluate(function (RecordModel $record) { - $this->expectNotToPerformAssertions(); + $isEvaluatingClosures->evaluate(function (RecordModel $record) use ($recordModel) { + expect($record)->toBe($recordModel); }); }); From 573663ca30dbab54bfc66f8a282b8e2e6ead8121 Mon Sep 17 00:00:00 2001 From: ralphjsmit Date: Thu, 26 Dec 2024 16:19:39 +0000 Subject: [PATCH 7/7] chore: fix code style --- .../forms/resources/js/components/select.js | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/forms/resources/js/components/select.js b/packages/forms/resources/js/components/select.js index d002657e97a..b02e1f81eef 100644 --- a/packages/forms/resources/js/components/select.js +++ b/packages/forms/resources/js/components/select.js @@ -97,21 +97,23 @@ export default function selectFormComponent({ if (hasDynamicSearchResults) { this.$refs.input.addEventListener('search', (event) => { - if (! this.select._isSearching) { + if (!this.select._isSearching) { return } let search = event.detail.value?.trim() - this.select._displayNotice([null, undefined, ''].includes(search) - ? loadingMessage - : searchingMessage) + this.select._displayNotice( + [null, undefined, ''].includes(search) + ? loadingMessage + : searchingMessage, + ) }) this.$refs.input.addEventListener( 'search', Alpine.debounce(async (event) => { - if (! this.select._isSearching) { + if (!this.select._isSearching) { return } @@ -170,7 +172,7 @@ export default function selectFormComponent({ return } - if (! this.isEmpty) { + if (!this.isEmpty) { this.select._clearNotice() } @@ -184,7 +186,10 @@ export default function selectFormComponent({ this.select.setChoiceByValue(this.formatState(this.state)) } - if (this.isEmpty && (! [null, undefined, ''].includes(config.search))) { + if ( + this.isEmpty && + ![null, undefined, ''].includes(config.search) + ) { this.select._displayNotice(noSearchResultsMessage) } }, @@ -210,7 +215,7 @@ export default function selectFormComponent({ let results = [] - if (! [null, undefined, ''].includes(search)) { + if (![null, undefined, ''].includes(search)) { results = await getSearchResultsUsing(search) } else { results = await getOptionsUsing()