diff options
| author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
|---|---|---|
| committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-08-20 21:42:06 +0300 |
| commit | 6e4e1050d9dba2b7b2523fdd1768823ab85feef4 (patch) | |
| tree | 78be5963ec075d80116a932011d695dd33910b4e /app/services | |
| parent | 1ce776de4ae122aba3f349c02c17cebeaa8ecf07 (diff) | |
Add latest changes from gitlab-org/gitlab@13-3-stable-ee
Diffstat (limited to 'app/services')
122 files changed, 1227 insertions, 843 deletions
diff --git a/app/services/admin/propagate_integration_service.rb b/app/services/admin/propagate_integration_service.rb index e21bb03ed68..9a5ce58ee2c 100644 --- a/app/services/admin/propagate_integration_service.rb +++ b/app/services/admin/propagate_integration_service.rb @@ -96,7 +96,7 @@ module Admin # rubocop: disable CodeReuse/ActiveRecord def run_callbacks(batch) - if active_external_issue_tracker? + if integration.issue_tracker? Project.where(id: batch).update_all(has_external_issue_tracker: true) end @@ -106,10 +106,6 @@ module Admin end # rubocop: enable CodeReuse/ActiveRecord - def active_external_issue_tracker? - integration.issue_tracker? && !integration.default - end - def active_external_wiki? integration.type == 'ExternalWikiService' end diff --git a/app/services/alert_management/alerts/update_service.rb b/app/services/alert_management/alerts/update_service.rb index 0b7216cd9f8..18d615aa7e7 100644 --- a/app/services/alert_management/alerts/update_service.rb +++ b/app/services/alert_management/alerts/update_service.rb @@ -96,12 +96,12 @@ module AlertManagement end def handle_assignement(old_assignees) - assign_todo + assign_todo(old_assignees) add_assignee_system_note(old_assignees) end - def assign_todo - todo_service.assign_alert(alert, current_user) + def assign_todo(old_assignees) + todo_service.reassigned_assignable(alert, current_user, old_assignees) end def add_assignee_system_note(old_assignees) diff --git a/app/services/alert_management/create_alert_issue_service.rb b/app/services/alert_management/create_alert_issue_service.rb index 6ea3fd867ef..f16b106b748 100644 --- a/app/services/alert_management/create_alert_issue_service.rb +++ b/app/services/alert_management/create_alert_issue_service.rb @@ -15,10 +15,10 @@ module AlertManagement return error_no_permissions unless allowed? return error_issue_already_exists if alert.issue - result = create_issue - issue = result.payload[:issue] + result = create_incident + return result unless result.success? - return error(result.message, issue) if result.error? + issue = result.payload[:issue] return error(object_errors(alert), issue) unless associate_alert_with_issue(issue) SystemNoteService.new_alert_issue(alert, issue, user) @@ -36,35 +36,19 @@ module AlertManagement user.can?(:create_issue, project) end - def create_issue - label_result = find_or_create_incident_label - - # Create an unlabelled issue if we couldn't create the label - # due to a race condition. - # See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042 - extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {} - - issue = Issues::CreateService.new( + def create_incident + ::IncidentManagement::Incidents::CreateService.new( project, user, title: alert_presenter.title, - description: alert_presenter.issue_description, - **extra_params + description: alert_presenter.issue_description ).execute - - return error(object_errors(issue), issue) unless issue.valid? - - success(issue) end def associate_alert_with_issue(issue) alert.update(issue_id: issue.id) end - def success(issue) - ServiceResponse.success(payload: { issue: issue }) - end - def error(message, issue = nil) ServiceResponse.error(payload: { issue: issue }, message: message) end @@ -83,10 +67,6 @@ module AlertManagement end end - def find_or_create_incident_label - IncidentManagement::CreateIncidentLabelService.new(project, user).execute - end - def object_errors(object) object.errors.full_messages.to_sentence end diff --git a/app/services/award_emojis/copy_service.rb b/app/services/award_emojis/copy_service.rb new file mode 100644 index 00000000000..2e500d4c697 --- /dev/null +++ b/app/services/award_emojis/copy_service.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +# This service copies AwardEmoji from one Awardable to another. +# +# It expects the calling code to have performed the necessary authorization +# checks in order to allow the copy to happen. +module AwardEmojis + class CopyService + def initialize(from_awardable, to_awardable) + raise ArgumentError, 'Awardables must be different' if from_awardable == to_awardable + + @from_awardable = from_awardable + @to_awardable = to_awardable + end + + def execute + from_awardable.award_emoji.find_each do |award| + new_award = award.dup + new_award.awardable = to_awardable + new_award.save! + end + + ServiceResponse.success + end + + private + + attr_accessor :from_awardable, :to_awardable + end +end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index e08509b84db..140420a32bd 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -45,6 +45,12 @@ module Boards # rubocop: enable CodeReuse/ActiveRecord def filter(issues) + # when grouping board issues by epics (used in board swimlanes) + # we need to get all issues in the board + # TODO: ignore hidden columns - + # https://gitlab.com/gitlab-org/gitlab/-/issues/233870 + return issues if params[:all_lists] + issues = without_board_labels(issues) unless list&.movable? || list&.closed? issues = with_list_label(issues) if list&.label? issues @@ -55,9 +61,17 @@ module Boards end def list - return @list if defined?(@list) + return unless params.key?(:id) + + strong_memoize(:list) do + id = params[:id] - @list = board.lists.find(params[:id]) if params.key?(:id) + if board.lists.loaded? + board.lists.find { |l| l.id == id } + else + board.lists.find(id) + end + end end def filter_params @@ -79,6 +93,8 @@ module Boards end def set_state + return if params[:all_lists] + params[:state] = list && list.closed? ? 'closed' : 'opened' end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 9e3c84d03ec..14e8683ebdf 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -130,7 +130,7 @@ module Boards def move_between_ids(move_params) ids = [move_params[:move_after_id], move_params[:move_before_id]] .map(&:to_i) - .map { |m| m.positive? ? m : nil } + .map { |m| m > 0 ? m : nil } ids.any? ? ids : nil end diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index 6f9a063cb16..9c7a165776e 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -7,34 +7,39 @@ module Boards def execute(board) List.transaction do - target = target(board) - position = next_position(board) - create_list(board, type, target, position) + case type + when :backlog + create_backlog(board) + else + target = target(board) + position = next_position(board) + + create_list(board, type, target, position) + end end end private def type - :label + # We don't ever expect to have more than one list + # type param at once. + if params.key?('backlog') + :backlog + else + :label + end end def target(board) strong_memoize(:target) do - available_labels_for(board).find(params[:label_id]) + available_labels.find(params[:label_id]) end end - def available_labels_for(board) - options = { include_ancestor_groups: true } - - if board.group_board? - options.merge!(group_id: parent.id, only_group_labels: true) - else - options[:project_id] = parent.id - end - - LabelsFinder.new(current_user, options).execute + def available_labels + ::Labels::AvailableLabelsService.new(current_user, parent, {}) + .available_labels end def next_position(board) @@ -49,6 +54,12 @@ module Boards def create_list_attributes(type, target, position) { type => target, list_type: type, position: position } end + + def create_backlog(board) + return board.lists.backlog.first if board.lists.backlog.exists? + + board.lists.create(list_type: :backlog, position: nil) + end end end end diff --git a/app/services/boards/lists/list_service.rb b/app/services/boards/lists/list_service.rb index 07ce58b6851..e4c789c4597 100644 --- a/app/services/boards/lists/list_service.rb +++ b/app/services/boards/lists/list_service.rb @@ -8,7 +8,8 @@ module Boards board.lists.create(list_type: :backlog) end - board.lists.preload_associated_models + lists = board.lists.preload_associated_models + params[:list_id].present? ? lists.where(id: params[:list_id]) : lists # rubocop: disable CodeReuse/ActiveRecord end end end diff --git a/app/services/branches/create_service.rb b/app/services/branches/create_service.rb index 958dd5c9965..8684da701db 100644 --- a/app/services/branches/create_service.rb +++ b/app/services/branches/create_service.rb @@ -16,8 +16,9 @@ module Branches else error("Invalid reference name: #{ref}") end - rescue Gitlab::Git::PreReceiveError => ex - error(ex.message) + rescue Gitlab::Git::PreReceiveError => e + Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, branch_name: branch_name, ref: ref) + error(e.message) end def success(branch) diff --git a/app/services/ci/build_report_result_service.rb b/app/services/ci/build_report_result_service.rb index 758ba1c73bf..ca66ad8249d 100644 --- a/app/services/ci/build_report_result_service.rb +++ b/app/services/ci/build_report_result_service.rb @@ -3,7 +3,6 @@ module Ci class BuildReportResultService def execute(build) - return unless Feature.enabled?(:build_report_summary, build.project) return unless build.has_test_reports? build.report_results.create!( diff --git a/app/services/ci/change_variable_service.rb b/app/services/ci/change_variable_service.rb new file mode 100644 index 00000000000..f515a335d54 --- /dev/null +++ b/app/services/ci/change_variable_service.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Ci + class ChangeVariableService < BaseContainerService + def execute + case params[:action] + when :create + container.variables.create(params[:variable_params]) + when :update + variable.tap do |target_variable| + target_variable.update(params[:variable_params].except(:key)) + end + when :destroy + variable.tap do |target_variable| + target_variable.destroy + end + end + end + + private + + def variable + params[:variable] || find_variable + end + + def find_variable + identifier = params[:variable_params].slice(:id).presence || params[:variable_params].slice(:key) + container.variables.find_by!(identifier) # rubocop:disable CodeReuse/ActiveRecord + end + end +end + +::Ci::ChangeVariableService.prepend_if_ee('EE::Ci::ChangeVariableService') diff --git a/app/services/ci/change_variables_service.rb b/app/services/ci/change_variables_service.rb new file mode 100644 index 00000000000..3337eb09411 --- /dev/null +++ b/app/services/ci/change_variables_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module Ci + class ChangeVariablesService < BaseContainerService + def execute + container.update(params) + end + end +end + +::Ci::ChangeVariablesService.prepend_if_ee('EE::Ci::ChangeVariablesService') diff --git a/app/services/ci/create_cross_project_pipeline_service.rb b/app/services/ci/create_cross_project_pipeline_service.rb index 1700312b941..23207d809d4 100644 --- a/app/services/ci/create_cross_project_pipeline_service.rb +++ b/app/services/ci/create_cross_project_pipeline_service.rb @@ -98,7 +98,7 @@ module Ci end def can_update_branch?(target_ref) - ::Gitlab::UserAccess.new(current_user, project: downstream_project).can_update_branch?(target_ref) + ::Gitlab::UserAccess.new(current_user, container: downstream_project).can_update_branch?(target_ref) end def downstream_project diff --git a/app/services/ci/create_job_artifacts_service.rb b/app/services/ci/create_job_artifacts_service.rb index 9a6e103e5dd..cd3807e0495 100644 --- a/app/services/ci/create_job_artifacts_service.rb +++ b/app/services/ci/create_job_artifacts_service.rb @@ -25,7 +25,7 @@ module Ci if lsif?(artifact_type) headers[:ProcessLsif] = true - headers[:ProcessLsifReferences] = Feature.enabled?(:code_navigation_references, project, default_enabled: false) + headers[:ProcessLsifReferences] = Feature.enabled?(:code_navigation_references, project, default_enabled: true) end success(headers: headers) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 2d7f5014aa9..70ad18e80eb 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -19,9 +19,13 @@ module Ci Gitlab::Ci::Pipeline::Chain::Limit::Size, Gitlab::Ci::Pipeline::Chain::Validate::External, Gitlab::Ci::Pipeline::Chain::Populate, + Gitlab::Ci::Pipeline::Chain::StopDryRun, Gitlab::Ci::Pipeline::Chain::Create, Gitlab::Ci::Pipeline::Chain::Limit::Activity, - Gitlab::Ci::Pipeline::Chain::Limit::JobActivity].freeze + Gitlab::Ci::Pipeline::Chain::Limit::JobActivity, + Gitlab::Ci::Pipeline::Chain::CancelPendingPipelines, + Gitlab::Ci::Pipeline::Chain::Metrics, + Gitlab::Ci::Pipeline::Chain::Pipeline::Process].freeze # Create a new pipeline in the specified project. # @@ -68,21 +72,14 @@ module Ci bridge: bridge, **extra_options(options)) - sequence = Gitlab::Ci::Pipeline::Chain::Sequence - .new(pipeline, command, SEQUENCE) + # Ensure we never persist the pipeline when dry_run: true + @pipeline.readonly! if command.dry_run? - sequence.build! do |pipeline, sequence| - schedule_head_pipeline_update + Gitlab::Ci::Pipeline::Chain::Sequence + .new(pipeline, command, SEQUENCE) + .build! - if sequence.complete? - cancel_pending_pipelines if project.auto_cancel_pending_pipelines? - pipeline_created_counter.increment(source: source) - - Ci::ProcessPipelineService - .new(pipeline) - .execute(nil, initial_process: true) - end - end + schedule_head_pipeline_update if pipeline.persisted? # If pipeline is not persisted, try to recover IID pipeline.reset_project_iid unless pipeline.persisted? || @@ -110,38 +107,14 @@ module Ci commit.try(:id) end - def cancel_pending_pipelines - Gitlab::OptimisticLocking.retry_lock(auto_cancelable_pipelines) do |cancelables| - cancelables.find_each do |cancelable| - cancelable.auto_cancel_running(pipeline) - end - end - end - - # rubocop: disable CodeReuse/ActiveRecord - def auto_cancelable_pipelines - project.ci_pipelines - .where(ref: pipeline.ref) - .where.not(id: pipeline.same_family_pipeline_ids) - .where.not(sha: project.commit(pipeline.ref).try(:id)) - .alive_or_scheduled - .with_only_interruptible_builds - end - # rubocop: enable CodeReuse/ActiveRecord - - def pipeline_created_counter - @pipeline_created_counter ||= Gitlab::Metrics - .counter(:pipelines_created_total, "Counter of pipelines created") - end - def schedule_head_pipeline_update pipeline.all_merge_requests.opened.each do |merge_request| UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end end - def extra_options(content: nil) - { content: content } + def extra_options(content: nil, dry_run: false) + { content: content, dry_run: dry_run } end end end diff --git a/app/services/ci/create_web_ide_terminal_service.rb b/app/services/ci/create_web_ide_terminal_service.rb index 29d40756ab4..4f1bf0447d2 100644 --- a/app/services/ci/create_web_ide_terminal_service.rb +++ b/app/services/ci/create_web_ide_terminal_service.rb @@ -32,7 +32,7 @@ module Ci Ci::ProcessPipelineService .new(pipeline) - .execute(nil, initial_process: true) + .execute pipeline_created_counter.increment(source: :webide) end diff --git a/app/services/ci/pipeline_processing/legacy_processing_service.rb b/app/services/ci/pipeline_processing/legacy_processing_service.rb deleted file mode 100644 index 56fbc7271da..00000000000 --- a/app/services/ci/pipeline_processing/legacy_processing_service.rb +++ /dev/null @@ -1,122 +0,0 @@ -# frozen_string_literal: true - -module Ci - module PipelineProcessing - class LegacyProcessingService - include Gitlab::Utils::StrongMemoize - - attr_reader :pipeline - - def initialize(pipeline) - @pipeline = pipeline - end - - def execute(trigger_build_ids = nil, initial_process: false) - success = process_stages_for_stage_scheduling - - # we evaluate dependent needs, - # only when the another job has finished - success = process_dag_builds_without_needs || success if initial_process - success = process_dag_builds_with_needs(trigger_build_ids) || success - - @pipeline.update_legacy_status - - success - end - - private - - def process_stages_for_stage_scheduling - stage_indexes_of_created_stage_scheduled_processables.flat_map do |index| - process_stage_for_stage_scheduling(index) - end.any? - end - - def process_stage_for_stage_scheduling(index) - current_status = status_for_prior_stages(index) - - return unless Ci::HasStatus::COMPLETED_STATUSES.include?(current_status) - - created_stage_scheduled_processables_in_stage(index).find_each.select do |build| - process_build(build, current_status) - end.any? - end - - def process_dag_builds_without_needs - created_processables.scheduling_type_dag.without_needs.each do |build| - process_build(build, 'success') - end - end - - def process_dag_builds_with_needs(trigger_build_ids) - return false unless trigger_build_ids.present? - - # we find processables that are dependent: - # 1. because of current dependency, - trigger_build_names = pipeline.processables.latest - .for_ids(trigger_build_ids).names - - # 2. does not have builds that not yet complete - incomplete_build_names = pipeline.processables.latest - .incomplete.names - - # Each found processable is guaranteed here to have completed status - created_processables - .scheduling_type_dag - .with_needs(trigger_build_names) - .without_needs(incomplete_build_names) - .find_each - .map(&method(:process_dag_build_with_needs)) - .any? - end - - def process_dag_build_with_needs(build) - current_status = status_for_build_needs(build.needs.map(&:name)) - - return unless Ci::HasStatus::COMPLETED_STATUSES.include?(current_status) - - process_build(build, current_status) - end - - def process_build(build, current_status) - Gitlab::OptimisticLocking.retry_lock(build) do |subject| - Ci::ProcessBuildService.new(project, subject.user) - .execute(subject, current_status) - end - end - - def status_for_prior_stages(index) - pipeline.processables.status_for_prior_stages(index, project: pipeline.project) - end - - def status_for_build_needs(needs) - pipeline.processables.status_for_names(needs, project: pipeline.project) - end - - # rubocop: disable CodeReuse/ActiveRecord - def stage_indexes_of_created_stage_scheduled_processables - created_stage_scheduled_processables.order(:stage_idx) - .pluck(Arel.sql('DISTINCT stage_idx')) - end - # rubocop: enable CodeReuse/ActiveRecord - - def created_stage_scheduled_processables_in_stage(index) - created_stage_scheduled_processables - .with_preloads - .for_stage(index) - end - - def created_stage_scheduled_processables - created_processables.scheduling_type_stage - end - - def created_processables - pipeline.processables.created - end - - def project - pipeline.project - end - end - end -end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 1f24dce0458..d84ef5fbb93 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -8,20 +8,14 @@ module Ci @pipeline = pipeline end - def execute(trigger_build_ids = nil, initial_process: false) + def execute increment_processing_counter update_retried - if ::Gitlab::Ci::Features.atomic_processing?(pipeline.project) - Ci::PipelineProcessing::AtomicProcessingService - .new(pipeline) - .execute - else - Ci::PipelineProcessing::LegacyProcessingService - .new(pipeline) - .execute(trigger_build_ids, initial_process: initial_process) - end + Ci::PipelineProcessing::AtomicProcessingService + .new(pipeline) + .execute end def metrics diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 3797ea1d96c..04d620d1d38 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -107,23 +107,15 @@ module Ci build.runner_id = runner.id build.runner_session_attributes = params[:session] if params[:session].present? - unless build.has_valid_build_dependencies? - build.drop!(:missing_dependency_failure) - return false - end - - unless build.supported_runner?(params.dig(:info, :features)) - build.drop!(:runner_unsupported) - return false - end + failure_reason, _ = pre_assign_runner_checks.find { |_, check| check.call(build, params) } - if build.archived? - build.drop!(:archived_failure) - return false + if failure_reason + build.drop!(failure_reason) + else + build.run! end - build.run! - true + !failure_reason end def scheduler_failure!(build) @@ -238,6 +230,14 @@ module Ci def job_queue_duration_seconds @job_queue_duration_seconds ||= Gitlab::Metrics.histogram(:job_queue_duration_seconds, 'Request handling execution time', {}, JOB_QUEUE_DURATION_SECONDS_BUCKETS) end + + def pre_assign_runner_checks + { + missing_dependency_failure: -> (build, _) { !build.has_valid_build_dependencies? }, + runner_unsupported: -> (build, params) { !build.supported_runner?(params.dig(:info, :features)) }, + archived_failure: -> (build, _) { build.archived? } + } + end end end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index 4229be6c7d7..2f52f0a39c1 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -22,12 +22,6 @@ module Ci needs += build.needs.map(&:name) end - # In a DAG, the dependencies may have already completed. Figure out - # which builds have succeeded and use them to update the pipeline. If we don't - # do this, then builds will be stuck in the created state since their dependencies - # will never run. - completed_build_ids = pipeline.find_successful_build_ids_by_names(needs) if needs.any? - pipeline.builds.latest.skipped.find_each do |skipped| retry_optimistic_lock(skipped) { |build| build.process } end @@ -38,7 +32,7 @@ module Ci Ci::ProcessPipelineService .new(pipeline) - .execute(completed_build_ids, initial_process: true) + .execute end end end diff --git a/app/services/clusters/aws/authorize_role_service.rb b/app/services/clusters/aws/authorize_role_service.rb index 6eafce0597e..fb620f77b9f 100644 --- a/app/services/clusters/aws/authorize_role_service.rb +++ b/app/services/clusters/aws/authorize_role_service.rb @@ -23,7 +23,9 @@ module Clusters @role = create_or_update_role! Response.new(:ok, credentials) - rescue *ERRORS + rescue *ERRORS => e + Gitlab::ErrorTracking.track_exception(e) + Response.new(:unprocessable_entity, {}) end diff --git a/app/services/clusters/parse_cluster_applications_artifact_service.rb b/app/services/clusters/parse_cluster_applications_artifact_service.rb index 6a0ca0ef9d0..b9b2953b6bd 100644 --- a/app/services/clusters/parse_cluster_applications_artifact_service.rb +++ b/app/services/clusters/parse_cluster_applications_artifact_service.rb @@ -5,7 +5,7 @@ module Clusters include Gitlab::Utils::StrongMemoize MAX_ACCEPTABLE_ARTIFACT_SIZE = 5.kilobytes - RELEASE_NAMES = %w[prometheus cilium].freeze + RELEASE_NAMES = %w[cilium].freeze def initialize(job, current_user) @job = job @@ -14,8 +14,6 @@ module Clusters end def execute(artifact) - return success unless Feature.enabled?(:cluster_applications_artifact, project) - raise ArgumentError, 'Artifact is not cluster_applications file type' unless artifact&.cluster_applications? return error(too_big_error_message, :bad_request) unless artifact.file.size < MAX_ACCEPTABLE_ARTIFACT_SIZE @@ -46,6 +44,8 @@ module Clusters releases = [] artifact.each_blob do |blob| + next if blob.empty? + releases.concat(Gitlab::Kubernetes::Helm::Parsers::ListV2.new(blob).releases) end diff --git a/app/services/cohorts_service.rb b/app/services/cohorts_service.rb index 03be87f4cc1..7bc3b267a12 100644 --- a/app/services/cohorts_service.rb +++ b/app/services/cohorts_service.rb @@ -63,7 +63,7 @@ class CohortsService overall_total = month_totals.first month_totals.map do |total| - { total: total, percentage: total.zero? ? 0 : 100 * total / overall_total } + { total: total, percentage: total == 0 ? 0 : 100 * total / overall_total } end end diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb index 661e654406e..edb9f04ccd7 100644 --- a/app/services/commits/change_service.rb +++ b/app/services/commits/change_service.rb @@ -22,7 +22,9 @@ module Commits @branch_name, message, start_project: @start_project, - start_branch_name: @start_branch) + start_branch_name: @start_branch, + dry_run: @dry_run + ) rescue Gitlab::Git::Repository::CreateTreeError => ex act = action.to_s.dasherize type = @commit.change_type_title(current_user) diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb index d80d9bebe9c..a1498da302e 100644 --- a/app/services/commits/create_service.rb +++ b/app/services/commits/create_service.rb @@ -21,6 +21,7 @@ module Commits @start_sha = params[:start_sha] @branch_name = params[:branch_name] @force = params[:force] || false + @dry_run = params[:dry_run] || false end def execute @@ -69,7 +70,7 @@ module Commits end def validate_permissions! - allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name) + allowed = ::Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(@branch_name) unless allowed raise_error("You are not allowed to push into this branch") diff --git a/app/services/concerns/incident_management/settings.rb b/app/services/concerns/incident_management/settings.rb index 491bd4fa6bf..13a047ec106 100644 --- a/app/services/concerns/incident_management/settings.rb +++ b/app/services/concerns/incident_management/settings.rb @@ -1,8 +1,11 @@ # frozen_string_literal: true + module IncidentManagement module Settings include Gitlab::Utils::StrongMemoize + delegate :send_email?, to: :incident_management_setting + def incident_management_setting strong_memoize(:incident_management_setting) do project.incident_management_setting || diff --git a/app/services/design_management/move_designs_service.rb b/app/services/design_management/move_designs_service.rb new file mode 100644 index 00000000000..de763caba2f --- /dev/null +++ b/app/services/design_management/move_designs_service.rb @@ -0,0 +1,84 @@ +# frozen_string_literal: true + +module DesignManagement + class MoveDesignsService < DesignService + # @param user [User] The current user + # @param [Hash] params + # @option params [DesignManagement::Design] :current_design + # @option params [DesignManagement::Design] :previous_design (nil) + # @option params [DesignManagement::Design] :next_design (nil) + def initialize(user, params) + super(nil, user, params.merge(issue: nil)) + end + + def execute + return error(:no_focus) unless current_design.present? + return error(:cannot_move) unless ::Feature.enabled?(:reorder_designs, project, default_enabled: true) + return error(:cannot_move) unless current_user.can?(:move_design, current_design) + return error(:no_neighbors) unless neighbors.present? + return error(:not_distinct) unless all_distinct? + return error(:not_adjacent) if any_in_gap? + return error(:not_same_issue) unless all_same_issue? + + move_nulls_to_end + current_design.move_between(previous_design, next_design) + current_design.save! + success + end + + def error(message) + ServiceResponse.error(message: message) + end + + def success + ServiceResponse.success + end + + private + + delegate :issue, :project, to: :current_design + + def move_nulls_to_end + moved_records = current_design.class.move_nulls_to_end(issue.designs.in_creation_order) + return if moved_records == 0 + + current_design.reset + next_design&.reset + previous_design&.reset + end + + def neighbors + [previous_design, next_design].compact + end + + def all_distinct? + ids.uniq.size == ids.size + end + + def any_in_gap? + return false unless previous_design&.relative_position && next_design&.relative_position + + !previous_design.immediately_before?(next_design) + end + + def all_same_issue? + issue.designs.id_in(ids).count == ids.size + end + + def ids + @ids ||= [current_design, *neighbors].map(&:id) + end + + def current_design + params[:current_design] + end + + def previous_design + params[:previous_design] + end + + def next_design + params[:next_design] + end + end +end diff --git a/app/services/discussions/capture_diff_note_position_service.rb b/app/services/discussions/capture_diff_note_position_service.rb index 8f12470d9e8..4e8fd90a2e7 100644 --- a/app/services/discussions/capture_diff_note_position_service.rb +++ b/app/services/discussions/capture_diff_note_position_service.rb @@ -50,9 +50,9 @@ module Discussions merge_ref_head = merge_request.merge_ref_head return unless merge_ref_head - start_sha, base_sha = merge_ref_head.parent_ids + start_sha, _ = merge_ref_head.parent_ids new_diff_refs = Gitlab::Diff::DiffRefs.new( - base_sha: base_sha, + base_sha: start_sha, start_sha: start_sha, head_sha: merge_ref_head.id) diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 946fb5f1372..cd5925cd9be 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -56,7 +56,7 @@ module Discussions def process_auto_merge return unless merge_request - return unless @resolved_count.positive? + return unless @resolved_count > 0 return unless discussions_ready_to_merge? AutoMergeProcessWorker.perform_async(merge_request.id) diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index ad36fe70b3a..3921dbefd06 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -100,25 +100,21 @@ class EventCreateService # @param [WikiPage::Meta] wiki_page_meta The event target # @param [User] author The event author # @param [Symbol] action One of the Event::WIKI_ACTIONS + # @param [String] fingerprint The de-duplication fingerprint # - # @return a tuple of event and either :found or :created - def wiki_event(wiki_page_meta, author, action) + # The fingerprint, if provided, should be sufficient to find duplicate events. + # Suitable values would be, for example, the current page SHA. + # + # @return [Event] the event + def wiki_event(wiki_page_meta, author, action, fingerprint) raise IllegalActionError, action unless Event::WIKI_ACTIONS.include?(action) - if duplicate = existing_wiki_event(wiki_page_meta, action) - return duplicate - end + Gitlab::UsageDataCounters::TrackUniqueActions.track_event(event_action: action, event_target: wiki_page_meta.class, author_id: author.id) - event = create_record_event(wiki_page_meta, author, action) - # Ensure that the event is linked in time to the metadata, for non-deletes - unless event.destroyed_action? - time_stamp = wiki_page_meta.updated_at - event.update_columns(updated_at: time_stamp, created_at: time_stamp) - end + duplicate = Event.for_wiki_meta(wiki_page_meta).for_fingerprint(fingerprint).first + return duplicate if duplicate.present? - Gitlab::UsageDataCounters::TrackUniqueActions.track_action(event_action: action, event_target: wiki_page_meta.class, author_id: author.id) - - event + create_record_event(wiki_page_meta, author, action, fingerprint.presence) end def approve_mr(merge_request, current_user) @@ -127,45 +123,38 @@ class EventCreateService private - def existing_wiki_event(wiki_page_meta, action) - if Event.actions.fetch(action) == Event.actions[:destroyed] - most_recent = Event.for_wiki_meta(wiki_page_meta).recent.first - return most_recent if most_recent.present? && Event.actions[most_recent.action] == Event.actions[action] - else - Event.for_wiki_meta(wiki_page_meta).created_at(wiki_page_meta.updated_at).first - end - end - - def create_record_event(record, current_user, status) + def create_record_event(record, current_user, status, fingerprint = nil) create_event(record.resource_parent, current_user, status, - target_id: record.id, target_type: record.class.name) + fingerprint: fingerprint, + target_id: record.id, + target_type: record.class.name) end # If creating several events, this method will insert them all in a single # statement # - # @param [[Eventable, Symbol]] a list of pairs of records and a valid status + # @param [[Eventable, Symbol, String]] a list of tuples of records, a valid status, and fingerprint # @param [User] the author of the event - def create_record_events(pairs, current_user) + def create_record_events(tuples, current_user) base_attrs = { created_at: Time.now.utc, updated_at: Time.now.utc, author_id: current_user.id } - attribute_sets = pairs.map do |record, status| + attribute_sets = tuples.map do |record, status, fingerprint| action = Event.actions[status] raise IllegalActionError, "#{status} is not a valid status" if action.nil? parent_attrs(record.resource_parent) .merge(base_attrs) - .merge(action: action, target_id: record.id, target_type: record.class.name) + .merge(action: action, fingerprint: fingerprint, target_id: record.id, target_type: record.class.name) end result = Event.insert_all(attribute_sets, returning: %w[id]) - pairs.each do |record, status| - Gitlab::UsageDataCounters::TrackUniqueActions.track_action(event_action: status, event_target: record.class, author_id: current_user.id) + tuples.each do |record, status, _| + Gitlab::UsageDataCounters::TrackUniqueActions.track_event(event_action: status, event_target: record.class, author_id: current_user.id) end result @@ -183,7 +172,7 @@ class EventCreateService new_event end - Gitlab::UsageDataCounters::TrackUniqueActions.track_action(event_action: :pushed, event_target: Project, author_id: current_user.id) + Gitlab::UsageDataCounters::TrackUniqueActions.track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) Users::LastPushEventService.new(current_user) .cache_last_push_event(event) @@ -198,7 +187,11 @@ class EventCreateService ) attributes.merge!(parent_attrs(resource_parent)) - Event.create!(attributes) + if attributes[:fingerprint].present? + Event.safe_find_or_create_by!(attributes) + else + Event.create!(attributes) + end end def parent_attrs(resource_parent) diff --git a/app/services/git/process_ref_changes_service.rb b/app/services/git/process_ref_changes_service.rb index 6d1ff97016b..c012c61a337 100644 --- a/app/services/git/process_ref_changes_service.rb +++ b/app/services/git/process_ref_changes_service.rb @@ -75,8 +75,6 @@ module Git end def merge_request_branches_for(changes) - return if Feature.disabled?(:refresh_only_existing_merge_requests_on_push, default_enabled: true) - @merge_requests_branches ||= MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute end end diff --git a/app/services/git/tag_push_service.rb b/app/services/git/tag_push_service.rb index 120c4cde94b..641fe8e3916 100644 --- a/app/services/git/tag_push_service.rb +++ b/app/services/git/tag_push_service.rb @@ -26,9 +26,5 @@ module Git def removing_tag? Gitlab::Git.blank_ref?(newrev) end - - def tag_name - Gitlab::Git.ref_name(ref) - end end end diff --git a/app/services/git/wiki_push_service.rb b/app/services/git/wiki_push_service.rb index b3937a10a70..f9de72f2d5f 100644 --- a/app/services/git/wiki_push_service.rb +++ b/app/services/git/wiki_push_service.rb @@ -41,7 +41,12 @@ module Git end def create_event_for(change) - event_service.execute(change.last_known_slug, change.page, change.event_action) + event_service.execute( + change.last_known_slug, + change.page, + change.event_action, + change.sha + ) end def event_service diff --git a/app/services/git/wiki_push_service/change.rb b/app/services/git/wiki_push_service/change.rb index 14e622dd147..562c43487e9 100644 --- a/app/services/git/wiki_push_service/change.rb +++ b/app/services/git/wiki_push_service/change.rb @@ -33,6 +33,10 @@ module Git strip_extension(raw_change.old_path || raw_change.new_path) end + def sha + change[:newrev] + end + private attr_reader :raw_change, :change, :wiki diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index f2fb494500d..2bd571f60af 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -47,6 +47,19 @@ module Groups raise_transfer_error(:namespace_with_same_path) if namespace_with_same_path? raise_transfer_error(:group_contains_images) if group_projects_contain_registry_images? raise_transfer_error(:cannot_transfer_to_subgroup) if transfer_to_subgroup? + raise_transfer_error(:group_contains_npm_packages) if group_with_npm_packages? + end + + def group_with_npm_packages? + return false unless group.packages_feature_enabled? + + npm_packages = ::Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute + + different_root_ancestor? && npm_packages.exists? + end + + def different_root_ancestor? + group.root_ancestor != new_parent_group&.root_ancestor end def group_is_already_root? @@ -144,7 +157,8 @@ module Groups same_parent_as_current: s_('TransferGroup|Group is already associated to the parent group.'), invalid_policies: s_("TransferGroup|You don't have enough permissions."), group_contains_images: s_('TransferGroup|Cannot update the path because there are projects under this group that contain Docker images in their Container Registry. Please remove the images from your projects first and try again.'), - cannot_transfer_to_subgroup: s_('TransferGroup|Cannot transfer group to one of its subgroup.') + cannot_transfer_to_subgroup: s_('TransferGroup|Cannot transfer group to one of its subgroup.'), + group_contains_npm_packages: s_('TransferGroup|Group contains projects with NPM packages.') }.freeze end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 948540619ae..81393681dc0 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -17,6 +17,8 @@ module Groups return false unless valid_share_with_group_lock_change? + return false unless valid_path_change_with_npm_packages? + before_assignment_hook(group, params) group.assign_attributes(params) @@ -36,6 +38,20 @@ module Groups private + def valid_path_change_with_npm_packages? + return true unless group.packages_feature_enabled? + return true if params[:path].blank? + return true if !group.has_parent? && group.path == params[:path] + + npm_packages = ::Packages::GroupPackagesFinder.new(current_user, group, package_type: :npm).execute + if npm_packages.exists? + group.errors.add(:path, s_('GroupSettings|cannot change when group contains projects with NPM packages')) + return + end + + true + end + def before_assignment_hook(group, params) # overridden in EE end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index 0cf17568c78..a2923b1e4f9 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -33,7 +33,7 @@ module Import end def repo - @repo ||= client.repo(params[:repo_id].to_i) + @repo ||= client.repository(params[:repo_id].to_i) end def project_name diff --git a/app/services/incident_management/create_incident_label_service.rb b/app/services/incident_management/create_incident_label_service.rb index dbd0d78fa3c..595f5df184f 100644 --- a/app/services/incident_management/create_incident_label_service.rb +++ b/app/services/incident_management/create_incident_label_service.rb @@ -14,27 +14,9 @@ module IncidentManagement def execute label = Labels::FindOrCreateService .new(current_user, project, **LABEL_PROPERTIES) - .execute - - if label.invalid? - log_invalid_label_info(label) - return ServiceResponse.error(payload: { label: label }, message: full_error_message(label)) - end + .execute(skip_authorization: true) ServiceResponse.success(payload: { label: label }) end - - private - - def log_invalid_label_info(label) - log_info <<~TEXT.chomp - Cannot create incident label "#{label.title}" \ - for "#{label.project.full_name}": #{full_error_message(label)}. - TEXT - end - - def full_error_message(label) - label.errors.full_messages.to_sentence - end end end diff --git a/app/services/incident_management/create_issue_service.rb b/app/services/incident_management/create_issue_service.rb deleted file mode 100644 index 5e1e0863115..00000000000 --- a/app/services/incident_management/create_issue_service.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true - -module IncidentManagement - class CreateIssueService < BaseService - include Gitlab::Utils::StrongMemoize - - def initialize(project, params) - super(project, User.alert_bot, params) - end - - def execute - return error_with('setting disabled') unless incident_management_setting.create_issue? - return error_with('invalid alert') unless alert.valid? - - issue = create_issue - return error_with(issue_errors(issue)) unless issue.valid? - - success(issue: issue) - end - - private - - def create_issue - label_result = find_or_create_incident_label - - # Create an unlabelled issue if we couldn't create the label - # due to a race condition. - # See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042 - extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {} - - Issues::CreateService.new( - project, - current_user, - title: issue_title, - description: issue_description, - **extra_params - ).execute - end - - def issue_title - alert.full_title - end - - def issue_description - horizontal_line = "\n\n---\n\n" - - [ - alert_summary, - alert_markdown, - issue_template_content - ].compact.join(horizontal_line) - end - - def find_or_create_incident_label - IncidentManagement::CreateIncidentLabelService.new(project, current_user).execute - end - - def alert_summary - alert.issue_summary_markdown - end - - def alert_markdown - alert.alert_markdown - end - - def alert - strong_memoize(:alert) do - Gitlab::Alerting::Alert.new(project: project, payload: params).present - end - end - - def issue_template_content - incident_management_setting.issue_template_content - end - - def incident_management_setting - strong_memoize(:incident_management_setting) do - project.incident_management_setting || - project.build_incident_management_setting - end - end - - def issue_errors(issue) - issue.errors.full_messages.to_sentence - end - - def error_with(message) - log_error(%{Cannot create incident issue for "#{project.full_name}": #{message}}) - - error(message) - end - end -end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb new file mode 100644 index 00000000000..7206eaf51b2 --- /dev/null +++ b/app/services/incident_management/incidents/create_service.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module IncidentManagement + module Incidents + class CreateService < BaseService + ISSUE_TYPE = 'incident' + + def initialize(project, current_user, title:, description:) + super(project, current_user) + + @title = title + @description = description + end + + def execute + issue = Issues::CreateService.new( + project, + current_user, + title: title, + description: description, + label_ids: [find_or_create_incident_label.id], + issue_type: ISSUE_TYPE + ).execute + + return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? + + success(issue) + end + + private + + attr_reader :title, :description + + def find_or_create_incident_label + IncidentManagement::CreateIncidentLabelService + .new(project, current_user) + .execute + .payload[:label] + end + + def success(issue) + ServiceResponse.success(payload: { issue: issue }) + end + + def error(message, issue = nil) + ServiceResponse.error(payload: { issue: issue }, message: message) + end + end + end +end diff --git a/app/services/incident_management/pager_duty/create_incident_issue_service.rb b/app/services/incident_management/pager_duty/create_incident_issue_service.rb index ee0feb49e0d..0c9ca2c0add 100644 --- a/app/services/incident_management/pager_duty/create_incident_issue_service.rb +++ b/app/services/incident_management/pager_duty/create_incident_issue_service.rb @@ -12,46 +12,30 @@ module IncidentManagement def execute return forbidden unless webhook_available? - issue = create_issue - return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? - - success(issue) + create_incident end private alias_method :incident_payload, :params - def create_issue - label_result = find_or_create_incident_label - - # Create an unlabelled issue if we couldn't create the label - # due to a race condition. - # See https://gitlab.com/gitlab-org/gitlab-foss/issues/65042 - extra_params = label_result.success? ? { label_ids: [label_result.payload[:label].id] } : {} - - Issues::CreateService.new( + def create_incident + ::IncidentManagement::Incidents::CreateService.new( project, current_user, title: issue_title, - description: issue_description, - **extra_params + description: issue_description ).execute end def webhook_available? - Feature.enabled?(:pagerduty_webhook, project) && - incident_management_setting.pagerduty_active? + incident_management_setting.pagerduty_active? end def forbidden ServiceResponse.error(message: 'Forbidden', http_status: :forbidden) end - def find_or_create_incident_label - ::IncidentManagement::CreateIncidentLabelService.new(project, current_user).execute - end - def issue_title incident_payload['title'] end @@ -59,14 +43,6 @@ module IncidentManagement def issue_description Gitlab::IncidentManagement::PagerDuty::IncidentIssueDescription.new(incident_payload).to_s end - - def success(issue) - ServiceResponse.success(payload: { issue: issue }) - end - - def error(message, issue = nil) - ServiceResponse.error(payload: { issue: issue }, message: message) - end end end end diff --git a/app/services/incident_management/pager_duty/process_webhook_service.rb b/app/services/incident_management/pager_duty/process_webhook_service.rb index 5dd3186694a..fd8252f75fb 100644 --- a/app/services/incident_management/pager_duty/process_webhook_service.rb +++ b/app/services/incident_management/pager_duty/process_webhook_service.rb @@ -39,8 +39,7 @@ module IncidentManagement end def webhook_setting_active? - Feature.enabled?(:pagerduty_webhook, project) && - incident_management_setting.pagerduty_active? + incident_management_setting.pagerduty_active? end def valid_token?(token) diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb index 0d1640924e5..b2f9c083b5b 100644 --- a/app/services/issuable/clone/base_service.rb +++ b/app/services/issuable/clone/base_service.rb @@ -24,12 +24,34 @@ module Issuable private + def copy_award_emoji + AwardEmojis::CopyService.new(original_entity, new_entity).execute + end + + def copy_notes + Notes::CopyService.new(current_user, original_entity, new_entity).execute + end + def update_new_entity - rewriters = [ContentRewriter, AttributesRewriter] + update_new_entity_description + update_new_entity_attributes + copy_award_emoji + copy_notes + end - rewriters.each do |rewriter| - rewriter.new(current_user, original_entity, new_entity).execute - end + def update_new_entity_description + rewritten_description = MarkdownContentRewriterService.new( + current_user, + original_entity.description, + original_entity.project, + new_parent + ).execute + + new_entity.update!(description: rewritten_description) + end + + def update_new_entity_attributes + AttributesRewriter.new(current_user, original_entity, new_entity).execute end def update_old_entity @@ -47,7 +69,7 @@ module Issuable end def new_parent - new_entity.project || new_entity.group + new_entity.resource_parent end def group diff --git a/app/services/issuable/clone/content_rewriter.rb b/app/services/issuable/clone/content_rewriter.rb deleted file mode 100644 index 67d2f9fd3fe..00000000000 --- a/app/services/issuable/clone/content_rewriter.rb +++ /dev/null @@ -1,74 +0,0 @@ -# frozen_string_literal: true - -module Issuable - module Clone - class ContentRewriter < ::Issuable::Clone::BaseService - def initialize(current_user, original_entity, new_entity) - @current_user = current_user - @original_entity = original_entity - @new_entity = new_entity - @project = original_entity.project - end - - def execute - rewrite_description - rewrite_award_emoji(original_entity, new_entity) - rewrite_notes - end - - private - - def rewrite_description - new_entity.update(description: rewrite_content(original_entity.description)) - end - - def rewrite_notes - new_discussion_ids = {} - original_entity.notes_with_associations.find_each do |note| - new_note = note.dup - new_discussion_ids[note.discussion_id] ||= Discussion.discussion_id(new_note) - new_params = { - project: new_entity.project, - noteable: new_entity, - discussion_id: new_discussion_ids[note.discussion_id], - note: rewrite_content(new_note.note), - note_html: nil, - created_at: note.created_at, - updated_at: note.updated_at - } - - if note.system_note_metadata - new_params[:system_note_metadata] = note.system_note_metadata.dup - - # TODO: Implement copying of description versions when an issue is moved - # https://gitlab.com/gitlab-org/gitlab/issues/32300 - new_params[:system_note_metadata].description_version = nil - end - - new_note.update(new_params) - - rewrite_award_emoji(note, new_note) - end - end - - def rewrite_content(content) - return unless content - - rewriters = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter] - - rewriters.inject(content) do |text, klass| - rewriter = klass.new(text, old_project, current_user) - rewriter.rewrite(new_parent) - end - end - - def rewrite_award_emoji(old_awardable, new_awardable) - old_awardable.award_emoji.each do |award| - new_award = award.dup - new_award.awardable = new_awardable - new_award.save - end - end - end - end -end diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb index 195616857dc..84024cca68c 100644 --- a/app/services/issuable/common_system_notes_service.rb +++ b/app/services/issuable/common_system_notes_service.rb @@ -108,7 +108,7 @@ module Issuable end def milestone_changes_tracking_enabled? - ::Feature.enabled?(:track_resource_milestone_change_events, issuable.project) + ::Feature.enabled?(:track_resource_milestone_change_events, issuable.project, default_enabled: true) end def create_due_date_note diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index e62315de5f9..2de6ed9fa1c 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -66,7 +66,7 @@ module Issues def whitelisted_issue_params base_params = [:title, :description, :confidential] - admin_params = [:milestone_id] + admin_params = [:milestone_id, :issue_type] if can?(current_user, :admin_issue, project) params.slice(*(base_params + admin_params)) diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 8594808cd44..e431c766df8 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -33,6 +33,7 @@ module Issues notification_service.async.close_issue(issue, current_user, closed_via: closed_via) if notifications todo_service.close_issue(issue, current_user) + resolve_alert(issue) execute_hooks(issue, 'close') invalidate_cache_counts(issue, users: issue.assignees) issue.update_project_counter_caches @@ -58,6 +59,22 @@ module Issues SystemNoteService.change_status(issue, issue.project, current_user, issue.state, current_commit) end + def resolve_alert(issue) + return unless alert = issue.alert_management_alert + return if alert.resolved? + + if alert.resolve + SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: current_user).closed_alert_issue(issue) + else + Gitlab::AppLogger.warn( + message: 'Cannot resolve an associated Alert Management alert', + issue_id: issue.id, + alert_id: alert.id, + alert_errors: alert.errors.messages + ) + end + end + def store_first_mentioned_in_commit_at(issue, merge_request) metrics = issue.metrics return if metrics.nil? || metrics.first_mentioned_in_commit_at diff --git a/app/services/issues/reorder_service.rb b/app/services/issues/reorder_service.rb index 02c18d31b5e..c82ad6ea501 100644 --- a/app/services/issues/reorder_service.rb +++ b/app/services/issues/reorder_service.rb @@ -40,7 +40,7 @@ module Issues def move_between_ids ids = [params[:move_after_id], params[:move_before_id]] .map(&:to_i) - .map { |m| m.positive? ? m : nil } + .map { |m| m > 0 ? m : nil } ids.any? ? ids : nil end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 8d22f0edcdd..ac7baba3b7c 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -22,7 +22,7 @@ module Issues end def after_update(issue) - IssuesChannel.broadcast_to(issue, event: 'updated') if Feature.enabled?(:broadcast_issue_updates, issue.project) + IssuesChannel.broadcast_to(issue, event: 'updated') if Gitlab::ActionCable::Config.in_app? || Feature.enabled?(:broadcast_issue_updates, issue.project) end def handle_changes(issue, options) @@ -43,7 +43,7 @@ module Issues if issue.assignees != old_assignees create_assignee_note(issue, old_assignees) notification_service.async.reassigned_issue(issue, current_user, old_assignees) - todo_service.reassigned_issuable(issue, current_user, old_assignees) + todo_service.reassigned_assignable(issue, current_user, old_assignees) end if issue.previous_changes.include?('confidential') diff --git a/app/services/jira/requests/projects/list_service.rb b/app/services/jira/requests/projects/list_service.rb index 8ecfd358ffb..373c536974a 100644 --- a/app/services/jira/requests/projects/list_service.rb +++ b/app/services/jira/requests/projects/list_service.rb @@ -6,7 +6,7 @@ module Jira class ListService < Base extend ::Gitlab::Utils::Override - def initialize(jira_service, params: {}) + def initialize(jira_service, params = {}) super(jira_service, params) @query = params[:query] @@ -33,9 +33,9 @@ module Jira end def match_query?(jira_project) - query = query.to_s.downcase + downcase_query = query.to_s.downcase - jira_project&.key&.downcase&.include?(query) || jira_project&.name&.downcase&.include?(query) + jira_project&.key&.downcase&.include?(downcase_query) || jira_project&.name&.downcase&.include?(downcase_query) end def empty_payload diff --git a/app/services/jira_import/cloud_users_mapper_service.rb b/app/services/jira_import/cloud_users_mapper_service.rb new file mode 100644 index 00000000000..b1c7aac584f --- /dev/null +++ b/app/services/jira_import/cloud_users_mapper_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module JiraImport + class CloudUsersMapperService < UsersMapperService + private + + def url + "/rest/api/2/users?maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}" + end + + def jira_user_id(jira_user) + jira_user['accountId'] + end + + def jira_user_name(jira_user) + jira_user['displayName'] + end + end +end diff --git a/app/services/jira_import/server_users_mapper_service.rb b/app/services/jira_import/server_users_mapper_service.rb new file mode 100644 index 00000000000..d38d134f55c --- /dev/null +++ b/app/services/jira_import/server_users_mapper_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module JiraImport + class ServerUsersMapperService < UsersMapperService + private + + def url + "/rest/api/2/user/search?username=''&maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}" + end + + def jira_user_id(jira_user) + jira_user['key'] + end + + def jira_user_name(jira_user) + jira_user['name'] + end + end +end diff --git a/app/services/jira_import/start_import_service.rb b/app/services/jira_import/start_import_service.rb index f85f686c61a..88cfe684125 100644 --- a/app/services/jira_import/start_import_service.rb +++ b/app/services/jira_import/start_import_service.rb @@ -42,7 +42,7 @@ module JiraImport ServiceResponse.success(payload: { import_data: jira_import } ) rescue => ex - # in case project.save! raises an erorr + # in case project.save! raises an error Gitlab::ErrorTracking.track_exception(ex, project_id: project.id) jira_import&.do_fail!(error_message: ex.message) build_error_response(ex.message) diff --git a/app/services/jira_import/users_importer.rb b/app/services/jira_import/users_importer.rb index 579d3675073..9babd468d56 100644 --- a/app/services/jira_import/users_importer.rb +++ b/app/services/jira_import/users_importer.rb @@ -2,9 +2,7 @@ module JiraImport class UsersImporter - attr_reader :user, :project, :start_at, :result - - MAX_USERS = 50 + attr_reader :user, :project, :start_at def initialize(user, project, start_at) @project = project @@ -15,29 +13,43 @@ module JiraImport def execute Gitlab::JiraImport.validate_project_settings!(project, user: user) - return ServiceResponse.success(payload: nil) if users.blank? - - result = UsersMapper.new(project, users).execute - ServiceResponse.success(payload: result) + ServiceResponse.success(payload: mapped_users) rescue Timeout::Error, Errno::EINVAL, Errno::ECONNRESET, Errno::ECONNREFUSED, URI::InvalidURIError, JIRA::HTTPError, OpenSSL::SSL::SSLError => error - Gitlab::ErrorTracking.track_exception(error, project_id: project.id, request: url) - ServiceResponse.error(message: "There was an error when communicating to Jira: #{error.message}") + Gitlab::ErrorTracking.track_exception(error, project_id: project.id) + ServiceResponse.error(message: "There was an error when communicating to Jira") rescue Projects::ImportService::Error => error ServiceResponse.error(message: error.message) end private - def users - @users ||= client.get(url) + def mapped_users + users_mapper_service.execute + end + + def users_mapper_service + @users_mapper_service ||= user_mapper_service_factory end - def url - "/rest/api/2/users?maxResults=#{MAX_USERS}&startAt=#{start_at.to_i}" + def deployment_type + # TODO: use project.jira_service.deployment_type value when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged + @deployment_type ||= client.ServerInfo.all.deploymentType end def client @client ||= project.jira_service.client end + + def user_mapper_service_factory + # TODO: use deployment_type enum from jira service when https://gitlab.com/gitlab-org/gitlab/-/merge_requests/37003 is merged + case deployment_type.upcase + when JiraService::DEPLOYMENT_TYPES[:server] + ServerUsersMapperService.new(project.jira_service, start_at) + when JiraService::DEPLOYMENT_TYPES[:cloud] + CloudUsersMapperService.new(project.jira_service, start_at) + else + raise ArgumentError + end + end end end diff --git a/app/services/jira_import/users_mapper.rb b/app/services/jira_import/users_mapper.rb deleted file mode 100644 index c3cbeb157bd..00000000000 --- a/app/services/jira_import/users_mapper.rb +++ /dev/null @@ -1,30 +0,0 @@ -# frozen_string_literal: true - -module JiraImport - class UsersMapper - attr_reader :project, :jira_users - - def initialize(project, jira_users) - @project = project - @jira_users = jira_users - end - - def execute - jira_users.to_a.map do |jira_user| - { - jira_account_id: jira_user['accountId'], - jira_display_name: jira_user['displayName'], - jira_email: jira_user['emailAddress'] - }.merge(match_user(jira_user)) - end - end - - private - - # TODO: Matching user by email and displayName will be done as the part - # of follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/219023 - def match_user(jira_user) - { gitlab_id: nil, gitlab_username: nil, gitlab_name: nil } - end - end -end diff --git a/app/services/jira_import/users_mapper_service.rb b/app/services/jira_import/users_mapper_service.rb new file mode 100644 index 00000000000..b5997d77215 --- /dev/null +++ b/app/services/jira_import/users_mapper_service.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +module JiraImport + class UsersMapperService + MAX_USERS = 50 + + attr_reader :jira_service, :start_at + + def initialize(jira_service, start_at) + @jira_service = jira_service + @start_at = start_at + end + + def execute + users.to_a.map do |jira_user| + { + jira_account_id: jira_user_id(jira_user), + jira_display_name: jira_user_name(jira_user), + jira_email: jira_user['emailAddress'] + }.merge(match_user(jira_user)) + end + end + + private + + def users + @users ||= client.get(url) + end + + def client + @client ||= jira_service.client + end + + def url + raise NotImplementedError + end + + def jira_user_id(jira_user) + raise NotImplementedError + end + + def jira_user_name(jira_user) + raise NotImplementedError + end + + # TODO: Matching user by email and displayName will be done as the part + # of follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/219023 + def match_user(jira_user) + { gitlab_id: nil, gitlab_username: nil, gitlab_name: nil } + end + end +end diff --git a/app/services/labels/available_labels_service.rb b/app/services/labels/available_labels_service.rb index 3b226f39d04..1d022740c44 100644 --- a/app/services/labels/available_labels_service.rb +++ b/app/services/labels/available_labels_service.rb @@ -30,7 +30,7 @@ module Labels end def filter_labels_ids_in_param(key) - ids = params[key].to_a + ids = Array.wrap(params[key]) return [] if ids.empty? # rubocop:disable CodeReuse/ActiveRecord @@ -39,12 +39,12 @@ module Labels ids.map(&:to_i) & existing_ids end - private - def available_labels @available_labels ||= LabelsFinder.new(current_user, finder_params).execute end + private + def finder_params params = { include_ancestor_groups: true } diff --git a/app/services/markdown_content_rewriter_service.rb b/app/services/markdown_content_rewriter_service.rb new file mode 100644 index 00000000000..bc6fd592eaa --- /dev/null +++ b/app/services/markdown_content_rewriter_service.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +# This service passes Markdown content through our GFM rewriter classes +# which rewrite references to GitLab objects and uploads within the content +# based on their visibility by the `target_parent`. +class MarkdownContentRewriterService + REWRITERS = [Gitlab::Gfm::ReferenceRewriter, Gitlab::Gfm::UploadsRewriter].freeze + + def initialize(current_user, content, source_parent, target_parent) + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/39654#note_399095117 + raise ArgumentError, 'The rewriter classes require that `source_parent` is a `Project`' \ + unless source_parent.is_a?(Project) + + @current_user = current_user + @content = content.presence + @source_parent = source_parent + @target_parent = target_parent + end + + def execute + return unless content + + REWRITERS.inject(content) do |text, klass| + rewriter = klass.new(text, source_parent, current_user) + rewriter.rewrite(target_parent) + end + end + + private + + attr_reader :current_user, :content, :source_parent, :target_parent +end diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index bc681397039..f0c85ae03c9 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -14,3 +14,5 @@ module MergeRequests end end end + +MergeRequests::AfterCreateService.prepend_if_ee('EE::MergeRequests::AfterCreateService') diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb index c6b3a6a1a69..30a493e91ce 100644 --- a/app/services/merge_requests/conflicts/list_service.rb +++ b/app/services/merge_requests/conflicts/list_service.rb @@ -8,7 +8,7 @@ module MergeRequests def can_be_resolved_by?(user) return false unless merge_request.source_project - access = ::Gitlab::UserAccess.new(user, project: merge_request.source_project) + access = ::Gitlab::UserAccess.new(user, container: merge_request.source_project) access.can_push_to_branch?(merge_request.source_branch) end diff --git a/app/services/merge_requests/ff_merge_service.rb b/app/services/merge_requests/ff_merge_service.rb index b3896d61a78..79011094e88 100644 --- a/app/services/merge_requests/ff_merge_service.rb +++ b/app/services/merge_requests/ff_merge_service.rb @@ -22,6 +22,7 @@ module MergeRequests ff_merge rescue Gitlab::Git::PreReceiveError => e + Gitlab::ErrorTracking.track_exception(e, pre_receive_message: e.raw_message, merge_request_id: merge_request&.id) raise MergeError, e.message rescue StandardError => e raise MergeError, "Something went wrong during merge: #{e.message}" diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index d3d661a3b75..a3c39fa2e32 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -125,7 +125,7 @@ module MergeRequests end def update_diff_discussion_positions! - return if Feature.disabled?(:merge_ref_head_comments, merge_request.target_project) + return if Feature.disabled?(:merge_ref_head_comments, merge_request.target_project, default_enabled: true) Discussions::CaptureDiffNotePositionsService.new(merge_request).execute end diff --git a/app/services/merge_requests/pushed_branches_service.rb b/app/services/merge_requests/pushed_branches_service.rb index afcf0f7678a..bbe75305d92 100644 --- a/app/services/merge_requests/pushed_branches_service.rb +++ b/app/services/merge_requests/pushed_branches_service.rb @@ -9,7 +9,7 @@ module MergeRequests def execute return [] if branch_names.blank? - source_branches = project.source_of_merge_requests.opened + source_branches = project.source_of_merge_requests.open_and_closed .from_source_branches(branch_names).pluck(:source_branch) target_branches = project.merge_requests.opened diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 29e0c22b155..cf02158b629 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -105,7 +105,7 @@ module MergeRequests def handle_assignees_change(merge_request, old_assignees) create_assignee_note(merge_request, old_assignees) notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignees) - todo_service.reassigned_issuable(merge_request, current_user, old_assignees) + todo_service.reassigned_assignable(merge_request, current_user, old_assignees) end def create_branch_change_note(issuable, branch_type, old_branch, new_branch) diff --git a/app/services/metrics/dashboard/base_service.rb b/app/services/metrics/dashboard/base_service.rb index 5fa127d64b2..5be8ae62548 100644 --- a/app/services/metrics/dashboard/base_service.rb +++ b/app/services/metrics/dashboard/base_service.rb @@ -13,7 +13,7 @@ module Metrics STAGES::MetricEndpointInserter, STAGES::VariableEndpointInserter, STAGES::PanelIdsInserter, - STAGES::Sorter, + STAGES::TrackPanelType, STAGES::AlertsInserter, STAGES::UrlValidator ].freeze @@ -34,7 +34,7 @@ module Metrics # Returns an un-processed dashboard from the cache. def raw_dashboard - Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard } + Gitlab::Metrics::Dashboard::Cache.for(project).fetch(cache_key) { get_raw_dashboard } end # Should return true if this dashboard service is for an out-of-the-box diff --git a/app/services/metrics/dashboard/clone_dashboard_service.rb b/app/services/metrics/dashboard/clone_dashboard_service.rb index a6bece391f2..d9bd9423a1b 100644 --- a/app/services/metrics/dashboard/clone_dashboard_service.rb +++ b/app/services/metrics/dashboard/clone_dashboard_service.rb @@ -9,12 +9,11 @@ module Metrics include Gitlab::Utils::StrongMemoize ALLOWED_FILE_TYPE = '.yml' - USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT + USER_DASHBOARDS_DIR = ::Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT SEQUENCES = { ::Metrics::Dashboard::SystemDashboardService::DASHBOARD_PATH => [ ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, - ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter, - ::Gitlab::Metrics::Dashboard::Stages::Sorter + ::Gitlab::Metrics::Dashboard::Stages::CustomMetricsInserter ].freeze, ::Metrics::Dashboard::SelfMonitoringDashboardService::DASHBOARD_PATH => [ @@ -22,8 +21,7 @@ module Metrics ].freeze, ::Metrics::Dashboard::ClusterDashboardService::DASHBOARD_PATH => [ - ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, - ::Gitlab::Metrics::Dashboard::Stages::Sorter + ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter ].freeze }.freeze @@ -112,7 +110,7 @@ module Metrics end def push_authorized? - Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) + Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(branch) end def dashboard_template diff --git a/app/services/metrics/dashboard/cluster_dashboard_service.rb b/app/services/metrics/dashboard/cluster_dashboard_service.rb index bfd5abf1126..4a28e847fdd 100644 --- a/app/services/metrics/dashboard/cluster_dashboard_service.rb +++ b/app/services/metrics/dashboard/cluster_dashboard_service.rb @@ -9,12 +9,11 @@ module Metrics DASHBOARD_NAME = 'Cluster' # SHA256 hash of dashboard content - DASHBOARD_VERSION = '9349afc1d96329c08ab478ea0b77db94ee5cc2549b8c754fba67a7f424666b22' + DASHBOARD_VERSION = 'e1a4f8cc2c044cf32273af2cd775eb484729baac0995db687d81d92686bf588e' SEQUENCE = [ STAGES::ClusterEndpointInserter, - STAGES::PanelIdsInserter, - STAGES::Sorter + STAGES::PanelIdsInserter ].freeze class << self diff --git a/app/services/metrics/dashboard/custom_dashboard_service.rb b/app/services/metrics/dashboard/custom_dashboard_service.rb index 741738cc3af..f0f19bf2ba3 100644 --- a/app/services/metrics/dashboard/custom_dashboard_service.rb +++ b/app/services/metrics/dashboard/custom_dashboard_service.rb @@ -6,16 +6,13 @@ module Metrics module Dashboard class CustomDashboardService < ::Metrics::Dashboard::BaseService - DASHBOARD_ROOT = ".gitlab/dashboards" - class << self def valid_params?(params) params[:dashboard_path].present? end def all_dashboard_paths(project) - file_finder(project) - .list_files_for(DASHBOARD_ROOT) + project.repository.user_defined_metrics_dashboard_paths .map do |filepath| { path: filepath, @@ -27,13 +24,9 @@ module Metrics end end - def file_finder(project) - Gitlab::Template::Finders::RepoTemplateFinder.new(project, DASHBOARD_ROOT, '.yml') - end - # Grabs the filepath after the base directory. def name_for_path(filepath) - filepath.delete_prefix("#{DASHBOARD_ROOT}/") + filepath.delete_prefix("#{Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT}/") end end @@ -41,7 +34,7 @@ module Metrics # Searches the project repo for a custom-defined dashboard. def get_raw_dashboard - yml = self.class.file_finder(project).read(dashboard_path) + yml = Gitlab::Metrics::Dashboard::RepoDashboardFinder.read_dashboard(project, dashboard_path) load_yaml(yml) end diff --git a/app/services/metrics/dashboard/custom_metric_embed_service.rb b/app/services/metrics/dashboard/custom_metric_embed_service.rb index 22b592c7aa5..229bd17f5cf 100644 --- a/app/services/metrics/dashboard/custom_metric_embed_service.rb +++ b/app/services/metrics/dashboard/custom_metric_embed_service.rb @@ -75,7 +75,6 @@ module Metrics def panels [{ type: DEFAULT_PANEL_TYPE, - weight: DEFAULT_PANEL_WEIGHT, title: title, y_label: y_label, metrics: metrics.map(&:to_metric_hash) diff --git a/app/services/metrics/dashboard/dynamic_embed_service.rb b/app/services/metrics/dashboard/dynamic_embed_service.rb index ff540c30579..0b198ecbbe9 100644 --- a/app/services/metrics/dashboard/dynamic_embed_service.rb +++ b/app/services/metrics/dashboard/dynamic_embed_service.rb @@ -18,7 +18,7 @@ module Metrics # Determines whether the provided params are sufficient # to uniquely identify a panel from a yml-defined dashboard. # - # See https://docs.gitlab.com/ee/user/project/integrations/prometheus.html#defining-custom-dashboards-per-project + # See https://docs.gitlab.com/ee/operations/metrics/dashboards/index.html#defining-custom-dashboards-per-project # for additional info on defining custom dashboards. def valid_params?(params) [ diff --git a/app/services/metrics/dashboard/gitlab_alert_embed_service.rb b/app/services/metrics/dashboard/gitlab_alert_embed_service.rb index 08d65413e1d..33c93b25c71 100644 --- a/app/services/metrics/dashboard/gitlab_alert_embed_service.rb +++ b/app/services/metrics/dashboard/gitlab_alert_embed_service.rb @@ -8,6 +8,7 @@ module Metrics module Dashboard class GitlabAlertEmbedService < ::Metrics::Dashboard::BaseEmbedService + include Gitlab::Metrics::Dashboard::Defaults include Gitlab::Utils::StrongMemoize SEQUENCE = [ @@ -63,7 +64,8 @@ module Metrics { title: prometheus_metric.title, y_label: prometheus_metric.y_label, - metrics: [prometheus_metric.to_metric_hash] + metrics: [prometheus_metric.to_metric_hash], + type: DEFAULT_PANEL_TYPE } end diff --git a/app/services/metrics/dashboard/grafana_metric_embed_service.rb b/app/services/metrics/dashboard/grafana_metric_embed_service.rb index 8e72a185406..b8c5c17c738 100644 --- a/app/services/metrics/dashboard/grafana_metric_embed_service.rb +++ b/app/services/metrics/dashboard/grafana_metric_embed_service.rb @@ -33,7 +33,7 @@ module Metrics def from_cache(project_id, user_id, grafana_url) project = Project.find(project_id) - user = User.find(user_id) + user = User.find(user_id) if user_id.present? new(project, user, grafana_url: grafana_url) end @@ -56,7 +56,7 @@ module Metrics end def cache_key(*args) - [project.id, current_user.id, grafana_url] + [project.id, current_user&.id, grafana_url] end # Required for ReactiveCaching; Usage overridden by diff --git a/app/services/metrics/dashboard/panel_preview_service.rb b/app/services/metrics/dashboard/panel_preview_service.rb new file mode 100644 index 00000000000..5b24d817fb6 --- /dev/null +++ b/app/services/metrics/dashboard/panel_preview_service.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +# Ingest YAML fragment with metrics dashboard panel definition +# https://docs.gitlab.com/ee/operations/metrics/dashboards/yaml.html#panel-panels-properties +# process it and returns renderable json version +module Metrics + module Dashboard + class PanelPreviewService + SEQUENCE = [ + ::Gitlab::Metrics::Dashboard::Stages::CommonMetricsInserter, + ::Gitlab::Metrics::Dashboard::Stages::MetricEndpointInserter, + ::Gitlab::Metrics::Dashboard::Stages::PanelIdsInserter, + ::Gitlab::Metrics::Dashboard::Stages::AlertsInserter, + ::Gitlab::Metrics::Dashboard::Stages::UrlValidator + ].freeze + + HANDLED_PROCESSING_ERRORS = [ + Gitlab::Metrics::Dashboard::Errors::DashboardProcessingError, + Gitlab::Config::Loader::Yaml::NotHashError, + Gitlab::Config::Loader::Yaml::DataTooLargeError, + Gitlab::Config::Loader::FormatError + ].freeze + + def initialize(project, panel_yaml, environment) + @project, @panel_yaml, @environment = project, panel_yaml, environment + end + + def execute + dashboard = ::Gitlab::Metrics::Dashboard::Processor.new(project, dashboard_structure, SEQUENCE, environment: environment).process + ServiceResponse.success(payload: dashboard[:panel_groups][0][:panels][0]) + rescue *HANDLED_PROCESSING_ERRORS => error + ServiceResponse.error(message: error.message) + end + + private + + attr_accessor :project, :panel_yaml, :environment + + def dashboard_structure + { + panel_groups: [ + { + panels: [panel_hash] + } + ] + } + end + + def panel_hash + ::Gitlab::Config::Loader::Yaml.new(panel_yaml).load_raw! + end + end + end +end diff --git a/app/services/metrics/dashboard/pod_dashboard_service.rb b/app/services/metrics/dashboard/pod_dashboard_service.rb index 8699189deac..c83f8618460 100644 --- a/app/services/metrics/dashboard/pod_dashboard_service.rb +++ b/app/services/metrics/dashboard/pod_dashboard_service.rb @@ -4,10 +4,28 @@ module Metrics module Dashboard class PodDashboardService < ::Metrics::Dashboard::PredefinedDashboardService DASHBOARD_PATH = 'config/prometheus/pod_metrics.yml' - DASHBOARD_NAME = 'Pod Health' + DASHBOARD_NAME = N_('K8s pod health') # SHA256 hash of dashboard content - DASHBOARD_VERSION = 'f12f641d2575d5dcb69e2c633ff5231dbd879ad35020567d8fc4e1090bfdb4b4' + DASHBOARD_VERSION = '3a91b32f91b2dd3d90275333c0ea3630b3f3f37c4296ede5b5eef59bf523d66b' + + SEQUENCE = [ + STAGES::MetricEndpointInserter, + STAGES::VariableEndpointInserter, + STAGES::PanelIdsInserter + ].freeze + + class << self + def all_dashboard_paths(_project) + [{ + path: DASHBOARD_PATH, + display_name: _(DASHBOARD_NAME), + default: false, + system_dashboard: false, + out_of_the_box_dashboard: out_of_the_box_dashboard? + }] + end + end private diff --git a/app/services/metrics/dashboard/predefined_dashboard_service.rb b/app/services/metrics/dashboard/predefined_dashboard_service.rb index c21083475f0..abdef66c2e0 100644 --- a/app/services/metrics/dashboard/predefined_dashboard_service.rb +++ b/app/services/metrics/dashboard/predefined_dashboard_service.rb @@ -12,8 +12,7 @@ module Metrics SEQUENCE = [ STAGES::MetricEndpointInserter, STAGES::VariableEndpointInserter, - STAGES::PanelIdsInserter, - STAGES::Sorter + STAGES::PanelIdsInserter ].freeze class << self @@ -30,6 +29,11 @@ module Metrics end end + # Returns an un-processed dashboard from the cache. + def raw_dashboard + Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard } + end + private def dashboard_version diff --git a/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb index f1f5cd7d77e..0651e569d07 100644 --- a/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb +++ b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb @@ -6,17 +6,16 @@ module Metrics module Dashboard class SelfMonitoringDashboardService < ::Metrics::Dashboard::PredefinedDashboardService DASHBOARD_PATH = 'config/prometheus/self_monitoring_default.yml' - DASHBOARD_NAME = N_('Default dashboard') + DASHBOARD_NAME = N_('Overview') # SHA256 hash of dashboard content - DASHBOARD_VERSION = '1dff3e3cb76e73c8e368823c98b34c61aec0d141978450dea195a3b3dc2415d6' + DASHBOARD_VERSION = '0f7ade2022e09f1a1da8e883cc95d84b9557e1e0e9b015c51eb964296aa73098' SEQUENCE = [ STAGES::CustomMetricsInserter, STAGES::MetricEndpointInserter, STAGES::VariableEndpointInserter, - STAGES::PanelIdsInserter, - STAGES::Sorter + STAGES::PanelIdsInserter ].freeze class << self @@ -29,7 +28,7 @@ module Metrics path: DASHBOARD_PATH, display_name: _(DASHBOARD_NAME), default: true, - system_dashboard: false, + system_dashboard: true, out_of_the_box_dashboard: out_of_the_box_dashboard? }] end diff --git a/app/services/metrics/dashboard/system_dashboard_service.rb b/app/services/metrics/dashboard/system_dashboard_service.rb index 5c3562b8ca0..29b8f23f40d 100644 --- a/app/services/metrics/dashboard/system_dashboard_service.rb +++ b/app/services/metrics/dashboard/system_dashboard_service.rb @@ -6,10 +6,10 @@ module Metrics module Dashboard class SystemDashboardService < ::Metrics::Dashboard::PredefinedDashboardService DASHBOARD_PATH = 'config/prometheus/common_metrics.yml' - DASHBOARD_NAME = N_('Default dashboard') + DASHBOARD_NAME = N_('Overview') # SHA256 hash of dashboard content - DASHBOARD_VERSION = '4685fe386c25b1a786b3be18f79bb2ee9828019003e003816284cdb634fa3e13' + DASHBOARD_VERSION = 'ce9ae27d2913f637de851d61099bc4151583eae68b1386a2176339ef6e653223' SEQUENCE = [ STAGES::CommonMetricsInserter, @@ -18,7 +18,6 @@ module Metrics STAGES::MetricEndpointInserter, STAGES::VariableEndpointInserter, STAGES::PanelIdsInserter, - STAGES::Sorter, STAGES::AlertsInserter ].freeze diff --git a/app/services/metrics/dashboard/update_dashboard_service.rb b/app/services/metrics/dashboard/update_dashboard_service.rb index d37d06a0222..d990e96ecb5 100644 --- a/app/services/metrics/dashboard/update_dashboard_service.rb +++ b/app/services/metrics/dashboard/update_dashboard_service.rb @@ -7,7 +7,7 @@ module Metrics include Stepable ALLOWED_FILE_TYPE = '.yml' - USER_DASHBOARDS_DIR = ::Metrics::Dashboard::CustomDashboardService::DASHBOARD_ROOT + USER_DASHBOARDS_DIR = ::Gitlab::Metrics::Dashboard::RepoDashboardFinder::DASHBOARD_ROOT steps :check_push_authorized, :check_branch_name, @@ -68,7 +68,7 @@ module Metrics end def push_authorized? - Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(branch) + Gitlab::UserAccess.new(current_user, container: project).can_push_to_branch?(branch) end def valid_branch_name? diff --git a/app/services/notes/copy_service.rb b/app/services/notes/copy_service.rb new file mode 100644 index 00000000000..6e5b4596602 --- /dev/null +++ b/app/services/notes/copy_service.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +# This service copies Notes from one Noteable to another. +# +# It expects the calling code to have performed the necessary authorization +# checks in order to allow the copy to happen. +module Notes + class CopyService + def initialize(current_user, from_noteable, to_noteable) + raise ArgumentError, 'Noteables must be different' if from_noteable == to_noteable + + @current_user = current_user + @from_noteable = from_noteable + @to_noteable = to_noteable + @from_project = from_noteable.project + @new_discussion_ids = {} + end + + def execute + from_noteable.notes_with_associations.find_each do |note| + copy_note(note) + end + + ServiceResponse.success + end + + private + + attr_reader :from_noteable, :to_noteable, :from_project, :current_user, :new_discussion_ids + + def copy_note(note) + new_note = note.dup + new_params = params_from_note(note, new_note) + new_note.update!(new_params) + + copy_award_emoji(note, new_note) + end + + def params_from_note(note, new_note) + new_discussion_ids[note.discussion_id] ||= Discussion.discussion_id(new_note) + rewritten_note = MarkdownContentRewriterService.new(current_user, note.note, from_project, to_noteable.resource_parent).execute + + new_params = { + project: to_noteable.project, + noteable: to_noteable, + discussion_id: new_discussion_ids[note.discussion_id], + note: rewritten_note, + note_html: nil, + created_at: note.created_at, + updated_at: note.updated_at + } + + if note.system_note_metadata + new_params[:system_note_metadata] = note.system_note_metadata.dup + + # TODO: Implement copying of description versions when an issue is moved + # https://gitlab.com/gitlab-org/gitlab/issues/32300 + new_params[:system_note_metadata].description_version = nil + end + + new_params + end + + def copy_award_emoji(from_note, to_note) + AwardEmojis::CopyService.new(from_note, to_note).execute + end + end +end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 935dbfb72dd..4f2329a42f2 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -54,7 +54,8 @@ module Notes def when_saved(note) if note.part_of_discussion? && note.discussion.can_convert_to_discussion? - note.discussion.convert_to_discussion!(save: true) + note.discussion.convert_to_discussion!.save + note.clear_memoization(:discussion) end todo_service.new_note(note, current_user) @@ -66,13 +67,13 @@ module Notes Gitlab::Tracking.event('Notes::CreateService', 'execute', tracking_data_for(note)) end - if Feature.enabled?(:merge_ref_head_comments, project) && note.for_merge_request? && note.diff_note? && note.start_of_discussion? + if Feature.enabled?(:merge_ref_head_comments, project, default_enabled: true) && note.for_merge_request? && note.diff_note? && note.start_of_discussion? Discussions::CaptureDiffNotePositionService.new(note.noteable, note.diff_file&.paths).execute(note.discussion) end end def do_commands(note, update_params, message, only_commands) - return if quick_actions_service.commands_executed_count.to_i.zero? + return if quick_actions_service.commands_executed_count.to_i == 0 if update_params.present? quick_actions_service.apply_updates(update_params, note) diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index c670f01e502..36d9f1d7867 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -50,6 +50,11 @@ module Notes return if update_params.empty? return unless supported?(note) + # We need the `id` after the note is persisted + if update_params[:spend_time] + update_params[:spend_time][:note_id] = note.id + end + self.class.noteable_update_service(note).new(note.resource_parent, current_user, update_params).execute(note.noteable) end end diff --git a/app/services/notes/update_service.rb b/app/services/notes/update_service.rb index 047848fd1a3..193d3080078 100644 --- a/app/services/notes/update_service.rb +++ b/app/services/notes/update_service.rb @@ -25,7 +25,7 @@ module Notes note.note = content end - unless only_commands + unless only_commands || note.for_personal_snippet? note.create_new_cross_references!(current_user) update_todos(note, old_mentioned_users) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index a4e935a8cf5..909a0033d12 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -66,6 +66,13 @@ class NotificationService mailer.access_token_about_to_expire_email(user).deliver_later end + # Notify the user when at least one of their personal access tokens has expired today + def access_token_expired(user) + return unless user.can?(:receive_notifications) + + mailer.access_token_expired_email(user).deliver_later + end + # Notify a user when a previously unknown IP or device is used to # sign in to their account def unknown_sign_in(user, ip, time) @@ -424,8 +431,8 @@ class NotificationService end def project_was_moved(project, old_path_with_namespace) - recipients = project.private? ? project.team.members_in_project_and_ancestors : project.team.members - recipients = notifiable_users(recipients, :mention, project: project) + recipients = project_moved_recipients(project) + recipients = notifiable_users(recipients, :custom, custom_action: :moved_project, project: project) recipients.each do |recipient| mailer.project_was_moved_email( @@ -705,6 +712,14 @@ class NotificationService recipients end + def project_moved_recipients(project) + finder = MembersFinder.new(project, nil, params: { + active_without_invites_and_requests: true, + owners_and_maintainers: true + }) + finder.execute.preload_user_and_notification_settings.map(&:user) + end + def project_maintainers_recipients(target, action:) NotificationRecipients::BuildService.build_project_maintainers_recipients(target, action: action) end diff --git a/app/services/packages/maven/find_or_create_package_service.rb b/app/services/packages/maven/find_or_create_package_service.rb index 50a008843ad..505f45a7b21 100644 --- a/app/services/packages/maven/find_or_create_package_service.rb +++ b/app/services/packages/maven/find_or_create_package_service.rb @@ -3,21 +3,33 @@ module Packages module Maven class FindOrCreatePackageService < BaseService MAVEN_METADATA_FILE = 'maven-metadata.xml'.freeze + SNAPSHOT_TERM = '-SNAPSHOT'.freeze def execute - package = ::Packages::Maven::PackageFinder - .new(params[:path], current_user, project: project).execute + package = + ::Packages::Maven::PackageFinder.new(params[:path], current_user, project: project) + .execute unless package - if params[:file_name] == MAVEN_METADATA_FILE - # Maven uploads several files during `mvn deploy` in next order: - # - my-company/my-app/1.0-SNAPSHOT/my-app.jar - # - my-company/my-app/1.0-SNAPSHOT/my-app.pom - # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml - # - my-company/my-app/maven-metadata.xml - # - # The last xml file does not have VERSION in URL because it contains - # information about all versions. + # Maven uploads several files during `mvn deploy` in next order: + # - my-company/my-app/1.0-SNAPSHOT/my-app.jar + # - my-company/my-app/1.0-SNAPSHOT/my-app.pom + # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml + # - my-company/my-app/maven-metadata.xml + # + # The last xml file does not have VERSION in URL because it contains + # information about all versions. When uploading such file, we create + # a package with a version set to `nil`. The xml file with a version + # is only created and uploaded for snapshot versions. + # + # Gradle has a different upload order: + # - my-company/my-app/1.0-SNAPSHOT/maven-metadata.xml + # - my-company/my-app/1.0-SNAPSHOT/my-app.jar + # - my-company/my-app/1.0-SNAPSHOT/my-app.pom + # - my-company/my-app/maven-metadata.xml + # + # The first upload has to create the proper package (the one with the version set). + if params[:file_name] == MAVEN_METADATA_FILE && !params[:path]&.ends_with?(SNAPSHOT_TERM) package_name, version = params[:path], nil else package_name, _, version = params[:path].rpartition('/') @@ -30,8 +42,9 @@ module Packages build: params[:build] } - package = ::Packages::Maven::CreatePackageService - .new(project, current_user, package_params).execute + package = + ::Packages::Maven::CreatePackageService.new(project, current_user, package_params) + .execute end package diff --git a/app/services/packages/nuget/metadata_extraction_service.rb b/app/services/packages/nuget/metadata_extraction_service.rb index 6fec398fab0..59125669f7d 100644 --- a/app/services/packages/nuget/metadata_extraction_service.rb +++ b/app/services/packages/nuget/metadata_extraction_service.rb @@ -42,7 +42,7 @@ module Packages def valid_package_file? package_file && package_file.package&.nuget? && - package_file.file.size.positive? + package_file.file.size > 0 # rubocop:disable Style/ZeroLengthPredicate end def extract_metadata(file) diff --git a/app/services/packages/nuget/search_service.rb b/app/services/packages/nuget/search_service.rb index f7e09e11819..b95aa30bec1 100644 --- a/app/services/packages/nuget/search_service.rb +++ b/app/services/packages/nuget/search_service.rb @@ -21,8 +21,8 @@ module Packages @search_term = search_term @options = DEFAULT_OPTIONS.merge(options) - raise ArgumentError, 'negative per_page' if per_page.negative? - raise ArgumentError, 'negative padding' if padding.negative? + raise ArgumentError, 'negative per_page' if per_page < 0 + raise ArgumentError, 'negative padding' if padding < 0 end def execute diff --git a/app/services/personal_access_tokens/revoke_service.rb b/app/services/personal_access_tokens/revoke_service.rb new file mode 100644 index 00000000000..16ba42bd317 --- /dev/null +++ b/app/services/personal_access_tokens/revoke_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module PersonalAccessTokens + class RevokeService + attr_reader :token, :current_user + + def initialize(current_user = nil, params = { token: nil }) + @current_user = current_user + @token = params[:token] + end + + def execute + return ServiceResponse.error(message: 'Not permitted to revoke') unless revocation_permitted? + + if token.revoke! + ServiceResponse.success(message: success_message) + else + ServiceResponse.error(message: error_message) + end + end + + private + + def error_message + _("Could not revoke personal access token %{personal_access_token_name}.") % { personal_access_token_name: token.name } + end + + def success_message + _("Revoked personal access token %{personal_access_token_name}!") % { personal_access_token_name: token.name } + end + + def revocation_permitted? + Ability.allowed?(current_user, :revoke_token, token) + end + end +end diff --git a/app/services/product_analytics/build_graph_service.rb b/app/services/product_analytics/build_graph_service.rb new file mode 100644 index 00000000000..31f9f093bb9 --- /dev/null +++ b/app/services/product_analytics/build_graph_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module ProductAnalytics + class BuildGraphService + def initialize(project, params) + @project = project + @params = params + end + + def execute + graph = @params[:graph].to_sym + timerange = @params[:timerange].days + + results = product_analytics_events.count_by_graph(graph, timerange) + + { + id: graph, + keys: results.keys, + values: results.values + } + end + + private + + def product_analytics_events + @project.product_analytics_events + end + end +end diff --git a/app/services/projects/alerting/notify_service.rb b/app/services/projects/alerting/notify_service.rb index e08bc8efb15..f883c8c7bd8 100644 --- a/app/services/projects/alerting/notify_service.rb +++ b/app/services/projects/alerting/notify_service.rb @@ -58,10 +58,6 @@ module Projects AlertManagement::Alert.not_resolved.for_fingerprint(project, fingerprint).first end - def send_email? - incident_management_setting.send_email? - end - def process_incident_issues(alert) return if alert.issue diff --git a/app/services/projects/auto_devops/disable_service.rb b/app/services/projects/auto_devops/disable_service.rb index c90510c581d..e10668ac9bd 100644 --- a/app/services/projects/auto_devops/disable_service.rb +++ b/app/services/projects/auto_devops/disable_service.rb @@ -23,7 +23,7 @@ module Projects # for more context. # rubocop: disable CodeReuse/ActiveRecord def first_pipeline_failure? - auto_devops_pipelines.success.limit(1).count.zero? && + auto_devops_pipelines.success.limit(1).count == 0 && auto_devops_pipelines.failed.limit(1).count.nonzero? end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/projects/cleanup_service.rb b/app/services/projects/cleanup_service.rb index 04624b96bf0..4ced9feff00 100644 --- a/app/services/projects/cleanup_service.rb +++ b/app/services/projects/cleanup_service.rb @@ -22,7 +22,7 @@ module Projects apply_bfg_object_map! # Remove older objects that are no longer referenced - GitGarbageCollectWorker.new.perform(project.id, :gc) + GitGarbageCollectWorker.new.perform(project.id, :gc, "project_cleanup:gc:#{project.id}") # The cache may now be inaccurate, and holding onto it could prevent # bugs assuming the presence of some object from manifesting for some diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index c5809c11ea9..204a54ff23a 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -39,11 +39,8 @@ module Projects end def filter_by_name(tags) - # Technical Debt: https://gitlab.com/gitlab-org/gitlab/issues/207267 - # name_regex to be removed when container_expiration_policies is updated - # to have both regex columns - regex_delete = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_delete'] || params['name_regex']}\\z") - regex_retain = Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_keep']}\\z") + regex_delete = ::Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_delete'] || params['name_regex']}\\z") + regex_retain = ::Gitlab::UntrustedRegexp.new("\\A#{params['name_regex_keep']}\\z") tags.select do |tag| # regex_retain will override any overlapping matches by regex_delete @@ -81,11 +78,11 @@ module Projects def valid_regex? %w(name_regex_delete name_regex name_regex_keep).each do |param_name| regex = params[param_name] - Gitlab::UntrustedRegexp.new(regex) unless regex.blank? + ::Gitlab::UntrustedRegexp.new(regex) unless regex.blank? end true rescue RegexpError => e - Gitlab::ErrorTracking.log_exception(e, project_id: project.id) + ::Gitlab::ErrorTracking.log_exception(e, project_id: project.id) false end end diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index 5d4059710bb..a23a6a369b2 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -6,65 +6,35 @@ module Projects LOG_DATA_BASE = { service_class: self.to_s }.freeze def execute(container_repository) + @container_repository = container_repository return error('access denied') unless can?(current_user, :destroy_container_image, project) - tag_names = params[:tags] - return error('not tags specified') if tag_names.blank? + @tag_names = params[:tags] + return error('not tags specified') if @tag_names.blank? - smart_delete(container_repository, tag_names) + delete_tags end private - # Delete tags by name with a single DELETE request. This is only supported - # by the GitLab Container Registry fork. See - # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23325 for details. - def fast_delete(container_repository, tag_names) - deleted_tags = tag_names.select do |name| - container_repository.delete_tag_by_name(name) - end - - deleted_tags.any? ? success(deleted: deleted_tags) : error('could not delete tags') + def delete_tags + delete_service.execute + .tap(&method(:log_response)) end - # Replace a tag on the registry with a dummy tag. - # This is a hack as the registry doesn't support deleting individual - # tags. This code effectively pushes a dummy image and assigns the tag to it. - # This way when the tag is deleted only the dummy image is affected. - # This is used to preverse compatibility with third-party registries that - # don't support fast delete. - # See https://gitlab.com/gitlab-org/gitlab/issues/15737 for a discussion - def slow_delete(container_repository, tag_names) - # generates the blobs for the dummy image - dummy_manifest = container_repository.client.generate_empty_manifest(container_repository.path) - return error('could not generate manifest') if dummy_manifest.nil? - - deleted_tags = replace_tag_manifests(container_repository, dummy_manifest, tag_names) + def delete_service + fast_delete_enabled = Feature.enabled?(:container_registry_fast_tag_delete, default_enabled: true) - # Deletes the dummy image - # All created tag digests are the same since they all have the same dummy image. - # a single delete is sufficient to remove all tags with it - if deleted_tags.any? && container_repository.delete_tag_by_digest(deleted_tags.each_value.first) - success(deleted: deleted_tags.keys) + if fast_delete_enabled && @container_repository.client.supports_tag_delete? + ::Projects::ContainerRepository::Gitlab::DeleteTagsService.new(@container_repository, @tag_names) else - error('could not delete tags') + ::Projects::ContainerRepository::ThirdParty::DeleteTagsService.new(@container_repository, @tag_names) end end - def smart_delete(container_repository, tag_names) - fast_delete_enabled = Feature.enabled?(:container_registry_fast_tag_delete, default_enabled: true) - response = if fast_delete_enabled && container_repository.client.supports_tag_delete? - fast_delete(container_repository, tag_names) - else - slow_delete(container_repository, tag_names) - end - - response.tap { |r| log_response(r, container_repository) } - end - - def log_response(response, container_repository) + def log_response(response) log_data = LOG_DATA_BASE.merge( - container_repository_id: container_repository.id, + container_repository_id: @container_repository.id, message: 'deleted tags' ) @@ -76,26 +46,6 @@ module Projects log_error(log_data) end end - - # update the manifests of the tags with the new dummy image - def replace_tag_manifests(container_repository, dummy_manifest, tag_names) - deleted_tags = {} - - tag_names.each do |name| - digest = container_repository.client.put_tag(container_repository.path, name, dummy_manifest) - next unless digest - - deleted_tags[name] = digest - end - - # make sure the digests are the same (it should always be) - digests = deleted_tags.values.uniq - - # rubocop: disable CodeReuse/ActiveRecord - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many? - - deleted_tags - end end end end diff --git a/app/services/projects/container_repository/gitlab/delete_tags_service.rb b/app/services/projects/container_repository/gitlab/delete_tags_service.rb new file mode 100644 index 00000000000..18049648e26 --- /dev/null +++ b/app/services/projects/container_repository/gitlab/delete_tags_service.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Projects + module ContainerRepository + module Gitlab + class DeleteTagsService + include BaseServiceUtility + + def initialize(container_repository, tag_names) + @container_repository = container_repository + @tag_names = tag_names + end + + # Delete tags by name with a single DELETE request. This is only supported + # by the GitLab Container Registry fork. See + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/23325 for details. + def execute + return success(deleted: []) if @tag_names.empty? + + deleted_tags = @tag_names.select do |name| + @container_repository.delete_tag_by_name(name) + end + + deleted_tags.any? ? success(deleted: deleted_tags) : error('could not delete tags') + end + end + end + end +end diff --git a/app/services/projects/container_repository/third_party/delete_tags_service.rb b/app/services/projects/container_repository/third_party/delete_tags_service.rb new file mode 100644 index 00000000000..6504172109e --- /dev/null +++ b/app/services/projects/container_repository/third_party/delete_tags_service.rb @@ -0,0 +1,64 @@ +# frozen_string_literal: true + +module Projects + module ContainerRepository + module ThirdParty + class DeleteTagsService + include BaseServiceUtility + + def initialize(container_repository, tag_names) + @container_repository = container_repository + @tag_names = tag_names + end + + # Replace a tag on the registry with a dummy tag. + # This is a hack as the registry doesn't support deleting individual + # tags. This code effectively pushes a dummy image and assigns the tag to it. + # This way when the tag is deleted only the dummy image is affected. + # This is used to preverse compatibility with third-party registries that + # don't support fast delete. + # See https://gitlab.com/gitlab-org/gitlab/issues/15737 for a discussion + def execute + return success(deleted: []) if @tag_names.empty? + + # generates the blobs for the dummy image + dummy_manifest = @container_repository.client.generate_empty_manifest(@container_repository.path) + return error('could not generate manifest') if dummy_manifest.nil? + + deleted_tags = replace_tag_manifests(dummy_manifest) + + # Deletes the dummy image + # All created tag digests are the same since they all have the same dummy image. + # a single delete is sufficient to remove all tags with it + if deleted_tags.any? && @container_repository.delete_tag_by_digest(deleted_tags.each_value.first) + success(deleted: deleted_tags.keys) + else + error('could not delete tags') + end + end + + private + + # update the manifests of the tags with the new dummy image + def replace_tag_manifests(dummy_manifest) + deleted_tags = {} + + @tag_names.each do |name| + digest = @container_repository.client.put_tag(@container_repository.path, name, dummy_manifest) + next unless digest + + deleted_tags[name] = digest + end + + # make sure the digests are the same (it should always be) + digests = deleted_tags.values.uniq + + # rubocop: disable CodeReuse/ActiveRecord + Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ArgumentError.new('multiple tag digests')) if digests.many? + + deleted_tags + end + end + end + end +end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 6569277ad9d..33ed1151407 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -55,9 +55,11 @@ module Projects save_project_and_import_data - after_create_actions if @project.persisted? + Gitlab::ApplicationContext.with_context(related_class: "Projects::CreateService", project: @project) do + after_create_actions if @project.persisted? - import_schedule + import_schedule + end @project rescue ActiveRecord::RecordInvalid => e diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 2e949f2fc55..37487261f2c 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -31,7 +31,7 @@ module Projects attempt_destroy_transaction(project) system_hook_service.execute_hooks_for(project, :destroy) - log_info("Project \"#{project.full_path}\" was removed") + log_info("Project \"#{project.full_path}\" was deleted") current_user.invalidate_personal_projects_count diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 6ac53b15ef9..bb660d47887 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -14,10 +14,10 @@ module Projects @valid_fork_targets ||= ForkTargetsFinder.new(@project, current_user).execute end - def valid_fork_target? + def valid_fork_target?(namespace = target_namespace) return true if current_user.admin? - valid_fork_targets.include?(target_namespace) + valid_fork_targets.include?(namespace) end private diff --git a/app/services/projects/prometheus/alerts/notify_service.rb b/app/services/projects/prometheus/alerts/notify_service.rb index ea557ebe20f..d32ead76d00 100644 --- a/app/services/projects/prometheus/alerts/notify_service.rb +++ b/app/services/projects/prometheus/alerts/notify_service.rb @@ -42,10 +42,6 @@ module Projects Gitlab::Utils::DeepSize.new(params).valid? end - def send_email? - incident_management_setting.send_email && firings.any? - end - def firings @firings ||= alerts_by_status('firing') end @@ -125,6 +121,8 @@ module Projects end def send_alert_email + return unless firings.any? + notification_service .async .prometheus_alerts_fired(project, firings) diff --git a/app/services/projects/propagate_service_template.rb b/app/services/projects/propagate_service_template.rb index b6465810fde..54d09b354a1 100644 --- a/app/services/projects/propagate_service_template.rb +++ b/app/services/projects/propagate_service_template.rb @@ -66,7 +66,7 @@ module Projects # rubocop: disable CodeReuse/ActiveRecord def run_callbacks(batch) - if active_external_issue_tracker? + if template.issue_tracker? Project.where(id: batch).update_all(has_external_issue_tracker: true) end @@ -76,10 +76,6 @@ module Projects end # rubocop: enable CodeReuse/ActiveRecord - def active_external_issue_tracker? - template.issue_tracker? && !template.default - end - def active_external_wiki? template.type == 'ExternalWikiService' end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 60e5b7e2639..0fb70feec86 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -55,10 +55,18 @@ module Projects raise TransferError.new(s_('TransferProject|Project cannot be transferred, because tags are present in its container registry')) end + if project.has_packages?(:npm) && !new_namespace_has_same_root?(project) + raise TransferError.new(s_("TransferProject|Root namespace can't be updated if project has NPM packages")) + end + attempt_transfer_transaction end # rubocop: enable CodeReuse/ActiveRecord + def new_namespace_has_same_root?(project) + new_namespace.root_ancestor == project.namespace.root_ancestor + end + def attempt_transfer_transaction Project.transaction do project.expire_caches_before_rename(@old_path) diff --git a/app/services/projects/update_pages_configuration_service.rb b/app/services/projects/update_pages_configuration_service.rb index 674071ad92a..88c17d502df 100644 --- a/app/services/projects/update_pages_configuration_service.rb +++ b/app/services/projects/update_pages_configuration_service.rb @@ -11,15 +11,20 @@ module Projects end def execute - if file_equals?(pages_config_file, pages_config_json) - return success(reload: false) + # If the pages were never deployed, we can't write out the config, as the + # directory would not exist. + # https://gitlab.com/gitlab-org/gitlab/-/issues/235139 + return success unless project.pages_deployed? + + unless file_equals?(pages_config_file, pages_config_json) + update_file(pages_config_file, pages_config_json) + reload_daemon end - update_file(pages_config_file, pages_config_json) - reload_daemon - success(reload: true) + success rescue => e - error(e.message) + Gitlab::ErrorTracking.track_exception(e) + error(e.message, pass_back: { exception: e }) end private diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 59389a0fa65..334f5993d15 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -136,7 +136,7 @@ module Projects def max_size max_pages_size = max_size_from_settings - return ::Gitlab::Pages::MAX_SIZE if max_pages_size.zero? + return ::Gitlab::Pages::MAX_SIZE if max_pages_size == 0 max_pages_size end diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index d6c0d647468..fe2610f89fb 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -25,14 +25,8 @@ module Projects def update_mirror(remote_mirror) remote_mirror.update_start! - remote_mirror.ensure_remote! - # https://gitlab.com/gitlab-org/gitaly/-/issues/2670 - if Feature.disabled?(:gitaly_ruby_remote_branches_ls_remote, default_enabled: true) - repository.fetch_remote(remote_mirror.remote_name, ssh_auth: remote_mirror, no_tags: true) - end - response = remote_mirror.update_repository if response.divergent_refs.any? diff --git a/app/services/projects/update_repository_storage_service.rb b/app/services/projects/update_repository_storage_service.rb index 7b346c09635..a479d53a43a 100644 --- a/app/services/projects/update_repository_storage_service.rb +++ b/app/services/projects/update_repository_storage_service.rb @@ -6,8 +6,7 @@ module Projects SameFilesystemError = Class.new(Error) attr_reader :repository_storage_move - delegate :project, :destination_storage_name, to: :repository_storage_move - delegate :repository, to: :project + delegate :project, :source_storage_name, :destination_storage_name, to: :repository_storage_move def initialize(repository_storage_move) @repository_storage_move = repository_storage_move @@ -20,21 +19,22 @@ module Projects repository_storage_move.start! end - raise SameFilesystemError if same_filesystem?(repository.storage, destination_storage_name) + raise SameFilesystemError if same_filesystem?(source_storage_name, destination_storage_name) mirror_repositories - project.transaction do - mark_old_paths_for_archive - - repository_storage_move.finish! + repository_storage_move.transaction do + repository_storage_move.finish_replication! project.leave_pool_repository project.track_project_repository end + remove_old_paths enqueue_housekeeping + repository_storage_move.finish_cleanup! + ServiceResponse.success rescue StandardError => e @@ -91,36 +91,31 @@ module Projects end end - def mark_old_paths_for_archive - old_repository_storage = project.repository_storage - new_project_path = moved_path(project.disk_path) - - # Notice that the block passed to `run_after_commit` will run with `repository_storage_move` - # as its context - repository_storage_move.run_after_commit do - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.disk_path, - new_project_path) - - if project.wiki.repository_exists? - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.wiki.disk_path, - "#{new_project_path}.wiki") - end - - if project.design_repository.exists? - GitlabShellWorker.perform_async(:mv_repository, - old_repository_storage, - project.design_repository.disk_path, - "#{new_project_path}.design") - end + def remove_old_paths + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.disk_path}.git", + nil, + nil + ).remove + + if project.wiki.repository_exists? + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.wiki.disk_path}.git", + nil, + nil + ).remove end - end - def moved_path(path) - "#{path}+#{project.id}+moved+#{Time.current.to_i}" + if project.design_repository.exists? + Gitlab::Git::Repository.new( + source_storage_name, + "#{project.design_repository.disk_path}.git", + nil, + nil + ).remove + end end # The underlying FetchInternalRemote call uses a `git fetch` to move data diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 58c9bce963b..c9ba7cde199 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -142,7 +142,13 @@ module Projects end def update_pages_config - Projects::UpdatePagesConfigurationService.new(project).execute + return unless project.pages_deployed? + + if Feature.enabled?(:async_update_pages_config, project) + PagesUpdateConfigurationWorker.perform_async(project.id) + else + Projects::UpdatePagesConfigurationService.new(project).execute + end end def changing_pages_https_only? diff --git a/app/services/resource_access_tokens/create_service.rb b/app/services/resource_access_tokens/create_service.rb index 2d0a78feb8e..c253154c1b7 100644 --- a/app/services/resource_access_tokens/create_service.rb +++ b/app/services/resource_access_tokens/create_service.rb @@ -32,7 +32,9 @@ module ResourceAccessTokens attr_reader :resource_type, :resource def feature_enabled? - ::Feature.enabled?(:resource_access_token, resource) + return false if ::Gitlab.com? + + ::Feature.enabled?(:resource_access_token, resource, default_enabled: true) end def has_permission_to_create? diff --git a/app/services/resource_events/base_change_timebox_service.rb b/app/services/resource_events/base_change_timebox_service.rb new file mode 100644 index 00000000000..5c83f7b12f7 --- /dev/null +++ b/app/services/resource_events/base_change_timebox_service.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module ResourceEvents + class BaseChangeTimeboxService + attr_reader :resource, :user, :event_created_at + + def initialize(resource, user, created_at: Time.current) + @resource = resource + @user = user + @event_created_at = created_at + end + + def execute + create_event + + resource.expire_note_etag_cache + end + + private + + def create_event + raise NotImplementedError + end + + def build_resource_args + key = resource.class.name.foreign_key + + { + user_id: user.id, + created_at: event_created_at, + key => resource.id + } + end + end +end diff --git a/app/services/resource_events/change_milestone_service.rb b/app/services/resource_events/change_milestone_service.rb index 82c3e2acad5..dcdf87599ac 100644 --- a/app/services/resource_events/change_milestone_service.rb +++ b/app/services/resource_events/change_milestone_service.rb @@ -1,37 +1,30 @@ # frozen_string_literal: true module ResourceEvents - class ChangeMilestoneService - attr_reader :resource, :user, :event_created_at, :milestone, :old_milestone + class ChangeMilestoneService < BaseChangeTimeboxService + attr_reader :milestone, :old_milestone def initialize(resource, user, created_at: Time.current, old_milestone:) - @resource = resource - @user = user - @event_created_at = created_at + super(resource, user, created_at: created_at) + @milestone = resource&.milestone @old_milestone = old_milestone end - def execute - ResourceMilestoneEvent.create(build_resource_args) + private - resource.expire_note_etag_cache + def create_event + ResourceMilestoneEvent.create(build_resource_args) end - private - def build_resource_args action = milestone.blank? ? :remove : :add - key = resource.class.name.foreign_key - { - user_id: user.id, - created_at: event_created_at, - milestone_id: action == :add ? milestone&.id : old_milestone&.id, + super.merge({ state: ResourceMilestoneEvent.states[resource.state], - action: ResourceMilestoneEvent.actions[action], - key => resource.id - } + action: ResourceTimeboxEvent.actions[action], + milestone_id: milestone.blank? ? old_milestone&.id : milestone&.id + }) end end end diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 650dc197f8c..278cf389e07 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -70,7 +70,7 @@ class SearchService def per_page per_page_param = params[:per_page].to_i - return DEFAULT_PER_PAGE unless per_page_param.positive? + return DEFAULT_PER_PAGE unless per_page_param > 0 [MAX_PER_PAGE, per_page_param].min end diff --git a/app/services/service_desk_settings/update_service.rb b/app/services/service_desk_settings/update_service.rb index 08106b04d18..c837b75f439 100644 --- a/app/services/service_desk_settings/update_service.rb +++ b/app/services/service_desk_settings/update_service.rb @@ -9,6 +9,8 @@ module ServiceDeskSettings params.delete(:project_key) end + params[:project_key] = nil if params[:project_key].blank? + if settings.update(params) success else diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb index 4bbde3a9648..9191943caa7 100644 --- a/app/services/submit_usage_ping_service.rb +++ b/app/services/submit_usage_ping_service.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class SubmitUsagePingService - URL = 'https://version.gitlab.com/usage_data' + PRODUCTION_URL = 'https://version.gitlab.com/usage_data' + STAGING_URL = 'https://gitlab-services-version-gitlab-com-staging.gs-staging.gitlab.org/usage_data' METRICS = %w[leader_issues instance_issues percentage_issues leader_notes instance_notes percentage_notes leader_milestones instance_milestones percentage_milestones @@ -13,28 +14,42 @@ class SubmitUsagePingService percentage_projects_prometheus_active leader_service_desk_issues instance_service_desk_issues percentage_service_desk_issues].freeze + SubmissionError = Class.new(StandardError) + def execute - return false unless Gitlab::CurrentSettings.usage_ping_enabled? - return false if User.single_user&.requires_usage_stats_consent? + return unless Gitlab::CurrentSettings.usage_ping_enabled? + return if User.single_user&.requires_usage_stats_consent? + + usage_data = Gitlab::UsageData.data(force_refresh: true) + + raise SubmissionError.new('Usage data is blank') if usage_data.blank? + + raw_usage_data = save_raw_usage_data(usage_data) response = Gitlab::HTTP.post( - URL, - body: Gitlab::UsageData.to_json(force_refresh: true), + url, + body: usage_data.to_json, allow_local_requests: true, headers: { 'Content-type' => 'application/json' } ) - store_metrics(response) + raise SubmissionError.new("Unsuccessful response code: #{response.code}") unless response.success? - true - rescue Gitlab::HTTP::Error => e - Gitlab::AppLogger.info("Unable to contact GitLab, Inc.: #{e}") + raw_usage_data.update_sent_at! if raw_usage_data - false + store_metrics(response) end private + def save_raw_usage_data(usage_data) + return unless Feature.enabled?(:save_raw_usage_data) + + RawUsageData.safe_find_or_create_by(recorded_at: usage_data[:recorded_at]) do |record| + record.payload = usage_data + end + end + def store_metrics(response) metrics = response['conv_index'] || response['dev_ops_score'] @@ -44,4 +59,13 @@ class SubmitUsagePingService metrics.slice(*METRICS) ) end + + # See https://gitlab.com/gitlab-org/gitlab/-/issues/233615 for details + def url + if Rails.env.production? + PRODUCTION_URL + else + STAGING_URL + end + end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index db5693960b2..6702596f17c 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -297,7 +297,7 @@ module SystemNoteService end def new_alert_issue(alert, issue, author) - ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).new_alert_issue(alert, issue) + ::SystemNotes::AlertManagementService.new(noteable: alert, project: alert.project, author: author).new_alert_issue(issue) end private diff --git a/app/services/system_notes/alert_management_service.rb b/app/services/system_notes/alert_management_service.rb index 55a6a17bbca..f835376727a 100644 --- a/app/services/system_notes/alert_management_service.rb +++ b/app/services/system_notes/alert_management_service.rb @@ -12,7 +12,7 @@ module SystemNotes # # Returns the created Note object def change_alert_status(alert) - status = AlertManagement::Alert::STATUSES.key(alert.status).to_s.titleize + status = alert.state.to_s.titleize body = "changed the status to **#{status}**" create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) @@ -20,7 +20,6 @@ module SystemNotes # Called when an issue is created based on an AlertManagement::Alert # - # alert - AlertManagement::Alert object. # issue - Issue object. # # Example Note text: @@ -28,10 +27,25 @@ module SystemNotes # "created issue #17 for this alert" # # Returns the created Note object - def new_alert_issue(alert, issue) + def new_alert_issue(issue) body = "created issue #{issue.to_reference(project)} for this alert" create_note(NoteSummary.new(noteable, project, author, body, action: 'alert_issue_added')) end + + # Called when an AlertManagement::Alert is resolved due to the associated issue being closed + # + # issue - Issue object. + # + # Example Note text: + # + # "changed the status to Resolved by closing issue #17" + # + # Returns the created Note object + def closed_alert_issue(issue) + body = "changed the status to **Resolved** by closing issue #{issue.to_reference(project)}" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'status')) + end end end diff --git a/app/services/system_notes/issuables_service.rb b/app/services/system_notes/issuables_service.rb index 76261aa716e..7535db54130 100644 --- a/app/services/system_notes/issuables_service.rb +++ b/app/services/system_notes/issuables_service.rb @@ -341,7 +341,7 @@ module SystemNotes def state_change_tracking_enabled? noteable.respond_to?(:resource_state_events) && - ::Feature.enabled?(:track_resource_state_change_events, noteable.project) + ::Feature.enabled?(:track_resource_state_change_events, noteable.project, default_enabled: true) end end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index ec15bdde8d7..a3db2ae7947 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -49,11 +49,11 @@ class TodoService todo_users.each(&:update_todos_count_cache) end - # When we reassign an issuable we should: + # When we reassign an assignable object (issuable, alert) we should: # - # * create a pending todo for new assignee if issuable is assigned + # * create a pending todo for new assignee if object is assigned # - def reassigned_issuable(issuable, current_user, old_assignees = []) + def reassigned_assignable(issuable, current_user, old_assignees = []) create_assignment_todo(issuable, current_user, old_assignees) end @@ -154,14 +154,6 @@ class TodoService resolve_todos_for_target(awardable, current_user) end - # When assigning an alert we should: - # - # * create a pending todo for new assignee if alert is assigned - # - def assign_alert(alert, current_user) - create_assignment_todo(alert, current_user, []) - end - # When user marks a target as todo def mark_todo(target, current_user) attributes = attributes_for_todo(target.project, target, current_user, Todo::MARKED) diff --git a/app/services/users/refresh_authorized_projects_service.rb b/app/services/users/refresh_authorized_projects_service.rb index 621266f00e1..d0939d5a542 100644 --- a/app/services/users/refresh_authorized_projects_service.rb +++ b/app/services/users/refresh_authorized_projects_service.rb @@ -53,7 +53,13 @@ module Users current = current_authorizations_per_project fresh = fresh_access_levels_per_project - remove = current.each_with_object([]) do |(project_id, row), array| + # Delete projects that have more than one authorizations associated with + # the user. The correct authorization is added to the ``add`` array in the + # next stage. + remove = projects_with_duplicates + current.except!(*projects_with_duplicates) + + remove |= current.each_with_object([]) do |(project_id, row), array| # rows not in the new list or with a different access level should be # removed. if !fresh[project_id] || fresh[project_id] != row.access_level @@ -106,7 +112,7 @@ module Users end def current_authorizations - user.project_authorizations.select(:project_id, :access_level) + @current_authorizations ||= user.project_authorizations.select(:project_id, :access_level) end def fresh_authorizations @@ -116,5 +122,12 @@ module Users private attr_reader :incorrect_auth_found_callback, :missing_auth_found_callback + + def projects_with_duplicates + @projects_with_duplicates ||= current_authorizations + .group_by(&:project_id) + .select { |project_id, authorizations| authorizations.count > 1 } + .keys + end end end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 91a26ff45b1..d6cb0729d6f 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -13,6 +13,7 @@ class WebHookService end end + REQUEST_BODY_SIZE_LIMIT = 25.megabytes GITLAB_EVENT_HEADER = 'X-Gitlab-Event' attr_accessor :hook, :data, :hook_name, :request_options @@ -53,17 +54,18 @@ class WebHookService http_status: response.code, message: response.to_s } - rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep => e + rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep, Gitlab::Json::LimitedEncoder::LimitExceeded => e + execution_duration = Gitlab::Metrics::System.monotonic_time - start_time log_execution( trigger: hook_name, url: hook.url, request_data: data, response: InternalErrorResponse.new, - execution_duration: Gitlab::Metrics::System.monotonic_time - start_time, + execution_duration: execution_duration, error_message: e.to_s ) - Gitlab::AppLogger.error("WebHook Error => #{e}") + Gitlab::AppLogger.error("WebHook Error after #{execution_duration.to_i.seconds}s => #{e}") { status: :error, @@ -83,7 +85,7 @@ class WebHookService def make_request(url, basic_auth = false) Gitlab::HTTP.post(url, - body: data.to_json, + body: Gitlab::Json::LimitedEncoder.encode(data, limit: REQUEST_BODY_SIZE_LIMIT), headers: build_headers(hook_name), verify: hook.enable_ssl_verification, basic_auth: basic_auth, diff --git a/app/services/wiki_pages/base_service.rb b/app/services/wiki_pages/base_service.rb index 2967684f7bc..fd234630633 100644 --- a/app/services/wiki_pages/base_service.rb +++ b/app/services/wiki_pages/base_service.rb @@ -44,7 +44,9 @@ module WikiPages end def create_wiki_event(page) - response = WikiPages::EventCreateService.new(current_user).execute(slug_for_page(page), page, event_action) + response = WikiPages::EventCreateService + .new(current_user) + .execute(slug_for_page(page), page, event_action, fingerprint(page)) log_error(response.message) if response.error? end @@ -52,6 +54,10 @@ module WikiPages def slug_for_page(page) page.slug end + + def fingerprint(page) + page.sha + end end end diff --git a/app/services/wiki_pages/create_service.rb b/app/services/wiki_pages/create_service.rb index 63107445782..9702876effa 100644 --- a/app/services/wiki_pages/create_service.rb +++ b/app/services/wiki_pages/create_service.rb @@ -10,7 +10,11 @@ module WikiPages execute_hooks(page) end - page + if page.persisted? + ServiceResponse.success(payload: { page: page }) + else + ServiceResponse.error(message: _('Could not create wiki page'), payload: { page: page }) + end end def usage_counter_action diff --git a/app/services/wiki_pages/destroy_service.rb b/app/services/wiki_pages/destroy_service.rb index d59c27bb92a..ab5abe1c82b 100644 --- a/app/services/wiki_pages/destroy_service.rb +++ b/app/services/wiki_pages/destroy_service.rb @@ -21,5 +21,9 @@ module WikiPages def event_action :destroyed end + + def fingerprint(page) + page.wiki.repository.head_commit.sha + end end end diff --git a/app/services/wiki_pages/event_create_service.rb b/app/services/wiki_pages/event_create_service.rb index 0453c90d693..ebfc2414f9e 100644 --- a/app/services/wiki_pages/event_create_service.rb +++ b/app/services/wiki_pages/event_create_service.rb @@ -9,11 +9,11 @@ module WikiPages @author = author end - def execute(slug, page, action) + def execute(slug, page, action, event_fingerprint) event = Event.transaction do wiki_page_meta = WikiPage::Meta.find_or_create(slug, page) - ::EventCreateService.new.wiki_event(wiki_page_meta, author, action) + ::EventCreateService.new.wiki_event(wiki_page_meta, author, action, event_fingerprint) end ServiceResponse.success(payload: { event: event }) |