Allow loaders to be used without an executor (#35)

Promise#source is used to sync the loaders promises

Author: dylanahsmith

lib/graphql/batch/execution_strategy.rb

@@ -1,14 +1,18 @@
 module GraphQL::Batch
   class ExecutionStrategy < GraphQL::Query::SerialExecution
     def execute(_, _, query)
-      Promise.sync(as_promise_unless_resolved(super))
+      deep_sync(super)
     rescue GraphQL::InvalidNullError => err
       err.parent_error? || query.context.errors.push(err)
       nil
     ensure
       GraphQL::Batch::Executor.current.clear
     end
 
+    def deep_sync(result)
+      Promise.sync(as_promise_unless_resolved(result))
+    end
+
     private
 
     def as_promise_unless_resolved(result)
@@ -21,7 +25,7 @@ def as_promise_unless_resolved(result)
         end
       end
       return result if all_promises.empty?
-      Promise.all(all_promises).then { result }
+      ::Promise.all(all_promises).then { result }
     end
 
     def each_promise(obj, &block)

lib/graphql/batch/executor.rb

@@ -19,19 +19,16 @@ def initialize
       @loading = false
     end
 
+    def resolve(loader)
+      with_loading(true) { loader.resolve }
+    end
+
     def shift
       @loaders.shift.last
     end
 
     def tick
-      with_loading(true) { shift.resolve }
-    end
-
-    def wait(promise)
-      tick while promise.pending? && !loaders.empty?
-      if promise.pending?
-        promise.reject(::Promise::BrokenError.new("Promise wasn't fulfilled after all queries were loaded"))
-      end
+      resolve(shift)
     end
 
     def wait_all

lib/graphql/batch/loader.rb

@@ -2,8 +2,10 @@ module GraphQL::Batch
   class Loader
     def self.for(*group_args)
       loader_key = [self].concat(group_args)
-      Executor.current.loaders[loader_key] ||= new(*group_args).tap do |loader|
+      executor = Executor.current
+      executor.loaders[loader_key] ||= new(*group_args).tap do |loader|
         loader.loader_key = loader_key
+        loader.executor = executor
       end
     end
 
@@ -15,21 +17,17 @@ def self.load_many(keys)
       self.for.load_many(keys)
     end
 
-    attr_accessor :loader_key
+    attr_accessor :loader_key, :executor
 
     def load(key)
-      loader = Executor.current.loaders[loader_key] ||= self
-      if loader != self
-        raise "load called on loader that wasn't registered with executor"
-      end
       cache[cache_key(key)] ||= begin
         queue << key
-        Promise.new
+        Promise.new.tap { |promise| promise.source = self }
       end
     end
 
     def load_many(keys)
-      Promise.all(keys.map { |key| load(key) })
+      ::Promise.all(keys.map { |key| load(key) })
     end
 
     def resolve #:nodoc:
@@ -44,6 +42,16 @@ def resolve #:nodoc:
       end
     end
 
+    # For Promise#sync
+    def wait #:nodoc:
+      if executor
+        executor.loaders.delete(loader_key)
+        executor.resolve(self)
+      else
+        resolve
+      end
+    end
+
     protected
 
     # Fulfill the key with provided value, for use in #perform

lib/graphql/batch/mutation_execution_strategy.rb

@@ -4,17 +4,14 @@ class MutationExecutionStrategy < GraphQL::Batch::ExecutionStrategy
 
     class FieldResolution < GraphQL::Batch::ExecutionStrategy::FieldResolution
       def get_finished_value(raw_value)
-        return super if execution_context.strategy.enable_batching
+        strategy = execution_context.strategy
+        return super if strategy.enable_batching
 
-        raw_value = Promise.sync(raw_value)
-
-        execution_context.strategy.enable_batching = true
         begin
-          result = super(raw_value)
-          GraphQL::Batch::Executor.current.wait_all
-          result
+          strategy.enable_batching = true
+          strategy.deep_sync(Promise.sync(super))
         ensure
-          execution_context.strategy.enable_batching = false
+          strategy.enable_batching = false
         end
       end
     end

lib/graphql/batch/promise.rb

@@ -1,11 +1,8 @@
 module GraphQL::Batch
   class Promise < ::Promise
-    def wait
-      Executor.current.wait(self)
-    end
-
     def defer
-      Executor.current.defer { super }
+      executor = Executor.current
+      executor ? executor.defer { super } : super
     end
   end
 end

test/loader_test.rb

@@ -79,9 +79,10 @@ def test_then
 
   def test_then_error
     query = GroupCountLoader.for("single").load(:a).then { raise "oops" }
-    assert_raises(RuntimeError, 'oops') do
+    err = assert_raises(RuntimeError) do
       query.sync
     end
+    assert_equal 'oops', err.message
   end
 
   def test_on_reject_without_error
@@ -97,13 +98,6 @@ def test_query_in_callback
     assert_equal 5, EchoLoader.load(4).then { |value| EchoLoader.load(value + 1) }.sync
   end
 
-  def test_broken_promise_executor_check
-    promise = GraphQL::Batch::Promise.new
-    promise.wait
-    assert_equal GraphQL::Batch::BrokenPromiseError, promise.reason.class
-    assert_equal "Promise wasn't fulfilled after all queries were loaded", promise.reason.message
-  end
-
   def test_broken_promise_loader_check
     promise = BrokenLoader.load(1)
     promise.wait
@@ -131,15 +125,12 @@ def test_load_on_different_loaders
     loader = EchoLoader.for
     assert_equal :a, loader.load(:a).sync
     loader2 = EchoLoader.for
-    promise = loader2.load(:b)
 
-    err = assert_raises(RuntimeError) do
-      loader.load(:c)
-    end
+    promise = loader2.load(:b)
+    promise2 = loader.load(:c)
 
-    assert_equal "load called on loader that wasn't registered with executor", err.message
     assert_equal :b, promise.sync
-    assert_equal :c, loader.load(:c).sync
+    assert_equal :c, promise2.sync
   end
 
   def test_derived_cache_key
@@ -150,4 +141,15 @@ def test_loader_for_without_load
     loader = EchoLoader.for
     GraphQL::Batch::Executor.current.wait_all
   end
+
+  def test_loader_without_executor
+    loader1 = GroupCountLoader.new('one')
+    loader2 = GroupCountLoader.new('two')
+    group = Promise.all([
+      loader2.load(:a),
+      loader1.load(:a),
+      loader2.load(:b),
+    ])
+    assert_equal [2, 1, 2], group.sync
+  end
 end