diff --git a/CHANGELOG.md b/CHANGELOG.md index f55a42c31..9feab6a72 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 +### Added + +- Add `@specifiedBy` built-in directive and `ScalarType::$specifiedByURL` property https://github.com/webonyx/graphql-php/pull/1913 + ## v15.32.3 ### Fixed diff --git a/docs/type-definitions/directives.md b/docs/type-definitions/directives.md index b0bef2536..a3829ad9d 100644 --- a/docs/type-definitions/directives.md +++ b/docs/type-definitions/directives.md @@ -4,10 +4,13 @@ The directive is a way for a client to give GraphQL server additional context an the query. The directive can be attached to a field or fragment and can affect the execution of the query in any way the server desires. -GraphQL specification includes two built-in directives: +The GraphQL specification includes the following built-in directives: - **@include(if: Boolean)** Only include this field or fragment in the result if the argument is **true** - **@skip(if: Boolean)** Skip this field or fragment if the argument is **true** +- **@deprecated(reason: String)** Marks a field, argument, input field, or enum value as deprecated with an optional reason +- **@specifiedBy(url: String!)** Links a custom scalar type to a human-readable specification URL +- **@oneOf** Marks an input object type as a "oneof" input, requiring exactly one field to be non-null For example: diff --git a/docs/type-definitions/scalars.md b/docs/type-definitions/scalars.md index 09cc82a35..55b71ec7c 100644 --- a/docs/type-definitions/scalars.md +++ b/docs/type-definitions/scalars.md @@ -107,6 +107,35 @@ $emailType = new CustomScalarType([ Keep in mind the passed functions will be called statically, so a passed in `callable` such as `[Foo::class, 'bar']` should only reference static class methods. +## Linking a Scalar to Its Specification + +The `@specifiedBy` built-in directive lets you attach a URL to a custom scalar that specifies the behavior of that scalar. +This URL is exposed through introspection and printed in SDL output. + +Pass `specifiedByURL` when constructing the type: + +```php +use GraphQL\Type\Definition\CustomScalarType; + +$uuidType = new CustomScalarType([ + 'name' => 'UUID', + 'specifiedByURL' => 'https://tools.ietf.org/html/rfc4122', + 'serialize' => static function ($value) { /* ... */ }, + 'parseValue' => static function ($value) { /* ... */ }, + 'parseLiteral' => static function ($valueNode, ?array $variables = null) { /* ... */ }, +]); + +$uuidType->specifiedByURL; // "https://tools.ietf.org/html/rfc4122" +``` + +When building a schema from SDL, `@specifiedBy` is parsed automatically: + +```graphql +scalar UUID @specifiedBy(url: "https://tools.ietf.org/html/rfc4122") +``` + +And `SchemaPrinter` will emit the directive when printing a schema that contains a scalar with a `specifiedByURL`. + ## Overriding Built-in Scalars You can override built-in scalars (`String`, `Int`, `Float`, `Boolean`, `ID`) on a per-schema basis by passing a custom scalar with the same name through the `types` option. diff --git a/src/Language/Printer.php b/src/Language/Printer.php index 8bf04943e..bc061ad0f 100644 --- a/src/Language/Printer.php +++ b/src/Language/Printer.php @@ -403,7 +403,8 @@ protected static function p(?Node $node): string return BlockString::print($node->value); } - return json_encode($node->value, JSON_THROW_ON_ERROR | JSON_UNESCAPED_SLASHES); + // Do not escape unicode or slashes to keep the output readable + return json_encode($node->value, JSON_THROW_ON_ERROR | JSON_UNESCAPED_UNICODE | JSON_UNESCAPED_SLASHES); case $node instanceof UnionTypeDefinitionNode: $typesStr = static::printList($node->types, ' | '); diff --git a/src/Type/Definition/CustomScalarType.php b/src/Type/Definition/CustomScalarType.php index 80694dca9..cd85f8d5f 100644 --- a/src/Type/Definition/CustomScalarType.php +++ b/src/Type/Definition/CustomScalarType.php @@ -18,6 +18,7 @@ * serialize?: callable(mixed): mixed, * parseValue: callable(mixed): mixed, * parseLiteral: callable(ValueNode&Node, array|null): mixed, + * specifiedByURL?: string|null, * astNode?: ScalarTypeDefinitionNode|null, * extensionASTNodes?: array|null * } @@ -27,6 +28,7 @@ * serialize: callable(mixed): mixed, * parseValue?: callable(mixed): mixed, * parseLiteral?: callable(ValueNode&Node, array|null): mixed, + * specifiedByURL?: string|null, * astNode?: ScalarTypeDefinitionNode|null, * extensionASTNodes?: array|null * } diff --git a/src/Type/Definition/Directive.php b/src/Type/Definition/Directive.php index 80d1ce956..5ca82abd5 100644 --- a/src/Type/Definition/Directive.php +++ b/src/Type/Definition/Directive.php @@ -22,10 +22,15 @@ class Directive public const DEFAULT_DEPRECATION_REASON = 'No longer supported'; public const INCLUDE_NAME = 'include'; - public const IF_ARGUMENT_NAME = 'if'; public const SKIP_NAME = 'skip'; + public const IF_ARGUMENT_NAME = 'if'; + public const DEPRECATED_NAME = 'deprecated'; public const REASON_ARGUMENT_NAME = 'reason'; + + public const SPECIFIED_BY_NAME = 'specifiedBy'; + public const URL_ARGUMENT_NAME = 'url'; + public const ONE_OF_NAME = 'oneOf'; /** @@ -82,6 +87,7 @@ public static function builtInDirectives(): array self::INCLUDE_NAME => self::includeDirective(), self::SKIP_NAME => self::skipDirective(), self::DEPRECATED_NAME => self::deprecatedDirective(), + self::SPECIFIED_BY_NAME => self::specifiedByDirective(), self::ONE_OF_NAME => self::oneOfDirective(), ]; } @@ -167,6 +173,23 @@ public static function oneOfDirective(): Directive ]); } + public static function specifiedByDirective(): Directive + { + return self::$internalDirectives[self::SPECIFIED_BY_NAME] ??= new self([ + 'name' => self::SPECIFIED_BY_NAME, + 'description' => 'Exposes a URL that specifies the behavior of this scalar.', + 'locations' => [ + DirectiveLocation::SCALAR, + ], + 'args' => [ + self::URL_ARGUMENT_NAME => [ + 'type' => Type::nonNull(Type::string()), + 'description' => 'The URL that specifies the behavior of this scalar.', + ], + ], + ]); + } + public static function isBuiltInDirective(self $directive): bool { return array_key_exists($directive->name, self::builtInDirectives()); diff --git a/src/Type/Definition/ScalarType.php b/src/Type/Definition/ScalarType.php index 48a97c7db..a78a4a86b 100644 --- a/src/Type/Definition/ScalarType.php +++ b/src/Type/Definition/ScalarType.php @@ -28,6 +28,7 @@ * @phpstan-type ScalarConfig array{ * name?: string|null, * description?: string|null, + * specifiedByURL?: string|null, * astNode?: ScalarTypeDefinitionNode|null, * extensionASTNodes?: array|null * } @@ -38,6 +39,8 @@ abstract class ScalarType extends Type implements OutputType, InputType, LeafTyp public ?ScalarTypeDefinitionNode $astNode; + public ?string $specifiedByURL; + /** @var array */ public array $extensionASTNodes; @@ -53,6 +56,7 @@ public function __construct(array $config = []) { $this->name = $config['name'] ?? $this->inferName(); $this->description = $config['description'] ?? $this->description ?? null; + $this->specifiedByURL = $config['specifiedByURL'] ?? $this->specifiedByURL ?? null; $this->astNode = $config['astNode'] ?? null; $this->extensionASTNodes = $config['extensionASTNodes'] ?? []; diff --git a/src/Type/Introspection.php b/src/Type/Introspection.php index 806658611..648540ea6 100644 --- a/src/Type/Introspection.php +++ b/src/Type/Introspection.php @@ -31,6 +31,7 @@ * descriptions?: bool, * directiveIsRepeatable?: bool, * schemaDescription?: bool, + * specifiedByURL?: bool, * typeIsOneOf?: bool, * } * @@ -41,6 +42,12 @@ * - directiveIsRepeatable * Include field `isRepeatable` for directives? * Default: false + * - schemaDescription + * Include `description` on the schema? + * Default: false + * - specifiedByURL + * Include field `specifiedByURL` for scalar types? + * Default: false * - typeIsOneOf * Include field `isOneOf` for types? * Default: false @@ -87,6 +94,7 @@ public static function getIntrospectionQuery(array $options = []): string 'descriptions' => true, 'directiveIsRepeatable' => false, 'schemaDescription' => false, + 'specifiedByURL' => false, 'typeIsOneOf' => false, ], $options); @@ -99,6 +107,9 @@ public static function getIntrospectionQuery(array $options = []): string $schemaDescription = $optionsWithDefaults['schemaDescription'] ? $descriptions : ''; + $specifiedByURL = $optionsWithDefaults['specifiedByURL'] + ? 'specifiedByURL' + : ''; $typeIsOneOf = $optionsWithDefaults['typeIsOneOf'] ? 'isOneOf' : ''; @@ -129,6 +140,7 @@ public static function getIntrospectionQuery(array $options = []): string kind name {$descriptions} + {$specifiedByURL} {$typeIsOneOf} fields(includeDeprecated: true) { name @@ -227,6 +239,7 @@ public static function fromSchema(Schema $schema, array $options = []): array $optionsWithDefaults = array_merge([ 'directiveIsRepeatable' => true, 'schemaDescription' => true, + 'specifiedByURL' => true, 'typeIsOneOf' => true, ], $options); @@ -361,6 +374,12 @@ public static function _type(): ObjectType ? $type->description : null, ], + 'specifiedByURL' => [ + 'type' => Type::string(), + 'resolve' => static fn (Type $type): ?string => $type instanceof ScalarType + ? $type->specifiedByURL + : null, + ], 'fields' => [ 'type' => Type::listOf(Type::nonNull(self::_field())), 'args' => [ diff --git a/src/Utils/ASTDefinitionBuilder.php b/src/Utils/ASTDefinitionBuilder.php index 1a9d4959b..9ccb1c135 100644 --- a/src/Utils/ASTDefinitionBuilder.php +++ b/src/Utils/ASTDefinitionBuilder.php @@ -24,6 +24,7 @@ use GraphQL\Language\AST\ObjectTypeExtensionNode; use GraphQL\Language\AST\ScalarTypeDefinitionNode; use GraphQL\Language\AST\ScalarTypeExtensionNode; +use GraphQL\Language\AST\StringValueNode; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeExtensionNode; use GraphQL\Language\AST\TypeNode; @@ -403,7 +404,6 @@ public function buildField(FieldDefinitionNode $field, object $node): array * @param EnumValueDefinitionNode|FieldDefinitionNode|InputValueDefinitionNode $node * * @throws \Exception - * @throws \ReflectionException * @throws InvariantViolation */ private function getDeprecationReason(Node $node): ?string @@ -416,6 +416,33 @@ private function getDeprecationReason(Node $node): ?string return $deprecated['reason'] ?? null; } + /** + * Returns the specifiedBy URL from a scalar type's definition and extension directives, + * reading directly from the AST to safely handle custom @specifiedBy directive definitions + * with different argument shapes. + * + * @param array $extensionNodes + */ + private function getSpecifiedByURL(ScalarTypeDefinitionNode $def, array $extensionNodes = []): ?string + { + foreach ([$def, ...$extensionNodes] as $node) { + foreach ($node->directives as $directive) { + if ($directive->name->value !== Directive::SPECIFIED_BY_NAME) { + continue; + } + + foreach ($directive->arguments as $argument) { + if ($argument->name->value === Directive::URL_ARGUMENT_NAME + && $argument->value instanceof StringValueNode) { + return $argument->value->value; + } + } + } + } + + return null; + } + /** * @param array $nodes * @@ -533,6 +560,7 @@ private function makeScalarDef(ScalarTypeDefinitionNode $def): CustomScalarType 'serialize' => static fn ($value) => $value, 'astNode' => $def, 'extensionASTNodes' => $extensionASTNodes, + 'specifiedByURL' => $this->getSpecifiedByURL($def, $extensionASTNodes), ]); } diff --git a/src/Utils/BuildClientSchema.php b/src/Utils/BuildClientSchema.php index 40845936a..33276c04b 100644 --- a/src/Utils/BuildClientSchema.php +++ b/src/Utils/BuildClientSchema.php @@ -314,7 +314,7 @@ private function buildType(array $type): NamedType } /** - * @param array $scalar + * @param array $scalar * * @throws InvariantViolation */ @@ -323,6 +323,7 @@ private function buildScalarDef(array $scalar): ScalarType return new CustomScalarType([ 'name' => $scalar['name'], 'description' => $scalar['description'], + 'specifiedByURL' => $scalar['specifiedByURL'] ?? null, 'serialize' => static fn ($value) => $value, ]); } diff --git a/src/Utils/BuildSchema.php b/src/Utils/BuildSchema.php index 1e47cf8b4..863946f92 100644 --- a/src/Utils/BuildSchema.php +++ b/src/Utils/BuildSchema.php @@ -231,6 +231,9 @@ static function (string $typeName): Type { if (! isset($directivesByName['deprecated'])) { $directives[] = Directive::deprecatedDirective(); } + if (! isset($directivesByName['specifiedBy'])) { + $directives[] = Directive::specifiedByDirective(); + } if (! isset($directivesByName['oneOf'])) { $directives[] = Directive::oneOfDirective(); } diff --git a/src/Utils/SchemaExtender.php b/src/Utils/SchemaExtender.php index 51e7c1bc1..8a3155c55 100644 --- a/src/Utils/SchemaExtender.php +++ b/src/Utils/SchemaExtender.php @@ -14,6 +14,7 @@ use GraphQL\Language\AST\ScalarTypeExtensionNode; use GraphQL\Language\AST\SchemaDefinitionNode; use GraphQL\Language\AST\SchemaExtensionNode; +use GraphQL\Language\AST\StringValueNode; use GraphQL\Language\AST\TypeDefinitionNode; use GraphQL\Language\AST\TypeExtensionNode; use GraphQL\Language\AST\UnionTypeExtensionNode; @@ -226,12 +227,32 @@ protected function extendScalarType(ScalarType $type): CustomScalarType /** @var array $extensionASTNodes */ $extensionASTNodes = $this->extensionASTNodes($type); + $specifiedByURL = $type->specifiedByURL; + if ($specifiedByURL === null) { + foreach ($extensionASTNodes as $extensionNode) { + foreach ($extensionNode->directives as $directive) { + if ($directive->name->value !== Directive::SPECIFIED_BY_NAME) { + continue; + } + + foreach ($directive->arguments as $argument) { + if ($argument->name->value === Directive::URL_ARGUMENT_NAME + && $argument->value instanceof StringValueNode) { + $specifiedByURL = $argument->value->value; + break 3; + } + } + } + } + } + return new CustomScalarType([ 'name' => $type->name, 'description' => $type->description, 'serialize' => [$type, 'serialize'], 'parseValue' => [$type, 'parseValue'], 'parseLiteral' => [$type, 'parseLiteral'], + 'specifiedByURL' => $specifiedByURL, 'astNode' => $type->astNode, 'extensionASTNodes' => $extensionASTNodes, ]); diff --git a/src/Utils/SchemaPrinter.php b/src/Utils/SchemaPrinter.php index f30a19f0b..baae94391 100644 --- a/src/Utils/SchemaPrinter.php +++ b/src/Utils/SchemaPrinter.php @@ -364,11 +364,14 @@ protected static function printInputValue($arg): string * @phpstan-param Options $options * * @throws \JsonException + * @throws InvariantViolation + * @throws SerializationError */ protected static function printScalar(ScalarType $type, array $options): string { return static::printDescription($options, $type) - . "scalar {$type->name}"; + . "scalar {$type->name}" + . static::printSpecifiedBy($type); } /** @@ -455,6 +458,26 @@ protected static function printDeprecated($deprecation): string return " @deprecated(reason: {$reasonASTString})"; } + /** + * @throws \JsonException + * @throws InvariantViolation + * @throws SerializationError + */ + protected static function printSpecifiedBy(ScalarType $type): string + { + $url = $type->specifiedByURL; + if ($url === null) { + return ''; + } + + $urlAST = AST::astFromValue($url, Type::string()); + assert($urlAST instanceof StringValueNode); + + $urlASTString = Printer::doPrint($urlAST); + + return " @specifiedBy(url: {$urlASTString})"; + } + protected static function printImplementedInterfaces(ImplementingType $type): string { $interfaces = $type->getInterfaces(); diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php index 9d5a6bf12..231adc500 100644 --- a/src/Validator/Rules/QueryComplexity.php +++ b/src/Validator/Rules/QueryComplexity.php @@ -42,6 +42,9 @@ class QueryComplexity extends QuerySecurityRule protected QueryValidationContext $context; + /** @var array|null Lazily coerced variable values; reset per document. */ + private ?array $coercedVariableValues = null; + /** @throws \InvalidArgumentException */ public function __construct(int $maxQueryComplexity) { @@ -54,6 +57,7 @@ public function getVisitor(QueryValidationContext $context): array $this->context = $context; $this->variableDefs = new NodeList([]); $this->fieldNodeAndDefs = new \ArrayObject(); + $this->coercedVariableValues = null; return $this->invokeIfNeeded( $context, @@ -182,45 +186,62 @@ protected function fieldDefinition(FieldNode $field): ?FieldDefinition protected function directiveExcludesField(FieldNode $node): bool { foreach ($node->directives as $directiveNode) { - if ($directiveNode->name->value === Directive::DEPRECATED_NAME) { - return false; - } - - [$errors, $variableValues] = Values::getVariableValues( - $this->context->getSchema(), - $this->variableDefs, - $this->getRawVariableValues() - ); - if ($errors !== null && $errors !== []) { - throw new Error(implode("\n\n", array_map(static fn (Error $error): string => $error->getMessage(), $errors))); - } - if ($directiveNode->name->value === Directive::INCLUDE_NAME) { $includeArguments = Values::getArgumentValues( Directive::includeDirective(), $directiveNode, - $variableValues + $this->getCoercedVariableValues() ); assert(is_bool($includeArguments['if']), 'ensured by query validation'); - return ! $includeArguments['if']; + if (! $includeArguments['if']) { + return true; + } } if ($directiveNode->name->value === Directive::SKIP_NAME) { $skipArguments = Values::getArgumentValues( Directive::skipDirective(), $directiveNode, - $variableValues + $this->getCoercedVariableValues() ); assert(is_bool($skipArguments['if']), 'ensured by query validation'); - return $skipArguments['if']; + if ($skipArguments['if']) { + return true; + } } } return false; } + /** + * Coerce variable values once per document and cache them. + * + * @throws \Exception + * @throws InvariantViolation + * + * @return array + */ + private function getCoercedVariableValues(): array + { + if ($this->coercedVariableValues !== null) { + return $this->coercedVariableValues; + } + + [$errors, $variableValues] = Values::getVariableValues( + $this->context->getSchema(), + $this->variableDefs, + $this->getRawVariableValues() + ); + if ($errors !== null && $errors !== []) { + throw new Error(implode("\n\n", array_map(static fn (Error $error): string => $error->getMessage(), $errors))); + } + + return $this->coercedVariableValues = $variableValues ?? []; + } + /** @return array */ public function getRawVariableValues(): array { diff --git a/tests/Type/IntrospectionTest.php b/tests/Type/IntrospectionTest.php index ce558a7c2..08bc8489d 100644 --- a/tests/Type/IntrospectionTest.php +++ b/tests/Type/IntrospectionTest.php @@ -7,6 +7,7 @@ use GraphQL\Language\DirectiveLocation; use GraphQL\Language\SourceLocation; use GraphQL\Tests\ErrorHelper; +use GraphQL\Type\Definition\CustomScalarType; use GraphQL\Type\Definition\Directive; use GraphQL\Type\Definition\EnumType; use GraphQL\Type\Definition\FieldDefinition; @@ -246,6 +247,17 @@ public function testExecutesAnIntrospectionQuery(): void 'deprecationReason' => null, ], 3 => [ + 'name' => 'specifiedByURL', + 'args' => [], + 'type' => [ + 'kind' => 'SCALAR', + 'name' => 'String', + 'ofType' => null, + ], + 'isDeprecated' => false, + 'deprecationReason' => null, + ], + 4 => [ 'name' => 'fields', 'args' => [ 0 => [ @@ -280,7 +292,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 4 => [ + 5 => [ 'name' => 'interfaces', 'args' => [], 'type' => [ @@ -299,7 +311,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 5 => [ + 6 => [ 'name' => 'possibleTypes', 'args' => [], 'type' => [ @@ -318,7 +330,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 6 => [ + 7 => [ 'name' => 'enumValues', 'args' => [ 0 => [ @@ -353,7 +365,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 7 => [ + 8 => [ 'name' => 'inputFields', 'args' => [ 0 => [ @@ -388,7 +400,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 8 => [ + 9 => [ 'name' => 'ofType', 'args' => [], 'type' => [ @@ -399,7 +411,7 @@ public function testExecutesAnIntrospectionQuery(): void 'isDeprecated' => false, 'deprecationReason' => null, ], - 9 => [ + 10 => [ 'name' => 'isOneOf', 'args' => [], 'type' => [ @@ -1033,6 +1045,30 @@ public function testExecutesAnIntrospectionQuery(): void 3 => 'INPUT_FIELD_DEFINITION', ], ], + [ + 'name' => 'specifiedBy', + 'args' => [ + 0 => [ + 'name' => 'url', + 'type' => [ + 'kind' => 'NON_NULL', + 'name' => null, + 'ofType' => [ + 'kind' => 'SCALAR', + 'name' => 'String', + 'ofType' => null, + ], + ], + 'defaultValue' => null, + 'isDeprecated' => false, + 'deprecationReason' => null, + ], + ], + 'isRepeatable' => false, + 'locations' => [ + 0 => 'SCALAR', + ], + ], [ 'name' => 'oneOf', 'args' => [], @@ -2007,7 +2043,44 @@ public function testRespectsTheIncludeDeprecatedParameterForDirectiveArgs(): voi self::assertSame($expected, $result['data']['__schema']['directives'] ?? null); } - /** @see it('include "description" field on schema') */ + /** @see it('executes an introspection query') */ + public function testSpecifiedByURL(): void + { + $scalarType = new CustomScalarType([ + 'name' => 'UUID', + 'specifiedByURL' => 'https://tools.ietf.org/html/rfc4122', + 'serialize' => static fn ($value) => $value, + ]); + + $query = new ObjectType([ + 'name' => 'QueryRoot', + 'fields' => [ + 'uuid' => ['type' => $scalarType], + ], + ]); + + $schema = new Schema(['query' => $query]); + + $result = GraphQL::executeQuery($schema, '{ __type(name: "UUID") { specifiedByURL } }')->toArray(); + + self::assertSame(['data' => ['__type' => ['specifiedByURL' => 'https://tools.ietf.org/html/rfc4122']]], $result); + } + + /** @see it('executes an introspection query') */ + public function testSpecifiedByURLIsNullForNonScalars(): void + { + $query = new ObjectType([ + 'name' => 'QueryRoot', + 'fields' => ['x' => ['type' => Type::string()]], + ]); + + $schema = new Schema(['query' => $query]); + + $result = GraphQL::executeQuery($schema, '{ __type(name: "QueryRoot") { specifiedByURL } }')->toArray(); + + self::assertSame(['data' => ['__type' => ['specifiedByURL' => null]]], $result); + } + public function testIncludeDescriptionFieldOnSchema(): void { preg_match_all('/\bdescription\b/', Introspection::getIntrospectionQuery(), $matches); @@ -2022,4 +2095,48 @@ public function testIncludeDescriptionFieldOnSchema(): void preg_match_all('/\bdescription\b/', Introspection::getIntrospectionQuery(['descriptions' => false, 'schemaDescription' => true]), $matches); self::assertCount(0, $matches[0]); } + + /** @see it('include "specifiedBy" field') */ + public function testSpecifiedByURLNotIncludedInIntrospectionQueryByDefault(): void + { + self::assertStringNotContainsString('specifiedByURL', Introspection::getIntrospectionQuery()); + } + + /** @see it('include "specifiedBy" field') */ + public function testSpecifiedByURLIncludedInIntrospectionQueryWhenEnabled(): void + { + self::assertStringContainsString('specifiedByURL', Introspection::getIntrospectionQuery(['specifiedByURL' => true])); + } + + /** @see it('include "specifiedBy" field') */ + public function testFromSchemaIncludesSpecifiedByURLByDefault(): void + { + $scalarType = new CustomScalarType([ + 'name' => 'UUID', + 'specifiedByURL' => 'https://tools.ietf.org/html/rfc4122', + 'serialize' => static fn ($value) => $value, + ]); + + $schema = new Schema([ + 'query' => new ObjectType([ + 'name' => 'Query', + 'fields' => ['uuid' => ['type' => $scalarType]], + ]), + ]); + + $introspection = Introspection::fromSchema($schema); + + // Find the UUID type in the introspection result + $uuidType = null; + foreach ($introspection['__schema']['types'] as $type) { + if ($type['name'] === 'UUID') { + $uuidType = $type; + break; + } + } + + self::assertNotNull($uuidType, 'UUID type not found in introspection result'); + self::assertArrayHasKey('specifiedByURL', $uuidType); + self::assertSame('https://tools.ietf.org/html/rfc4122', $uuidType['specifiedByURL']); + } } diff --git a/tests/Utils/BreakingChangesFinderTest.php b/tests/Utils/BreakingChangesFinderTest.php index 2f4a6c760..1fe895d88 100644 --- a/tests/Utils/BreakingChangesFinderTest.php +++ b/tests/Utils/BreakingChangesFinderTest.php @@ -1329,7 +1329,7 @@ public function testShouldDetectIfADirectiveWasImplicitlyRemoved(): void $oldSchema = new Schema([]); $newSchema = new Schema([ - 'directives' => [Directive::skipDirective(), Directive::includeDirective()], + 'directives' => [Directive::skipDirective(), Directive::includeDirective(), Directive::specifiedByDirective()], ]); $deprecatedDirective = Directive::deprecatedDirective(); diff --git a/tests/Utils/BuildClientSchemaTest.php b/tests/Utils/BuildClientSchemaTest.php index 278be5d45..8d7e17599 100644 --- a/tests/Utils/BuildClientSchemaTest.php +++ b/tests/Utils/BuildClientSchemaTest.php @@ -1041,4 +1041,16 @@ public function testCustomScalarValueMixedExecution(): void self::assertSame(['foo' => ['baz' => 'value']], $result->data); } + + /** @see it('builds a schema with specifiedBy url') */ + public function testRoundTripPreservesSpecifiedByURL(): void + { + self::assertCycleIntrospection(' + scalar Foo @specifiedBy(url: "https://example.com/foo_spec") + + type Query { + foo: Foo + } + '); + } } diff --git a/tests/Utils/BuildSchemaTest.php b/tests/Utils/BuildSchemaTest.php index 877fdb3ff..8cf75f0d4 100644 --- a/tests/Utils/BuildSchemaTest.php +++ b/tests/Utils/BuildSchemaTest.php @@ -277,14 +277,11 @@ public function testMaintainsIncludeSkipAndSpecifiedBy(): void { $schema = BuildSchema::buildAST(Parser::parse('type Query')); - // TODO switch to 5 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 - self::assertCount(4, $schema->getDirectives()); + self::assertCount(5, $schema->getDirectives()); self::assertSame(Directive::skipDirective(), $schema->getDirective('skip')); self::assertSame(Directive::includeDirective(), $schema->getDirective('include')); self::assertSame(Directive::deprecatedDirective(), $schema->getDirective('deprecated')); self::assertSame(Directive::oneOfDirective(), $schema->getDirective('oneOf')); - - self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertSame(Directive::specifiedByDirective(), $schema->getDirective('specifiedBy')); } @@ -295,7 +292,7 @@ public function testOverridingDirectivesExcludesSpecified(): void directive @skip on FIELD directive @include on FIELD directive @deprecated on FIELD_DEFINITION - directive @specifiedBy on FIELD_DEFINITION + directive @specifiedBy on SCALAR ')); self::assertCount(5, $schema->getDirectives()); @@ -303,8 +300,6 @@ public function testOverridingDirectivesExcludesSpecified(): void self::assertNotEquals(Directive::includeDirective(), $schema->getDirective('include')); self::assertNotEquals(Directive::deprecatedDirective(), $schema->getDirective('deprecated')); self::assertSame(Directive::oneOfDirective(), $schema->getDirective('oneOf')); - - self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertNotEquals(Directive::specifiedByDirective(), $schema->getDirective('specifiedBy')); } @@ -317,15 +312,12 @@ public function testAddingDirectivesMaintainsIncludeSkipAndSpecifiedBy(): void GRAPHQL; $schema = BuildSchema::buildAST(Parser::parse($sdl)); - // TODO switch to 6 when adding @specifiedBy - see https://github.com/webonyx/graphql-php/issues/1140 - self::assertCount(5, $schema->getDirectives()); + self::assertCount(6, $schema->getDirectives()); self::assertNotNull($schema->getDirective('foo')); self::assertNotNull($schema->getDirective('skip')); self::assertNotNull($schema->getDirective('include')); self::assertNotNull($schema->getDirective('deprecated')); self::assertNotNull($schema->getDirective('oneOf')); - - self::markTestIncomplete('See https://github.com/webonyx/graphql-php/issues/1140'); self::assertNotNull($schema->getDirective('specifiedBy')); } @@ -826,7 +818,6 @@ enum: MyEnum /** @see it('Supports @specifiedBy') */ public function testSupportsSpecifiedBy(): void { - self::markTestSkipped('See https://github.com/webonyx/graphql-php/issues/1140'); $sdl = <<getType('Foo'); + + self::assertInstanceOf(ScalarType::class, $type); + self::assertSame('https://example.com/foo_spec', $type->specifiedByURL); + } + + /** + * Verifies that @specifiedBy on a scalar extension node is picked up by BuildSchema. + * The URL should come from the `extend scalar` directive, not just the base definition. + */ + public function testSpecifiedByURLFromExtensionNode(): void + { + $sdl = <<getType('Foo'); + + self::assertInstanceOf(ScalarType::class, $type); + self::assertSame('https://example.com/foo_spec', $type->specifiedByURL); + } + + /** + * Verifies that overriding @specifiedBy with a custom directive (no url arg) does not throw + * when building a schema with a scalar that uses the custom directive. + */ + public function testCustomSpecifiedByDirectiveOverrideShouldNotThrow(): void + { + // A schema that overrides @specifiedBy with a different (arg-less) definition + // and uses it on a scalar should not throw during schema building. + $sdl = <<getType('Foo'); - self::assertSame('https://example.com/foo_spec', $schema->getType('Foo')->specifiedByURL); + self::assertInstanceOf(ScalarType::class, $type); + // When the built-in @specifiedBy is overridden with no url arg, specifiedByURL stays null + self::assertNull($type->specifiedByURL); } /** @see it('Correctly extend scalar type') */ diff --git a/tests/Utils/SchemaExtenderTest.php b/tests/Utils/SchemaExtenderTest.php index 16119f5af..2035f2f43 100644 --- a/tests/Utils/SchemaExtenderTest.php +++ b/tests/Utils/SchemaExtenderTest.php @@ -456,8 +456,6 @@ public function testExtendsScalarsByAddingNewDirectives(): void /** @see it('extends scalars by adding specifiedBy directive') */ public function testExtendsScalarsByAddingSpecifiedByDirective(): void { - // @phpstan-ignore-next-line - $this->markTestSkipped('See https://github.com/webonyx/graphql-php/issues/1140'); $schema = BuildSchema::build(' type Query { foo: Foo @@ -471,9 +469,12 @@ public function testExtendsScalarsByAddingSpecifiedByDirective(): void extend scalar Foo @foo extend scalar Foo @specifiedBy(url: "https://example.com/foo_spec") + GRAPHQL; + $extendedSchema = SchemaExtender::extend($schema, Parser::parse($extensionSDL)); $foo = $extendedSchema->getType('Foo'); + assert($foo instanceof ScalarType); self::assertSame('https://example.com/foo_spec', $foo->specifiedByURL); self::assertEmpty($extendedSchema->validate()); @@ -483,6 +484,31 @@ public function testExtendsScalarsByAddingSpecifiedByDirective(): void ); } + /** + * Verifies that overriding @specifiedBy with a custom directive (no url arg) + * and using it in an extension does not throw. + */ + public function testExtendsScalarsWithCustomSpecifiedByOverrideShouldNotThrow(): void + { + $schema = BuildSchema::build(' + directive @specifiedBy on SCALAR + + type Query { + foo: Foo + } + + scalar Foo + '); + $extendedSchema = SchemaExtender::extend($schema, Parser::parse(' + extend scalar Foo @specifiedBy + ')); + $foo = $extendedSchema->getType('Foo'); + assert($foo instanceof ScalarType); + + // Custom @specifiedBy without url arg leaves specifiedByURL as null + self::assertNull($foo->specifiedByURL); + } + /** @see it('correctly assign AST nodes to new and extended types') */ public function testCorrectlyAssignASTNodesToNewAndExtendedTypes(): void { @@ -1065,9 +1091,9 @@ interface AnotherNewInterface { self::printSchemaChanges($schema, $schemaWithNewTypes) ); - // TODO see https://github.com/webonyx/graphql-php/issues/1140 - // extend scalar SomeScalar @specifiedBy(url: "http://example.com/foo_spec") $extendAST = Parser::parse(' + extend scalar SomeScalar @specifiedBy(url: "http://example.com/foo_spec") + extend type SomeObject implements NewInterface { newField: String } @@ -1100,9 +1126,9 @@ interface AnotherNewInterface { self::assertEmpty($extendedSchema->validate()); self::assertSame( - // TODO see https://github.com/webonyx/graphql-php/issues/1140 - // scalar SomeScalar @specifiedBy(url: \"http://example.com/foo_spec\") << 'Foo', + 'specifiedByURL' => 'https://example.com/foo_spec', + 'serialize' => static fn () => null, + ]); + + $schema = new Schema([ + 'types' => [$fooType], + ]); + + self::assertPrintedSchemaEquals( + <<<'GRAPHQL' + scalar Foo @specifiedBy(url: "https://example.com/foo_spec") + + GRAPHQL, + $schema + ); + } + /** @see it('Enum') */ public function testEnum(): void { @@ -1073,6 +1095,12 @@ public function testPrintIntrospectionSchema(): void reason: String = "No longer supported" ) on FIELD_DEFINITION | ENUM_VALUE | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION + "Exposes a URL that specifies the behavior of this scalar." + directive @specifiedBy( + "The URL that specifies the behavior of this scalar." + url: String! + ) on SCALAR + "Indicates that an Input Object is a OneOf Input Object (and thus requires exactly one of its fields be provided)." directive @oneOf on INPUT_OBJECT @@ -1105,6 +1133,7 @@ public function testPrintIntrospectionSchema(): void kind: __TypeKind! name: String description: String + specifiedByURL: String fields(includeDeprecated: Boolean! = false): [__Field!] interfaces: [__Type!] possibleTypes: [__Type!] diff --git a/tests/Validator/QueryComplexityTest.php b/tests/Validator/QueryComplexityTest.php index cbd145a5b..db6050a01 100644 --- a/tests/Validator/QueryComplexityTest.php +++ b/tests/Validator/QueryComplexityTest.php @@ -176,6 +176,36 @@ public function testQueryWithCustomAndSkipDirective(): void $this->assertDocumentValidators($query, 1, 2); } + /** + * Verifies that a non-excluding directive appearing before @skip on the same field + * does not prevent the @skip from being evaluated, avoiding incorrect complexity. + */ + public function testQueryWithNonExcludingDirectiveBeforeSkip(): void + { + // @foo appears before @skip on the same field; @skip(if:true) should still exclude dogs + $query = 'query MyQuery($withoutDogs: Boolean!) { human { dogs(name: "Root") @foo(bar: true) @skip(if:$withoutDogs) { name } } }'; + + $this->getRule()->setRawVariableValues(['withoutDogs' => true]); + + // dogs is excluded by @skip, so complexity is 1 (only human) + $this->assertDocumentValidators($query, 1, 2); + } + + /** + * Verifies that @include(if:true) followed by @skip(if:true) on the same field correctly excludes it. + * Without evaluating all directives, returning early on @include(if:true) would yield the wrong result. + */ + public function testQueryWithIncludeAndSkipDirectives(): void + { + // @include(if:true) alone would include the field, but @skip(if:true) should still exclude it + $query = 'query MyQuery($withDogs: Boolean!, $withoutDogs: Boolean!) { human { dogs(name: "Root") @include(if:$withDogs) @skip(if:$withoutDogs) { name } } }'; + + $this->getRule()->setRawVariableValues(['withDogs' => true, 'withoutDogs' => true]); + + // dogs is excluded by @skip(if:true), so complexity is 1 (only human) + $this->assertDocumentValidators($query, 1, 2); + } + public function testComplexityIntrospectionQuery(): void { $query = Introspection::getIntrospectionQuery(); @@ -258,4 +288,47 @@ protected static function getErrorMessage(int $max, int $count): string { return QueryComplexity::maxQueryComplexityErrorMessage($max, $count); } + + /** + * Verifies that variable coercion is not triggered for fields without @include/@skip + * directives. Previously, directiveExcludesField() called getVariableValues() for + * every field unconditionally. Now it is lazy: coercion only happens when an + * \@include or \@skip directive is actually encountered on the field. + * + * This test passes a PHP integer for a variable declared as `String!`. The `StringType` + * rejects non-string values in `parseValue`, so coercing this variable would throw. With + * the old code (unconditional coercion) this would propagate as an exception through + * `directiveExcludesField` for every field. With lazy coercion it is never triggered, + * because no field in the query uses @include or @skip. + */ + public function testVariableCoercionIsLazyWhenNoIncludeOrSkipDirectives(): void + { + // $dog is declared but none of the fields use @include/@skip, + // so variable coercion should never be triggered by directiveExcludesField. + $query = 'query MyQuery($dog: String!) { human { firstName } }'; + + // An integer is rejected by StringType::parseValue, so if coercion ran (as it did + // before the lazy-coercion fix) it would throw. With the fix, coercion is never + // triggered here because no field uses @include or @skip. + $this->getRule()->setRawVariableValues(['dog' => 42]); + + // Should produce no errors from the complexity rule itself (complexity is within bounds) + $this->assertDocumentValidators($query, 2, 3); + } + + /** + * Verifies that variable coercion is cached: when multiple fields each have @skip, + * coercion is only performed once, not once per field. + */ + public function testVariableCoercionIsCachedAcrossMultipleDirectives(): void + { + // Two fields each with @skip using the same variable. + // Coercion should occur once and be cached. + $query = 'query MyQuery($skipIt: Boolean!) { human { firstName @skip(if:$skipIt) dogs { name @skip(if:$skipIt) } } }'; + + $this->getRule()->setRawVariableValues(['skipIt' => false]); + + // skipIt=false means nothing is skipped: complexity = human(1) + firstName(1) + dogs(10) + name(1) = 13 + $this->assertDocumentValidators($query, 13, 14); + } } diff --git a/tests/Validator/ValidatorTestCase.php b/tests/Validator/ValidatorTestCase.php index eb006cfc0..afab588c5 100644 --- a/tests/Validator/ValidatorTestCase.php +++ b/tests/Validator/ValidatorTestCase.php @@ -367,10 +367,7 @@ public static function getTestSchema(): Schema return new Schema([ 'query' => $queryRoot, 'subscription' => $subscriptionRoot, - 'directives' => [ - Directive::includeDirective(), - Directive::skipDirective(), - Directive::deprecatedDirective(), + 'directives' => array_merge(Directive::getInternalDirectives(), [ new Directive([ 'name' => 'directive', 'locations' => [DirectiveLocation::FIELD, DirectiveLocation::FRAGMENT_DEFINITION], @@ -420,7 +417,7 @@ public static function getTestSchema(): Schema 'name' => 'onVariableDefinition', 'locations' => [DirectiveLocation::VARIABLE_DEFINITION], ]), - ], + ]), ]); }