diff options
author | Dylan Griffith <dyl.griffith@gmail.com> | 2018-05-03 09:54:12 +0200 |
---|---|---|
committer | Dylan Griffith <dyl.griffith@gmail.com> | 2018-05-03 09:54:12 +0200 |
commit | d39b3d4b8d2d4c5ded46182a5353c68a8f5bb5cd (patch) | |
tree | e8f2fac760b252928ff23074dea9d1f209838a33 /app/services | |
parent | dcb67951a817db262ddcd3b777fafc4e1995fc04 (diff) | |
parent | 2c9568edeea7d95b6e4ec3c23cdc1c027bf86d5f (diff) | |
download | gitlab-ce-d39b3d4b8d2d4c5ded46182a5353c68a8f5bb5cd.tar.gz |
Merge branch 'master' into feature/runner-per-group
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/ci/ensure_stage_service.rb | 1 | ||||
-rw-r--r-- | app/services/issues/close_service.rb | 2 | ||||
-rw-r--r-- | app/services/issues/move_service.rb | 2 | ||||
-rw-r--r-- | app/services/issues/reopen_service.rb | 2 | ||||
-rw-r--r-- | app/services/issues/update_service.rb | 6 | ||||
-rw-r--r-- | app/services/merge_requests/close_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 29 | ||||
-rw-r--r-- | app/services/merge_requests/reopen_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/resolved_discussion_notification_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 11 | ||||
-rw-r--r-- | app/services/notes/resolve_service.rb | 9 | ||||
-rw-r--r-- | app/services/notification_service.rb | 75 | ||||
-rw-r--r-- | app/services/projects/update_pages_service.rb | 29 | ||||
-rw-r--r-- | app/services/repository_archive_clean_up_service.rb | 5 | ||||
-rw-r--r-- | app/services/system_note_service.rb | 2 |
15 files changed, 103 insertions, 76 deletions
diff --git a/app/services/ci/ensure_stage_service.rb b/app/services/ci/ensure_stage_service.rb index 87f19b333de..b8c7be2d350 100644 --- a/app/services/ci/ensure_stage_service.rb +++ b/app/services/ci/ensure_stage_service.rb @@ -42,6 +42,7 @@ module Ci def create_stage Ci::Stage.create!(name: @build.stage, + position: @build.stage_idx, pipeline: @build.pipeline, project: @build.project) end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index fee5bc38f7b..4a99367c575 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -26,7 +26,7 @@ module Issues issue.update(closed_by: current_user) event_service.close_issue(issue, current_user) create_note(issue, commit) if system_note - notification_service.close_issue(issue, current_user) if notifications + notification_service.async.close_issue(issue, current_user) if notifications todo_service.close_issue(issue, current_user) execute_hooks(issue, 'close') invalidate_cache_counts(issue, users: issue.assignees) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index 7140890d201..78e79344c99 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -139,7 +139,7 @@ module Issues end def notify_participants - notification_service.issue_moved(@old_issue, @new_issue, @current_user) + notification_service.async.issue_moved(@old_issue, @new_issue, @current_user) end end end diff --git a/app/services/issues/reopen_service.rb b/app/services/issues/reopen_service.rb index 62b4b4b6a1e..02224f3357a 100644 --- a/app/services/issues/reopen_service.rb +++ b/app/services/issues/reopen_service.rb @@ -6,7 +6,7 @@ module Issues if issue.reopen event_service.reopen_issue(issue, current_user) create_note(issue, 'reopened') - notification_service.reopen_issue(issue, current_user) + notification_service.async.reopen_issue(issue, current_user) execute_hooks(issue, 'reopen') invalidate_cache_counts(issue, users: issue.assignees) issue.update_project_counter_caches diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index 1374f10c586..1000e1842b6 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -30,7 +30,7 @@ module Issues if issue.assignees != old_assignees create_assignee_note(issue, old_assignees) - notification_service.reassigned_issue(issue, current_user, old_assignees) + notification_service.async.reassigned_issue(issue, current_user, old_assignees) todo_service.reassigned_issue(issue, current_user, old_assignees) end @@ -41,13 +41,13 @@ module Issues added_labels = issue.labels - old_labels if added_labels.present? - notification_service.relabeled_issue(issue, added_labels, current_user) + notification_service.async.relabeled_issue(issue, added_labels, current_user) end added_mentions = issue.mentioned_users - old_mentioned_users if added_mentions.present? - notification_service.new_mentions_in_issue(issue, added_mentions, current_user) + notification_service.async.new_mentions_in_issue(issue, added_mentions, current_user) end end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index f727ec002e7..db701c1145d 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -10,7 +10,7 @@ module MergeRequests if merge_request.close create_event(merge_request) create_note(merge_request) - notification_service.close_mr(merge_request, current_user) + notification_service.async.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) execute_hooks(merge_request, 'close') invalidate_cache_counts(merge_request, users: merge_request.assignees) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index cedfcb50e09..2209a60a840 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -50,21 +50,30 @@ module MergeRequests end def commit - message = params[:commit_message] || merge_request.merge_commit_message - log_info("Git merge started on JID #{merge_jid}") - commit_id = repository.merge(current_user, source, merge_request, message) - log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") + commit_id = try_merge + + if commit_id + log_info("Git merge finished on JID #{merge_jid} commit #{commit_id}") + else + raise MergeError, 'Conflicts detected during merge' + end - raise MergeError, 'Conflicts detected during merge' unless commit_id + merge_request.update!(merge_commit_sha: commit_id) + end + + def try_merge + message = params[:commit_message] || merge_request.merge_commit_message - merge_request.update(merge_commit_sha: commit_id) + repository.merge(current_user, source, merge_request, message) rescue Gitlab::Git::HooksService::PreReceiveError => e - raise MergeError, e.message - rescue StandardError => e - raise MergeError, "Something went wrong during merge: #{e.message}" + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge pre-receive hook' + rescue => e + handle_merge_error(log_message: e.message) + raise MergeError, 'Something went wrong during merge' ensure - merge_request.update(in_progress_merge_commit_sha: nil) + merge_request.update!(in_progress_merge_commit_sha: nil) end def after_merge diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index 120677a7149..8f1c95ac1b7 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.reopen create_event(merge_request) create_note(merge_request, 'reopened') - notification_service.reopen_mr(merge_request, current_user) + notification_service.async.reopen_mr(merge_request, current_user) execute_hooks(merge_request, 'reopen') merge_request.reload_diff(current_user) merge_request.mark_as_unchecked diff --git a/app/services/merge_requests/resolved_discussion_notification_service.rb b/app/services/merge_requests/resolved_discussion_notification_service.rb index 3a09350c847..66a0cbc81d4 100644 --- a/app/services/merge_requests/resolved_discussion_notification_service.rb +++ b/app/services/merge_requests/resolved_discussion_notification_service.rb @@ -4,7 +4,7 @@ module MergeRequests return unless merge_request.discussions_resolved? SystemNoteService.resolve_all_discussions(merge_request, project, current_user) - notification_service.resolve_all_discussions(merge_request, current_user) + notification_service.async.resolve_all_discussions(merge_request, current_user) end end end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 8a40ad88182..7350725e223 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -21,6 +21,7 @@ module MergeRequests update(merge_request) end + # rubocop:disable Metrics/AbcSize def handle_changes(merge_request, options) old_associations = options.fetch(:old_associations, {}) old_labels = old_associations.fetch(:labels, []) @@ -42,8 +43,11 @@ module MergeRequests end if merge_request.previous_changes.include?('assignee_id') + old_assignee_id = merge_request.previous_changes['assignee_id'].first + old_assignee = User.find(old_assignee_id) if old_assignee_id + create_assignee_note(merge_request) - notification_service.reassigned_merge_request(merge_request, current_user) + notification_service.async.reassigned_merge_request(merge_request, current_user, old_assignee) todo_service.reassigned_merge_request(merge_request, current_user) end @@ -54,7 +58,7 @@ module MergeRequests added_labels = merge_request.labels - old_labels if added_labels.present? - notification_service.relabeled_merge_request( + notification_service.async.relabeled_merge_request( merge_request, added_labels, current_user @@ -63,13 +67,14 @@ module MergeRequests added_mentions = merge_request.mentioned_users - old_mentioned_users if added_mentions.present? - notification_service.new_mentions_in_merge_request( + notification_service.async.new_mentions_in_merge_request( merge_request, added_mentions, current_user ) end end + # rubocop:enable Metrics/AbcSize def merge_from_quick_action(merge_request) last_diff_sha = params.delete(:merge) diff --git a/app/services/notes/resolve_service.rb b/app/services/notes/resolve_service.rb new file mode 100644 index 00000000000..0db8ee809a9 --- /dev/null +++ b/app/services/notes/resolve_service.rb @@ -0,0 +1,9 @@ +module Notes + class ResolveService < ::BaseService + def execute(note) + note.resolve!(current_user) + + ::MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable) + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 274161df946..55a1735e54b 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -7,7 +7,32 @@ # Ex. # NotificationService.new.new_issue(issue, current_user) # +# When calculating the recipients of a notification is expensive (for instance, +# in the new issue case), `#async` will make that calculation happen in Sidekiq +# instead: +# +# NotificationService.new.async.new_issue(issue, current_user) +# class NotificationService + class Async + attr_reader :parent + delegate :respond_to_missing, to: :parent + + def initialize(parent) + @parent = parent + end + + def method_missing(meth, *args) + return super unless parent.respond_to?(meth) + + MailScheduler::NotificationServiceWorker.perform_async(meth.to_s, *args) + end + end + + def async + @async ||= Async.new(self) + end + # Always notify user about ssh key added # only if ssh key is not deploy key # @@ -142,8 +167,23 @@ class NotificationService # * merge_request assignee if their notification level is not Disabled # * users with custom level checked with "reassign merge request" # - def reassigned_merge_request(merge_request, current_user) - reassign_resource_email(merge_request, current_user, :reassigned_merge_request_email) + def reassigned_merge_request(merge_request, current_user, previous_assignee) + recipients = NotificationRecipientService.build_recipients( + merge_request, + current_user, + action: "reassign", + previous_assignee: previous_assignee + ) + + recipients.each do |recipient| + mailer.reassigned_merge_request_email( + recipient.user.id, + merge_request.id, + previous_assignee&.id, + current_user.id, + recipient.reason + ).deliver_later + end end # When we add labels to a merge request we should send an email to: @@ -421,29 +461,6 @@ class NotificationService end end - def reassign_resource_email(target, current_user, method) - previous_assignee_id = previous_record(target, 'assignee_id') - previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - - recipients = NotificationRecipientService.build_recipients( - target, - current_user, - action: "reassign", - previous_assignee: previous_assignee - ) - - recipients.each do |recipient| - mailer.send( - method, - recipient.user.id, - target.id, - previous_assignee_id, - current_user.id, - recipient.reason - ).deliver_later - end - end - def relabeled_resource_email(target, labels, current_user, method) recipients = labels.flat_map { |l| l.subscribers(target.project) }.uniq recipients = notifiable_users( @@ -471,14 +488,6 @@ class NotificationService Notify end - def previous_record(object, attribute) - return unless object && attribute - - if object.previous_changes.include?(attribute) - object.previous_changes[attribute].first - end - end - private def recipients_for_pages_domain(domain) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index de77f6bf585..1d8caec9c6f 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -1,6 +1,6 @@ module Projects class UpdatePagesService < BaseService - InvaildStateError = Class.new(StandardError) + InvalidStateError = Class.new(StandardError) FailedToExtractError = Class.new(StandardError) BLOCK_SIZE = 32.kilobytes @@ -21,8 +21,8 @@ module Projects @status.enqueue! @status.run! - raise InvaildStateError, 'missing pages artifacts' unless build.artifacts? - raise InvaildStateError, 'pages are outdated' unless latest? + raise InvalidStateError, 'missing pages artifacts' unless build.artifacts? + raise InvalidStateError, 'pages are outdated' unless latest? # Create temporary directory in which we will extract the artifacts FileUtils.mkdir_p(tmp_path) @@ -31,16 +31,16 @@ module Projects # Check if we did extract public directory archive_public_path = File.join(archive_path, 'public') - raise InvaildStateError, 'pages miss the public folder' unless Dir.exist?(archive_public_path) - raise InvaildStateError, 'pages are outdated' unless latest? + raise InvalidStateError, 'pages miss the public folder' unless Dir.exist?(archive_public_path) + raise InvalidStateError, 'pages are outdated' unless latest? deploy_page!(archive_public_path) success end - rescue InvaildStateError => e + rescue InvalidStateError => e error(e.message) rescue => e - error(e.message, false) + error(e.message) raise e end @@ -48,17 +48,15 @@ module Projects def success @status.success - delete_artifact! super end - def error(message, allow_delete_artifact = true) + def error(message) register_failure log_error("Projects::UpdatePagesService: #{message}") @status.allow_failure = !latest? @status.description = message @status.drop(:script_failure) - delete_artifact! if allow_delete_artifact super end @@ -77,18 +75,18 @@ module Projects if artifacts.ends_with?('.zip') extract_zip_archive!(temp_path) else - raise InvaildStateError, 'unsupported artifacts format' + raise InvalidStateError, 'unsupported artifacts format' end end def extract_zip_archive!(temp_path) - raise InvaildStateError, 'missing artifacts metadata' unless build.artifacts_metadata? + raise InvalidStateError, 'missing artifacts metadata' unless build.artifacts_metadata? # Calculate page size after extract public_entry = build.artifacts_metadata_entry(SITE_PATH, recursive: true) if public_entry.total_size > max_size - raise InvaildStateError, "artifacts for pages are too large: #{public_entry.total_size}" + raise InvalidStateError, "artifacts for pages are too large: #{public_entry.total_size}" end # Requires UnZip at least 6.00 Info-ZIP. @@ -162,11 +160,6 @@ module Projects build.artifacts_file.path end - def delete_artifact! - build.reload # Reload stable object to prevent erase artifacts with old state - build.erase_artifacts! unless build.has_expiring_artifacts? - end - def latest_sha project.commit(build.ref).try(:sha).to_s ensure diff --git a/app/services/repository_archive_clean_up_service.rb b/app/services/repository_archive_clean_up_service.rb index 9a88459b511..ba7be4b3f89 100644 --- a/app/services/repository_archive_clean_up_service.rb +++ b/app/services/repository_archive_clean_up_service.rb @@ -20,11 +20,12 @@ class RepositoryArchiveCleanUpService private def clean_up_old_archives - run(%W(find #{path} -not -path #{path} -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -maxdepth 2 -mmin +#{mmin} -delete)) + run(%W(find #{path} -mindepth 1 -maxdepth 3 -type f \( -name \*.tar -o -name \*.bz2 -o -name \*.tar.gz -o -name \*.zip \) -mmin +#{mmin} -delete)) end def clean_up_empty_directories - run(%W(find #{path} -not -path #{path} -type d -empty -name \*.git -maxdepth 1 -delete)) + run(%W(find #{path} -mindepth 2 -maxdepth 2 -type d -empty -delete)) + run(%W(find #{path} -mindepth 1 -maxdepth 1 -type d -empty -delete)) end def run(cmd) diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 958ef065012..00bf5434b7f 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -159,7 +159,7 @@ module SystemNoteService body = if noteable.time_estimate == 0 "removed time estimate" else - "changed time estimate to #{parsed_time}," + "changed time estimate to #{parsed_time}" end create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) |