diff options
Diffstat (limited to 'app/services')
20 files changed, 103 insertions, 44 deletions
diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 5cb7a86a5ee..db82b8f6c30 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -2,7 +2,7 @@ module Auth class ContainerRegistryAuthenticationService < BaseService include Gitlab::CurrentSettings - AUDIENCE = 'container_registry' + AUDIENCE = 'container_registry'.freeze def execute(authentication_abilities:) @authentication_abilities = authentication_abilities diff --git a/app/services/base_service.rb b/app/services/base_service.rb index fa45506317e..745c2c4b681 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -28,9 +28,7 @@ class BaseService SystemHooksService.new end - def repository - project.repository - end + delegate :repository, to: :project # Add an error to the specified model for restricted visibility levels def deny_visibility_level(model, denied_visibility_level = nil) diff --git a/app/services/ci/create_pipeline_builds_service.rb b/app/services/ci/create_pipeline_builds_service.rb index b7da3f8e7eb..70fb2c5e38f 100644 --- a/app/services/ci/create_pipeline_builds_service.rb +++ b/app/services/ci/create_pipeline_builds_service.rb @@ -10,9 +10,7 @@ module Ci end end - def project - pipeline.project - end + delegate :project, to: :pipeline private diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index 4b47ee489cf..38ef323f6e5 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -1,16 +1,17 @@ module Ci class RetryBuildService < ::BaseService - CLONE_ATTRIBUTES = %i[pipeline ref tag options commands tag_list name + CLONE_ATTRIBUTES = %i[pipeline project ref tag options commands name allow_failure stage stage_idx trigger_request yaml_variables when environment coverage_regex] .freeze REJECT_ATTRIBUTES = %i[id status user token coverage trace runner - artifacts_file artifacts_metadata artifacts_size + artifacts_expire_at artifacts_file + artifacts_metadata artifacts_size created_at updated_at started_at finished_at queued_at erased_by erased_at].freeze - IGNORE_ATTRIBUTES = %i[trace type lock_version project target_url + IGNORE_ATTRIBUTES = %i[type lock_version gl_project_id target_url deploy job_id description].freeze def execute(build) diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index 2c5e130e5aa..574561adc4c 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -1,5 +1,7 @@ module Ci class RetryPipelineService < ::BaseService + include Gitlab::OptimisticLocking + def execute(pipeline) unless can?(current_user, :update_pipeline, pipeline) raise Gitlab::Access::AccessDeniedError @@ -12,6 +14,10 @@ module Ci .reprocess(build) end + pipeline.builds.skipped.find_each do |skipped| + retry_optimistic_lock(skipped) { |build| build.process } + end + MergeRequests::AddTodoWhenBuildFailsService .new(project, current_user) .close_all(pipeline) diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index 77459d8779d..b07338d500a 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -1,5 +1,7 @@ class CreateBranchService < BaseService def execute(branch_name, ref) + create_master_branch if project.empty_repo? + result = ValidateNewBranchService.new(project, current_user) .execute(branch_name) @@ -19,4 +21,16 @@ class CreateBranchService < BaseService def success(branch) super().merge(branch: branch) end + + private + + def create_master_branch + project.repository.commit_file( + current_user, + '/README.md', + '', + message: 'Add README.md', + branch_name: 'master', + update: false) + end end diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb index 858de5f0538..083ffdc634c 100644 --- a/app/services/files/create_dir_service.rb +++ b/app/services/files/create_dir_service.rb @@ -1,7 +1,7 @@ module Files class CreateDirService < Files::BaseService def commit - repository.commit_dir( + repository.create_dir( current_user, @file_path, message: @commit_message, diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 88dd7bbaedb..65b5537fb68 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,13 +1,12 @@ module Files class CreateService < Files::BaseService def commit - repository.commit_file( + repository.create_file( current_user, @file_path, @file_content, message: @commit_message, branch_name: @target_branch, - update: false, author_email: @author_email, author_name: @author_name, start_project: @start_project, @@ -17,6 +16,10 @@ module Files def validate super + if @file_content.nil? + raise_error("You must provide content.") + end + if @file_path =~ Gitlab::Regex.directory_traversal_regex raise_error( 'Your changes could not be committed, because the file name ' + diff --git a/app/services/files/destroy_service.rb b/app/services/files/destroy_service.rb index c3be806a42d..e294659bc98 100644 --- a/app/services/files/destroy_service.rb +++ b/app/services/files/destroy_service.rb @@ -1,7 +1,7 @@ module Files class DestroyService < Files::BaseService def commit - repository.remove_file( + repository.delete_file( current_user, @file_path, message: @commit_message, diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index af6da5b9d56..0609c6219e7 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -2,6 +2,8 @@ module Files class MultiService < Files::BaseService class FileChangedError < StandardError; end + ACTIONS = %w[create update delete move].freeze + def commit repository.multi_action( user: current_user, @@ -21,10 +23,19 @@ module Files super params[:actions].each_with_index do |action, index| + if ACTIONS.include?(action[:action].to_s) + action[:action] = action[:action].to_sym + else + raise_error("Unknown action type `#{action[:action]}`.") + end + unless action[:file_path].present? raise_error("You must specify a file_path.") end + action[:file_path].slice!(0) if action[:file_path] && action[:file_path].start_with?('/') + action[:previous_path].slice!(0) if action[:previous_path] && action[:previous_path].start_with?('/') + regex_check(action[:file_path]) regex_check(action[:previous_path]) if action[:previous_path] @@ -43,8 +54,6 @@ module Files validate_delete(action) when :move validate_move(action, index) - else - raise_error("Unknown action type `#{action[:action]}`.") end end end @@ -92,6 +101,20 @@ module Files if repository.blob_at_branch(params[:branch], action[:file_path]) raise_error("Your changes could not be committed because a file with the name `#{action[:file_path]}` already exists.") end + + if action[:content].nil? + raise_error("You must provide content.") + end + end + + def validate_update(action) + if action[:content].nil? + raise_error("You must provide content.") + end + + if file_has_changed? + raise FileChangedError.new("You are attempting to update a file `#{action[:file_path]}` that has changed since you started editing it.") + end end def validate_delete(action) @@ -114,11 +137,5 @@ module Files params[:actions][index][:content] = blob.data end end - - def validate_update(action) - if file_has_changed? - raise FileChangedError.new("You are attempting to update a file `#{action[:file_path]}` that has changed since you started editing it.") - end - end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index a71fe61a4b6..54e1aaf3f67 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -18,6 +18,10 @@ module Files def validate super + if @file_content.nil? + raise_error("You must provide content.") + end + if file_has_changed? raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.") end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 431da8372c9..2e089149ca8 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -4,7 +4,7 @@ module Members attr_accessor :source - ALLOWED_SCOPES = %i[members requesters all] + ALLOWED_SCOPES = %i[members requesters all].freeze def initialize(source, current_user, params = {}) @source = source diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index 56913568cae..ad1e6f6774a 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -3,7 +3,7 @@ module Notes UPDATE_SERVICES = { 'Issue' => Issues::UpdateService, 'MergeRequest' => MergeRequests::UpdateService - } + }.freeze def self.noteable_update_service(note) UPDATE_SERVICES[note.noteable_type] diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 9716a1780a9..2e06826c311 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -4,7 +4,7 @@ module Projects class DestroyError < StandardError; end - DELETED_FLAG = '+deleted' + DELETED_FLAG = '+deleted'.freeze def async_execute project.transaction do diff --git a/app/services/projects/download_service.rb b/app/services/projects/download_service.rb index f06a3d44c17..604747e39d0 100644 --- a/app/services/projects/download_service.rb +++ b/app/services/projects/download_service.rb @@ -2,7 +2,7 @@ module Projects class DownloadService < BaseService WHITELIST = [ /^[^.]+\.fogbugz.com$/ - ] + ].freeze def initialize(project, url) @project, @url = project, url @@ -25,7 +25,7 @@ module Projects end def http?(url) - url =~ /\A#{URI::regexp(['http', 'https'])}\z/ + url =~ /\A#{URI.regexp(%w(http https))}\z/ end def valid_domain?(url) diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index f5f9ee88912..2d42c4fc04a 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -2,7 +2,7 @@ module Projects class UpdatePagesService < BaseService BLOCK_SIZE = 32.kilobytes MAX_SIZE = 1.terabyte - SITE_PATH = 'public/' + SITE_PATH = 'public/'.freeze attr_reader :build diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb index 050cb3b738b..bdb0e0cc8bf 100644 --- a/app/services/protected_branches/api_update_service.rb +++ b/app/services/protected_branches/api_update_service.rb @@ -15,16 +15,16 @@ module ProtectedBranches case @developers_can_push when true - params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + params[:push_access_levels_attributes] = [{ access_level: Gitlab::Access::DEVELOPER }] when false - params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + params[:push_access_levels_attributes] = [{ access_level: Gitlab::Access::MASTER }] end case @developers_can_merge when true - params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + params[:merge_access_levels_attributes] = [{ access_level: Gitlab::Access::DEVELOPER }] when false - params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + params[:merge_access_levels_attributes] = [{ access_level: Gitlab::Access::MASTER }] end service = ProtectedBranches::UpdateService.new(@project, @current_user, @params) diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index a2bfa422c9d..9b6dd013e3a 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -33,9 +33,7 @@ class SystemHooksService data.merge!(project_data(model)) if event == :rename || event == :transfer - data.merge!({ - old_path_with_namespace: model.old_path_with_namespace - }) + data[:old_path_with_namespace] = model.old_path_with_namespace end data diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 87ba72cf991..55b548a12f9 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -356,10 +356,10 @@ module SystemNoteService note: cross_reference_note_content(gfm_reference) } - if noteable.kind_of?(Commit) + if noteable.is_a?(Commit) note_options.merge!(noteable_type: 'Commit', commit_id: noteable.id) else - note_options.merge!(noteable: noteable) + note_options[:noteable] = noteable end if noteable.is_a?(ExternalIssue) @@ -408,12 +408,13 @@ module SystemNoteService # Initial scope should be system notes of this noteable type notes = Note.system.where(noteable_type: noteable.class) - if noteable.is_a?(Commit) - # Commits have non-integer IDs, so they're stored in `commit_id` - notes = notes.where(commit_id: noteable.id) - else - notes = notes.where(noteable_id: noteable.id) - end + notes = + if noteable.is_a?(Commit) + # Commits have non-integer IDs, so they're stored in `commit_id` + notes.where(commit_id: noteable.id) + else + notes.where(noteable_id: noteable.id) + end notes_for_mentioner(mentioner, noteable, notes).exists? end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index bc0653cb634..833da5bc5d1 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -7,7 +7,7 @@ module Users end def execute(user, options = {}) - unless current_user.admin? || current_user == user + unless Ability.allowed?(current_user, :destroy_user, user) raise Gitlab::Access::AccessDeniedError, "#{current_user} tried to destroy user #{user}!" end @@ -26,6 +26,8 @@ module Users ::Projects::DestroyService.new(project, current_user, skip_repo: true).async_execute end + move_issues_to_ghost_user(user) + # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing namespace = user.namespace user_data = user.destroy @@ -33,5 +35,22 @@ module Users user_data end + + private + + def move_issues_to_ghost_user(user) + # Block the user before moving issues to prevent a data race. + # If the user creates an issue after `move_issues_to_ghost_user` + # runs and before the user is destroyed, the destroy will fail with + # an exception. We block the user so that issues can't be created + # after `move_issues_to_ghost_user` runs and before the destroy happens. + user.block + + ghost_user = User.ghost + + user.issues.update_all(author_id: ghost_user.id) + + user.reload + end end end |