BUGFIX: Throw on duplicate derived model names (#421)

src/Config/ModelConfiguration.php

@@ -47,17 +47,12 @@ public function getOperationConfig(string $operation): array
 
     /**
      * @param string $class
+     * @param array $mapping
      * @return string
      * @throws SchemaBuilderException
      */
-    public function getTypeName(string $class): string
+    public function getTypeName(string $class, $mapping = []): string
     {
-        $mapping = $this->get('type_mapping', []);
-        $custom = $mapping[$class] ?? null;
-        if ($custom) {
-            return $custom;
-        }
-
         $typeName = $this->formatClass($class);
         $prefix = $this->getPrefix($class);
 

src/Schema/DataObject/DataObjectModel.php

@@ -372,7 +372,13 @@ public function getDataObject(): DataObject
      */
     public function getTypeName(): string
     {
-        return $this->getModelConfiguration()->getTypeName(get_class($this->dataObject));
+        $class = get_class($this->dataObject);
+        $mapping = $this->getSchemaConfig()->getTypeMapping();
+        if (isset($mapping[$class])) {
+            return $mapping[$class];
+        }
+
+        return $this->getModelConfiguration()->getTypeName($class);
     }
 
     /**

src/Schema/Schema.php

@@ -860,6 +860,16 @@ public function removeScalar(string $name): self
     public function addModel(ModelType $modelType, ?callable $callback = null): Schema
     {
         $existing = $this->models[$modelType->getName()] ?? null;
+        if ($existing) {
+            Schema::invariant(
+                $existing->getModel()->getSourceClass() === $modelType->getModel()->getSourceClass(),
+                'Name collision for model "%s". It is used for both %s and %s. Use the tyepMapping setting in your schema
+            config to provide a custom name for the model.',
+                $modelType->getName(),
+                $existing->getModel()->getSourceClass(),
+                $modelType->getModel()->getSourceClass()
+            );
+        }
         $typeObj = $existing
             ? $existing->mergeWith($modelType)
             : $modelType;

src/Schema/SchemaConfig.php

@@ -140,6 +140,15 @@ public function setTypeMapping(array $typeMapping): self
         return $this->set('typeMapping', $typeMapping);
     }
 
+    /**
+     * @return array
+     * @throws SchemaBuilderException
+     */
+    public function getTypeMapping(): array
+    {
+        return $this->get('typeMapping', []);
+    }
+
     /**
      * @param string[] $fields
      * @return $this

tests/Fake/SubFake/FakePage.php

@@ -0,0 +1,14 @@
+<?php
+
+namespace SilverStripe\GraphQL\Tests\Fake\SubFake;
+
+use SilverStripe\GraphQL\Tests\Fake\FakeSiteTree;
+
+class FakePage extends FakeSiteTree
+{
+    private static $table_name = "GraphQL_SubFakePage";
+
+    private static $db = [
+        'SubFakePageField' => 'Varchar',
+    ];
+}

tests/Schema/DataObject/Plugin/DBFieldArgs/DBTimeArgsTest.php

@@ -12,7 +12,7 @@
 use SilverStripe\ORM\FieldType\DBField;
 use SilverStripe\ORM\FieldType\DBTime;
 
-class DBTimeTest extends SapphireTest
+class DBTimeArgsTest extends SapphireTest
 {
     public function testApply()
     {

tests/Schema/SchemaContextTest.php

@@ -6,12 +6,17 @@
 use SilverStripe\Dev\SapphireTest;
 use SilverStripe\GraphQL\Schema\DataObject\DataObjectModel;
 use SilverStripe\GraphQL\Schema\Exception\SchemaBuilderException;
+use SilverStripe\GraphQL\Schema\Field\Field;
+use SilverStripe\GraphQL\Schema\Resolver\DefaultResolverStrategy;
 use SilverStripe\GraphQL\Schema\Schema;
 use SilverStripe\GraphQL\Schema\SchemaBuilder;
 use SilverStripe\GraphQL\Schema\SchemaConfig;
 use SilverStripe\GraphQL\Schema\Type\ModelType;
+use SilverStripe\GraphQL\Schema\Type\Type;
 use SilverStripe\GraphQL\Tests\Fake\DataObjectFake;
 use SilverStripe\GraphQL\Tests\Fake\FakeSiteTree;
+use SilverStripe\GraphQL\Tests\Fake\SchemaContextTestResolverA;
+use SilverStripe\GraphQL\Tests\Fake\SchemaContextTestResolverB;
 
 class SchemaContextTest extends SapphireTest
 {
@@ -41,4 +46,31 @@ public function testGetTypeNameForClass()
             $storeableSchema->getConfig()->getTypeNameForClass(FakeSiteTree::class)
         );
     }
+
+    public function testResolverDiscovery()
+    {
+        $context = new SchemaConfig([
+            'resolvers' => [
+                SchemaContextTestResolverA::class,
+                SchemaContextTestResolverB::class,
+            ],
+            'resolverStrategy' => [DefaultResolverStrategy::class, 'getResolverMethod']
+        ]);
+
+        $result = $context->discoverResolver(new Type('TypeName'), new Field('fieldName'));
+        $this->assertEquals('resolveTypeNameFieldName', $result->getMethod());
+        $this->assertEquals(SchemaContextTestResolverA::class, $result->getClass());
+
+        $result = $context->discoverResolver(new Type('TypeName'), new Field('foo'));
+        $this->assertEquals('resolveTypeName', $result->getMethod());
+        $this->assertEquals(SchemaContextTestResolverA::class, $result->getClass());
+
+        $result = $context->discoverResolver(new Type('Nothing'), new Field('foo'));
+        $this->assertEquals('resolve', $result->getMethod());
+        $this->assertEquals(SchemaContextTestResolverA::class, $result->getClass());
+
+        $result = $context->discoverResolver(new Type('Nothing'), new Field('specialField'));
+        $this->assertEquals('resolveSpecialField', $result->getMethod());
+        $this->assertEquals(SchemaContextTestResolverB::class, $result->getClass());
+    }
 }

tests/Schema/SchemaTest.php

@@ -4,12 +4,11 @@
 
 use SilverStripe\Core\Config\Config;
 use SilverStripe\Dev\SapphireTest;
-use SilverStripe\GraphQL\Schema\DataObject\DataObjectModel;
+use SilverStripe\GraphQL\Config\ModelConfiguration;
 use SilverStripe\GraphQL\Schema\DataObject\ModelCreator;
 use SilverStripe\GraphQL\Schema\Exception\SchemaBuilderException;
 use SilverStripe\GraphQL\Schema\Field\Mutation;
 use SilverStripe\GraphQL\Schema\Field\Query;
-use SilverStripe\GraphQL\Schema\Interfaces\SchemaStorageInterface;
 use SilverStripe\GraphQL\Schema\Schema;
 use SilverStripe\GraphQL\Schema\SchemaConfig;
 use SilverStripe\GraphQL\Schema\Type\Enum;
@@ -19,7 +18,9 @@
 use SilverStripe\GraphQL\Schema\Type\Type;
 use SilverStripe\GraphQL\Schema\Type\UnionType;
 use SilverStripe\GraphQL\Tests\Fake\DataObjectFake;
+use SilverStripe\GraphQL\Tests\Fake\FakePage;
 use SilverStripe\GraphQL\Tests\Fake\FakeSiteTree;
+use SilverStripe\GraphQL\Tests\Fake\SubFake\FakePage as SubFakePage;
 
 class SchemaTest extends SapphireTest
 {
@@ -259,6 +260,27 @@ public function testValidConfig()
         Schema::assertValidConfig([]);
     }
 
+    public function testDuplicateModelThrows()
+    {
+        $schema = $this->buildSchema();
+        $schema->addModelbyClassName(FakePage::class);
+        $this->expectException(SchemaBuilderException::class);
+        $schema->addModelbyClassName(SubFakePage::class);
+    }
+
+    public function testTypeMapping()
+    {
+        $schema = $this->buildSchema();
+        $schema->getConfig()->setTypeMapping([
+            SubFakePage::class => 'SubFakePage',
+        ]);
+        $schema->addModelbyClassName(FakePage::class);
+        $schema->addModelbyClassName(SubFakePage::class);
+
+        $this->assertInstanceOf(ModelType::class, $schema->getModel('FakePage'));
+        $this->assertInstanceOf(ModelType::class, $schema->getModel('SubFakePage'));
+    }
+
     public static function noop()
     {
     }