Merge pull request #5550 from rmosolgo/null-dataloader-isolated

Null dataloader isolated

Author: rmosolgo

lib/graphql/dataloader/null_dataloader.rb

@@ -37,13 +37,17 @@ def run(trace_query_lazy: nil)
       end
 
       def run_isolated
-        new_dl = self.class.new
+        # Reuse this instance because execution code may already have a reference to _this_ `dataloader` inside the given block.
+        prev_lazies_at_depth = @lazies_at_depth
+        @lazies_at_depth = @lazies_at_depth.dup.clear
         res = nil
-        new_dl.append_job {
+        append_job {
           res = yield
         }
-        new_dl.run
+        run
         res
+      ensure
+        @lazies_at_depth = prev_lazies_at_depth
       end
 
       def clear_cache; end

lib/graphql/query/null_context.rb

@@ -4,7 +4,13 @@ module GraphQL
   class Query
     # This object can be `ctx` in places where there is no query
     class NullContext < Context
-      include Singleton
+      def self.instance
+        @instance ||= self.new
+      end
+
+      def self.instance=(new_inst)
+        @instance = new_inst
+      end
 
       class NullQuery
         def after_lazy(value)
@@ -20,10 +26,10 @@ class NullSchema < GraphQL::Schema
       attr_reader :schema, :query, :warden, :dataloader
       def_delegators GraphQL::EmptyObjects::EMPTY_HASH, :[], :fetch, :dig, :key?, :to_h
 
-      def initialize
+      def initialize(schema: NullSchema)
         @query = NullQuery.new
         @dataloader = GraphQL::Dataloader::NullDataloader.new
-        @schema = NullSchema
+        @schema = schema
         @warden = Schema::Warden::NullWarden.new(context: self, schema: @schema)
         @types = @warden.visibility_profile
         freeze

lib/graphql/schema.rb

@@ -330,10 +330,16 @@ def plugins
         find_inherited_value(:plugins, EMPTY_ARRAY) + own_plugins
       end
 
+      attr_writer :null_context
+
+      def null_context
+        @null_context || GraphQL::Query::NullContext.instance
+      end
+
       # Build a map of `{ name => type }` and return it
       # @return [Hash<String => Class>] A dictionary of type classes by their GraphQL name
       # @see get_type Which is more efficient for finding _one type_ by name, because it doesn't merge hashes.
-      def types(context = GraphQL::Query::NullContext.instance)
+      def types(context = null_context)
         if use_visibility_profile?
           types = Visibility::Profile.from_context(context, self)
           return types.all_types_h
@@ -366,7 +372,7 @@ def types(context = GraphQL::Query::NullContext.instance)
       # @param context [GraphQL::Query::Context] Used for filtering definitions at query-time
       # @param use_visibility_profile Private, for migration to {Schema::Visibility}
       # @return [Module, nil] A type, or nil if there's no type called `type_name`
-      def get_type(type_name, context = GraphQL::Query::NullContext.instance, use_visibility_profile = use_visibility_profile?)
+      def get_type(type_name, context = null_context, use_visibility_profile = use_visibility_profile?)
         if use_visibility_profile
           profile = Visibility::Profile.from_context(context, self)
           return profile.type(type_name)
@@ -617,7 +623,7 @@ def use_visibility_profile?
       # @param use_visibility_profile Private, for migration to {Schema::Visibility}
       # @return [Hash<String, Module>] All possible types, if no `type` is given.
       # @return [Array<Module>] Possible types for `type`, if it's given.
-      def possible_types(type = nil, context = GraphQL::Query::NullContext.instance, use_visibility_profile = use_visibility_profile?)
+      def possible_types(type = nil, context = null_context, use_visibility_profile = use_visibility_profile?)
         if use_visibility_profile
           if type
             return Visibility::Profile.from_context(context, self).possible_types(type)
@@ -701,7 +707,7 @@ def type_from_ast(ast_node, context: self.query_class.new(self, "{ __typename }"
         GraphQL::Schema::TypeExpression.build_type(context.query.types, ast_node)
       end
 
-      def get_field(type_or_name, field_name, context = GraphQL::Query::NullContext.instance, use_visibility_profile = use_visibility_profile?)
+      def get_field(type_or_name, field_name, context = null_context, use_visibility_profile = use_visibility_profile?)
         if use_visibility_profile
           profile = Visibility::Profile.from_context(context, self)
           parent_type = case type_or_name
@@ -738,7 +744,7 @@ def get_field(type_or_name, field_name, context = GraphQL::Query::NullContext.in
         end
       end
 
-      def get_fields(type, context = GraphQL::Query::NullContext.instance)
+      def get_fields(type, context = null_context)
         type.fields(context)
       end
 
@@ -1228,6 +1234,7 @@ def inherited(child_class)
           vis = self.visibility
           child_class.visibility = vis.dup_for(child_class)
         end
+        child_class.null_context = Query::NullContext.new(schema: child_class)
         super
       end
 

lib/graphql/schema/visibility.rb

@@ -191,7 +191,7 @@ def top_level_profile(refresh: false)
         if refresh
           @top_level_profile = nil
         end
-        @top_level_profile ||= @schema.visibility_profile_class.new(context: Query::NullContext.instance, schema: @schema, visibility: self)
+        @top_level_profile ||= @schema.visibility_profile_class.new(context: @schema.null_context, schema: @schema, visibility: self)
       end
 
       private

lib/graphql/subscriptions.rb

@@ -80,7 +80,7 @@ def trigger(event_name, args, object, scope: nil, context: {})
 
       # Normalize symbol-keyed args to strings, try camelizing them
       # Should this accept a real context somehow?
-      normalized_args = normalize_arguments(normalized_event_name, field, args, GraphQL::Query::NullContext.instance)
+      normalized_args = normalize_arguments(normalized_event_name, field, args, @schema.null_context)
 
       event = Subscriptions::Event.new(
         name: normalized_event_name,

spec/graphql/dataloader/null_dataloader_spec.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+require "spec_helper"
+
+describe "GraphQL NullDataloader" do
+  it "can run_isolated with previously-captured blocks that register lazies" do
+    dl = GraphQL::Dataloader::NullDataloader.new
+    result = 0
+    dl.run_isolated {
+      lazy = GraphQL::Execution::Lazy.new { result = 100 }
+      dl.lazy_at_depth(1, lazy)
+    }
+    assert_equal 100, result
+  end
+end

spec/spec_helper.rb

@@ -49,7 +49,7 @@
 end
 
 # C methods aren't fair game in non-main Ractors
-RUN_RACTOR_TESTS = (defined?(::Ractor) && !USING_C_PARSER && !ENV["TEST"])
+RUN_RACTOR_TESTS = (defined?(::Ractor) && !USING_C_PARSER && (ENV["TEST"].nil? || ENV["TEST"].include?("ractor_shareable")))
 
 require "rake"
 require "graphql/rake_task"