From 4719a8f30098b85c6152c07b91a59902fd9e3fde Mon Sep 17 00:00:00 2001 From: ArthurZaharov Date: Mon, 22 May 2023 19:20:06 +0300 Subject: [PATCH 1/3] Use data loader --- Gemfile | 3 +- Gemfile.lock | 9 +--- app/graphql/application_schema.rb | 4 +- app/graphql/loaders/association_loader.rb | 51 ------------------- app/graphql/loaders/record_loader.rb | 14 ----- app/graphql/sources/active_record_object.rb | 10 ++++ app/graphql/types/activity_type.rb | 2 +- app/graphql/types/public_activity_type.rb | 5 ++ .../types/activity_type_n_plus_one_spec.rb | 10 ++-- .../succeed_graphql_request.rb | 7 +++ 10 files changed, 34 insertions(+), 81 deletions(-) delete mode 100644 app/graphql/loaders/association_loader.rb delete mode 100644 app/graphql/loaders/record_loader.rb create mode 100644 app/graphql/sources/active_record_object.rb create mode 100644 spec/support/shared_examples/succeed_graphql_request.rb diff --git a/Gemfile b/Gemfile index c4fb3ae..c137524 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,6 @@ gem "bootsnap", require: false gem "enumerize" gem "google-api-client" gem "graphql" -gem "graphql-batch" gem "graphql-persisted_queries" gem "graphql-rails_logger" gem "health_check" @@ -51,7 +50,7 @@ end group :test do gem "database_cleaner-active_record" - gem "email_spec" + gem "email_spec", "2.2.0" gem "n_plus_one_control" gem "simplecov", require: false end diff --git a/Gemfile.lock b/Gemfile.lock index 37b448f..e8cd972 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -118,7 +118,7 @@ GEM railties (>= 3.2) down (5.4.0) addressable (~> 2.8) - email_spec (2.2.1) + email_spec (2.2.0) htmlentities (~> 4.3.3) launchy (~> 2.1) mail (~> 2.7) @@ -167,9 +167,6 @@ GEM os (>= 0.9, < 2.0) signet (>= 0.16, < 2.a) graphql (2.0.22) - graphql-batch (0.5.3) - graphql (>= 1.12.18, < 3) - promise.rb (~> 0.7.2) graphql-persisted_queries (1.7.0) graphql (>= 1.12) graphql-rails_logger (1.2.4) @@ -234,7 +231,6 @@ GEM parser (3.2.2.1) ast (~> 2.4.1) pg (1.5.3) - promise.rb (0.7.4) public_suffix (5.0.1) puma (6.2.2) nio4r (~> 2.0) @@ -386,13 +382,12 @@ DEPENDENCIES byebug database_cleaner-active_record dotenv-rails - email_spec + email_spec (= 2.2.0) enumerize factory_bot_rails ffaker google-api-client graphql - graphql-batch graphql-persisted_queries graphql-rails_logger health_check diff --git a/app/graphql/application_schema.rb b/app/graphql/application_schema.rb index 018a71e..035a48a 100644 --- a/app/graphql/application_schema.rb +++ b/app/graphql/application_schema.rb @@ -1,10 +1,8 @@ -require "graphql/batch" - class ApplicationSchema < GraphQL::Schema mutation(Types::MutationType) query(Types::QueryType) - use GraphQL::Batch + use GraphQL::Dataloader use GraphQL::PersistedQueries, compiled_queries: true, store: :redis_with_local_cache, diff --git a/app/graphql/loaders/association_loader.rb b/app/graphql/loaders/association_loader.rb deleted file mode 100644 index 3b9f81d..0000000 --- a/app/graphql/loaders/association_loader.rb +++ /dev/null @@ -1,51 +0,0 @@ -module Loaders - class AssociationLoader < GraphQL::Batch::Loader - def self.validate(model, association_name) - new(model, association_name) - nil - end - - def initialize(model, association_name) - super - @model = model - @association_name = association_name - validate - end - - def load(record) - raise TypeError, "#{@model} loader can't load association for #{record.class}" unless record.is_a?(@model) - return Promise.resolve(read_association(record)) if association_loaded?(record) - - super - end - - def cache_key(record) - record.object_id - end - - def perform(records) - preload_association(records) - records.each { |record| fulfill(record, read_association(record)) } - end - - private - - def validate - return if @model.reflect_on_association(@association_name) - - raise ArgumentError, "No association #{@association_name} on #{@model}" - end - - def preload_association(records) - ::ActiveRecord::Associations::Preloader.new.preload(records, @association_name) - end - - def read_association(record) - record.public_send(@association_name) - end - - def association_loaded?(record) - record.association(@association_name).loaded? - end - end -end diff --git a/app/graphql/loaders/record_loader.rb b/app/graphql/loaders/record_loader.rb deleted file mode 100644 index 02de7c9..0000000 --- a/app/graphql/loaders/record_loader.rb +++ /dev/null @@ -1,14 +0,0 @@ -module Loaders - class RecordLoader < GraphQL::Batch::Loader - def initialize(model) - super() - - @model = model - end - - def perform(ids) - @model.where(id: ids).each { |record| fulfill(record.id, record) } - ids.each { |id| fulfill(id, nil) unless fulfilled?(id) } - end - end -end diff --git a/app/graphql/sources/active_record_object.rb b/app/graphql/sources/active_record_object.rb new file mode 100644 index 0000000..db29c7b --- /dev/null +++ b/app/graphql/sources/active_record_object.rb @@ -0,0 +1,10 @@ +class Sources::ActiveRecordObject < GraphQL::Dataloader::Source + def initialize(model_class) + @model_class = model_class + end + + def fetch(ids) + records = @model_class.where(id: ids) + ids.map { |id| records.find { |r| r.id == id.to_i } } + end +end diff --git a/app/graphql/types/activity_type.rb b/app/graphql/types/activity_type.rb index 5fbd6d2..fcefd9e 100644 --- a/app/graphql/types/activity_type.rb +++ b/app/graphql/types/activity_type.rb @@ -11,7 +11,7 @@ class ActivityType < Types::BaseObject field :user, Types::UserType, "Activity initiator", null: false def user - Loaders::RecordLoader.for(User).load(object.user_id) + dataloader.with(Sources::ActiveRecordObject, User).load(object.user_id) end end end diff --git a/app/graphql/types/public_activity_type.rb b/app/graphql/types/public_activity_type.rb index 0eeffcc..7f5b9e9 100644 --- a/app/graphql/types/public_activity_type.rb +++ b/app/graphql/types/public_activity_type.rb @@ -6,5 +6,10 @@ class PublicActivityType < Types::BaseObject field :body, String, "Body", null: false field :title, String, "Title", null: false + field :user, Types::UserType, "Activity initiator", null: false + + def user + dataloader.with(Sources::ActiveRecordObject, User).load(object.user_id) + end end end diff --git a/spec/graphql/types/activity_type_n_plus_one_spec.rb b/spec/graphql/types/activity_type_n_plus_one_spec.rb index 7115ace..43dcd6f 100644 --- a/spec/graphql/types/activity_type_n_plus_one_spec.rb +++ b/spec/graphql/types/activity_type_n_plus_one_spec.rb @@ -1,6 +1,6 @@ require "rails_helper" -describe Types::ActivityType, :n_plus_one do +describe Types::ActivityType do let!(:user) { create(:user) } let(:query) do @@ -21,7 +21,11 @@ GRAPHQL end - populate { |n| create_list(:activity, n, user: user) } + it_behaves_like "succeed graphql request" - include_examples "performs constant number of queries request" + context "N+1", :n_plus_one do + populate { |n| create_list(:activity, n, user: user) } + + include_examples "performs constant number of queries request" + end end diff --git a/spec/support/shared_examples/succeed_graphql_request.rb b/spec/support/shared_examples/succeed_graphql_request.rb new file mode 100644 index 0000000..3b62c21 --- /dev/null +++ b/spec/support/shared_examples/succeed_graphql_request.rb @@ -0,0 +1,7 @@ +shared_examples "succeed graphql request" do + let(:response) { ApplicationSchema.execute(query).as_json } + + it "succeeds" do + expect(response["errors"]).to be_blank + end +end From a55426700fe288d8b0ba9e1537f295a6e5c7a8eb Mon Sep 17 00:00:00 2001 From: ArthurZaharov Date: Mon, 22 May 2023 19:24:24 +0300 Subject: [PATCH 2/3] Fix quality --- .rubocop.yml | 2 +- app/graphql/sources/active_record_object.rb | 17 ++++++++++------- .../types/activity_type_n_plus_one_spec.rb | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 85f1537..6d3bae5 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -142,4 +142,4 @@ GraphQL/ObjectDescription: Exclude: - app/graphql/application_schema.rb - app/graphql/types/base_**.rb - - app/graphql/loaders/**/* + - app/graphql/sources/**/* diff --git a/app/graphql/sources/active_record_object.rb b/app/graphql/sources/active_record_object.rb index db29c7b..7547b5d 100644 --- a/app/graphql/sources/active_record_object.rb +++ b/app/graphql/sources/active_record_object.rb @@ -1,10 +1,13 @@ -class Sources::ActiveRecordObject < GraphQL::Dataloader::Source - def initialize(model_class) - @model_class = model_class - end +module Sources + class ActiveRecordObject < GraphQL::Dataloader::Source + def initialize(model_class) + super() + @model_class = model_class + end - def fetch(ids) - records = @model_class.where(id: ids) - ids.map { |id| records.find { |r| r.id == id.to_i } } + def fetch(ids) + records = @model_class.where(id: ids) + ids.map { |id| records.find { |r| r.id == id.to_i } } + end end end diff --git a/spec/graphql/types/activity_type_n_plus_one_spec.rb b/spec/graphql/types/activity_type_n_plus_one_spec.rb index 43dcd6f..4393b2e 100644 --- a/spec/graphql/types/activity_type_n_plus_one_spec.rb +++ b/spec/graphql/types/activity_type_n_plus_one_spec.rb @@ -23,7 +23,7 @@ it_behaves_like "succeed graphql request" - context "N+1", :n_plus_one do + context "when N+1", :n_plus_one do populate { |n| create_list(:activity, n, user: user) } include_examples "performs constant number of queries request" From 2dadaf8d3883ff4e77ece8ba3f526e4f900ddf32 Mon Sep 17 00:00:00 2001 From: ArthurZaharov Date: Wed, 24 May 2023 11:03:09 +0300 Subject: [PATCH 3/3] Remove gem fixed version --- Gemfile | 2 +- Gemfile.lock | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile b/Gemfile index c137524..20e4646 100644 --- a/Gemfile +++ b/Gemfile @@ -50,7 +50,7 @@ end group :test do gem "database_cleaner-active_record" - gem "email_spec", "2.2.0" + gem "email_spec" gem "n_plus_one_control" gem "simplecov", require: false end diff --git a/Gemfile.lock b/Gemfile.lock index e8cd972..ba03837 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -382,7 +382,7 @@ DEPENDENCIES byebug database_cleaner-active_record dotenv-rails - email_spec (= 2.2.0) + email_spec enumerize factory_bot_rails ffaker