diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-04-20 23:50:22 +0000 |
commit | 9dc93a4519d9d5d7be48ff274127136236a3adb3 (patch) | |
tree | 70467ae3692a0e35e5ea56bcb803eb512a10bedb /app/finders | |
parent | 4b0f34b6d759d6299322b3a54453e930c6121ff0 (diff) | |
download | gitlab-ce-9dc93a4519d9d5d7be48ff274127136236a3adb3.tar.gz |
Add latest changes from gitlab-org/gitlab@13-11-stable-eev13.11.0-rc43
Diffstat (limited to 'app/finders')
49 files changed, 501 insertions, 346 deletions
diff --git a/app/finders/alert_management/http_integrations_finder.rb b/app/finders/alert_management/http_integrations_finder.rb index 9f511be0ace..5d4c9b6fbe3 100644 --- a/app/finders/alert_management/http_integrations_finder.rb +++ b/app/finders/alert_management/http_integrations_finder.rb @@ -27,7 +27,7 @@ module AlertManagement first_id = project.alert_management_http_integrations .ordered_by_id .select(:id) - .at_most(1) + .limit(1) @collection = collection.id_in(first_id) end diff --git a/app/finders/applications_finder.rb b/app/finders/applications_finder.rb index 3ded90f3fd5..c5b5094b195 100644 --- a/app/finders/applications_finder.rb +++ b/app/finders/applications_finder.rb @@ -17,6 +17,6 @@ class ApplicationsFinder def by_id(applications) return applications unless params[:id] - Doorkeeper::Application.find_by(id: params[:id]) # rubocop: disable CodeReuse/ActiveRecord + applications.find_by(id: params[:id]) # rubocop: disable CodeReuse/ActiveRecord end end diff --git a/app/finders/award_emojis_finder.rb b/app/finders/award_emojis_finder.rb index 7882beb64bf..9ff64637128 100644 --- a/app/finders/award_emojis_finder.rb +++ b/app/finders/award_emojis_finder.rb @@ -13,8 +13,7 @@ class AwardEmojisFinder def execute awards = awardable.award_emoji awards = by_name(awards) - awards = by_awarded_by(awards) - awards + by_awarded_by(awards) end private diff --git a/app/finders/branches_finder.rb b/app/finders/branches_finder.rb index 2eee90a512a..157c454183a 100644 --- a/app/finders/branches_finder.rb +++ b/app/finders/branches_finder.rb @@ -11,8 +11,7 @@ class BranchesFinder < GitRefsFinder else branches = repository.branches_sorted_by(sort) branches = by_search(branches) - branches = by_names(branches) - branches + by_names(branches) end end diff --git a/app/finders/ci/commit_statuses_finder.rb b/app/finders/ci/commit_statuses_finder.rb index d49ec7ebb40..c3d0a34d2ff 100644 --- a/app/finders/ci/commit_statuses_finder.rb +++ b/app/finders/ci/commit_statuses_finder.rb @@ -21,9 +21,9 @@ module Ci def latest_commits strong_memoize(:latest_commits) do - refs.map do |ref| + refs.to_h do |ref| [ref.name, @repository.commit(ref.dereferenced_target).sha] - end.to_h + end end end diff --git a/app/finders/ci/daily_build_group_report_results_finder.rb b/app/finders/ci/daily_build_group_report_results_finder.rb index 9e736c70dda..5ac1bbd0670 100644 --- a/app/finders/ci/daily_build_group_report_results_finder.rb +++ b/app/finders/ci/daily_build_group_report_results_finder.rb @@ -35,8 +35,7 @@ module Ci return Ci::DailyBuildGroupReportResult.none unless query_allowed? collection = Ci::DailyBuildGroupReportResult.by_projects(params[:project]) - collection = filter_report_results(collection) - collection + filter_report_results(collection) end private @@ -51,8 +50,7 @@ module Ci collection = by_dates(collection) collection = sort(collection) - collection = limit_by(collection) - collection + limit_by(collection) end def by_coverage(items) diff --git a/app/finders/ci/pipelines_finder.rb b/app/finders/ci/pipelines_finder.rb index a77faebb160..e509cf940b8 100644 --- a/app/finders/ci/pipelines_finder.rb +++ b/app/finders/ci/pipelines_finder.rb @@ -131,7 +131,7 @@ module Ci def by_yaml_errors(items) case Gitlab::Utils.to_boolean(params[:yaml_errors]) when true - items.where("yaml_errors IS NOT NULL") + items.where.not(yaml_errors: nil) when false items.where("yaml_errors IS NULL") else diff --git a/app/finders/ci/variables_finder.rb b/app/finders/ci/variables_finder.rb index d933643ffb2..39a1a60596d 100644 --- a/app/finders/ci/variables_finder.rb +++ b/app/finders/ci/variables_finder.rb @@ -2,23 +2,23 @@ module Ci class VariablesFinder - attr_reader :project, :params - - def initialize(project, params) - @project, @params = project, params + def initialize(resource, params) + @resource = resource + @params = params raise ArgumentError, 'Please provide params[:key]' if params[:key].blank? end def execute - variables = project.variables + variables = resource.variables variables = by_key(variables) - variables = by_environment_scope(variables) - variables + by_environment_scope(variables) end private + attr_reader :resource, :params + def by_key(variables) variables.by_key(params[:key]) end diff --git a/app/finders/concerns/finder_with_group_hierarchy.rb b/app/finders/concerns/finder_with_group_hierarchy.rb new file mode 100644 index 00000000000..86ccac19b63 --- /dev/null +++ b/app/finders/concerns/finder_with_group_hierarchy.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +# Module to include into finders to provide support for querying for +# objects up and down the group hierarchy. Extracted from LabelsFinder +# +# Supports params: +# :group +# :group_id +# :include_ancestor_groups +# :include_descendant_groups +module FinderWithGroupHierarchy + extend ActiveSupport::Concern + + private + + def item_ids + raise NotImplementedError + end + + # Gets redacted array of group ids + # which can include the ancestors and descendants of the requested group. + def group_ids_for(group) + strong_memoize(:group_ids) do + groups = groups_to_include(group) + + # Because we are sure that all groups are in the same hierarchy tree + # we can preset root group for all of them to optimize permission checks + Group.preset_root_ancestor_for(groups) + + groups_user_can_read_items(groups).map(&:id) + end + end + + def groups_to_include(group) + groups = [group] + + groups += group.ancestors if include_ancestor_groups? + groups += group.descendants if include_descendant_groups? + + groups + end + + def include_ancestor_groups? + params[:include_ancestor_groups] + end + + def include_descendant_groups? + params[:include_descendant_groups] + end + + def group? + params[:group].present? || params[:group_id].present? + end + + def group + strong_memoize(:group) { params[:group].presence || Group.find(params[:group_id]) } + end + + def read_permission + raise NotImplementedError + end + + def authorized_to_read_item?(item_parent) + return true if skip_authorization + + Ability.allowed?(current_user, read_permission, item_parent) + end + + def groups_user_can_read_items(groups) + DeclarativePolicy.user_scope do + groups.select { |group| authorized_to_read_item?(group) } + end + end +end diff --git a/app/finders/concerns/merged_at_filter.rb b/app/finders/concerns/merged_at_filter.rb index 581bcca3c25..e44354f36d1 100644 --- a/app/finders/concerns/merged_at_filter.rb +++ b/app/finders/concerns/merged_at_filter.rb @@ -10,7 +10,7 @@ module MergedAtFilter mr_metrics_scope = mr_metrics_scope.merged_after(merged_after) if merged_after.present? mr_metrics_scope = mr_metrics_scope.merged_before(merged_before) if merged_before.present? - items.join_metrics.merge(mr_metrics_scope) + join_metrics(items, mr_metrics_scope) end def merged_after @@ -20,4 +20,22 @@ module MergedAtFilter def merged_before params[:merged_before] end + + # rubocop: disable CodeReuse/ActiveRecord + # + # This join optimizes merged_at queries when the finder is invoked for a project by moving + # the target_project_id condition from merge_requests table to merge_request_metrics table. + def join_metrics(items, mr_metrics_scope) + scope = if project_id = items.where_values_hash["target_project_id"] + # removing the original merge_requests.target_project_id condition + items = items.unscope(where: :target_project_id) + # adding the target_project_id condition to merge_request_metrics + items.join_metrics(project_id) + else + items.join_metrics + end + + scope.merge(mr_metrics_scope) + end + # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/finders/concerns/packages/finder_helper.rb b/app/finders/concerns/packages/finder_helper.rb index 30bc0ff7909..39c018818d1 100644 --- a/app/finders/concerns/packages/finder_helper.rb +++ b/app/finders/concerns/packages/finder_helper.rb @@ -11,22 +11,26 @@ module Packages def packages_visible_to_user(user, within_group:) return ::Packages::Package.none unless within_group - return ::Packages::Package.none unless Ability.allowed?(user, :read_package, within_group) + return ::Packages::Package.none unless Ability.allowed?(user, :read_group, within_group) - projects = projects_visible_to_reporters(user, within_group.self_and_descendants.select(:id)) + projects = projects_visible_to_reporters(user, within_group: within_group) ::Packages::Package.for_projects(projects.select(:id)) end def projects_visible_to_user(user, within_group:) return ::Project.none unless within_group - return ::Project.none unless Ability.allowed?(user, :read_package, within_group) + return ::Project.none unless Ability.allowed?(user, :read_group, within_group) - projects_visible_to_reporters(user, within_group.self_and_descendants.select(:id)) + projects_visible_to_reporters(user, within_group: within_group) end - def projects_visible_to_reporters(user, namespace_ids) - ::Project.in_namespace(namespace_ids) - .public_or_visible_to_user(user, ::Gitlab::Access::REPORTER) + def projects_visible_to_reporters(user, within_group:) + if user.is_a?(DeployToken) && Feature.enabled?(:packages_finder_helper_deploy_token) + user.accessible_projects + else + within_group.all_projects + .public_or_visible_to_user(user, ::Gitlab::Access::REPORTER) + end end def package_type diff --git a/app/finders/context_commits_finder.rb b/app/finders/context_commits_finder.rb index de89a556ee0..d623854ada4 100644 --- a/app/finders/context_commits_finder.rb +++ b/app/finders/context_commits_finder.rb @@ -11,9 +11,7 @@ class ContextCommitsFinder def execute commits = init_collection - commits = filter_existing_commits(commits) - - commits + filter_existing_commits(commits) end private @@ -21,19 +19,15 @@ class ContextCommitsFinder attr_reader :project, :merge_request, :search, :limit, :offset def init_collection - commits = - if search.present? - search_commits - else - project.repository.commits(merge_request.target_branch, { limit: limit, offset: offset }) - end - - commits + if search.present? + search_commits + else + project.repository.commits(merge_request.target_branch, { limit: limit, offset: offset }) + end end def filter_existing_commits(commits) commits.select! { |commit| already_included_ids.exclude?(commit.id) } - commits end diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index 89a28d9dfb8..ae26fc14ad5 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -33,9 +33,7 @@ class DeploymentsFinder items = by_environment(items) items = by_status(items) items = preload_associations(items) - items = sort(items) - - items + sort(items) end private diff --git a/app/finders/environments_by_deployments_finder.rb b/app/finders/environments_by_deployments_finder.rb new file mode 100644 index 00000000000..76e1c050ea5 --- /dev/null +++ b/app/finders/environments_by_deployments_finder.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +class EnvironmentsByDeploymentsFinder + attr_reader :project, :current_user, :params + + def initialize(project, current_user, params = {}) + @project = project + @current_user = current_user + @params = params + end + + # rubocop: disable CodeReuse/ActiveRecord + def execute + deployments = project.deployments + deployments = + if ref + deployments_query = params[:with_tags] ? 'ref = :ref OR tag IS TRUE' : 'ref = :ref' + deployments.where(deployments_query, ref: ref.to_s) + elsif commit + deployments.where(sha: commit.sha) + else + deployments.none + end + + environment_ids = deployments + .group(:environment_id) + .select(:environment_id) + + environments = project.environments.available + .where(id: environment_ids) + + if params[:find_latest] + find_one(environments.order_by_last_deployed_at_desc) + else + find_all(environments.order_by_last_deployed_at.to_a) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + def find_one(environments) + [environments.find { |environment| valid_environment?(environment) }].compact + end + + def find_all(environments) + environments.select { |environment| valid_environment?(environment) } + end + + def valid_environment?(environment) + # Go in order of cost: SQL calls are cheaper than Gitaly calls + return false unless Ability.allowed?(current_user, :read_environment, environment) + + return false if ref && params[:recently_updated] && !environment.recently_updated_on_branch?(ref) + return false if ref && commit && !environment.includes_commit?(commit) + + true + end + + def ref + params[:ref].try(:to_s) + end + + def commit + params[:commit] + end +end diff --git a/app/finders/environments_finder.rb b/app/finders/environments_finder.rb index 32ca1a42db7..c64e850f440 100644 --- a/app/finders/environments_finder.rb +++ b/app/finders/environments_finder.rb @@ -6,81 +6,22 @@ class EnvironmentsFinder InvalidStatesError = Class.new(StandardError) def initialize(project, current_user, params = {}) - @project, @current_user, @params = project, current_user, params + @project = project + @current_user = current_user + @params = params end - # rubocop: disable CodeReuse/ActiveRecord def execute - deployments = project.deployments - deployments = - if ref - deployments_query = params[:with_tags] ? 'ref = :ref OR tag IS TRUE' : 'ref = :ref' - deployments.where(deployments_query, ref: ref.to_s) - elsif commit - deployments.where(sha: commit.sha) - else - deployments.none - end - - environment_ids = deployments - .group(:environment_id) - .select(:environment_id) - - environments = project.environments.available - .where(id: environment_ids) - - if params[:find_latest] - find_one(environments.order_by_last_deployed_at_desc) - else - find_all(environments.order_by_last_deployed_at.to_a) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - # This method will eventually take the place of `#execute` as an - # efficient way to get relevant environment entries. - # Currently, `#execute` method has a serious technical debt and - # we will likely rework on it in the future. - # See more https://gitlab.com/gitlab-org/gitlab-foss/issues/63381 - def find environments = project.environments environments = by_name(environments) environments = by_search(environments) # Raises InvalidStatesError if params[:states] contains invalid states. - environments = by_states(environments) - - environments + by_states(environments) end private - def find_one(environments) - [environments.find { |environment| valid_environment?(environment) }].compact - end - - def find_all(environments) - environments.select { |environment| valid_environment?(environment) } - end - - def valid_environment?(environment) - # Go in order of cost: SQL calls are cheaper than Gitaly calls - return false unless Ability.allowed?(current_user, :read_environment, environment) - - return false if ref && params[:recently_updated] && !environment.recently_updated_on_branch?(ref) - return false if ref && commit && !environment.includes_commit?(commit) - - true - end - - def ref - params[:ref].try(:to_s) - end - - def commit - params[:commit] - end - def by_name(environments) if params[:name].present? environments.for_name(params[:name]) diff --git a/app/finders/git_refs_finder.rb b/app/finders/git_refs_finder.rb index 2289b34e562..11af659d37c 100644 --- a/app/finders/git_refs_finder.rb +++ b/app/finders/git_refs_finder.rb @@ -33,15 +33,21 @@ class GitRefsFinder end def filter_refs_with_prefix(refs, prefix) - refs.select { |ref| ref.name.upcase.starts_with?(prefix.upcase) } + prefix = prefix.downcase + + refs.select { |ref| ref.name.downcase.starts_with?(prefix) } end def filter_refs_with_suffix(refs, suffix) - refs.select { |ref| ref.name.upcase.ends_with?(suffix.upcase) } + suffix = suffix.downcase + + refs.select { |ref| ref.name.downcase.ends_with?(suffix) } end def filter_refs_by_name(refs, term) - refs.select { |ref| ref.name.upcase.include?(term.upcase) } + term = term.downcase + + refs.select { |ref| ref.name.downcase.include?(term) } end def set_exact_match_as_first_result(matches, term) diff --git a/app/finders/group_members_finder.rb b/app/finders/group_members_finder.rb index 2417b1e0771..a6ecd835527 100644 --- a/app/finders/group_members_finder.rb +++ b/app/finders/group_members_finder.rb @@ -21,28 +21,13 @@ class GroupMembersFinder < UnionFinder end def execute(include_relations: DEFAULT_RELATIONS) - group_members = group_members_list - relations = [] + return filter_members(group_members_list) if include_relations == [:direct] - return filter_members(group_members) if include_relations == [:direct] + groups = groups_by_relations(include_relations) + return GroupMember.none unless groups - relations << group_members if include_relations.include?(:direct) + members = all_group_members(groups).distinct_on_user_with_max_access_level - if include_relations.include?(:inherited) && group.parent - parents_members = relation_group_members(group.ancestors) - - relations << parents_members - end - - if include_relations.include?(:descendants) - descendant_members = relation_group_members(group.descendants) - - relations << descendant_members - end - - return GroupMember.none if relations.empty? - - members = find_union(relations, GroupMember) filter_members(members) end @@ -50,6 +35,25 @@ class GroupMembersFinder < UnionFinder attr_reader :user, :group + def groups_by_relations(include_relations) + case include_relations.sort + when [:inherited] + group.ancestors + when [:descendants] + group.descendants + when [:direct, :inherited] + group.self_and_ancestors + when [:descendants, :direct] + group.self_and_descendants + when [:descendants, :inherited] + find_union([group.ancestors, group.descendants], Group) + when [:descendants, :direct, :inherited] + group.self_and_hierarchy + else + nil + end + end + def filter_members(members) members = members.search(params[:search]) if params[:search].present? members = members.sort_by_attribute(params[:sort]) if params[:sort].present? @@ -69,17 +73,13 @@ class GroupMembersFinder < UnionFinder group.members end - def relation_group_members(relation) - all_group_members(relation).non_minimal_access + def all_group_members(groups) + members_of_groups(groups).non_minimal_access end - # rubocop: disable CodeReuse/ActiveRecord - def all_group_members(relation) - GroupMember.non_request - .where(source_id: relation.select(:id)) - .where.not(user_id: group.users.select(:id)) + def members_of_groups(groups) + GroupMember.non_request.of_groups(groups) end - # rubocop: enable CodeReuse/ActiveRecord end GroupMembersFinder.prepend_if_ee('EE::GroupMembersFinder') diff --git a/app/finders/group_projects_finder.rb b/app/finders/group_projects_finder.rb index 8362e782ad1..dfdf821e3f0 100644 --- a/app/finders/group_projects_finder.rb +++ b/app/finders/group_projects_finder.rb @@ -48,8 +48,7 @@ class GroupProjectsFinder < ProjectsFinder def filter_projects(collection) projects = super - projects = by_feature_availability(projects) - projects + by_feature_availability(projects) end def limit(collection) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 2409dc9d77d..40a4e2b4f26 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -119,20 +119,18 @@ class IssuableFinder # https://www.postgresql.org/docs/current/static/queries-with.html items = by_search(items) - items = sort(items) - - items + sort(items) end def filter_items(items) + # Selection by group is already covered by `by_project` and `projects` for project-based issuables + # Group-based issuables have their own group filter methods items = by_project(items) - items = by_group(items) items = by_scope(items) items = by_created_at(items) items = by_updated_at(items) items = by_closed_at(items) items = by_state(items) - items = by_group(items) items = by_assignee(items) items = by_author(items) items = by_non_archived(items) @@ -244,7 +242,7 @@ class IssuableFinder # These are "helper" params that modify the results, like :in and :search. They usually come in at the top-level # params, but if they do come in inside the `:not` params, the inner ones should take precedence. - not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].slice(*NEGATABLE_PARAMS_HELPER_KEYS)) + not_helpers = params.slice(*NEGATABLE_PARAMS_HELPER_KEYS).merge(params[:not].to_h.slice(*NEGATABLE_PARAMS_HELPER_KEYS)) not_helpers.each do |key, value| not_params[key] = value unless not_params[key].present? end @@ -320,11 +318,6 @@ class IssuableFinder end end - def by_group(items) - # Selection by group is already covered by `by_project` and `projects` - items - end - # rubocop: disable CodeReuse/ActiveRecord def by_project(items) if params.project? @@ -400,8 +393,6 @@ class IssuableFinder # We want CE users to be able to say "Issues not assigned to either PersonA nor PersonB" if not_params.assignees.present? items.not_assigned_to(not_params.assignees) - elsif not_params.assignee_id? || not_params.assignee_username? # assignee not found - items.none else items end diff --git a/app/finders/issues_finder.rb b/app/finders/issues_finder.rb index 5c9010ee3e0..e1a334413f8 100644 --- a/app/finders/issues_finder.rb +++ b/app/finders/issues_finder.rb @@ -47,6 +47,13 @@ class IssuesFinder < IssuableFinder # rubocop: disable CodeReuse/ActiveRecord def with_confidentiality_access_check return Issue.all if params.user_can_see_all_confidential_issues? + + # If already filtering by assignee we can skip confidentiality since a user + # can always see confidential issues assigned to them. This is just an + # optimization since a very common usecase of this Finder is to load the + # count of issues assigned to the user for the header bar. + return Issue.all if current_user && params.assignees.include?(current_user) + return Issue.where('issues.confidential IS NOT TRUE') if params.user_cannot_see_confidential_issues? Issue.where(' @@ -74,8 +81,7 @@ class IssuesFinder < IssuableFinder issues = super issues = by_due_date(issues) issues = by_confidential(issues) - issues = by_issue_types(issues) - issues + by_issue_types(issues) end def by_confidential(items) diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index bedd6891d02..ecd6270ed47 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -2,6 +2,7 @@ class LabelsFinder < UnionFinder prepend FinderWithCrossProjectAccess + include FinderWithGroupHierarchy include FinderMethods include Gitlab::Utils::StrongMemoize @@ -14,7 +15,7 @@ class LabelsFinder < UnionFinder def execute(skip_authorization: false) @skip_authorization = skip_authorization - items = find_union(label_ids, Label) || Label.none + items = find_union(item_ids, Label) || Label.none items = with_title(items) items = by_subscription(items) items = by_search(items) @@ -26,8 +27,8 @@ class LabelsFinder < UnionFinder attr_reader :current_user, :params, :skip_authorization # rubocop: disable CodeReuse/ActiveRecord - def label_ids - label_ids = [] + def item_ids + item_ids = [] if project? if project @@ -35,25 +36,25 @@ class LabelsFinder < UnionFinder labels_table = Label.arel_table group_ids = group_ids_for(project.group) - label_ids << Label.where( + item_ids << Label.where( labels_table[:type].eq('GroupLabel').and(labels_table[:group_id].in(group_ids)).or( labels_table[:type].eq('ProjectLabel').and(labels_table[:project_id].eq(project.id)) ) ) else - label_ids << project.labels + item_ids << project.labels end end else if group? - label_ids << Label.where(group_id: group_ids_for(group)) + item_ids << Label.where(group_id: group_ids_for(group)) end - label_ids << Label.where(group_id: projects.group_ids) - label_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels? + item_ids << Label.where(group_id: projects.group_ids) + item_ids << Label.where(project_id: ids_user_can_read_labels(projects)) unless only_group_labels? end - label_ids + item_ids end # rubocop: enable CodeReuse/ActiveRecord @@ -94,49 +95,6 @@ class LabelsFinder < UnionFinder params[:subscribed] == 'true' end - # Gets redacted array of group ids - # which can include the ancestors and descendants of the requested group. - def group_ids_for(group) - strong_memoize(:group_ids) do - groups = groups_to_include(group) - - # Because we are sure that all groups are in the same hierarchy tree - # we can preset root group for all of them to optimize permission checks - Group.preset_root_ancestor_for(groups) - - groups_user_can_read_labels(groups).map(&:id) - end - end - - def groups_to_include(group) - groups = [group] - - groups += group.ancestors if include_ancestor_groups? - groups += group.descendants if include_descendant_groups? - - groups - end - - def include_ancestor_groups? - params[:include_ancestor_groups] - end - - def include_descendant_groups? - params[:include_descendant_groups] - end - - def group? - params[:group].present? || params[:group_id].present? - end - - def group - strong_memoize(:group) { params[:group].presence || Group.find(params[:group_id]) } - end - - def project? - params[:project].present? || params[:project_id].present? - end - def projects? params[:project_ids] end @@ -153,12 +111,16 @@ class LabelsFinder < UnionFinder params[:title] || params[:name] end + def project? + params[:project].present? || params[:project_id].present? + end + def project return @project if defined?(@project) if project? @project = params[:project] || Project.find(params[:project_id]) - @project = nil unless authorized_to_read_labels?(@project) + @project = nil unless authorized_to_read_item?(@project) else @project = nil end @@ -191,16 +153,8 @@ class LabelsFinder < UnionFinder end # rubocop: enable CodeReuse/ActiveRecord - def authorized_to_read_labels?(label_parent) - return true if skip_authorization - - Ability.allowed?(current_user, :read_label, label_parent) - end - - def groups_user_can_read_labels(groups) - DeclarativePolicy.user_scope do - groups.select { |group| authorized_to_read_labels?(group) } - end + def read_permission + :read_label end # rubocop: disable CodeReuse/ActiveRecord diff --git a/app/finders/merge_request/metrics_finder.rb b/app/finders/merge_request/metrics_finder.rb index d93e53d1636..1a3732bbdf9 100644 --- a/app/finders/merge_request/metrics_finder.rb +++ b/app/finders/merge_request/metrics_finder.rb @@ -14,9 +14,7 @@ class MergeRequest::MetricsFinder items = init_collection items = by_target_project(items) items = by_merged_after(items) - items = by_merged_before(items) - - items + by_merged_before(items) end private diff --git a/app/finders/merge_requests/by_approvals_finder.rb b/app/finders/merge_requests/by_approvals_finder.rb index e6ab1467f06..94f13468327 100644 --- a/app/finders/merge_requests/by_approvals_finder.rb +++ b/app/finders/merge_requests/by_approvals_finder.rb @@ -60,14 +60,14 @@ module MergeRequests ids.first.to_s.downcase == label || usernames.map(&:downcase).include?(label) end - # Merge Requests without any approval + # Merge requests without any approval # # @param [ActiveRecord::Relation] items def without_approvals(items) items.without_approvals end - # Merge Requests with any number of approvals + # Merge requests with any number of approvals # # @param [ActiveRecord::Relation] items the activerecord relation def with_any_approvals(items) @@ -76,14 +76,14 @@ module MergeRequests ]) end - # Merge Requests approved by given usernames + # Merge requests approved by given usernames # # @param [ActiveRecord::Relation] items the activerecord relation def find_approved_by_names(items) items.approved_by_users_with_usernames(*usernames) end - # Merge Requests approved by given user IDs + # Merge requests approved by given user IDs # # @param [ActiveRecord::Relation] items the activerecord relation def find_approved_by_ids(items) diff --git a/app/finders/merge_requests/oldest_per_commit_finder.rb b/app/finders/merge_requests/oldest_per_commit_finder.rb index 5360f301036..5da7a08e36c 100644 --- a/app/finders/merge_requests/oldest_per_commit_finder.rb +++ b/app/finders/merge_requests/oldest_per_commit_finder.rb @@ -18,8 +18,8 @@ module MergeRequests mapping = {} shas = commits.map(&:id) - # To include merge requests by the commit SHA, we don't need to go through - # any diff rows. + # To include merge requests by the merge/squash SHA, we don't need to go + # through any diff rows. # # We can't squeeze all this into a single query, as the diff based data # relies on a GROUP BY. On the other hand, retrieving MRs by their merge @@ -27,12 +27,17 @@ module MergeRequests @project .merge_requests .preload_target_project - .by_merge_commit_sha(shas) + .by_merge_or_squash_commit_sha(shas) .each do |mr| - # Merge SHAs can't be in the merge request itself. It _is_ possible a - # newer merge request includes the merge commit, but in that case we - # still want the oldest merge request. - mapping[mr.merge_commit_sha] = mr + # Merge/squash SHAs can't be in the merge request itself. It _is_ + # possible a newer merge request includes the commit, but in that case + # we still want the oldest merge request. + # + # It's also possible that a merge request produces both a squashed + # commit and a merge commit. In that case we want to store the mapping + # for both the SHAs. + mapping[mr.squash_commit_sha] = mr if mr.squash_commit_sha + mapping[mr.merge_commit_sha] = mr if mr.merge_commit_sha end remaining = shas - mapping.keys diff --git a/app/finders/metrics/dashboards/annotations_finder.rb b/app/finders/metrics/dashboards/annotations_finder.rb index c42b8bf40e5..e97704738ea 100644 --- a/app/finders/metrics/dashboards/annotations_finder.rb +++ b/app/finders/metrics/dashboards/annotations_finder.rb @@ -4,7 +4,8 @@ module Metrics module Dashboards class AnnotationsFinder def initialize(dashboard:, params:) - @dashboard, @params = dashboard, params + @dashboard = dashboard + @params = params end def execute diff --git a/app/finders/metrics/users_starred_dashboards_finder.rb b/app/finders/metrics/users_starred_dashboards_finder.rb index 7244c51f9a7..2ef706c1b11 100644 --- a/app/finders/metrics/users_starred_dashboards_finder.rb +++ b/app/finders/metrics/users_starred_dashboards_finder.rb @@ -3,7 +3,9 @@ module Metrics class UsersStarredDashboardsFinder def initialize(user:, project:, params: {}) - @user, @project, @params = user, project, params + @user = user + @project = project + @params = params end def execute diff --git a/app/finders/namespaces/projects_finder.rb b/app/finders/namespaces/projects_finder.rb index a6d98015e9d..bac5328d077 100644 --- a/app/finders/namespaces/projects_finder.rb +++ b/app/finders/namespaces/projects_finder.rb @@ -39,8 +39,7 @@ module Namespaces def filter_projects(collection) collection = by_ids(collection) - collection = by_similarity(collection) - collection + by_similarity(collection) end def by_ids(items) diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb index 1a3f011d9eb..96966001e85 100644 --- a/app/finders/notes_finder.rb +++ b/app/finders/notes_finder.rb @@ -17,6 +17,7 @@ class NotesFinder # target_id: integer # last_fetched_at: time # search: string + # sort: string # def initialize(current_user, params = {}) @project = params[:project] @@ -29,8 +30,7 @@ class NotesFinder notes = init_collection notes = since_fetch_at(notes) notes = notes.with_notes_filter(@params[:notes_filter]) if notes_filter? - - notes.fresh + sort(notes) end def target @@ -173,6 +173,14 @@ class NotesFinder def notes_filter? @params[:notes_filter].present? end + + def sort(notes) + sort = @params[:sort].presence + + return notes.fresh unless sort + + notes.order_by(sort) + end end NotesFinder.prepend_if_ee('EE::NotesFinder') diff --git a/app/finders/packages/conan/package_file_finder.rb b/app/finders/packages/conan/package_file_finder.rb index edf35388a36..a1ebf9f40fa 100644 --- a/app/finders/packages/conan/package_file_finder.rb +++ b/app/finders/packages/conan/package_file_finder.rb @@ -8,8 +8,7 @@ module Packages def package_files files = super files = by_conan_file_type(files) - files = by_conan_package_reference(files) - files + by_conan_package_reference(files) end def by_conan_file_type(files) diff --git a/app/finders/packages/debian/distributions_finder.rb b/app/finders/packages/debian/distributions_finder.rb index e64b6bdfec1..a5ac9f7e2f7 100644 --- a/app/finders/packages/debian/distributions_finder.rb +++ b/app/finders/packages/debian/distributions_finder.rb @@ -4,15 +4,15 @@ module Packages module Debian class DistributionsFinder def initialize(container, params = {}) - @container, @params = container, params + @container = container + @params = params end def execute collection = relation.with_container(container) collection = by_codename(collection) collection = by_suite(collection) - collection = by_codename_or_suite(collection) - collection + by_codename_or_suite(collection) end private diff --git a/app/finders/packages/go/package_finder.rb b/app/finders/packages/go/package_finder.rb new file mode 100644 index 00000000000..4573417d11f --- /dev/null +++ b/app/finders/packages/go/package_finder.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Packages + module Go + class PackageFinder + delegate :exists?, to: :candidates + + def initialize(project, module_name, module_version) + @project = project + @module_name = module_name + @module_version = module_version + end + + def execute + candidates.first + end + + private + + def candidates + @project + .packages + .golang + .with_name(@module_name) + .with_version(@module_version) + end + end + end +end diff --git a/app/finders/packages/go/version_finder.rb b/app/finders/packages/go/version_finder.rb index 8e2fab8ba35..6ee02b8c6f6 100644 --- a/app/finders/packages/go/version_finder.rb +++ b/app/finders/packages/go/version_finder.rb @@ -23,7 +23,8 @@ module Packages when String if pseudo_version? target semver = parse_semver(target) - commit = pseudo_version_commit(@mod.project, semver) + version = parse_pseudo_version(semver) + commit = validate_pseudo_version(@mod.project, version) Packages::Go::ModuleVersion.new(@mod, :pseudo, commit, name: target, semver: semver) else @mod.version_by(ref: target) diff --git a/app/finders/packages/group_packages_finder.rb b/app/finders/packages/group_packages_finder.rb index db5161d6e16..8771bf90e75 100644 --- a/app/finders/packages/group_packages_finder.rb +++ b/app/finders/packages/group_packages_finder.rb @@ -32,8 +32,7 @@ module Packages packages = filter_with_version(packages) packages = filter_by_package_type(packages) packages = filter_by_package_name(packages) - packages = filter_by_status(packages) - packages + filter_by_status(packages) end def group_projects_visible_to_current_user diff --git a/app/finders/packages/maven/package_finder.rb b/app/finders/packages/maven/package_finder.rb index ba3d4631f55..eefcdaba3c8 100644 --- a/app/finders/packages/maven/package_finder.rb +++ b/app/finders/packages/maven/package_finder.rb @@ -3,13 +3,15 @@ module Packages module Maven class PackageFinder - attr_reader :path, :current_user, :project, :group + include ::Packages::FinderHelper + include Gitlab::Utils::StrongMemoize - def initialize(path, current_user, project: nil, group: nil) + def initialize(path, current_user, project: nil, group: nil, order_by_package_file: false) @path = path @current_user = current_user @project = project @group = group + @order_by_package_file = order_by_package_file end def execute @@ -23,9 +25,9 @@ module Packages private def base - if project + if @project packages_for_a_single_project - elsif group + elsif @group packages_for_multiple_projects else ::Packages::Package.none @@ -33,8 +35,13 @@ module Packages end def packages_with_path - matching_packages = base.only_maven_packages_with_path(path) - matching_packages = matching_packages.order_by_package_file if versionless_package?(matching_packages) + matching_packages = base.only_maven_packages_with_path(@path, use_cte: @group.present?) + + if group_level_improvements? + matching_packages = matching_packages.order_by_package_file if @order_by_package_file + else + matching_packages = matching_packages.order_by_package_file if versionless_package?(matching_packages) + end matching_packages end @@ -48,19 +55,29 @@ module Packages # Produces a query that retrieves packages from a single project. def packages_for_a_single_project - project.packages + @project.packages end # Produces a query that retrieves packages from multiple projects that # the current user can view within a group. def packages_for_multiple_projects - ::Packages::Package.for_projects(projects_visible_to_current_user) + if group_level_improvements? + packages_visible_to_user(@current_user, within_group: @group) + else + ::Packages::Package.for_projects(projects_visible_to_current_user) + end end # Returns the projects that the current user can view within a group. def projects_visible_to_current_user - group.all_projects - .public_or_visible_to_user(current_user) + @group.all_projects + .public_or_visible_to_user(@current_user) + end + + def group_level_improvements? + strong_memoize(:group_level_improvements) do + Feature.enabled?(:maven_packages_group_level_improvements, default_enabled: :yaml) + end end end end diff --git a/app/finders/packages/package_file_finder.rb b/app/finders/packages/package_file_finder.rb index d015f4adfa6..792ffa0591b 100644 --- a/app/finders/packages/package_file_finder.rb +++ b/app/finders/packages/package_file_finder.rb @@ -21,9 +21,7 @@ class Packages::PackageFileFinder def package_files files = package.package_files - files = by_file_name(files) - - files + by_file_name(files) end def by_file_name(files) diff --git a/app/finders/packages/packages_finder.rb b/app/finders/packages/packages_finder.rb index bd9e62e3f2a..840cbbf7b9d 100644 --- a/app/finders/packages/packages_finder.rb +++ b/app/finders/packages/packages_finder.rb @@ -22,8 +22,7 @@ module Packages packages = filter_by_package_type(packages) packages = filter_by_package_name(packages) packages = filter_by_status(packages) - packages = order_packages(packages) - packages + order_packages(packages) end private diff --git a/app/finders/pending_todos_finder.rb b/app/finders/pending_todos_finder.rb index c21d90c9182..d79a2340379 100644 --- a/app/finders/pending_todos_finder.rb +++ b/app/finders/pending_todos_finder.rb @@ -11,23 +11,22 @@ # change the various `by_*` methods in this finder, without having to touch # everything that uses it. class PendingTodosFinder - attr_reader :current_user, :params + attr_reader :users, :params - # current_user - The user to retrieve the todos for. + # users - The list of users to retrieve the todos for. # params - A Hash containing columns and values to use for filtering todos. - def initialize(current_user, params = {}) - @current_user = current_user + def initialize(users, params = {}) + @users = users @params = params end def execute - todos = current_user.todos.pending + todos = Todo.for_user(users) + todos = todos.pending todos = by_project(todos) todos = by_target_id(todos) todos = by_target_type(todos) - todos = by_commit_id(todos) - - todos + by_commit_id(todos) end def by_project(todos) diff --git a/app/finders/projects/export_job_finder.rb b/app/finders/projects/export_job_finder.rb index c26a7a3f1a6..c270feb23dc 100644 --- a/app/finders/projects/export_job_finder.rb +++ b/app/finders/projects/export_job_finder.rb @@ -12,9 +12,7 @@ module Projects def execute export_jobs = project.export_jobs - export_jobs = by_status(export_jobs) - - export_jobs + by_status(export_jobs) end private diff --git a/app/finders/projects/prometheus/alerts_finder.rb b/app/finders/projects/prometheus/alerts_finder.rb index 3e3b72647c5..2105516db5f 100644 --- a/app/finders/projects/prometheus/alerts_finder.rb +++ b/app/finders/projects/prometheus/alerts_finder.rb @@ -30,9 +30,7 @@ module Projects relation = by_environment(relation) relation = by_metric(relation) relation = by_id(relation) - relation = ordered(relation) - - relation + ordered(relation) end private diff --git a/app/finders/projects_finder.rb b/app/finders/projects_finder.rb index f1df4fbb0d8..893e89daa3c 100644 --- a/app/finders/projects_finder.rb +++ b/app/finders/projects_finder.rb @@ -83,8 +83,7 @@ class ProjectsFinder < UnionFinder collection = by_deleted_status(collection) collection = by_last_activity_after(collection) collection = by_last_activity_before(collection) - collection = by_repository_storage(collection) - collection + by_repository_storage(collection) end def collection_with_user @@ -131,7 +130,7 @@ class ProjectsFinder < UnionFinder public_visibility_levels = Gitlab::VisibilityLevel.levels_for_user(current_user) - !public_visibility_levels.include?(params[:visibility_level]) + !public_visibility_levels.include?(params[:visibility_level].to_i) end def owned_projects? diff --git a/app/finders/prometheus_metrics_finder.rb b/app/finders/prometheus_metrics_finder.rb index 84a071abbd5..152ee73ef26 100644 --- a/app/finders/prometheus_metrics_finder.rb +++ b/app/finders/prometheus_metrics_finder.rb @@ -36,9 +36,7 @@ class PrometheusMetricsFinder metrics = by_common(metrics) metrics = by_ordered(metrics) metrics = by_identifier(metrics) - metrics = by_id(metrics) - - metrics + by_id(metrics) end private diff --git a/app/finders/protected_branches_finder.rb b/app/finders/protected_branches_finder.rb index 68e8d2a9f54..a452a1f993b 100644 --- a/app/finders/protected_branches_finder.rb +++ b/app/finders/protected_branches_finder.rb @@ -20,9 +20,7 @@ class ProtectedBranchesFinder def execute protected_branches = project.limited_protected_branches(LIMIT) - protected_branches = by_name(protected_branches) - - protected_branches + by_name(protected_branches) end private diff --git a/app/finders/releases_finder.rb b/app/finders/releases_finder.rb index da72178169e..0cfa4310ab7 100644 --- a/app/finders/releases_finder.rb +++ b/app/finders/releases_finder.rb @@ -20,8 +20,7 @@ class ReleasesFinder releases = get_releases releases = by_tag(releases) releases = releases.preloaded if preload - releases = order_releases(releases) - releases + order_releases(releases) end private diff --git a/app/finders/repositories/branch_names_finder.rb b/app/finders/repositories/branch_names_finder.rb new file mode 100644 index 00000000000..5bb67425aa5 --- /dev/null +++ b/app/finders/repositories/branch_names_finder.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Repositories + class BranchNamesFinder + attr_reader :repository, :params + + def initialize(repository, params = {}) + @repository = repository + @params = params + end + + def execute + return unless search + + repository.search_branch_names(search) + end + + private + + def search + @params[:search].presence + end + end +end diff --git a/app/finders/repositories/changelog_tag_finder.rb b/app/finders/repositories/changelog_tag_finder.rb new file mode 100644 index 00000000000..3c110e6c65d --- /dev/null +++ b/app/finders/repositories/changelog_tag_finder.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Repositories + # A finder class for getting the tag of the last release before a given + # version, used when generating changelogs. + # + # Imagine a project with the following tags: + # + # * v1.0.0 + # * v1.1.0 + # * v2.0.0 + # + # If the version supplied is 2.1.0, the tag returned will be v2.0.0. And when + # the version is 1.1.1, or 1.2.0, the returned tag will be v1.1.0. + # + # To obtain the tags, this finder requires a regular expression (using the re2 + # syntax) to be provided. This regex must produce the following named + # captures: + # + # - major (required) + # - minor (required) + # - patch (required) + # - pre + # - meta + # + # If the `pre` group has a value, the tag is ignored. If any of the required + # capture groups don't have a value, the tag is also ignored. + class ChangelogTagFinder + def initialize(project, regex: Gitlab::Changelog::Config::DEFAULT_TAG_REGEX) + @project = project + @regex = regex + end + + def execute(new_version) + tags = {} + versions = [new_version] + + begin + regex = Gitlab::UntrustedRegexp.new(@regex) + rescue RegexpError => ex + # The error messages produced by default are not very helpful, so we + # raise a better one here. We raise the specific error here so its + # message is displayed in the API (where we catch this specific + # error). + raise( + Gitlab::Changelog::Error, + "The regular expression to use for finding the previous tag for a version is invalid: #{ex.message}" + ) + end + + @project.repository.tags.each do |tag| + matches = regex.match(tag.name) + + next unless matches + + # When using this class for generating changelog data for a range of + # commits, we want to compare against the tag of the last _stable_ + # release; not some random RC that came after that. + next if matches[:pre] + + major = matches[:major] + minor = matches[:minor] + patch = matches[:patch] + build = matches[:meta] + + next unless major && minor && patch + + version = "#{major}.#{minor}.#{patch}" + version += "+#{build}" if build + + tags[version] = tag + versions << version + end + + VersionSorter.sort!(versions) + + index = versions.index(new_version) + + tags[versions[index - 1]] if index&.positive? + end + end +end diff --git a/app/finders/repositories/previous_tag_finder.rb b/app/finders/repositories/previous_tag_finder.rb deleted file mode 100644 index b5e786c30e9..00000000000 --- a/app/finders/repositories/previous_tag_finder.rb +++ /dev/null @@ -1,57 +0,0 @@ -# frozen_string_literal: true - -module Repositories - # A finder class for getting the tag of the last release before a given - # version. - # - # Imagine a project with the following tags: - # - # * v1.0.0 - # * v1.1.0 - # * v2.0.0 - # - # If the version supplied is 2.1.0, the tag returned will be v2.0.0. And when - # the version is 1.1.1, or 1.2.0, the returned tag will be v1.1.0. - # - # This finder expects that all tags to consider meet the following - # requirements: - # - # * They start with the letter "v" followed by a version, or immediately start - # with a version - # * They use semantic versioning for the version format - # - # Tags not meeting these requirements are ignored. - class PreviousTagFinder - TAG_REGEX = /\Av?(?<version>#{Gitlab::Regex.unbounded_semver_regex})\z/.freeze - - def initialize(project) - @project = project - end - - def execute(new_version) - tags = {} - versions = [new_version] - - @project.repository.tags.each do |tag| - matches = tag.name.match(TAG_REGEX) - - next unless matches - - # When using this class for generating changelog data for a range of - # commits, we want to compare against the tag of the last _stable_ - # release; not some random RC that came after that. - next if matches[:prerelease] - - version = matches[:version] - tags[version] = tag - versions << version - end - - VersionSorter.sort!(versions) - - index = versions.index(new_version) - - tags[versions[index - 1]] if index&.positive? - end - end -end diff --git a/app/finders/tags_finder.rb b/app/finders/tags_finder.rb index fd58f478b45..d9848d027cf 100644 --- a/app/finders/tags_finder.rb +++ b/app/finders/tags_finder.rb @@ -7,7 +7,6 @@ class TagsFinder < GitRefsFinder def execute tags = repository.tags_sorted_by(sort) - tags = by_search(tags) - tags + by_search(tags) end end diff --git a/app/finders/user_group_notification_settings_finder.rb b/app/finders/user_group_notification_settings_finder.rb index a6f6769116f..4ad9d1d7bf4 100644 --- a/app/finders/user_group_notification_settings_finder.rb +++ b/app/finders/user_group_notification_settings_finder.rb @@ -14,6 +14,8 @@ class UserGroupNotificationSettingsFinder @loaded_groups_with_ancestors = groups_with_ancestors.index_by(&:id) @loaded_notification_settings = user.notification_settings_for_groups(groups_with_ancestors).preload_source_route.index_by(&:source_id) + preload_emails_disabled + groups.map do |group| find_notification_setting_for(group) end @@ -45,4 +47,19 @@ class UserGroupNotificationSettingsFinder parent_setting.level != NotificationSetting.levels[:global] || parent_setting.notification_email.present? end + + # This method preloads the `emails_disabled` strong memoized method for the given groups. + # + # For each group, look up the ancestor hierarchy and look for any group where emails_disabled is true. + # The lookup is implemented with an EXISTS subquery, so we can look up the ancestor chain for each group individually. + # The query will return groups where at least one ancestor has the `emails_disabled` set to true. + # + # After the query, we set the instance variable. + def preload_emails_disabled + group_ids_with_disabled_email = Group.ids_with_disabled_email(groups.to_a) + + groups.each do |group| + group.emails_disabled_memoized = group_ids_with_disabled_email.include?(group.id) if group.parent_id + end + end end diff --git a/app/finders/users_star_projects_finder.rb b/app/finders/users_star_projects_finder.rb index 49c4e087b4b..7a7587c8631 100644 --- a/app/finders/users_star_projects_finder.rb +++ b/app/finders/users_star_projects_finder.rb @@ -15,9 +15,7 @@ class UsersStarProjectsFinder stars = UsersStarProject.all stars = by_project(stars) stars = by_search(stars) - stars = filter_visible_profiles(stars) - - stars + filter_visible_profiles(stars) end private |