From 8370c3099e84a6dff7624f5b6f69045a8ce83b2d Mon Sep 17 00:00:00 2001 From: ulferts Date: Tue, 3 Dec 2019 08:28:29 +0100 Subject: [PATCH] avoid n+1 queries on query requests by caching --- .../queries/filters/shared/custom_field_filter.rb | 12 +++++++++++- .../work_packages/filter/filter_for_wp_mixin.rb | 6 +++++- .../queries/work_packages/filter/status_filter.rb | 8 ++++++-- .../projects/filters/custom_field_filter_spec.rb | 8 ++++---- .../filter/custom_fields/custom_field_filter_spec.rb | 8 ++++---- .../work_packages/filter/status_filter_spec.rb | 4 ++-- spec/requests/api/v3/queries/update_form_api_spec.rb | 5 +++++ spec/requests/api/v3/queries/update_query_spec.rb | 1 + 8 files changed, 38 insertions(+), 14 deletions(-) diff --git a/app/models/queries/filters/shared/custom_field_filter.rb b/app/models/queries/filters/shared/custom_field_filter.rb index 0e0cc47399..3d56b6f7a7 100644 --- a/app/models/queries/filters/shared/custom_field_filter.rb +++ b/app/models/queries/filters/shared/custom_field_filter.rb @@ -66,7 +66,7 @@ module Queries::Filters::Shared::CustomFieldFilter match = name.match /cf_(\d+)/ if match.present? && match[1].to_i > 0 - custom_field_context.custom_field_class.find_by(id: match[1].to_i) + all_custom_fields.detect { |cf| cf.id == match[1].to_i } end end @@ -104,5 +104,15 @@ module Queries::Filters::Shared::CustomFieldFilter :Base end end + + def all_custom_fields + key = ['Queries::Filters::Shared::CustomFieldFilter', + custom_field_context.custom_field_class, + 'all_custom_fields'] + + RequestStore.fetch(key.join('/')) do + custom_field_context.custom_field_class.all.to_a + end + end end end diff --git a/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb b/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb index 9a1fd251cc..15eeb85959 100644 --- a/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb +++ b/app/models/queries/work_packages/filter/filter_for_wp_mixin.rb @@ -52,7 +52,11 @@ module Queries::WorkPackages::Filter::FilterForWpMixin end def available? - visible_scope.exists? + key = 'Queries::WorkPackages::Filter::FilterForWpMixin/available' + + RequestStore.fetch(key) do + visible_scope.exists? + end end def ar_object_filter? diff --git a/app/models/queries/work_packages/filter/status_filter.rb b/app/models/queries/work_packages/filter/status_filter.rb index e1adec2ba8..04e3a77b3d 100644 --- a/app/models/queries/work_packages/filter/status_filter.rb +++ b/app/models/queries/work_packages/filter/status_filter.rb @@ -42,7 +42,7 @@ class Queries::WorkPackages::Filter::StatusFilter < Queries::WorkPackages::Filte end def available? - Status.exists? + all_statuses.any? end def type @@ -70,7 +70,11 @@ class Queries::WorkPackages::Filter::StatusFilter < Queries::WorkPackages::Filte private def all_statuses - @all_statuses ||= Status.all + key = 'Queries::WorkPackages::Filter::StatusFilter/all_statuses' + + RequestStore.fetch(key) do + Status.all.to_a + end end def operator_strategy diff --git a/spec/models/queries/projects/filters/custom_field_filter_spec.rb b/spec/models/queries/projects/filters/custom_field_filter_spec.rb index 2ad3097d6c..759c5dccf6 100644 --- a/spec/models/queries/projects/filters/custom_field_filter_spec.rb +++ b/spec/models/queries/projects/filters/custom_field_filter_spec.rb @@ -37,7 +37,7 @@ describe Queries::Projects::Filters::CustomFieldFilter, type: :model do let(:instance_key) { nil } let(:name) { field.name } - let(:list_project_custom_field) { FactoryBot.create(:list_project_custom_field) } + shared_let(:list_project_custom_field) { FactoryBot.create(:list_project_custom_field) } let(:bool_project_custom_field) { FactoryBot.build_stubbed(:bool_project_custom_field) } let(:int_project_custom_field) { FactoryBot.build_stubbed(:int_project_custom_field) } let(:float_project_custom_field) { FactoryBot.build_stubbed(:float_project_custom_field) } @@ -61,9 +61,9 @@ describe Queries::Projects::Filters::CustomFieldFilter, type: :model do end before do - allow(ProjectCustomField).to receive(:find_by) do |args| - all_custom_fields.detect { |stubbed| stubbed.id == args[:id] } - end + allow(ProjectCustomField) + .to receive(:all) + .and_return(all_custom_fields) end describe 'invalid custom field' do diff --git a/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb b/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb index 0572fb3c4e..0bd7249beb 100644 --- a/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/custom_fields/custom_field_filter_spec.rb @@ -38,7 +38,7 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter, type: :model do let(:instance_key) { nil } let(:name) { field.name } - let(:list_wp_custom_field) { FactoryBot.create(:list_wp_custom_field) } + shared_let(:list_wp_custom_field) { FactoryBot.create(:list_wp_custom_field) } let(:bool_wp_custom_field) { FactoryBot.build_stubbed(:bool_wp_custom_field) } let(:int_wp_custom_field) { FactoryBot.build_stubbed(:int_wp_custom_field) } let(:float_wp_custom_field) { FactoryBot.build_stubbed(:float_wp_custom_field) } @@ -62,9 +62,9 @@ describe Queries::WorkPackages::Filter::CustomFieldFilter, type: :model do end before do - allow(WorkPackageCustomField).to receive(:find_by) do |args| - all_custom_fields.detect { |stubbed| stubbed.id == args[:id] } - end + allow(WorkPackageCustomField) + .to receive(:all) + .and_return(all_custom_fields) end describe 'invalid custom field' do diff --git a/spec/models/queries/work_packages/filter/status_filter_spec.rb b/spec/models/queries/work_packages/filter/status_filter_spec.rb index 51066d9a35..f710e691cf 100644 --- a/spec/models/queries/work_packages/filter/status_filter_spec.rb +++ b/spec/models/queries/work_packages/filter/status_filter_spec.rb @@ -39,8 +39,8 @@ describe Queries::WorkPackages::Filter::StatusFilter, type: :model do describe '#available?' do it 'is true if any status exists' do allow(Status) - .to receive(:exists?) - .and_return true + .to receive(:all) + .and_return [status] expect(instance).to be_available end diff --git a/spec/requests/api/v3/queries/update_form_api_spec.rb b/spec/requests/api/v3/queries/update_form_api_spec.rb index 6d0702eca2..b5d707a862 100644 --- a/spec/requests/api/v3/queries/update_form_api_spec.rb +++ b/spec/requests/api/v3/queries/update_form_api_spec.rb @@ -309,6 +309,11 @@ describe "POST /api/v3/queries/form", type: :request do describe 'with all parameters given' do let(:status) { FactoryBot.create :status } + let(:additional_setup) do + status + RequestStore.clear! + end + let(:parameters) do { name: "Some Query", diff --git a/spec/requests/api/v3/queries/update_query_spec.rb b/spec/requests/api/v3/queries/update_query_spec.rb index 31dc7ebddb..fe0eb54648 100644 --- a/spec/requests/api/v3/queries/update_query_spec.rb +++ b/spec/requests/api/v3/queries/update_query_spec.rb @@ -108,6 +108,7 @@ describe "PATCH /api/v3/queries/:id", type: :request do end before do + RequestStore.clear! login_as user end