diff options
Diffstat (limited to 'app/services')
33 files changed, 644 insertions, 363 deletions
diff --git a/app/services/after_branch_delete_service.rb b/app/services/after_branch_delete_service.rb deleted file mode 100644 index ece9fbbef43..00000000000 --- a/app/services/after_branch_delete_service.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -# Branch can be deleted either by DeleteBranchService or by Git::BranchPushService. -class AfterBranchDeleteService < BaseService - attr_reader :branch_name - - def execute(branch_name) - @branch_name = branch_name - - stop_environments - end - - private - - def stop_environments - Ci::StopEnvironmentsService - .new(project, current_user) - .execute(branch_name) - end -end diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb index 9146eb96533..7eeaf8aade1 100644 --- a/app/services/application_settings/update_service.rb +++ b/app/services/application_settings/update_service.rb @@ -2,9 +2,17 @@ module ApplicationSettings class UpdateService < ApplicationSettings::BaseService + include ValidatesClassificationLabel + attr_reader :params, :application_setting def execute + validate_classification_label(application_setting, :external_authorization_service_default_label) + + if application_setting.errors.any? + return false + end + update_terms(@params.delete(:terms)) if params.key?(:performance_bar_allowed_group_path) diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb index 8973c5ffc9e..41dee4e5641 100644 --- a/app/services/ci/create_pipeline_service.rb +++ b/app/services/ci/create_pipeline_service.rb @@ -37,7 +37,7 @@ module Ci variables_attributes: params[:variables_attributes], project: project, current_user: current_user, - push_options: params[:push_options], + push_options: params[:push_options] || {}, chat_data: params[:chat_data], **extra_options(options)) diff --git a/app/services/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index adaa68b1efb..3e7f55f0c63 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -16,6 +16,7 @@ module Clusters error_code: error.respond_to?(:error_code) ? error.error_code : nil, service: self.class.name, app_id: app.id, + app_name: app.name, project_ids: app.cluster.project_ids, group_ids: app.cluster.group_ids } @@ -30,6 +31,19 @@ module Clusters Gitlab::Sentry.track_acceptable_exception(error, extra: meta) end + def log_event(event) + meta = { + service: self.class.name, + app_id: app.id, + app_name: app.name, + project_ids: app.cluster.project_ids, + group_ids: app.cluster.group_ids, + event: event + } + + logger.info(meta) + end + def logger @logger ||= Gitlab::Kubernetes::Logger.build end diff --git a/app/services/clusters/applications/install_service.rb b/app/services/clusters/applications/install_service.rb index 5bd3623a558..1f62b3eb4de 100644 --- a/app/services/clusters/applications/install_service.rb +++ b/app/services/clusters/applications/install_service.rb @@ -7,8 +7,10 @@ module Clusters return unless app.scheduled? app.make_installing! + log_event(:begin_install) helm_api.install(install_command) + log_event(:schedule_wait_for_installation) ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue Kubeclient::HttpError => e diff --git a/app/services/clusters/applications/patch_service.rb b/app/services/clusters/applications/patch_service.rb index 20c739af7a2..c3d317e226b 100644 --- a/app/services/clusters/applications/patch_service.rb +++ b/app/services/clusters/applications/patch_service.rb @@ -8,8 +8,10 @@ module Clusters app.make_updating! + log_event(:begin_patch) helm_api.update(update_command) + log_event(:schedule_wait_for_patch) ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue Kubeclient::HttpError => e diff --git a/app/services/clusters/applications/upgrade_service.rb b/app/services/clusters/applications/upgrade_service.rb index a0ece1d2635..c34391bc8ad 100644 --- a/app/services/clusters/applications/upgrade_service.rb +++ b/app/services/clusters/applications/upgrade_service.rb @@ -9,10 +9,12 @@ module Clusters begin app.make_updating! + log_event(:begin_upgrade) # install_command works with upgrades too # as it basically does `helm upgrade --install` helm_api.update(install_command) + log_event(:schedule_wait_for_upgrade) ClusterWaitForAppInstallationWorker.perform_in( ClusterWaitForAppInstallationWorker::INTERVAL, app.name, app.id) rescue Kubeclient::HttpError => e diff --git a/app/services/concerns/validates_classification_label.rb b/app/services/concerns/validates_classification_label.rb new file mode 100644 index 00000000000..ebcf5c24ff8 --- /dev/null +++ b/app/services/concerns/validates_classification_label.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module ValidatesClassificationLabel + def validate_classification_label(record, attribute_name) + return unless ::Gitlab::ExternalAuthorization.enabled? + return unless classification_label_change?(record, attribute_name) + + new_label = params[attribute_name].presence + new_label ||= ::Gitlab::CurrentSettings.current_application_settings + .external_authorization_service_default_label + + unless ::Gitlab::ExternalAuthorization.access_allowed?(current_user, new_label) + reason = rejection_reason_for_label(new_label) + message = s_('ClassificationLabelUnavailable|is unavailable: %{reason}') % { reason: reason } + record.errors.add(attribute_name, message) + end + end + + def rejection_reason_for_label(label) + reason_from_service = ::Gitlab::ExternalAuthorization.rejection_reason(current_user, label).presence + reason_from_service || _("Access to '%{classification_label}' not allowed") % { classification_label: label } + end + + def classification_label_change?(record, attribute_name) + params.key?(attribute_name) || record.new_record? + end +end diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 8322a3d74f4..4c3ac19f754 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -29,14 +29,4 @@ class DeleteBranchService < BaseService def success(message) super().merge(message: message) end - - def build_push_data(branch) - Gitlab::DataBuilder::Push.build( - project, - current_user, - branch.dereferenced_target.sha, - Gitlab::Git::BLANK_SHA, - "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", - []) - end end diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb new file mode 100644 index 00000000000..fce4040e390 --- /dev/null +++ b/app/services/git/base_hooks_service.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +module Git + class BaseHooksService < ::BaseService + include Gitlab::Utils::StrongMemoize + + # The N most recent commits to process in a single push payload. + PROCESS_COMMIT_LIMIT = 100 + + def execute + project.repository.after_create if project.empty_repo? + + create_events + create_pipelines + execute_project_hooks + + # Not a hook, but it needs access to the list of changed commits + enqueue_invalidate_cache + + push_data + end + + private + + def hook_name + raise NotImplementedError, "Please implement #{self.class}##{__method__}" + end + + def commits + raise NotImplementedError, "Please implement #{self.class}##{__method__}" + end + + def limited_commits + commits.last(PROCESS_COMMIT_LIMIT) + end + + def commits_count + commits.count + end + + def event_message + nil + end + + def invalidated_file_types + [] + end + + def create_events + EventCreateService.new.push(project, current_user, push_data) + end + + def create_pipelines + Ci::CreatePipelineService + .new(project, current_user, push_data) + .execute(:push, pipeline_options) + end + + def execute_project_hooks + project.execute_hooks(push_data, hook_name) + project.execute_services(push_data, hook_name) + end + + def enqueue_invalidate_cache + ProjectCacheWorker.perform_async( + project.id, + invalidated_file_types, + [:commit_count, :repository_size] + ) + end + + def push_data + @push_data ||= Gitlab::DataBuilder::Push.build( + project, + current_user, + params[:oldrev], + params[:newrev], + params[:ref], + limited_commits, + event_message, + commits_count: commits_count, + push_options: params[:push_options] || {} + ) + + # Dependent code may modify the push data, so return a duplicate each time + @push_data.dup + end + + # to be overridden in EE + def pipeline_options + {} + end + end +end diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb new file mode 100644 index 00000000000..d21a6bb1b9a --- /dev/null +++ b/app/services/git/branch_hooks_service.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +module Git + class BranchHooksService < ::Git::BaseHooksService + def execute + execute_branch_hooks + + super.tap do + enqueue_update_gpg_signatures + end + end + + private + + def hook_name + :push_hooks + end + + def commits + strong_memoize(:commits) do + if creating_default_branch? + # The most recent PROCESS_COMMIT_LIMIT commits in the default branch + offset = [count_commits_in_branch - PROCESS_COMMIT_LIMIT, 0].max + project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) + elsif creating_branch? + # Use the pushed commits that aren't reachable by the default branch + # as a heuristic. This may include more commits than are actually + # pushed, but that shouldn't matter because we check for existing + # cross-references later. + project.repository.commits_between(project.default_branch, params[:newrev]) + elsif updating_branch? + project.repository.commits_between(params[:oldrev], params[:newrev]) + else # removing branch + [] + end + end + end + + def commits_count + return count_commits_in_branch if creating_default_branch? + + super + end + + def invalidated_file_types + return super unless default_branch? && !creating_branch? + + paths = limited_commits.each_with_object(Set.new) do |commit, set| + commit.raw_deltas.each do |diff| + set << diff.new_path + end + end + + Gitlab::FileDetector.types_in_paths(paths) + end + + def execute_branch_hooks + project.repository.after_push_commit(branch_name) + + branch_create_hooks if creating_branch? + branch_update_hooks if updating_branch? + branch_change_hooks if creating_branch? || updating_branch? + branch_remove_hooks if removing_branch? + end + + def branch_create_hooks + project.repository.after_create_branch + project.after_create_default_branch if default_branch? + end + + def branch_update_hooks + # Update the bare repositories info/attributes file using the contents of + # the default branch's .gitattributes file + project.repository.copy_gitattributes(params[:ref]) if default_branch? + end + + def branch_change_hooks + enqueue_process_commit_messages + end + + def branch_remove_hooks + project.repository.after_remove_branch + end + + # Schedules processing of commit messages + def enqueue_process_commit_messages + # don't process commits for the initial push to the default branch + return if creating_default_branch? + + limited_commits.each do |commit| + next unless commit.matches_cross_reference_regex? + + ProcessCommitWorker.perform_async( + project.id, + current_user.id, + commit.to_hash, + default_branch? + ) + end + end + + def enqueue_update_gpg_signatures + unsigned = GpgSignature.unsigned_commit_shas(limited_commits.map(&:sha)) + return if unsigned.empty? + + signable = Gitlab::Git::Commit.shas_with_signatures(project.repository, unsigned) + return if signable.empty? + + CreateGpgSignatureWorker.perform_async(signable, project.id) + end + + def creating_branch? + Gitlab::Git.blank_ref?(params[:oldrev]) + end + + def updating_branch? + !creating_branch? && !removing_branch? + end + + def removing_branch? + Gitlab::Git.blank_ref?(params[:newrev]) + end + + def creating_default_branch? + creating_branch? && default_branch? + end + + def count_commits_in_branch + strong_memoize(:count_commits_in_branch) do + project.repository.commit_count_for_ref(params[:ref]) + end + end + + def default_branch? + strong_memoize(:default_branch) do + [nil, branch_name].include?(project.default_branch) + end + end + + def branch_name + strong_memoize(:branch_name) { Gitlab::Git.ref_name(params[:ref]) } + end + end +end diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index b55aeb5f2b9..da053ce80c7 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -1,14 +1,10 @@ # frozen_string_literal: true module Git - class BranchPushService < BaseService - attr_accessor :push_data, :push_commits + class BranchPushService < ::BaseService include Gitlab::Access include Gitlab::Utils::StrongMemoize - # The N most recent commits to process in a single push payload. - PROCESS_COMMIT_LIMIT = 100 - # This method will be called after each git update # and only if the provided user and project are present in GitLab. # @@ -23,108 +19,43 @@ module Git # 6. Checks if the project's main language has changed # def execute - update_commits + return unless Gitlab::Git.branch_ref?(params[:ref]) + + enqueue_update_mrs + enqueue_detect_repository_languages + execute_related_hooks perform_housekeeping update_remote_mirrors - update_caches + stop_environments - update_signatures + true end - def update_commits - project.repository.after_create if project.empty_repo? - project.repository.after_push_commit(branch_name) - - if push_remove_branch? - project.repository.after_remove_branch - @push_commits = [] - elsif push_to_new_branch? - project.repository.after_create_branch - - # Re-find the pushed commits. - if default_branch? - # Initial push to the default branch. Take the full history of that branch as "newly pushed". - process_default_branch - else - # Use the pushed commits that aren't reachable by the default branch - # as a heuristic. This may include more commits than are actually pushed, but - # that shouldn't matter because we check for existing cross-references later. - @push_commits = project.repository.commits_between(project.default_branch, params[:newrev]) - - # don't process commits for the initial push to the default branch - process_commit_messages - end - elsif push_to_existing_branch? - # Collect data for this git push - @push_commits = project.repository.commits_between(params[:oldrev], params[:newrev]) - - process_commit_messages - - # Update the bare repositories info/attributes file using the contents of the default branches - # .gitattributes file - update_gitattributes if default_branch? - end - end - - def update_gitattributes - project.repository.copy_gitattributes(params[:ref]) - end - - def update_caches - if default_branch? - if push_to_new_branch? - # If this is the initial push into the default branch, the file type caches - # will already be reset as a result of `Project#change_head`. - types = [] - else - paths = Set.new - - last_pushed_commits.each do |commit| - commit.raw_deltas.each do |diff| - paths << diff.new_path - end - end - - types = Gitlab::FileDetector.types_in_paths(paths.to_a) - end - - DetectRepositoryLanguagesWorker.perform_async(@project.id, current_user.id) - else - types = [] - end - - ProjectCacheWorker.perform_async(project.id, types, [:commit_count, :repository_size]) + # Update merge requests that may be affected by this push. A new branch + # could cause the last commit of a merge request to change. + def enqueue_update_mrs + UpdateMergeRequestsWorker.perform_async( + project.id, + current_user.id, + params[:oldrev], + params[:newrev], + params[:ref] + ) end - # rubocop: disable CodeReuse/ActiveRecord - def update_signatures - commit_shas = last_pushed_commits.map(&:sha) + def enqueue_detect_repository_languages + return unless default_branch? - return if commit_shas.empty? - - shas_with_cached_signatures = GpgSignature.where(commit_sha: commit_shas).pluck(:commit_sha) - commit_shas -= shas_with_cached_signatures - - return if commit_shas.empty? - - commit_shas = Gitlab::Git::Commit.shas_with_signatures(project.repository, commit_shas) - - CreateGpgSignatureWorker.perform_async(commit_shas, project.id) + DetectRepositoryLanguagesWorker.perform_async(project.id, current_user.id) end - # rubocop: enable CodeReuse/ActiveRecord - # Schedules processing of commit messages. - def process_commit_messages - default = default_branch? + # Only stop environments if the ref is a branch that is being deleted + def stop_environments + return unless removing_branch? - last_pushed_commits.each do |commit| - if commit.matches_cross_reference_regex? - ProcessCommitWorker - .perform_async(project.id, current_user.id, commit.to_hash, default) - end - end + Ci::StopEnvironmentsService.new(project, current_user).execute(branch_name) end def update_remote_mirrors @@ -135,23 +66,7 @@ module Git end def execute_related_hooks - # Update merge requests that may be affected by this push. A new branch - # could cause the last commit of a merge request to change. - # - UpdateMergeRequestsWorker - .perform_async(project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) - - EventCreateService.new.push(project, current_user, build_push_data) - Ci::CreatePipelineService.new(project, current_user, build_push_data).execute(:push, pipeline_options) - - project.execute_hooks(build_push_data.dup, :push_hooks) - project.execute_services(build_push_data.dup, :push_hooks) - - if push_remove_branch? - AfterBranchDeleteService - .new(project, current_user) - .execute(branch_name) - end + BranchHooksService.new(project, current_user, params).execute end def perform_housekeeping @@ -161,84 +76,18 @@ module Git rescue Projects::HousekeepingService::LeaseTaken end - def process_default_branch - offset = [push_commits_count_for_ref - PROCESS_COMMIT_LIMIT, 0].max - @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - - project.after_create_default_branch - end - - def build_push_data - @push_data ||= Gitlab::DataBuilder::Push.build( - project, - current_user, - params[:oldrev], - params[:newrev], - params[:ref], - @push_commits, - commits_count: commits_count, - push_options: params[:push_options] || [] - ) - end - - def push_to_existing_branch? - # Return if this is not a push to a branch (e.g. new commits) - branch_ref? && !Gitlab::Git.blank_ref?(params[:oldrev]) - end - - def push_to_new_branch? - strong_memoize(:push_to_new_branch) do - branch_ref? && Gitlab::Git.blank_ref?(params[:oldrev]) - end - end - - def push_remove_branch? - strong_memoize(:push_remove_branch) do - branch_ref? && Gitlab::Git.blank_ref?(params[:newrev]) - end - end - - def default_branch? - branch_ref? && - (branch_name == project.default_branch || project.default_branch.nil?) - end - - def commit_user(commit) - commit.author || current_user + def removing_branch? + Gitlab::Git.blank_ref?(params[:newrev]) end def branch_name - strong_memoize(:branch_name) do - Gitlab::Git.ref_name(params[:ref]) - end + strong_memoize(:branch_name) { Gitlab::Git.ref_name(params[:ref]) } end - def branch_ref? - strong_memoize(:branch_ref) do - Gitlab::Git.branch_ref?(params[:ref]) - end - end - - def commits_count - return push_commits_count_for_ref if default_branch? && push_to_new_branch? - - Array(@push_commits).size - end - - def push_commits_count_for_ref - strong_memoize(:push_commits_count_for_ref) do - project.repository.commit_count_for_ref(params[:ref]) + def default_branch? + strong_memoize(:default_branch) do + [nil, branch_name].include?(project.default_branch) end end - - def last_pushed_commits - @last_pushed_commits ||= @push_commits.last(PROCESS_COMMIT_LIMIT) - end - - private - - def pipeline_options - {} # to be overridden in EE - end end end diff --git a/app/services/git/tag_hooks_service.rb b/app/services/git/tag_hooks_service.rb new file mode 100644 index 00000000000..18eb780579f --- /dev/null +++ b/app/services/git/tag_hooks_service.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Git + class TagHooksService < ::Git::BaseHooksService + private + + def hook_name + :tag_push_hooks + end + + def commits + [tag_commit].compact + end + + def event_message + tag&.message + end + + def tag + strong_memoize(:tag) do + next if Gitlab::Git.blank_ref?(params[:newrev]) + + tag_name = Gitlab::Git.ref_name(params[:ref]) + tag = project.repository.find_tag(tag_name) + + tag if tag && tag.target == params[:newrev] + end + end + + def tag_commit + strong_memoize(:tag_commit) do + project.commit(tag.dereferenced_target) if tag + end + end + end +end diff --git a/app/services/git/tag_push_service.rb b/app/services/git/tag_push_service.rb index 9ce0fbdb206..ee4166dccd0 100644 --- a/app/services/git/tag_push_service.rb +++ b/app/services/git/tag_push_service.rb @@ -1,56 +1,14 @@ # frozen_string_literal: true module Git - class TagPushService < BaseService - attr_accessor :push_data - + class TagPushService < ::BaseService def execute - project.repository.after_create if project.empty_repo? - project.repository.before_push_tag - - @push_data = build_push_data - - EventCreateService.new.push(project, current_user, push_data) - Ci::CreatePipelineService.new(project, current_user, push_data).execute(:push, pipeline_options) + return unless Gitlab::Git.tag_ref?(params[:ref]) - project.execute_hooks(push_data.dup, :tag_push_hooks) - project.execute_services(push_data.dup, :tag_push_hooks) - - ProjectCacheWorker.perform_async(project.id, [], [:commit_count, :repository_size]) + project.repository.before_push_tag + TagHooksService.new(project, current_user, params).execute true end - - private - - def build_push_data - commits = [] - message = nil - - unless Gitlab::Git.blank_ref?(params[:newrev]) - tag_name = Gitlab::Git.ref_name(params[:ref]) - tag = project.repository.find_tag(tag_name) - - if tag && tag.target == params[:newrev] - commit = project.commit(tag.dereferenced_target) - commits = [commit].compact - message = tag.message - end - end - - Gitlab::DataBuilder::Push.build( - project, - current_user, - params[:oldrev], - params[:newrev], - params[:ref], - commits, - message, - push_options: params[:push_options] || []) - end - - def pipeline_options - {} # to be overridden in EE - end end end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 7a4ccf0d178..26132f1824a 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -34,14 +34,20 @@ class IssuableBaseService < BaseService end def filter_assignee(issuable) - return unless params[:assignee_id].present? + return if params[:assignee_ids].blank? - assignee_id = params[:assignee_id] + unless issuable.allows_multiple_assignees? + params[:assignee_ids] = params[:assignee_ids].first(1) + end + + assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) } - if assignee_id.to_s == IssuableFinder::NONE - params[:assignee_id] = "" + if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE] + params[:assignee_ids] = [] + elsif assignee_ids.any? + params[:assignee_ids] = assignee_ids else - params.delete(:assignee_id) unless assignee_can_read?(issuable, assignee_id) + params.delete(:assignee_ids) end end @@ -352,7 +358,7 @@ class IssuableBaseService < BaseService end def has_changes?(issuable, old_labels: [], old_assignees: []) - valid_attrs = [:title, :description, :assignee_id, :milestone_id, :target_branch] + valid_attrs = [:title, :description, :assignee_ids, :milestone_id, :target_branch] attrs_changed = valid_attrs.any? do |attr| issuable.previous_changes.include?(attr.to_s) diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index ef08adf4f92..48ed5afbc2a 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -20,7 +20,7 @@ module Issues private def create_assignee_note(issue, old_assignees) - SystemNoteService.change_issue_assignees( + SystemNoteService.change_issuable_assignees( issue, issue.project, current_user, old_assignees) end @@ -31,26 +31,6 @@ module Issues issue.project.execute_services(issue_data, hooks_scope) end - # rubocop: disable CodeReuse/ActiveRecord - def filter_assignee(issuable) - return if params[:assignee_ids].blank? - - unless issuable.allows_multiple_assignees? - params[:assignee_ids] = params[:assignee_ids].take(1) - end - - assignee_ids = params[:assignee_ids].select { |assignee_id| assignee_can_read?(issuable, assignee_id) } - - if params[:assignee_ids].map(&:to_s) == [IssuableFinder::NONE] - params[:assignee_ids] = [] - elsif assignee_ids.any? - params[:assignee_ids] = assignee_ids - else - params.delete(:assignee_ids) - end - end - # rubocop: enable CodeReuse/ActiveRecord - def update_project_counter_caches?(issue) super || issue.confidential_changed? end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index cec5b5734c0..cb2337d29d4 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -39,7 +39,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_issue(issue, current_user, old_assignees) + todo_service.reassigned_issuable(issue, current_user, old_assignees) end if issue.previous_changes.include?('confidential') diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 8a9e5ebb014..b8334a87f6d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -49,9 +49,9 @@ module MergeRequests MergeRequestMetricsService.new(merge_request.metrics) end - def create_assignee_note(merge_request) - SystemNoteService.change_assignee( - merge_request, merge_request.project, current_user, merge_request.assignee) + def create_assignee_note(merge_request, old_assignees) + SystemNoteService.change_issuable_assignees( + merge_request, merge_request.project, current_user, old_assignees) end def create_pipeline_for(merge_request, user) diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb new file mode 100644 index 00000000000..a24163331e8 --- /dev/null +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +module MergeRequests + class PushOptionsHandlerService + LIMIT = 10 + + attr_reader :branches, :changes_by_branch, :current_user, :errors, + :project, :push_options, :target_project + + def initialize(project, current_user, changes, push_options) + @project = project + @target_project = @project.default_merge_request_target + @current_user = current_user + @branches = get_branches(changes) + @push_options = push_options + @errors = [] + end + + def execute + validate_service + return self if errors.present? + + branches.each do |branch| + execute_for_branch(branch) + rescue Gitlab::Access::AccessDeniedError + errors << 'User access was denied' + rescue StandardError => e + Gitlab::AppLogger.error(e) + errors << 'An unknown error occurred' + end + + self + end + + private + + def get_branches(raw_changes) + Gitlab::ChangesList.new(raw_changes).map do |changes| + next unless Gitlab::Git.branch_ref?(changes[:ref]) + + # Deleted branch + next if Gitlab::Git.blank_ref?(changes[:newrev]) + + # Default branch + branch_name = Gitlab::Git.branch_name(changes[:ref]) + next if branch_name == target_project.default_branch + + branch_name + end.compact.uniq + end + + def validate_service + errors << 'User is required' if current_user.nil? + + unless target_project.merge_requests_enabled? + errors << "Merge requests are not enabled for project #{target_project.full_path}" + end + + if branches.size > LIMIT + errors << "Too many branches pushed (#{branches.size} were pushed, limit is #{LIMIT})" + end + + if push_options[:target] && !target_project.repository.branch_exists?(push_options[:target]) + errors << "Branch #{push_options[:target]} does not exist" + end + end + + # Returns a Hash of branch => MergeRequest + def merge_requests + @merge_requests ||= MergeRequest.from_project(target_project) + .opened + .from_source_branches(branches) + .index_by(&:source_branch) + end + + def execute_for_branch(branch) + merge_request = merge_requests[branch] + + if merge_request + update!(merge_request) + else + create!(branch) + end + end + + def create!(branch) + unless push_options[:create] + errors << "A merge_request.create push option is required to create a merge request for branch #{branch}" + return + end + + # Use BuildService to assign the standard attributes of a merge request + merge_request = ::MergeRequests::BuildService.new( + project, + current_user, + create_params(branch) + ).execute + + unless merge_request.errors.present? + merge_request = ::MergeRequests::CreateService.new( + project, + current_user, + merge_request.attributes.merge(assignees: merge_request.assignees) + ).execute + end + + collect_errors_from_merge_request(merge_request) unless merge_request.persisted? + end + + def update!(merge_request) + merge_request = ::MergeRequests::UpdateService.new( + target_project, + current_user, + update_params + ).execute(merge_request) + + collect_errors_from_merge_request(merge_request) unless merge_request.valid? + end + + def create_params(branch) + params = { + assignees: [current_user], + source_branch: branch, + source_project: project, + target_branch: push_options[:target] || target_project.default_branch, + target_project: target_project + } + + if push_options.key?(:merge_when_pipeline_succeeds) + params.merge!( + merge_when_pipeline_succeeds: push_options[:merge_when_pipeline_succeeds], + merge_user: current_user + ) + end + + params + end + + def update_params + params = {} + + if push_options.key?(:merge_when_pipeline_succeeds) + params.merge!( + merge_when_pipeline_succeeds: push_options[:merge_when_pipeline_succeeds], + merge_user: current_user + ) + end + + if push_options.key?(:target) + params[:target_branch] = push_options[:target] + end + + params + end + + def collect_errors_from_merge_request(merge_request) + merge_request.errors.full_messages.each do |error| + errors << error + end + end + end +end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 51d27673787..3abea1ad1ae 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -14,13 +14,16 @@ module MergeRequests private def refresh_merge_requests! + # n + 1: https://gitlab.com/gitlab-org/gitlab-ce/issues/60289 Gitlab::GitalyClient.allow_n_plus_1_calls(&method(:find_new_commits)) + # Be sure to close outstanding MRs before reloading them to avoid generating an # empty diff during a manual merge close_upon_missing_source_branch_ref post_merge_manually_merged reload_merge_requests outdate_suggestions + refresh_pipelines_on_merge_requests reset_merge_when_pipeline_succeeds mark_pending_todos_done cache_merge_requests_closing_issues @@ -107,8 +110,6 @@ module MergeRequests end merge_request.mark_as_unchecked - create_pipeline_for(merge_request, current_user) - UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end # Upcoming method calls need the refreshed version of @@ -134,6 +135,13 @@ module MergeRequests end end + def refresh_pipelines_on_merge_requests + merge_requests_for_source_branch.each do |merge_request| + create_pipeline_for(merge_request, current_user) + UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) + end + end + def reset_merge_when_pipeline_succeeds merge_requests_for_source_branch.each(&:reset_merge_when_pipeline_succeeds) end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 8112c2a4299..faaa4d66726 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -24,13 +24,13 @@ module MergeRequests update_task_event(merge_request) || update(merge_request) end - # rubocop:disable Metrics/AbcSize def handle_changes(merge_request, options) old_associations = options.fetch(:old_associations, {}) old_labels = old_associations.fetch(:labels, []) old_mentioned_users = old_associations.fetch(:mentioned_users, []) + old_assignees = old_associations.fetch(:assignees, []) - if has_changes?(merge_request, old_labels: old_labels) + if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees) todo_service.mark_pending_todos_as_done(merge_request, current_user) end @@ -45,15 +45,10 @@ module MergeRequests merge_request.target_branch) end - if merge_request.previous_changes.include?('assignee_id') - reassigned_merge_request_args = [merge_request, current_user] - - old_assignee_id = merge_request.previous_changes['assignee_id'].first - reassigned_merge_request_args << User.find(old_assignee_id) if old_assignee_id - - create_assignee_note(merge_request) - notification_service.async.reassigned_merge_request(*reassigned_merge_request_args) - todo_service.reassigned_merge_request(merge_request, current_user) + if merge_request.assignees != 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) end if merge_request.previous_changes.include?('target_branch') || @@ -81,7 +76,6 @@ module MergeRequests ) end end - # rubocop:enable Metrics/AbcSize def handle_task_changes(merge_request) todo_service.mark_pending_todos_as_done(merge_request, current_user) diff --git a/app/services/note_summary.rb b/app/services/note_summary.rb index 81f6f92f75c..60a68568833 100644 --- a/app/services/note_summary.rb +++ b/app/services/note_summary.rb @@ -5,7 +5,9 @@ class NoteSummary attr_reader :metadata def initialize(noteable, project, author, body, action: nil, commit_count: nil) - @note = { noteable: noteable, project: project, author: author, note: body } + @note = { noteable: noteable, + created_at: noteable.system_note_timestamp, + project: project, author: author, note: body } @metadata = { action: action, commit_count: commit_count }.compact set_commit_params if note[:noteable].is_a?(Commit) diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 56f11b31110..760962346fb 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -247,15 +247,15 @@ module NotificationRecipientService attr_reader :target attr_reader :current_user attr_reader :action - attr_reader :previous_assignee + attr_reader :previous_assignees attr_reader :skip_current_user - def initialize(target, current_user, action:, custom_action: nil, previous_assignee: nil, skip_current_user: true) + def initialize(target, current_user, action:, custom_action: nil, previous_assignees: nil, skip_current_user: true) @target = target @current_user = current_user @action = action @custom_action = custom_action - @previous_assignee = previous_assignee + @previous_assignees = previous_assignees @skip_current_user = skip_current_user end @@ -270,11 +270,7 @@ module NotificationRecipientService # Re-assign is considered as a mention of the new assignee case custom_action - when :reassign_merge_request - add_recipients(previous_assignee, :mention, nil) - add_recipients(target.assignee, :mention, NotificationReason::ASSIGNED) - when :reassign_issue - previous_assignees = Array(previous_assignee) + when :reassign_merge_request, :reassign_issue add_recipients(previous_assignees, :mention, nil) add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED) end @@ -287,17 +283,11 @@ module NotificationRecipientService # receive them, too. add_mentions(current_user, target: target) - # Add the assigned users, if any - assignees = case custom_action - when :new_issue - target.assignees - else - target.assignee - end - # We use the `:participating` notification level in order to match existing legacy behavior as captured # in existing specs (notification_service_spec.rb ~ line 507) - add_recipients(assignees, :participating, NotificationReason::ASSIGNED) if assignees + if target.is_a?(Issuable) + add_recipients(target.assignees, :participating, NotificationReason::ASSIGNED) + end add_labels_subscribers end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 1a65561dd70..8d3b569498f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -95,8 +95,8 @@ class NotificationService # When we reassign an issue we should send an email to: # - # * issue old assignee if their notification level is not Disabled - # * issue new assignee if their notification level is not Disabled + # * issue old assignees if their notification level is not Disabled + # * issue new assignees if their notification level is not Disabled # * users with custom level checked with "reassign issue" # def reassigned_issue(issue, current_user, previous_assignees = []) @@ -104,7 +104,7 @@ class NotificationService issue, current_user, action: "reassign", - previous_assignee: previous_assignees + previous_assignees: previous_assignees ) previous_assignee_ids = previous_assignees.map(&:id) @@ -140,7 +140,7 @@ class NotificationService # When create a merge request we should send an email to: # # * mr author - # * mr assignee if their notification level is not Disabled + # * mr assignees if their notification level is not Disabled # * project team members with notification level higher then Participating # * watchers of the mr's labels # * users with custom level checked with "new merge request" @@ -184,23 +184,25 @@ class NotificationService # When we reassign a merge_request we should send an email to: # - # * merge_request old assignee if their notification level is not Disabled - # * merge_request assignee if their notification level is not Disabled + # * merge_request old assignees if their notification level is not Disabled + # * merge_request new assignees if their notification level is not Disabled # * users with custom level checked with "reassign merge request" # - def reassigned_merge_request(merge_request, current_user, previous_assignee = nil) + def reassigned_merge_request(merge_request, current_user, previous_assignees = []) recipients = NotificationRecipientService.build_recipients( merge_request, current_user, action: "reassign", - previous_assignee: previous_assignee + previous_assignees: previous_assignees ) + previous_assignee_ids = previous_assignees.map(&:id) + recipients.each do |recipient| mailer.reassigned_merge_request_email( recipient.user.id, merge_request.id, - previous_assignee&.id, + previous_assignee_ids, current_user.id, recipient.reason ).deliver_later diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index d03137b63b2..3723c5ef7d7 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -2,6 +2,8 @@ module Projects class CreateService < BaseService + include ValidatesClassificationLabel + def initialize(user, params) @current_user, @params = user, params.dup @skip_wiki = @params.delete(:skip_wiki) @@ -45,6 +47,8 @@ module Projects relations_block&.call(@project) yield(@project) if block_given? + validate_classification_label(@project, :external_authorization_classification_label) + # If the block added errors, don't try to save the project return @project if @project.errors.any? diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 6856009b395..bc36bb8659d 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -3,6 +3,7 @@ module Projects class UpdateService < BaseService include UpdateVisibilityLevel + include ValidatesClassificationLabel ValidationError = Class.new(StandardError) @@ -14,6 +15,8 @@ module Projects yield if block_given? + validate_classification_label(project, :external_authorization_classification_label) + # If the block added errors, don't try to save the project return update_failed! if project.errors.any? diff --git a/app/services/projects/update_statistics_service.rb b/app/services/projects/update_statistics_service.rb new file mode 100644 index 00000000000..f32a779fab0 --- /dev/null +++ b/app/services/projects/update_statistics_service.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Projects + class UpdateStatisticsService < BaseService + def execute + return unless project && project.repository.exists? + + Rails.logger.info("Updating statistics for project #{project.id}") + + project.statistics.refresh!(only: statistics.map(&:to_sym)) + end + + private + + def statistics + params[:statistics] + end + end +end diff --git a/app/services/releases/concerns.rb b/app/services/releases/concerns.rb index a04bb8f9e14..ff6b696ca96 100644 --- a/app/services/releases/concerns.rb +++ b/app/services/releases/concerns.rb @@ -15,7 +15,7 @@ module Releases end def name - params[:name] + params[:name] || tag_name end def description diff --git a/app/services/releases/create_service.rb b/app/services/releases/create_service.rb index c6e143d440d..a271a7e5e49 100644 --- a/app/services/releases/create_service.rb +++ b/app/services/releases/create_service.rb @@ -15,6 +15,10 @@ module Releases create_release(tag) end + def find_or_build_release + release || build_release(existing_tag) + end + private def ensure_tag @@ -38,7 +42,17 @@ module Releases end def create_release(tag) - release = project.releases.create!( + release = build_release(tag) + + release.save! + + success(tag: tag, release: release) + rescue => e + error(e.message, 400) + end + + def build_release(tag) + project.releases.build( name: name, description: description, author: current_user, @@ -46,10 +60,6 @@ module Releases sha: tag.dereferenced_target.sha, links_attributes: params.dig(:assets, 'links') || [] ) - - success(tag: tag, release: release) - rescue => e - error(e.message, 400) end end end diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb index 039d6e2ebad..b45e567079b 100644 --- a/app/services/resource_events/change_labels_service.rb +++ b/app/services/resource_events/change_labels_service.rb @@ -12,7 +12,7 @@ module ResourceEvents label_hash = { resource_column(resource) => resource.id, user_id: user.id, - created_at: Time.now + created_at: resource.system_note_timestamp } labels = added_labels.map do |label| diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index ea8ac7e4656..a39ff76b798 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -69,7 +69,7 @@ module SystemNoteService # Called when the assignees of an Issue is changed or removed # - # issue - Issue object + # issuable - Issuable object (responds to assignees) # project - Project owning noteable # author - User performing the change # assignees - Users being assigned, or nil @@ -85,9 +85,9 @@ module SystemNoteService # "assigned to @user1 and @user2" # # Returns the created Note object - def change_issue_assignees(issue, project, author, old_assignees) - unassigned_users = old_assignees - issue.assignees - added_users = issue.assignees.to_a - old_assignees + def change_issuable_assignees(issuable, project, author, old_assignees) + unassigned_users = old_assignees - issuable.assignees + added_users = issuable.assignees.to_a - old_assignees text_parts = [] text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any? @@ -95,7 +95,7 @@ module SystemNoteService body = text_parts.join(' and ') - create_note(NoteSummary.new(issue, project, author, body, action: 'assignee')) + create_note(NoteSummary.new(issuable, project, author, body, action: 'assignee')) end # Called when the milestone of a Noteable is changed @@ -258,7 +258,7 @@ module SystemNoteService body = "created #{issue.to_reference} to continue this discussion" note_attributes = discussion.reply_attributes.merge(project: project, author: author, note: body) - note = Note.create(note_attributes.merge(system: true)) + note = Note.create(note_attributes.merge(system: true, created_at: issue.system_note_timestamp)) note.system_note_metadata = SystemNoteMetadata.new(action: 'discussion') note diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index f357dc37fe7..0ea230a44a1 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -49,12 +49,12 @@ class TodoService todo_users.each(&:update_todos_count_cache) end - # When we reassign an issue we should: + # When we reassign an issuable we should: # - # * create a pending todo for new assignee if issue is assigned + # * create a pending todo for new assignee if issuable is assigned # - def reassigned_issue(issue, current_user, old_assignees = []) - create_assignment_todo(issue, current_user, old_assignees) + def reassigned_issuable(issuable, current_user, old_assignees = []) + create_assignment_todo(issuable, current_user, old_assignees) end # When create a merge request we should: @@ -82,14 +82,6 @@ class TodoService mark_pending_todos_as_done(merge_request, current_user) end - # When we reassign a merge request we should: - # - # * creates a pending todo for new assignee if merge request is assigned - # - def reassigned_merge_request(merge_request, current_user) - create_assignment_todo(merge_request, current_user) - end - # When merge a merge request we should: # # * mark all pending todos related to the target for the current user as done diff --git a/app/services/verify_pages_domain_service.rb b/app/services/verify_pages_domain_service.rb index 07f7391f877..b53c3145caf 100644 --- a/app/services/verify_pages_domain_service.rb +++ b/app/services/verify_pages_domain_service.rb @@ -8,6 +8,7 @@ class VerifyPagesDomainService < BaseService # How long verification lasts for VERIFICATION_PERIOD = 7.days + REMOVAL_DELAY = 1.week.freeze attr_reader :domain @@ -36,7 +37,7 @@ class VerifyPagesDomainService < BaseService # Prevent any pre-existing grace period from being truncated reverify = [domain.enabled_until, VERIFICATION_PERIOD.from_now].compact.max - domain.assign_attributes(verified_at: Time.now, enabled_until: reverify) + domain.assign_attributes(verified_at: Time.now, enabled_until: reverify, remove_at: nil) domain.save!(validate: false) if was_disabled @@ -49,18 +50,20 @@ class VerifyPagesDomainService < BaseService end def unverify_domain! - if domain.verified? - domain.assign_attributes(verified_at: nil) - domain.save!(validate: false) + was_verified = domain.verified? - notify(:verification_failed) - end + domain.assign_attributes(verified_at: nil) + domain.remove_at ||= REMOVAL_DELAY.from_now unless domain.enabled? + domain.save!(validate: false) + + notify(:verification_failed) if was_verified error("Couldn't verify #{domain.domain}") end def disable_domain! domain.assign_attributes(verified_at: nil, enabled_until: nil) + domain.remove_at ||= REMOVAL_DELAY.from_now domain.save!(validate: false) notify(:disabled) |