diff options
Diffstat (limited to 'app/services')
43 files changed, 475 insertions, 139 deletions
diff --git a/app/services/akismet_service.rb b/app/services/akismet_service.rb index aa6f0e841c9..0521393dd27 100644 --- a/app/services/akismet_service.rb +++ b/app/services/akismet_service.rb @@ -1,6 +1,4 @@ class AkismetService - include Gitlab::CurrentSettings - attr_accessor :owner, :text, :options def initialize(owner, text, options = {}) @@ -41,12 +39,12 @@ class AkismetService private def akismet_client - @akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key, + @akismet_client ||= ::Akismet::Client.new(Gitlab::CurrentSettings.akismet_api_key, Gitlab.config.gitlab.url) end def akismet_enabled? - current_application_settings.akismet_enabled + Gitlab::CurrentSettings.akismet_enabled end def submit(type) diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index f40cd2b06c8..2b77f6be72a 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -1,7 +1,5 @@ module Auth class ContainerRegistryAuthenticationService < BaseService - extend Gitlab::CurrentSettings - AUDIENCE = 'container_registry'.freeze def execute(authentication_abilities:) @@ -32,7 +30,7 @@ module Auth end def self.token_expire_at - Time.now + current_application_settings.container_registry_token_expire_delay.minutes + Time.now + Gitlab::CurrentSettings.container_registry_token_expire_delay.minutes end private diff --git a/app/services/base_service.rb b/app/services/base_service.rb index a0cb00dba58..6883ba36c71 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -1,6 +1,5 @@ class BaseService include Gitlab::Allowable - include Gitlab::CurrentSettings attr_accessor :project, :current_user, :params diff --git a/app/services/check_gcp_project_billing_service.rb b/app/services/check_gcp_project_billing_service.rb index 854adf2177d..ea82b61b279 100644 --- a/app/services/check_gcp_project_billing_service.rb +++ b/app/services/check_gcp_project_billing_service.rb @@ -2,7 +2,10 @@ class CheckGcpProjectBillingService def execute(token) client = GoogleApi::CloudPlatform::Client.new(token, nil) client.projects_list.select do |project| - client.projects_get_billing_info(project.name).billingEnabled + begin + client.projects_get_billing_info(project.project_id).billing_enabled + rescue + end end end end diff --git a/app/services/ci/create_trace_artifact_service.rb b/app/services/ci/create_trace_artifact_service.rb new file mode 100644 index 00000000000..280a2c3afa4 --- /dev/null +++ b/app/services/ci/create_trace_artifact_service.rb @@ -0,0 +1,16 @@ +module Ci + class CreateTraceArtifactService < BaseService + def execute(job) + return if job.job_artifacts_trace + + job.trace.read do |stream| + if stream.file? + job.create_job_artifacts_trace!( + project: job.project, + file_type: :trace, + file: stream) + end + end + end + end +end diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb index dc2f49e8db1..87f19b333de 100644 --- a/app/services/ci/ensure_stage_service.rb +++ b/app/services/ci/ensure_stage_service.rb @@ -7,6 +7,8 @@ module Ci # stage. # class EnsureStageService < BaseService + EnsureStageError = Class.new(StandardError) + def execute(build) @build = build @@ -22,8 +24,16 @@ module Ci private - def ensure_stage + def ensure_stage(attempts: 2) find_stage || create_stage + rescue ActiveRecord::RecordNotUnique + retry if (attempts -= 1) > 0 + + raise EnsureStageError, <<~EOS + We failed to find or create a unique pipeline stage after 2 retries. + This should never happen and is most likely the result of a bug in + the database load balancing code. + EOS end def find_stage diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index f832b79ef21..e09b445636f 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -2,8 +2,6 @@ module Ci # This class responsible for assigning # proper pending build to runner on runner API request class RegisterJobService - include Gitlab::CurrentSettings - attr_reader :runner Result = Struct.new(:build, :valid?) diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index c552193e66b..6128b2a8fbb 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -1,7 +1,7 @@ module Ci class RetryBuildService < ::BaseService CLONE_ACCESSORS = %i[pipeline project ref tag options commands name - allow_failure stage_id stage stage_idx trigger_request + allow_failure stage stage_id stage_idx trigger_request yaml_variables when environment coverage_regex description tag_list protected].freeze diff --git a/app/services/clusters/create_service.rb b/app/services/clusters/create_service.rb index 0471b0f17a2..418888e3293 100644 --- a/app/services/clusters/create_service.rb +++ b/app/services/clusters/create_service.rb @@ -5,7 +5,7 @@ module Clusters def execute(access_token = nil) @access_token = access_token - raise ArgumentError.new('Instance does not support multiple clusters') unless can_create_cluster? + raise ArgumentError.new(_('Instance does not support multiple Kubernetes clusters')) unless can_create_cluster? create_cluster.tap do |cluster| ClusterProvisionWorker.perform_async(cluster.id) if cluster.persisted? diff --git a/app/services/clusters/gcp/verify_provision_status_service.rb b/app/services/clusters/gcp/verify_provision_status_service.rb index bc33756f27c..f994aacd086 100644 --- a/app/services/clusters/gcp/verify_provision_status_service.rb +++ b/app/services/clusters/gcp/verify_provision_status_service.rb @@ -28,7 +28,7 @@ module Clusters if elapsed_time_from_creation(operation) < TIMEOUT WaitForClusterCreationWorker.perform_in(EAGER_INTERVAL, provider.cluster_id) else - provider.make_errored!("Cluster creation time exceeds timeout; #{TIMEOUT}") + provider.make_errored!(_('Kubernetes cluster creation time exceeds timeout; %{timeout}') % { timeout: TIMEOUT }) end end diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index 63b85c3de7d..88dfb7a4a90 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -16,6 +16,7 @@ class CreateDeploymentService ActiveRecord::Base.transaction do environment.external_url = expanded_environment_url if expanded_environment_url + environment.fire_state_event(action) return unless environment.save diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 00a8dcf0934..46acdc5406c 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,10 +1,20 @@ module Files class CreateService < Files::BaseService def create_commit! + handler = Lfs::FileModificationHandler.new(project, @branch_name) + + handler.new_file(@file_path, @file_content) do |content_or_lfs_pointer| + create_transformed_commit(content_or_lfs_pointer) + end + end + + private + + def create_transformed_commit(content_or_lfs_pointer) repository.create_file( current_user, @file_path, - @file_content, + content_or_lfs_pointer, message: @commit_message, branch_name: @branch_name, author_email: @author_email, diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index e6fd193ffb3..c037141fcde 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -1,6 +1,5 @@ class GitPushService < BaseService attr_accessor :push_data, :push_commits - include Gitlab::CurrentSettings include Gitlab::Access # The N most recent commits to process in a single push payload. diff --git a/app/services/gravatar_service.rb b/app/services/gravatar_service.rb index e77e08aa380..c6e52c3bb91 100644 --- a/app/services/gravatar_service.rb +++ b/app/services/gravatar_service.rb @@ -1,8 +1,6 @@ class GravatarService - include Gitlab::CurrentSettings - def execute(email, size = nil, scale = 2, username: nil) - return unless current_application_settings.gravatar_enabled? + return unless Gitlab::CurrentSettings.gravatar_enabled? identifier = email.presence || username.presence return unless identifier diff --git a/app/services/groups/destroy_service.rb b/app/services/groups/destroy_service.rb index e3f9d9ee95d..58e88688dfa 100644 --- a/app/services/groups/destroy_service.rb +++ b/app/services/groups/destroy_service.rb @@ -1,7 +1,6 @@ module Groups class DestroyService < Groups::BaseService def async_execute - group.soft_delete_without_removing_associations job_id = GroupDestroyWorker.perform_async(group.id, current_user.id) Rails.logger.info("User #{current_user.id} scheduled a deletion of group ID #{group.id} with job ID #{job_id}") end @@ -23,7 +22,7 @@ module Groups group.chat_team&.remove_mattermost_team(current_user) - group.really_destroy! + group.destroy end end end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb new file mode 100644 index 00000000000..e591c820cff --- /dev/null +++ b/app/services/groups/transfer_service.rb @@ -0,0 +1,96 @@ +module Groups + class TransferService < Groups::BaseService + ERROR_MESSAGES = { + database_not_supported: 'Database is not supported.', + namespace_with_same_path: 'The parent group already has a subgroup with the same path.', + group_is_already_root: 'Group is already a root group.', + same_parent_as_current: 'Group is already associated to the parent group.', + invalid_policies: "You don't have enough permissions." + }.freeze + + TransferError = Class.new(StandardError) + + attr_reader :error + + def initialize(group, user, params = {}) + super + @error = nil + end + + def execute(new_parent_group) + @new_parent_group = new_parent_group + ensure_allowed_transfer + proceed_to_transfer + + rescue TransferError, ActiveRecord::RecordInvalid, Gitlab::UpdatePathError => e + @group.errors.clear + @error = "Transfer failed: " + e.message + false + end + + private + + def proceed_to_transfer + Group.transaction do + update_group_attributes + end + end + + def ensure_allowed_transfer + raise_transfer_error(:group_is_already_root) if group_is_already_root? + raise_transfer_error(:database_not_supported) unless Group.supports_nested_groups? + raise_transfer_error(:same_parent_as_current) if same_parent? + raise_transfer_error(:invalid_policies) unless valid_policies? + raise_transfer_error(:namespace_with_same_path) if namespace_with_same_path? + end + + def group_is_already_root? + !@new_parent_group && !@group.has_parent? + end + + def same_parent? + @new_parent_group && @new_parent_group.id == @group.parent_id + end + + def valid_policies? + return false unless can?(current_user, :admin_group, @group) + + if @new_parent_group + can?(current_user, :create_subgroup, @new_parent_group) + else + can?(current_user, :create_group) + end + end + + def namespace_with_same_path? + Namespace.exists?(path: @group.path, parent: @new_parent_group) + end + + def update_group_attributes + if @new_parent_group && @new_parent_group.visibility_level < @group.visibility_level + update_children_and_projects_visibility + @group.visibility_level = @new_parent_group.visibility_level + end + + @group.parent = @new_parent_group + @group.save! + end + + def update_children_and_projects_visibility + descendants = @group.descendants.where("visibility_level > ?", @new_parent_group.visibility_level) + + Group + .where(id: descendants.select(:id)) + .update_all(visibility_level: @new_parent_group.visibility_level) + + @group + .all_projects + .where("visibility_level > ?", @new_parent_group.visibility_level) + .update_all(visibility_level: @new_parent_group.visibility_level) + end + + def raise_transfer_error(message) + raise TransferError, ERROR_MESSAGES[message] + end + end +end diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 29def25719d..2f511ab44b7 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -24,7 +24,7 @@ module Issues @new_issue = create_new_issue rewrite_notes - rewrite_award_emoji + rewrite_issue_award_emoji add_note_moved_from # Old issue tasks @@ -76,7 +76,7 @@ module Issues end def rewrite_notes - @old_issue.notes.find_each do |note| + @old_issue.notes_with_associations.find_each do |note| new_note = note.dup new_params = { project: @new_project, noteable: @new_issue, note: rewrite_content(new_note.note), @@ -84,13 +84,19 @@ module Issues updated_at: note.updated_at } new_note.update(new_params) + + rewrite_award_emoji(note, new_note) end end - def rewrite_award_emoji - @old_issue.award_emoji.each do |award| + def rewrite_issue_award_emoji + rewrite_award_emoji(@old_issue, @new_issue) + end + + def rewrite_award_emoji(old_awardable, new_awardable) + old_awardable.award_emoji.each do |award| new_award = award.dup - new_award.awardable = @new_issue + new_award.awardable = new_awardable new_award.save end end diff --git a/app/services/labels/promote_service.rb b/app/services/labels/promote_service.rb index 997d247be46..74a85e5c9f0 100644 --- a/app/services/labels/promote_service.rb +++ b/app/services/labels/promote_service.rb @@ -13,6 +13,7 @@ module Labels update_issuables(new_label, batched_ids) update_issue_board_lists(new_label, batched_ids) update_priorities(new_label, batched_ids) + subscribe_users(new_label, batched_ids) # Order is important, project labels need to be last update_project_labels(batched_ids) end @@ -26,6 +27,15 @@ module Labels private + def subscribe_users(new_label, label_ids) + # users can be subscribed to multiple labels that will be merged into the group one + # we want to keep only one subscription / user + ids_to_update = Subscription.where(subscribable_id: label_ids, subscribable_type: 'Label') + .group(:user_id) + .pluck('MAX(id)') + Subscription.where(id: ids_to_update).update_all(subscribable_id: new_label.id) + end + def label_ids_for_merge(new_label) LabelsFinder .new(current_user, title: new_label.title, group_id: project.group.id) @@ -53,7 +63,7 @@ module Labels end def update_project_labels(label_ids) - Label.where(id: label_ids).delete_all + Label.where(id: label_ids).destroy_all end def clone_label_to_group_label(label) diff --git a/app/services/lfs/file_modification_handler.rb b/app/services/lfs/file_modification_handler.rb new file mode 100644 index 00000000000..fe9091a6e5d --- /dev/null +++ b/app/services/lfs/file_modification_handler.rb @@ -0,0 +1,42 @@ +module Lfs + class FileModificationHandler + attr_reader :project, :branch_name + + delegate :repository, to: :project + + def initialize(project, branch_name) + @project = project + @branch_name = branch_name + end + + def new_file(file_path, file_content) + if project.lfs_enabled? && lfs_file?(file_path) + lfs_pointer_file = Gitlab::Git::LfsPointerFile.new(file_content) + lfs_object = create_lfs_object!(lfs_pointer_file, file_content) + content = lfs_pointer_file.pointer + + success = yield(content) + + link_lfs_object!(lfs_object) if success + else + yield(file_content) + end + end + + private + + def lfs_file?(file_path) + repository.attributes_at(branch_name, file_path)['filter'] == 'lfs' + end + + def create_lfs_object!(lfs_pointer_file, file_content) + LfsObject.find_or_create_by(oid: lfs_pointer_file.sha256, size: lfs_pointer_file.size) do |lfs_object| + lfs_object.file = CarrierWaveStringFile.new(file_content) + end + end + + def link_lfs_object!(lfs_object) + project.lfs_objects << lfs_object + end + end +end diff --git a/app/services/lfs/lock_file_service.rb b/app/services/lfs/lock_file_service.rb new file mode 100644 index 00000000000..bbe10f84ef4 --- /dev/null +++ b/app/services/lfs/lock_file_service.rb @@ -0,0 +1,39 @@ +module Lfs + class LockFileService < BaseService + def execute + unless can?(current_user, :push_code, project) + raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions' + end + + create_lock! + rescue ActiveRecord::RecordNotUnique + error('already locked', 409, current_lock) + rescue Gitlab::GitAccess::UnauthorizedError => ex + error(ex.message, 403) + rescue => ex + error(ex.message, 500) + end + + private + + def current_lock + project.lfs_file_locks.find_by(path: params[:path]) + end + + def create_lock! + lock = project.lfs_file_locks.create!(user: current_user, + path: params[:path]) + + success(http_status: 201, lock: lock) + end + + def error(message, http_status, lock = nil) + { + status: :error, + message: message, + http_status: http_status, + lock: lock + } + end + end +end diff --git a/app/services/lfs/locks_finder_service.rb b/app/services/lfs/locks_finder_service.rb new file mode 100644 index 00000000000..13c6cc6f81c --- /dev/null +++ b/app/services/lfs/locks_finder_service.rb @@ -0,0 +1,17 @@ +module Lfs + class LocksFinderService < BaseService + def execute + success(locks: find_locks) + rescue => ex + error(ex.message, 500) + end + + private + + def find_locks + options = params.slice(:id, :path).compact.symbolize_keys + + project.lfs_file_locks.where(options) + end + end +end diff --git a/app/services/lfs/unlock_file_service.rb b/app/services/lfs/unlock_file_service.rb new file mode 100644 index 00000000000..6c93dc69bb0 --- /dev/null +++ b/app/services/lfs/unlock_file_service.rb @@ -0,0 +1,43 @@ +module Lfs + class UnlockFileService < BaseService + def execute + unless can?(current_user, :push_code, project) + raise Gitlab::GitAccess::UnauthorizedError, 'You have no permissions' + end + + unlock_file + rescue Gitlab::GitAccess::UnauthorizedError => ex + error(ex.message, 403) + rescue ActiveRecord::RecordNotFound + error('Lock not found', 404) + rescue => ex + error(ex.message, 500) + end + + private + + def unlock_file + forced = params[:force] == true + + if lock.can_be_unlocked_by?(current_user, forced) + lock.destroy! + + success(lock: lock, http_status: :ok) + elsif forced + error('You must have master access to force delete a lock', 403) + else + error("#{lock.path} is locked by GitLab User #{lock.user_id}", 403) + end + end + + def lock + return @lock if defined?(@lock) + + @lock = if params[:id].present? + project.lfs_file_locks.find(params[:id]) + elsif params[:path].present? + project.lfs_file_locks.find_by!(path: params[:path]) + end + end + end +end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 9622a5c5462..2ae855d078b 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -1,7 +1,9 @@ module MergeRequests class BuildService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize + def execute - @issue_iid = params.delete(:issue_iid) + @params_issue_iid = params.delete(:issue_iid) self.merge_request = MergeRequest.new(params) merge_request.compare_commits = [] @@ -123,7 +125,7 @@ module MergeRequests # def assign_title_and_description assign_title_and_description_from_single_commit - assign_title_from_issue + assign_title_from_issue if target_project.issues_enabled? || target_project.external_issue_tracker merge_request.title ||= source_branch.titleize.humanize merge_request.title = wip_title if compare_commits.empty? @@ -132,9 +134,9 @@ module MergeRequests end def append_closes_description - return unless issue_iid + return unless issue - closes_issue = "Closes ##{issue_iid}" + closes_issue = "Closes #{issue.to_reference}" if description.present? merge_request.description += closes_issue.prepend("\n\n") @@ -156,15 +158,25 @@ module MergeRequests def assign_title_from_issue return unless issue - merge_request.title = - case issue - when Issue then "Resolve \"#{issue.title}\"" - when ExternalIssue then "Resolve #{issue.title}" - end + merge_request.title = "Resolve \"#{issue.title}\"" if issue.is_a?(Issue) + + unless merge_request.title + branch_title = source_branch.downcase.remove(issue_iid.downcase).titleize.humanize + merge_request.title = "Resolve #{issue_iid}" + merge_request.title += " \"#{branch_title}\"" unless branch_title.empty? + end end def issue_iid - @issue_iid ||= source_branch.match(/\A(\d+)-/).try(:[], 1) + strong_memoize(:issue_iid) do + @params_issue_iid || begin + id = if target_project.external_issue_tracker + source_branch.match(target_project.external_issue_reference_pattern).try(:[], 0) + end + + id || source_branch.match(/\A(\d+)-/).try(:[], 1) + end + end end def issue diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb index 89dab1dd028..cf687b71d16 100644 --- a/app/services/merge_requests/create_from_issue_service.rb +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -54,6 +54,7 @@ module MergeRequests source_project_id: project.id, source_branch: branch_name, target_project_id: project.id, + target_branch: ref, milestone_id: issue.milestone_id } end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 49cf534dc0d..a18b1c90765 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -1,22 +1,15 @@ module MergeRequests class CreateService < MergeRequests::BaseService def execute - # @project is used to determine whether the user can set the merge request's - # assignee, milestone and labels. Whether they can depends on their - # permissions on the target project. - source_project = @project - @project = Project.find(params[:target_project_id]) if params[:target_project_id] + set_projects! merge_request = MergeRequest.new merge_request.target_project = @project - merge_request.source_project = source_project + merge_request.source_project = @source_project merge_request.source_branch = params[:source_branch] merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) - # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/37439 - Gitlab::GitalyClient.allow_n_plus_1_calls do - create(merge_request) - end + create(merge_request) end def before_create(merge_request) @@ -58,5 +51,25 @@ module MergeRequests pipelines.order(id: :desc).first end + + def set_projects! + # @project is used to determine whether the user can set the merge request's + # assignee, milestone and labels. Whether they can depends on their + # permissions on the target project. + @source_project = @project + @project = Project.find(params[:target_project_id]) if params[:target_project_id] + + # make sure that source/target project ids are not in + # params so it can't be overridden later when updating attributes + # from params when applying quick actions + params.delete(:source_project_id) + params.delete(:target_project_id) + + unless can?(current_user, :read_project, @source_project) && + can?(current_user, :read_project, @project) + + raise Gitlab::Access::AccessDeniedError + end + end end end diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 0d5a25fa28e..c0083cd6afd 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -1,12 +1,14 @@ module MergeRequests class RebaseService < MergeRequests::WorkingCopyBaseService + REBASE_ERROR = 'Rebase failed. Please rebase locally'.freeze + def execute(merge_request) @merge_request = merge_request if rebase success else - error('Failed to rebase. Should be done manually') + error(REBASE_ERROR) end end @@ -22,8 +24,8 @@ module MergeRequests true rescue => e - log_error('Failed to rebase branch:') - log_error(e.message, save_message_on_model: true) + log_error(REBASE_ERROR, save_message_on_model: true) + log_error(e.message) false end end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 9f05535d4d4..18c40ce8992 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -9,7 +9,8 @@ module MergeRequests 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_merge_requests + close_upon_missing_source_branch_ref + post_merge_manually_merged reload_merge_requests reset_merge_when_pipeline_succeeds mark_pending_todos_done @@ -29,11 +30,22 @@ module MergeRequests private + def close_upon_missing_source_branch_ref + # MergeRequest#reload_diff ignores not opened MRs. This means it won't + # create an `empty` diff for `closed` MRs without a source branch, keeping + # the latest diff state as the last _valid_ one. + merge_requests_for_source_branch.reject(&:source_branch_exists?).each do |mr| + MergeRequests::CloseService + .new(mr.target_project, @current_user) + .execute(mr) + end + end + # Collect open merge requests that target same branch we push into # and close if push to master include last commit from merge request # We need this to close(as merged) merge requests that were merged into # target branch manually - def close_merge_requests + def post_merge_manually_merged commit_ids = @commits.map(&:id) merge_requests = @project.merge_requests.preload(:latest_merge_request_diff).opened.where(target_branch: @branch_name).to_a merge_requests = merge_requests.select(&:diff_head_commit) @@ -78,6 +90,10 @@ module MergeRequests merge_request.mark_as_unchecked UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end + + # Upcoming method calls need the refreshed version of + # @source_merge_requests diffs (for MergeRequest#commit_shas for instance). + merge_requests_for_source_branch(reload: true) end def reset_merge_when_pipeline_succeeds @@ -183,7 +199,8 @@ module MergeRequests merge_requests.uniq.select(&:source_project) end - def merge_requests_for_source_branch + def merge_requests_for_source_branch(reload: false) + @source_merge_requests = nil if reload @source_merge_requests ||= merge_requests_for(@branch_name) end diff --git a/app/services/notification_recipient_service.rb b/app/services/notification_recipient_service.rb index 3eb8cfcca9b..6835b14648b 100644 --- a/app/services/notification_recipient_service.rb +++ b/app/services/notification_recipient_service.rb @@ -11,11 +11,11 @@ module NotificationRecipientService end def self.build_recipients(*a) - Builder::Default.new(*a).recipient_users + Builder::Default.new(*a).notification_recipients end def self.build_new_note_recipients(*a) - Builder::NewNote.new(*a).recipient_users + Builder::NewNote.new(*a).notification_recipients end module Builder @@ -49,25 +49,24 @@ module NotificationRecipientService @recipients ||= [] end - def <<(pair) - users, type = pair - + def add_recipients(users, type, reason) if users.is_a?(ActiveRecord::Relation) users = users.includes(:notification_settings) end users = Array(users) users.compact! - recipients.concat(users.map { |u| make_recipient(u, type) }) + recipients.concat(users.map { |u| make_recipient(u, type, reason) }) end def user_scope User.includes(:notification_settings) end - def make_recipient(user, type) + def make_recipient(user, type, reason) NotificationRecipient.new( user, type, + reason: reason, project: project, custom_action: custom_action, target: target, @@ -75,14 +74,13 @@ module NotificationRecipientService ) end - def recipient_users - @recipient_users ||= + def notification_recipients + @notification_recipients ||= begin build! filter! - users = recipients.map(&:user) - users.uniq! - users.freeze + recipients = self.recipients.sort_by { |r| NotificationReason.priority(r.reason) }.uniq(&:user) + recipients.freeze end end @@ -95,13 +93,13 @@ module NotificationRecipientService def add_participants(user) return unless target.respond_to?(:participants) - self << [target.participants(user), :participating] + add_recipients(target.participants(user), :participating, nil) end def add_mentions(user, target:) return unless target.respond_to?(:mentioned_users) - self << [target.mentioned_users(user), :mention] + add_recipients(target.mentioned_users(user), :mention, NotificationReason::MENTIONED) end # Get project/group users with CUSTOM notification level @@ -119,11 +117,11 @@ module NotificationRecipientService global_users_ids = user_ids_with_project_level_global.concat(user_ids_with_group_level_global) user_ids += user_ids_with_global_level_custom(global_users_ids, custom_action) - self << [user_scope.where(id: user_ids), :watch] + add_recipients(user_scope.where(id: user_ids), :watch, nil) end def add_project_watchers - self << [project_watchers, :watch] + add_recipients(project_watchers, :watch, nil) end # Get project users with WATCH notification level @@ -144,7 +142,7 @@ module NotificationRecipientService def add_subscribed_users return unless target.respond_to? :subscribers - self << [target.subscribers(project), :subscription] + add_recipients(target.subscribers(project), :subscription, nil) end def user_ids_notifiable_on(resource, notification_level = nil) @@ -195,7 +193,7 @@ module NotificationRecipientService return unless target.respond_to? :labels (labels || target.labels).each do |label| - self << [label.subscribers(project), :subscription] + add_recipients(label.subscribers(project), :subscription, nil) end end end @@ -222,12 +220,12 @@ module NotificationRecipientService # Re-assign is considered as a mention of the new assignee case custom_action when :reassign_merge_request - self << [previous_assignee, :mention] - self << [target.assignee, :mention] + add_recipients(previous_assignee, :mention, nil) + add_recipients(target.assignee, :mention, NotificationReason::ASSIGNED) when :reassign_issue previous_assignees = Array(previous_assignee) - self << [previous_assignees, :mention] - self << [target.assignees, :mention] + add_recipients(previous_assignees, :mention, nil) + add_recipients(target.assignees, :mention, NotificationReason::ASSIGNED) end add_subscribed_users @@ -238,6 +236,12 @@ module NotificationRecipientService # receive them, too. add_mentions(current_user, target: target) + # Add the assigned users, if any + assignees = custom_action == :new_issue ? target.assignees : target.assignee + # 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 + add_labels_subscribers end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index be3b4b2ba07..8c84ccfcc92 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -85,10 +85,11 @@ class NotificationService recipients.each do |recipient| mailer.send( :reassigned_issue_email, - recipient.id, + recipient.user.id, issue.id, previous_assignee_ids, - current_user.id + current_user.id, + recipient.reason ).deliver_later end end @@ -176,7 +177,7 @@ class NotificationService action: "resolve_all_discussions") recipients.each do |recipient| - mailer.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id).deliver_later + mailer.resolved_all_discussions_email(recipient.user.id, merge_request.id, current_user.id, recipient.reason).deliver_later end end @@ -199,7 +200,7 @@ class NotificationService recipients = NotificationRecipientService.build_new_note_recipients(note) recipients.each do |recipient| - mailer.send(notify_method, recipient.id, note.id).deliver_later + mailer.send(notify_method, recipient.user.id, note.id).deliver_later end end @@ -299,7 +300,7 @@ class NotificationService recipients = NotificationRecipientService.build_recipients(issue, current_user, action: 'moved') recipients.map do |recipient| - email = mailer.issue_moved_email(recipient, issue, new_issue, current_user) + email = mailer.issue_moved_email(recipient.user, issue, new_issue, current_user, recipient.reason) email.deliver_later email end @@ -339,16 +340,16 @@ class NotificationService recipients = NotificationRecipientService.build_recipients(target, target.author, action: "new") recipients.each do |recipient| - mailer.send(method, recipient.id, target.id).deliver_later + mailer.send(method, recipient.user.id, target.id, recipient.reason).deliver_later end end def new_mentions_in_resource_email(target, new_mentioned_users, current_user, method) recipients = NotificationRecipientService.build_recipients(target, current_user, action: "new") - recipients = recipients & new_mentioned_users + recipients = recipients.select {|r| new_mentioned_users.include?(r.user) } recipients.each do |recipient| - mailer.send(method, recipient.id, target.id, current_user.id).deliver_later + mailer.send(method, recipient.user.id, target.id, current_user.id, recipient.reason).deliver_later end end @@ -363,7 +364,7 @@ class NotificationService ) recipients.each do |recipient| - mailer.send(method, recipient.id, target.id, current_user.id).deliver_later + mailer.send(method, recipient.user.id, target.id, current_user.id, recipient.reason).deliver_later end end @@ -381,10 +382,11 @@ class NotificationService recipients.each do |recipient| mailer.send( method, - recipient.id, + recipient.user.id, target.id, previous_assignee_id, - current_user.id + current_user.id, + recipient.reason ).deliver_later end end @@ -408,7 +410,7 @@ class NotificationService recipients = NotificationRecipientService.build_recipients(target, current_user, action: "reopen") recipients.each do |recipient| - mailer.send(method, recipient.id, target.id, status, current_user.id).deliver_later + mailer.send(method, recipient.user.id, target.id, status, current_user.id, recipient.reason).deliver_later end end diff --git a/app/services/projects/gitlab_projects_import_service.rb b/app/services/projects/gitlab_projects_import_service.rb index 4ca6414b73b..a3d7f5cbed5 100644 --- a/app/services/projects/gitlab_projects_import_service.rb +++ b/app/services/projects/gitlab_projects_import_service.rb @@ -26,7 +26,7 @@ module Projects end def tmp_filename - "#{SecureRandom.hex}_#{params[:path]}" + SecureRandom.hex end def file diff --git a/app/services/projects/hashed_storage/migrate_attachments_service.rb b/app/services/projects/hashed_storage/migrate_attachments_service.rb index f8aaec8a9c0..bc897d891d5 100644 --- a/app/services/projects/hashed_storage/migrate_attachments_service.rb +++ b/app/services/projects/hashed_storage/migrate_attachments_service.rb @@ -14,9 +14,9 @@ module Projects @old_path = project.full_path @new_path = project.disk_path - origin = FileUploader.dynamic_path_segment(project) + origin = FileUploader.absolute_base_dir(project) project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] - target = FileUploader.dynamic_path_segment(project) + target = FileUploader.absolute_base_dir(project) result = move_folder!(origin, target) project.save! diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index dcef8b66215..120d57a188d 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -7,8 +7,6 @@ # module Projects class HousekeepingService < BaseService - include Gitlab::CurrentSettings - # Timeout set to 24h LEASE_TIMEOUT = 86400 @@ -83,19 +81,19 @@ module Projects end def housekeeping_enabled? - current_application_settings.housekeeping_enabled + Gitlab::CurrentSettings.housekeeping_enabled end def gc_period - current_application_settings.housekeeping_gc_period + Gitlab::CurrentSettings.housekeeping_gc_period end def full_repack_period - current_application_settings.housekeeping_full_repack_period + Gitlab::CurrentSettings.housekeeping_full_repack_period end def repack_period - current_application_settings.housekeeping_incremental_repack_period + Gitlab::CurrentSettings.housekeeping_incremental_repack_period end end end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index a773222bf17..c760bd3b626 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -1,7 +1,5 @@ module Projects class UpdatePagesService < BaseService - include Gitlab::CurrentSettings - BLOCK_SIZE = 32.kilobytes MAX_SIZE = 1.terabyte SITE_PATH = 'public/'.freeze @@ -134,7 +132,7 @@ module Projects end def max_size - max_pages_size = current_application_settings.max_pages_size.megabytes + max_pages_size = Gitlab::CurrentSettings.max_pages_size.megabytes return MAX_SIZE if max_pages_size.zero? diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index ff4c73c886e..0e235a6d2a0 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -34,7 +34,7 @@ module Projects def run_auto_devops_pipeline? return false if project.repository.gitlab_ci_yml || !project.auto_devops.previous_changes.include?('enabled') - project.auto_devops.enabled? || (project.auto_devops.enabled.nil? && current_application_settings.auto_devops_enabled?) + project.auto_devops.enabled? || (project.auto_devops.enabled.nil? && Gitlab::CurrentSettings.auto_devops_enabled?) end private diff --git a/app/services/submit_usage_ping_service.rb b/app/services/submit_usage_ping_service.rb index 14171bce782..2623f253d98 100644 --- a/app/services/submit_usage_ping_service.rb +++ b/app/services/submit_usage_ping_service.rb @@ -11,10 +11,8 @@ class SubmitUsagePingService percentage_projects_prometheus_active leader_service_desk_issues instance_service_desk_issues percentage_service_desk_issues].freeze - include Gitlab::CurrentSettings - def execute - return false unless current_application_settings.usage_ping_enabled? + return false unless Gitlab::CurrentSettings.usage_ping_enabled? response = HTTParty.post( URL, diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 690918b4a00..a6b7a6e1416 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -8,7 +8,7 @@ class SystemHooksService end def execute_hooks(data, hooks_scope = :all) - SystemHook.public_send(hooks_scope).find_each do |hook| # rubocop:disable GitlabSecurity/PublicSend + SystemHook.hooks_for(hooks_scope).find_each do |hook| hook.async_execute(data, 'system_hooks') end end @@ -41,8 +41,11 @@ class SystemHooksService when User data.merge!(user_data(model)) - if event == :rename + case event + when :rename data[:old_username] = model.username_was + when :failed_login + data[:state] = model.state end when ProjectMember data.merge!(project_member_data(model)) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 30a5aab13bf..2253d638e93 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -22,8 +22,7 @@ module SystemNoteService commits_text = "#{total_count} commit".pluralize(total_count) body = "added #{commits_text}\n\n" - body << existing_commit_summary(noteable, existing_commits, oldrev) - body << new_commit_summary(new_commits).join("\n") + body << commits_list(noteable, new_commits, existing_commits, oldrev) body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})" create_note(NoteSummary.new(noteable, project, author, body, action: 'commit', commit_count: total_count)) @@ -68,21 +67,14 @@ module SystemNoteService # # Returns the created Note object def change_issue_assignees(issue, project, author, old_assignees) - body = - if issue.assignees.any? && old_assignees.any? - unassigned_users = old_assignees - issue.assignees - added_users = issue.assignees.to_a - old_assignees - - text_parts = [] - text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any? - text_parts << "unassigned #{unassigned_users.map(&:to_reference).to_sentence}" if unassigned_users.any? - - text_parts.join(' and ') - elsif old_assignees.any? - "removed assignee" - elsif issue.assignees.any? - "assigned to #{issue.assignees.map(&:to_reference).to_sentence}" - end + unassigned_users = old_assignees - issue.assignees + added_users = issue.assignees.to_a - old_assignees + + text_parts = [] + text_parts << "assigned to #{added_users.map(&:to_reference).to_sentence}" if added_users.any? + text_parts << "unassigned #{unassigned_users.map(&:to_reference).to_sentence}" if unassigned_users.any? + + body = text_parts.join(' and ') create_note(NoteSummary.new(issue, project, author, body, action: 'assignee')) end @@ -488,7 +480,7 @@ module SystemNoteService # Returns an Array of Strings def new_commit_summary(new_commits) new_commits.collect do |commit| - "* #{commit.short_id} - #{escape_html(commit.title)}" + content_tag('li', "#{commit.short_id} - #{commit.title}") end end @@ -611,6 +603,16 @@ module SystemNoteService "#{cross_reference_note_prefix}#{gfm_reference}" end + # Builds a list of existing and new commits according to existing_commits and + # new_commits methods. + # Returns a String wrapped in `ul` and `li` tags. + def commits_list(noteable, new_commits, existing_commits, oldrev) + existing_commit_summary = existing_commit_summary(noteable, existing_commits, oldrev) + new_commit_summary = new_commit_summary(new_commits).join + + content_tag('ul', "#{existing_commit_summary}#{new_commit_summary}".html_safe) + end + # Build a single line summarizing existing commits being added in a merge # request # @@ -647,11 +649,8 @@ module SystemNoteService branch = noteable.target_branch branch = "#{noteable.target_project_namespace}:#{branch}" if noteable.for_fork? - "* #{commit_ids} - #{commits_text} from branch `#{branch}`\n" - end - - def escape_html(text) - Rack::Utils.escape_html(text) + branch_name = content_tag('code', branch) + content_tag('li', "#{commit_ids} - #{commits_text} from branch #{branch_name}".html_safe) end def url_helpers @@ -668,4 +667,8 @@ module SystemNoteService start_sha: oldrev ) end + + def content_tag(*args) + ActionController::Base.helpers.content_tag(*args) + end end diff --git a/app/services/test_hooks/base_service.rb b/app/services/test_hooks/base_service.rb index 20d90504bd2..e9aefb1fb75 100644 --- a/app/services/test_hooks/base_service.rb +++ b/app/services/test_hooks/base_service.rb @@ -9,7 +9,7 @@ module TestHooks end def execute - trigger_key = hook.class::TRIGGERS.key(trigger.to_sym) + trigger_key = hook.class.triggers.key(trigger.to_sym) trigger_data_method = "#{trigger}_data" if trigger_key.nil? || !self.respond_to?(trigger_data_method, true) diff --git a/app/services/test_hooks/system_service.rb b/app/services/test_hooks/system_service.rb index 67552edefc9..9016c77b7f0 100644 --- a/app/services/test_hooks/system_service.rb +++ b/app/services/test_hooks/system_service.rb @@ -13,5 +13,12 @@ module TestHooks def repository_update_events_data Gitlab::DataBuilder::Repository.sample_data end + + def merge_requests_events_data + merge_request = MergeRequest.of_projects(current_user.projects.select(:id)).first + throw(:validation_error, 'Ensure one of your projects has merge requests.') unless merge_request.present? + + merge_request.to_hook_data(current_user) + end end end diff --git a/app/services/upload_service.rb b/app/services/upload_service.rb index 76700dfcdee..d5a9b344905 100644 --- a/app/services/upload_service.rb +++ b/app/services/upload_service.rb @@ -1,6 +1,4 @@ class UploadService - include Gitlab::CurrentSettings - def initialize(model, file, uploader_class = FileUploader) @model, @file, @uploader_class = model, file, uploader_class end @@ -17,6 +15,6 @@ class UploadService private def max_attachment_size - current_application_settings.max_attachment_size.megabytes.to_i + Gitlab::CurrentSettings.max_attachment_size.megabytes.to_i end end diff --git a/app/services/users/build_service.rb b/app/services/users/build_service.rb index 61f1568f366..4fb6d221909 100644 --- a/app/services/users/build_service.rb +++ b/app/services/users/build_service.rb @@ -1,7 +1,5 @@ module Users class BuildService < BaseService - include Gitlab::CurrentSettings - def initialize(current_user, params = {}) @current_user = current_user @params = params.dup @@ -34,7 +32,7 @@ module Users private def can_create_user? - (current_user.nil? && current_application_settings.allow_signup?) || current_user&.admin? + (current_user.nil? && Gitlab::CurrentSettings.allow_signup?) || current_user&.admin? end # Allowed params for creating a user (admins only) @@ -102,7 +100,7 @@ module Users end def skip_user_confirmation_email_from_setting - !current_application_settings.send_user_confirmation_email + !Gitlab::CurrentSettings.send_user_confirmation_email end end end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 00db8a2c434..b71002433d6 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -53,7 +53,7 @@ module Users # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing user_data = user.destroy - namespace.really_destroy! + namespace.destroy user_data end diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 6ebc7c89500..36e589d5aa8 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -113,7 +113,7 @@ class WebHookService 'Content-Type' => 'application/json', 'X-Gitlab-Event' => hook_name.singularize.titleize }.tap do |hash| - hash['X-Gitlab-Token'] = hook.token if hook.token.present? + hash['X-Gitlab-Token'] = Gitlab::Utils.remove_line_breaks(hook.token) if hook.token.present? end end end |