Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid eager-loading schema unnecessarily #1104

Merged
merged 39 commits into from
May 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
178f5b0
Align `SchemaExtenderTest` with `graphql-js` v14.0
vhenzl Sep 3, 2021
618b657
Align `SchemaExtenderTest` with `graphql-js` v14.7
vhenzl Sep 4, 2021
d0de14c
Add `concatAST` utility function
vhenzl Sep 4, 2021
ce80b08
Align `SchemaExtenderTest` with `graphql-js` v15.0
vhenzl Sep 4, 2021
8d7a4b8
Align `SchemaExtenderTest` with `graphql-js` v15.5 (excluding specifi…
vhenzl Sep 4, 2021
fbb8a5b
Align `SchemaExtenderTest` with `graphql-js` v15.5 (specifiedBy direc…
vhenzl Sep 4, 2021
69a4403
Fix a test to assert correct variable (v14.7)
vhenzl Sep 8, 2021
c1ef656
Merge branch 'master' into schema-extend-tests
vhenzl Oct 16, 2021
6ef73fd
Avoid eager-loading schema unnecessarily
spawnia Apr 7, 2022
5d0439a
Apply php-cs-fixer changes
spawnia Apr 7, 2022
588e936
Merge branch 'master' into avoid-eager-loading-schema
spawnia Apr 8, 2022
942e635
Merge branch 'master' into avoid-eager-loading-schema
spawnia May 4, 2022
9b82707
fix more tests
spawnia May 4, 2022
07c0585
remove lingering state
spawnia May 4, 2022
376778b
stan
spawnia May 4, 2022
a963ff3
fix test formatting
spawnia May 4, 2022
36b4192
Merge remote-tracking branch 'vhenzl/schema-extend-tests' into avoid-…
spawnia May 5, 2022
e0178ee
fix merge
spawnia May 5, 2022
3a2103f
Merge branch 'master' into avoid-eager-loading-schema
spawnia May 5, 2022
3f302fc
merge master
spawnia May 5, 2022
051b2c0
Update test
spawnia May 5, 2022
2de9532
avoid messing up order in Schema
spawnia May 5, 2022
81e956f
Do not sort types in printer
spawnia May 5, 2022
43f6afd
whitespace
spawnia May 5, 2022
aa7b3bc
remove legacy test
spawnia May 5, 2022
af15733
Merge branch 'master' into avoid-eager-loading-schema
spawnia May 5, 2022
180baf9
use smart assertion
spawnia May 5, 2022
4b902de
Fix introspection order
spawnia May 5, 2022
7288c7f
Apply php-cs-fixer changes
spawnia May 5, 2022
437dff4
tweak introspection and related tests
spawnia May 5, 2022
fd0fcf6
simplify implementations
spawnia May 5, 2022
36acc33
improve ordering
spawnia May 5, 2022
eb226a8
remove unnecessary old tests
spawnia May 5, 2022
e5daf2d
Apply php-cs-fixer changes
spawnia May 5, 2022
7dadcc0
simplify
spawnia May 6, 2022
a544be7
finally fix order completely
spawnia May 6, 2022
79fc9f6
Merge branch 'master' into avoid-eager-loading-schema
spawnia May 6, 2022
9dcbdb8
simplify more
spawnia May 6, 2022
1a3346e
changelog, remove todo
spawnia May 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

All notable changes to this project will be documented in this file.

The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

You can find and compare releases at the [GitHub release page](https://github.com/webonyx/graphql-php/releases).
Expand Down Expand Up @@ -42,6 +42,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
- Reorganize abstract class `ASTValidationContext` to interface `ValidationContext`
- Reorganize AST interfaces related to schema and type extensions
- Align `Utils::suggestionList()` with the reference implementation (#1075)
- Order schema topologically and according to the user-defined order, affects introspection and printing

### Added

Expand Down Expand Up @@ -116,6 +117,8 @@ You can find and compare releases at the [GitHub release page](https://github.co
- Remove `GraphQL\Exception\InvalidArgument`
- Remove `Utils::find()`, `Utils::every()` and `Utils::invariant()`
- Remove argument `bool $exitWhenDone` from `StandardServer::send500Error()` and `StandardServer::handleRequest()`
- Remove `Schema::getAstNode()` in favor of `Schema::$astNode`
- Remove ability to override standard types through `Schema` option `types`, use `Type::overrideStandardTypes()`

## 14.11.6

Expand Down
4 changes: 1 addition & 3 deletions src/Type/Definition/Directive.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ public static function getInternalDirectives(): array
'args' => [
self::REASON_ARGUMENT_NAME => [
'type' => Type::string(),
'description' => 'Explains why this element was deprecated, usually also including a '
. 'suggestion for how to access supported similar data. Formatted using '
. 'the Markdown syntax (as specified by [CommonMark](https://commonmark.org/).',
'description' => 'Explains why this element was deprecated, usually also including a suggestion for how to access supported similar data. Formatted using the Markdown syntax, as specified by [CommonMark](https://commonmark.org/).',
'defaultValue' => self::DEFAULT_DEPRECATION_REASON,
],
],
Expand Down
8 changes: 4 additions & 4 deletions src/Type/Introspection.php
Original file line number Diff line number Diff line change
Expand Up @@ -586,10 +586,6 @@ public static function _directive(): ObjectType
'type' => Type::string(),
'resolve' => static fn (Directive $directive): ?string => $directive->description,
],
'args' => [
'type' => Type::nonNull(Type::listOf(Type::nonNull(self::_inputValue()))),
'resolve' => static fn (Directive $directive): array => $directive->args,
],
'isRepeatable' => [
'type' => Type::nonNull(Type::boolean()),
'resolve' => static fn (Directive $directive): bool => $directive->isRepeatable,
Expand All @@ -600,6 +596,10 @@ public static function _directive(): ObjectType
))),
'resolve' => static fn (Directive $directive): array => $directive->locations,
],
'args' => [
'type' => Type::nonNull(Type::listOf(Type::nonNull(self::_inputValue()))),
'resolve' => static fn (Directive $directive): array => $directive->args,
],
],
]);
}
Expand Down
171 changes: 58 additions & 113 deletions src/Type/Schema.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

