Skip to content

Commit 98e5835

Browse files
vladarvladar
committed
Avoid infinite loop when using recursive types and interfaces (#16)
1 parent 4591840 commit 98e5835

File tree

5 files changed

+92
-46
lines changed

5 files changed

+92
-46
lines changed

‎src/Type/Definition/ListOfType.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ public function toString()
3434
}
3535

3636
/**
37-
* @return Type
37+
* @param bool $recurse
38+
* @return mixed
3839
*/
39-
public function getWrappedType()
40+
public function getWrappedType($recurse = false)
4041
{
41-
return Type::resolve($this->ofType);
42+
$type = Type::resolve($this->ofType);
43+
return ($recurse && $type instanceof WrappingType) ? $type->getWrappedType($recurse) : $type;
4244
}
4345
}

‎src/Type/Definition/NonNull.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@ public function __construct($type)
2929
}
3030

3131
/**
32-
* @return Type|callable
32+
* @param bool $recurse
33+
* @return Type
34+
* @throws \Exception
3335
*/
34-
public function getWrappedType()
36+
public function getWrappedType($recurse = false)
3537
{
3638
$type = Type::resolve($this->ofType);
3739

@@ -40,7 +42,7 @@ public function getWrappedType()
4042
'Cannot nest NonNull inside NonNull'
4143
);
4244

43-
return $type;
45+
return ($recurse && $type instanceof WrappingType) ? $type->getWrappedType($recurse) : $type;
4446
}
4547

4648
/**

‎src/Type/Definition/ObjectType.php‎

Lines changed: 19 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,6 @@ class ObjectType extends Type implements OutputType, CompositeType
6464
*/
6565
private $_config;
6666

