diff options
Diffstat (limited to 'spec/models/preloaders')
3 files changed, 140 insertions, 17 deletions
diff --git a/spec/models/preloaders/group_policy_preloader_spec.rb b/spec/models/preloaders/group_policy_preloader_spec.rb new file mode 100644 index 00000000000..f6e40d1f033 --- /dev/null +++ b/spec/models/preloaders/group_policy_preloader_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::GroupPolicyPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_parent) } + let_it_be(:private_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer') } + + let(:base_groups) { [guest_group, private_maintainer_group, private_developer_group, public_maintainer_group] } + + before_all do + guest_group.add_guest(user) + private_maintainer_group.add_maintainer(user) + private_developer_group.add_developer(user) + public_maintainer_group.add_maintainer(user) + end + + it 'avoids N+1 queries when authorizing a list of groups', :request_store do + preload_groups_for_policy(user) + control = ActiveRecord::QueryRecorder.new { authorize_all_groups(user) } + + new_group1 = create(:group, :private).tap { |group| group.add_maintainer(user) } + new_group2 = create(:group, :private, parent: private_maintainer_group) + + another_root = create(:group, :private, name: 'root-3', path: 'root-3') + new_group3 = create(:group, :private, parent: another_root).tap { |group| group.add_maintainer(user) } + + pristine_groups = Group.where(id: base_groups + [new_group1, new_group2, new_group3]) + + preload_groups_for_policy(user, pristine_groups) + expect { authorize_all_groups(user, pristine_groups) }.not_to exceed_query_limit(control) + end + + def authorize_all_groups(current_user, group_list = base_groups) + group_list.each { |group| current_user.can?(:read_group, group) } + end + + def preload_groups_for_policy(current_user, group_list = base_groups) + described_class.new(group_list, current_user).execute + end +end diff --git a/spec/models/preloaders/group_root_ancestor_preloader_spec.rb b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb new file mode 100644 index 00000000000..0d622e84ef1 --- /dev/null +++ b/spec/models/preloaders/group_root_ancestor_preloader_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::GroupRootAncestorPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:root_parent1) { create(:group, :private, name: 'root-1', path: 'root-1') } + let_it_be(:root_parent2) { create(:group, :private, name: 'root-2', path: 'root-2') } + let_it_be(:guest_group) { create(:group, name: 'public guest', path: 'public-guest') } + let_it_be(:private_maintainer_group) { create(:group, :private, name: 'b private maintainer', path: 'b-private-maintainer', parent: root_parent1) } + let_it_be(:private_developer_group) { create(:group, :private, project_creation_level: nil, name: 'c public developer', path: 'c-public-developer') } + let_it_be(:public_maintainer_group) { create(:group, :private, name: 'a public maintainer', path: 'a-public-maintainer', parent: root_parent2) } + + let(:root_query_regex) { /\ASELECT.+FROM "namespaces" WHERE "namespaces"."id" = \d+/ } + let(:additional_preloads) { [] } + let(:groups) { [guest_group, private_maintainer_group, private_developer_group, public_maintainer_group] } + let(:pristine_groups) { Group.where(id: groups) } + + shared_examples 'executes N matching DB queries' do |expected_query_count, query_method = nil| + it 'executes the specified root_ancestor queries' do + expect do + pristine_groups.each do |group| + root_ancestor = group.root_ancestor + + root_ancestor.public_send(query_method) if query_method.present? + end + end.to make_queries_matching(root_query_regex, expected_query_count) + end + + it 'strong_memoizes the correct root_ancestor' do + pristine_groups.each do |group| + expected_parent_id = group.root_ancestor.id == group.id ? nil : group.root_ancestor.id + + expect(group.parent_id).to eq(expected_parent_id) + end + end + end + + context 'when the preloader is used' do + before do + preload_ancestors + end + + context 'when no additional preloads are provided' do + it_behaves_like 'executes N matching DB queries', 0 + end + + context 'when additional preloads are provided' do + let(:additional_preloads) { [:route] } + let(:root_query_regex) { /\ASELECT.+FROM "routes" WHERE "routes"."source_id" = \d+/ } + + it_behaves_like 'executes N matching DB queries', 0, :full_path + end + end + + context 'when the preloader is not used' do + it_behaves_like 'executes N matching DB queries', 2 + end + + def preload_ancestors + described_class.new(pristine_groups, additional_preloads).execute + end +end diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb index 8144e1ad233..5fc7bfb1f62 100644 --- a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb +++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb @@ -13,32 +13,47 @@ RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do shared_examples 'executes N max member permission queries to the DB' do it 'executes the specified max membership queries' do - queries = ActiveRecord::QueryRecorder.new do - groups.each { |group| user.can?(:read_group, group) } - end + expect { groups.each { |group| user.can?(:read_group, group) } }.to make_queries_matching(max_query_regex, expected_query_count) + end - max_queries = queries.log.grep(max_query_regex) + it 'caches the correct access_level for each group' do + groups.each do |group| + access_level_from_db = group.members_with_parents.where(user_id: user.id).group(:user_id).maximum(:access_level)[user.id] || Gitlab::Access::NO_ACCESS + cached_access_level = group.max_member_access_for_user(user) - expect(max_queries.count).to eq(expected_query_count) + expect(cached_access_level).to eq(access_level_from_db) + end end end context 'when the preloader is used', :request_store do - before do - described_class.new(groups, user).execute - end + context 'when user has indirect access to groups' do + let_it_be(:child_maintainer) { create(:group, :private, parent: group1).tap {|g| g.add_maintainer(user)} } + let_it_be(:child_indirect_access) { create(:group, :private, parent: group1) } - it_behaves_like 'executes N max member permission queries to the DB' do - # Will query all groups where the user is not already a member - let(:expected_query_count) { 1 } - end + let(:groups) { [group1, group2, group3, child_maintainer, child_indirect_access] } + + context 'when traversal_ids feature flag is disabled' do + it_behaves_like 'executes N max member permission queries to the DB' do + before do + stub_feature_flags(use_traversal_ids: false) + described_class.new(groups, user).execute + end + + # One query for group with no access and another one per group where the user is not a direct member + let(:expected_query_count) { 2 } + end + end - context 'when user has access but is not a direct member of the group' do - let(:groups) { [group1, group2, group3, create(:group, :private, parent: group1)] } + context 'when traversal_ids feature flag is enabled' do + it_behaves_like 'executes N max member permission queries to the DB' do + before do + stub_feature_flags(use_traversal_ids: true) + described_class.new(groups, user).execute + end - it_behaves_like 'executes N max member permission queries to the DB' do - # One query for group with no access and another one where the user is not a direct member - let(:expected_query_count) { 2 } + let(:expected_query_count) { 0 } + end end end end |