Refactor Manager to register type/query directly

We need a reference to $manager in the type and query creators in order to build up relationships. This has to lazy load (can't operate on array pass-by-reference), since types needed in other types might not exist yet by the time TypeCreator->toType() is called.
open-sausages · Sep 22, 2016 · 6603a36 · 6603a36
1 parent 5781397
commit 6603a36
Show file tree

Hide file tree

Showing 8 changed files with 121 additions and 144 deletions.
diff --git a/README.md b/README.md
@@ -80,7 +80,9 @@ class MemberQueryCreator extends QueryCreator {
 
     public function type()
     {
-        return Type::listOf($this->types['member']->toType()));
+        return function() {
+            return Type::listOf($this->manager->getType('member');
+        };
     }
 
 

diff --git a/src/Controller.php b/src/Controller.php
@@ -25,7 +25,6 @@ public function index(HTTPRequest $request)
         }
 
         $query = isset($data['query']) ? $data['query'] : null;
-//        $operation = isset($data['operation']) ? $data['operation'] : null;
         $variables = isset($data['variables']) ? $data['variables'] : null;
 
         $manager = $this->getManager();
@@ -46,7 +45,7 @@ public function getManager()
 
         // Get a service rather than an instance (to allow procedural configuration)
         $config = Config::inst()->get('Chillu\GraphQL', 'schema');
-        $manager = Injector::inst()->create(Manager::class, $config);
+        $manager = Manager::createFromConfig($config);
 
         return $manager;
     }

diff --git a/src/FieldCreator.php b/src/FieldCreator.php
@@ -4,23 +4,22 @@
 
 use SilverStripe\Core\Object;
 use GraphQL\Type\Definition\Type;
