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

Migrate internal runtime state to Fiber storage #5176

Merged
merged 2 commits into from
Nov 26, 2024
Merged
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/current.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def self.operation_name
# @see GraphQL::Field#path for a string identifying this field
# @return [GraphQL::Field, nil] The currently-running field, if there is one.
def self.field
Thread.current[:__graphql_runtime_info]&.values&.first&.current_field
Fiber[:__graphql_runtime_info]&.values&.first&.current_field
end

# @return [Class, nil] The currently-running {Dataloader::Source} class, if there is one.
Expand Down
5 changes: 1 addition & 4 deletions lib/graphql/dataloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,7 @@ def nonblocking?
def get_fiber_variables
fiber_vars = {}
Thread.current.keys.each do |fiber_var_key|
# This variable should be fresh in each new fiber
if fiber_var_key != :__graphql_runtime_info
fiber_vars[fiber_var_key] = Thread.current[fiber_var_key]
end
fiber_vars[fiber_var_key] = Thread.current[fiber_var_key]
end
fiber_vars
end
Expand Down
4 changes: 2 additions & 2 deletions lib/graphql/dataloader/async_dataloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module GraphQL
class Dataloader
class AsyncDataloader < Dataloader
def yield
if (condition = Thread.current[:graphql_dataloader_next_tick])
if (condition = Fiber[:graphql_dataloader_next_tick])
condition.wait
else
Fiber.yield
Expand Down Expand Up @@ -78,7 +78,7 @@ def spawn_source_task(parent_task, condition)
fiber_vars = get_fiber_variables
parent_task.async do
set_fiber_variables(fiber_vars)
Thread.current[:graphql_dataloader_next_tick] = condition
Fiber[:graphql_dataloader_next_tick] = condition
pending_sources.each(&:run_pending_keys)
cleanup_fiber
end
Expand Down
6 changes: 3 additions & 3 deletions lib/graphql/execution/interpreter/runtime.rb
Original file line number Diff line number Diff line change
Expand Up @@ -736,7 +736,7 @@ def directives_include?(node, graphql_object, parent_type)
end

def get_current_runtime_state
current_state = Thread.current[:__graphql_runtime_info] ||= {}.compare_by_identity
current_state = Fiber[:__graphql_runtime_info] ||= {}.compare_by_identity
current_state[@query] ||= CurrentState.new
end

Expand Down Expand Up @@ -821,11 +821,11 @@ def arguments(graphql_object, arg_owner, ast_node)
end

def delete_all_interpreter_context
per_query_state = Thread.current[:__graphql_runtime_info]
per_query_state = Fiber[:__graphql_runtime_info]
if per_query_state
per_query_state.delete(@query)
if per_query_state.size == 0
Thread.current[:__graphql_runtime_info] = nil
Fiber[:__graphql_runtime_info] = nil
end
end
nil
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/pagination/connection.rb
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def cursor_for(item)

def detect_was_authorized_by_scope_items
if @context &&
(current_runtime_state = Thread.current[:__graphql_runtime_info]) &&
(current_runtime_state = Fiber[:__graphql_runtime_info]) &&
(query_runtime_state = current_runtime_state[@context.query])
query_runtime_state.was_authorized_by_scope_items
else
Expand Down
8 changes: 4 additions & 4 deletions lib/graphql/query/context.rb
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ def [](key)
if key == :current_path
current_path
else
(current_runtime_state = Thread.current[:__graphql_runtime_info]) &&
(current_runtime_state = Fiber[:__graphql_runtime_info]) &&
(query_runtime_state = current_runtime_state[@query]) &&
(query_runtime_state.public_send(key))
end
Expand Down Expand Up @@ -144,7 +144,7 @@ def execution_errors
end

def current_path
current_runtime_state = Thread.current[:__graphql_runtime_info]
current_runtime_state = Fiber[:__graphql_runtime_info]
query_runtime_state = current_runtime_state && current_runtime_state[@query]

path = query_runtime_state &&
Expand All @@ -169,7 +169,7 @@ def delete(key)

