diff options
author | Valery Sizov <valery@gitlab.com> | 2016-07-06 16:26:59 +0300 |
---|---|---|
committer | Valery Sizov <valery@gitlab.com> | 2016-07-06 17:44:27 +0300 |
commit | c14acda909a6b463a29069568be803c78a2f9b46 (patch) | |
tree | b985bac8f28fabfa52d61e0e3c7535bf9c917fe5 | |
parent | cfd5870b62e9d76e564ffc64db1d1281b4a363bb (diff) | |
download | gitlab-ce-services_refactoring.tar.gz |
Services: code style fixes, minor refactoringservices_refactoring
-rw-r--r-- | app/services/audit_event_service.rb | 2 | ||||
-rw-r--r-- | app/services/auth/container_registry_authentication_service.rb | 2 | ||||
-rw-r--r-- | app/services/base_service.rb | 8 | ||||
-rw-r--r-- | app/services/create_branch_service.rb | 10 | ||||
-rw-r--r-- | app/services/create_release_service.rb | 4 | ||||
-rw-r--r-- | app/services/create_snippet_service.rb | 10 | ||||
-rw-r--r-- | app/services/create_tag_service.rb | 2 | ||||
-rw-r--r-- | app/services/delete_branch_service.rb | 12 | ||||
-rw-r--r-- | app/services/delete_tag_service.rb | 9 | ||||
-rw-r--r-- | app/services/files/base_service.rb | 7 | ||||
-rw-r--r-- | app/services/files/create_service.rb | 2 | ||||
-rw-r--r-- | app/services/git_tag_push_service.rb | 1 | ||||
-rw-r--r-- | app/services/issues/bulk_update_service.rb | 1 | ||||
-rw-r--r-- | app/services/merge_requests/post_merge_service.rb | 1 | ||||
-rw-r--r-- | app/services/notification_service.rb | 2 | ||||
-rw-r--r-- | app/services/projects/update_service.rb | 3 | ||||
-rw-r--r-- | app/services/system_note_service.rb | 7 | ||||
-rw-r--r-- | app/services/update_release_service.rb | 4 | ||||
-rw-r--r-- | app/services/update_snippet_service.rb | 1 |
19 files changed, 40 insertions, 48 deletions
diff --git a/app/services/audit_event_service.rb b/app/services/audit_event_service.rb index a7f090655e1..8a000585e89 100644 --- a/app/services/audit_event_service.rb +++ b/app/services/audit_event_service.rb @@ -7,7 +7,7 @@ class AuditEventService @details = { with: @details[:with], target_id: @author.id, - target_type: "User", + target_type: 'User', target_details: @author.name, } diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index e57b95f21ec..e294a962352 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -20,9 +20,11 @@ module Auth token.issuer = registry.issuer token.audience = AUDIENCE token.expire_time = token_expire_at + token[:access] = names.map do |name| { type: 'repository', name: name, actions: %w(*) } end + token.encoded end diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 0d55ba5a981..b57f1f5c18b 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -51,13 +51,11 @@ class BaseService private def error(message, http_status = nil) - result = { + { message: message, - status: :error + status: :error, + http_status: (http_status if http_status) } - - result[:http_status] = http_status if http_status - result end def success diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index cc128563437..f9efaa6de32 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -3,17 +3,20 @@ require_relative 'base_service' class CreateBranchService < BaseService def execute(branch_name, ref, source_project: @project) valid_branch = Gitlab::GitRefValidator.validate(branch_name) - if valid_branch == false + + unless valid_branch return error('Branch name is invalid') end repository = project.repository existing_branch = repository.find_branch(branch_name) + if existing_branch return error('Branch already exists') end new_branch = nil + if source_project != @project repository.with_tmp_ref do |tmp_ref| repository.fetch_ref( @@ -29,7 +32,6 @@ class CreateBranchService < BaseService end if new_branch - # GitPushService handles execution of services and hooks for branch pushes success(new_branch) else error('Invalid reference name') @@ -39,8 +41,6 @@ class CreateBranchService < BaseService end def success(branch) - out = super() - out[:branch] = branch - out + super.merge(branch: branch) end end diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb index f029db72d40..6e224921ec0 100644 --- a/app/services/create_release_service.rb +++ b/app/services/create_release_service.rb @@ -23,8 +23,6 @@ class CreateReleaseService < BaseService end def success(release) - out = super() - out[:release] = release - out + super.merge(release: release) end end diff --git a/app/services/create_snippet_service.rb b/app/services/create_snippet_service.rb index 9884cb96661..95cc9baf406 100644 --- a/app/services/create_snippet_service.rb +++ b/app/services/create_snippet_service.rb @@ -1,10 +1,10 @@ class CreateSnippetService < BaseService def execute - if project.nil? - snippet = PersonalSnippet.new(params) - else - snippet = project.snippets.build(params) - end + snippet = if project + project.snippets.build(params) + else + PersonalSnippet.new(params) + end unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) deny_visibility_level(snippet) diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index bd8d982e1fb..c0e7ecf6a96 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -9,6 +9,7 @@ class CreateTagService < BaseService message.strip! if message new_tag = nil + begin new_tag = repository.add_tag(current_user, tag_name, target, message) rescue Rugged::TagError @@ -22,6 +23,7 @@ class CreateTagService < BaseService CreateReleaseService.new(@project, @current_user). execute(tag_name, release_description) end + success.merge(tag: new_tag) else error("Target #{target} is invalid") diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 752a7029952..dbd7efa0a6f 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -5,7 +5,6 @@ class DeleteBranchService < BaseService repository = project.repository branch = repository.find_branch(branch_name) - # No such branch unless branch return error('No such branch', 404) end @@ -14,18 +13,15 @@ class DeleteBranchService < BaseService return error('Cannot remove HEAD branch', 405) end - # Dont allow remove of protected branch if project.protected_branch?(branch_name) return error('Protected branch cant be removed', 405) end - # Dont allow user to remove branch if he is not allowed to push unless current_user.can?(:push_code, project) return error('You dont have push access to repo', 405) end if repository.rm_branch(current_user, branch_name) - # GitPushService handles execution of services and hooks for branch pushes success('Branch was removed') else error('Failed to remove branch') @@ -35,15 +31,11 @@ class DeleteBranchService < BaseService end def error(message, return_code = 400) - out = super(message) - out[:return_code] = return_code - out + super(message).merge(return_code: return_code) end def success(message) - out = super() - out[:message] = message - out + super.merge(message: message) end def build_push_data(branch) diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index de3352a6756..e00d49c4f64 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -5,7 +5,6 @@ class DeleteTagService < BaseService repository = project.repository tag = repository.find_tag(tag_name) - # No such tag unless tag return error('No such tag', 404) end @@ -26,15 +25,11 @@ class DeleteTagService < BaseService end def error(message, return_code = 400) - out = super(message) - out[:return_code] = return_code - out + super(message).merge(return_code: return_code) end def success(message) - out = super() - out[:message] = message - out + super.merge(message: message) end def build_push_data(tag) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 4bdb68a3698..37c5e321b39 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -15,7 +15,6 @@ module Files params[:file_content] end - # Validate parameters validate # Create new branch if it different from source_branch @@ -26,7 +25,7 @@ module Files if commit success else - error("Something went wrong. Your changes were not committed") + error('Something went wrong. Your changes were not committed') end rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex error(ex.message) @@ -51,12 +50,12 @@ module Files unless project.empty_repo? unless @source_project.repository.branch_names.include?(@source_branch) - raise_error("You can only create or edit files when you are on a branch") + raise_error('You can only create or edit files when you are on a branch') end if different_branch? if repository.branch_names.include?(@target_branch) - raise_error("Branch with such name already exists. You need to switch to this branch in order to make changes") + raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes') end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index e4cde4a2fd8..8eaf6db8012 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -29,7 +29,7 @@ module Files blob = repository.blob_at_branch(@source_branch, @file_path) if blob - raise_error("Your changes could not be committed because a file with the same name already exists") + raise_error('Your changes could not be committed because a file with the same name already exists') end end end diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index 299a0a967b0..58573078048 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -26,6 +26,7 @@ class GitTagPushService < BaseService unless Gitlab::Git.blank_ref?(params[:newrev]) tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) + if tag && tag.target == params[:newrev] commit = project.commit(tag.target) commits = [commit].compact diff --git a/app/services/issues/bulk_update_service.rb b/app/services/issues/bulk_update_service.rb index 15825b81685..cd08c3a0cb9 100644 --- a/app/services/issues/bulk_update_service.rb +++ b/app/services/issues/bulk_update_service.rb @@ -9,6 +9,7 @@ module Issues end issues = Issue.where(id: issues_ids) + issues.each do |issue| next unless can?(current_user, :update_issue, issue) diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 064910f81f7..8437d9b8b43 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -20,6 +20,7 @@ module MergeRequests return unless merge_request.target_branch == project.default_branch closed_issues = merge_request.closes_issues(current_user) + closed_issues.each do |issue| if can?(current_user, :update_issue, issue) Issues::CloseService.new(project, current_user, {}).execute(issue, commit: merge_request) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 590350a11e5..ab6e51209ee 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -153,6 +153,7 @@ class NotificationService else mentioned_users end + recipients = recipients.concat(participants) # Merge project watchers @@ -176,6 +177,7 @@ class NotificationService # build notify method like 'note_commit_email' notify_method = "note_#{note.noteable_type.underscore}_email".to_sym + recipients.each do |recipient| mailer.send(notify_method, recipient.id, note.id).deliver_later end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 941df08995c..f06311511cc 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -3,10 +3,11 @@ module Projects def execute # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] + if new_visibility && new_visibility.to_i != project.visibility_level unless can?(current_user, :change_visibility_level, project) && Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) - + deny_visibility_level(project, new_visibility) return project end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index b868d2e7e83..1ab3b5789bc 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -82,7 +82,7 @@ class SystemNoteService end body << ' ' << 'label'.pluralize(labels_count) - body = "#{body.capitalize}" + body = body.capitalize create_note(noteable: noteable, project: project, author: author, note: body) end @@ -125,7 +125,7 @@ class SystemNoteService # Returns the created Note object def self.change_status(noteable, project, author, status, source) body = "Status changed to #{status}" - body += " by #{source.gfm_reference(project)}" if source + body << " by #{source.gfm_reference(project)}" if source create_note(noteable: noteable, project: project, author: author, note: body) end @@ -139,7 +139,7 @@ class SystemNoteService # Called when 'merge when build succeeds' is canceled def self.cancel_merge_when_build_succeeds(noteable, project, author) - body = "Canceled the automatic merge" + body = 'Canceled the automatic merge' create_note(noteable: noteable, project: project, author: author, note: body) end @@ -236,6 +236,7 @@ class SystemNoteService else 'deleted' end + body = "#{verb} #{branch_type.to_s} branch `#{branch}`".capitalize create_note(noteable: noteable, project: project, author: author, note: body) end diff --git a/app/services/update_release_service.rb b/app/services/update_release_service.rb index 0c0f68d169b..517373d2411 100644 --- a/app/services/update_release_service.rb +++ b/app/services/update_release_service.rb @@ -21,8 +21,6 @@ class UpdateReleaseService < BaseService end def success(release) - out = super() - out[:release] = release - out + super.merge(release: release) end end diff --git a/app/services/update_snippet_service.rb b/app/services/update_snippet_service.rb index 93af8f21972..a6bb36821c3 100644 --- a/app/services/update_snippet_service.rb +++ b/app/services/update_snippet_service.rb @@ -9,6 +9,7 @@ class UpdateSnippetService < BaseService def execute # check that user is allowed to set specified visibility_level new_visibility = params[:visibility_level] + if new_visibility && new_visibility.to_i != snippet.visibility_level unless Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) deny_visibility_level(snippet, new_visibility) |