+use Chillu\GraphQL\Manager;
 
 class FieldCreator extends Object
 {
     /**
-     * @var array Of type {@link \GraphQL\Type\Definition\ObjectType}.
-     *            Allows selection of an existing type in {@link type()}
+     * @var Manager
      */
-    protected $types = [];
+    protected $manager;
 
     /**
-     * @param array|null $types Of type {@link \GraphQL\Type\Definition\ObjectType}
+     * @param Manager|null Used to retrieve types (including the one returned from this creator),
+     * and nest field types regardless of instantiation of their creators.
      */
-    public function __construct($types = null)
+    public function __construct(Manager $manager = null)
     {
-        if ($types) {
-            $this->types = $types;
-        }
+        $this->manager = $manager;
 
         parent::__construct();
     }

diff --git a/src/Manager.php b/src/Manager.php
@@ -8,6 +8,7 @@
 use SilverStripe\Core\Injector\Injector;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Error;
+use GraphQL\Type\Definition\Type;
 
 class Manager
 {
@@ -26,19 +27,44 @@ class Manager
      */
     protected $errorFormatter = [self::class, 'formatError'];
 
-    public function __construct($config = null)
+    /**
+     * @param array $config An array with optional 'types' and 'queries' keys
+     * @return Manager
+     */
+    public static function createFromConfig($config)
     {
+        $manager = Injector::inst()->create(Manager::class);
         if ($config && array_key_exists('types', $config)) {
-            foreach ($config['types'] as $name => $type) {
-                $this->addType($type, $name);
+            foreach ($config['types'] as $name => $typeCreatorClass) {
+                $typeCreator = Injector::inst()->create($typeCreatorClass, $manager);
+                if (!($typeCreator instanceof TypeCreator)) {
+                    throw new InvalidArgumentException(sprintf(
+                        'The type named "%s" needs to be a class extending ' . TypeCreator::class,
+                        $name
+                    ));
+                }
+
+                $type = $typeCreator->toType();
+                $manager->addType($type, $name);
             }
         }
 
         if ($config && array_key_exists('queries', $config)) {
-            foreach ($config['queries'] as $name => $query) {
-                $this->addQuery($query, $name);
+            foreach ($config['queries'] as $name => $queryCreatorClass) {
+                $queryCreator = Injector::inst()->create($queryCreatorClass, $manager);
+                if (!($queryCreator instanceof QueryCreator)) {
+                    throw new InvalidArgumentException(sprintf(
+                        'The type named "%s" needs to be a class extending ' . QueryCreator::class,
+                        $name
+                    ));
+                }
+
+                $query = $queryCreator->toArray();
+                $manager->addQuery($query, $name);
             }
         }
+
+        return $manager;
     }
 
     /**
@@ -48,9 +74,7 @@ public function schema()
     {
         $queryType = new ObjectType([
             'name' => 'Query',
-            'fields' => array_map(function ($query) {
-                return $query->toArray();
-            }, $this->queries),
+            'fields' => $this->queries,
         ]);
 
         return new Schema([
@@ -81,6 +105,13 @@ public function query($query, $params = [], $schema = null)
         }
     }
 
+    /**
+     * @param string $query
+     * @param array  $params
+     * @param null   $schema
+     *
+     * @return array
+     */
     public function queryAndReturnResult($query, $params = [], $schema = null)
     {
         $schema = $this->schema($schema);
@@ -90,24 +121,14 @@ public function queryAndReturnResult($query, $params = [], $schema = null)
     }
 
     /**
-     * @param string|Type $type An instance of {@link Chillu\GraphQL\TypeCreator} (or a class name)
-     * @param string      $name An optional identifier for this type (defaults to class name)
+     * @param Type   $type
+     * @param string $name An optional identifier for this type (defaults to 'name' attribute in type definition).
+     *                     Needs to be unique in schema.
      */
-    public function addType($type, $name = '')
+    public function addType(Type $type, $name = '')
     {
-        if (!$name) {
-            $name = is_object($type) ? get_class($type) : $type;
-        }
-
-        if (!is_object($type)) {
-            $type = Injector::inst()->get($type);
-        }
-
-        if (!($type instanceof TypeCreator)) {
-            throw new InvalidArgumentException(sprintf(
-                'The type named "%s" needs to be a class name or instance of Chillu\GraphQL\TypeCreator',
-                $name
-            ));
+        if(!$name) {
+            $name = (string)$type;
         }
 
         $this->types[$name] = $type;
@@ -116,41 +137,26 @@ public function addType($type, $name = '')
     /**
      * @param string $name
      *
-     * @return TypeCreator
+     * @return Type
      */
     public function getType($name)
     {
         return $this->types[$name];
     }
 
     /**
-     * @param string|Type $query An instance of {@link Chillu\GraphQL\QueryCreator} (or a class name)
-     * @param string      $name  An optional identifier for this type (defaults to class name)
+     * @param array  $query
+     * @param string $name Identifier for this query (unique in schema)
      */
-    public function addQuery($query, $name = '')
+    public function addQuery($query, $name)
     {
-        if (!$name) {
-            $name = is_object($query) ? get_class($query) : $query;
-        }
-
-        if (!is_object($query)) {
-            $query = Injector::inst()->create($query, $this->types);
-        }
-
-        if (!($query instanceof QueryCreator)) {
-            throw new InvalidArgumentException(sprintf(
-                'The type named "%s" needs to be a class name or instance of Chillu\GraphQL\QueryCreator',
-                $name
-            ));
-        }
-
         $this->queries[$name] = $query;
     }
 
     /**
      * @param string $name
      *
-     * @return Type
+     * @return array
      */
     public function getQuery($name)
     {

diff --git a/src/TypeCreator.php b/src/TypeCreator.php
@@ -4,13 +4,31 @@
 
 use SilverStripe\Core\Object;
 use GraphQL\Type\Definition\ObjectType;
+use Chillu\GraphQL\Manager;
 
 /**
  * Represents a GraphQL type in a way that allows customisation
  * through SilverStripe's DataExtension system.
  */
 class TypeCreator extends Object
 {
+
+    /**
+     * @var Manager
+     */
+    protected $manager;
+
+    /**
+     * @param Manager|null Used to retrieve types (including the one returned from this creator),
+     * and nest field types regardless of instantiation of their creators.
+     */
+    public function __construct(Manager $manager = null)
+    {
+        $this->manager = $manager;
+
+        parent::__construct();
+    }
+
     /**
      * @return array
      */

diff --git a/tests/ControllerTest.php b/tests/ControllerTest.php
@@ -17,15 +17,16 @@ public function testIndex()
     {
         $controller = new Controller();
         $manager = new Manager();
-        $manager->addType(TypeCreatorFake::class, 'mytype');
-        $manager->addQuery(QueryCreatorFake::class, 'myquery');
+        $manager->addType($this->getType($manager), 'mytype');
+        $manager->addQuery($this->getQuery($manager), 'myquery');
         $controller->setManager($manager);
         $response = $controller->index(new HTTPRequest('GET', ''));
         $this->assertFalse($response->isError());
     }
 
     public function testGetGetManagerPopulatesFromConfig()
     {
+        Config::inst()->remove('Chillu\GraphQL', 'schema');
         Config::inst()->update('Chillu\GraphQL', 'schema', [
             'types' => [
                 'mytype' => TypeCreatorFake::class,
@@ -37,9 +38,18 @@ public function testGetGetManagerPopulatesFromConfig()
         $method = $reflection->getMethod('getManager');
         $method->setAccessible(true);
         $manager = $method->invoke($controller);
-        $this->assertInstanceOf(
-            TypeCreatorFake::class,
+        $this->assertNotNull(
             $manager->getType('mytype')
         );
     }
+
+    protected function getType(Manager $manager)
+    {
+        return (new TypeCreatorFake($manager))->toType();
+    }
+
+    protected function getQuery(Manager $manager)
+    {
+        return (new QueryCreatorFake($manager))->toArray();
+    }
 }
diff --git a/tests/Fake/QueryCreatorFake.php b/tests/Fake/QueryCreatorFake.php
@@ -9,7 +9,8 @@ class QueryCreatorFake extends QueryCreator
 {
     public function type()
     {
-        // TODO Avoid type conversion in userland
-        return Type::listOf($this->types['mytype']->toType());
+        return function() {
+            return Type::listOf($this->manager->getType('mytype'));
+        };
     }
 }