diff --git a/.gitignore b/.gitignore index 0cb6eeb..76d24ab 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ /pkg/ /spec/reports/ /tmp/ +*.db diff --git a/.ruby-version b/.ruby-version index cd57a8b..282895a 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.1.5 +3.0.3 \ No newline at end of file diff --git a/README.md b/README.md index 92ef554..5777835 100644 --- a/README.md +++ b/README.md @@ -28,10 +28,61 @@ First, enable preloading in your `GraphQL::Schema`: Schema = GraphQL::Schema.define do use GraphQL::Batch - enable_preloading + enable_preloading # Only use this line if using the deprecated `Field Instrument` interface. end ``` +There are two approaches available to config the caching for the field. +Using currently supported [GraphQL gem](https://graphql-ruby.org/) `Field Extension` interface or `Field Instrument` interface +which is no longer supported as of `graphql 1.10.0`. + + +#### Field Extension (Preferred): + +```ruby +PostType = GraphQL::ObjectType.define do + name 'Post' + + field :comments, [CommentType], null: false do + # Post.includes(:comments) + extension GraphQL::Preload::FieldExtension, preload: :comments + + # Post.includes(:comments, :authors) + extension GraphQL::Preload::FieldExtension, preload: [:comments, :authors] + + # Post.includes(:comments, authors: [:followers, :posts]) + extension GraphQL::Preload::FieldExtension, preload: [:comments, { authors: [:followers, :posts] }] + + resolve ->(obj, args, ctx) { obj.comments } + end +end +``` + +### `preload_scope` +Starting with Rails 4.1, you can scope your preloaded records by passing a valid scope to [`ActiveRecord::Associations::Preloader`](https://apidock.com/rails/v4.1.8/ActiveRecord/Associations/Preloader/preload). Scoping can improve performance by reducing the number of models to be instantiated and can help with certain business goals (e.g., only returning records that have not been soft deleted). + +This functionality is surfaced through the `preload_scope` option: + +```ruby +PostType = GraphQL::ObjectType.define do + name 'Post' + + field :comments, [CommentType], null: false do + extension GraphQL::Preload::FieldExtension, { preload: :comments, preload_scope: ->(args, ctx) { Comment.where(deleted_at: nil) } } + + # Resolves with records returned from the following query: + # SELECT "comments".* + # FROM "comments" + # WHERE "comments"."deleted_at" IS NULL + # AND "comments"."post_id" IN (1, 2, 3) + resolve ->(obj, args, ctx) { obj.comments } + end +end +``` + + +#### Field Instrument (Deprecated): + Call `preload` when defining your field: ```ruby diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index c2bd62d..143047b 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -21,14 +21,15 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] - spec.add_runtime_dependency 'activerecord', '>= 4.1', '< 6' - spec.add_runtime_dependency 'graphql', '>= 1.8', '< 2' - spec.add_runtime_dependency 'graphql-batch', '~> 0.3' + spec.add_runtime_dependency 'activerecord', '>= 4.1', '< 7.1' + spec.add_runtime_dependency 'graphql', '>= 1.9', '~> 2.0' + spec.add_runtime_dependency 'graphql-batch', '~> 0.4' spec.add_runtime_dependency 'promise.rb', '~> 0.7' - spec.add_development_dependency 'bundler', '~> 1.16' + spec.add_development_dependency 'bundler', '~> 2.1' spec.add_development_dependency 'minitest', '~> 5.0' - spec.add_development_dependency 'pry', '~> 0.10' + spec.add_development_dependency 'pry-byebug', '~> 3.7' spec.add_development_dependency 'rake', '~> 10.0' + spec.add_development_dependency 'sqlite3', '~> 1.4' spec.add_development_dependency 'yard', '~> 0.9' end diff --git a/lib/graphql/preload.rb b/lib/graphql/preload.rb index b268cb7..5c94f21 100644 --- a/lib/graphql/preload.rb +++ b/lib/graphql/preload.rb @@ -1,25 +1,14 @@ +# frozen_string_literal: true + require 'graphql' require 'graphql/batch' require 'promise.rb' -GraphQL::Field.accepts_definitions( - preload: ->(type, *args) do - type.metadata[:preload] ||= [] - type.metadata[:preload].concat(args) - end, - preload_scope: ->(type, arg) { type.metadata[:preload_scope] = arg } -) - -GraphQL::Schema.accepts_definitions( - enable_preloading: ->(schema) do - schema.instrument(:field, GraphQL::Preload::Instrument.new) - end -) - module GraphQL # Provides a GraphQL::Field definition to preload ActiveRecord::Associations module Preload autoload :Instrument, 'graphql/preload/instrument' + autoload :FieldExtension, 'graphql/preload/field_extension' autoload :Loader, 'graphql/preload/loader' autoload :VERSION, 'graphql/preload/version' @@ -30,21 +19,24 @@ def enable_preloading end module FieldMetadata + attr_reader :preload + attr_reader :preload_scope + def initialize(*args, preload: nil, preload_scope: nil, **kwargs, &block) if preload @preload ||= [] @preload.concat Array.wrap preload end - if preload_scope - @preload_scope = preload_scope - end + + @preload_scope = preload_scope if preload_scope + super(*args, **kwargs, &block) end def to_graphql field_defn = super - field_defn.metadata[:preload] = @preload - field_defn.metadata[:preload_scope] = @preload_scope + field_defn.metadata[:preload] = @preload if defined?(@preload) && @preload + field_defn.metadata[:preload_scope] = @preload_scope if defined?(@preload_scope) && @preload_scope field_defn end end diff --git a/lib/graphql/preload/field_extension.rb b/lib/graphql/preload/field_extension.rb new file mode 100644 index 0000000..b63aaac --- /dev/null +++ b/lib/graphql/preload/field_extension.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'graphql/preload/field_preloader' + +module GraphQL + module Preload + class FieldExtension < GraphQL::Schema::FieldExtension + include FieldPreloader + + def resolve(object:, arguments:, context:) + yield(object, arguments) unless object + + scope = options[:preload_scope].call(arguments, context) if options[:preload_scope] + + record = object.is_a?(GraphQL::Schema::Object) && object.respond_to?(:object) ? object.object : object + + preload(record, options[:preload], scope).then do + yield(object, arguments) + end + end + end + end +end diff --git a/lib/graphql/preload/field_preloader.rb b/lib/graphql/preload/field_preloader.rb new file mode 100644 index 0000000..9b9a50e --- /dev/null +++ b/lib/graphql/preload/field_preloader.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module GraphQL + module Preload + module FieldPreloader + private + + def preload(record, associations, scope) + if associations.is_a?(String) + raise TypeError, "Expected #{associations} to be a Symbol, not a String" + elsif associations.is_a?(Symbol) + return preload_single_association(record, associations, scope) + end + + promises = [] + + Array.wrap(associations).each do |association| + case association + when Symbol + promises << preload_single_association(record, association, scope) + when Array + association.each do |sub_association| + promises << preload(record, sub_association, scope) + end + when Hash + association.each do |sub_association, nested_association| + promises << preload_single_association(record, sub_association, scope).then do + associated_records = record.public_send(sub_association) + + case associated_records + when ActiveRecord::Base + preload(associated_records, nested_association, scope) + else + Promise.all( + Array.wrap(associated_records).map do |associated_record| + preload(associated_record, nested_association, scope) + end + ) + end + end + end + end + end + + Promise.all(promises) + end + + def preload_single_association(record, association, scope) + # We would like to pass the `scope` (which is an `ActiveRecord::Relation`), + # directly into `Loader.for`. However, because the scope is + # created for each parent record, they are different objects and + # return different loaders, breaking batching. + # Therefore, we pass in `scope.to_sql`, which is the same for all the + # scopes and set the `scope` using an accessor. The actual scope + # object used will be the last one, which shouldn't make any difference, + # because even though they are different objects, they are all + # functionally equivalent. + loader = GraphQL::Preload::Loader.for(record.class, association, scope.try(:to_sql)) + loader.scope = scope + loader.load(record) + end + end + end +end diff --git a/lib/graphql/preload/instrument.rb b/lib/graphql/preload/instrument.rb index cd43e5b..f08c17f 100644 --- a/lib/graphql/preload/instrument.rb +++ b/lib/graphql/preload/instrument.rb @@ -1,98 +1,35 @@ +# frozen_string_literal: true + +require 'graphql/preload/field_preloader' + module GraphQL module Preload - # Provides an instrument for the GraphQL::Field :preload definition class Instrument - def instrument(_type, field) - metadata = merged_metadata(field) - return field if metadata.fetch(:preload, nil).nil? - - old_resolver = field.resolve_proc - new_resolver = ->(obj, args, ctx) do - return old_resolver.call(obj, args, ctx) unless obj - - if metadata[:preload_scope] - scope = metadata[:preload_scope].call(args, ctx) - end - - is_graphql_object = obj.is_a?(GraphQL::Schema::Object) - respond_to_object = obj.respond_to?(:object) - record = is_graphql_object && respond_to_object ? obj.object : obj - - preload(record, metadata[:preload], scope).then do - old_resolver.call(obj, args, ctx) - end - end - - field.redefine do - resolve(new_resolver) - end - end + include FieldPreloader - private def preload(record, associations, scope) - if associations.is_a?(String) - raise TypeError, "Expected #{associations} to be a Symbol, not a String" - elsif associations.is_a?(Symbol) - return preload_single_association(record, associations, scope) - end + def instrument(_type, field) + return field unless field.metadata.include?(:preload) - promises = [] + if defined?(FieldExtension) && (type_class = field.metadata[:type_class]) + type_class.extension(FieldExtension) + field + else + old_resolver = field.resolve_proc + new_resolver = lambda do |obj, args, ctx| + return old_resolver.call(obj, args, ctx) unless obj - Array.wrap(associations).each do |association| - case association - when Symbol - promises << preload_single_association(record, association, scope) - when Array - association.each do |sub_association| - promises << preload(record, sub_association, scope) - end - when Hash - association.each do |sub_association, nested_association| - promises << preload_single_association(record, sub_association, scope).then do - associated_records = record.public_send(sub_association) + scope = field.metadata[:preload_scope].call(args, ctx) if field.metadata[:preload_scope] - case associated_records - when ActiveRecord::Base - preload(associated_records, nested_association, scope) - else - Promise.all( - Array.wrap(associated_records).map do |associated_record| - preload(associated_record, nested_association, scope) - end - ) - end - end + preload(obj.object, field.metadata[:preload], scope).then do + old_resolver.call(obj, args, ctx) end end - end - - Promise.all(promises) - end - private def preload_single_association(record, association, scope) - # We would like to pass the `scope` (which is an `ActiveRecord::Relation`), - # directly into `Loader.for`. However, because the scope is - # created for each parent record, they are different objects and - # return different loaders, breaking batching. - # Therefore, we pass in `scope.to_sql`, which is the same for all the - # scopes and set the `scope` using an accessor. The actual scope - # object used will be the last one, which shouldn't make any difference, - # because even though they are different objects, they are all - # functionally equivalent. - loader = GraphQL::Preload::Loader.for(record.class, association, scope.try(:to_sql)) - loader.scope = scope - loader.load(record) - end - - private def merged_metadata(field) - type_class = field.metadata.fetch(:type_class, nil) - - if type_class.nil? || !type_class.respond_to?(:to_graphql) - field.metadata - else - field.metadata.merge(type_class.to_graphql.metadata) + field.redefine do + resolve(new_resolver) + end end end - end end end diff --git a/lib/graphql/preload/loader.rb b/lib/graphql/preload/loader.rb index 08182e3..15a98f3 100644 --- a/lib/graphql/preload/loader.rb +++ b/lib/graphql/preload/loader.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module GraphQL module Preload # Preloads ActiveRecord::Associations when called from the Preload::Instrument @@ -22,6 +24,7 @@ def load(record) end return Promise.resolve(record) if association_loaded?(record) + super end @@ -30,31 +33,47 @@ def perform(records) records.each { |record| fulfill(record, record) } end - private def association_loaded?(record) + private + + def association_loaded?(record) record.association(association).loaded? end private def preload_association(records) - ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope) + preloader = ActiveRecord::Associations::Preloader.new(records: records, associations: association, scope: preload_scope) + return unless preload_scope + return if Gem::Version.new(ActiveRecord::VERSION::STRING) < Gem::Version.new("6.0.0") + + # See https://github.com/rails/rails/issues/36638 for details + # Solution adapted from https://gist.github.com/palkan/03eb5306a1a3e8addbe8df97a298a466 + if preloader.is_a?(::ActiveRecord::Associations::Preloader::AlreadyLoaded) + raise ArgumentError, + "Preloading association twice is not possible. " \ + "To resolve this add `preload #{association.inspect}` to the GraphQL field definition." + end + + # this commit changes the way preloader works with scopes + # https://github.com/rails/rails/commit/2847653869ffc1ff5139c46e520c72e26618c199#diff-3bba5f66eb1ed62bd5700872fcd6c632 + preloader.send(:owners).each do |owner| + preloader.send(:associate_records_to_owner, owner, preloader.records_by_owner[owner] || []) + end end - private def preload_scope + def preload_scope return nil unless scope + reflection = model.reflect_on_association(association) raise ArgumentError, 'Cannot specify preload_scope for polymorphic associations' if reflection.polymorphic? + scope if scope.try(:klass) == reflection.klass end - private def validate_association - unless association.is_a?(Symbol) - raise ArgumentError, 'Association must be a Symbol object' - end - - unless model < ActiveRecord::Base - raise ArgumentError, 'Model must be an ActiveRecord::Base descendant' - end + def validate_association + raise ArgumentError, 'Association must be a Symbol object' unless association.is_a?(Symbol) + raise ArgumentError, "Model #{model} must be an ActiveRecord::Base descendant" unless model < ActiveRecord::Base return if model.reflect_on_association(association) + raise TypeError, "Association :#{association} does not exist on #{model}" end end diff --git a/lib/graphql/preload/version.rb b/lib/graphql/preload/version.rb index 7e29b58..c61bbcb 100644 --- a/lib/graphql/preload/version.rb +++ b/lib/graphql/preload/version.rb @@ -1,5 +1,7 @@ +# frozen_string_literal: true + module GraphQL module Preload - VERSION = '2.1.0'.freeze + VERSION = '2.1.0' end end diff --git a/test/graphql/preload_test.rb b/test/graphql/preload_test.rb index 35280d3..95e11d2 100644 --- a/test/graphql/preload_test.rb +++ b/test/graphql/preload_test.rb @@ -1,9 +1,174 @@ +# frozen_string_literal: true + require 'test_helper' -module GraphQL - class PreloadTest < Minitest::Test - def test_that_it_has_a_version_number - refute_nil ::GraphQL::Preload::VERSION - end +class GraphQL::PreloadTest < Minitest::Test + attr_reader :queries + + def setup + @queries = [] + QueryNotifier.subscriber = ->(query) { @queries << query } + end + + def teardown + QueryNotifier.subscriber = nil + end + + def test_that_it_has_a_version_number + refute_nil ::GraphQL::Preload::VERSION + end + + def test_batched_association_preload + query_string = %| + { + products(first: 2) { + id + title + variants { + id + title + } + } + } + | + + result = Schema.execute(query_string) + + expected = { + "data" => { + "products" => [ + { + "id" => "1", + "title" => "Shirt", + "variants" => [ + { "id" => "1", "title" => "Red" }, + { "id" => "2", "title" => "Blue" } + ] + }, + { + "id" => "2", + "title" => "Pants", + "variants" => [ + { "id" => "4", "title" => "Small" }, + { "id" => "5", "title" => "Medium" }, + { "id" => "6", "title" => "Large" } + ] + } + ] + } + } + + assert_equal expected, result + assert_equal ["Product?limit=2", "Product/1,2/variants"], queries + end + + def test_batched_association_nested_preload + query_string = %| + { + products(first: 2) { + id + title + variantsNestedPreload { + id + title + } + } + } + | + + result = Schema.execute(query_string) + + expected = { + "data" => { + "products" => [ + { + "id" => "1", + "title" => "Shirt", + "variantsNestedPreload" => [ + { "id" => "1", "title" => "Red" }, + { "id" => "2", "title" => "Blue" } + ] + }, + { + "id" => "2", + "title" => "Pants", + "variantsNestedPreload" => [ + { "id" => "4", "title" => "Small" }, + { "id" => "5", "title" => "Medium" }, + { "id" => "6", "title" => "Large" } + ] + } + ] + } + } + + assert_equal expected, result + assert_equal ["Product?limit=2", "Product/1,2/variants", "ProductVariant/1,2,4,5,6/images"], queries + end + + def test_query_group_with_sub_queries + query_string = %| + { + product(id: "1") { + variants { + images { id, filename } + } + } + } + | + + result = Schema.execute(query_string) + + expected = { + "data" => { + "product" => { + "variants" => [{ + "images" => [ + { "id" => "4", "filename" => "red-shirt.jpg" } + ] + }, { + "images" => [ + { "id" => "5", "filename" => "blue-shirt.jpg" } + ] + }] + } + } + } + + assert_equal expected, result + assert_equal ["Product/1", "Product/1/variants", "ProductVariant/1,2/images"], queries + end + + def test_loader_reused_after_loading + query_string = %| + { + product(id: "2") { + variants { + id + product { + id + title + } + } + } + } + | + + result = Schema.execute(query_string) + + expected = { + "data" => { + "product" => { + "variants" => [ + { "id" => "4", "product" => { "id" => "2", "title" => "Pants" } }, + { "id" => "5", "product" => { "id" => "2", "title" => "Pants" } }, + { "id" => "6", "product" => { "id" => "2", "title" => "Pants" } } + ] + } + } + } + + assert_equal expected, result + assert_equal ["Product/2", "Product/2/variants"], queries end end diff --git a/test/support/db.rb b/test/support/db.rb new file mode 100644 index 0000000..325e491 --- /dev/null +++ b/test/support/db.rb @@ -0,0 +1,89 @@ +class QueryNotifier + class << self + attr_accessor :subscriber + + def call(query) + subscriber&.call(query) + end + end +end + +class Base < ActiveRecord::Base + self.abstract_class = true + + def self.first(count) + QueryNotifier.call("#{name}?limit=#{count}") + super + end + + def self.find(ids) + QueryNotifier.call("#{name}/#{Array(ids).join(',')}") + super + end + + def self.preload_association(owners, association) + QueryNotifier.call("#{name}/#{owners.map(&:id).join(',')}/#{association}") + super + end +end +ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: './_test_.db') + +ActiveRecord::Schema.define do + self.verbose = false + create_table :images, force: true do |t| + t.column :owner_id, :integer + t.column :owner_type, :string + t.column :filename, :string + end + + create_table :products, force: true do |t| + t.column :title, :string + t.column :image_id, :integer + end + + create_table :product_variants, force: true do |t| + t.column :title, :string + t.column :product_id, :integer + end +end + +class Image < Base; end + +class ProductVariant < Base + belongs_to :product + has_many :images, -> { where(owner_type: 'ProductVariant') }, foreign_key: :owner_id +end + +class Product < Base + has_many :images, -> { where(owner_type: 'Product') }, foreign_key: :owner_id + has_many :variants, class_name: 'ProductVariant' +end + +Product.create(id: 1, title: 'Shirt', image_id: 1) +Product.create(id: 2, title: 'Pants', image_id: 2) +Product.create(id: 3, title: 'Sweater', image_id: 3) + +ProductVariant.create(id: 1, product_id: 1, title: 'Red') +ProductVariant.create(id: 2, product_id: 1, title: 'Blue') +ProductVariant.create(id: 4, product_id: 2, title: 'Small') +ProductVariant.create(id: 5, product_id: 2, title: 'Medium') +ProductVariant.create(id: 6, product_id: 2, title: 'Large') +ProductVariant.create(id: 7, product_id: 3, title: 'Default') + +Image.create(id: 1, owner_type: 'Product', owner_id: 1, filename: 'shirt.jpg') +Image.create(id: 2, owner_type: 'Product', owner_id: 2, filename: 'pants.jpg') +Image.create(id: 3, owner_type: 'Product', owner_id: 3, filename: 'sweater.jpg') +Image.create(id: 4, owner_type: 'ProductVariant', owner_id: 1, filename: 'red-shirt.jpg') +Image.create(id: 5, owner_type: 'ProductVariant', owner_id: 2, filename: 'blue-shirt.jpg') +Image.create(id: 6, owner_type: 'ProductVariant', owner_id: 3, filename: 'small-pants.jpg') + +module LoaderExtensions + private + + def preload_association(records) + QueryNotifier.call("#{model.name}/#{records.map(&:id).join(',')}/#{association}") + super + end +end + +GraphQL::Preload::Loader.prepend(LoaderExtensions) diff --git a/test/support/schema.rb b/test/support/schema.rb new file mode 100644 index 0000000..9860cb7 --- /dev/null +++ b/test/support/schema.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +class ImageType < GraphQL::Schema::Object + field :id, ID, null: false + field :filename, String, null: false +end + +class ProductVariantType < GraphQL::Schema::Object + field :id, ID, null: false + field :title, String, null: false + field :images, [ImageType], null: true do + extension GraphQL::Preload::FieldExtension, preload: :images + end + field :product, GraphQL::Schema::LateBoundType.new('Product'), null: false +end + +class ProductType < GraphQL::Schema::Object + field :id, ID, null: false + field :title, String, null: false + field :image, ImageType, null: true do + extension GraphQL::Preload::FieldExtension, preload: :image + end + field :variants, [ProductVariantType], null: true do + extension GraphQL::Preload::FieldExtension, preload: :variants + end + field :variants_nested_preload, [ProductVariantType], null: true do + extension GraphQL::Preload::FieldExtension, preload: { variants: :images } + end + def variants_nested_preload + object.variants + end +end + +class QueryType < GraphQL::Schema::Object + graphql_name 'query' + + field :product, ProductType, null: true do + argument :id, ID, required: true + end + + def product(id:) + Product.find(id) + end + + field :products, [ProductType], null: true do + argument :first, Int, required: true + extension GraphQL::Preload::FieldExtension, preload_scope: ->(_args, _ctx) { Product.where(title: 'Shirt') } + end + + def products(first:) + Product.first(first) + end +end + +class Schema < GraphQL::Schema + query QueryType + + use GraphQL::Batch + enable_preloading +end diff --git a/test/test_helper.rb b/test/test_helper.rb index e25a6a0..c6656c0 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -1,4 +1,9 @@ $LOAD_PATH.unshift File.expand_path('../../lib', __FILE__) +require "pry-byebug" +require "active_record" require 'graphql/preload' +require_relative 'support/db' +require_relative 'support/schema' + require 'minitest/autorun'