diff --git a/benchmark/run.rb b/benchmark/run.rb index ad9a49d531..8052e0486f 100644 --- a/benchmark/run.rb +++ b/benchmark/run.rb @@ -1,5 +1,6 @@ # frozen_string_literal: true require "graphql" +ADD_WARDEN = false require "jazz" require "benchmark/ips" require "stackprof" diff --git a/lib/graphql/schema/has_single_input_argument.rb b/lib/graphql/schema/has_single_input_argument.rb index 26c266840f..bf98d4d930 100644 --- a/lib/graphql/schema/has_single_input_argument.rb +++ b/lib/graphql/schema/has_single_input_argument.rb @@ -136,6 +136,8 @@ def description(new_desc = nil) super || "Autogenerated input type of #{self.mutation.graphql_name}" end end + # For compatibility, in case no arguments are defined: + has_no_arguments(true) mutation(mutation_class) # these might be inherited: mutation_args.each do |arg| diff --git a/lib/graphql/schema/input_object.rb b/lib/graphql/schema/input_object.rb index 92f87293c4..a04c1877ad 100644 --- a/lib/graphql/schema/input_object.rb +++ b/lib/graphql/schema/input_object.rb @@ -10,6 +10,14 @@ class InputObject < GraphQL::Schema::Member include GraphQL::Dig + # Raised when an InputObject doesn't have any arguments defined and hasn't explicitly opted out of this requirement + class ArgumentsAreRequiredError < GraphQL::Error + def initialize(input_object_type) + message = "Input Object types must have arguments, but #{input_object_type.graphql_name} doesn't have any. Define an argument for this type, remove it from your schema, or add `has_no_arguments(true)` to its definition." + super(message) + end + end + # @return [GraphQL::Query::Context] The context for this query attr_reader :context # @return [GraphQL::Execution::Interpereter::Arguments] The underlying arguments instance @@ -56,33 +64,6 @@ def prepare end end - def self.authorized?(obj, value, ctx) - # Authorize each argument (but this doesn't apply if `prepare` is implemented): - if value.respond_to?(:key?) - ctx.types.arguments(self).each do |input_obj_arg| - if value.key?(input_obj_arg.keyword) && - !input_obj_arg.authorized?(obj, value[input_obj_arg.keyword], ctx) - return false - end - end - end - # It didn't early-return false: - true - end - - def self.one_of - if !one_of? - if all_argument_definitions.any? { |arg| arg.type.non_null? } - raise ArgumentError, "`one_of` may not be used with required arguments -- add `required: false` to argument definitions to use `one_of`" - end - directive(GraphQL::Schema::Directive::OneOf) - end - end - - def self.one_of? - false # Re-defined when `OneOf` is added - end - def unwrap_value(value) case value when Array @@ -121,6 +102,33 @@ def to_kwargs end class << self + def authorized?(obj, value, ctx) + # Authorize each argument (but this doesn't apply if `prepare` is implemented): + if value.respond_to?(:key?) + ctx.types.arguments(self).each do |input_obj_arg| + if value.key?(input_obj_arg.keyword) && + !input_obj_arg.authorized?(obj, value[input_obj_arg.keyword], ctx) + return false + end + end + end + # It didn't early-return false: + true + end + + def one_of + if !one_of? + if all_argument_definitions.any? { |arg| arg.type.non_null? } + raise ArgumentError, "`one_of` may not be used with required arguments -- add `required: false` to argument definitions to use `one_of`" + end + directive(GraphQL::Schema::Directive::OneOf) + end + end + + def one_of? + false # Re-defined when `OneOf` is added + end + def argument(*args, **kwargs, &block) argument_defn = super(*args, **kwargs, &block) if one_of? @@ -246,6 +254,25 @@ def coerce_result(value, ctx) result end + # @param new_has_no_arguments [Boolean] Call with `true` to make this InputObject type ignore the requirement to have any defined arguments. + # @return [void] + def has_no_arguments(new_has_no_arguments) + @has_no_arguments = new_has_no_arguments + nil + end + + # @return [Boolean] `true` if `has_no_arguments(true)` was configued + def has_no_arguments? + @has_no_arguments + end + + def arguments(context = GraphQL::Query::NullContext.instance, require_defined_arguments = true) + if require_defined_arguments && !has_no_arguments? && !any_arguments? + warn(GraphQL::Schema::InputObject::ArgumentsAreRequiredError.new(self).message + "\n\nThis will raise an error in a future GraphQL-Ruby version.") + end + super(context, false) + end + private # Suppress redefinition warning for objectId arguments diff --git a/lib/graphql/schema/member/has_arguments.rb b/lib/graphql/schema/member/has_arguments.rb index 126c406ab8..3aa52a56ea 100644 --- a/lib/graphql/schema/member/has_arguments.rb +++ b/lib/graphql/schema/member/has_arguments.rb @@ -76,7 +76,7 @@ def remove_argument(arg_defn) end # @return [Hash GraphQL::Schema::Argument] Arguments defined on this thing, keyed by name. Includes inherited definitions - def arguments(context = GraphQL::Query::NullContext.instance) + def arguments(context = GraphQL::Query::NullContext.instance, _require_defined_arguments = nil) if own_arguments.any? own_arguments_that_apply = {} own_arguments.each do |name, args_entry| @@ -100,9 +100,9 @@ def inherited(child_class) end module InheritedArguments - def arguments(context = GraphQL::Query::NullContext.instance) - own_arguments = super - inherited_arguments = superclass.arguments(context) + def arguments(context = GraphQL::Query::NullContext.instance, require_defined_arguments = true) + own_arguments = super(context, require_defined_arguments) + inherited_arguments = superclass.arguments(context, false) if own_arguments.any? if inherited_arguments.any? @@ -149,7 +149,7 @@ def get_argument(argument_name, context = GraphQL::Query::NullContext.instance) end module FieldConfigured - def arguments(context = GraphQL::Query::NullContext.instance) + def arguments(context = GraphQL::Query::NullContext.instance, _require_defined_arguments = nil) own_arguments = super if @resolver_class inherited_arguments = @resolver_class.field_arguments(context) diff --git a/lib/graphql/schema/member/has_fields.rb b/lib/graphql/schema/member/has_fields.rb index 3a5065500f..be041b1698 100644 --- a/lib/graphql/schema/member/has_fields.rb +++ b/lib/graphql/schema/member/has_fields.rb @@ -79,6 +79,18 @@ def global_id_field(field_name, **kwargs) end end + # @param new_has_no_fields [Boolean] Call with `true` to make this Object type ignore the requirement to have any defined fields. + # @return [void] + def has_no_fields(new_has_no_fields) + @has_no_fields = new_has_no_fields + nil + end + + # @return [Boolean] `true` if `has_no_fields(true)` was configued + def has_no_fields? + @has_no_fields + end + # @return [Hash GraphQL::Schema::Field, Array>] Fields defined on this class _specifically_, not parent classes def own_fields @own_fields ||= {} @@ -155,9 +167,11 @@ def fields(context = GraphQL::Query::NullContext.instance) warden = Warden.from_context(context) # Local overrides take precedence over inherited fields visible_fields = {} + had_any_fields_at_all = false for ancestor in ancestors if ancestor.respond_to?(:own_fields) && visible_interface_implementation?(ancestor, context, warden) ancestor.own_fields.each do |field_name, fields_entry| + had_any_fields_at_all = true # Choose the most local definition that passes `.visible?` -- # stop checking for fields by name once one has been found. if !visible_fields.key?(field_name) && (f = Warden.visible_entry?(:visible_field?, fields_entry, context, warden)) @@ -166,6 +180,9 @@ def fields(context = GraphQL::Query::NullContext.instance) end end end + if !had_any_fields_at_all && !has_no_fields? + warn(GraphQL::Schema::Object::FieldsAreRequiredError.new(self).message + "\n\nThis will raise an error in a future GraphQL-Ruby version.") + end visible_fields end end @@ -188,6 +205,7 @@ def inherited(subclass) subclass.class_eval do @own_fields ||= nil @field_class ||= nil + @has_no_fields ||= false end end diff --git a/lib/graphql/schema/object.rb b/lib/graphql/schema/object.rb index e778c331c1..25dafdd239 100644 --- a/lib/graphql/schema/object.rb +++ b/lib/graphql/schema/object.rb @@ -8,6 +8,14 @@ class Object < GraphQL::Schema::Member extend GraphQL::Schema::Member::HasFields extend GraphQL::Schema::Member::HasInterfaces + # Raised when an Object doesn't have any field defined and hasn't explicitly opted out of this requirement + class FieldsAreRequiredError < GraphQL::Error + def initialize(object_type) + message = "Object types must have fields, but #{object_type.graphql_name} doesn't have any. Define a field for this type, remove it from your schema, or add `has_no_fields(true)` to its definition." + super(message) + end + end + # @return [Object] the application object this type is wrapping attr_reader :object diff --git a/spec/graphql/logger_spec.rb b/spec/graphql/logger_spec.rb index c811067685..f63ad88c78 100644 --- a/spec/graphql/logger_spec.rb +++ b/spec/graphql/logger_spec.rb @@ -78,6 +78,8 @@ class Query < GraphQL::Schema::Object def node(id:) end + + field :something_else, String end query(Query) end diff --git a/spec/graphql/schema/build_from_definition_spec.rb b/spec/graphql/schema/build_from_definition_spec.rb index 7fa9ba157a..0a6d8b5f51 100644 --- a/spec/graphql/schema/build_from_definition_spec.rb +++ b/spec/graphql/schema/build_from_definition_spec.rb @@ -18,15 +18,12 @@ def assert_schema_and_compare_output(definition) query: HelloScalars } -type EmptyType - type HelloScalars { bool: Boolean float: Float id: ID int: Int str: String! - t: EmptyType } SCHEMA diff --git a/spec/graphql/schema/directive/flagged_spec.rb b/spec/graphql/schema/directive/flagged_spec.rb index e794f5f9a2..466adfdd83 100644 --- a/spec/graphql/schema/directive/flagged_spec.rb +++ b/spec/graphql/schema/directive/flagged_spec.rb @@ -41,6 +41,8 @@ def antarctica; true; end field :santas_workshop, Boolean, null: false, directives: { GraphQL::Schema::Directive::Flagged => { by: ["northPole"] } } def santas_workshop; true; end + + field :something_not_flagged, String end query(Query) diff --git a/spec/graphql/schema/has_single_input_argument_spec.rb b/spec/graphql/schema/has_single_input_argument_spec.rb index e250b27c7e..5a242d8568 100644 --- a/spec/graphql/schema/has_single_input_argument_spec.rb +++ b/spec/graphql/schema/has_single_input_argument_spec.rb @@ -124,7 +124,6 @@ def resolve name: 'name', } end - end class Mutation < GraphQL::Schema::Object field_class GraphQL::Schema::Field diff --git a/spec/graphql/schema/input_object_spec.rb b/spec/graphql/schema/input_object_spec.rb index 81a788b636..9b5f01a716 100644 --- a/spec/graphql/schema/input_object_spec.rb +++ b/spec/graphql/schema/input_object_spec.rb @@ -1419,4 +1419,58 @@ def result(values:) end end end + + describe "when no arguments are defined" do + describe "when defined with no fields" do + class NoArgumentsSchema < GraphQL::Schema + class NoArgumentsInput < GraphQL::Schema::InputObject + end + + class NoArgumentsCompatInput < GraphQL::Schema::InputObject + has_no_arguments(true) + end + + class Query < GraphQL::Schema::Object + field :no_arguments, String, fallback_value: "NO_ARGS" do + argument :input, NoArgumentsInput + end + + field :no_arguments_compat, String, fallback_value: "OK" do + argument :input, NoArgumentsCompatInput + end + end + + query(Query) + end + + it "raises an error at runtime and printing" do + refute NoArgumentsSchema::NoArgumentsInput.has_no_arguments? + + expected_message = "Input Object types must have arguments, but NoArgumentsInput doesn't have any. Define an argument for this type, remove it from your schema, or add `has_no_arguments(true)` to its definition. + +This will raise an error in a future GraphQL-Ruby version. +" + res = assert_warns(expected_message) do + NoArgumentsSchema.execute("{ noArguments(input: {}) }") + end + assert_equal "NO_ARGS", res["data"]["noArguments"] + + assert_warns(expected_message) do + NoArgumentsSchema.to_definition + end + + assert_warns(expected_message) do + NoArgumentsSchema.to_json + end + end + + it "doesn't raise an error if has_no_arguments(true)" do + assert NoArgumentsSchema::NoArgumentsCompatInput.has_no_arguments? + res = assert_warns("") do + NoArgumentsSchema.execute("{ noArgumentsCompat(input: {}) }") + end + assert_equal "OK", res["data"]["noArgumentsCompat"] + end + end + end end diff --git a/spec/graphql/schema/object_spec.rb b/spec/graphql/schema/object_spec.rb index 6f7e24f49e..15589978d9 100644 --- a/spec/graphql/schema/object_spec.rb +++ b/spec/graphql/schema/object_spec.rb @@ -512,4 +512,52 @@ def self.wrap(obj, ctx) assert_nil obj1.comment end end + + describe "when defined with no fields" do + class NoFieldsSchema < GraphQL::Schema + class NoFieldsThing < GraphQL::Schema::Object + end + + class NoFieldsCompatThing < GraphQL::Schema::Object + has_no_fields(true) + end + + class Query < GraphQL::Schema::Object + field :no_fields_thing, NoFieldsThing + field :no_fields_compat_thing, NoFieldsCompatThing + end + + query(Query) + end + + it "raises an error at runtime and printing" do + refute NoFieldsSchema::NoFieldsThing.has_no_fields? + + expected_message = "Object types must have fields, but NoFieldsThing doesn't have any. Define a field for this type, remove it from your schema, or add `has_no_fields(true)` to its definition. + +This will raise an error in a future GraphQL-Ruby version. +" + res = assert_warns(expected_message) do + NoFieldsSchema.execute("{ noFieldsThing { blah } }") + end + assert_equal ["Field 'blah' doesn't exist on type 'NoFieldsThing'"], res["errors"].map { |err| err["message"] } + + assert_warns(expected_message) do + NoFieldsSchema.to_definition + end + + assert_warns(expected_message) do + NoFieldsSchema.to_json + end + end + + it "doesn't raise an error if has_no_fields(true)" do + assert NoFieldsSchema::NoFieldsCompatThing.has_no_fields? + + res = assert_warns "" do + NoFieldsSchema.execute("{ noFieldsCompatThing { blah } }") + end + assert_equal ["Field 'blah' doesn't exist on type 'NoFieldsCompatThing'"], res["errors"].map { |e| e["message"] } + end + end end diff --git a/spec/graphql/schema/printer_spec.rb b/spec/graphql/schema/printer_spec.rb index 49676d133f..b00cce7847 100644 --- a/spec/graphql/schema/printer_spec.rb +++ b/spec/graphql/schema/printer_spec.rb @@ -73,10 +73,13 @@ class MediaRating < GraphQL::Schema::Enum value :BOO_HISS end + class NoFields < GraphQL::Schema::Object + has_no_fields(true) end class NoArguments < GraphQL::Schema::InputObject + has_no_arguments(true) end class Query < GraphQL::Schema::Object @@ -710,7 +713,6 @@ def self.visible?(member, ctx) assert_equal expected, custom_filter_schema.to_definition(context: context) end - it "applies an `except` filter" do expected = <