def fetch(key, default = UNSPECIFIED_FETCH_DEFAULT)
if RUNTIME_METADATA_KEYS.include?(key)
(runtime = Thread.current[:__graphql_runtime_info]) &&
(runtime = Fiber[:__graphql_runtime_info]) &&
(query_runtime_state = runtime[@query]) &&
(query_runtime_state.public_send(key))
elsif @scoped_context.key?(key)
Expand All @@ -187,7 +187,7 @@ def fetch(key, default = UNSPECIFIED_FETCH_DEFAULT)

def dig(key, *other_keys)
if RUNTIME_METADATA_KEYS.include?(key)
(current_runtime_state = Thread.current[:__graphql_runtime_info]) &&
(current_runtime_state = Fiber[:__graphql_runtime_info]) &&
(query_runtime_state = current_runtime_state[@query]) &&
(obj = query_runtime_state.public_send(key)) &&
if other_keys.empty?
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/schema/field/scope_extension.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def after_resolve(object:, arguments:, context:, value:, memo:)
if ret_type.respond_to?(:scope_items)
scoped_items = ret_type.scope_items(value, context)
if !scoped_items.equal?(value) && !ret_type.reauthorize_scoped_objects
if (current_runtime_state = Thread.current[:__graphql_runtime_info]) &&
if (current_runtime_state = Fiber[:__graphql_runtime_info]) &&
(query_runtime_state = current_runtime_state[context.query])
query_runtime_state.was_authorized_by_scope_items = true
end
Expand Down
4 changes: 2 additions & 2 deletions lib/graphql/types/relay/connection_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def add_page_info_field(obj_type)
def edges
# Assume that whatever authorization needed to happen
# already happened at the connection level.
current_runtime_state = Thread.current[:__graphql_runtime_info]
current_runtime_state = Fiber[:__graphql_runtime_info]
query_runtime_state = current_runtime_state[context.query]
query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
@object.edges
Expand All @@ -205,7 +205,7 @@ def edges
def nodes
# Assume that whatever authorization needed to happen
# already happened at the connection level.
current_runtime_state = Thread.current[:__graphql_runtime_info]
current_runtime_state = Fiber[:__graphql_runtime_info]
query_runtime_state = current_runtime_state[context.query]
query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
@object.nodes
Expand Down
2 changes: 1 addition & 1 deletion lib/graphql/types/relay/edge_behaviors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def self.included(child_class)
end

def node
current_runtime_state = Thread.current[:__graphql_runtime_info]
current_runtime_state = Fiber[:__graphql_runtime_info]
query_runtime_state = current_runtime_state[context.query]
query_runtime_state.was_authorized_by_scope_items = @object.was_authorized_by_scope_items?
@object.node
Expand Down
10 changes: 5 additions & 5 deletions spec/graphql/execution/interpreter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def self.trace(event, data)
module EnsureThreadCleanedUp
def execute_multiplex(multiplex:)
res = super
runtime_info = Thread.current[:__graphql_runtime_info]
runtime_info = Fiber[:__graphql_runtime_info]
if !runtime_info.nil? && runtime_info != {}
if !multiplex.context[:allow_pending_thread_state]
# `nestedQuery` can allow this
Expand Down Expand Up @@ -369,15 +369,15 @@ def execute_multiplex(multiplex:)
{"__typename" => "Expansion", "sym" => "RAV"},
]
assert_equal expected_abstract_list, result["data"]["find"]
assert_nil Thread.current[:__graphql_runtime_info]
assert_nil Fiber[:__graphql_runtime_info]
end

it "runs a nested query and maintains proper state" do
query_str = "query($queryStr: String!) { nestedQuery(query: $queryStr) { result currentPath } }"
result = InterpreterTest::Schema.execute(query_str, variables: { queryStr: "{ __typename }" })
assert_equal '{"data":{"__typename":"Query"}}', result["data"]["nestedQuery"]["result"]
assert_equal ["nestedQuery"], result["data"]["nestedQuery"]["currentPath"]
assert_nil Thread.current[:__graphql_runtime_info]
assert_nil Fiber[:__graphql_runtime_info]
end