namespace GraphQL\Type;

use Generator;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\GraphQL;
Expand All @@ -22,7 +21,6 @@
use function implode;
use function is_array;
use function is_callable;
use function is_iterable;

/**
* Schema Definition (see [schema definition docs](schema-definition.md)).
Expand Down Expand Up @@ -70,6 +68,8 @@ class Schema
/** @var array<int, Error> */
private array $validationErrors;

public ?SchemaDefinitionNode $astNode;

/** @var array<int, SchemaExtensionNode> */
public array $extensionASTNodes = [];

Expand All @@ -90,77 +90,10 @@ public function __construct($config)
$this->validationErrors = [];
}

$this->config = $config;
$this->astNode = $config->astNode;
$this->extensionASTNodes = $config->extensionASTNodes;

// TODO can we make the following assumption hold true?
// No need to check for the existence of the root query type
// since we already validated the schema thus it must exist.
$query = $config->query;
if ($query !== null) {
$this->resolvedTypes[$query->name] = $query;
}

$mutation = $config->mutation;
if ($mutation !== null) {
$this->resolvedTypes[$mutation->name] = $mutation;
}

$subscription = $config->subscription;
if ($subscription !== null) {
$this->resolvedTypes[$subscription->name] = $subscription;
}

$types = $this->config->types;
if (is_array($types)) {
foreach ($this->resolveAdditionalTypes() as $type) {
$typeName = $type->name;
if (isset($this->resolvedTypes[$typeName])) {
if ($type !== $this->resolvedTypes[$typeName]) {
throw new InvariantViolation("Schema must contain unique named types but contains multiple types named \"{$type}\" (see https://webonyx.github.io/graphql-php/type-definitions/#type-registry).");
}
}

$this->resolvedTypes[$typeName] = $type;
}
// @phpstan-ignore-next-line not strictly enforced until we can use actual union types
} elseif (! is_callable($types)) {
$invalidTypes = Utils::printSafe($types);

throw new InvariantViolation("\"types\" must be array or callable if provided but got: {$invalidTypes}");
}

$this->resolvedTypes += Introspection::getTypes();

if (isset($this->config->typeLoader)) {
return;
}

// Perform full scan of the schema
$this->getTypeMap();
}

/**
* @return Generator<Type&NamedType>
*/
private function resolveAdditionalTypes(): Generator
{
$types = $this->config->types;

if (is_callable($types)) {
$types = $types();
}

// @phpstan-ignore-next-line not strictly enforced unless PHP supports function types
if (! is_iterable($types)) {
$notIterable = Utils::printSafe($types);

throw new InvariantViolation("Schema types callable must return iterable but got: {$notIterable}");
}

foreach ($types as $type) {
yield self::resolveType($type);
}
$this->config = $config;
}

