Merge pull request #5072 from adampetro/support-variable-definition-d…

…irectives Allow variable definition directives in validation
rmosolgo · Aug 23, 2024 · 8ec90a4 · 8ec90a4
2 parents a794fc8 + 493416a
commit 8ec90a4
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 17 deletions.
diff --git a/lib/graphql/static_validation/rules/directives_are_in_valid_locations.rb b/lib/graphql/static_validation/rules/directives_are_in_valid_locations.rb
@@ -19,13 +19,15 @@ def on_directive(node, parent)
         GraphQL::Schema::Directive::FRAGMENT_DEFINITION => "fragment definitions",
         GraphQL::Schema::Directive::FRAGMENT_SPREAD =>     "fragment spreads",
         GraphQL::Schema::Directive::INLINE_FRAGMENT =>     "inline fragments",
+        GraphQL::Schema::Directive::VARIABLE_DEFINITION => "variable definitions",
       }
 
       SIMPLE_LOCATIONS = {
         Nodes::Field =>               GraphQL::Schema::Directive::FIELD,
         Nodes::InlineFragment =>      GraphQL::Schema::Directive::INLINE_FRAGMENT,
         Nodes::FragmentSpread =>      GraphQL::Schema::Directive::FRAGMENT_SPREAD,
         Nodes::FragmentDefinition =>  GraphQL::Schema::Directive::FRAGMENT_DEFINITION,
+        Nodes::VariableDefinition =>  GraphQL::Schema::Directive::VARIABLE_DEFINITION,
       }
 
       SIMPLE_LOCATION_NODES = SIMPLE_LOCATIONS.keys

diff --git a/spec/graphql/introspection/directive_type_spec.rb b/spec/graphql/introspection/directive_type_spec.rb
@@ -49,6 +49,15 @@
             "onFragment" => false,
             "onOperation" => false,
           },
+          {
+            "name"=>"directiveForVariableDefinition",
+            "args"=>[],
+            "locations"=>["VARIABLE_DEFINITION"],
+            "isRepeatable"=>false,
+            "onField"=>false,
+            "onFragment"=>false,
+            "onOperation"=>false,
+          },
           {
             "name"=>"doStuff",
             "args"=>[],

diff --git a/spec/graphql/static_validation/rules/directives_are_in_valid_locations_spec.rb b/spec/graphql/static_validation/rules/directives_are_in_valid_locations_spec.rb
@@ -3,40 +3,56 @@
 
 describe GraphQL::StaticValidation::DirectivesAreInValidLocations do
   include StaticValidationHelpers
-  let(:query_string) {"
-    query getCheese @skip(if: true) {
-      okCheese: cheese(id: 1) {
-        id @skip(if: true),
-        source
-        ... on Cheese @skip(if: true) {
-          flavor
-          ... whatever
+
+  describe "invalid directive locations" do
+    let(:query_string) {"
+      query getCheese @skip(if: true) {
+        okCheese: cheese(id: 1) {
+          id @skip(if: true),
+          source
+          ... on Cheese @skip(if: true) {
+            flavor
+            ... whatever
+          }
         }
       }
-    }
-
-    fragment whatever on Cheese @skip(if: true) {
-      id
-    }
-  "}
+  
+      fragment whatever on Cheese @skip(if: true) {
+        id
+      }
+    "}
 
-  describe "invalid directive locations" do
     it "makes errors for them" do
       expected = [
         {
           "message"=> "'@skip' can't be applied to queries (allowed: fields, fragment spreads, inline fragments)",
-          "locations"=>[{"line"=>2, "column"=>21}],
+          "locations"=>[{"line"=>2, "column"=>23}],
           "path"=>["query getCheese"],
           "extensions"=>{"code"=>"directiveCannotBeApplied", "targetName"=>"queries", "name"=>"skip"}
         },
         {
           "message"=>"'@skip' can't be applied to fragment definitions (allowed: fields, fragment spreads, inline fragments)",
-          "locations"=>[{"line"=>13, "column"=>33}],
+          "locations"=>[{"line"=>13, "column"=>35}],
            "path"=>["fragment whatever"],
            "extensions"=>{"code"=>"directiveCannotBeApplied", "targetName"=>"fragment definitions", "name"=>"skip"}
         },
       ]
       assert_equal(expected, errors)
     end
   end
+
+  describe "valid directive locations" do
+    let(:query_string) {"
+      query getCheese($id: Int! @directiveForVariableDefinition) {
+        cheese(id: $id) {
+          id
+        }
+      }
+    "}
+
+    it "does not make errors for them" do
+      expected = []
+      assert_equal(expected, errors)
+    end
+  end
 end
diff --git a/spec/support/dummy/schema.rb b/spec/support/dummy/schema.rb
@@ -33,6 +33,9 @@ class BaseInputObject < GraphQL::Schema::InputObject
   class BaseScalar < GraphQL::Schema::Scalar
   end
 
+  class BaseDirective < GraphQL::Schema::Directive
+  end
+
   module LocalProduct
     include BaseInterface
     description "Something that comes from somewhere"
@@ -530,6 +533,10 @@ def replace_values(input:)
     end
   end
 
+  class DirectiveForVariableDefinition < BaseDirective
+    locations(VARIABLE_DEFINITION)
+  end
+
   class Subscription < BaseObject
     field :test, String
     def test; "Test"; end
@@ -542,6 +549,7 @@ class Schema < GraphQL::Schema
     max_depth 5
     orphan_types Honey
     trace_with GraphQL::Tracing::CallLegacyTracers
+    directives(DirectiveForVariableDefinition)
 
     rescue_from(NoSuchDairyError) { |err| raise GraphQL::ExecutionError, err.message  }