it "runs mutation roots atomically and sequentially" do
Expand Down Expand Up @@ -428,7 +428,7 @@ def execute_multiplex(multiplex:)
"exp5" => {"name" => "Ravnica, City of Guilds"},
}
assert_equal expected_data, result["data"]
assert_nil Thread.current[:__graphql_runtime_info]
assert_nil Fiber[:__graphql_runtime_info]
end

describe "runtime info in context" do
Expand Down Expand Up @@ -469,7 +469,7 @@ def execute_multiplex(multiplex:)
# propagated to here
assert_nil res["data"].fetch("expansion")
assert_equal ["Cannot return null for non-nullable field Expansion.name"], res["errors"].map { |e| e["message"] }
assert_nil Thread.current[:__graphql_runtime_info]
assert_nil Fiber[:__graphql_runtime_info]
end

it "places errors ahead of data in the response" do
Expand Down
4 changes: 2 additions & 2 deletions spec/graphql/pagination/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
conn.context = context
assert_nil conn.was_authorized_by_scope_items?

Thread.current[:__graphql_runtime_info] = { context.query => OpenStruct.new(was_authorized_by_scope_items: true) }
Fiber[:__graphql_runtime_info] = { context.query => OpenStruct.new(was_authorized_by_scope_items: true) }
conn.context = context
assert_equal true, conn.was_authorized_by_scope_items?
ensure
Thread.current[:__graphql_runtime_info] = nil
Fiber[:__graphql_runtime_info] = nil
end
end
end
10 changes: 5 additions & 5 deletions spec/graphql/query/context_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
after do
# Clean up test fixtures so they don't pollute later tests
# (Usually this is cleaned up by execution code, but many tests here don't actually execute queries)
Thread.current[:__graphql_runtime_info] = nil
Fiber[:__graphql_runtime_info] = nil
end

class ContextTestSchema < GraphQL::Schema
Expand Down Expand Up @@ -100,7 +100,7 @@ def push_query_error

it "allows you to read values of contexts using dig" do
assert_equal(1, context.dig(:a, :b))
Thread.current[:__graphql_runtime_info] = { context.query => OpenStruct.new(current_arguments: {c: 1}) }
Fiber[:__graphql_runtime_info] = { context.query => OpenStruct.new(current_arguments: {c: 1}) }
assert_equal 1, context.dig(:current_arguments, :c)
assert_equal({c: 1}, context.dig(:current_arguments))
end
Expand All @@ -118,7 +118,7 @@ def push_query_error

it "can override values set by runtime" do
context = GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: {a: {b: 1}})
Thread.current[:__graphql_runtime_info] = { context.query => OpenStruct.new({ current_object: :runtime_value }) }
Fiber[:__graphql_runtime_info] = { context.query => OpenStruct.new({ current_object: :runtime_value }) }
assert_equal :runtime_value, context[:current_object]
context[:current_object] = :override_value
assert_equal :override_value, context[:current_object]
Expand Down Expand Up @@ -391,7 +391,7 @@ class ContextSchema < GraphQL::Schema
it "always retrieves a scoped context value if set" do
context = GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil)
dummy_runtime = OpenStruct.new(current_result: nil)
Thread.current[:__graphql_runtime_info] = { context.query => dummy_runtime }
Fiber[:__graphql_runtime_info] = { context.query => dummy_runtime }
dummy_runtime.current_result = OpenStruct.new(path: ["somewhere"])
expected_key = :a
expected_value = :test
Expand Down Expand Up @@ -453,7 +453,7 @@ class ContextSchema < GraphQL::Schema
it "has a #current_path method" do
context = GraphQL::Query::Context.new(query: OpenStruct.new(schema: schema), values: nil)
current_result = OpenStruct.new(path: ["somewhere", "child", "grandchild"])
Thread.current[:__graphql_runtime_info] = { context.query => OpenStruct.new(current_result: current_result) }
Fiber[:__graphql_runtime_info] = { context.query => OpenStruct.new(current_result: current_result) }
assert_equal ["somewhere", "child", "grandchild"], context.scoped_context.current_path
end
end
Expand Down
Loading