Skip to content

Commit

Permalink
Fix returning [Type, false] from resolve_type
Browse files Browse the repository at this point in the history
#4130 fixed returning
`[Type, nil]` from a union's `resolve_type`, but it caused a regression
for literal `false` values.

`Schema#resolve_type` had a `if resolved_value` conditional would be false
for `false` values causing it _always_ return a single value (the `resolve_type`).

The runtime's `resolve_type` method would then _always_ return the tuple
version (in non-lazy cases): `[type, value]`. However, since
`Schema#resolve_type` was only returning a single value, the resolved
value would be lost resulting in `[type, nil]` being passed on.

The fix is to remove the `if resolved_value` conditional entirely and
always return the two-value tuple ensuring the original resolved value
is correctly passed along.

Note: the runtime still has a branch to check if the `resolve_type`
value is a tuple or not. The only case where this won't be true is for
lazy resolved types.
  • Loading branch information
swalkinshaw authored and rmosolgo committed Mar 29, 2023
1 parent 96ae99b commit 7e077be
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 34 deletions.
6 changes: 1 addition & 5 deletions lib/graphql/schema.rb
Expand Up @@ -785,11 +785,7 @@ def resolve_type(type, obj, ctx)
end

if resolved_type.nil? || (resolved_type.is_a?(Module) && resolved_type.respond_to?(:kind))
if resolved_value
[resolved_type, resolved_value]
else
resolved_type
end
[resolved_type, resolved_value]
else
raise ".resolve_type should return a type definition, but got #{resolved_type.inspect} (#{resolved_type.class}) from `resolve_type(#{type}, #{obj}, #{ctx})`"
end
Expand Down
135 changes: 107 additions & 28 deletions spec/graphql/schema/union_spec.rb
Expand Up @@ -67,7 +67,7 @@
assert_equal "Fragment on Ensemble can't be spread inside PerformingAct", res.to_h["errors"].first["message"]
end

describe "two-value type resolution" do
describe "type resolution" do
Box = Struct.new(:value)

class Schema < GraphQL::Schema
Expand All @@ -79,60 +79,139 @@ class B < GraphQL::Schema::Object
field :b, String, method: :itself
end

class MyUnion < GraphQL::Schema::Union
possible_types A, B
class C < GraphQL::Schema::Object
field :c, Boolean, method: :itself
end

class UnboxedUnion < GraphQL::Schema::Union
possible_types A, C

def self.resolve_type(object, ctx)
if object.value == "return-nil"
case object
when FalseClass
C
else
A
end
end
end

class BoxedUnion < GraphQL::Schema::Union
possible_types A, B, C

def self.resolve_type(object, ctx)
case object.value
when "return-nil"
[B, nil]
when FalseClass
[C, object.value]
else
[A, object.value]
end
end
end

class Query < GraphQL::Schema::Object
field :my_union, MyUnion
field :boxed_union, BoxedUnion

def my_union
def boxed_union
Box.new(context[:value])
end

field :unboxed_union, UnboxedUnion

def unboxed_union
context[:value]
end
end

query(Query)
end

it "can cast the object after resolving the type" do
describe "two-value resolution" do
it "can cast the object after resolving the type" do

query_str = <<-GRAPHQL
{
myUnion {
... on A { a }
query_str = <<-GRAPHQL
{
boxedUnion {
... on A { a }
}
}
}
GRAPHQL
GRAPHQL

res = Schema.execute(query_str, context: { value: "unwrapped" })

assert_equal({
'data' => { 'boxedUnion' => { 'a' => 'unwrapped' } }
}, res.to_h)
end

it "uses `false` when returned from resolve_type" do
query_str = <<-GRAPHQL
{
boxedUnion {
... on C { c }
}
}
GRAPHQL

res = Schema.execute(query_str, context: { value: false })

res = Schema.execute(query_str, context: { value: "unwrapped" })
assert_equal({
'data' => { 'boxedUnion' => { 'c' => false } }
}, res.to_h)
end

assert_equal({
'data' => { 'myUnion' => { 'a' => 'unwrapped' } }
}, res.to_h)
it "uses `nil` when returned from resolve_type" do
query_str = <<-GRAPHQL
{
boxedUnion {
... on B { b }
}
}
GRAPHQL

res = Schema.execute(query_str, context: { value: "return-nil" })

assert_equal({
'data' => { 'boxedUnion' => { 'b' => nil } }
}, res.to_h)
end
end

it "uses `nil` when returned from resolve_type" do
query_str = <<-GRAPHQL
{
myUnion {
... on B { b }
describe "single-value resolution" do
it "can cast the object after resolving the type" do

query_str = <<-GRAPHQL
{
unboxedUnion {
... on A { a }
}
}
}
GRAPHQL
GRAPHQL

res = Schema.execute(query_str, context: { value: "return-nil" })
res = Schema.execute(query_str, context: { value: "string" })

assert_equal({
'data' => { 'myUnion' => { 'b' => nil } }
}, res.to_h)
assert_equal({
'data' => { 'unboxedUnion' => { 'a' => 'string' } }
}, res.to_h)
end

it "works with literal false values" do
query_str = <<-GRAPHQL
{
unboxedUnion {
... on C { c }
}
}
GRAPHQL

res = Schema.execute(query_str, context: { value: false })

assert_equal({
'data' => { 'unboxedUnion' => { 'c' => false } }
}, res.to_h)
end
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion spec/integration/rails/graphql/schema_spec.rb
Expand Up @@ -61,7 +61,7 @@
describe "when the return value is nil" do
it "returns nil" do
result = relay_schema.resolve_type(123, nil, GraphQL::Query::NullContext)
assert_equal(nil, result)
assert_equal([nil, nil], result)
end
end

Expand Down

0 comments on commit 7e077be

Please sign in to comment.