fix(schema): Fix altering schema extensions when they are empty initi…

…ally (#1398)
drupal-graphql · May 30, 2024 · 46a63fd · 46a63fd
1 parent 40a9b80
commit 46a63fd
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 10 deletions.
diff --git a/src/Plugin/GraphQL/Schema/AlterableComposableSchema.php b/src/Plugin/GraphQL/Schema/AlterableComposableSchema.php
@@ -169,7 +169,8 @@ protected function getExtensionDocument(array $extensions = []) {
       $event,
       AlterSchemaExtensionDataEvent::EVENT_NAME
     );
-    $ast = !empty($extensions) ? Parser::parse(implode("\n\n", $event->getSchemaExtensionData()), ['noLocation' => TRUE]) : NULL;
+    $extensions = array_filter($event->getSchemaExtensionData());
+    $ast = !empty($extensions) ? Parser::parse(implode("\n\n", $extensions), ['noLocation' => TRUE]) : NULL;
 
     // No AST caching here as that will be done in getFullSchemaDocument().
     return $ast;

diff --git a/tests/modules/graphql_alterable_schema_test/src/EventSubscriber/GraphQlSubscriber.php b/tests/modules/graphql_alterable_schema_test/src/EventSubscriber/GraphQlSubscriber.php
@@ -33,7 +33,18 @@ public function alterSchemaExtensionData(AlterSchemaExtensionDataEvent $event):
     $schemaData = $event->getSchemaExtensionData();
     // I do not recommend direct replace, better user parsing or regex.
     // But this is an example of what you can do.
-    $schemaData['graphql_alterable_schema_test'] = str_replace('position: Int', 'position: Int!', $schemaData['graphql_alterable_schema_test']);
+    $schemaData['graphql_alterable_schema_test'] = str_replace('position: Int', 'position: Int!', $schemaData['graphql_alterable_schema_test'] ?? '');
+
+    // Test empty extensions can still extend the schema.
+    // https://github.com/drupal-graphql/graphql/issues/1395
+    if (empty($schemaData['graphql_alterable_schema_test'])) {
+      $schemaData['graphql_alterable_schema_test'] = <<<GQL
+        extend type Result {
+          empty: Boolean!
+        }
+      GQL;
+    }
+
     $event->setSchemaExtensionData($schemaData);
   }
 

diff --git a/tests/src/Kernel/AlterableSchemaTest.php b/tests/src/Kernel/AlterableSchemaTest.php
@@ -1,11 +1,10 @@
 <?php
 
-namespace Drupal\Tests\graphql\Kernel\Framework;
+namespace Drupal\Tests\graphql\Kernel;
 
 use Drupal\graphql\GraphQL\ResolverRegistry;
 use Drupal\graphql\Plugin\GraphQL\SchemaExtension\SdlSchemaExtensionPluginBase;
 use Drupal\graphql\Plugin\SchemaExtensionPluginManager;
-use Drupal\Tests\graphql\Kernel\GraphQLTestBase;
 use Drupal\Tests\graphql\Kernel\Schema\AlterableComposableTestSchema;
 
 /**
@@ -104,6 +103,40 @@ public function testSchemaExtensionAlteredQueryResultPropertyToNonNull(): void {
     ], json_decode($result->getContent(), TRUE));
   }
 
+  /**
+   * Test if schema extension altering is working with empty extensions.
+   */
+  public function testEmptySchemaExtensionAlteredQueryResultPropertyAdded(): void {
+    $result = $this->query('query { alterableQuery(id: 1) { id, empty } }');
+    $this->assertSame(200, $result->getStatusCode());
+    // Here should be error that query result empty variable cannot be null.
+    // This leads to the internal server error with reference to the variable.
+    $this->assertSame([
+      'errors' => [
+        0 => [
+          'message' => 'Internal server error',
+          'extensions' => [
+            'category' => 'internal',
+          ],
+          'locations' => [
+            0 => [
+              'line' => 1,
+              'column' => 37,
+            ],
+          ],
+          'path' => [
+            'alterableQuery',
+            // Reference to our variable in the error.
+            'empty',
+          ],
+        ],
+      ],
+      'data' => [
+        'alterableQuery' => NULL,
+      ],
+    ], json_decode($result->getContent(), TRUE));
+  }
+
   /**
    * {@inheritdoc}
    */
@@ -124,15 +157,24 @@ protected function mockSchema($id, $schema, array $extensions = []): void {
       ->method('getBaseDefinition')
       ->willReturn('');
 
-    $extensions['graphql_alterable_schema_test']->expects(static::any())
-      ->method('getExtensionDefinition')
-      ->willReturn(
-        <<<GQL
+    // Different extension definition for different tests.
+    switch ($this->getName()) {
+      case 'testEmptySchemaExtensionAlteredQueryResultPropertyAdded':
+        $extensionDefinition = '';
+        break;
+
+      default:
+        $extensionDefinition = <<<GQL
           extend type Result {
             position: Int
           }
-        GQL
-      );
+        GQL;
+        break;
+    }
+
+    $extensions['graphql_alterable_schema_test']->expects(static::any())
+      ->method('getExtensionDefinition')
+      ->willReturn($extensionDefinition);
 
     $extensionManager->expects(static::any())
       ->method('getExtensions')