67-
private $_initialized = false;
68-
6967
/**
7068
* @var callable
7169
*/
@@ -75,36 +73,9 @@ public function __construct(array $config)
7573
{
7674
Utils::invariant(!empty($config['name']), 'Every type is expected to have name');
7775

78-
$this->name = $config['name'];
79-
$this->description = isset($config['description']) ? $config['description'] : null;
80-
$this->resolveFieldFn = isset($config['resolveField']) ? $config['resolveField'] : null;
81-
$this->_config = $config;
82-
83-
if (isset($config['interfaces'])) {
84-
InterfaceType::addImplementationToInterfaces($this);
85-
}
86-
}
87-
88-
/**
89-
* Late instance initialization
90-
*/
91-
private function initialize()
92-
{
93-
if ($this->_initialized) {
94-
return ;
95-
}
96-
$config = $this->_config;
97-
98-
if (isset($config['fields']) && is_callable($config['fields'])) {
99-
$config['fields'] = call_user_func($config['fields']);
100-
}
101-
if (isset($config['interfaces']) && is_callable($config['interfaces'])) {
102-
$config['interfaces'] = call_user_func($config['interfaces']);
103-
}
104-
10576
// Note: this validation is disabled by default, because it is resource-consuming
10677
// TODO: add bin/validate script to check if schema is valid during development
107-
Config::validate($this->_config, [
78+
Config::validate($config, [
10879
'name' => Config::STRING | Config::REQUIRED,
10980
'fields' => Config::arrayOf(
11081
FieldDefinition::getDefinition(),
@@ -118,19 +89,26 @@ private function initialize()
11889
'resolveField' => Config::CALLBACK
11990
]);
12091

121-
$this->_fields = FieldDefinition::createMap($config['fields']);
122-
$this->_interfaces = isset($config['interfaces']) ? $config['interfaces'] : [];
92+
$this->name = $config['name'];
93+
$this->description = isset($config['description']) ? $config['description'] : null;
94+
$this->resolveFieldFn = isset($config['resolveField']) ? $config['resolveField'] : null;
12395
$this->_isTypeOf = isset($config['isTypeOf']) ? $config['isTypeOf'] : null;
124-
$this->_initialized = true;
96+
$this->_config = $config;
97+
98+
if (isset($config['interfaces'])) {
99+
InterfaceType::addImplementationToInterfaces($this);
100+
}
125101
}
126102

127103
/**
128104
* @return FieldDefinition[]
129105
*/
130106
public function getFields()
131107
{
132-
if (false === $this->_initialized) {
133-
$this->initialize();
108+
if (null === $this->_fields) {
109+
$fields = isset($this->_config['fields']) ? $this->_config['fields'] : [];
110+
$fields = is_callable($fields) ? call_user_func($fields) : $fields;
111+
$this->_fields = FieldDefinition::createMap($fields);
134112
}
135113
return $this->_fields;
136114
}
@@ -142,8 +120,8 @@ public function getFields()
142120
*/
143121
public function getField($name)
144122
{
145-
if (false === $this->_initialized) {
146-
$this->initialize();
123+
if (null === $this->_fields) {
124+
$this->getFields();
147125
}
148126
Utils::invariant(isset($this->_fields[$name]), "Field '%s' is not defined for type '%s'", $name, $this->name);
149127
return $this->_fields[$name];
@@ -154,8 +132,10 @@ public function getField($name)
154132
*/
155133
public function getInterfaces()
156134
{
157-
if (false === $this->_initialized) {
158-
$this->initialize();
135+
if (null === $this->_interfaces) {
136+
$interfaces = isset($this->_config['interfaces']) ? $this->_config['interfaces'] : [];
137+
$interfaces = is_callable($interfaces) ? call_user_func($interfaces) : $interfaces;
138+
$this->_interfaces = $interfaces;
159139
}
160140
return $this->_interfaces;
161141
}

‎src/Type/Definition/WrappingType.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ interface WrappingType
88
NonNullType
99
ListOfType
1010
*/
11-
public function getWrappedType();
11+
public function getWrappedType($recurse = false);
1212
}

‎tests/Type/DefinitionTest.php‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,4 +342,66 @@ public function testProhibitsPuttingNonObjectTypesInUnions()
342342
}
343343
Config::disableValidation();
344344
}
345+
346+
public function testAllowsRecursiveDefinitions()
347+
{
348+
// See https://github.com/webonyx/graphql-php/issues/16
349+
$node = new InterfaceType([
350+
'name' => 'Node',
351+
'fields' => [
352+
'id' => ['type' => Type::nonNull(Type::id())]
353+
]
354+
]);
355+
356+
$blog = null;
357+
$called = false;
358+
359+
$user = new ObjectType([
360+
'name' => 'User',
361+
'fields' => function() use (&$blog, &$called) {
362+
$this->assertNotNull($blog, 'Blog type is expected to be defined at this point, but it is null');
363+
$called = true;
364+
365+
return [
366+
'id' => ['type' => Type::nonNull(Type::id())],
367+
'blogs' => ['type' => Type::nonNull(Type::listOf(Type::nonNull($blog)))]
368+
];
369+
},
370+
'interfaces' => function() use ($node) {
371+
return [$node];
372+
}
373+
]);
374+
375+
$blog = new ObjectType([
376+
'name' => 'Blog',
377+
'fields' => function() use ($user) {
378+
return [
379+
'id' => ['type' => Type::nonNull(Type::id())],
380+
'owner' => ['type' => Type::nonNull($user)]
381+
];
382+
},
383+
'interfaces' => function() use ($node) {
384+
return [$node];
385+
}
386+
]);
387+
388+
$schema = new Schema(new ObjectType([
389+
'name' => 'Query',
390+
'fields' => [
391+
'node' => ['type' => $node]
392+
]
393+
]));
394+
395+
$this->assertTrue($called);
396+
397+
$this->assertEquals([$node], $blog->getInterfaces());
398+
$this->assertEquals([$node], $user->getInterfaces());
399+
400+
$this->assertNotNull($user->getField('blogs'));
401+
$this->assertSame($blog, $user->getField('blogs')->getType()->getWrappedType(true));
402+
403+
$this->assertNotNull($blog->getField('owner'));
404+
$this->assertSame($user, $blog->getField('owner')->getType()->getWrappedType(true));
405+
406+
}
345407
}

0 commit comments

Comments
 (0)