rmosolgo/graphql-ruby

Fields with lazy results aren't resolved inside directive context

qiffp opened this issue · 1 comments

qiffp commented

Describe the bug

If you have a field that is resolved lazily, e.g. using a GraphQL::Batch::Loader, and that field is selected in a query using a directive which applies to the field, the field will be resolved outside of the directive's scope (outside of the yield of the directive's resolve).

Say you're using a gem with a class method that's given a block and that method sets/reverts a class variable before/after the block is called. Then you write a directive that passes resolve's block into that gem's class method as its block. Given a lazy field that uses the class variable, that field will be resolved using the "after" value instead of the "during" value.

Versions

graphql version: 2.3.7
rails (or other framework): N/A
other applicable versions (graphql-batch, etc): graphql-batch 0.6.0

GraphQL schema

class Attributed
  class << self
    attr_reader :attr

    def with_attribute(attr)
      previous_attr = @attr
      @attr = attr

      yield
    ensure
      @attr = previous_attr
    end
  end
end

class TestLoader < GraphQL::Batch::Loader
  def perform(keys)
    keys.each { fulfill(_1, Attributed.attr) }
  end
end

class TestSchema < GraphQL::Schema
  class TestQuery < GraphQL::Schema::Object
    field :lazy_value, String
    def lazy_value
      TestLoader.for.load(nil)
    end

    field :value, String
    def value
      Attributed.attr
    end
  end

  class WithAttribute < GraphQL::Schema::Directive
    locations(GraphQL::Schema::Directive::QUERY)

    def self.resolve(*, &block)
      Attributed.with_attribute("red", &block)
    end
  end

  query(TestQuery)
  directives(WithAttribute)
  use(GraphQL::Batch)
end

GraphQL query

query @withAttribute {
  value
  lazyValue
}
{
  "data": {
    "value": "red",
    "lazyValue": nil
  }
}

Steps to reproduce

Run the query above using the provided schema and classes

Expected behavior

Both value and lazyValue resolve to red

Actual behavior

lazyValue resolves to nil because it's outside of the withAttribute directive scope when it gets resolved and so Attributed.with_attributed has reverted to its previous attr value.

rmosolgo commented

Hey, thanks for the detailed report. I agree this is how this should work. Unfortunately it doesn't, and it's not going to be easy.

The problem is that directives are .resolve'd inside run_eager, which is executing query fields that aren't lazy:

graphql-ruby/lib/graphql/execution/interpreter/runtime.rb

Line 86 in 516a2f6

call_method_on_directives(:resolve, runtime_object, root_operation.directives) do # execute query level directives

(At first, I thought putting GraphQL::Batch.batch { ... } inside def self.resolve would fix this problem, but it doesn't, because of that problem.)

And it won't be as easy as "just moving it up" to a higher spot in the call stack because of how GraphQL-Ruby does multiplexing. Given two queries like this:

q1 = "query Query1 @withAttribute { lazyValue}"
q2 = "query Query2 { lazyValue }"
r1, r2 = MySchema.multiplex([q1, q2])

We'd expect the first lazyValue to be run inside @withAttribute, but the second lazyValue not to have @withAttribute. But GraphQL-Ruby can't make that guarantee because it runs all lazy values inside the same pool.

It might be possible to split them out -- detect queries which have directives and run them separately. That's the technique GraphQL-Ruby uses for fields and fragments, but there isn't already a place to do it for queries.)

I'll keep this open while I'm thinking it over and follow up here when I have anything more to report.

  • ❤️1