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

Does not work across multiplexed queries #90

Open
TSMMark opened this issue Aug 6, 2018 · 5 comments
Open

Does not work across multiplexed queries #90

TSMMark opened this issue Aug 6, 2018 · 5 comments

Comments

@TSMMark
Copy link

TSMMark commented Aug 6, 2018

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
@dylanahsmith
Copy link
Contributor

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?

@TSMMark
Copy link
Author

TSMMark commented Aug 9, 2018

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

@TSMMark
Copy link
Author

TSMMark commented Aug 13, 2018

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

@guilherme
Copy link

guilherme commented Aug 20, 2018

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?

@dylanahsmith
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants