WIP includeSubclasses

Approximating "Option C" in silverstripe#209. Probably will be abandoned for an alternative "Option D" approach.
open-sausages · Feb 19, 2019 · c1ca9f7 · c1ca9f7
1 parent c691a47
commit c1ca9f7
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 35 deletions.
diff --git a/src/Scaffolding/Scaffolders/DataObjectScaffolder.php b/src/Scaffolding/Scaffolders/DataObjectScaffolder.php
@@ -5,6 +5,7 @@
 use Exception;
 use GraphQL\Type\Definition\ObjectType;
 use InvalidArgumentException;
+use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Core\Extensible;
 use SilverStripe\Core\Injector\Injector;
@@ -63,6 +64,13 @@ class DataObjectScaffolder implements ManagerMutatorInterface, ScaffolderInterfa
      */
     protected $nestedQueries = [];
 
+    /**
+     * @var bool Include fields from descendants in the same type.
+     * If set to false, scaffolded read queries will return a union
+     * of types.
+     */
+    protected $flattenDescendants = false;
+
     /**
      * DataObjectScaffold constructor.
      *
@@ -75,10 +83,26 @@ public function __construct($dataObjectClass)
         $this->setDataObjectClass($dataObjectClass);
     }
 
+    /**
+     * @return bool
+     */
+    public function getFlattenDescendants()
+    {
+        return $this->flattenDescendants;
+    }
+
+    /**
+     * @param bool $flattenDescendants
+     */
+    public function setFlattenDescendants($flattenDescendants)
+    {
+        $this->flattenDescendants = $flattenDescendants;
+    }
+
     /**
      * Name of graphql type
      *
-     * @return string
+     * @return strinnestedQueryg
      */
     public function getTypeName()
     {
@@ -129,12 +153,12 @@ public function addField($field, $description = null)
      * Adds all db fields, and optionally has_one.
      *
      * @param bool $includeHasOne
-     *
+     * @param bool $includeSubclasses Includes fields from all subclasses
      * @return $this
      */
-    public function addAllFields($includeHasOne = false)
+    public function addAllFields($includeHasOne = false, $includeSubclasses = false)
     {
-        $fields = $this->allFieldsFromDataObject($includeHasOne);
+        $fields = $this->allFieldsFromDataObject($includeHasOne, $includeSubclasses);
 
         return $this->addFields($fields);
     }
@@ -144,15 +168,15 @@ public function addAllFields($includeHasOne = false)
      *
      * @param array|string $exclusions
      * @param bool $includeHasOne
-     *
+     * @param bool $includeSubclasses Includes fields from all subclasses
      * @return $this
      */
-    public function addAllFieldsExcept($exclusions, $includeHasOne = false)
+    public function addAllFieldsExcept($exclusions, $includeHasOne = false, $includeSubclasses = false)
     {
         if (!is_array($exclusions)) {
             $exclusions = [$exclusions];
         }
-        $fields = $this->allFieldsFromDataObject($includeHasOne);
+        $fields = $this->allFieldsFromDataObject($includeHasOne, $includeSubclasses);
         $filteredFields = array_diff($fields, $exclusions);
 
         return $this->addFields($filteredFields);
@@ -378,10 +402,14 @@ function ($obj) use ($fieldName) {
      */
     public function getDependentClasses()
     {
-        return array_merge(
-            array_values($this->nestedDataObjectClasses()),
-            array_values($this->nestedConnections())
-        );
+        $classes = array_values($this->nestedConnections());
+
+        // Descendants are only "dependant" if their fields aren't flattened into the "base type"
+        if (!$this->getFlattenDescendants()) {
+            $classes = array_merge($classes, array_values($this->nestedDataObjectClasses()));
+        }
+
+        return $classes;
     }
 
     /**
@@ -407,7 +435,7 @@ public function cloneTo(DataObjectScaffolder $target)
         $inst = $target->getDataObjectInstance();
 
         foreach ($this->getFields() as $field) {
-            if (StaticSchema::inst()->isValidFieldName($inst, $field->Name)) {
+            if (StaticSchema::inst()->isValidFieldName($inst, $field->Name, $this->getFlattenDescendants())) {
                 $target->addField($field->Name, $field->Description);
             }
         }
@@ -429,14 +457,20 @@ public function cloneTo(DataObjectScaffolder $target)
     public function applyConfig(array $config)
     {
         $dataObjectClass = $this->getDataObjectClass();
+
         if (empty($config['fields'])) {
             throw new Exception(
                 "No array of fields defined for $dataObjectClass"
             );
         }
+
+        if (isset($config['flattenDescendants'])) {
+            $this->setFlattenDescendants((bool)$config['flattenDescendants']);
+        }
+
         if (isset($config['fields'])) {
             if ($config['fields'] === SchemaScaffolder::ALL) {
-                $this->addAllFields(true);
+                $this->addAllFields(true, $this->getFlattenDescendants());
             } elseif (is_array($config['fields'])) {
                 $this->addFields($config['fields']);
             } else {
@@ -578,21 +612,24 @@ public function addToManager(Manager $manager)
     }
 
     /**
-     * @param bool $includeHasOne
-     *
+     * @param bool $includeHasOne Includes has_one as nested types (not foreign key fields)
+     * @param bool $includeSubclasses Includes fields from all subclasses
      * @return array
      */
-    protected function allFieldsFromDataObject($includeHasOne = false)
+    protected function allFieldsFromDataObject($includeHasOne = false, $includeSubclasses = false)
     {
-        $fields = [];
-        $db = DataObject::config()->get('fixed_fields');
-        $extra = Config::inst()->get($this->getDataObjectClass(), 'db');
-        if ($extra) {
-            $db = array_merge($db, $extra);
-        }
+        $fields = array_keys(DataObject::config()->get('fixed_fields'));
+
+        // Not using DataObjectSchema->databaseFields() because it includes has_one as foreign keys,
+        // and composite fields which can't be resolved without special handling.
+        $baseFieldSpecs = Config::inst()->get($this->getDataObjectClass(), 'db', Config::UNINHERITED) ?: [];
+        $fields = array_merge($fields, array_keys($baseFieldSpecs));
 
-        foreach ($db as $fieldName => $type) {
-            $fields[] = $fieldName;
+        if ($includeSubclasses) {
+            foreach (ClassInfo::subclassesFor($this->getDataObjectClass()) as $subclass) {
+                $subclassFieldSpecs = Config::inst()->get($subclass, 'db', Config::UNINHERITED) ?: [];
+                $fields = array_merge($fields, array_keys($subclassFieldSpecs));
+            }
         }
 
         if ($includeHasOne) {
@@ -678,7 +715,7 @@ protected function createFields(Manager $manager)
 
         foreach ($this->fields as $fieldData) {
             $fieldName = $fieldData->Name;
-            if (!StaticSchema::inst()->isValidFieldName($instance, $fieldName)) {
+            if (!StaticSchema::inst()->isValidFieldName($instance, $fieldName, $this->getFlattenDescendants())) {
                 throw new InvalidArgumentException(
                     sprintf(
                         'Invalid field "%s" on %s',

diff --git a/src/Scaffolding/Scaffolders/SchemaScaffolder.php b/src/Scaffolding/Scaffolders/SchemaScaffolder.php
@@ -291,13 +291,18 @@ public function addToManager(Manager $manager)
         // Add all DataObjects to the manager
         foreach ($this->types as $scaffold) {
             $scaffold->addToManager($manager);
-            $inheritanceScaffolder = new InheritanceScaffolder(
-                $scaffold->getDataObjectClass(),
-                StaticSchema::config()->get('inheritanceTypeSuffix')
-            );
-            // Due to shared ancestry, it's inevitable that the same union type will get added multiple times.
-            if (!$manager->hasType($inheritanceScaffolder->getName())) {
-                $inheritanceScaffolder->addToManager($manager);
+
+            // Optionally add descendant types. If this isn't set,
+            // fields on those descendant types can only be queried via type-specific operations.
+            if (!$scaffold->getFlattenDescendants()) {
+                $inheritanceScaffolder = new InheritanceScaffolder(
+                    $scaffold->getDataObjectClass(),
+                    StaticSchema::config()->get('inheritanceTypeSuffix')
+                );
+                // Due to shared ancestry, it's inevitable that the same union type will get added multiple times.
+                if (!$manager->hasType($inheritanceScaffolder->getName())) {
+                    $inheritanceScaffolder->addToManager($manager);
+                }
             }
         }
 

diff --git a/src/Scaffolding/StaticSchema.php b/src/Scaffolding/StaticSchema.php
@@ -128,11 +128,27 @@ public function typeName($str)
      *
      * @param ViewableData $instance
      * @param string $fieldName
+     * @param bool $includeSubclasses
      * @return bool
      */
-    public function isValidFieldName(ViewableData $instance, $fieldName)
+    public function isValidFieldName(ViewableData $instance, $fieldName, $includeSubclasses = false)
     {
-        return ($instance->hasMethod($fieldName) || $instance->hasField($fieldName));
+        $isValid = false;
+
+        if ($includeSubclasses) {
+            // Includes current class instance
+            foreach(ClassInfo::subclassesFor(get_class($instance)) as $subclass) {
+                $subclassInstance = singleton($subclass);
+                if (!$isValid && $subclassInstance->hasMethod($fieldName) || $subclassInstance->hasField($fieldName)) {
+                    // Early exit to improve performance
+                    $isValid = true;
+                }
+            }
+        } else {
+            $isValid = ($instance->hasMethod($fieldName) || $instance->hasField($fieldName));
+        }
+
+        return $isValid;
     }
 
     /**

diff --git a/tests/Fake/ExtendedDataObjectFake.php b/tests/Fake/ExtendedDataObjectFake.php
@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\GraphQL\Tests\Fake;
+
+use SilverStripe\Dev\TestOnly;
+
+class ExtendedDataObjectFake extends DataObjectFake implements TestOnly
+{
+    private static $table_name = 'GraphQL_ExtendedDataObjectFake';
+
+    private static $db = [
+        'MyExtendedField' => 'Varchar',
+    ];
+}
diff --git a/tests/Scaffolding/Scaffolders/DataObjectScaffolderTest.php b/tests/Scaffolding/Scaffolders/DataObjectScaffolderTest.php
@@ -19,6 +19,7 @@
 use SilverStripe\GraphQL\Scaffolding\Scaffolders\QueryScaffolder;
 use SilverStripe\GraphQL\Scaffolding\Scaffolders\SchemaScaffolder;
 use SilverStripe\GraphQL\Tests\Fake\DataObjectFake;
+use SilverStripe\GraphQL\Tests\Fake\ExtendedDataObjectFake;
 use SilverStripe\GraphQL\Tests\Fake\FakePage;
 use SilverStripe\GraphQL\Tests\Fake\FakeRedirectorPage;
 use SilverStripe\GraphQL\Tests\Fake\FakeSiteTree;
@@ -86,7 +87,7 @@ public function testDataObjectScaffolderFields()
         );
 
         $scaffolder = $this->getFakeScaffolder();
-        $scaffolder->addAllFields(true);
+        $scaffolder->addAllFields($includeHasOne=true);
         $this->assertEquals(
             ['ID', 'ClassName', 'LastEdited', 'Created', 'MyField', 'MyInt', 'Author'],
             $scaffolder->getFields()->column('Name')
@@ -118,6 +119,32 @@ public function testDataObjectScaffolderFields()
         );
     }
 
+    public function testDataObjectScaffolderFieldsIncludeSublasses()
+    {
+        // DataObjectFake has a subclass called ExtendedDataObjectFake with MyExtendedField
+
+        $scaffolder = $this->getFakeScaffolder();
+        $scaffolder->addAllFields($includeHasOne=false, $includeSubclasses=false);
+        $this->assertEquals(
+            ['ID', 'ClassName', 'LastEdited', 'Created', 'MyField', 'MyInt'],
+            $scaffolder->getFields()->column('Name')
+        );
+
+        $scaffolder = $this->getFakeScaffolder();
+        $scaffolder->addAllFields($includeHasOne=false, $includeSubclasses=true);
+        $this->assertEquals(
+            ['ID', 'ClassName', 'LastEdited', 'Created', 'MyField', 'MyInt', 'MyExtendedField'],
+            $scaffolder->getFields()->column('Name')
+        );
+
+        $scaffolder = $this->getFakeScaffolder();
+        $scaffolder->addAllFieldsExcept(['MyExtendedField'], $includeHasOne=false, $includeSubclasses=true);
+        $this->assertEquals(
+            ['ID', 'ClassName', 'LastEdited', 'Created', 'MyField', 'MyInt'],
+            $scaffolder->getFields()->column('Name')
+        );
+    }
+
     public function testDataObjectScaffolderOperations()
     {
         $scaffolder = $this->getFakeScaffolder();
@@ -490,4 +517,12 @@ protected function getFakeScaffolder()
     {
         return new DataObjectScaffolder(DataObjectFake::class);
     }
+
+    /**
+     * @return DataObjectScaffolder
+     */
+    protected function getExtendedFakeScaffolder()
+    {
+        return new DataObjectScaffolder(ExtendedDataObjectFake::class);
+    }
 }