From bb4c5eab52df82d370ccad82030888d2b02830c1 Mon Sep 17 00:00:00 2001 From: Filip Zachar Date: Mon, 21 Oct 2019 09:42:57 +0200 Subject: [PATCH 01/14] Adds support to preload decorated objects --- lib/graphql/preload/instrument.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/graphql/preload/instrument.rb b/lib/graphql/preload/instrument.rb index 454d9e1..a42e0d3 100644 --- a/lib/graphql/preload/instrument.rb +++ b/lib/graphql/preload/instrument.rb @@ -24,6 +24,7 @@ def instrument(_type, field) end private def preload(record, associations, scope) + record = record.to_model if record.respond_to?(:to_model) if associations.is_a?(String) raise TypeError, "Expected #{associations} to be a Symbol, not a String" elsif associations.is_a?(Symbol) From 5fe5d457da140e6343ae80e0dbd3341db71a5d9d Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Fri, 27 Mar 2020 17:20:30 -0500 Subject: [PATCH 02/14] Allow rails 6 --- graphql-preload.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index c2bd62d..1be29c7 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -21,7 +21,7 @@ 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 'activerecord', '>= 4.1', '< 7' spec.add_runtime_dependency 'graphql', '>= 1.8', '< 2' spec.add_runtime_dependency 'graphql-batch', '~> 0.3' spec.add_runtime_dependency 'promise.rb', '~> 0.7' From 13c02edc6f82f6afedbc7ecb90d4fdfb7d3114d8 Mon Sep 17 00:00:00 2001 From: Manuel Puyol Date: Fri, 27 Mar 2020 17:21:28 -0500 Subject: [PATCH 03/14] Change preload_associations --- lib/graphql/preload/loader.rb | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/graphql/preload/loader.rb b/lib/graphql/preload/loader.rb index 08182e3..582192c 100644 --- a/lib/graphql/preload/loader.rb +++ b/lib/graphql/preload/loader.rb @@ -35,7 +35,23 @@ def perform(records) end private def preload_association(records) - ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope) + preloader = ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope).first + 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 From e4d74e01d39eab3abfa20fda9deca4c3771868e8 Mon Sep 17 00:00:00 2001 From: Robert Korzeniec Date: Tue, 18 Aug 2020 13:31:13 +0200 Subject: [PATCH 04/14] update ruby and gems --- .ruby-version | 2 +- graphql-preload.gemspec | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.ruby-version b/.ruby-version index cd57a8b..ecd7ee5 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.1.5 +2.5.8 diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index c2bd62d..2599aa3 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -22,11 +22,11 @@ Gem::Specification.new do |spec| 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 'graphql', '>= 1.9', '< 2' + 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 'rake', '~> 10.0' From 6678d7f2f56b98ea4900f8c2e54ba0f09c01234c Mon Sep 17 00:00:00 2001 From: Robert Korzeniec Date: Tue, 18 Aug 2020 13:39:30 +0200 Subject: [PATCH 05/14] implement field extensions --- .gitignore | 1 + graphql-preload.gemspec | 3 +- lib/graphql/preload.rb | 18 ++- lib/graphql/preload/field_extension.rb | 19 +++ lib/graphql/preload/field_preloader.rb | 62 +++++++++ lib/graphql/preload/instrument.rb | 101 +++------------ lib/graphql/preload/loader.rb | 23 ++-- test/graphql/preload_test.rb | 173 ++++++++++++++++++++++++- test/support/db.rb | 89 +++++++++++++ test/support/schema.rb | 61 +++++++++ test/test_helper.rb | 5 + 11 files changed, 448 insertions(+), 107 deletions(-) create mode 100644 lib/graphql/preload/field_extension.rb create mode 100644 lib/graphql/preload/field_preloader.rb create mode 100644 test/support/db.rb create mode 100644 test/support/schema.rb 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/graphql-preload.gemspec b/graphql-preload.gemspec index 2599aa3..99a9184 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -28,7 +28,8 @@ Gem::Specification.new do |spec| 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..f13c99d 100644 --- a/lib/graphql/preload.rb +++ b/lib/graphql/preload.rb @@ -3,7 +3,7 @@ require 'promise.rb' GraphQL::Field.accepts_definitions( - preload: ->(type, *args) do + preload: lambda do |type, *args| type.metadata[:preload] ||= [] type.metadata[:preload].concat(args) end, @@ -11,7 +11,7 @@ ) GraphQL::Schema.accepts_definitions( - enable_preloading: ->(schema) do + enable_preloading: lambda do |schema| schema.instrument(:field, GraphQL::Preload::Instrument.new) end ) @@ -20,6 +20,7 @@ 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 +31,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 @preload + field_defn.metadata[:preload_scope] = @preload_scope if @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..498ff46 --- /dev/null +++ b/lib/graphql/preload/field_extension.rb @@ -0,0 +1,19 @@ +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 = field.preload_scope.call(arguments, context) if field.preload_scope + + preload(object.object, options, 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..0e5177e --- /dev/null +++ b/lib/graphql/preload/field_preloader.rb @@ -0,0 +1,62 @@ +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..fb42a00 100644 --- a/lib/graphql/preload/instrument.rb +++ b/lib/graphql/preload/instrument.rb @@ -1,98 +1,33 @@ +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? + include FieldPreloader - 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 - - 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..3594482 100644 --- a/lib/graphql/preload/loader.rb +++ b/lib/graphql/preload/loader.rb @@ -22,6 +22,7 @@ def load(record) end return Promise.resolve(record) if association_loaded?(record) + super end @@ -30,31 +31,31 @@ 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) + def preload_association(records) ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope) 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/test/graphql/preload_test.rb b/test/graphql/preload_test.rb index 35280d3..f543836 100644 --- a/test/graphql/preload_test.rb +++ b/test/graphql/preload_test.rb @@ -1,9 +1,172 @@ 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..66da28c --- /dev/null +++ b/test/support/schema.rb @@ -0,0 +1,61 @@ +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, :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, preload: :image + field :variants, [ProductVariantType], null: true do + extension GraphQL::Preload::FieldExtension, :variants + end + field :variants_nested_preload, [ProductVariantType], null: true do + extension GraphQL::Preload::FieldExtension, 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 + end + + def products(first:) + Product.first(first) + end +end + +class Schema < GraphQL::Schema + query QueryType + + if ENV['TESTING_INTERPRETER'] == 'true' + use GraphQL::Execution::Interpreter + # This probably has no effect, but just to get the full test: + use GraphQL::Analysis::AST + end + + 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' From 568b991f8bb70608663bab82b73d50922cca054a Mon Sep 17 00:00:00 2001 From: Robert Korzeniec Date: Thu, 27 Aug 2020 12:02:10 +0200 Subject: [PATCH 06/14] add some frozen string literal magic comments --- lib/graphql/preload.rb | 2 ++ lib/graphql/preload/field_extension.rb | 2 ++ lib/graphql/preload/field_preloader.rb | 2 ++ lib/graphql/preload/instrument.rb | 2 ++ lib/graphql/preload/loader.rb | 2 ++ lib/graphql/preload/version.rb | 4 +++- 6 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/graphql/preload.rb b/lib/graphql/preload.rb index f13c99d..9c63ac0 100644 --- a/lib/graphql/preload.rb +++ b/lib/graphql/preload.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'graphql' require 'graphql/batch' require 'promise.rb' diff --git a/lib/graphql/preload/field_extension.rb b/lib/graphql/preload/field_extension.rb index 498ff46..2bc03b8 100644 --- a/lib/graphql/preload/field_extension.rb +++ b/lib/graphql/preload/field_extension.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'graphql/preload/field_preloader' module GraphQL diff --git a/lib/graphql/preload/field_preloader.rb b/lib/graphql/preload/field_preloader.rb index 0e5177e..9b9a50e 100644 --- a/lib/graphql/preload/field_preloader.rb +++ b/lib/graphql/preload/field_preloader.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module GraphQL module Preload module FieldPreloader diff --git a/lib/graphql/preload/instrument.rb b/lib/graphql/preload/instrument.rb index fb42a00..f08c17f 100644 --- a/lib/graphql/preload/instrument.rb +++ b/lib/graphql/preload/instrument.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'graphql/preload/field_preloader' module GraphQL diff --git a/lib/graphql/preload/loader.rb b/lib/graphql/preload/loader.rb index 3594482..c066e9c 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 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 From b25df91fc9c7799ba59437b7c2855d68f5d2aaf8 Mon Sep 17 00:00:00 2001 From: Robert Korzeniec Date: Thu, 27 Aug 2020 15:31:24 +0200 Subject: [PATCH 07/14] fix passing preloading in an option to field extension Previously the `field.preload_scope` returned nil when using the field extensions, since field instruments are deprecated. This commit fixes that by checking for the preload scope in the options hash passed to the extension object. --- lib/graphql/preload.rb | 4 ++-- lib/graphql/preload/field_extension.rb | 4 ++-- test/graphql/preload_test.rb | 2 ++ test/support/schema.rb | 20 +++++++++++--------- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/graphql/preload.rb b/lib/graphql/preload.rb index 9c63ac0..912b645 100644 --- a/lib/graphql/preload.rb +++ b/lib/graphql/preload.rb @@ -49,8 +49,8 @@ def initialize(*args, preload: nil, preload_scope: nil, **kwargs, &block) def to_graphql field_defn = super - field_defn.metadata[:preload] = @preload if @preload - field_defn.metadata[:preload_scope] = @preload_scope if @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 index 2bc03b8..9bdd4b7 100644 --- a/lib/graphql/preload/field_extension.rb +++ b/lib/graphql/preload/field_extension.rb @@ -10,9 +10,9 @@ class FieldExtension < GraphQL::Schema::FieldExtension def resolve(object:, arguments:, context:) yield(object, arguments) unless object - scope = field.preload_scope.call(arguments, context) if field.preload_scope + scope = options[:preload_scope].call(arguments, context) if options[:preload_scope] - preload(object.object, options, scope).then do + preload(object.object, options[:preload], scope).then do yield(object, arguments) end end diff --git a/test/graphql/preload_test.rb b/test/graphql/preload_test.rb index f543836..95e11d2 100644 --- a/test/graphql/preload_test.rb +++ b/test/graphql/preload_test.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + require 'test_helper' class GraphQL::PreloadTest < Minitest::Test diff --git a/test/support/schema.rb b/test/support/schema.rb index 66da28c..5e92e6c 100644 --- a/test/support/schema.rb +++ b/test/support/schema.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + class ImageType < GraphQL::Schema::Object field :id, ID, null: false field :filename, String, null: false @@ -7,7 +9,7 @@ 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, :images + extension GraphQL::Preload::FieldExtension, preload: :images end field :product, GraphQL::Schema::LateBoundType.new('Product'), null: false end @@ -15,12 +17,14 @@ class ProductVariantType < GraphQL::Schema::Object class ProductType < GraphQL::Schema::Object field :id, ID, null: false field :title, String, null: false - field :image, ImageType, null: true, preload: :image + field :image, ImageType, null: true do + extension GraphQL::Preload::FieldExtension, preload: :image + end field :variants, [ProductVariantType], null: true do - extension GraphQL::Preload::FieldExtension, :variants + extension GraphQL::Preload::FieldExtension, preload: :variants end field :variants_nested_preload, [ProductVariantType], null: true do - extension GraphQL::Preload::FieldExtension, variants: :images + extension GraphQL::Preload::FieldExtension, preload: { variants: :images } end def variants_nested_preload object.variants @@ -40,6 +44,7 @@ def product(id:) 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:) @@ -50,11 +55,8 @@ def products(first:) class Schema < GraphQL::Schema query QueryType - if ENV['TESTING_INTERPRETER'] == 'true' - use GraphQL::Execution::Interpreter - # This probably has no effect, but just to get the full test: - use GraphQL::Analysis::AST - end + use GraphQL::Execution::Interpreter + use GraphQL::Analysis::AST use GraphQL::Batch enable_preloading From 6a3f07a1cf4e5c9025467c4400f08f27080bf221 Mon Sep 17 00:00:00 2001 From: Robert Korzeniec Date: Thu, 27 Aug 2020 16:09:17 +0200 Subject: [PATCH 08/14] update readme regarding field extensions --- README.md | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/README.md b/README.md index 92ef554..f310201 100644 --- a/README.md +++ b/README.md @@ -32,8 +32,14 @@ Schema = GraphQL::Schema.define do 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`. + Call `preload` when defining your field: +#### Field Instrument: + ```ruby PostType = GraphQL::ObjectType.define do name 'Post' @@ -76,6 +82,48 @@ PostType = GraphQL::ObjectType.define do end ``` +#### Field Extension: +```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 +``` + ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake test` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. From 2f202c471968e92fc6263840f62a326df0e632fe Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Thu, 20 Jan 2022 11:43:50 -0500 Subject: [PATCH 09/14] Add support for rails ~> 7.0.0 --- .ruby-version | 2 +- graphql-preload.gemspec | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ruby-version b/.ruby-version index cd57a8b..a603bb5 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.1.5 +2.7.5 diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index c2bd62d..1eaf731 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -21,7 +21,7 @@ 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 'activerecord', '>= 4.1', '< 7.1' spec.add_runtime_dependency 'graphql', '>= 1.8', '< 2' spec.add_runtime_dependency 'graphql-batch', '~> 0.3' spec.add_runtime_dependency 'promise.rb', '~> 0.7' From d119d7c5abdf319d4d384b832c8d1aefe79cd183 Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Tue, 5 Apr 2022 12:17:21 -0400 Subject: [PATCH 10/14] Add support for Rails 7 and Ruby 3 --- .ruby-version | 2 +- graphql-preload.gemspec | 2 +- lib/graphql/preload/field_extension.rb | 4 +++- lib/graphql/preload/field_preloader.rb | 2 +- lib/graphql/preload/loader.rb | 2 +- test/support/schema.rb | 3 --- 6 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.ruby-version b/.ruby-version index ecd7ee5..75a22a2 100644 --- a/.ruby-version +++ b/.ruby-version @@ -1 +1 @@ -2.5.8 +3.0.3 diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index 99a9184..aa5c9a5 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -21,7 +21,7 @@ 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 'activerecord', '>= 4.1', '< 7.1' spec.add_runtime_dependency 'graphql', '>= 1.9', '< 2' spec.add_runtime_dependency 'graphql-batch', '~> 0.4' spec.add_runtime_dependency 'promise.rb', '~> 0.7' diff --git a/lib/graphql/preload/field_extension.rb b/lib/graphql/preload/field_extension.rb index 9bdd4b7..b63aaac 100644 --- a/lib/graphql/preload/field_extension.rb +++ b/lib/graphql/preload/field_extension.rb @@ -12,7 +12,9 @@ def resolve(object:, arguments:, context:) scope = options[:preload_scope].call(arguments, context) if options[:preload_scope] - preload(object.object, options[:preload], scope).then do + 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 diff --git a/lib/graphql/preload/field_preloader.rb b/lib/graphql/preload/field_preloader.rb index 9b9a50e..e56d23d 100644 --- a/lib/graphql/preload/field_preloader.rb +++ b/lib/graphql/preload/field_preloader.rb @@ -55,7 +55,7 @@ def preload_single_association(record, association, 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 = GraphQL::Batch.batch{ GraphQL::Preload::Loader.for(record.class, association, scope.try(:to_sql)) } loader.scope = scope loader.load(record) end diff --git a/lib/graphql/preload/loader.rb b/lib/graphql/preload/loader.rb index c066e9c..0ca3094 100644 --- a/lib/graphql/preload/loader.rb +++ b/lib/graphql/preload/loader.rb @@ -40,7 +40,7 @@ def association_loaded?(record) end def preload_association(records) - ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope) + ActiveRecord::Associations::Preloader.new(records: records, associations: association, scope: preload_scope) end def preload_scope diff --git a/test/support/schema.rb b/test/support/schema.rb index 5e92e6c..9860cb7 100644 --- a/test/support/schema.rb +++ b/test/support/schema.rb @@ -55,9 +55,6 @@ def products(first:) class Schema < GraphQL::Schema query QueryType - use GraphQL::Execution::Interpreter - use GraphQL::Analysis::AST - use GraphQL::Batch enable_preloading end From 6cbec95e5d468e47e02ac3436586218b725c657a Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Tue, 5 Apr 2022 12:19:13 -0400 Subject: [PATCH 11/14] Remove unnecessary batch block --- lib/graphql/preload/field_preloader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphql/preload/field_preloader.rb b/lib/graphql/preload/field_preloader.rb index e56d23d..9b9a50e 100644 --- a/lib/graphql/preload/field_preloader.rb +++ b/lib/graphql/preload/field_preloader.rb @@ -55,7 +55,7 @@ def preload_single_association(record, association, 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::Batch.batch{ GraphQL::Preload::Loader.for(record.class, association, scope.try(:to_sql)) } + loader = GraphQL::Preload::Loader.for(record.class, association, scope.try(:to_sql)) loader.scope = scope loader.load(record) end From bd1ecef06e286b3601e8a6384c72c618cc160bf2 Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Tue, 5 Apr 2022 12:50:02 -0400 Subject: [PATCH 12/14] Update README to encourage using field extensions instead of instruments --- README.md | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index f310201..5777835 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,7 @@ 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 ``` @@ -36,23 +36,22 @@ 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`. -Call `preload` when defining your field: -#### Field Instrument: +#### Field Extension (Preferred): ```ruby PostType = GraphQL::ObjectType.define do name 'Post' - field :comments, !types[!CommentType] do + field :comments, [CommentType], null: false do # Post.includes(:comments) - preload :comments + extension GraphQL::Preload::FieldExtension, preload: :comments # Post.includes(:comments, :authors) - preload [:comments, :authors] + extension GraphQL::Preload::FieldExtension, preload: [:comments, :authors] # Post.includes(:comments, authors: [:followers, :posts]) - preload [:comments, { authors: [:followers, :posts] }] + extension GraphQL::Preload::FieldExtension, preload: [:comments, { authors: [:followers, :posts] }] resolve ->(obj, args, ctx) { obj.comments } end @@ -68,9 +67,8 @@ This functionality is surfaced through the `preload_scope` option: PostType = GraphQL::ObjectType.define do name 'Post' - field :comments, !types[!CommentType] do - preload :comments - preload_scope ->(args, ctx) { Comment.where(deleted_at: nil) } + 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".* @@ -82,20 +80,24 @@ PostType = GraphQL::ObjectType.define do end ``` -#### Field Extension: + +#### Field Instrument (Deprecated): + +Call `preload` when defining your field: + ```ruby PostType = GraphQL::ObjectType.define do name 'Post' - field :comments, [CommentType], null: false do + field :comments, !types[!CommentType] do # Post.includes(:comments) - extension GraphQL::Preload::FieldExtension, preload: :comments + preload :comments # Post.includes(:comments, :authors) - extension GraphQL::Preload::FieldExtension, preload: [:comments, :authors] + preload [:comments, :authors] # Post.includes(:comments, authors: [:followers, :posts]) - extension GraphQL::Preload::FieldExtension, preload: [:comments, { authors: [:followers, :posts] }] + preload [:comments, { authors: [:followers, :posts] }] resolve ->(obj, args, ctx) { obj.comments } end @@ -111,8 +113,9 @@ This functionality is surfaced through the `preload_scope` option: 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) } } + field :comments, !types[!CommentType] do + preload :comments + preload_scope ->(args, ctx) { Comment.where(deleted_at: nil) } # Resolves with records returned from the following query: # SELECT "comments".* From 093419ff7e173435e717f87d8e0b5f0a6b7df5d3 Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Tue, 5 Apr 2022 14:11:16 -0400 Subject: [PATCH 13/14] Update loader preloader syntax --- lib/graphql/preload/loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/graphql/preload/loader.rb b/lib/graphql/preload/loader.rb index 493a0ef..15a98f3 100644 --- a/lib/graphql/preload/loader.rb +++ b/lib/graphql/preload/loader.rb @@ -40,7 +40,7 @@ def association_loaded?(record) end private def preload_association(records) - preloader = ActiveRecord::Associations::Preloader.new.preload(records, association, preload_scope).first + 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") From d16e337eae31248d3db6aefc4b353ba320b8233d Mon Sep 17 00:00:00 2001 From: Joey Paris Date: Tue, 5 Apr 2022 16:00:10 -0400 Subject: [PATCH 14/14] Add support for Ruby GraphQL ~> 2.0 --- graphql-preload.gemspec | 2 +- lib/graphql/preload.rb | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/graphql-preload.gemspec b/graphql-preload.gemspec index aa5c9a5..143047b 100644 --- a/graphql-preload.gemspec +++ b/graphql-preload.gemspec @@ -22,7 +22,7 @@ Gem::Specification.new do |spec| spec.require_paths = ['lib'] spec.add_runtime_dependency 'activerecord', '>= 4.1', '< 7.1' - spec.add_runtime_dependency 'graphql', '>= 1.9', '< 2' + 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' diff --git a/lib/graphql/preload.rb b/lib/graphql/preload.rb index 912b645..5c94f21 100644 --- a/lib/graphql/preload.rb +++ b/lib/graphql/preload.rb @@ -4,20 +4,6 @@ require 'graphql/batch' require 'promise.rb' -GraphQL::Field.accepts_definitions( - preload: lambda do |type, *args| - type.metadata[:preload] ||= [] - type.metadata[:preload].concat(args) - end, - preload_scope: ->(type, arg) { type.metadata[:preload_scope] = arg } -) - -GraphQL::Schema.accepts_definitions( - enable_preloading: lambda do |schema| - schema.instrument(:field, GraphQL::Preload::Instrument.new) - end -) - module GraphQL # Provides a GraphQL::Field definition to preload ActiveRecord::Associations module Preload