diff options
Diffstat (limited to 'app/services')
78 files changed, 1327 insertions, 339 deletions
diff --git a/app/services/after_branch_delete_service.rb b/app/services/after_branch_delete_service.rb new file mode 100644 index 00000000000..227e9ea9c6d --- /dev/null +++ b/app/services/after_branch_delete_service.rb @@ -0,0 +1,21 @@ +## +# Branch can be deleted either by DeleteBranchService +# or by GitPushService. +# +class AfterBranchDeleteService < BaseService + attr_reader :branch_name + + def execute(branch_name) + @branch_name = branch_name + + stop_environments + end + + private + + def stop_environments + Ci::StopEnvironmentsService + .new(project, current_user) + .execute(branch_name) + end +end diff --git a/app/services/akismet_service.rb b/app/services/akismet_service.rb index 5c60addbe7c..76b9f1feda7 100644 --- a/app/services/akismet_service.rb +++ b/app/services/akismet_service.rb @@ -29,25 +29,25 @@ class AkismetService end def submit_ham - return false unless akismet_enabled? + submit(:ham) + end - params = { - type: 'comment', - text: text, - author: owner.name, - author_email: owner.email - } + def submit_spam + submit(:spam) + end - begin - akismet_client.submit_ham(options[:ip_address], options[:user_agent], params) - true - rescue => e - Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") - false - end + private + + def akismet_client + @akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key, + Gitlab.config.gitlab.url) end - def submit_spam + def akismet_enabled? + current_application_settings.akismet_enabled + end + + def submit(type) return false unless akismet_enabled? params = { @@ -58,22 +58,11 @@ class AkismetService } begin - akismet_client.submit_spam(options[:ip_address], options[:user_agent], params) + akismet_client.public_send(type, options[:ip_address], options[:user_agent], params) true rescue => e Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") false end end - - private - - def akismet_client - @akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key, - Gitlab.config.gitlab.url) - end - - def akismet_enabled? - current_application_settings.akismet_enabled - end end diff --git a/app/services/auth/container_registry_authentication_service.rb b/app/services/auth/container_registry_authentication_service.rb index 6072123b851..c00c5aebf57 100644 --- a/app/services/auth/container_registry_authentication_service.rb +++ b/app/services/auth/container_registry_authentication_service.rb @@ -4,11 +4,13 @@ module Auth AUDIENCE = 'container_registry' - def execute - return error('not found', 404) unless registry.enabled + def execute(authentication_abilities:) + @authentication_abilities = authentication_abilities - unless current_user || project - return error('forbidden', 403) unless scope + return error('UNAVAILABLE', status: 404, message: 'registry not enabled') unless registry.enabled + + unless scope || current_user || project + return error('DENIED', status: 403, message: 'access forbidden') end { token: authorized_token(scope).encoded } @@ -74,9 +76,9 @@ module Auth case requested_action when 'pull' - requested_project == project || can?(current_user, :read_container_image, requested_project) + build_can_pull?(requested_project) || user_can_pull?(requested_project) when 'push' - requested_project == project || can?(current_user, :create_container_image, requested_project) + build_can_push?(requested_project) || user_can_push?(requested_project) else false end @@ -85,5 +87,40 @@ module Auth def registry Gitlab.config.registry end + + def build_can_pull?(requested_project) + # Build can: + # 1. pull from its own project (for ex. a build) + # 2. read images from dependent projects if creator of build is a team member + has_authentication_ability?(:build_read_container_image) && + (requested_project == project || can?(current_user, :build_read_container_image, requested_project)) + end + + def user_can_pull?(requested_project) + has_authentication_ability?(:read_container_image) && + can?(current_user, :read_container_image, requested_project) + end + + def build_can_push?(requested_project) + # Build can push only to the project from which it originates + has_authentication_ability?(:build_create_container_image) && + requested_project == project + end + + def user_can_push?(requested_project) + has_authentication_ability?(:create_container_image) && + can?(current_user, :create_container_image, requested_project) + end + + def error(code, status:, message: '') + { + errors: [{ code: code, message: message }], + http_status: status + } + end + + def has_authentication_ability?(capability) + (@authentication_abilities || []).include?(capability) + end end end diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 0c208150fb8..1a2bad77a02 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -56,9 +56,8 @@ class BaseService result end - def success - { - status: :success - } + def success(pass_back = {}) + pass_back[:status] = :success + pass_back end end diff --git a/app/services/boards/base_service.rb b/app/services/boards/base_service.rb deleted file mode 100644 index b2069ca825a..00000000000 --- a/app/services/boards/base_service.rb +++ /dev/null @@ -1,5 +0,0 @@ -module Boards - class BaseService < ::BaseService - delegate :board, to: :project - end -end diff --git a/app/services/boards/create_service.rb b/app/services/boards/create_service.rb index 072a0749285..9bdd7b6f0cf 100644 --- a/app/services/boards/create_service.rb +++ b/app/services/boards/create_service.rb @@ -1,16 +1,21 @@ module Boards - class CreateService < Boards::BaseService + class CreateService < BaseService def execute - create_board! unless project.board.present? - project.board + if project.boards.empty? + create_board! + else + project.boards.first + end end private def create_board! - project.create_board - project.board.lists.create(list_type: :backlog) - project.board.lists.create(list_type: :done) + board = project.boards.create + board.lists.create(list_type: :backlog) + board.lists.create(list_type: :done) + + board end end end diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb new file mode 100644 index 00000000000..c0d7ff5b585 --- /dev/null +++ b/app/services/boards/issues/create_service.rb @@ -0,0 +1,23 @@ +module Boards + module Issues + class CreateService < BaseService + def execute + create_issue(params.merge(label_ids: [list.label_id])) + end + + private + + def board + @board ||= project.boards.find(params.delete(:board_id)) + end + + def list + @list ||= board.lists.find(params.delete(:list_id)) + end + + def create_issue(params) + ::Issues::CreateService.new(project, current_user, params).execute + end + end + end +end diff --git a/app/services/boards/issues/list_service.rb b/app/services/boards/issues/list_service.rb index 34efd09ed9f..fd4a462c7b2 100644 --- a/app/services/boards/issues/list_service.rb +++ b/app/services/boards/issues/list_service.rb @@ -1,6 +1,6 @@ module Boards module Issues - class ListService < Boards::BaseService + class ListService < BaseService def execute issues = IssuesFinder.new(current_user, filter_params).execute issues = without_board_labels(issues) unless list.movable? @@ -10,6 +10,10 @@ module Boards private + def board + @board ||= project.boards.find(params[:board_id]) + end + def list @list ||= board.lists.find(params[:id]) end @@ -36,12 +40,7 @@ module Boards end def set_state - params[:state] = - case list.list_type.to_sym - when :backlog then 'opened' - when :done then 'closed' - else 'all' - end + params[:state] = list.done? ? 'closed' : 'opened' end def board_label_ids diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 84dc3f70e76..96554a92a02 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -1,6 +1,6 @@ module Boards module Issues - class MoveService < Boards::BaseService + class MoveService < BaseService def execute(issue) return false unless can?(current_user, :update_issue, issue) return false unless valid_move? @@ -10,6 +10,10 @@ module Boards private + def board + @board ||= project.boards.find(params[:board_id]) + end + def valid_move? moving_from_list.present? && moving_to_list.present? && moving_from_list != moving_to_list @@ -49,7 +53,7 @@ module Boards if moving_to_list.movable? moving_from_list.label_id else - board.lists.movable.pluck(:label_id) + project.boards.joins(:lists).merge(List.movable).pluck(:label_id) end Array(label_ids).compact diff --git a/app/services/boards/list_service.rb b/app/services/boards/list_service.rb new file mode 100644 index 00000000000..84f1fc3a4e2 --- /dev/null +++ b/app/services/boards/list_service.rb @@ -0,0 +1,14 @@ +module Boards + class ListService < BaseService + def execute + create_board! if project.boards.empty? + project.boards + end + + private + + def create_board! + Boards::CreateService.new(project, current_user).execute + end + end +end diff --git a/app/services/boards/lists/create_service.rb b/app/services/boards/lists/create_service.rb index b1887820bd4..fe0d762ccd2 100644 --- a/app/services/boards/lists/create_service.rb +++ b/app/services/boards/lists/create_service.rb @@ -1,23 +1,27 @@ module Boards module Lists - class CreateService < Boards::BaseService - def execute + class CreateService < BaseService + def execute(board) List.transaction do - label = project.labels.find(params[:label_id]) - position = next_position + label = available_labels.find(params[:label_id]) + position = next_position(board) - create_list(label, position) + create_list(board, label, position) end end private - def next_position + def available_labels + LabelsFinder.new(current_user, project_id: project.id).execute + end + + def next_position(board) max_position = board.lists.movable.maximum(:position) max_position.nil? ? 0 : max_position.succ end - def create_list(label, position) + def create_list(board, label, position) board.lists.create(label: label, list_type: :label, position: position) end end diff --git a/app/services/boards/lists/destroy_service.rb b/app/services/boards/lists/destroy_service.rb index 25da3bfb56d..f986e05944c 100644 --- a/app/services/boards/lists/destroy_service.rb +++ b/app/services/boards/lists/destroy_service.rb @@ -1,9 +1,11 @@ module Boards module Lists - class DestroyService < Boards::BaseService + class DestroyService < BaseService def execute(list) return false unless list.destroyable? + @board = list.board + list.with_lock do decrement_higher_lists(list) remove_list(list) @@ -12,6 +14,8 @@ module Boards private + attr_reader :board + def decrement_higher_lists(list) board.lists.movable.where('position > ?', list.position) .update_all('position = position - 1') diff --git a/app/services/boards/lists/generate_service.rb b/app/services/boards/lists/generate_service.rb index 1c48b9786e4..939f9bfd068 100644 --- a/app/services/boards/lists/generate_service.rb +++ b/app/services/boards/lists/generate_service.rb @@ -1,11 +1,11 @@ module Boards module Lists - class GenerateService < Boards::BaseService - def execute + class GenerateService < BaseService + def execute(board) return false unless board.lists.movable.empty? List.transaction do - label_params.each { |params| create_list(params) } + label_params.each { |params| create_list(board, params) } end true @@ -13,22 +13,19 @@ module Boards private - def create_list(params) + def create_list(board, params) label = find_or_create_label(params) - Lists::CreateService.new(project, current_user, label_id: label.id).execute + Lists::CreateService.new(project, current_user, label_id: label.id).execute(board) end def find_or_create_label(params) - project.labels.create_with(color: params[:color]) - .find_or_create_by(name: params[:name]) + ::Labels::FindOrCreateService.new(current_user, project, params).execute end def label_params [ - { name: 'Development', color: '#5CB85C' }, - { name: 'Testing', color: '#F0AD4E' }, - { name: 'Production', color: '#FF5F00' }, - { name: 'Ready', color: '#FF0000' } + { name: 'To Do', color: '#F0AD4E' }, + { name: 'Doing', color: '#5CB85C' } ] end end diff --git a/app/services/boards/lists/list_service.rb b/app/services/boards/lists/list_service.rb new file mode 100644 index 00000000000..c579ed4c869 --- /dev/null +++ b/app/services/boards/lists/list_service.rb @@ -0,0 +1,9 @@ +module Boards + module Lists + class ListService < BaseService + def execute(board) + board.lists + end + end + end +end diff --git a/app/services/boards/lists/move_service.rb b/app/services/boards/lists/move_service.rb index 020ff69f4a7..f2a68865f7b 100644 --- a/app/services/boards/lists/move_service.rb +++ b/app/services/boards/lists/move_service.rb @@ -1,7 +1,8 @@ module Boards module Lists - class MoveService < Boards::BaseService + class MoveService < BaseService def execute(list) + @board = list.board @old_position = list.position @new_position = params[:position] @@ -16,7 +17,7 @@ module Boards private - attr_reader :old_position, :new_position + attr_reader :board, :old_position, :new_position def valid_move? new_position.present? && new_position != old_position && diff --git a/app/services/chat_names/authorize_user_service.rb b/app/services/chat_names/authorize_user_service.rb new file mode 100644 index 00000000000..321bf3a9205 --- /dev/null +++ b/app/services/chat_names/authorize_user_service.rb @@ -0,0 +1,38 @@ +module ChatNames + class AuthorizeUserService + include Gitlab::Routing.url_helpers + + def initialize(service, params) + @service = service + @params = params + end + + def execute + return unless chat_name_params.values.all?(&:present?) + + token = request_token + + new_profile_chat_name_url(token: token) if token + end + + private + + def request_token + chat_name_token.store!(chat_name_params) + end + + def chat_name_token + Gitlab::ChatNameToken.new + end + + def chat_name_params + { + service_id: @service.id, + team_id: @params[:team_id], + team_domain: @params[:team_domain], + chat_id: @params[:user_id], + chat_name: @params[:user_name] + } + end + end +end diff --git a/app/services/chat_names/find_user_service.rb b/app/services/chat_names/find_user_service.rb new file mode 100644 index 00000000000..4f5c5567b42 --- /dev/null +++ b/app/services/chat_names/find_user_service.rb @@ -0,0 +1,26 @@ +module ChatNames + class FindUserService + def initialize(service, params) + @service = service + @params = params + end + + def execute + chat_name = find_chat_name + return unless chat_name + + chat_name.touch(:last_used_at) + chat_name.user + end + + private + + def find_chat_name + ChatName.find_by( + service: @service, + team_id: @params[:team_id], + chat_id: @params[:user_id] + ) + end + end +end diff --git a/app/services/ci/process_pipeline_service.rb b/app/services/ci/process_pipeline_service.rb index 36c93dddadb..8face432d97 100644 --- a/app/services/ci/process_pipeline_service.rb +++ b/app/services/ci/process_pipeline_service.rb @@ -10,15 +10,14 @@ module Ci create_builds! end - @pipeline.with_lock do - new_builds = - stage_indexes_of_created_builds.map do |index| - process_stage(index) - end + new_builds = + stage_indexes_of_created_builds.map do |index| + process_stage(index) + end - # Return a flag if a when builds got enqueued - new_builds.flatten.any? - end + @pipeline.update_status + + new_builds.flatten.any? end private @@ -30,9 +29,11 @@ module Ci def process_stage(index) current_status = status_for_prior_stages(index) - created_builds_in_stage(index).select do |build| - if HasStatus::COMPLETED_STATUSES.include?(current_status) - process_build(build, current_status) + if HasStatus::COMPLETED_STATUSES.include?(current_status) + created_builds_in_stage(index).select do |build| + Gitlab::OptimisticLocking.retry_lock(build) do |subject| + process_build(subject, current_status) + end end end end diff --git a/app/services/ci/register_build_service.rb b/app/services/ci/register_build_service.rb index 6973191b203..74b5ebf372b 100644 --- a/app/services/ci/register_build_service.rb +++ b/app/services/ci/register_build_service.rb @@ -28,17 +28,14 @@ module Ci if build # In case when 2 runners try to assign the same build, second runner will be declined - # with StateMachines::InvalidTransition in run! method. - build.with_lock do - build.runner_id = current_runner.id - build.save! - build.run! - end + # with StateMachines::InvalidTransition or StaleObjectError when doing run! or save method. + build.runner_id = current_runner.id + build.run! end build - rescue StateMachines::InvalidTransition + rescue StateMachines::InvalidTransition, ActiveRecord::StaleObjectError nil end diff --git a/app/services/ci/stop_environments_service.rb b/app/services/ci/stop_environments_service.rb new file mode 100644 index 00000000000..cf590459cb2 --- /dev/null +++ b/app/services/ci/stop_environments_service.rb @@ -0,0 +1,29 @@ +module Ci + class StopEnvironmentsService < BaseService + attr_reader :ref + + def execute(branch_name) + @ref = branch_name + + return unless has_ref? + + environments.each do |environment| + next unless environment.stoppable? + next unless can?(current_user, :create_deployment, project) + + environment.stop!(current_user) + end + end + + private + + def has_ref? + @ref.present? + end + + def environments + @environments ||= project + .environments_recently_updated_on_branch(@ref) + end + end +end diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 6d6075628af..5e8fafca98c 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -3,7 +3,7 @@ require 'securerandom' # Compare 2 branches for one repo or between repositories # and return Gitlab::Git::Compare object that responds to commits and diffs class CompareService - def execute(source_project, source_branch, target_project, target_branch) + def execute(source_project, source_branch, target_project, target_branch, straight: false) source_commit = source_project.commit(source_branch) return unless source_commit @@ -23,9 +23,10 @@ class CompareService raw_compare = Gitlab::Git::Compare.new( target_project.repository.raw_repository, target_branch, - source_sha + source_sha, + straight ) - Compare.new(raw_compare, target_project) + Compare.new(raw_compare, target_project, straight: straight) end end diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index 757fc35a78f..e004a303496 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class CreateBranchService < BaseService def execute(branch_name, ref, source_project: @project) valid_branch = Gitlab::GitRefValidator.validate(branch_name) diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index efeb9df9527..47f9b2c621c 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -1,18 +1,71 @@ -require_relative 'base_service' - class CreateDeploymentService < BaseService def execute(deployable = nil) - environment = project.environments.find_or_create_by( - name: params[:environment] - ) + return unless executable? + + ActiveRecord::Base.transaction do + @deployable = deployable + + @environment = environment + @environment.external_url = expanded_url if expanded_url + @environment.fire_state_event(action) + + return unless @environment.save + return if @environment.stopped? + + deploy.tap do |deployment| + deployment.update_merge_request_metrics! + end + end + end + + private + def executable? + project && name.present? + end + + def deploy project.deployments.create( - environment: environment, + environment: @environment, ref: params[:ref], tag: params[:tag], sha: params[:sha], user: current_user, - deployable: deployable - ) + deployable: @deployable, + on_stop: options[:on_stop]) + end + + def environment + @environment ||= project.environments.find_or_create_by(name: expanded_name) + end + + def expanded_name + ExpandVariables.expand(name, variables) + end + + def expanded_url + return unless url + + @expanded_url ||= ExpandVariables.expand(url, variables) + end + + def name + params[:environment] + end + + def url + options[:url] + end + + def options + params[:options] || {} + end + + def variables + params[:variables] || [] + end + + def action + options[:action] || 'start' end end diff --git a/app/services/create_release_service.rb b/app/services/create_release_service.rb index d6d4afcf29a..54ff1f74126 100644 --- a/app/services/create_release_service.rb +++ b/app/services/create_release_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class CreateReleaseService < BaseService def execute(tag_name, release_description) repository = project.repository diff --git a/app/services/create_tag_service.rb b/app/services/create_tag_service.rb index c0e7ecf6a96..fe9353afeb8 100644 --- a/app/services/create_tag_service.rb +++ b/app/services/create_tag_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class CreateTagService < BaseService def execute(tag_name, target, message, release_description = nil) valid_tag = Gitlab::GitRefValidator.validate(tag_name) diff --git a/app/services/delete_branch_service.rb b/app/services/delete_branch_service.rb index 918eddaa53a..11a045f4c31 100644 --- a/app/services/delete_branch_service.rb +++ b/app/services/delete_branch_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class DeleteBranchService < BaseService def execute(branch_name) repository = project.repository @@ -42,7 +40,7 @@ class DeleteBranchService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - branch.target.sha, + branch.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch.name}", []) diff --git a/app/services/delete_merged_branches_service.rb b/app/services/delete_merged_branches_service.rb new file mode 100644 index 00000000000..1b5623baebe --- /dev/null +++ b/app/services/delete_merged_branches_service.rb @@ -0,0 +1,16 @@ +class DeleteMergedBranchesService < BaseService + def async_execute + DeleteMergedBranchesWorker.perform_async(project.id, current_user.id) + end + + def execute + raise Gitlab::Access::AccessDeniedError unless can?(current_user, :push_code, project) + + branches = project.repository.branch_names + branches = branches.select { |branch| project.repository.merged_to_root_ref?(branch) } + + branches.each do |branch| + DeleteBranchService.new(project, current_user).execute(branch) + end + end +end diff --git a/app/services/delete_tag_service.rb b/app/services/delete_tag_service.rb index d0cb151a010..a44dee14a0f 100644 --- a/app/services/delete_tag_service.rb +++ b/app/services/delete_tag_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class DeleteTagService < BaseService def execute(tag_name) repository = project.repository @@ -36,7 +34,7 @@ class DeleteTagService < BaseService Gitlab::DataBuilder::Push.build( project, current_user, - tag.target.sha, + tag.dereferenced_target.sha, Gitlab::Git::BLANK_SHA, "#{Gitlab::Git::TAG_REF_PREFIX}#{tag.name}", []) diff --git a/app/services/destroy_group_service.rb b/app/services/destroy_group_service.rb index 0081364b8aa..a880952e274 100644 --- a/app/services/destroy_group_service.rb +++ b/app/services/destroy_group_service.rb @@ -6,12 +6,10 @@ class DestroyGroupService end def async_execute - group.transaction do - # Soft delete via paranoia gem - group.destroy - 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 + # Soft delete via paranoia gem + group.destroy + 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 def execute diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 07fc77001a5..e24cc66e0fe 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -62,6 +62,10 @@ class EventCreateService create_event(project, current_user, Event::LEFT) end + def expired_leave_project(project, current_user) + create_event(project, current_user, Event::EXPIRED) + end + def create_project(project, current_user) create_event(project, current_user, Event::CREATED) end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index ea94818713b..9bd4bd464f7 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -16,6 +16,8 @@ module Files params[:file_content] end @last_commit_sha = params[:last_commit_sha] + @author_email = params[:author_email] + @author_name = params[:author_name] # Validate parameters validate @@ -25,8 +27,9 @@ module Files create_target_branch end - if commit - success + result = commit + if result + success(result: result) else error('Something went wrong. Your changes were not committed') end @@ -40,6 +43,12 @@ module Files @source_branch != @target_branch || @source_project != @project end + def file_has_changed? + return false unless @last_commit_sha && last_commit + + @last_commit_sha != last_commit.sha + end + def raise_error(message) raise ValidationError.new(message) end diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb index 6107254a34e..e5b4d60e467 100644 --- a/app/services/files/create_dir_service.rb +++ b/app/services/files/create_dir_service.rb @@ -1,9 +1,7 @@ -require_relative "base_service" - module Files class CreateDirService < Files::BaseService def commit - repository.commit_dir(current_user, @file_path, @commit_message, @target_branch) + repository.commit_dir(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name) end def validate diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 8eaf6db8012..b23576b9a28 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,9 +1,7 @@ -require_relative "base_service" - module Files class CreateService < Files::BaseService def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false) + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false, author_email: @author_email, author_name: @author_name) end def validate diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 27c881c3430..4f7e7a5baaa 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -1,9 +1,7 @@ -require_relative "base_service" - module Files class DeleteService < Files::BaseService def commit - repository.remove_file(current_user, @file_path, @commit_message, @target_branch) + repository.remove_file(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name) end end end diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb new file mode 100644 index 00000000000..54446e90007 --- /dev/null +++ b/app/services/files/multi_service.rb @@ -0,0 +1,122 @@ +module Files + class MultiService < Files::BaseService + class FileChangedError < StandardError; end + + def commit + repository.multi_action( + user: current_user, + branch: @target_branch, + message: @commit_message, + actions: params[:actions], + author_email: @author_email, + author_name: @author_name + ) + end + + private + + def validate + super + + params[:actions].each_with_index do |action, index| + unless action[:file_path].present? + raise_error("You must specify a file_path.") + end + + regex_check(action[:file_path]) + regex_check(action[:previous_path]) if action[:previous_path] + + if project.empty_repo? && action[:action] != :create + raise_error("No files to #{action[:action]}.") + end + + validate_file_exists(action) + + case action[:action] + when :create + validate_create(action) + when :update + validate_update(action) + when :delete + validate_delete(action) + when :move + validate_move(action, index) + else + raise_error("Unknown action type `#{action[:action]}`.") + end + end + end + + def validate_file_exists(action) + return if action[:action] == :create + + file_path = action[:file_path] + file_path = action[:previous_path] if action[:action] == :move + + blob = repository.blob_at_branch(params[:branch_name], file_path) + + unless blob + raise_error("File to be #{action[:action]}d `#{file_path}` does not exist.") + end + end + + def last_commit + Gitlab::Git::Commit.last_for_path(repository, @source_branch, @file_path) + end + + def regex_check(file) + if file =~ Gitlab::Regex.directory_traversal_regex + raise_error( + 'Your changes could not be committed, because the file name, `' + + file + + '` ' + + Gitlab::Regex.directory_traversal_regex_message + ) + end + + unless file =~ Gitlab::Regex.file_path_regex + raise_error( + 'Your changes could not be committed, because the file name, `' + + file + + '` ' + + Gitlab::Regex.file_path_regex_message + ) + end + end + + def validate_create(action) + return if project.empty_repo? + + if repository.blob_at_branch(params[:branch_name], action[:file_path]) + raise_error("Your changes could not be committed because a file with the name `#{action[:file_path]}` already exists.") + end + end + + def validate_delete(action) + end + + def validate_move(action, index) + if action[:previous_path].nil? + raise_error("You must supply the original file path when moving file `#{action[:file_path]}`.") + end + + blob = repository.blob_at_branch(params[:branch_name], action[:file_path]) + + if blob + raise_error("Move destination `#{action[:file_path]}` already exists.") + end + + if action[:content].nil? + blob = repository.blob_at_branch(params[:branch_name], action[:previous_path]) + blob.load_all_data!(repository) if blob.truncated? + 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 4fc3b640799..47a18e3e132 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -1,5 +1,3 @@ -require_relative "base_service" - module Files class UpdateService < Files::BaseService class FileChangedError < StandardError; end @@ -8,7 +6,9 @@ module Files repository.update_file(current_user, @file_path, @file_content, branch: @target_branch, previous_path: @previous_path, - message: @commit_message) + message: @commit_message, + author_email: @author_email, + author_name: @author_name) end private @@ -21,12 +21,6 @@ module Files end end - def file_has_changed? - return false unless @last_commit_sha && last_commit - - @last_commit_sha != last_commit.sha - end - def last_commit @last_commit ||= Gitlab::Git::Commit. last_for_path(@source_project.repository, @source_branch, @file_path) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 78feb37aa2a..647930d555c 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -18,7 +18,7 @@ class GitPushService < BaseService # def execute @project.repository.after_create if @project.empty_repo? - @project.repository.after_push_commit(branch_name, params[:newrev]) + @project.repository.after_push_commit(branch_name) if push_remove_branch? @project.repository.after_remove_branch @@ -49,28 +49,53 @@ class GitPushService < BaseService update_gitattributes if is_default_branch? end - # Update merge requests that may be affected by this push. A new branch - # could cause the last commit of a merge request to change. - update_merge_requests - + execute_related_hooks perform_housekeeping + + update_caches end def update_gitattributes @project.repository.copy_gitattributes(params[:ref]) end + def update_caches + if is_default_branch? + paths = Set.new + + @push_commits.each do |commit| + commit.raw_diffs(deltas_only: true).each do |diff| + paths << diff.new_path + end + end + + types = Gitlab::FileDetector.types_in_paths(paths.to_a) + else + types = [] + end + + ProjectCacheWorker.perform_async(@project.id, types) + end + protected - def update_merge_requests - @project.update_merge_requests(params[:oldrev], params[:newrev], params[:ref], current_user) + def execute_related_hooks + # Update merge requests that may be affected by this push. A new branch + # could cause the last commit of a merge request to change. + # + UpdateMergeRequestsWorker + .perform_async(@project.id, current_user.id, params[:oldrev], params[:newrev], params[:ref]) EventCreateService.new.push(@project, current_user, build_push_data) - SystemHooksService.new.execute_hooks(build_push_data_system_hook.dup, :push_hooks) @project.execute_hooks(build_push_data.dup, :push_hooks) @project.execute_services(build_push_data.dup, :push_hooks) - Ci::CreatePipelineService.new(project, current_user, build_push_data).execute - ProjectCacheWorker.perform_async(@project.id) + Ci::CreatePipelineService.new(@project, current_user, build_push_data).execute + + if push_remove_branch? + AfterBranchDeleteService + .new(project, current_user) + .execute(branch_name) + end end def perform_housekeeping @@ -87,7 +112,7 @@ class GitPushService < BaseService project.change_head(branch_name) # Set protection on the default branch if configured - if current_application_settings.default_branch_protection != PROTECTION_NONE + if current_application_settings.default_branch_protection != PROTECTION_NONE && !@project.protected_branch?(@project.default_branch) params = { name: @project.default_branch, @@ -106,34 +131,11 @@ class GitPushService < BaseService # Extract any GFM references from the pushed commit messages. If the configured issue-closing regex is matched, # close the referenced Issue. Create cross-reference Notes corresponding to any other referenced Mentionables. def process_commit_messages - is_default_branch = is_default_branch? - - authors = Hash.new do |hash, commit| - email = commit.author_email - next hash[email] if hash.has_key?(email) - - hash[email] = commit_user(commit) - end + default = is_default_branch? @push_commits.each do |commit| - # Keep track of the issues that will be actually closed because they are on a default branch. - # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches) - # will also have cross-reference. - closed_issues = [] - - if is_default_branch - # Close issues if these commits were pushed to the project's default branch and the commit message matches the - # closing regex. Exclude any mentioned Issues from cross-referencing even if the commits are being pushed to - # a different branch. - closed_issues = commit.closes_issues(current_user) - closed_issues.each do |issue| - if can?(current_user, :update_issue, issue) - Issues::CloseService.new(project, authors[commit], {}).execute(issue, commit: commit) - end - end - end - - commit.create_cross_references!(authors[commit], closed_issues) + ProcessCommitWorker. + perform_async(project.id, current_user.id, commit.id, default) end end @@ -147,16 +149,6 @@ class GitPushService < BaseService push_commits) end - def build_push_data_system_hook - @push_data_system ||= Gitlab::DataBuilder::Push.build( - @project, - current_user, - params[:oldrev], - params[:newrev], - params[:ref], - []) - end - def push_to_existing_branch? # Return if this is not a push to a branch (e.g. new commits) Gitlab::Git.branch_ref?(params[:ref]) && !Gitlab::Git.blank_ref?(params[:oldrev]) diff --git a/app/services/git_tag_push_service.rb b/app/services/git_tag_push_service.rb index e6002b03b93..20a4445bddf 100644 --- a/app/services/git_tag_push_service.rb +++ b/app/services/git_tag_push_service.rb @@ -27,8 +27,8 @@ class GitTagPushService < BaseService tag_name = Gitlab::Git.ref_name(params[:ref]) tag = project.repository.find_tag(tag_name) - if tag && tag.object_sha == params[:newrev] - commit = project.commit(tag.target) + if tag && tag.target == params[:newrev] + commit = project.commit(tag.dereferenced_target) commits = [commit].compact message = tag.message end diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 4c8d93999a7..575795788de 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -50,6 +50,7 @@ class IssuableBaseService < BaseService params.delete(:remove_label_ids) params.delete(:label_ids) params.delete(:assignee_id) + params.delete(:due_date) end end @@ -79,17 +80,18 @@ class IssuableBaseService < BaseService def filter_labels_in_param(key) return if params[key].to_a.empty? - params[key] = project.labels.where(id: params[key]).pluck(:id) + params[key] = available_labels.where(id: params[key]).pluck(:id) end def find_or_create_label_ids labels = params.delete(:labels) return unless labels - params[:label_ids] = labels.split(",").map do |label_name| - project.labels.create_with(color: Label::DEFAULT_COLOR) - .find_or_create_by(title: label_name.strip) - .id + params[:label_ids] = labels.split(',').map do |label_name| + service = Labels::FindOrCreateService.new(current_user, project, title: label_name.strip) + label = service.execute + + label.id end end @@ -110,6 +112,10 @@ class IssuableBaseService < BaseService new_label_ids end + def available_labels + LabelsFinder.new(current_user, project_id: @project.id).execute + end + def merge_slash_commands_into_params!(issuable) description, command_params = SlashCommands::InterpretService.new(project, current_user). @@ -157,6 +163,10 @@ class IssuableBaseService < BaseService # To be overridden by subclasses end + def after_update(issuable) + # To be overridden by subclasses + end + def update_issuable(issuable, attributes) issuable.with_transaction_returning_status do issuable.update(attributes.merge(updated_by: current_user)) @@ -182,6 +192,7 @@ class IssuableBaseService < BaseService end handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users) + after_update(issuable) issuable.create_new_cross_references!(current_user) execute_hooks(issuable, 'update') end @@ -201,9 +212,9 @@ class IssuableBaseService < BaseService def change_subscription(issuable) case params.delete(:subscription_event) when 'subscribe' - issuable.subscribe(current_user) + issuable.subscribe(current_user, project) when 'unsubscribe' - issuable.unsubscribe(current_user) + issuable.unsubscribe(current_user, project) end end diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 45cca216ccc..ab4c51386a4 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -1,8 +1,21 @@ module Issues class CloseService < Issues::BaseService + # Closes the supplied issue if the current user is able to do so. def execute(issue, commit: nil, notifications: true, system_note: true) return issue unless can?(current_user, :update_issue, issue) + close_issue(issue, + commit: commit, + notifications: notifications, + system_note: system_note) + end + + # Closes the supplied issue without checking if the user is authorized to + # do so. + # + # The code calling this method is responsible for ensuring that a user is + # allowed to close the given issue. + def close_issue(issue, commit: nil, notifications: true, system_note: true) if project.jira_tracker? && project.jira_service.active project.jira_service.execute(commit, issue) todo_service.close_issue(issue, current_user) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index ab667456db7..a2a5f57d069 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -52,8 +52,12 @@ module Issues end def cloneable_label_ids - @new_project.labels - .where(title: @old_issue.labels.pluck(:title)).pluck(:id) + params = { + project_id: @new_project.id, + title: @old_issue.labels.pluck(:title) + } + + LabelsFinder.new(current_user, params).execute.pluck(:id) end def cloneable_milestone_id diff --git a/app/services/labels/find_or_create_service.rb b/app/services/labels/find_or_create_service.rb new file mode 100644 index 00000000000..d622f9edd33 --- /dev/null +++ b/app/services/labels/find_or_create_service.rb @@ -0,0 +1,36 @@ +module Labels + class FindOrCreateService + def initialize(current_user, project, params = {}) + @current_user = current_user + @project = project + @params = params.dup + end + + def execute(skip_authorization: false) + @skip_authorization = skip_authorization + find_or_create_label + end + + private + + attr_reader :current_user, :project, :params, :skip_authorization + + def available_labels + @available_labels ||= LabelsFinder.new( + current_user, + project_id: project.id + ).execute(skip_authorization: skip_authorization) + end + + def find_or_create_label + new_label = available_labels.find_by(title: title) + new_label ||= project.labels.create(params) + + new_label + end + + def title + params[:title] || params[:name] + end + end +end diff --git a/app/services/labels/transfer_service.rb b/app/services/labels/transfer_service.rb new file mode 100644 index 00000000000..514679ed29d --- /dev/null +++ b/app/services/labels/transfer_service.rb @@ -0,0 +1,78 @@ +# Labels::TransferService class +# +# User for recreate the missing group labels at project level +# +module Labels + class TransferService + def initialize(current_user, old_group, project) + @current_user = current_user + @old_group = old_group + @project = project + end + + def execute + return unless old_group.present? + + Label.transaction do + labels_to_transfer.find_each do |label| + new_label_id = find_or_create_label!(label) + + next if new_label_id == label.id + + update_label_links(group_labels_applied_to_issues, old_label_id: label.id, new_label_id: new_label_id) + update_label_links(group_labels_applied_to_merge_requests, old_label_id: label.id, new_label_id: new_label_id) + update_label_priorities(old_label_id: label.id, new_label_id: new_label_id) + end + end + end + + private + + attr_reader :current_user, :old_group, :project + + def labels_to_transfer + label_ids = [] + label_ids << group_labels_applied_to_issues.select(:id) + label_ids << group_labels_applied_to_merge_requests.select(:id) + + union = Gitlab::SQL::Union.new(label_ids) + + Label.where("labels.id IN (#{union.to_sql})").reorder(nil).uniq + end + + def group_labels_applied_to_issues + Label.joins(:issues). + where( + issues: { project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) + end + + def group_labels_applied_to_merge_requests + Label.joins(:merge_requests). + where( + merge_requests: { target_project_id: project.id }, + labels: { type: 'GroupLabel', group_id: old_group.id } + ) + end + + def find_or_create_label!(label) + params = label.attributes.slice('title', 'description', 'color') + new_label = FindOrCreateService.new(current_user, project, params).execute + + new_label.id + end + + def update_label_links(labels, old_label_id:, new_label_id:) + LabelLink.joins(:label). + merge(labels). + where(label_id: old_label_id). + update_all(label_id: new_label_id) + end + + def update_label_priorities(old_label_id:, new_label_id:) + LabelPriority.where(project_id: project.id, label_id: old_label_id). + update_all(label_id: new_label_id) + end + end +end diff --git a/app/services/members/approve_access_request_service.rb b/app/services/members/approve_access_request_service.rb new file mode 100644 index 00000000000..c13f289f61e --- /dev/null +++ b/app/services/members/approve_access_request_service.rb @@ -0,0 +1,42 @@ +module Members + class ApproveAccessRequestService < BaseService + include MembersHelper + + attr_accessor :source + + # source - The source object that respond to `#requesters` (i.g. project or group) + # current_user - The user that performs the access request approval + # params - A hash of parameters + # :user_id - User ID used to retrieve the access requester + # :id - Member ID used to retrieve the access requester + # :access_level - Optional access level set when the request is accepted + def initialize(source, current_user, params = {}) + @source = source + @current_user = current_user + @params = params.slice(:user_id, :id, :access_level) + end + + # opts - A hash of options + # :force - Bypass permission check: current_user can be nil in that case + def execute(opts = {}) + condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] } + access_requester = source.requesters.find_by!(condition) + + raise Gitlab::Access::AccessDeniedError unless can_update_access_requester?(access_requester, opts) + + access_requester.access_level = params[:access_level] if params[:access_level] + access_requester.accept_request + + access_requester + end + + private + + def can_update_access_requester?(access_requester, opts = {}) + access_requester && ( + opts[:force] || + can?(current_user, action_member_permission(:update, access_requester), access_requester) + ) + end + end +end diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb index ca9db59cac7..b7a244c2029 100644 --- a/app/services/members/authorized_destroy_service.rb +++ b/app/services/members/authorized_destroy_service.rb @@ -14,6 +14,8 @@ module Members if member.request? && member.user != user notification_service.decline_access_request(member) end + + member end end end diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb new file mode 100644 index 00000000000..e4b24ccef92 --- /dev/null +++ b/app/services/members/create_service.rb @@ -0,0 +1,16 @@ +module Members + class CreateService < BaseService + def execute + return false if params[:user_ids].blank? + + project.team.add_users( + params[:user_ids].split(','), + params[:access_level], + expires_at: params[:expires_at], + current_user: current_user + ) + + true + end + end +end diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb index 9a2bf82ef51..431da8372c9 100644 --- a/app/services/members/destroy_service.rb +++ b/app/services/members/destroy_service.rb @@ -1,17 +1,42 @@ module Members class DestroyService < BaseService - attr_accessor :member, :current_user + include MembersHelper - def initialize(member, current_user) - @member = member + attr_accessor :source + + ALLOWED_SCOPES = %i[members requesters all] + + def initialize(source, current_user, params = {}) + @source = source @current_user = current_user + @params = params end - def execute - unless member && can?(current_user, "destroy_#{member.type.underscore}".to_sym, member) - raise Gitlab::Access::AccessDeniedError - end + def execute(scope = :members) + raise "scope :#{scope} is not allowed!" unless ALLOWED_SCOPES.include?(scope) + + member = find_member!(scope) + + raise Gitlab::Access::AccessDeniedError unless can_destroy_member?(member) + AuthorizedDestroyService.new(member, current_user).execute end + + private + + def find_member!(scope) + condition = params[:user_id] ? { user_id: params[:user_id] } : { id: params[:id] } + case scope + when :all + source.members.find_by(condition) || + source.requesters.find_by!(condition) + else + source.public_send(scope).find_by!(condition) + end + end + + def can_destroy_member?(member) + member && can?(current_user, action_member_permission(:destroy, member), member) + end end end diff --git a/app/services/members/request_access_service.rb b/app/services/members/request_access_service.rb new file mode 100644 index 00000000000..2614153d900 --- /dev/null +++ b/app/services/members/request_access_service.rb @@ -0,0 +1,25 @@ +module Members + class RequestAccessService < BaseService + attr_accessor :source + + def initialize(source, current_user) + @source = source + @current_user = current_user + end + + def execute + raise Gitlab::Access::AccessDeniedError unless can_request_access?(source) + + source.members.create( + access_level: Gitlab::Access::DEVELOPER, + user: current_user, + requested_at: Time.now.utc) + end + + private + + def can_request_access?(source) + source && can?(current_user, :request_access, source) + end + end +end diff --git a/app/services/merge_requests/add_todo_when_build_fails_service.rb b/app/services/merge_requests/add_todo_when_build_fails_service.rb index 566049525cb..12a8415d9a5 100644 --- a/app/services/merge_requests/add_todo_when_build_fails_service.rb +++ b/app/services/merge_requests/add_todo_when_build_fails_service.rb @@ -1,15 +1,20 @@ module MergeRequests class AddTodoWhenBuildFailsService < MergeRequests::BaseService # Adds a todo to the parent merge_request when a CI build fails + # def execute(commit_status) - each_merge_request(commit_status) do |merge_request| + return if commit_status.allow_failure? + + commit_status_merge_requests(commit_status) do |merge_request| todo_service.merge_request_build_failed(merge_request) end end - # Closes any pending build failed todos for the parent MRs when a build is retried + # Closes any pending build failed todos for the parent MRs when a + # build is retried + # def close(commit_status) - each_merge_request(commit_status) do |merge_request| + commit_status_merge_requests(commit_status) do |merge_request| todo_service.merge_request_build_retried(merge_request) end end diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb new file mode 100644 index 00000000000..066efa1acc3 --- /dev/null +++ b/app/services/merge_requests/assign_issues_service.rb @@ -0,0 +1,35 @@ +module MergeRequests + class AssignIssuesService < BaseService + def assignable_issues + @assignable_issues ||= begin + if current_user == merge_request.author + closes_issues.select do |issue| + !issue.is_a?(ExternalIssue) && !issue.assignee_id? && can?(current_user, :admin_issue, issue) + end + else + [] + end + end + end + + def execute + assignable_issues.each do |issue| + Issues::UpdateService.new(issue.project, current_user, assignee_id: current_user.id).execute(issue) + end + + { + count: assignable_issues.count + } + end + + private + + def merge_request + params[:merge_request] + end + + def closes_issues + @closes_issues ||= params[:closes_issues] || merge_request.closes_issues(current_user) + end + end +end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index ba424b09463..58f69a41e14 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -5,16 +5,17 @@ module MergeRequests end def create_title_change_note(issuable, old_title) - removed_wip = old_title =~ MergeRequest::WIP_REGEX && !issuable.work_in_progress? - added_wip = old_title !~ MergeRequest::WIP_REGEX && issuable.work_in_progress? + removed_wip = MergeRequest.work_in_progress?(old_title) && !issuable.work_in_progress? + added_wip = !MergeRequest.work_in_progress?(old_title) && issuable.work_in_progress? + changed_title = MergeRequest.wipless_title(old_title) != issuable.wipless_title if removed_wip SystemNoteService.remove_merge_request_wip(issuable, issuable.project, current_user) elsif added_wip SystemNoteService.add_merge_request_wip(issuable, issuable.project, current_user) - else - super end + + super if changed_title end def hook_data(merge_request, action, oldrev = nil) @@ -41,28 +42,33 @@ module MergeRequests super(:merge_request) end - def merge_request_from(commit_status) - branches = commit_status.ref + def merge_requests_for(branch) + origin_merge_requests = @project.origin_merge_requests + .opened.where(source_branch: branch).to_a - # This is for ref-less builds - branches ||= @project.repository.branch_names_contains(commit_status.sha) + fork_merge_requests = @project.fork_merge_requests + .opened.where(source_branch: branch).to_a - return [] if branches.blank? + (origin_merge_requests + fork_merge_requests) + .uniq.select(&:source_project) + end - merge_requests = @project.origin_merge_requests.opened.where(source_branch: branches).to_a - merge_requests += @project.fork_merge_requests.opened.where(source_branch: branches).to_a + def pipeline_merge_requests(pipeline) + merge_requests_for(pipeline.ref).each do |merge_request| + next unless pipeline == merge_request.pipeline - merge_requests.uniq.select(&:source_project) + yield merge_request + end end - def each_merge_request(commit_status) - merge_request_from(commit_status).each do |merge_request| + def commit_status_merge_requests(commit_status) + merge_requests_for(commit_status.ref).each do |merge_request| pipeline = merge_request.pipeline next unless pipeline next unless pipeline.sha == commit_status.sha - yield merge_request, pipeline + yield merge_request end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index e57791f6818..dd0d738674e 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -4,7 +4,7 @@ module MergeRequests merge_request = MergeRequest.new(params) # Set MR attributes - merge_request.can_be_created = false + merge_request.can_be_created = true merge_request.compare_commits = [] merge_request.source_project = project unless merge_request.source_project @@ -13,14 +13,8 @@ module MergeRequests merge_request.target_project ||= (project.forked_from_project || project) merge_request.target_branch ||= merge_request.target_project.default_branch - if merge_request.target_branch.blank? || merge_request.source_branch.blank? - message = - if params[:source_branch] || params[:target_branch] - "You must select source and target branch" - end - - return build_failed(merge_request, message) - end + messages = validate_branches(merge_request) + return build_failed(merge_request, messages) unless messages.empty? compare = CompareService.new.execute( merge_request.source_project, @@ -29,23 +23,42 @@ module MergeRequests merge_request.target_branch, ) - commits = compare.commits - - # At this point we decide if merge request can be created - # If we have at least one commit to merge -> creation allowed - if commits.present? - merge_request.compare_commits = commits - merge_request.can_be_created = true - merge_request.compare = compare - else - merge_request.can_be_created = false - end + merge_request.compare_commits = compare.commits + merge_request.compare = compare set_title_and_description(merge_request) end private + def validate_branches(merge_request) + messages = [] + + if merge_request.target_branch.blank? || merge_request.source_branch.blank? + messages << + if params[:source_branch] || params[:target_branch] + "You must select source and target branch" + end + end + + if merge_request.source_project == merge_request.target_project && + merge_request.target_branch == merge_request.source_branch + + messages << 'You must select different branches' + end + + # See if source and target branches exist + if merge_request.source_branch.present? && !merge_request.source_project.commit(merge_request.source_branch) + messages << "Source branch \"#{merge_request.source_branch}\" does not exist" + end + + if merge_request.target_branch.present? && !merge_request.target_project.commit(merge_request.target_branch) + messages << "Target branch \"#{merge_request.target_branch}\" does not exist" + end + + messages + end + # When your branch name starts with an iid followed by a dash this pattern will be # interpreted as the user wants to close that issue on this project. # @@ -89,11 +102,15 @@ module MergeRequests end end + merge_request.title = merge_request.wip_title if commits.empty? + merge_request end - def build_failed(merge_request, message) - merge_request.errors.add(:base, message) unless message.nil? + def build_failed(merge_request, messages) + messages.compact.each do |message| + merge_request.errors.add(:base, message) + end merge_request.compare_commits = [] merge_request.can_be_created = false merge_request diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 73247e62421..b0ae2dfe4ce 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -20,6 +20,7 @@ module MergeRequests event_service.open_mr(issuable, current_user) notification_service.new_merge_request(issuable, current_user) todo_service.new_merge_request(issuable, current_user) + issuable.cache_merge_request_closes_issues!(current_user) end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index b037780c431..ab9056a3250 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -11,14 +11,14 @@ module MergeRequests def execute(merge_request) @merge_request = merge_request - return error('Merge request is not mergeable') unless @merge_request.mergeable? + return log_merge_error('Merge request is not mergeable', true) unless @merge_request.mergeable? merge_request.in_locked_state do if commit after_merge success else - error('Can not merge changes') + log_merge_error('Can not merge changes', true) end end end @@ -46,8 +46,8 @@ module MergeRequests merge_request.update(merge_error: e.message) false rescue StandardError => e - merge_request.update(merge_error: "Something went wrong during merge") - Rails.logger.error(e.message) + merge_request.update(merge_error: "Something went wrong during merge: #{e.message}") + log_merge_error(e.message) false ensure merge_request.update(in_progress_merge_commit_sha: nil) @@ -65,5 +65,17 @@ module MergeRequests def branch_deletion_user @merge_request.force_remove_source_branch? ? @merge_request.author : current_user end + + def log_merge_error(message, http_error = false) + Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}") + + error(message) if http_error + end + + def merge_request_info + project = merge_request.project + + "#{project.to_reference}#{merge_request.to_reference}" + end end end diff --git a/app/services/merge_requests/merge_when_build_succeeds_service.rb b/app/services/merge_requests/merge_when_build_succeeds_service.rb index 4ad5fb08311..dc159de0058 100644 --- a/app/services/merge_requests/merge_when_build_succeeds_service.rb +++ b/app/services/merge_requests/merge_when_build_succeeds_service.rb @@ -18,12 +18,13 @@ module MergeRequests merge_request.save end - # Triggers the automatic merge of merge_request once the build succeeds - def trigger(commit_status) - each_merge_request(commit_status) do |merge_request, pipeline| + # Triggers the automatic merge of merge_request once the pipeline succeeds + def trigger(pipeline) + return unless pipeline.success? + + pipeline_merge_requests(pipeline) do |merge_request| next unless merge_request.merge_when_build_succeeds? next unless merge_request.mergeable? - next unless pipeline.success? MergeWorker.perform_async(merge_request.id, merge_request.merge_user_id, merge_request.merge_params) end diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 8437d9b8b43..e8fb1b59752 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -7,6 +7,7 @@ module MergeRequests class PostMergeService < MergeRequests::BaseService def execute(merge_request) 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_note(merge_request) diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 5cedd6f11d9..22596b4014a 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -13,6 +13,7 @@ module MergeRequests reload_merge_requests reset_merge_when_build_succeeds mark_pending_todos_done + cache_merge_requests_closing_issues # Leave a system note if a branch was deleted/added if branch_added? || branch_removed? @@ -141,6 +142,14 @@ module MergeRequests end end + # If the merge requests closes any issues, save this information in the + # `MergeRequestsClosingIssues` model (as a performance optimization). + def cache_merge_requests_closing_issues + @project.merge_requests.where(source_branch: @branch_name).each do |merge_request| + merge_request.cache_merge_request_closes_issues!(@current_user) + end + end + def filter_merge_requests(merge_requests) merge_requests.uniq.select(&:source_project) end diff --git a/app/services/merge_requests/resolve_service.rb b/app/services/merge_requests/resolve_service.rb index 19caa038c44..d22a1d3e0ad 100644 --- a/app/services/merge_requests/resolve_service.rb +++ b/app/services/merge_requests/resolve_service.rb @@ -1,5 +1,8 @@ module MergeRequests class ResolveService < MergeRequests::BaseService + class MissingFiles < Gitlab::Conflict::ResolutionError + end + attr_accessor :conflicts, :rugged, :merge_index, :merge_request def execute(merge_request) @@ -10,8 +13,16 @@ module MergeRequests fetch_their_commit! - conflicts.files.each do |file| - write_resolved_file_to_index(file, params[:sections]) + params[:files].each do |file_params| + conflict_file = merge_request.conflicts.file_for_path(file_params[:old_path], file_params[:new_path]) + + write_resolved_file_to_index(conflict_file, file_params) + end + + unless merge_index.conflicts.empty? + missing_files = merge_index.conflicts.map { |file| file[:ours][:path] } + + raise MissingFiles, "Missing resolutions for the following files: #{missing_files.join(', ')}" end commit_params = { @@ -23,8 +34,13 @@ module MergeRequests project.repository.resolve_conflicts(current_user, merge_request.source_branch, commit_params) end - def write_resolved_file_to_index(file, resolutions) - new_file = file.resolve_lines(resolutions).map(&:text).join("\n") + def write_resolved_file_to_index(file, params) + new_file = if params[:sections] + file.resolve_lines(params[:sections]).map(&:text).join("\n") + elsif params[:content] + file.resolve_content(params[:content]) + end + our_path = file.our_path merge_index.add(path: our_path, oid: rugged.write(new_file, :blob), mode: file.our_mode) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 398ec47f0ea..fda0da19d87 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -1,7 +1,3 @@ -require_relative 'base_service' -require_relative 'reopen_service' -require_relative 'close_service' - module MergeRequests class UpdateService < MergeRequests::BaseService def execute(merge_request) @@ -15,8 +11,11 @@ module MergeRequests params.except!(:target_branch, :force_remove_source_branch) end - merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) + if params[:force_remove_source_branch].present? + merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) + end + handle_wip_event(merge_request) update(merge_request) end @@ -77,5 +76,22 @@ module MergeRequests def close_service MergeRequests::CloseService end + + def after_update(issuable) + issuable.cache_merge_request_closes_issues!(current_user) + end + + private + + def handle_wip_event(merge_request) + if wip_event = params.delete(:wip_event) + # We update the title that is provided in the params or we use the mr title + title = params[:title] || merge_request.title + params[:title] = case wip_event + when 'wip' then MergeRequest.wip_title(title) + when 'unwip' then MergeRequest.wipless_title(title) + end + end + end end end diff --git a/app/services/milestones/create_service.rb b/app/services/milestones/create_service.rb index 3b90399af64..b8e08c9f1eb 100644 --- a/app/services/milestones/create_service.rb +++ b/app/services/milestones/create_service.rb @@ -3,7 +3,7 @@ module Milestones def execute milestone = project.milestones.new(params) - if milestone.save! + if milestone.save event_service.open_milestone(milestone, current_user) end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index a36008c3ef5..d75592e31f3 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -7,8 +7,10 @@ module Notes if note.award_emoji? noteable = note.noteable - todo_service.new_award_emoji(noteable, current_user) - return noteable.create_award_emoji(note.award_emoji_name, current_user) + if noteable.user_can_award?(current_user, note.award_emoji_name) + todo_service.new_award_emoji(noteable, current_user) + return noteable.create_award_emoji(note.award_emoji_name, current_user) + end end # We execute commands (extracted from `params[:note]`) on the noteable @@ -24,13 +26,16 @@ module Notes note.note = content end - if !only_commands && note.save + note.run_after_commit do # Finish the harder work in the background - NewNoteWorker.perform_in(2.seconds, note.id, params) + NewNoteWorker.perform_async(note.id) + end + + if !only_commands && note.save todo_service.new_note(note, current_user) end - if command_params && command_params.any? + if command_params.present? slash_commands_service.execute(command_params, note) # We must add the error after we call #save because errors are reset @@ -38,6 +43,8 @@ module Notes if only_commands note.errors.add(:commands_only, 'Your commands have been executed!') end + + note.commands_changes = command_params.keys end note diff --git a/app/services/notes/slash_commands_service.rb b/app/services/notes/slash_commands_service.rb index 4a9a8a64653..2edbd39a9e7 100644 --- a/app/services/notes/slash_commands_service.rb +++ b/app/services/notes/slash_commands_service.rb @@ -5,9 +5,18 @@ module Notes 'MergeRequest' => MergeRequests::UpdateService } - def supported?(note) + def self.noteable_update_service(note) + UPDATE_SERVICES[note.noteable_type] + end + + def self.supported?(note, current_user) noteable_update_service(note) && - can?(current_user, :"update_#{note.noteable_type.underscore}", note.noteable) + current_user && + current_user.can?(:"update_#{note.noteable_type.underscore}", note.noteable) + end + + def supported?(note) + self.class.supported?(note, current_user) end def extract_commands(note) @@ -21,13 +30,7 @@ module Notes return if command_params.empty? return unless supported?(note) - noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) - end - - private - - def noteable_update_service(note) - UPDATE_SERVICES[note.noteable_type] + self.class.noteable_update_service(note).new(project, current_user, command_params).execute(note.noteable) end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 6139ed56e25..ecdcbf08ee1 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -75,7 +75,7 @@ class NotificationService # * watchers of the issue's labels # def relabeled_issue(issue, added_labels, current_user) - relabeled_resource_email(issue, added_labels, current_user, :relabeled_issue_email) + relabeled_resource_email(issue, issue.project, added_labels, current_user, :relabeled_issue_email) end # When create a merge request we should send an email to: @@ -118,7 +118,7 @@ class NotificationService # * watchers of the mr's labels # def relabeled_merge_request(merge_request, added_labels, current_user) - relabeled_resource_email(merge_request, added_labels, current_user, :relabeled_merge_request_email) + relabeled_resource_email(merge_request, merge_request.target_project, added_labels, current_user, :relabeled_merge_request_email) end def close_mr(merge_request, current_user) @@ -134,7 +134,8 @@ class NotificationService merge_request, merge_request.target_project, current_user, - :merged_merge_request_email + :merged_merge_request_email, + skip_current_user: !merge_request.merge_when_build_succeeds? ) end @@ -204,7 +205,7 @@ class NotificationService recipients = reject_muted_users(recipients, note.project) - recipients = add_subscribed_users(recipients, note.noteable) + recipients = add_subscribed_users(recipients, note.project, note.noteable) recipients = reject_unsubscribed_users(recipients, note.noteable) recipients = reject_users_without_access(recipients, note.noteable) @@ -311,6 +312,22 @@ class NotificationService mailer.project_was_not_exported_email(current_user, project, errors).deliver_later end + def pipeline_finished(pipeline, recipients = nil) + email_template = "pipeline_#{pipeline.status}_email" + + return unless mailer.respond_to?(email_template) + + recipients ||= build_recipients( + pipeline, + pipeline.project, + nil, # The acting user, who won't be added to recipients + action: pipeline.status).map(&:notification_email) + + if recipients.any? + mailer.public_send(email_template, pipeline, recipients).deliver_later + end + end + protected # Get project/group users with CUSTOM notification level @@ -376,7 +393,7 @@ class NotificationService ) end - # Build a list of users based on project notifcation settings + # Build a list of users based on project notification settings def select_project_member_setting(project, global_setting, users_global_level_watch) users = notification_settings_for(project, :watch) @@ -474,24 +491,31 @@ class NotificationService end def reject_users_without_access(recipients, target) - return recipients unless target.is_a?(Issue) + ability = case target + when Issuable + :"read_#{target.to_ability_name}" + when Ci::Pipeline + :read_build # We have build trace in pipeline emails + end + + return recipients unless ability recipients.select do |user| - user.can?(:read_issue, target) + user.can?(ability, target) end end - def add_subscribed_users(recipients, target) + def add_subscribed_users(recipients, project, target) return recipients unless target.respond_to? :subscribers - recipients + target.subscribers + recipients + target.subscribers(project) end - def add_labels_subscribers(recipients, target, labels: nil) + def add_labels_subscribers(recipients, project, target, labels: nil) return recipients unless target.respond_to? :labels (labels || target.labels).each do |label| - recipients += label.subscribers + recipients += label.subscribers(project) end recipients @@ -514,9 +538,16 @@ class NotificationService end end - def close_resource_email(target, project, current_user, method) + def close_resource_email(target, project, current_user, method, skip_current_user: true) action = method == :merged_merge_request_email ? "merge" : "close" - recipients = build_recipients(target, project, current_user, action: action) + + recipients = build_recipients( + target, + project, + current_user, + action: action, + skip_current_user: skip_current_user + ) recipients.each do |recipient| mailer.send(method, recipient.id, target.id, current_user.id).deliver_later @@ -540,8 +571,8 @@ class NotificationService end end - def relabeled_resource_email(target, labels, current_user, method) - recipients = build_relabeled_recipients(target, current_user, labels: labels) + def relabeled_resource_email(target, project, labels, current_user, method) + recipients = build_relabeled_recipients(target, project, current_user, labels: labels) label_names = labels.map(&:name) recipients.each do |recipient| @@ -557,7 +588,7 @@ class NotificationService end end - def build_recipients(target, project, current_user, action: nil, previous_assignee: nil) + def build_recipients(target, project, current_user, action: nil, previous_assignee: nil, skip_current_user: true) custom_action = build_custom_key(action, target) recipients = target.participants(current_user) @@ -577,21 +608,22 @@ class NotificationService end recipients = reject_muted_users(recipients, project) - recipients = add_subscribed_users(recipients, target) + recipients = add_subscribed_users(recipients, project, target) if [:new_issue, :new_merge_request].include?(custom_action) - recipients = add_labels_subscribers(recipients, target) + recipients = add_labels_subscribers(recipients, project, target) end recipients = reject_unsubscribed_users(recipients, target) recipients = reject_users_without_access(recipients, target) - recipients.delete(current_user) + recipients.delete(current_user) if skip_current_user + recipients.uniq end - def build_relabeled_recipients(target, current_user, labels:) - recipients = add_labels_subscribers([], target, labels: labels) + def build_relabeled_recipients(target, project, current_user, labels:) + recipients = add_labels_subscribers([], project, target, labels: labels) recipients = reject_unsubscribed_users(recipients, target) recipients = reject_users_without_access(recipients, target) recipients.delete(current_user) @@ -613,6 +645,6 @@ class NotificationService # Build event key to search on custom notification level # Check NotificationSetting::EMAIL_EVENTS def build_custom_key(action, object) - "#{action}_#{object.class.name.underscore}".to_sym + "#{action}_#{object.class.model_name.name.underscore}".to_sym end end diff --git a/app/services/projects/autocomplete_service.rb b/app/services/projects/autocomplete_service.rb index f578f8dbea2..015f2828921 100644 --- a/app/services/projects/autocomplete_service.rb +++ b/app/services/projects/autocomplete_service.rb @@ -13,7 +13,7 @@ module Projects end def labels - @project.labels.select([:title, :color]) + LabelsFinder.new(current_user, project_id: project.id).execute.select([:title, :color]) end def commands(noteable, type) diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index be749ba4a1c..159f46cd465 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -7,6 +7,8 @@ module Projects def execute forked_from_project_id = params.delete(:forked_from_project_id) import_data = params.delete(:import_data) + @skip_wiki = params.delete(:skip_wiki) + @project = Project.new(params) # Make sure that the user is allowed to use the specified visibility level @@ -15,6 +17,11 @@ module Projects return @project end + unless allowed_fork?(forked_from_project_id) + @project.errors.add(:forked_from_project_id, 'is forbidden') + return @project + end + # Set project name from path if @project.name.present? && @project.path.present? # if both name and path set - everything is ok @@ -71,6 +78,13 @@ module Projects @project.errors.add(:namespace, "is not valid") end + def allowed_fork?(source_project_id) + return true if source_project_id.nil? + + source_project = Project.find_by(id: source_project_id) + current_user.can?(:fork_project, source_project) + end + def allowed_namespace?(user, namespace_id) namespace = Namespace.find_by(id: namespace_id) current_user.can?(:create_projects, namespace) @@ -80,8 +94,8 @@ module Projects log_info("#{@project.owner.name} created a new project \"#{@project.name_with_namespace}\"") unless @project.gitlab_project_import? - @project.create_wiki if @project.feature_available?(:wiki, current_user) - @project.build_missing_services + @project.create_wiki unless skip_wiki? + create_services_from_active_templates(@project) @project.create_labels end @@ -92,6 +106,12 @@ module Projects unless @project.group || @project.gitlab_project_import? @project.team << [current_user, :master, current_user] end + + @project.group.refresh_members_authorized_projects if @project.group + end + + def skip_wiki? + !@project.feature_available?(:wiki, current_user) || @skip_wiki end def save_project_and_import_data(import_data) @@ -117,5 +137,12 @@ module Projects @project end + + def create_services_from_active_templates(project) + Service.where(template: true, active: true).each do |template| + service = Service.build_from_template(project.id, template) + service.save! + end + end end end diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index a2de4dccece..a2b23ea6171 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -16,6 +16,8 @@ module Projects end new_project = CreateService.new(current_user, new_params).execute + return new_project unless new_project.persisted? + builds_access_level = @project.project_feature.builds_access_level new_project.project_feature.update_attributes(builds_access_level: builds_access_level) diff --git a/app/services/projects/housekeeping_service.rb b/app/services/projects/housekeeping_service.rb index c3dfc8cfbe8..4b8946f8ee2 100644 --- a/app/services/projects/housekeeping_service.rb +++ b/app/services/projects/housekeeping_service.rb @@ -7,6 +7,8 @@ # module Projects class HousekeepingService < BaseService + include Gitlab::CurrentSettings + LEASE_TIMEOUT = 3600 class LeaseTaken < StandardError @@ -20,13 +22,14 @@ module Projects end def execute - raise LeaseTaken unless try_obtain_lease + lease_uuid = try_obtain_lease + raise LeaseTaken unless lease_uuid.present? - execute_gitlab_shell_gc + execute_gitlab_shell_gc(lease_uuid) end def needed? - @project.pushes_since_gc >= 10 + pushes_since_gc > 0 && period_match? && housekeeping_enabled? end def increment! @@ -37,19 +40,59 @@ module Projects private - def execute_gitlab_shell_gc - GitGarbageCollectWorker.perform_async(@project.id) + def execute_gitlab_shell_gc(lease_uuid) + GitGarbageCollectWorker.perform_async(@project.id, task, lease_key, lease_uuid) ensure - Gitlab::Metrics.measure(:reset_pushes_since_gc) do - @project.reset_pushes_since_gc + if pushes_since_gc >= gc_period + Gitlab::Metrics.measure(:reset_pushes_since_gc) do + @project.reset_pushes_since_gc + end end end def try_obtain_lease Gitlab::Metrics.measure(:obtain_housekeeping_lease) do - lease = ::Gitlab::ExclusiveLease.new("project_housekeeping:#{@project.id}", timeout: LEASE_TIMEOUT) + lease = ::Gitlab::ExclusiveLease.new(lease_key, timeout: LEASE_TIMEOUT) lease.try_obtain end end + + def lease_key + "project_housekeeping:#{@project.id}" + end + + def pushes_since_gc + @project.pushes_since_gc + end + + def task + if pushes_since_gc % gc_period == 0 + :gc + elsif pushes_since_gc % full_repack_period == 0 + :full_repack + else + :incremental_repack + end + end + + def period_match? + [gc_period, full_repack_period, repack_period].any? { |period| pushes_since_gc % period == 0 } + end + + def housekeeping_enabled? + current_application_settings.housekeeping_enabled + end + + def gc_period + current_application_settings.housekeeping_gc_period + end + + def full_repack_period + current_application_settings.housekeeping_full_repack_period + end + + def repack_period + current_application_settings.housekeeping_incremental_repack_period + end end end diff --git a/app/services/projects/import_service.rb b/app/services/projects/import_service.rb index cdad0426b02..d7221fe993c 100644 --- a/app/services/projects/import_service.rb +++ b/app/services/projects/import_service.rb @@ -29,7 +29,7 @@ module Projects if unknown_url? # In this case, we only want to import issues, not a repository. create_repository - else + elsif !project.repository_exists? import_repository end end @@ -44,6 +44,11 @@ module Projects begin gitlab_shell.import_repository(project.repository_storage_path, project.path_with_namespace, project.import_url) rescue => e + # Expire cache to prevent scenarios such as: + # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true + # 2. Retried import, repo is broken or not imported but +exists?+ still returns true + project.repository.before_import if project.repository_exists? + raise Error, "Error importing repository #{project.import_url} into #{project.path_with_namespace} - #{e.message}" end end diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index d38328403c1..6040391fd94 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -1,7 +1,7 @@ module Projects class ParticipantsService < BaseService attr_reader :noteable - + def execute(noteable) @noteable = noteable @@ -15,7 +15,8 @@ module Projects [{ name: noteable.author.name, - username: noteable.author.username + username: noteable.author.username, + avatar_url: noteable.author.avatar_url }] end @@ -28,14 +29,14 @@ module Projects def sorted(users) users.uniq.to_a.compact.sort_by(&:username).map do |user| - { username: user.username, name: user.name } + { username: user.username, name: user.name, avatar_url: user.avatar_url } end end def groups current_user.authorized_groups.sort_by(&:path).map do |group| count = group.users.count - { username: group.path, name: group.name, count: count } + { username: group.path, name: group.name, count: count, avatar_url: group.avatar.url } end end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index bc7f8bf433b..28470f59807 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -28,6 +28,7 @@ module Projects Project.transaction do old_path = project.path_with_namespace old_namespace = project.namespace + old_group = project.group new_path = File.join(new_namespace.try(:path) || '', project.path) if Project.where(path: project.path, namespace_id: new_namespace.try(:id)).present? @@ -57,6 +58,9 @@ module Projects # Move wiki repo also if present gitlab_shell.mv_repository(project.repository_storage_path, "#{old_path}.wiki", "#{new_path}.wiki") + # Move missing group labels to project + Labels::TransferService.new(current_user, old_group, project).execute + # clear project cached events project.reset_events_cache diff --git a/app/services/protected_branches/api_create_service.rb b/app/services/protected_branches/api_create_service.rb new file mode 100644 index 00000000000..f2040dfa03a --- /dev/null +++ b/app/services/protected_branches/api_create_service.rb @@ -0,0 +1,29 @@ +# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# flags for backward compatibility, and so performs translation between that format and the +# internal data model (separate access levels). The translation code is non-trivial, and so +# lives in this service. +module ProtectedBranches + class ApiCreateService < BaseService + def execute + push_access_level = + if params.delete(:developers_can_push) + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MASTER + end + + merge_access_level = + if params.delete(:developers_can_merge) + Gitlab::Access::DEVELOPER + else + Gitlab::Access::MASTER + end + + @params.merge!(push_access_levels_attributes: [{ access_level: push_access_level }], + merge_access_levels_attributes: [{ access_level: merge_access_level }]) + + service = ProtectedBranches::CreateService.new(@project, @current_user, @params) + service.execute + end + end +end diff --git a/app/services/protected_branches/api_update_service.rb b/app/services/protected_branches/api_update_service.rb new file mode 100644 index 00000000000..050cb3b738b --- /dev/null +++ b/app/services/protected_branches/api_update_service.rb @@ -0,0 +1,47 @@ +# The protected branches API still uses the `developers_can_push` and `developers_can_merge` +# flags for backward compatibility, and so performs translation between that format and the +# internal data model (separate access levels). The translation code is non-trivial, and so +# lives in this service. +module ProtectedBranches + class ApiUpdateService < BaseService + def execute(protected_branch) + @developers_can_push = params.delete(:developers_can_push) + @developers_can_merge = params.delete(:developers_can_merge) + + @protected_branch = protected_branch + + protected_branch.transaction do + delete_redundant_access_levels + + case @developers_can_push + when true + params.merge!(push_access_levels_attributes: [{ access_level: Gitlab::Access::DEVELOPER }]) + when false + params.merge!(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 }]) + when false + params.merge!(merge_access_levels_attributes: [{ access_level: Gitlab::Access::MASTER }]) + end + + service = ProtectedBranches::UpdateService.new(@project, @current_user, @params) + service.execute(protected_branch) + end + end + + private + + def delete_redundant_access_levels + unless @developers_can_merge.nil? + @protected_branch.merge_access_levels.destroy_all + end + + unless @developers_can_push.nil? + @protected_branch.push_access_levels.destroy_all + end + end + end +end diff --git a/app/services/slash_commands/interpret_service.rb b/app/services/slash_commands/interpret_service.rb index 9ac1124abc1..d75c5b1800e 100644 --- a/app/services/slash_commands/interpret_service.rb +++ b/app/services/slash_commands/interpret_service.rb @@ -116,13 +116,20 @@ module SlashCommands desc 'Add label(s)' params '~label1 ~"label 2"' condition do + available_labels = LabelsFinder.new(current_user, project_id: project.id).execute + current_user.can?(:"admin_#{issuable.to_ability_name}", project) && - project.labels.any? + available_labels.any? end command :label do |labels_param| label_ids = find_label_ids(labels_param) - @updates[:add_label_ids] = label_ids unless label_ids.empty? + if label_ids.any? + @updates[:add_label_ids] ||= [] + @updates[:add_label_ids] += label_ids + + @updates[:add_label_ids].uniq! + end end desc 'Remove all or specific label(s)' @@ -136,7 +143,12 @@ module SlashCommands if labels_param.present? label_ids = find_label_ids(labels_param) - @updates[:remove_label_ids] = label_ids unless label_ids.empty? + if label_ids.any? + @updates[:remove_label_ids] ||= [] + @updates[:remove_label_ids] += label_ids + + @updates[:remove_label_ids].uniq! + end else @updates[:label_ids] = [] end @@ -152,7 +164,12 @@ module SlashCommands command :relabel do |labels_param| label_ids = find_label_ids(labels_param) - @updates[:label_ids] = label_ids unless label_ids.empty? + if label_ids.any? + @updates[:label_ids] ||= [] + @updates[:label_ids] += label_ids + + @updates[:label_ids].uniq! + end end desc 'Add a todo' @@ -176,7 +193,7 @@ module SlashCommands desc 'Subscribe' condition do issuable.persisted? && - !issuable.subscribed?(current_user) + !issuable.subscribed?(current_user, project) end command :subscribe do @updates[:subscription_event] = 'subscribe' @@ -185,7 +202,7 @@ module SlashCommands desc 'Unsubscribe' condition do issuable.persisted? && - issuable.subscribed?(current_user) + issuable.subscribed?(current_user, project) end command :unsubscribe do @updates[:subscription_event] = 'unsubscribe' @@ -195,7 +212,7 @@ module SlashCommands params '<in 2 days | this Friday | December 31st>' condition do issuable.respond_to?(:due_date) && - current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :due do |due_date_param| due_date = Chronic.parse(due_date_param).try(:to_date) @@ -208,12 +225,24 @@ module SlashCommands issuable.persisted? && issuable.respond_to?(:due_date) && issuable.due_date? && - current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + current_user.can?(:"admin_#{issuable.to_ability_name}", project) end command :remove_due_date do @updates[:due_date] = nil end + desc do + "Toggle the Work In Progress status" + end + condition do + issuable.persisted? && + issuable.respond_to?(:work_in_progress?) && + current_user.can?(:"update_#{issuable.to_ability_name}", issuable) + end + command :wip do + @updates[:wip_event] = issuable.work_in_progress? ? 'unwip' : 'wip' + end + # This is a dummy command, so that it appears in the autocomplete commands desc 'CC' params '@user' @@ -221,7 +250,7 @@ module SlashCommands def find_label_ids(labels_param) label_ids_by_reference = extract_references(labels_param, :label).map(&:id) - labels_ids_by_name = @project.labels.where(name: labels_param.split).select(:id) + labels_ids_by_name = LabelsFinder.new(current_user, project_id: project.id, name: labels_param.split).execute.select(:id) label_ids_by_reference | labels_ids_by_name end diff --git a/app/services/system_hooks_service.rb b/app/services/system_hooks_service.rb index 1fb72cf89e9..a2bfa422c9d 100644 --- a/app/services/system_hooks_service.rb +++ b/app/services/system_hooks_service.rb @@ -72,7 +72,7 @@ class SystemHooksService return 'user_add_to_group' if event == :create return 'user_remove_from_group' if event == :destroy else - "#{model.class.name.downcase}_#{event.to_s}" + "#{model.class.name.downcase}_#{event}" end end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 0c8446e7c3d..1ce66d50368 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -24,6 +24,7 @@ module SystemNoteService body = "Added #{commits_text}:\n\n" body << existing_commit_summary(noteable, existing_commits, oldrev) body << new_commit_summary(new_commits).join("\n") + body << "\n\n[Compare with previous version](#{diff_comparison_url(noteable, project, oldrev)})" create_note(noteable: noteable, project: project, author: author, note: body) end @@ -245,7 +246,7 @@ module SystemNoteService 'deleted' end - body = "#{verb} #{branch_type.to_s} branch `#{branch}`".capitalize + body = "#{verb} #{branch_type} branch `#{branch}`".capitalize create_note(noteable: noteable, project: project, author: author, note: body) end @@ -254,8 +255,7 @@ module SystemNoteService # # "Started branch `201-issue-branch-button`" def new_issue_branch(issue, project, author, branch) - h = Gitlab::Routing.url_helpers - link = h.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch) + link = url_helpers.namespace_project_compare_url(project.namespace, project, from: project.default_branch, to: branch) body = "Started branch [`#{branch}`](#{link})" create_note(noteable: issue, project: project, author: author, note: body) @@ -347,7 +347,7 @@ module SystemNoteService notes = notes.where(noteable_id: noteable.id) end - notes_for_mentioner(mentioner, noteable, notes).count > 0 + notes_for_mentioner(mentioner, noteable, notes).exists? end # Build an Array of lines detailing each commit added in a merge request @@ -466,4 +466,20 @@ module SystemNoteService def escape_html(text) Rack::Utils.escape_html(text) end + + def url_helpers + @url_helpers ||= Gitlab::Routing.url_helpers + end + + def diff_comparison_url(merge_request, project, oldrev) + diff_id = merge_request.merge_request_diff.id + + url_helpers.diffs_namespace_project_merge_request_url( + project.namespace, + project, + merge_request.iid, + diff_id: diff_id, + start_sha: oldrev + ) + end end diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 776530ac0a5..f8e6b2ef094 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -273,12 +273,12 @@ class TodoService end def reject_users_without_access(users, project, target) - if target.is_a?(Note) && target.for_issue? + if target.is_a?(Note) && (target.for_issue? || target.for_merge_request?) target = target.noteable end - if target.is_a?(Issue) - select_users(users, :read_issue, target) + if target.is_a?(Issuable) + select_users(users, :"read_#{target.to_ability_name}", target) else select_users(users, :read_project, project) end diff --git a/app/services/update_release_service.rb b/app/services/update_release_service.rb index 0ee1ff2d7d9..b7c36651968 100644 --- a/app/services/update_release_service.rb +++ b/app/services/update_release_service.rb @@ -1,5 +1,3 @@ -require_relative 'base_service' - class UpdateReleaseService < BaseService def execute(tag_name, release_description) repository = project.repository diff --git a/app/services/user_project_access_changed_service.rb b/app/services/user_project_access_changed_service.rb new file mode 100644 index 00000000000..2469b4f0d7c --- /dev/null +++ b/app/services/user_project_access_changed_service.rb @@ -0,0 +1,9 @@ +class UserProjectAccessChangedService + def initialize(user_ids) + @user_ids = Array.wrap(user_ids) + end + + def execute + AuthorizedProjectsWorker.bulk_perform_async(@user_ids.map { |id| [id] }) + end +end |