diff options
Diffstat (limited to 'app/services')
48 files changed, 360 insertions, 176 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 new file mode 100644 index 00000000000..ea82b61b279 --- /dev/null +++ b/app/services/check_gcp_project_billing_service.rb @@ -0,0 +1,11 @@ +class CheckGcpProjectBillingService + def execute(token) + client = GoogleApi::CloudPlatform::Client.new(token, nil) + client.projects_list.select do |project| + begin + client.projects_get_billing_info(project.project_id).billing_enabled + rescue + end + end + end +end 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/clusters/applications/base_helm_service.rb b/app/services/clusters/applications/base_helm_service.rb index 9a4ce31cb39..cba1b920f7c 100644 --- a/app/services/clusters/applications/base_helm_service.rb +++ b/app/services/clusters/applications/base_helm_service.rb @@ -18,7 +18,7 @@ module Clusters end def helm_api - @helm_api ||= Gitlab::Kubernetes::Helm.new(kubeclient) + @helm_api ||= Gitlab::Kubernetes::Helm::Api.new(kubeclient) end def install_command 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/event_create_service.rb b/app/services/event_create_service.rb index 6328d567a07..44dc90b3462 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -103,6 +103,6 @@ class EventCreateService author_id: current_user.id ) - Event.create(attributes) + Event.create!(attributes) end end diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index 98a3e83c130..a03c59f569d 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -4,7 +4,7 @@ module Files def create_commit! repository.multi_action( - user: current_user, + current_user, message: @commit_message, branch_name: @branch_name, actions: params[:actions], @@ -13,6 +13,8 @@ module Files start_project: @start_project, start_branch_name: @start_branch ) + rescue ArgumentError => e + raise_error(e) end private @@ -20,16 +22,7 @@ module Files def validate! super - params[:actions].each do |action| - validate_action!(action) - validate_file_status!(action) - end - end - - def validate_action!(action) - unless Gitlab::Git::Index::ACTIONS.include?(action[:action].to_s) - raise_error("Unknown action '#{action[:action]}'") - end + params[:actions].each { |action| validate_file_status!(action) } end def validate_file_status!(action) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index bb61136e33b..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. @@ -154,24 +153,7 @@ class GitPushService < BaseService offset = [@push_commits_count - PROCESS_COMMIT_LIMIT, 0].max @push_commits = project.repository.commits(params[:newrev], offset: offset, limit: PROCESS_COMMIT_LIMIT) - # Ensure HEAD points to the default branch in case it is not master - project.change_head(branch_name) - - # Set protection on the default branch if configured - if current_application_settings.default_branch_protection != PROTECTION_NONE && !ProtectedBranch.protected?(@project, @project.default_branch) - - params = { - name: @project.default_branch, - push_access_levels_attributes: [{ - access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_PUSH ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }], - merge_access_levels_attributes: [{ - access_level: current_application_settings.default_branch_protection == PROTECTION_DEV_CAN_MERGE ? Gitlab::Access::DEVELOPER : Gitlab::Access::MASTER - }] - } - - ProtectedBranches::CreateService.new(@project, current_user, params).execute - end + @project.after_create_default_branch end def build_push_data 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/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/merge_request_metrics_service.rb b/app/services/merge_request_metrics_service.rb new file mode 100644 index 00000000000..9248de14a53 --- /dev/null +++ b/app/services/merge_request_metrics_service.rb @@ -0,0 +1,19 @@ +class MergeRequestMetricsService + delegate :update!, to: :@merge_request_metrics + + def initialize(merge_request_metrics) + @merge_request_metrics = merge_request_metrics + end + + def merge(event) + update!(merged_by_id: event.author_id, merged_at: event.created_at) + end + + def close(event) + update!(latest_closed_by_id: event.author_id, latest_closed_at: event.created_at) + end + + def reopen + update!(latest_closed_by_id: nil, latest_closed_at: nil) + end +end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 6b32d65a74b..20a2b50d3de 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -24,6 +24,10 @@ module MergeRequests private + def merge_request_metrics_service(merge_request) + 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) 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/close_service.rb b/app/services/merge_requests/close_service.rb index 40213c99014..f727ec002e7 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -8,7 +8,7 @@ module MergeRequests merge_request.allow_broken = true if merge_request.close - event_service.close_mr(merge_request, current_user) + create_event(merge_request) create_note(merge_request) notification_service.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) @@ -19,5 +19,16 @@ module MergeRequests merge_request end + + private + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + close_event = event_service.close_mr(merge_request, current_user) + merge_request_metrics_service(merge_request).close(close_event) + end + end end end diff --git a/app/services/merge_requests/conflicts/list_service.rb b/app/services/merge_requests/conflicts/list_service.rb index 0f677a996f7..ca9a33678e4 100644 --- a/app/services/merge_requests/conflicts/list_service.rb +++ b/app/services/merge_requests/conflicts/list_service.rb @@ -23,7 +23,7 @@ module MergeRequests # when there are no conflict files. conflicts.files.each(&:lines) @conflicts_can_be_resolved_in_ui = conflicts.files.length > 0 - rescue Rugged::OdbError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing + rescue Gitlab::Git::CommandError, Gitlab::Git::Conflict::Parser::UnresolvableError, Gitlab::Git::Conflict::Resolver::ConflictSideMissing @conflicts_can_be_resolved_in_ui = false end end 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..634bf3bd690 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -1,15 +1,11 @@ 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) @@ -58,5 +54,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/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index b1d6bac4d4a..c78e78afcd1 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -9,7 +9,7 @@ module MergeRequests close_issues(merge_request) todo_service.merge_merge_request(merge_request, current_user) merge_request.mark_as_merged - create_merge_event(merge_request, current_user) + create_event(merge_request) create_note(merge_request) notification_service.merge_mr(merge_request, current_user) execute_hooks(merge_request, 'merge') @@ -34,5 +34,14 @@ module MergeRequests def create_merge_event(merge_request, current_user) EventCreateService.new.merge_mr(merge_request, current_user) end + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + merge_event = create_merge_event(merge_request, current_user) + merge_request_metrics_service(merge_request).merge(merge_event) + end + end end end diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb new file mode 100644 index 00000000000..c0083cd6afd --- /dev/null +++ b/app/services/merge_requests/rebase_service.rb @@ -0,0 +1,32 @@ +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(REBASE_ERROR) + end + end + + def rebase + if merge_request.rebase_in_progress? + log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true) + return false + end + + rebase_sha = repository.rebase(current_user, merge_request) + + merge_request.update_attributes(rebase_commit_sha: rebase_sha) + + true + rescue => e + log_error(REBASE_ERROR, save_message_on_model: true) + log_error(e.message) + false + end + 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/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index c599a90f9fe..120677a7149 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -4,7 +4,7 @@ module MergeRequests return merge_request unless can?(current_user, :update_merge_request, merge_request) if merge_request.reopen - event_service.reopen_mr(merge_request, current_user) + create_event(merge_request) create_note(merge_request, 'reopened') notification_service.reopen_mr(merge_request, current_user) execute_hooks(merge_request, 'reopen') @@ -16,5 +16,16 @@ module MergeRequests merge_request end + + private + + def create_event(merge_request) + # Making sure MergeRequest::Metrics updates are in sync with + # Event creation. + Event.transaction do + event_service.reopen_mr(merge_request, current_user) + merge_request_metrics_service(merge_request).reopen + end + end end end diff --git a/app/services/merge_requests/working_copy_base_service.rb b/app/services/merge_requests/working_copy_base_service.rb new file mode 100644 index 00000000000..186e05bf966 --- /dev/null +++ b/app/services/merge_requests/working_copy_base_service.rb @@ -0,0 +1,24 @@ +module MergeRequests + class WorkingCopyBaseService < MergeRequests::BaseService + attr_reader :merge_request + + def source_project + @source_project ||= merge_request.source_project + end + + def target_project + @target_project ||= merge_request.target_project + end + + def log_error(message, save_message_on_model: false) + Gitlab::GitLogger.error("#{self.class.name} error (#{merge_request.to_reference(full: true)}): #{message}") + + merge_request.update(merge_error: message) if save_message_on_model + end + + # Don't try to print expensive instance variables. + def inspect + "#<#{self.class} #{merge_request.to_reference(full: true)}>" + end + end +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/create_service.rb b/app/services/projects/create_service.rb index 71533da31b1..01838ec6b5d 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -56,11 +56,7 @@ module Projects after_create_actions if @project.persisted? - if @project.errors.empty? - @project.import_schedule if @project.import? - else - fail(error: @project.errors.full_messages.join(', ')) - end + import_schedule @project rescue ActiveRecord::RecordInvalid => e @@ -92,6 +88,7 @@ module Projects log_info("#{@project.owner.name} created a new project \"#{@project.name_with_namespace}\"") unless @project.gitlab_project_import? + @project.write_repository_config @project.create_wiki unless skip_wiki? create_services_from_active_templates(@project) @@ -164,5 +161,15 @@ module Projects @project.path = @project.name.dup.parameterize end end + + private + + def import_schedule + if @project.errors.empty? + @project.import_schedule if @project.import? && !@project.bare_repository_import? + else + fail(error: @project.errors.full_messages.join(', ')) + end + end end end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 03be7039b2a..348eb0bf8d8 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -26,7 +26,7 @@ module Projects name: @project.name, path: @project.path, shared_runners_enabled: @project.shared_runners_enabled, - namespace_id: @params[:namespace].try(:id) || current_user.namespace.id + namespace_id: target_namespace.id } if @project.avatar.present? && @project.avatar.image? @@ -74,14 +74,14 @@ module Projects Projects::ForksCountService.new(@project).refresh_cache end + def target_namespace + @target_namespace ||= @params[:namespace] || current_user.namespace + end + def allowed_visibility_level - project_level = @project.visibility_level + target_level = [@project.visibility_level, target_namespace.visibility_level].min - if Gitlab::VisibilityLevel.non_restricted_level?(project_level) - project_level - else - Gitlab::VisibilityLevel.highest_allowed_level - end + Gitlab::VisibilityLevel.closest_allowed_level(target_level) end 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/hashed_storage/migrate_repository_service.rb b/app/services/projects/hashed_storage/migrate_repository_service.rb index 7212e7524ab..67178de75de 100644 --- a/app/services/projects/hashed_storage/migrate_repository_service.rb +++ b/app/services/projects/hashed_storage/migrate_repository_service.rb @@ -27,7 +27,9 @@ module Projects result &&= move_repository("#{@old_wiki_disk_path}", "#{@new_disk_path}.wiki") end - unless result + if result + project.write_repository_config + else rollback_folder_move project.storage_version = nil end 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/transfer_service.rb b/app/services/projects/transfer_service.rb index e5cd6fcdfe3..26765e5c3f3 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -75,6 +75,8 @@ module Projects project.old_path_with_namespace = @old_path project.expires_full_path_cache + write_repository_config(@new_path) + execute_system_hooks end rescue Exception # rubocop:disable Lint/RescueException @@ -98,6 +100,10 @@ module Projects project.save! end + def write_repository_config(full_path) + project.write_repository_config(gl_full_path: full_path) + end + def refresh_permissions # This ensures we only schedule 1 job for every user that has access to # the namespaces. @@ -110,6 +116,7 @@ module Projects def rollback_side_effects rollback_folder_move update_namespace_and_visibility(@old_namespace) + write_repository_config(@old_path) end def rollback_folder_move 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/protected_branches/create_service.rb b/app/services/protected_branches/create_service.rb index a84e335340d..6212fd69077 100644 --- a/app/services/protected_branches/create_service.rb +++ b/app/services/protected_branches/create_service.rb @@ -2,8 +2,8 @@ module ProtectedBranches class CreateService < BaseService attr_reader :protected_branch - def execute - raise Gitlab::Access::AccessDeniedError unless can?(current_user, :admin_project, project) + def execute(skip_authorization: false) + raise Gitlab::Access::AccessDeniedError unless skip_authorization || can?(current_user, :admin_project, project) project.protected_branches.create(params) end diff --git a/app/services/reset_project_cache_service.rb b/app/services/reset_project_cache_service.rb new file mode 100644 index 00000000000..a162a6eedb9 --- /dev/null +++ b/app/services/reset_project_cache_service.rb @@ -0,0 +1,5 @@ +class ResetProjectCacheService < BaseService + def execute + @project.increment!(:jobs_cache_index) + end +end 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 8e20de8dfa5..b71002433d6 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -31,6 +31,11 @@ module Users return user end + # Calling all before/after_destroy hooks for the user because + # there is no dependent: destroy in the relationship. And the removal + # is done by a foreign_key. Otherwise they won't be called + user.members.find_each { |member| member.run_callbacks(:destroy) } + user.solo_owned_groups.each do |group| Groups::DestroyService.new(group, current_user).execute end @@ -48,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 |