From 62828bfcbabedf0821fc6d1b5462b7d3057dd814 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 5 Jun 2023 10:57:15 +0200 Subject: [PATCH 1/4] Respect Warning settings before calling custom handler --- src/Error/Warning.php | 35 +++++++++++++---------- tests/Executor/ExecutorLazySchemaTest.php | 14 +++++---- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/Error/Warning.php b/src/Error/Warning.php index ac7a56028..850884dba 100644 --- a/src/Error/Warning.php +++ b/src/Error/Warning.php @@ -40,7 +40,7 @@ final class Warning * * @api */ - public static function setWarningHandler(callable $warningHandler = null): void + public static function setWarningHandler(?callable $warningHandler): void { self::$warningHandler = $warningHandler; } @@ -59,7 +59,7 @@ public static function setWarningHandler(callable $warningHandler = null): void public static function suppress($suppress = true): void { if ($suppress === true) { - self::$enableWarnings = 0; + self::$enableWarnings = self::NONE; } elseif ($suppress === false) { self::$enableWarnings = self::ALL; // @phpstan-ignore-next-line necessary until we can use proper unions @@ -87,7 +87,7 @@ public static function enable($enable = true): void if ($enable === true) { self::$enableWarnings = self::ALL; } elseif ($enable === false) { - self::$enableWarnings = 0; + self::$enableWarnings = self::NONE; // @phpstan-ignore-next-line necessary until we can use proper unions } elseif (\is_int($enable)) { self::$enableWarnings |= $enable; @@ -97,26 +97,31 @@ public static function enable($enable = true): void } } - public static function warnOnce(string $errorMessage, int $warningId, int $messageLevel = null): void + public static function warnOnce(string $errorMessage, int $warningId, int $messageLevel = \E_USER_WARNING): void { - $messageLevel ??= \E_USER_WARNING; - - if (self::$warningHandler !== null) { - (self::$warningHandler)($errorMessage, $warningId, $messageLevel); - } elseif ((self::$enableWarnings & $warningId) > 0 && ! isset(self::$warned[$warningId])) { - self::$warned[$warningId] = true; - \trigger_error($errorMessage, $messageLevel); + if (isset(self::$warned[$warningId])) { + return; } + + self::warn($errorMessage, $warningId, $messageLevel); } - public static function warn(string $errorMessage, int $warningId, int $messageLevel = null): void + public static function warn(string $errorMessage, int $warningId, int $messageLevel = \E_USER_WARNING): void { - $messageLevel ??= \E_USER_WARNING; + if (! self::shouldWarn($warningId)) { + return; + } + self::$warned[$warningId] = true; - if (self::$warningHandler !== null) { + if (isset(self::$warningHandler)) { (self::$warningHandler)($errorMessage, $warningId, $messageLevel); - } elseif ((self::$enableWarnings & $warningId) > 0) { + } else { \trigger_error($errorMessage, $messageLevel); } } + + private static function shouldWarn(int $warningId): bool + { + return (self::$enableWarnings & $warningId) > 0; + } } diff --git a/tests/Executor/ExecutorLazySchemaTest.php b/tests/Executor/ExecutorLazySchemaTest.php index 948806ef5..622f51cc5 100644 --- a/tests/Executor/ExecutorLazySchemaTest.php +++ b/tests/Executor/ExecutorLazySchemaTest.php @@ -129,15 +129,17 @@ public function testWarnsAboutSlowIsTypeOfForLazySchema(): void $result = Executor::execute($schema, Parser::parse($query)); self::assertEquals($expected, $result); + $warnings = []; + Warning::setWarningHandler(function ($warning) use (&$warnings): void { + $warnings[]= $warning; + }); Warning::enable(Warning::WARNING_FULL_SCHEMA_SCAN); $result = Executor::execute($schema, Parser::parse($query)); - self::assertCount(1, $result->errors); - $error = $result->errors[0] ?? null; - self::assertInstanceOf(Error::class, $error); - self::assertSame( + self::assertEquals($expected, $result); + + self::assertSame([ 'GraphQL Interface Type `Pet` returned `null` from its `resolveType` function for value: instance of GraphQL\Tests\Executor\TestClasses\Dog. Switching to slow resolution method using `isTypeOf` of all possible implementations. It requires full schema scan and degrades query performance significantly. Make sure your `resolveType` function always returns a valid implementation or throws.', - $error->getMessage() - ); + ], $warnings); } public function testHintsOnConflictingTypeInstancesInDefinitions(): void From 8f1bf634690086da6936a760a789a00b6a792641 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Mon, 5 Jun 2023 10:58:03 +0200 Subject: [PATCH 2/4] cl --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 93d19b5aa..67c6e4d73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Changed + +- Respect settings in `GraphQL\Error\Warning` before calling custom `$warningHandler` + ## v15.5.0 ### Added From 3500e13de9e05c6d8fbb1a1155050fdf894f2331 Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 5 Jun 2023 08:58:43 +0000 Subject: [PATCH 3/4] Prettify docs --- docs/class-reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/class-reference.md b/docs/class-reference.md index ee08e66ab..0f2aa6b47 100644 --- a/docs/class-reference.md +++ b/docs/class-reference.md @@ -1691,7 +1691,7 @@ const ALL = 63; * * @api */ -static function setWarningHandler(?callable $warningHandler = null): void +static function setWarningHandler(?callable $warningHandler): void ``` ```php From 82dd89010030eba6ffeded989c9fdde769ba8520 Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 15 Jan 2024 16:07:42 +0000 Subject: [PATCH 4/4] Apply php-cs-fixer changes --- tests/Executor/ExecutorLazySchemaTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Executor/ExecutorLazySchemaTest.php b/tests/Executor/ExecutorLazySchemaTest.php index bd8beb19e..3a2161fcb 100644 --- a/tests/Executor/ExecutorLazySchemaTest.php +++ b/tests/Executor/ExecutorLazySchemaTest.php @@ -131,7 +131,7 @@ public function testWarnsAboutSlowIsTypeOfForLazySchema(): void $warnings = []; Warning::setWarningHandler(function ($warning) use (&$warnings): void { - $warnings[]= $warning; + $warnings[] = $warning; }); Warning::enable(Warning::WARNING_FULL_SCHEMA_SCAN); $result = Executor::execute($schema, Parser::parse($query));