Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return ordered result hashes #5060

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/graphql/execution/interpreter/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ def evaluate_selections(gathered_selections, selections_result, target_result, r
finished_jobs = 0
enqueued_jobs = gathered_selections.size
gathered_selections.each do |result_name, field_ast_nodes_or_ast_node|

selections_result.set_placeholder(result_name)
# Field resolution may pause the fiber,
# so it wouldn't get to the `Resolve` call that happens below.
# So instead trigger a run from this outer context.
Expand Down
13 changes: 13 additions & 0 deletions lib/graphql/execution/interpreter/runtime/graphql_result.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,19 @@ def set_child_result(key, value)
value
end


# @api private
PLACEHOLDER = Object.new

def set_placeholder(key)
@graphql_result_data[key] = PLACEHOLDER
@graphql_metadata && @graphql_metadata[key] = PLACEHOLDER
if (t = @graphql_merged_into)
t.set_placeholder(key)
end
nil
end

def delete(key)
@graphql_metadata && @graphql_metadata.delete(key)
@graphql_result_data.delete(key)
Expand Down
23 changes: 13 additions & 10 deletions spec/graphql/dataloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def self.included(child_class)
]
}
}
assert_equal expected_data, res
assert_graphql_equal expected_data, res
assert_equal [[:mget, ["5", "6"]], [:mget, ["2", "3"]]], database_log
end

Expand All @@ -549,7 +549,7 @@ def self.included(child_class)
}
GRAPHQL

assert_equal({ "first" => "first", "second" => "second" }, res["data"])
assert_graphql_equal({ "first" => "first", "second" => "second" }, res["data"])
assert_equal ["begin first", "end first", "begin second", "end second"], res.context[:mutation_log]
end

Expand All @@ -561,7 +561,7 @@ def self.included(child_class)
}
GRAPHQL

assert_equal({"setCache" => "Salad", "getCache" => "1"}, res["data"])
assert_graphql_equal({"setCache" => "Salad", "getCache" => "1"}, res["data"])
end

it "batch-loads" do
Expand All @@ -577,6 +577,7 @@ def self.included(child_class)
ri1: recipeIngredient(recipe: { id: 6, ingredientNumber: 3 }) {
name
}
__typename
}
GRAPHQL

Expand All @@ -594,8 +595,10 @@ def self.included(child_class)
"ri1" => {
"name" => "Cheese",
},
"__typename" => "Query",
}
assert_equal(expected_data, res["data"])

assert_graphql_equal(expected_data, res["data"])

expected_log = [
[:mget, [
Expand Down Expand Up @@ -626,7 +629,7 @@ def self.included(child_class)
{"data"=>{"i2"=>{"name"=>"Corn"}, "r1"=>{"ingredients"=>[{"name"=>"Wheat"}, {"name"=>"Corn"}, {"name"=>"Butter"}, {"name"=>"Baking Soda"}]}}},
{"data"=>{"i1"=>{"name"=>"Wheat"}, "ri1"=>{"name"=>"Corn"}}},
]
assert_equal expected_result, result
assert_graphql_equal expected_result, result
expected_log = [
[:mget, ["1", "2", "5"]],
[:mget, ["3", "4"]],
Expand All @@ -643,7 +646,7 @@ def self.included(child_class)
GRAPHQL

expected_data = { "i1" => { "name" => "Wheat" }, "i2" => { "name" => "Corn" } }
assert_equal expected_data, res["data"]
assert_graphql_equal expected_data, res["data"]
assert_equal [[:mget, ["1", "2"]]], database_log
end

Expand Down Expand Up @@ -709,7 +712,7 @@ def self.included(child_class)
"name" => "Wheat",
}
}
assert_equal expected_data, res["data"]
assert_graphql_equal expected_data, res["data"]
end

it "Works when the parent field didn't yield" do
Expand Down Expand Up @@ -739,7 +742,7 @@ def self.included(child_class)
]},
]
}
assert_equal expected_data, res["data"]
assert_graphql_equal expected_data, res["data"]

expected_log = [
[:mget, ["5", "6"]],
Expand All @@ -764,7 +767,7 @@ def self.included(child_class)
{"name"=>"Butter"},
]
}
assert_equal expected_data, res["data"]
assert_graphql_equal expected_data, res["data"]

expected_log = [
[:mget, ["5", "6"]],
Expand All @@ -790,7 +793,7 @@ def self.included(child_class)
"name" => "Wheat",
}
}
assert_equal expected_data, res["data"]
assert_graphql_equal expected_data, res["data"]
end

it "Works with analyzing arguments with `loads:`, even with .request" do
Expand Down
18 changes: 18 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,24 @@
require "minitest/reporters"
require "graphql/batch"

module Minitest
module Assertions
def assert_graphql_equal(data1, data2, message = "GraphQL Result was equal")
case data1
when Hash
assert_equal(data1, data2, message)
assert_equal(data1.keys, data2.keys, "Order of keys matched (#{message})")
when Array
data1.each_with_index do |item1, idx|
assert_graphql_equal(item1, data2[idx], message + "[Item #{idx + 1}] ")
end
else
raise ArgumentError, "assert_graphql_equal doesn't support #{data1.class} yet"
end
end
end
end

running_in_rubymine = ENV["RM_INFO"]
unless running_in_rubymine
Minitest::Reporters.use! Minitest::Reporters::DefaultReporter.new(color: true)
Expand Down
Loading