From 253a8c5d759f79930a57229cd063ceb30bfa1d77 Mon Sep 17 00:00:00 2001 From: Joel Turkel Date: Tue, 11 Aug 2020 13:49:34 -0400 Subject: [PATCH 1/2] Argument deprecation --- guides/fields/arguments.md | 10 ++ lib/graphql/argument.rb | 6 +- lib/graphql/introspection.rb | 96 +++++++++++ lib/graphql/introspection/field_type.rb | 10 +- lib/graphql/introspection/input_value_type.rb | 6 + .../introspection/introspection_query.rb | 98 +----------- lib/graphql/introspection/type_type.rb | 10 +- lib/graphql/schema.rb | 4 +- lib/graphql/schema/argument.rb | 18 ++- lib/graphql/schema/build_from_definition.rb | 1 + lib/graphql/schema/directive/deprecated.rb | 2 +- lib/graphql/schema/loader.rb | 1 + .../introspection/directive_type_spec.rb | 2 +- spec/graphql/introspection/type_type_spec.rb | 151 +++++++++++++----- spec/graphql/rake_task_spec.rb | 2 +- spec/graphql/schema/argument_spec.rb | 20 ++- .../schema/build_from_definition_spec.rb | 6 + spec/graphql/schema/loader_spec.rb | 5 +- spec/graphql/schema/printer_spec.rb | 19 ++- .../rails/graphql/input_object_type_spec.rb | 4 +- spec/support/dummy/schema.rb | 2 + 21 files changed, 317 insertions(+), 156 deletions(-) diff --git a/guides/fields/arguments.md b/guides/fields/arguments.md index 486a1eaa27..9958ccf7ca 100644 --- a/guides/fields/arguments.md +++ b/guides/fields/arguments.md @@ -62,6 +62,16 @@ def search_posts(category:) end ``` +**Experimental:** __Deprecated__ arguments can be marked by adding a `deprecation_reason:` keyword argument: + +```ruby +field :search_posts, [PostType], null: false do + argument :name, String, required: false, deprecation_reason: "Use `query` instead." + argument :query, String, required: false +end +``` +Note argument deprecation is a stage 2 GraphQL [proposal](https://github.com/graphql/graphql-spec/pull/525) so not all clients will leverage this information. + Use `as: :alternate_name` to use a different key from within your resolvers while exposing another key to clients. diff --git a/lib/graphql/argument.rb b/lib/graphql/argument.rb index 148d87cec4..526b5b07ed 100644 --- a/lib/graphql/argument.rb +++ b/lib/graphql/argument.rb @@ -3,14 +3,14 @@ module GraphQL # @api deprecated class Argument include GraphQL::Define::InstanceDefinable - accepts_definitions :name, :type, :description, :default_value, :as, :prepare, :method_access + accepts_definitions :name, :type, :description, :default_value, :as, :prepare, :method_access, :deprecation_reason attr_reader :default_value - attr_accessor :description, :name, :as + attr_accessor :description, :name, :as, :deprecation_reason attr_accessor :ast_node attr_accessor :method_access alias :graphql_name :name - ensure_defined(:name, :description, :default_value, :type=, :type, :as, :expose_as, :prepare, :method_access) + ensure_defined(:name, :description, :default_value, :type=, :type, :as, :expose_as, :prepare, :method_access, :deprecation_reason) # @api private module DefaultPrepare diff --git a/lib/graphql/introspection.rb b/lib/graphql/introspection.rb index 018560db11..0f3ea73f60 100644 --- a/lib/graphql/introspection.rb +++ b/lib/graphql/introspection.rb @@ -1,6 +1,102 @@ # frozen_string_literal: true module GraphQL module Introspection + def self.query(include_deprecated_args: false) + # The introspection query to end all introspection queries, copied from + # https://github.com/graphql/graphql-js/blob/master/src/utilities/introspectionQuery.js + <<-QUERY +query IntrospectionQuery { + __schema { + queryType { name } + mutationType { name } + subscriptionType { name } + types { + ...FullType + } + directives { + name + description + locations + args { + ...InputValue + } + } + } +} +fragment FullType on __Type { + kind + name + description + fields(includeDeprecated: true) { + name + description + args#{include_deprecated_args ? '(includeDeprecated: true)' : ''} { + ...InputValue + } + type { + ...TypeRef + } + isDeprecated + deprecationReason + } + inputFields#{include_deprecated_args ? '(includeDeprecated: true)' : ''} { + ...InputValue + } + interfaces { + ...TypeRef + } + enumValues(includeDeprecated: true) { + name + description + isDeprecated + deprecationReason + } + possibleTypes { + ...TypeRef + } +} +fragment InputValue on __InputValue { + name + description + type { ...TypeRef } + defaultValue + #{include_deprecated_args ? 'isDeprecated' : ''} + #{include_deprecated_args ? 'deprecationReason' : ''} +} +fragment TypeRef on __Type { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + ofType { + kind + name + } + } + } + } + } + } + } +} + QUERY + end end end diff --git a/lib/graphql/introspection/field_type.rb b/lib/graphql/introspection/field_type.rb index 15a2c568ea..a9b15ab4cb 100644 --- a/lib/graphql/introspection/field_type.rb +++ b/lib/graphql/introspection/field_type.rb @@ -7,7 +7,9 @@ class FieldType < Introspection::BaseObject "a name, potentially a list of arguments, and a return type." field :name, String, null: false field :description, String, null: true - field :args, [GraphQL::Schema::LateBoundType.new("__InputValue")], null: false + field :args, [GraphQL::Schema::LateBoundType.new("__InputValue")], null: false do + argument :include_deprecated, Boolean, required: false, default_value: false + end field :type, GraphQL::Schema::LateBoundType.new("__Type"), null: false field :is_deprecated, Boolean, null: false field :deprecation_reason, String, null: true @@ -16,8 +18,10 @@ def is_deprecated !!@object.deprecation_reason end - def args - @context.warden.arguments(@object) + def args(include_deprecated:) + args = @context.warden.arguments(@object) + args = args.reject(&:deprecation_reason) unless include_deprecated + args end end end diff --git a/lib/graphql/introspection/input_value_type.rb b/lib/graphql/introspection/input_value_type.rb index d4385f9033..33e544ffe8 100644 --- a/lib/graphql/introspection/input_value_type.rb +++ b/lib/graphql/introspection/input_value_type.rb @@ -10,6 +10,12 @@ class InputValueType < Introspection::BaseObject field :description, String, null: true field :type, GraphQL::Schema::LateBoundType.new("__Type"), null: false field :default_value, String, "A GraphQL-formatted string representing the default value for this input value.", null: true + field :is_deprecated, Boolean, null: false + field :deprecation_reason, String, null: true + + def is_deprecated + !!@object.deprecation_reason + end def default_value if @object.default_value? diff --git a/lib/graphql/introspection/introspection_query.rb b/lib/graphql/introspection/introspection_query.rb index c1a5783ca0..acb30beeeb 100644 --- a/lib/graphql/introspection/introspection_query.rb +++ b/lib/graphql/introspection/introspection_query.rb @@ -1,93 +1,7 @@ # frozen_string_literal: true -# The introspection query to end all introspection queries, copied from -# https://github.com/graphql/graphql-js/blob/master/src/utilities/introspectionQuery.js -GraphQL::Introspection::INTROSPECTION_QUERY = " -query IntrospectionQuery { - __schema { - queryType { name } - mutationType { name } - subscriptionType { name } - types { - ...FullType - } - directives { - name - description - locations - args { - ...InputValue - } - } - } -} -fragment FullType on __Type { - kind - name - description - fields(includeDeprecated: true) { - name - description - args { - ...InputValue - } - type { - ...TypeRef - } - isDeprecated - deprecationReason - } - inputFields { - ...InputValue - } - interfaces { - ...TypeRef - } - enumValues(includeDeprecated: true) { - name - description - isDeprecated - deprecationReason - } - possibleTypes { - ...TypeRef - } -} -fragment InputValue on __InputValue { - name - description - type { ...TypeRef } - defaultValue -} -fragment TypeRef on __Type { - kind - name - ofType { - kind - name - ofType { - kind - name - ofType { - kind - name - ofType { - kind - name - ofType { - kind - name - ofType { - kind - name - ofType { - kind - name - } - } - } - } - } - } - } -} -" + +# This query is used by graphql-client so don't add the includeDeprecated +# argument for inputFields since the server may not support it. Two stage +# introspection queries will be required to handle this in clients. +GraphQL::Introspection::INTROSPECTION_QUERY = GraphQL::Introspection.query + diff --git a/lib/graphql/introspection/type_type.rb b/lib/graphql/introspection/type_type.rb index 8c6cb8d319..ba834555e5 100644 --- a/lib/graphql/introspection/type_type.rb +++ b/lib/graphql/introspection/type_type.rb @@ -22,7 +22,9 @@ class TypeType < Introspection::BaseObject field :enum_values, [GraphQL::Schema::LateBoundType.new("__EnumValue")], null: true do argument :include_deprecated, Boolean, required: false, default_value: false end - field :input_fields, [GraphQL::Schema::LateBoundType.new("__InputValue")], null: true + field :input_fields, [GraphQL::Schema::LateBoundType.new("__InputValue")], null: true do + argument :include_deprecated, Boolean, required: false, default_value: false + end field :of_type, GraphQL::Schema::LateBoundType.new("__Type"), null: true def name @@ -55,9 +57,11 @@ def interfaces end end - def input_fields + def input_fields(include_deprecated:) if @object.kind.input_object? - @context.warden.arguments(@object) + args = @context.warden.arguments(@object) + args = args.reject(&:deprecation_reason) unless include_deprecated + args else nil end diff --git a/lib/graphql/schema.rb b/lib/graphql/schema.rb index df3e83581d..fca7560140 100644 --- a/lib/graphql/schema.rb +++ b/lib/graphql/schema.rb @@ -832,7 +832,7 @@ def to_document(only: nil, except: nil, context: {}) # @param except [<#call(member, ctx)>] # @return [Hash] GraphQL result def as_json(only: nil, except: nil, context: {}) - execute(Introspection::INTROSPECTION_QUERY, only: only, except: except, context: context).to_h + execute(Introspection.query(include_deprecated_args: true), only: only, except: except, context: context).to_h end # Returns the JSON response of {Introspection::INTROSPECTION_QUERY}. @@ -880,7 +880,7 @@ def to_json(**args) # @param except [<#call(member, ctx)>] # @return [Hash] GraphQL result def as_json(only: nil, except: nil, context: {}) - execute(Introspection::INTROSPECTION_QUERY, only: only, except: except, context: context).to_h + execute(Introspection.query(include_deprecated_args: true), only: only, except: except, context: context).to_h end # Return the GraphQL IDL for the schema diff --git a/lib/graphql/schema/argument.rb b/lib/graphql/schema/argument.rb index 59fc6b9471..14a9c2ab4a 100644 --- a/lib/graphql/schema/argument.rb +++ b/lib/graphql/schema/argument.rb @@ -45,7 +45,8 @@ def from_resolver? # @param camelize [Boolean] if true, the name will be camelized when building the schema # @param from_resolver [Boolean] if true, a Resolver class defined this argument # @param method_access [Boolean] If false, don't build method access on legacy {Query::Arguments} instances. - def initialize(arg_name = nil, type_expr = nil, desc = nil, required:, type: nil, name: nil, loads: nil, description: nil, ast_node: nil, default_value: NO_DEFAULT, as: nil, from_resolver: false, camelize: true, prepare: nil, method_access: true, owner:, &definition_block) + # @param deprecation_reason [String] + def initialize(arg_name = nil, type_expr = nil, desc = nil, required:, type: nil, name: nil, loads: nil, description: nil, ast_node: nil, default_value: NO_DEFAULT, as: nil, from_resolver: false, camelize: true, prepare: nil, method_access: true, owner:, deprecation_reason: nil, &definition_block) arg_name ||= name @name = -(camelize ? Member::BuildType.camelize(arg_name.to_s) : arg_name.to_s) @type_expr = type_expr || type @@ -60,6 +61,7 @@ def initialize(arg_name = nil, type_expr = nil, desc = nil, required:, type: nil @ast_node = ast_node @from_resolver = from_resolver @method_access = method_access + @deprecation_reason = deprecation_reason if definition_block if definition_block.arity == 1 @@ -89,6 +91,17 @@ def description(text = nil) end end + attr_writer :deprecation_reason + + # @return [String] Deprecation reason for this argument + def deprecation_reason(text = nil) + if text + @deprecation_reason = text + else + @deprecation_reason + end + end + def visible?(context) true end @@ -143,6 +156,9 @@ def to_graphql if NO_DEFAULT != @default_value argument.default_value = @default_value end + if @deprecation_reason + argument.deprecation_reason = @deprecation_reason + end argument end diff --git a/lib/graphql/schema/build_from_definition.rb b/lib/graphql/schema/build_from_definition.rb index 757a573a10..a8c0b7bbc1 100644 --- a/lib/graphql/schema/build_from_definition.rb +++ b/lib/graphql/schema/build_from_definition.rb @@ -261,6 +261,7 @@ def build_arguments(type_class, arguments, type_resolver) type: type_resolver.call(argument_defn.type), required: false, description: argument_defn.description, + deprecation_reason: builder.build_deprecation_reason(argument_defn.directives), ast_node: argument_defn, camelize: false, method_access: false, diff --git a/lib/graphql/schema/directive/deprecated.rb b/lib/graphql/schema/directive/deprecated.rb index bcef5ea16b..6cc8d12a16 100644 --- a/lib/graphql/schema/directive/deprecated.rb +++ b/lib/graphql/schema/directive/deprecated.rb @@ -4,7 +4,7 @@ class Schema class Directive < GraphQL::Schema::Member class Deprecated < GraphQL::Schema::Directive description "Marks an element of a GraphQL schema as no longer supported." - locations(GraphQL::Schema::Directive::FIELD_DEFINITION, GraphQL::Schema::Directive::ENUM_VALUE) + locations(GraphQL::Schema::Directive::FIELD_DEFINITION, GraphQL::Schema::Directive::ENUM_VALUE, GraphQL::Schema::Directive::ARGUMENT_DEFINITION, GraphQL::Schema::Directive::INPUT_FIELD_DEFINITION) reason_description = "Explains why this element was deprecated, usually also including a "\ "suggestion for how to access supported similar data. Formatted "\ diff --git a/lib/graphql/schema/loader.rb b/lib/graphql/schema/loader.rb index 2dde7d8ce9..9031b2b76a 100644 --- a/lib/graphql/schema/loader.rb +++ b/lib/graphql/schema/loader.rb @@ -189,6 +189,7 @@ def build_arguments(arg_owner, args, type_resolver) kwargs = { type: type_resolver.call(arg["type"]), description: arg["description"], + deprecation_reason: arg["deprecationReason"], required: false, method_access: false, camelize: false, diff --git a/spec/graphql/introspection/directive_type_spec.rb b/spec/graphql/introspection/directive_type_spec.rb index ab96e3b971..3bfec39854 100644 --- a/spec/graphql/introspection/directive_type_spec.rb +++ b/spec/graphql/introspection/directive_type_spec.rb @@ -53,7 +53,7 @@ "args" => [ {"name"=>"reason", "type"=>{"kind"=>"SCALAR", "name"=>"String", "ofType"=>nil}} ], - "locations"=>["FIELD_DEFINITION", "ENUM_VALUE"], + "locations"=>["FIELD_DEFINITION", "ENUM_VALUE", "ARGUMENT_DEFINITION", "INPUT_FIELD_DEFINITION"], "onField" => false, "onFragment" => false, "onOperation" => false, diff --git a/spec/graphql/introspection/type_type_spec.rb b/spec/graphql/introspection/type_type_spec.rb index dc04da00af..3aa45d1f62 100644 --- a/spec/graphql/introspection/type_type_spec.rb +++ b/spec/graphql/introspection/type_type_spec.rb @@ -105,51 +105,126 @@ assert_equal(expected, result) end - describe "input objects" do - let(:query_string) {%| - query introspectionQuery { - __type(name: "DairyProductInput") { name, description, kind, inputFields { name, type { kind, name }, defaultValue } } - } - |} + it "hides deprecated field arguments by default" do + result = Dummy::Schema.execute <<-GRAPHQL + { + __type(name: "Query") { + fields { + name + args { + name + } + } + } + } + GRAPHQL - it "exposes metadata about input objects" do - expected = { "data" => { - "__type" => { - "name"=>"DairyProductInput", - "description"=>"Properties for finding a dairy product", - "kind"=>"INPUT_OBJECT", - "inputFields"=>[ - {"name"=>"source", "type"=>{"kind"=>"NON_NULL","name"=>nil, }, "defaultValue"=>nil}, - {"name"=>"originDairy", "type"=>{"kind"=>"SCALAR","name"=>"String"}, "defaultValue"=>"\"Sugar Hollow Dairy\""}, - {"name"=>"fatContent", "type"=>{"kind"=>"SCALAR","name" => "Float"}, "defaultValue"=>"0.3"}, - {"name"=>"organic", "type"=>{"kind"=>"SCALAR","name" => "Boolean"}, "defaultValue"=>"false"}, - {"name"=>"order_by", "type"=>{"kind"=>"INPUT_OBJECT", "name"=>"ResourceOrderType"}, "defaultValue"=>"{direction: \"ASC\"}"}, - ] + from_source_field = result['data']['__type']['fields'].find { |f| f['name'] == 'fromSource' } + expected = [ + {"name" => "source"} + ] + assert_equal(expected, from_source_field['args']) + end + + it "can expose deprecated field arguments" do + result = Dummy::Schema.execute <<-GRAPHQL + { + __type(name: "Query") { + fields { + name + args(includeDeprecated: true) { + name + isDeprecated + deprecationReason } - }} - assert_equal(expected, result) - end + } + } + } + GRAPHQL + + from_source_field = result['data']['__type']['fields'].find { |f| f['name'] == 'fromSource' } + expected = [ + {"name" => "source", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "oldSource", "isDeprecated" => true, "deprecationReason" => "No longer supported"} + ] + assert_equal(expected, from_source_field['args']) + end + end + + describe "input objects" do + let(:query_string) {%| + query introspectionQuery { + __type(name: "DairyProductInput") { name, description, kind, inputFields { name, type { kind, name }, defaultValue } } + } + |} + + it "exposes metadata about input objects" do + expected = { "data" => { + "__type" => { + "name"=>"DairyProductInput", + "description"=>"Properties for finding a dairy product", + "kind"=>"INPUT_OBJECT", + "inputFields"=>[ + {"name"=>"source", "type"=>{"kind"=>"NON_NULL","name"=>nil, }, "defaultValue"=>nil}, + {"name"=>"originDairy", "type"=>{"kind"=>"SCALAR","name"=>"String"}, "defaultValue"=>"\"Sugar Hollow Dairy\""}, + {"name"=>"fatContent", "type"=>{"kind"=>"SCALAR","name" => "Float"}, "defaultValue"=>"0.3"}, + {"name"=>"organic", "type"=>{"kind"=>"SCALAR","name" => "Boolean"}, "defaultValue"=>"false"}, + {"name"=>"order_by", "type"=>{"kind"=>"INPUT_OBJECT", "name"=>"ResourceOrderType"}, "defaultValue"=>"{direction: \"ASC\"}"}, + ] + } + }} + assert_equal(expected, result) + end + + it "can expose deprecated input fields" do + result = Dummy::Schema.execute <<-GRAPHQL + { + __type(name: "DairyProductInput") { + inputFields(includeDeprecated: true) { + name + isDeprecated + deprecationReason + } + } + } + GRAPHQL + + expected = { + "data" => { + "__type" => { + "inputFields" => [ + {"name" => "source", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "originDairy", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "fatContent", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "organic", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "order_by", "isDeprecated" => false, "deprecationReason" => nil}, + {"name" => "oldSource", "isDeprecated" => true, "deprecationReason" => "No longer supported"}, + ] + } + } + } + assert_equal(expected, result) + end - it "includes Relay fields" do - res = StarWars::Schema.execute <<-GRAPHQL - { - __schema { - types { + it "includes Relay fields" do + res = StarWars::Schema.execute <<-GRAPHQL + { + __schema { + types { + name + fields { name - fields { - name - args { name } - } + args { name } } } } - GRAPHQL - type_result = res["data"]["__schema"]["types"].find { |t| t["name"] == "Faction" } - field_result = type_result["fields"].find { |f| f["name"] == "bases" } - all_arg_names = ["after", "before", "first", "last", "nameIncludes", "complexOrder"] - returned_arg_names = field_result["args"].map { |a| a["name"] } - assert_equal all_arg_names, returned_arg_names - end + } + GRAPHQL + type_result = res["data"]["__schema"]["types"].find { |t| t["name"] == "Faction" } + field_result = type_result["fields"].find { |f| f["name"] == "bases" } + all_arg_names = ["after", "before", "first", "last", "nameIncludes", "complexOrder"] + returned_arg_names = field_result["args"].map { |a| a["name"] } + assert_equal all_arg_names, returned_arg_names end end end diff --git a/spec/graphql/rake_task_spec.rb b/spec/graphql/rake_task_spec.rb index 1962580562..bcd48b493c 100644 --- a/spec/graphql/rake_task_spec.rb +++ b/spec/graphql/rake_task_spec.rb @@ -33,7 +33,7 @@ Rake::Task["graphql:schema:dump"].invoke end dumped_json = File.read("./schema.json") - expected_json = JSON.pretty_generate(RakeTaskSchema.execute(GraphQL::Introspection::INTROSPECTION_QUERY)) + expected_json = JSON.pretty_generate(RakeTaskSchema.execute(GraphQL::Introspection.query(include_deprecated_args: true))) # Test that that JSON is logically equivalent, not serialized the same assert_equal(JSON.parse(expected_json), JSON.parse(dumped_json)) diff --git a/spec/graphql/schema/argument_spec.rb b/spec/graphql/schema/argument_spec.rb index 0f4e1a0e22..bebba6a30b 100644 --- a/spec/graphql/schema/argument_spec.rb +++ b/spec/graphql/schema/argument_spec.rb @@ -6,6 +6,7 @@ module SchemaArgumentTest class Query < GraphQL::Schema::Object field :field, String, null: true do argument :arg, String, description: "test", required: false + argument :deprecated_arg, String, deprecation_reason: "don't use me!", required: false argument :arg_with_block, String, required: false do description "test" @@ -62,7 +63,7 @@ def self.object_from_id(id, ctx) describe "#keys" do it "is not overwritten by the 'keys' argument" do - expected_keys = ["aliasedArg", "arg", "argWithBlock", "explodingPreparedArg", "instrumentId", "instrumentIds", "keys", "preparedArg", "preparedByCallableArg", "preparedByProcArg", "requiredWithDefaultArg"] + expected_keys = ["aliasedArg", "arg", "argWithBlock", "deprecatedArg", "explodingPreparedArg", "instrumentId", "instrumentIds", "keys", "preparedArg", "preparedByCallableArg", "preparedByProcArg", "requiredWithDefaultArg"] assert_equal expected_keys, SchemaArgumentTest::Query.fields["field"].arguments.keys.sort end end @@ -266,4 +267,21 @@ def self.object_from_id(id, ctx) assert_nil res5["data"].fetch("nullableEnsemble") end end + + describe "deprecation_reason:" do + let(:arg) { SchemaArgumentTest::Query.fields["field"].arguments["arg"] } + it "sets deprecation reason" do + arg.deprecation_reason "new deprecation reason" + assert_equal "new deprecation reason", arg.deprecation_reason + end + + it "returns the deprecation reason" do + assert_equal "don't use me!", SchemaArgumentTest::Query.fields["field"].arguments["deprecatedArg"].deprecation_reason + end + + it "has an assignment method" do + arg.deprecation_reason = "another new deprecation reason" + assert_equal "another new deprecation reason", arg.deprecation_reason + end + end end diff --git a/spec/graphql/schema/build_from_definition_spec.rb b/spec/graphql/schema/build_from_definition_spec.rb index 6d777a2a1b..ed351daf9b 100644 --- a/spec/graphql/schema/build_from_definition_spec.rb +++ b/spec/graphql/schema/build_from_definition_spec.rb @@ -743,10 +743,16 @@ def assert_schema_and_compare_output(definition) VALUE } +input MyInput { + int: Int @deprecated(reason: "This is not the argument you're looking for") + string: String +} + type Query { enum: MyEnum field1: String @deprecated field2: Int @deprecated(reason: "Because I said so") + field3(deprecatedArg: MyInput @deprecated(reason: "Use something else")): String } SCHEMA diff --git a/spec/graphql/schema/loader_spec.rb b/spec/graphql/schema/loader_spec.rb index 7ebf56217d..d199c0a63c 100644 --- a/spec/graphql/schema/loader_spec.rb +++ b/spec/graphql/schema/loader_spec.rb @@ -47,6 +47,7 @@ def self.coerce_result(value, _ctx) argument :bool, Boolean, required: false argument :enum, choice_type, required: false argument :sub, [sub_input_type], required: false + argument :deprecated_arg, String, required: false, deprecation_reason: "Don't use Varied.deprecatedArg" end variant_input_type_with_nulls = Class.new(GraphQL::Schema::InputObject) do @@ -119,6 +120,7 @@ def self.coerce_result(value, _ctx) argument :variedArray, [variant_input_type], required: false, default_value: [{ id: "123", int: 234, float: 2.3, enum: :foo, sub: [{ string: "str" }] }] argument :enum, choice_type, required: false, default_value: :foo argument :array, [String], required: false, default_value: ["foo", "bar"] + argument :deprecated_arg, String, required: false, deprecation_reason: "Don't use Varied.deprecatedArg" end field :content, content_type, null: true @@ -142,7 +144,7 @@ def self.coerce_result(value, _ctx) } let(:schema_json) { - schema.execute(GraphQL::Introspection::INTROSPECTION_QUERY) + schema.execute(GraphQL::Introspection.query(include_deprecated_args: true)) } describe "load" do @@ -164,6 +166,7 @@ def assert_deep_equal(expected_type, actual_type) elsif actual_type.is_a?(GraphQL::Schema::Argument) assert_equal expected_type.graphql_name, actual_type.graphql_name assert_equal expected_type.description, actual_type.description + assert_equal expected_type.deprecation_reason, actual_type.deprecation_reason assert_deep_equal expected_type.type, actual_type.type elsif actual_type.is_a?(GraphQL::Schema::NonNull) || actual_type.is_a?(GraphQL::Schema::List) assert_equal expected_type.class, actual_type.class diff --git a/spec/graphql/schema/printer_spec.rb b/spec/graphql/schema/printer_spec.rb index c1088d7355..2b4dbd1a7c 100644 --- a/spec/graphql/schema/printer_spec.rb +++ b/spec/graphql/schema/printer_spec.rb @@ -26,7 +26,7 @@ name "Sub" description "Test" input_field :string, types.String, 'Something' - input_field :int, types.Int, 'Something' + input_field :int, types.Int, 'Something', deprecation_reason: 'Do something else' end variant_input_type = GraphQL::InputObjectType.define do @@ -91,6 +91,7 @@ argument :id, !types.ID, 'Post ID' argument :varied, variant_input_type, default_value: { id: "123", int: 234, float: 2.3, enum: :foo, sub: [{ string: "str" }] } argument :variedWithNulls, variant_input_type, default_value: { id: nil, int: nil, float: nil, enum: nil, sub: nil } + argument :deprecatedArg, types.String, deprecation_reason: 'Use something else' resolve ->(obj, args, ctx) { Post.find(args["id"]) } end end @@ -150,7 +151,7 @@ [Markdown](https://daringfireball.net/projects/markdown/). """ reason: String = "No longer supported" -) on FIELD_DEFINITION | ENUM_VALUE +) on FIELD_DEFINITION | ENUM_VALUE | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION """ Directs the executor to include this field or fragment only when the `if` argument is true. @@ -303,7 +304,7 @@ a name, potentially a list of arguments, and a return type. """ type __Field { - args: [__InputValue!]! + args(includeDeprecated: Boolean = false): [__InputValue!]! deprecationReason: String description: String isDeprecated: Boolean! @@ -321,7 +322,9 @@ A GraphQL-formatted string representing the default value for this input value. """ defaultValue: String + deprecationReason: String description: String + isDeprecated: Boolean! name: String! type: __Type! } @@ -372,7 +375,7 @@ description: String enumValues(includeDeprecated: Boolean = false): [__EnumValue!] fields(includeDeprecated: Boolean = false): [__Field!] - inputFields: [__InputValue!] + inputFields(includeDeprecated: Boolean = false): [__InputValue!] interfaces: [__Type!] kind: __TypeKind! name: String @@ -559,6 +562,8 @@ """ type Query { post( + deprecatedArg: String @deprecated(reason: "Use something else") + """ Post ID """ @@ -575,7 +580,7 @@ """ Something """ - int: Int + int: Int @deprecated(reason: "Do something else") """ Something @@ -636,7 +641,7 @@ def foobar The query root of this schema """ type Query { - post: Post + post(deprecatedArg: String @deprecated(reason: "Use something else")): Post } SCHEMA @@ -776,7 +781,7 @@ def foobar """ Something """ - int: Int + int: Int @deprecated(reason: "Do something else") """ Something diff --git a/spec/integration/rails/graphql/input_object_type_spec.rb b/spec/integration/rails/graphql/input_object_type_spec.rb index 8f3d17794f..7cd7375258 100644 --- a/spec/integration/rails/graphql/input_object_type_spec.rb +++ b/spec/integration/rails/graphql/input_object_type_spec.rb @@ -342,8 +342,8 @@ it "shallow-copies internal state" do input_object_2 = input_object.dup input_object_2.arguments["nonsense"] = GraphQL::Argument.define(name: "int", type: GraphQL::INT_TYPE) - assert_equal 5, input_object.arguments.size - assert_equal 6, input_object_2.arguments.size + assert_equal 6, input_object.arguments.size + assert_equal 7, input_object_2.arguments.size end end end diff --git a/spec/support/dummy/schema.rb b/spec/support/dummy/schema.rb index 7882ede57a..c454f4a3e5 100644 --- a/spec/support/dummy/schema.rb +++ b/spec/support/dummy/schema.rb @@ -264,6 +264,7 @@ class DairyProductInput < BaseInputObject argument :fat_content, Float, required: false, description: "How much fat it has", default_value: 0.3 argument :organic, Boolean, required: false, default_value: false argument :order_by, ResourceOrder, required: false, default_value: { direction: "ASC" }, camelize: false + argument :old_source, String, required: false, deprecation_reason: "No longer supported" end class DeepNonNull < BaseObject @@ -344,6 +345,7 @@ def root field :dairy, resolver: GetSingleton.build(type: Dairy, data: DAIRY) field :from_source, [Cheese, null: true], null: true, description: "Cheese from source" do argument :source, DairyAnimal, required: false, default_value: 1 + argument :old_source, String, required: false, deprecation_reason: "No longer supported" end def from_source(source:) CHEESES.values.select { |c| c.source == source } From 4e2c13aa5e9a533e4bc4e189a3545d508b441e3d Mon Sep 17 00:00:00 2001 From: Joel Turkel Date: Fri, 4 Sep 2020 10:41:31 -0400 Subject: [PATCH 2/2] Validate that deprecated argument are optional --- lib/graphql/schema/validation.rb | 8 ++++++ spec/graphql/schema/validation_spec.rb | 36 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/graphql/schema/validation.rb b/lib/graphql/schema/validation.rb index 3a15023507..011cb0f765 100644 --- a/lib/graphql/schema/validation.rb +++ b/lib/graphql/schema/validation.rb @@ -133,6 +133,12 @@ def self.assert_named_items_are_valid(item_name, get_items_proc) end } + DEPRECATED_ARGUMENTS_ARE_OPTIONAL = ->(argument) { + if argument.deprecation_reason && argument.type.non_null? + "must be optional because it's deprecated" + end + } + TYPE_IS_VALID_INPUT_TYPE = ->(type) { outer_type = type.type inner_type = outer_type.respond_to?(:unwrap) ? outer_type.unwrap : nil @@ -265,8 +271,10 @@ def self.assert_named_items_are_valid(item_name, get_items_proc) Rules::NAME_IS_STRING, Rules::RESERVED_NAME, Rules::DESCRIPTION_IS_STRING_OR_NIL, + Rules.assert_property(:deprecation_reason, String, NilClass), Rules::TYPE_IS_VALID_INPUT_TYPE, Rules::DEFAULT_VALUE_IS_VALID_FOR_TYPE, + Rules::DEPRECATED_ARGUMENTS_ARE_OPTIONAL, ], GraphQL::BaseType => [ Rules::NAME_IS_STRING, diff --git a/spec/graphql/schema/validation_spec.rb b/spec/graphql/schema/validation_spec.rb index 0408c3702f..f98c5ea656 100644 --- a/spec/graphql/schema/validation_spec.rb +++ b/spec/graphql/schema/validation_spec.rb @@ -344,6 +344,30 @@ def assert_validation_warns(object, warning) end } + let(:deprecated_optional_argument) { + GraphQL::Argument.define do + name "Something" + deprecation_reason "Don't use me" + type GraphQL::INT_TYPE + end + } + + let(:deprecated_required_argument) { + GraphQL::Argument.define do + name "Something" + deprecation_reason "Don't use me" + type !GraphQL::INT_TYPE + end + } + + let(:invalid_deprecation_reason_argument) { + GraphQL::Argument.define do + name "Something" + deprecation_reason 1 + type GraphQL::INT_TYPE + end + } + it "requires the type is a Base type" do assert_error_includes untyped_argument, "must be a valid input type (Scalar or InputObject), not Symbol" end @@ -359,6 +383,18 @@ def assert_validation_warns(object, warning) it "allows null default value for nullable argument" do assert_nil GraphQL::Schema::Validation.validate(null_default_value) end + + it "allows deprecated optional arguments" do + assert_nil GraphQL::Schema::Validation.validate(deprecated_optional_argument) + end + + it "disallows deprecated required arguments" do + assert_error_includes deprecated_required_argument, "must be optional because it's deprecated" + end + + it "disallows non-string deprecation reasons" do + assert_error_includes invalid_deprecation_reason_argument, "deprecation_reason must return String or NilClass, not #{integer_class_name} (1)" + end end describe "validating instrumentation" do