diff --git a/ee/app/models/preloaders/user_member_roles_in_projects_preloader.rb b/ee/app/models/preloaders/user_member_roles_in_projects_preloader.rb index 3131ff9b1d6b8c6b4066a693560b555b140a3306..a5d79ea22052b966c1b2afd75d493f1bfd977d46 100644 --- a/ee/app/models/preloaders/user_member_roles_in_projects_preloader.rb +++ b/ee/app/models/preloaders/user_member_roles_in_projects_preloader.rb @@ -2,6 +2,8 @@ module Preloaders class UserMemberRolesInProjectsPreloader + include Gitlab::Utils::StrongMemoize + def initialize(projects:, user:) @projects = if projects.is_a?(Array) Project.select(:id, :namespace_id).where(id: projects) @@ -15,11 +17,6 @@ def initialize(projects:, user:) end def execute - ActiveRecord::Associations::Preloader.new( - records: projects, - associations: [:group] - ).call - ::Preloaders::ProjectRootAncestorPreloader.new(projects, :namespace).execute ::Gitlab::SafeRequestLoader.execute( @@ -34,7 +31,7 @@ def execute def abilities_for_user_grouped_by_project sql_values_array = projects.filter_map do |project| - next unless project.custom_roles_enabled? + next unless custom_roles_enabled_on?(project) [project.id, Arel.sql("ARRAY[#{project.namespace.traversal_ids.join(',')}]::integer[]")] end @@ -88,6 +85,21 @@ def abilities_for_user_grouped_by_project end end + def custom_roles_enabled_on + Hash.new do |hash, namespace| + hash[namespace] = namespace&.custom_roles_enabled? + end + end + strong_memoize_attr :custom_roles_enabled_on + + def custom_roles_enabled_on?(project) + if Feature.enabled?(:search_filter_by_ability, user) + custom_roles_enabled_on[project&.root_ancestor] + else + project.custom_roles_enabled? + end + end + def resource_key "member_roles_in_projects:user:#{user.id}" end diff --git a/ee/spec/lib/elastic/latest/git_class_proxy_spec.rb b/ee/spec/lib/elastic/latest/git_class_proxy_spec.rb index e013e69f7d78225faefb2bec5b035bd60664fe85..d6deafeafce310170eae91457e979a4f1ccc8330 100644 --- a/ee/spec/lib/elastic/latest/git_class_proxy_spec.rb +++ b/ee/spec/lib/elastic/latest/git_class_proxy_spec.rb @@ -127,6 +127,44 @@ ) end + context 'with saas', :saas do + let_it_be(:subscription) do + create(:gitlab_subscription, namespace: project.group, hosted_plan: create(:ultimate_plan)) + end + + before do + stub_ee_application_setting(should_check_namespace_plan: true) + end + + it 'returns matching search results' do + expect(search_results[:blobs][:results].count).to eq(1) + expect(search_results[:blobs][:results][0][:_source][:blob][:path]).to eq( + 'files/markdown/ruby-style-guide.md' + ) + end + + it 'avoids N+1 queries' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + included_class + .new(project.repository.class) + .elastic_search('Mailer.deliver', type: 'blob', options: search_options) + end + + projects = [ + project, + create(:project, :private, :repository, group: create(:group, parent: project.group)) + ] + + expect do + included_class + .new(project.repository.class) + .elastic_search('Mailer.deliver', type: 'blob', options: search_options.merge( + project_ids: projects.map(&:id) + )) + end.to issue_same_number_of_queries_as(control).or_fewer + end + end + context 'when the `search_filter_by_ability` feature flag is disabled' do before do stub_feature_flags(search_filter_by_ability: false) diff --git a/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb b/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb index b1e2c5a0a48bbfb77ec06b44bb54b2453a725781..653aa815ff7fc81daa21c29de0bba44fd22c0f04 100644 --- a/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb +++ b/ee/spec/models/preloaders/user_member_roles_in_projects_preloader_spec.rb @@ -49,11 +49,44 @@ def create_member_role(ability, member) create_member_role(ability, project_member) end - context 'when custom role has ability: true' do + context "when custom role has #{ability}: true" do context 'when Array of project passed' do it 'returns the project_id with a value array that includes the ability' do expect(result[project.id]).to match_array(expected_abilities) end + + context 'when saas', :saas do + let_it_be(:subscription) do + create(:gitlab_subscription, namespace: project.group, hosted_plan: create(:ultimate_plan)) + end + + before do + stub_ee_application_setting(should_check_namespace_plan: true) + end + + it 'avoids N+1 queries' do + projects = [project] + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + described_class.new(projects: projects, user: user).execute + end + + projects << create(:project, :private, group: create(:group, parent: project.group)) + + expect do + described_class.new(projects: projects, user: user).execute + end.to issue_same_number_of_queries_as(control).or_fewer + end + + context 'with the `search_filter_by_ability` feature flag disabled' do + before do + stub_feature_flags(search_filter_by_ability: false) + end + + it 'returns the expect results' do + expect(result[project.id]).to match_array(expected_abilities) + end + end + end end context 'when ActiveRecord::Relation of projects passed' do diff --git a/ee/spec/requests/custom_roles/read_code/request_spec.rb b/ee/spec/requests/custom_roles/read_code/request_spec.rb index 89b0f9b848853a6b033abe012406164a86936f5b..9a813c6dff07bd4671cfe597080d41e61aa2bb39 100644 --- a/ee/spec/requests/custom_roles/read_code/request_spec.rb +++ b/ee/spec/requests/custom_roles/read_code/request_spec.rb @@ -45,6 +45,58 @@ expect(response.body).to include('/files/markdown/ruby-style-guide.md#L452') expect(response.body).to include(source_code) end + + context 'when saas', :saas do + let_it_be(:subscription) do + create(:gitlab_subscription, namespace: project.group, hosted_plan: create(:ultimate_plan)) + end + + before do + stub_ee_application_setting( + elasticsearch_indexing: true, + elasticsearch_search: true, + should_check_namespace_plan: true + ) + end + + it 'allows access via a custom role' do + get search_path, params: { + group_id: project.group.id, + scope: 'blobs', + search: 'Mailer.deliver' + } + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).not_to include('We couldn't find any code results matching') + expect(response.body).to include('/files/markdown/ruby-style-guide.md#L452') + expect(response.body).to include(source_code) + end + + it 'avoids N+1 queries' do + get search_path, params: { group_id: project.group.id, scope: 'blobs', search: 'Mailer.deliver' } # warmup + expect(response.body).to include(source_code) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get search_path, params: { + group_id: project.group.id, + scope: 'blobs', + search: 'Mailer.deliver' + } + expect(response.body).to include(source_code) + end + + create(:project, :private, :repository, group: create(:group, parent: project.group)) + + expect do + get search_path, params: { + group_id: project.group.id, + scope: 'blobs', + search: 'Mailer.deliver' + } + expect(response.body).to include(source_code) + end.to issue_same_number_of_queries_as(control).or_fewer + end + end end context 'when searching a project' do