From 139e329abe62c349b5c9ee28ea4449b52cbd66c7 Mon Sep 17 00:00:00 2001 From: tminich Date: Thu, 5 Jun 2025 07:31:50 +0000 Subject: [PATCH 1/7] Apply php-cs-fixer changes --- src/Auth/BaseCanDirective.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Auth/BaseCanDirective.php b/src/Auth/BaseCanDirective.php index 95f5a8d010..fc600bc57a 100644 --- a/src/Auth/BaseCanDirective.php +++ b/src/Auth/BaseCanDirective.php @@ -3,7 +3,6 @@ namespace Nuwave\Lighthouse\Auth; use Illuminate\Contracts\Auth\Access\Gate; -use Nuwave\Lighthouse\Auth\Contracts\ResolvesDuringAuthorization; use Nuwave\Lighthouse\Exceptions\AuthorizationException; use Nuwave\Lighthouse\Execution\ResolveInfo; use Nuwave\Lighthouse\Schema\Directives\BaseDirective; From b471974f90e5e800bdc73c420dd831f158cf7784 Mon Sep 17 00:00:00 2001 From: Tobias Minich Date: Fri, 6 Jun 2025 08:59:08 +0000 Subject: [PATCH 2/7] Test for accessing regular php properties --- .../Integration/Models/PropertyAccessTest.php | 99 +++++++++++++++++++ tests/Utils/Models/User.php | 11 +++ 2 files changed, 110 insertions(+) create mode 100644 tests/Integration/Models/PropertyAccessTest.php diff --git a/tests/Integration/Models/PropertyAccessTest.php b/tests/Integration/Models/PropertyAccessTest.php new file mode 100644 index 0000000000..c7a7478d48 --- /dev/null +++ b/tests/Integration/Models/PropertyAccessTest.php @@ -0,0 +1,99 @@ +create(['name' => 'foobar']); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + { + user(id: 1) { + name + } + } + ')->assertJson([ + 'data' => [ + 'user' => [ + 'name' => 'foobar', + ], + ], + ]); + } + + public function testLaravelFunctionProperty(): void + { + factory(User::class)->create(); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + { + user(id: 1) { + laravel_function_property + } + } + ')->assertJson([ + 'data' => [ + 'user' => [ + 'laravel_function_property' => 'foo', + ], + ], + ]); + } + + public function testPhpProperty(): void + { + factory(User::class)->create(); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + { + user(id: 1) { + php_property + } + } + ')->assertJson([ + 'data' => [ + 'user' => [ + 'php_property' => 'foo', + ], + ], + ]); + } +} diff --git a/tests/Utils/Models/User.php b/tests/Utils/Models/User.php index 25e590244f..88113d8008 100644 --- a/tests/Utils/Models/User.php +++ b/tests/Utils/Models/User.php @@ -37,6 +37,7 @@ * * Virtual * @property-read string|null $company_name + * @property-read string $laravel_function_property * * Relations * @property-read \Illuminate\Database\Eloquent\Collection $alternateConnections @@ -177,4 +178,14 @@ public function nonRelationPrimitive(): string { return 'foo'; } + + /** + * For property tests + */ + public function getLaravelFunctionPropertyAttribute(): string + { + return 'foo'; + } + + public string $php_property = 'foo'; } From 017de752fb9f47b170e5cb97583d88defa964d03 Mon Sep 17 00:00:00 2001 From: tminich Date: Fri, 6 Jun 2025 09:00:26 +0000 Subject: [PATCH 3/7] Apply php-cs-fixer changes --- tests/Integration/Models/PropertyAccessTest.php | 2 +- tests/Utils/Models/User.php | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/Integration/Models/PropertyAccessTest.php b/tests/Integration/Models/PropertyAccessTest.php index c7a7478d48..981ae5af8d 100644 --- a/tests/Integration/Models/PropertyAccessTest.php +++ b/tests/Integration/Models/PropertyAccessTest.php @@ -1,4 +1,4 @@ - Date: Thu, 12 Jun 2025 11:22:50 +0200 Subject: [PATCH 4/7] clean up --- .../Integration/Models/PropertyAccessTest.php | 43 ++++++++++++------- tests/Utils/Models/User.php | 5 ++- 2 files changed, 31 insertions(+), 17 deletions(-) diff --git a/tests/Integration/Models/PropertyAccessTest.php b/tests/Integration/Models/PropertyAccessTest.php index 981ae5af8d..7173573aaa 100644 --- a/tests/Integration/Models/PropertyAccessTest.php +++ b/tests/Integration/Models/PropertyAccessTest.php @@ -1,15 +1,20 @@ create(['name' => 'foobar']); + $name = 'foobar'; + + $user = factory(User::class)->make(); + assert($user instanceof User); + $user->name = $name; + $user->save(); $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' - { - user(id: 1) { + query ($id: ID!) { + user(id: $id) { name } } - ')->assertJson([ + ', [ + 'id' => $user->id, + ])->assertJson([ 'data' => [ 'user' => [ - 'name' => 'foobar', + 'name' => $name, ], ], ]); @@ -39,7 +46,8 @@ public function testLaravelDatabaseProperty(): void public function testLaravelFunctionProperty(): void { - factory(User::class)->create(); + $user = factory(User::class)->create(); + assert($user instanceof User); $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' - { - user(id: 1) { + query ($id: ID!) { + user(id: $id) { laravel_function_property } } - ')->assertJson([ + ', [ + 'id' => $user->id, + ])->assertJson([ 'data' => [ 'user' => [ 'laravel_function_property' => 'foo', @@ -69,7 +79,8 @@ public function testLaravelFunctionProperty(): void public function testPhpProperty(): void { - factory(User::class)->create(); + $user = factory(User::class)->create(); + assert($user instanceof User); $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' - { - user(id: 1) { + query ($id: ID!) { + user(id: $id) { php_property } } - ')->assertJson([ + ', [ + 'id' => $user->id, + ])->assertJson([ 'data' => [ 'user' => [ 'php_property' => 'foo', diff --git a/tests/Utils/Models/User.php b/tests/Utils/Models/User.php index 322970e332..ab9177da19 100644 --- a/tests/Utils/Models/User.php +++ b/tests/Utils/Models/User.php @@ -37,7 +37,7 @@ * * Virtual * @property-read string|null $company_name - * @property-read string $laravel_function_property + * @property-read string $laravel_function_property @see \Tests\Integration\Models\PropertyAccessTest * * Relations * @property-read \Illuminate\Database\Eloquent\Collection $alternateConnections @@ -179,11 +179,12 @@ public function nonRelationPrimitive(): string return 'foo'; } - /** For property tests */ + /** @see \Tests\Integration\Models\PropertyAccessTest */ public function getLaravelFunctionPropertyAttribute(): string { return 'foo'; } + /** @see \Tests\Integration\Models\PropertyAccessTest */ public string $php_property = 'foo'; } From 5bf8a536c30058470ff11fd1aeecffb597323050 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 12 Jun 2025 12:05:35 +0200 Subject: [PATCH 5/7] add test for https://github.com/nuwave/lighthouse/issues/1671 --- .../Integration/Models/PropertyAccessTest.php | 35 +++++++++++++++++++ tests/Utils/Models/User.php | 20 +++++++---- 2 files changed, 48 insertions(+), 7 deletions(-) diff --git a/tests/Integration/Models/PropertyAccessTest.php b/tests/Integration/Models/PropertyAccessTest.php index 7173573aaa..7c5c594a81 100644 --- a/tests/Integration/Models/PropertyAccessTest.php +++ b/tests/Integration/Models/PropertyAccessTest.php @@ -77,6 +77,7 @@ public function testLaravelFunctionProperty(): void ]); } + /** @see https://github.com/nuwave/lighthouse/issues/2687 */ public function testPhpProperty(): void { $user = factory(User::class)->create(); @@ -109,4 +110,38 @@ public function testPhpProperty(): void ], ]); } + + /** @see https://github.com/nuwave/lighthouse/issues/1671 */ + public function testExpensivePropertyIsOnlyCalledOnce(): void + { + $user = factory(User::class)->create(); + assert($user instanceof User); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + query ($id: ID!) { + user(id: $id) { + expensive_property + } + } + ', [ + 'id' => $user->id, + ])->assertJson([ + 'data' => [ + 'user' => [ + 'expensive_property' => 1, + ], + ], + ]); + } } diff --git a/tests/Utils/Models/User.php b/tests/Utils/Models/User.php index 3e4ee97f64..25076ceee7 100644 --- a/tests/Utils/Models/User.php +++ b/tests/Utils/Models/User.php @@ -38,6 +38,7 @@ * Virtual * @property-read string|null $company_name * @property-read string $laravel_function_property @see \Tests\Integration\Models\PropertyAccessTest + * @property-read int $expensive_property @see \Tests\Integration\Models\PropertyAccessTest * * Relations * @property-read \Illuminate\Database\Eloquent\Collection $alternateConnections @@ -64,6 +65,9 @@ final class User extends Authenticatable 'email_verified_at' => 'datetime', ]; + /** @see \Tests\Integration\Models\PropertyAccessTest */ + public string $php_property = 'foo'; + public function newEloquentBuilder($query): UserBuilder { return new UserBuilder($query); @@ -147,9 +151,7 @@ public function tasksCountLoaded(): bool public function postsCommentsLoaded(): bool { return $this->relationLoaded('posts') - && $this - ->posts - ->first() + && $this->posts->first() ?->relationLoaded('comments'); } @@ -162,9 +164,7 @@ public function tasksAndPostsCommentsLoaded(): bool public function postsTaskLoaded(): bool { return $this->relationLoaded('posts') - && $this - ->posts - ->first() + && $this->posts->first() ?->relationLoaded('task'); } @@ -186,5 +186,11 @@ public function getLaravelFunctionPropertyAttribute(): string } /** @see \Tests\Integration\Models\PropertyAccessTest */ - public string $php_property = 'foo'; + public function getExpensivePropertyAttribute(): int + { + static $counter = 0; + $counter++; + + return $counter; + } } From 9829c244cdcc23ab14516fa9771f8583fcc919ae Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 12 Jun 2025 12:28:09 +0200 Subject: [PATCH 6/7] add more tests, attempt to fix everything --- src/LighthouseServiceProvider.php | 25 ++++++- .../Integration/Models/PropertyAccessTest.php | 72 ++++++++++++++++++- tests/Utils/Models/User.php | 24 ++++++- 3 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index 46fd630391..f839cbbce0 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -6,9 +6,13 @@ use GraphQL\Error\Error; use GraphQL\Error\ProvidesExtensions; use GraphQL\Executor\ExecutionResult; +use GraphQL\Executor\Executor; +use GraphQL\Type\Definition\ResolveInfo; +use GraphQL\Utils\Utils; use Illuminate\Contracts\Config\Repository as ConfigRepository; use Illuminate\Contracts\Debug\ExceptionHandler as ExceptionHandlerContract; use Illuminate\Contracts\Events\Dispatcher as EventsDispatcher; +use Illuminate\Database\Eloquent\Model; use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler; use Illuminate\Http\JsonResponse; use Illuminate\Support\ServiceProvider; @@ -99,14 +103,14 @@ public function provideSubscriptionResolver(FieldValue $fieldValue): \Closure }); $this->app->bind(ProvidesValidationRules::class, CacheableValidationRulesProvider::class); - - $this->commands(self::COMMANDS); } public function boot(ConfigRepository $configRepository, EventsDispatcher $dispatcher): void { $dispatcher->listen(RegisterDirectiveNamespaces::class, static fn (): string => __NAMESPACE__ . '\\Schema\\Directives'); + $this->commands(self::COMMANDS); + $this->publishes([ __DIR__ . '/lighthouse.php' => $this->app->configPath() . '/lighthouse.php', ], 'lighthouse-config'); @@ -142,6 +146,23 @@ public function boot(ConfigRepository $configRepository, EventsDispatcher $dispa return new JsonResponse($serializableResult); }); } + + Executor::setDefaultFieldResolver(static function ($objectLikeValue, array $args, $contextValue, ResolveInfo $info): mixed { + $fieldName = $info->fieldName; + + if ($objectLikeValue instanceof Model) { + $property = $objectLikeValue->getAttribute($fieldName); + if ($property === null && property_exists($objectLikeValue, $fieldName)) { + $property = $objectLikeValue->{$fieldName}; + } + } else { + $property = Utils::extractKey($objectLikeValue, $fieldName); + } + + return $property instanceof \Closure + ? $property($objectLikeValue, $args, $contextValue, $info) + : $property; + }); } protected function loadRoutesFrom($path): void diff --git a/tests/Integration/Models/PropertyAccessTest.php b/tests/Integration/Models/PropertyAccessTest.php index 7c5c594a81..6d3cbc18f6 100644 --- a/tests/Integration/Models/PropertyAccessTest.php +++ b/tests/Integration/Models/PropertyAccessTest.php @@ -71,7 +71,7 @@ public function testLaravelFunctionProperty(): void ])->assertJson([ 'data' => [ 'user' => [ - 'laravel_function_property' => 'foo', + 'laravel_function_property' => User::FUNCTION_PROPERTY_ATTRIBUTE_VALUE, ], ], ]); @@ -105,7 +105,75 @@ public function testPhpProperty(): void ])->assertJson([ 'data' => [ 'user' => [ - 'php_property' => 'foo', + 'php_property' => User::PHP_PROPERTY_VALUE, + ], + ], + ]); + } + + /** @see https://github.com/nuwave/lighthouse/issues/2687 */ + public function testPrefersAttributeAccessorThatShadowsPhpProperty(): void + { + $user = factory(User::class)->create(); + assert($user instanceof User); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + query ($id: ID!) { + user(id: $id) { + incrementing + } + } + ', [ + 'id' => $user->id, + ])->assertJson([ + 'data' => [ + 'user' => [ + 'incrementing' => User::INCREMENTING_ATTRIBUTE_VALUE, + ], + ], + ]); + } + + /** @see https://github.com/nuwave/lighthouse/issues/2687 */ + public function testPrefersAttributeAccessorNullThatShadowsPhpProperty(): void + { + $user = factory(User::class)->create(); + assert($user instanceof User); + + $this->schema = /** @lang GraphQL */ <<graphQL(/** @lang GraphQL */ ' + query ($id: ID!) { + user(id: $id) { + exists + } + } + ', [ + 'id' => $user->id, + ])->assertJson([ + 'data' => [ + 'user' => [ + 'exists' => null, ], ], ]); diff --git a/tests/Utils/Models/User.php b/tests/Utils/Models/User.php index 25076ceee7..2cacff91af 100644 --- a/tests/Utils/Models/User.php +++ b/tests/Utils/Models/User.php @@ -53,6 +53,12 @@ */ final class User extends Authenticatable { + public const INCREMENTING_ATTRIBUTE_VALUE = 'value of the incrementing attribute'; + + public const FUNCTION_PROPERTY_ATTRIBUTE_VALUE = 'value of the virtual property'; + + public const PHP_PROPERTY_VALUE = 'value of the PHP property'; + /** * Ensure that this is functionally equivalent to leaving this as null. * @@ -66,7 +72,7 @@ final class User extends Authenticatable ]; /** @see \Tests\Integration\Models\PropertyAccessTest */ - public string $php_property = 'foo'; + public string $php_property = self::PHP_PROPERTY_VALUE; public function newEloquentBuilder($query): UserBuilder { @@ -182,15 +188,27 @@ public function nonRelationPrimitive(): string /** @see \Tests\Integration\Models\PropertyAccessTest */ public function getLaravelFunctionPropertyAttribute(): string { - return 'foo'; + return self::FUNCTION_PROPERTY_ATTRIBUTE_VALUE; } /** @see \Tests\Integration\Models\PropertyAccessTest */ public function getExpensivePropertyAttribute(): int { static $counter = 0; - $counter++; + ++$counter; return $counter; } + + /** @see \Tests\Integration\Models\PropertyAccessTest */ + public function getIncrementingAttribute(): string + { + return self::INCREMENTING_ATTRIBUTE_VALUE; + } + + /** @see \Tests\Integration\Models\PropertyAccessTest */ + public function getExistsAttribute(): ?bool // @phpstan-ignore return.unusedType + { + return null; + } } From be73761382ddc8c2a99aba8b7605730c5272803f Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 12 Jun 2025 12:35:56 +0200 Subject: [PATCH 7/7] document --- .../digging-deeper/extending-lighthouse.md | 8 +++++--- src/LighthouseServiceProvider.php | 20 +++++++++++++++++-- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/docs/master/digging-deeper/extending-lighthouse.md b/docs/master/digging-deeper/extending-lighthouse.md index 45df26493c..1272b64896 100644 --- a/docs/master/digging-deeper/extending-lighthouse.md +++ b/docs/master/digging-deeper/extending-lighthouse.md @@ -35,15 +35,17 @@ final class SomePackageServiceProvider extends ServiceProvider ## Changing the default resolver -Lighthouse will fall back to using [webonyx's default resolver](https://webonyx.github.io/graphql-php/data-fetching/#default-field-resolver) +Lighthouse overrides [webonyx's default resolver](https://webonyx.github.io/graphql-php/data-fetching#default-field-resolver) for non-root fields, [see resolver precedence](../the-basics/fields.md#resolver-precedence). -You may overwrite this by passing a `callable` to `GraphQL\Executor\Executor::setDefaultFieldResolver()`. +See `Nuwave\Lighthouse\LighthouseServiceProvider::defaultFieldResolver()` for the implementation. + +You may override this by calling `GraphQL\Executor\Executor::setDefaultFieldResolver()` in your service provider's `boot()` method. ## Use a custom `GraphQLContext` The context is the third argument of any resolver function. -You may replace the default `\Nuwave\Lighthouse\Schema\Context` with your own +You may replace the default `Nuwave\Lighthouse\Schema\Context` with your own implementation of the interface `Nuwave\Lighthouse\Support\Contracts\GraphQLContext`. The following example is just a starting point of what you can do: diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index f839cbbce0..68f79f6181 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -147,7 +147,23 @@ public function boot(ConfigRepository $configRepository, EventsDispatcher $dispa }); } - Executor::setDefaultFieldResolver(static function ($objectLikeValue, array $args, $contextValue, ResolveInfo $info): mixed { + Executor::setDefaultFieldResolver([static::class, 'defaultFieldResolver']); + } + + /** + * The default field resolver for GraphQL queries. + * + * This method is used to resolve fields on the object-like value returned by a resolver. + * It checks if the value is an Eloquent model and retrieves the attribute or property accordingly. + * Otherwise, it falls back to the default behavior from webonyx/graphql-php's default field resolver. + * + * @see \GraphQL\Executor\Executor::defaultFieldResolver() + * + * @return callable(mixed $objectLikeValue, array $args, mixed $contextValue, ResolveInfo $info): mixed + */ + public static function defaultFieldResolver(): callable + { + return static function ($objectLikeValue, array $args, $contextValue, ResolveInfo $info): mixed { $fieldName = $info->fieldName; if ($objectLikeValue instanceof Model) { @@ -162,7 +178,7 @@ public function boot(ConfigRepository $configRepository, EventsDispatcher $dispa return $property instanceof \Closure ? $property($objectLikeValue, $args, $contextValue, $info) : $property; - }); + }; } protected function loadRoutesFrom($path): void