Skip to content

Commit ebd6aff

Browse files
ruudkruudk
authored
Fix scalar overrides not applied without assertValid() (#1886)
1 parent 6702705 commit ebd6aff

2 files changed

Lines changed: 172 additions & 10 deletions

File tree

‎src/Type/Schema.php‎

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ class Schema
6767
/** True when $resolvedTypes contains all possible schema types. */
6868
private bool $fullyLoaded = false;
6969

70+
/** @var array<string, ScalarType>|null Lazily initialised by getScalarOverrides(). */
71+
private ?array $scalarOverrides = null;
72+
7073
/** @var array<int, Error> */
7174
private array $validationErrors;
7275

@@ -127,11 +130,7 @@ public function getTypeMap(): array
127130
// Reset order of user provided types, since calls to getType() may have loaded them
128131
$this->resolvedTypes = [];
129132

130-
// Separate built-in scalar overrides to avoid identity conflicts
131-
// with Type::string() etc. references in field definitions during extractTypes.
132-
/** @var array<string, ScalarType> $scalarOverrides */
133-
$scalarOverrides = [];
134-
$builtInScalars = Type::builtInScalars();
133+
$scalarOverrides = $this->getScalarOverrides();
135134

136135
foreach ($types as $typeOrLazyType) {
137136
/** @var Type|callable(): Type $typeOrLazyType */
@@ -141,11 +140,7 @@ public function getTypeMap(): array
141140
/** @var string $typeName Necessary assertion for PHPStan + PHP 8.2 */
142141
$typeName = $type->name;
143142

144-
if ($type instanceof ScalarType
145-
&& isset($builtInScalars[$typeName])
146-
&& $type !== $builtInScalars[$typeName]
147-
) {
148-
$scalarOverrides[$typeName] = $type;
143+
if (isset($scalarOverrides[$typeName])) {
149144
continue;
150145
}
151146

@@ -325,6 +320,11 @@ public function getType(string $name): ?Type
325320
return $this->resolvedTypes[$name] = self::resolveType($type);
326321
}
327322

323+
$scalarOverrides = $this->getScalarOverrides();
324+
if (isset($scalarOverrides[$name])) {
325+
return $this->resolvedTypes[$name] = $scalarOverrides[$name];
326+
}
327+
328328
$builtInScalars = Type::builtInScalars();
329329
if (isset($builtInScalars[$name])) {
330330
return $this->resolvedTypes[$name] = $builtInScalars[$name];
@@ -374,6 +374,40 @@ private function loadType(string $typeName): ?Type
374374
return $type;
375375
}
376376

377+
/** @return array<string, ScalarType> */
378+
private function getScalarOverrides(): array
379+
{
380+
if ($this->scalarOverrides === null) {
381+
$this->scalarOverrides = [];
382+
383+
$types = $this->config->types;
384+
if (is_callable($types)) {
385+
$types = $types();
386+
}
387+
388+
// Materialize the iterable in case it is a generator, so that
389+
// getTypeMap() can still iterate config->types later.
390+
if (! is_array($types)) {
391+
$types = iterator_to_array($types);
392+
$this->config->types = $types;
393+
}
394+
395+
$builtInScalars = Type::builtInScalars();
396+
foreach ($types as $typeOrLazyType) {
397+
/** @var Type|callable(): Type $typeOrLazyType */
398+
$type = self::resolveType($typeOrLazyType);
399+
if ($type instanceof ScalarType
400+
&& isset($builtInScalars[$type->name])
401+
&& $type !== $builtInScalars[$type->name]
402+
) {
403+
$this->scalarOverrides[$type->name] = $type;
404+
}
405+
}
406+
}
407+
408+
return $this->scalarOverrides;
409+
}
410+
377411
/**
378412
* @template T of Type
379413
*

‎tests/Type/ScalarOverridesTest.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,134 @@ public function testTypesOverrideWorksWithTypeLoader(): void
413413
'types' => [$uppercaseString],
414414
]);
415415

416+
$result = GraphQL::executeQuery($schema, '{ greeting user { name } }');
417+
418+
self::assertSame(['data' => ['greeting' => 'HELLO WORLD', 'user' => ['name' => 'JANE']]], $result->toArray());
419+
}
420+
421+
public function testTypesOverrideWorksWithTypeLoaderAndAssertValid(): void
422+
{
423+
$uppercaseString = self::createUppercaseString();
424+
425+
$userType = new ObjectType([
426+
'name' => 'User',
427+
'fields' => [
428+
'name' => Type::string(),
429+
],
430+
]);
431+
432+
$queryType = new ObjectType([
433+
'name' => 'Query',
434+
'fields' => [
435+
'greeting' => [
436+
'type' => Type::string(),
437+
'resolve' => static fn (): string => 'hello world',
438+
],
439+
'user' => [
440+
'type' => $userType,
441+
'resolve' => static fn (): array => ['name' => 'Jane'],
442+
],
443+
],
444+
]);
445+
446+
$types = ['Query' => $queryType, 'User' => $userType];
447+
$schema = new Schema([
448+
'query' => $queryType,
449+
'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null,
450+
'types' => [$uppercaseString],
451+
]);
452+
453+
$schema->assertValid();
454+
455+
$result = GraphQL::executeQuery($schema, '{ greeting user { name } }');
456+
457+
self::assertSame(['data' => ['greeting' => 'HELLO WORLD', 'user' => ['name' => 'JANE']]], $result->toArray());
458+
}
459+
460+
public function testGetTypeReturnsScalarOverrideWithTypeLoader(): void
461+
{
462+
$uppercaseString = self::createUppercaseString();
463+
$queryType = self::createQueryType();
464+
465+
$schema = new Schema([
466+
'query' => $queryType,
467+
'typeLoader' => static fn (string $name): ?Type => $name === 'Query' ? $queryType : null,
468+
'types' => [$uppercaseString],
469+
]);
470+
471+
self::assertSame($uppercaseString, $schema->getType(Type::STRING));
472+
}
473+
474+
public function testTypesOverrideWorksWithCallableTypesConfig(): void
475+
{
476+
$uppercaseString = self::createUppercaseString();
477+
$queryType = self::createQueryType();
478+
479+
$schema = new Schema([
480+
'query' => $queryType,
481+
'typeLoader' => static fn (string $name): ?Type => $name === 'Query' ? $queryType : null,
482+
'types' => static fn (): array => [$uppercaseString],
483+
]);
484+
485+
$result = GraphQL::executeQuery($schema, '{ greeting }');
486+
487+
self::assertSame(['data' => ['greeting' => 'HELLO WORLD']], $result->toArray());
488+
}
489+
490+
public function testTypesOverrideWorksWithGeneratorTypesConfig(): void
491+
{
492+
$uppercaseString = self::createUppercaseString();
493+
$queryType = self::createQueryType();
494+
495+
$schema = new Schema([
496+
'query' => $queryType,
497+
'typeLoader' => static fn (string $name): ?Type => $name === 'Query' ? $queryType : null,
498+
'types' => static function () use ($uppercaseString): \Generator {
499+
yield $uppercaseString;
500+
},
501+
]);
502+
503+
$result = GraphQL::executeQuery($schema, '{ greeting }');
504+
505+
self::assertSame(['data' => ['greeting' => 'HELLO WORLD']], $result->toArray());
506+
507+
$schema->assertValid();
508+
}
509+
510+
public function testGetTypeThenAssertValidBothWorkWithTypeLoader(): void
511+
{
512+
$uppercaseString = self::createUppercaseString();
513+
514+
$userType = new ObjectType([
515+
'name' => 'User',
516+
'fields' => [
517+
'name' => Type::string(),
518+
],
519+
]);
520+
521+
$queryType = new ObjectType([
522+
'name' => 'Query',
523+
'fields' => [
524+
'greeting' => [
525+
'type' => Type::string(),
526+
'resolve' => static fn (): string => 'hello world',
527+
],
528+
'user' => [
529+
'type' => $userType,
530+
'resolve' => static fn (): array => ['name' => 'Jane'],
531+
],
532+
],
533+
]);
534+
535+
$types = ['Query' => $queryType, 'User' => $userType];
536+
$schema = new Schema([
537+
'query' => $queryType,
538+
'typeLoader' => static fn (string $name): ?Type => $types[$name] ?? null,
539+
'types' => [$uppercaseString],
540+
]);
541+
542+
self::assertSame($uppercaseString, $schema->getType(Type::STRING));
543+
416544
$schema->assertValid();
417545

418546
$result = GraphQL::executeQuery($schema, '{ greeting user { name } }');

0 commit comments

Comments
 (0)