graphql-batch icon indicating copy to clipboard operation
graphql-batch copied to clipboard

Does not work across multiplexed queries

Open TSMMark opened this issue 7 years ago • 5 comments

This library is fantastic and thanks for publishing it!

In my testing, IDs are not always aggregated across multiple queries when Schema#multiplex is used. The promises do resolve correctly, but one DB query is made per multiplexed gql query.

I found a test in this repo, which runs queries with multiplex and uses QueryNotifier.subscriber to assert that only 1 query is made, but my loader does not behave.

Current gem versions:

gem "graphql", "=1.6.0"
gem "graphql-batch", "=0.3.9"

The loader in question is very simple:

module Loaders
  class Single < ::GraphQL::Batch::Loader

    attr_reader :model, :options, :key

    def initialize(model_arg, options_arg = {})
      @model = model_arg
      @options = options_arg
      @key = options.fetch(:key, :id)
    end

    def perform(ids)
      model.where(key => ids).all.each(&method(:fulfill_record))
      ids.reject(&method(:fulfilled?)).each(&method(:fulfill_nil))
    end

    private

    def fulfill_record(record)
      fulfill(record.send(key), record)
    end

    def fulfill_nil(id)
      fulfill(id, nil)
    end

  end
end

TSMMark avatar Aug 06 '18 00:08 TSMMark

The loader itself doesn't give enough to reproduce this problem, especially when there is already a test that covers a basic use case. Could you try to provide a minimal example schema and query so that we can reproduce the bug?

dylanahsmith avatar Aug 09 '18 13:08 dylanahsmith

Yes I'll post here as soon as I get a minimal repro

TSMMark avatar Aug 09 '18 16:08 TSMMark

Unable to reproduce the issue using the test suite in this project. Going to have to punt on this for a while until I can dedicate more time

TSMMark avatar Aug 13 '18 18:08 TSMMark

on the same topic: is there any special cares that you have to do in order to use this library with multiplexed queries ? things like: worry about thread-safety in the code? database adapters that aren't supported? or should it just work and that's it, don't worry about this stuff?

guilherme avatar Aug 20 '18 03:08 guilherme

graphql-batch is database independent. Query multiplexing happens in the same thread, so it shouldn't cause new thread-safety issues. If it doesn't just work, then try to isolate the issue to report the issue to the appropriate project with instructions on how to reproduce the issue.

dylanahsmith avatar Aug 20 '18 13:08 dylanahsmith