diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2017-09-26 12:22:52 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2017-10-04 23:49:42 +0300 |
commit | ac0b104ae4968adaed7b94db76c0ac86badb6d6b (patch) | |
tree | be0737e1d9971911bedc48bb05d1fe84e46ac93a /spec/controllers/groups_controller_spec.rb | |
parent | cd85c22faa7092edabf252fa157125ea571ed054 (diff) |
Minimize the number of queries by preloading counts and ancestors
By preloading the count of members, projects and subgroups of a group,
we don't need to query them later.
We also preload the entire hierarchy for a search result and include
the counts so we don't need to query for them again
Diffstat (limited to 'spec/controllers/groups_controller_spec.rb')
-rw-r--r-- | spec/controllers/groups_controller_spec.rb | 34 |
1 files changed, 11 insertions, 23 deletions
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb index befd346596f..84207144036 100644 --- a/spec/controllers/groups_controller_spec.rb +++ b/spec/controllers/groups_controller_spec.rb @@ -303,10 +303,12 @@ describe GroupsController do end context 'queries per rendered element', :request_store do - # The expected extra queries for the rendered group are: + # We need to make sure the following counts are preloaded + # otherwise they will cause an extra query # 1. Count of visible projects in the element # 2. Count of visible subgroups in the element - let(:expected_queries_per_group) { 2 } + # 3. Count of members of a group + let(:expected_queries_per_group) { 0 } let(:expected_queries_per_project) { 0 } def get_list @@ -329,13 +331,9 @@ describe GroupsController do end context 'when rendering hierarchies' do - # Extra queries per group when rendering a hierarchy: - # The route and the namespace are `included` for all matched elements - # But the parent's above those are not, so there's 2 extra queries per - # nested level: - # 1. Loading the parent that wasn't loaded yet - # 2. Loading the route for that parent. - let(:extra_queries_per_nested_level) { expected_queries_per_group + 2 } + # When loading hierarchies we load the all the ancestors for matched projects + # in 1 separate query + let(:extra_queries_for_hierarchies) { 1 } def get_filtered_list get :children, id: group.to_param, filter: 'filter', format: :json @@ -348,7 +346,7 @@ describe GroupsController do matched_group.update!(parent: public_subgroup) - expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_per_nested_level) + expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_for_hierarchies) end it 'queries the expected amount when a new group match is added' do @@ -357,8 +355,9 @@ describe GroupsController do control = ActiveRecord::QueryRecorder.new { get_filtered_list } create(:group, :public, parent: public_subgroup, name: 'filterme2') + create(:group, :public, parent: public_subgroup, name: 'filterme3') - expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_per_nested_level) + expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_for_hierarchies) end it 'queries the expected amount when nested rows are increased for a project' do @@ -368,18 +367,7 @@ describe GroupsController do matched_project.update!(namespace: public_subgroup) - expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_per_nested_level) - end - - it 'queries the expected amount when a new project match is added' do - create(:project, :public, namespace: public_subgroup, name: 'filterme') - - control = ActiveRecord::QueryRecorder.new { get_filtered_list } - - nested_group = create(:group, :public, parent: group) - create(:project, :public, namespace: nested_group, name: 'filterme2') - - expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_per_nested_level) + expect { get_filtered_list }.not_to exceed_query_limit(control).with_threshold(extra_queries_for_hierarchies) end end end |