Merge pull request #660 from rebing/mfn-detect-unused

Add ability to detect unused variables

CHANGELOG.md

@@ -46,6 +46,7 @@ CHANGELOG
 ### Added
 - Automatic Persisted Queries (APQ) now cache the parsed query [\#740 / mfn](https://github.com/rebing/graphql-laravel/pull/740)\
   This avoids having to re-parse the same queries over and over again.
+- Add ability to detect unused GraphQL variables [\#660 / mfn](https://github.com/rebing/graphql-laravel/pull/660)
 
 2021-04-10, 7.2.0
 -----------------

README.md

@@ -131,6 +131,7 @@ To work this around:
     - [Default field resolver](#default-field-resolver)
     - [Macros](#macros)
     - [Automatic Persisted Queries support](#automatic-persisted-queries-support) 
+    - [Misc features](#misc-features)
   - [Guides](#guides)
     - [Upgrading from v1 to v2](#upgrading-from-v1-to-v2)
     - [Migrating from Folklore](#migrating-from-folklore)
@@ -2492,6 +2493,36 @@ const app = new Vue({
 </script>
 ```
 
+## Misc features
+
+### Detecting unused variables
+
+By default, `'variables'` provided alongside the GraphQL query which are **not**
+consumed, are silently ignored.
+
+If you consider the hypothetical case you have an optional (nullable) argument
+in your query, and you provide a variable argument for it but you make a typo,
+this can go unnoticed.
+
+Example:
+```graphql
+mutation test($value:ID) {
+  someMutation(type:"falbala", optional_id: $value)
+}
+```
+Variables provided:
+```json5
+{
+  // Ops! typo in `values`
+  "values": "138"
+}
+```
+
+In this case, nothing happens and `optional_id` will be treated as not being provided.
+
+To prevent such scenarios, you can enable the config option `detect_unused_variables`
+and set it to `true`.
+
 ## Guides
 
 ### Upgrading from v1 to v2

config/config.php

@@ -235,4 +235,9 @@
         // The cache ttl in minutes - See https://www.apollographql.com/docs/apollo-server/performance/apq/#adjusting-cache-time-to-live-ttl
         'cache_ttl' => 300,
     ],
+
+    /*
+     * If enabled, variables provided but not consumed by the query will throw an error
+     */
+    'detect_unused_variables' => false,
 ];

src/GraphQL.php

@@ -11,6 +11,8 @@
 use GraphQL\Executor\ExecutionResult;
 use GraphQL\GraphQL as GraphQLBase;
 use GraphQL\Language\AST\DocumentNode;
+use GraphQL\Language\AST\OperationDefinitionNode;
+use GraphQL\Language\Parser;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Definition\Type;
 use GraphQL\Type\Schema;
@@ -115,7 +117,9 @@ public function schema($schema = null): Schema
      */
     public function query($query, ?array $variables = [], array $opts = []): array
     {
-        return $this->queryAndReturnResult($query, $variables, $opts)->toArray();
+        $result = $this->queryAndReturnResult($query, $variables, $opts);
+
+        return $this->decorateExecutionResult($result)->toArray();
     }
 
     /**
@@ -133,10 +137,17 @@ public function queryAndReturnResult($query, ?array $variables = [], array $opts
         $schema = $this->schema($schemaName);
 
         $defaultFieldResolver = config('graphql.defaultFieldResolver', null);
+        $detectUnusedVariables = config('graphql.detect_unused_variables', false);
+
+        if ($variables && $detectUnusedVariables) {
+            $result = $this->detectUnusedVariables($query, $variables);
 
-        $result = GraphQLBase::executeQuery($schema, $query, $rootValue, $context, $variables, $operationName, $defaultFieldResolver);
+            if ($result) {
+                return $result;
+            }
+        }
 
-        return $this->decorateExecutionResult($result);
+        return GraphQLBase::executeQuery($schema, $query, $rootValue, $context, $variables, $operationName, $defaultFieldResolver);
     }
 
     public function addTypes(array $types): void
@@ -550,4 +561,45 @@ public function decorateExecutionResult(ExecutionResult $executionResult): Execu
             ->setErrorsHandler($errorsHandler)
             ->setErrorFormatter($errorFormatter);
     }
+
+    /**
+     * Returning an ExecutionResult here is an indicator of an error
+     * (either due to parsing the query or because unused variables were detected).
+     *
+     * Otherwise "all is good" and `null` is an indicator to carry on.
+     *
+     * @param string|DocumentNode $query
+     * @param array<string,mixed> $variables
+     */
+    protected function detectUnusedVariables($query, array $variables): ?ExecutionResult
+    {
+        if (is_string($query)) {
+            try {
+                $query = Parser::parse($query);
+            } catch (Error $error) {
+                return new ExecutionResult(null, [$error]);
+            }
+        }
+
+        $unusedVariables = $variables;
+
+        foreach ($query->definitions as $definition) {
+            if ($definition instanceof OperationDefinitionNode) {
+                foreach ($definition->variableDefinitions as $variableDefinition) {
+                    unset($unusedVariables[$variableDefinition->variable->name->value]);
+                }
+            }
+        }
+
+        if (!$unusedVariables) {
+            return null;
+        }
+
+        $msg = sprintf(
+            'The following variables were provided but not consumed: %s',
+            implode(', ', array_keys($unusedVariables))
+        );
+
+        return new ExecutionResult(null, [new Error($msg)]);
+    }
 }

tests/Unit/UnusedVariablesTest.php

@@ -0,0 +1,72 @@
+<?php
+
+declare(strict_types = 1);
+namespace Rebing\GraphQL\Tests\Unit;
+
+use Rebing\GraphQL\Tests\TestCase;
+
+class UnusedVariablesTest extends TestCase
+{
+    public function testFeatureNotEnabledUnusedVariableIsIgnored(): void
+    {
+        config([
+            'graphql.detect_unused_variables' => false,
+        ]);
+
+        $response = $this->call('GET', '/graphql', [
+            'query' => $this->queries['examplesWithVariables'],
+            'variables' => [
+                'index' => 1,
+                'unused_variable' => 'value',
+                'another_unused_variable' => 'value',
+            ],
+        ]);
+
+        self::assertEquals(200, $response->getStatusCode());
+
+        $content = $response->getData(true);
+
+        $expected = [
+            'data' => [
+                'examples' => [
+                    [
+                        'test' => 'Example 2',
+                    ],
+                ],
+            ],
+        ];
+        self::assertEquals($expected, $content);
+    }
+
+    public function testFeatureEnabledUnusedVariableThrowsError(): void
+    {
+        config([
+            'graphql.detect_unused_variables' => true,
+        ]);
+
+        $response = $this->call('GET', '/graphql', [
+            'query' => $this->queries['examplesWithVariables'],
+            'variables' => [
+                'index' => 1,
+                'unused_variable' => 'value',
+                'another_unused_variable' => 'value',
+            ],
+        ]);
+
+        self::assertEquals(200, $response->getStatusCode());
+
+        $content = $response->getData(true);
+
+        $expected = [
+            'errors' => [
+                [
+                    'message' => 'The following variables were provided but not consumed: unused_variable, another_unused_variable',
+                    'extensions' => [
+                        'category' => 'graphql',
+                    ],
+                ],
+            ],
+        ];
+        self::assertEquals($expected, $content);
+    }
+}