From 20f78421c82d626a3b43924146b7878fe4777ae8 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 30 Nov 2017 14:26:08 +0100 Subject: Cache the forks in a namespace in the RequestStore On the `show` of a project that is part of a fork network. We check if the user already created a fork of this project in their personal namespace. We do this in several places, so caching the result of this query in the request store prevents us from repeating it. --- spec/controllers/projects_controller_spec.rb | 23 +++++++++++++++++++++++ spec/models/namespace_spec.rb | 10 +++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index e7ab714c550..39ee5e8691b 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -261,6 +261,29 @@ describe ProjectsController do expect(response).to redirect_to(namespace_project_path) end end + + context 'when the project is forked and has a repository', :request_store do + let(:public_project) { create(:project, :public, :repository) } + + render_views + + before do + # View the project as a user that does not have any rights + sign_in(create(:user)) + + fork_project(public_project) + end + + it 'does not increase the number of queries when the project is forked' do + # When a project is part of a fork network, we check if the `current_user` + # has a fork in their own namespace. We query this several times. Caching + # the result in the RequestStore brings the number of queries for this + # request down from 64 to 59. + + expect { get(:show, namespace_id: public_project.namespace, id: public_project) } + .not_to exceed_query_limit(59) + end + end end describe "#update" do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index 90b768f595e..3817f20bfe7 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -531,7 +531,7 @@ describe Namespace do end end - describe '#has_forks_of?' do + describe '#find_fork_of?' do let(:project) { create(:project, :public) } let!(:forked_project) { fork_project(project, namespace.owner, namespace: namespace) } @@ -550,5 +550,13 @@ describe Namespace do expect(other_namespace.find_fork_of(project)).to eq(other_fork) end + + context 'with request store enabled', :request_store do + it 'only queries once' do + expect(project.fork_network).to receive(:find_forks_in).once.and_call_original + + 2.times { namespace.find_fork_of(project) } + end + end end end -- cgit v1.2.1 From 3d4ba90c5007cca3e8619afe8f40675962a9bc73 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 4 Dec 2017 16:58:44 +0100 Subject: Count occurrences of a specific query in the query recorder. --- spec/controllers/projects_controller_spec.rb | 10 ++++------ spec/support/query_recorder.rb | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 11 deletions(-) (limited to 'spec') diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index 39ee5e8691b..e4b2bbb7c51 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/spec/controllers/projects_controller_spec.rb @@ -264,24 +264,22 @@ describe ProjectsController do context 'when the project is forked and has a repository', :request_store do let(:public_project) { create(:project, :public, :repository) } + let(:other_user) { create(:user) } render_views before do # View the project as a user that does not have any rights - sign_in(create(:user)) + sign_in(other_user) fork_project(public_project) end it 'does not increase the number of queries when the project is forked' do - # When a project is part of a fork network, we check if the `current_user` - # has a fork in their own namespace. We query this several times. Caching - # the result in the RequestStore brings the number of queries for this - # request down from 64 to 59. + expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/ expect { get(:show, namespace_id: public_project.namespace, id: public_project) } - .not_to exceed_query_limit(59) + .not_to exceed_query_limit(1).for_query(expected_query) end end end diff --git a/spec/support/query_recorder.rb b/spec/support/query_recorder.rb index 369775db462..8cf8f45a8b2 100644 --- a/spec/support/query_recorder.rb +++ b/spec/support/query_recorder.rb @@ -41,7 +41,8 @@ RSpec::Matchers.define :exceed_query_limit do |expected| supports_block_expectations match do |block| - query_count(&block) > expected_count + threshold + @subject_block = block + actual_count > expected_count + threshold end failure_message_when_negated do |actual| @@ -55,6 +56,11 @@ RSpec::Matchers.define :exceed_query_limit do |expected| self end + def for_query(query) + @query = query + self + end + def threshold @threshold.to_i end @@ -68,12 +74,15 @@ RSpec::Matchers.define :exceed_query_limit do |expected| end def actual_count - @recorder.count + @actual_count ||= if @query + recorder.log.select { |recorded| recorded =~ @query }.size + else + recorder.count + end end - def query_count(&block) - @recorder = ActiveRecord::QueryRecorder.new(&block) - @recorder.count + def recorder + @recorder ||= ActiveRecord::QueryRecorder.new(&@subject_block) end def count_queries(queries) -- cgit v1.2.1