From 059a5b7ea404e81f043beec000018bf5101bc7ae Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 11:54:22 -0700 Subject: [PATCH 01/11] =?UTF-8?q?=F0=9F=90=9B=20=20Move=20the=20`hash=5Fke?= =?UTF-8?q?ys`=20call=20into=20`Hash`=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: George Xu --- lib/graphql/schema/field.rb | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 227f56456a..955cdebe78 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -642,10 +642,6 @@ def resolve(object, args, query_ctx) inner_object = obj.object - if defined?(@hash_key) - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) @@ -658,10 +654,16 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if inner_object.key?(@method_sym) - inner_object[@method_sym] + if defined?(@hash_key) + inner_object.fetch(@hash_key) { + inner_object[@hash_key_str] + } else - inner_object[@method_str] + if inner_object.key?(@method_sym) + inner_object[@method_sym] + else + inner_object[@method_str] + end end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym From 30711e0ace73203ed7dc13f756097dcb60cf193d Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 13:09:53 -0700 Subject: [PATCH 02/11] =?UTF-8?q?=F0=9F=90=9B=20=20move=20it=20back=20and?= =?UTF-8?q?=20use=20something=20safer.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: georgexu22@chime.com --- lib/graphql/schema/field.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 955cdebe78..df12068410 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -642,6 +642,8 @@ def resolve(object, args, query_ctx) inner_object = obj.object + if defined?(@hash_key) + inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) @@ -654,16 +656,10 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if defined?(@hash_key) - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } + if inner_object.key?(@method_sym) + inner_object[@method_sym] else - if inner_object.key?(@method_sym) - inner_object[@method_sym] - else - inner_object[@method_str] - end + inner_object[@method_str] end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym From 3a6672703f381eb491e18ddd739ef4493c4a9285 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 13:19:07 -0700 Subject: [PATCH 03/11] Fix the bug by using nil key check, Co-authored-by: George Xu --- lib/graphql/schema/field.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index df12068410..87a7d15deb 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -643,7 +643,13 @@ def resolve(object, args, query_ctx) inner_object = obj.object if defined?(@hash_key) +<<<<<<< HEAD inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] +======= + inner_object.fetch(@hash_key) { + inner_object[@hash_key_str] + } +>>>>>>> parent of 059a5b7ea (🐛 Move the `hash_keys` call into `Hash` check) elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) From 4e9e3f95c1ff26cccdff1d3fd11ba7f4cf7b1c4e Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 13:21:30 -0700 Subject: [PATCH 04/11] =?UTF-8?q?=F0=9F=90=9B=20=20Move=20it=20back=20to?= =?UTF-8?q?=20being=20a=20bit=20safer.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: George Xu --- lib/graphql/schema/field.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 87a7d15deb..b9d221461c 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -643,13 +643,7 @@ def resolve(object, args, query_ctx) inner_object = obj.object if defined?(@hash_key) -<<<<<<< HEAD inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] -======= - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } ->>>>>>> parent of 059a5b7ea (🐛 Move the `hash_keys` call into `Hash` check) elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) @@ -662,10 +656,16 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if inner_object.key?(@method_sym) - inner_object[@method_sym] + if defined?(@hash_key) + inner_object.fetch(@hash_key) { + inner_object[@hash_key_str] + } else - inner_object[@method_str] + if inner_object.key?(@method_sym) + inner_object[@method_sym] + else + inner_object[@method_str] + end end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym From c243cadbda84373c7b77727d088a54252c756e5c Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 13:33:36 -0700 Subject: [PATCH 05/11] =?UTF-8?q?=E2=8F=AA=20=20revert=20accidental=20chan?= =?UTF-8?q?ges?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: georgexu22@chime.com --- lib/graphql/schema/field.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index b9d221461c..df12068410 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -656,16 +656,10 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if defined?(@hash_key) - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } + if inner_object.key?(@method_sym) + inner_object[@method_sym] else - if inner_object.key?(@method_sym) - inner_object[@method_sym] - else - inner_object[@method_str] - end + inner_object[@method_str] end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym From 9c6ff495f833a3aa1240199ccd167edbacd1d640 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 14:18:40 -0700 Subject: [PATCH 06/11] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20=20Maybe=20a=20refac?= =?UTF-8?q?tor=20--=20but=20let's=20explicitly=20check=20that=20it's=20a?= =?UTF-8?q?=20`Hash`=20before=20trying=20to=20see=20if=20`@hash=5Fkey`=20i?= =?UTF-8?q?s=20defined?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/graphql/schema/field.rb | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index df12068410..9e8a27372c 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "graphql/schema/field/connection_extension" require "graphql/schema/field/scope_extension" - +require "pry" module GraphQL class Schema class Field @@ -642,8 +642,12 @@ def resolve(object, args, query_ctx) inner_object = obj.object - if defined?(@hash_key) - inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] + if inner_object.respond_to?(resolver_method) && inner_object.is_a?(Hash) + if defined?(@hash_key) + inner_object.fetch(@hash_key) { + inner_object[@hash_key_str] + } + end elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) @@ -656,10 +660,16 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if inner_object.key?(@method_sym) - inner_object[@method_sym] + if defined?(@hash_key) + inner_object.fetch(@hash_key) { + inner_object[@hash_key_str] + } else - inner_object[@method_str] + if inner_object.key?(@method_sym) + inner_object[@method_sym] + else + inner_object[@method_str] + end end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym From d49424961bd28bcb06d0502038833b35d9dee6bc Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 14:20:23 -0700 Subject: [PATCH 07/11] =?UTF-8?q?=F0=9F=94=A5=20=20remove=20`pry`=20requir?= =?UTF-8?q?es?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/graphql/schema/field.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 9e8a27372c..13b550e13a 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require "graphql/schema/field/connection_extension" require "graphql/schema/field/scope_extension" -require "pry" + module GraphQL class Schema class Field From 1ec41553aa28e535066a16823cf0049fd4275bbe Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 14:22:22 -0700 Subject: [PATCH 08/11] =?UTF-8?q?=F0=9F=93=9D=20=20add=20comment?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/graphql/schema/field.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 13b550e13a..c6f042a4b9 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -642,6 +642,10 @@ def resolve(object, args, query_ctx) inner_object = obj.object + # regression against https://github.com/rmosolgo/graphql-ruby/issues/3944 + # check if the object is a hash, but also responds to the method. If so, + # revert to using #fetch to grab the item, otherwise revert to using + # the string field. if inner_object.respond_to?(resolver_method) && inner_object.is_a?(Hash) if defined?(@hash_key) inner_object.fetch(@hash_key) { From cd7f02df4e30df95dff873901bdf18b0b8c45c37 Mon Sep 17 00:00:00 2001 From: Daniel Dao Date: Wed, 4 May 2022 14:36:59 -0700 Subject: [PATCH 09/11] =?UTF-8?q?=E2=8F=AA=20=20revert=20back=20to=20the?= =?UTF-8?q?=20original?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/graphql/schema/field.rb | 26 ++++++-------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index c6f042a4b9..a40cbc6d74 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -642,16 +642,8 @@ def resolve(object, args, query_ctx) inner_object = obj.object - # regression against https://github.com/rmosolgo/graphql-ruby/issues/3944 - # check if the object is a hash, but also responds to the method. If so, - # revert to using #fetch to grab the item, otherwise revert to using - # the string field. - if inner_object.respond_to?(resolver_method) && inner_object.is_a?(Hash) - if defined?(@hash_key) - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } - end + if defined?(@hash_key) + inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] elsif @dig_keys inner_object.dig(*@dig_keys) elsif obj.respond_to?(resolver_method) @@ -664,16 +656,10 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if defined?(@hash_key) - inner_object.fetch(@hash_key) { - inner_object[@hash_key_str] - } + if inner_object.key?(@method_sym) + inner_object[@method_sym] else - if inner_object.key?(@method_sym) - inner_object[@method_sym] - else - inner_object[@method_str] - end + inner_object[@method_str] end elsif inner_object.respond_to?(@method_sym) method_to_call = @method_sym @@ -833,4 +819,4 @@ def run_extensions_before_resolve(obj, args, ctx, extended, idx: 0) end end end -end +end \ No newline at end of file From 3a47a4c3f702527f801b4580ca585fe6d9fb3632 Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 24 May 2022 10:16:14 -0400 Subject: [PATCH 10/11] When hash_key: is provided, do a key lookup with it or a stringified version of it --- lib/graphql/schema/field.rb | 18 +++++++++----- spec/graphql/schema/field_spec.rb | 41 +++++++++++++++++++++++++++---- 2 files changed, 48 insertions(+), 11 deletions(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index a40cbc6d74..739433b81e 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -643,9 +643,7 @@ def resolve(object, args, query_ctx) inner_object = obj.object if defined?(@hash_key) - inner_object[@hash_key].nil? ? inner_object[@hash_key_str] : inner_object[@hash_key] - elsif @dig_keys - inner_object.dig(*@dig_keys) + inner_object[@hash_key] || inner_object[@hash_key_str] elsif obj.respond_to?(resolver_method) method_to_call = resolver_method method_receiver = obj @@ -656,7 +654,15 @@ def resolve(object, args, query_ctx) obj.public_send(resolver_method) end elsif inner_object.is_a?(Hash) - if inner_object.key?(@method_sym) + if @dig_keys + inner_object.dig(*@dig_keys) + elsif defined?(@hash_key) + if inner_object.key?(@hash_key) + inner_object[@hash_key] + else + inner_object[@hash_key_str] + end + elsif inner_object.key?(@method_sym) inner_object[@method_sym] else inner_object[@method_str] @@ -749,7 +755,7 @@ def assert_satisfactory_implementation(receiver, method_name, ruby_kwargs) if unsatisfied_ruby_kwargs.any? || unsatisfied_method_params.any? raise FieldImplementationFailed.new, <<-ERR -Failed to call #{method_name} on #{receiver.inspect} because the Ruby method params were incompatible with the GraphQL arguments: +Failed to call `#{method_name.inspect}` on #{receiver.inspect} because the Ruby method params were incompatible with the GraphQL arguments: #{ unsatisfied_ruby_kwargs .map { |key, value| "- `#{key}: #{value}` was given by GraphQL but not defined in the Ruby method. Add `#{key}:` to the method parameters." } @@ -819,4 +825,4 @@ def run_extensions_before_resolve(obj, args, ctx, extended, idx: 0) end end end -end \ No newline at end of file +end diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index dd3bed2cee..c26fc4143d 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -208,7 +208,7 @@ def f5(**ok_keyrest) err = assert_raises GraphQL::Schema::Field::FieldImplementationFailed do ArgumentErrorSchema.execute("{ f1(something: 12) }") end - assert_equal "Failed to call f1 on # because the Ruby method params were incompatible with the GraphQL arguments: + assert_equal "Failed to call `:f1` on # because the Ruby method params were incompatible with the GraphQL arguments: - `something: 12` was given by GraphQL but not defined in the Ruby method. Add `something:` to the method parameters. ", err.message @@ -218,7 +218,7 @@ def f5(**ok_keyrest) err = assert_raises GraphQL::Schema::Field::FieldImplementationFailed do ArgumentErrorSchema.execute("{ f2(something: 12) }") end - assert_equal "Failed to call field_2 on # because the Ruby method params were incompatible with the GraphQL arguments: + assert_equal "Failed to call `:field_2` on # because the Ruby method params were incompatible with the GraphQL arguments: - `something: 12` was given by GraphQL but not defined in the Ruby method. Add `something:` to the method parameters. ", err.message @@ -227,7 +227,7 @@ def f5(**ok_keyrest) err = assert_raises GraphQL::Schema::Field::FieldImplementationFailed do ArgumentErrorSchema.execute("{ f3(something: 1) }") end - assert_equal "Failed to call f3 on # because the Ruby method params were incompatible with the GraphQL arguments: + assert_equal "Failed to call `:f3` on # because the Ruby method params were incompatible with the GraphQL arguments: - `something: 1` was given by GraphQL but not defined in the Ruby method. Add `something:` to the method parameters. - `always_missing:` is required by Ruby, but not by GraphQL. Consider `always_missing: nil` instead, or making this argument required in GraphQL. @@ -236,7 +236,7 @@ def f5(**ok_keyrest) err = assert_raises GraphQL::Schema::Field::FieldImplementationFailed do ArgumentErrorSchema.execute("{ f4 }") end - assert_equal "Failed to call f4 on # because the Ruby method params were incompatible with the GraphQL arguments: + assert_equal "Failed to call `:f4` on # because the Ruby method params were incompatible with the GraphQL arguments: - `never_positional` is required by Ruby, but GraphQL doesn't pass positional arguments. If it's meant to be a GraphQL argument, use `never_positional:` instead. Otherwise, remove it. ", err.message @@ -611,9 +611,14 @@ def search_results "OtherCapital" => "explicit-hash-key-works", "some_random_key" => "hash-key-works-when-underlying-object-responds-to-field-name", "stringified_hash_key" => "hash-key-is-tried-as-string", - } end + + field :ostruct_results, ResultType, null: false + + def ostruct_results + OpenStruct.new(search_results) + end end query(QueryType) @@ -644,6 +649,32 @@ def search_results } assert_equal expected_result, search_results end + + it "works with non-hash instances" do + res = HashKeySchema.execute <<-GRAPHQL + { + ostructResults { + method + lowercase + Capital + Other + OtherCapital + stringifiedHashKey + } + } + GRAPHQL + + search_results = res["data"]["ostructResults"] + expected_result = { + "lowercase" => "lowercase-works", + "Capital" => "capital-camelize-false-works", + "Other" => "capital-camelize-true-works", + "OtherCapital" => "explicit-hash-key-works", + "method" => "hash-key-works-when-underlying-object-responds-to-field-name", + "stringifiedHashKey" => "hash-key-is-tried-as-string" + } + assert_equal expected_result, search_results + end end describe "when the owner is nil" do From adba39c2ad78c340020e3de6dcddfcb198475cae Mon Sep 17 00:00:00 2001 From: Robert Mosolgo Date: Tue, 24 May 2022 10:20:09 -0400 Subject: [PATCH 11/11] Fix method_str with hash_key --- lib/graphql/schema/field.rb | 2 +- spec/graphql/schema/field_spec.rb | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/graphql/schema/field.rb b/lib/graphql/schema/field.rb index 739433b81e..eca1979084 100644 --- a/lib/graphql/schema/field.rb +++ b/lib/graphql/schema/field.rb @@ -246,7 +246,7 @@ def initialize(type: nil, name: nil, owner: nil, null: nil, description: :not_gi end end - method_name = method || name_s + method_name = method || hash_key || name_s @dig_keys = dig if hash_key @hash_key = hash_key diff --git a/spec/graphql/schema/field_spec.rb b/spec/graphql/schema/field_spec.rb index c26fc4143d..461a0249e1 100644 --- a/spec/graphql/schema/field_spec.rb +++ b/spec/graphql/schema/field_spec.rb @@ -675,6 +675,11 @@ def ostruct_results } assert_equal expected_result, search_results end + + it "populates `method_str`" do + hash_key_field = HashKeySchema.get_field("Result", "method") + assert_equal "some_random_key", hash_key_field.method_str + end end describe "when the owner is nil" do