/**
Expand All @@ -175,39 +108,57 @@ private function resolveAdditionalTypes(): Generator
public function getTypeMap(): array
{
if (! $this->fullyLoaded) {
$this->resolvedTypes = $this->collectAllTypes();
$this->fullyLoaded = true;
}
$types = $this->config->types;
if (is_callable($types)) {
$types = $types();
}

return $this->resolvedTypes;
}
// Reset order of user provided types, since calls to getType() may have loaded them
$this->resolvedTypes = [];

/**
* @return array<string, Type&NamedType>
*/
private function collectAllTypes(): array
{
/** @var array<string, Type&NamedType> $typeMap */
$typeMap = [];
foreach ($this->resolvedTypes as $type) {
TypeInfo::extractTypes($type, $typeMap);
}
foreach ($types as $typeOrLazyType) {
$type = self::resolveType($typeOrLazyType);

foreach ($this->getDirectives() as $directive) {
// @phpstan-ignore-next-line generics are not strictly enforceable, error will be caught during schema validation
if ($directive instanceof Directive) {
TypeInfo::extractTypesFromDirectives($directive, $typeMap);
$typeName = $type->name;
if (isset($this->resolvedTypes[$typeName])) {
if ($type !== $this->resolvedTypes[$typeName]) {
throw new InvariantViolation("Schema must contain unique named types but contains multiple types named \"$type\" (see https://webonyx.github.io/graphql-php/type-definitions/#type-registry).");
}
}

$this->resolvedTypes[$typeName] = $type;
}

// To preserve order of user-provided types, we add first to add them to
// the set of "collected" types, so `collectReferencedTypes` ignore them.
/** @var array<string, Type&NamedType> $allReferencedTypes */
$allReferencedTypes = [];
foreach ($this->resolvedTypes as $type) {
// When we ready to process this type, we remove it from "collected" types
// and then add it together with all dependent types in the correct position.
unset($allReferencedTypes[$type->name]);
TypeInfo::extractTypes($type, $allReferencedTypes);
}
}

// When types are set as array they are resolved in constructor
if (is_callable($this->config->types)) {
foreach ($this->resolveAdditionalTypes() as $type) {
TypeInfo::extractTypes($type, $typeMap);
foreach ([$this->config->query, $this->config->mutation, $this->config->subscription] as $rootType) {
if ($rootType instanceof ObjectType) {
TypeInfo::extractTypes($rootType, $allReferencedTypes);
}
}

foreach ($this->getDirectives() as $directive) {
// @phpstan-ignore-next-line generics are not strictly enforceable, error will be caught during schema validation
if ($directive instanceof Directive) {
TypeInfo::extractTypesFromDirectives($directive, $allReferencedTypes);
}
}
TypeInfo::extractTypes(Introspection::_schema(), $allReferencedTypes);

$this->resolvedTypes = $allReferencedTypes;
$this->fullyLoaded = true;
}

return $typeMap;
return $this->resolvedTypes;
}

/**
Expand Down Expand Up @@ -303,9 +254,17 @@ public function getConfig(): SchemaConfig
public function getType(string $name): ?Type
{
if (! isset($this->resolvedTypes[$name])) {
$type = Type::getStandardTypes()[$name]
?? $this->loadType($name);
$introspectionTypes = Introspection::getTypes();
if (isset($introspectionTypes[$name])) {
return $introspectionTypes[$name];
}

$standardTypes = Type::getStandardTypes();
if (isset($standardTypes[$name])) {
return $standardTypes[$name];
}

$type = $this->loadType($name);
if ($type === null) {
return null;
}
Expand All @@ -328,7 +287,7 @@ private function loadType(string $typeName): ?Type
{
$typeLoader = $this->config->typeLoader;
if ($typeLoader === null) {
return $this->defaultTypeLoader($typeName);
return $this->getTypeMap()[$typeName] ?? null;
}

$type = $typeLoader($typeName);
Expand All @@ -348,15 +307,6 @@ private function loadType(string $typeName): ?Type
return $type;
}

/**
* @return (Type&NamedType)|null
*/
private function defaultTypeLoader(string $typeName): ?Type
{
// Default type loader simply falls back to collecting all types
return $this->getTypeMap()[$typeName] ?? null;
}

/**
* @template T of Type
*
Expand Down Expand Up @@ -494,11 +444,6 @@ public function getDirective(string $name): ?Directive
return null;
}

public function getAstNode(): ?SchemaDefinitionNode
{
return $this->config->astNode;
}

/**
* Throws if the schema is not valid.
*
Expand Down
7 changes: 2 additions & 5 deletions src/Type/SchemaValidationContext.php
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ public function getErrors(): array
public function validateRootTypes(): void
{
if ($this->schema->getQueryType() === null) {
$this->reportError(
'Query root type must be provided.',
$this->schema->getAstNode()
);
$this->reportError('Query root type must be provided.', $this->schema->astNode);
}

// Triggers a type error if wrong
Expand Down Expand Up @@ -475,7 +472,7 @@ private function validateFields(Type $type): void
private function getAllNodes(object $obj): array
{
if ($obj instanceof Schema) {
$astNode = $obj->getAstNode();
$astNode = $obj->astNode;
$extensionNodes = $obj->extensionASTNodes;
} elseif ($obj instanceof Directive) {
$astNode = $obj->astNode;
Expand Down
Loading