diff options
Diffstat (limited to 'app')
39 files changed, 463 insertions, 326 deletions
diff --git a/app/assets/javascripts/merge_request_widget.js.coffee b/app/assets/javascripts/merge_request_widget.js.coffee index 5ab91261d75..995a2f24093 100644 --- a/app/assets/javascripts/merge_request_widget.js.coffee +++ b/app/assets/javascripts/merge_request_widget.js.coffee @@ -19,7 +19,7 @@ class @MergeRequestWidget when 'merged' location.reload() else - setTimeout(merge_request_widget.mergeInProgress, 3000) + setTimeout(merge_request_widget.mergeInProgress, 2000) dataType: 'json' getMergeStatus: -> diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index b762518d377..100d3d3b317 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -13,27 +13,20 @@ class Projects::BlobController < Projects::ApplicationController before_action :commit, except: [:new, :create] before_action :blob, except: [:new, :create] before_action :from_merge_request, only: [:edit, :update] - before_action :after_edit_path, only: [:edit, :update] before_action :require_branch_head, only: [:edit, :update] + before_action :editor_variables, except: [:show, :preview, :diff] + before_action :after_edit_path, only: [:edit, :update] def new commit unless @repository.empty? end def create - file_path = File.join(@path, File.basename(params[:file_name])) - result = Files::CreateService.new( - @project, - current_user, - params.merge(new_branch: sanitized_new_branch_name), - @ref, - file_path - ).execute + result = Files::CreateService.new(@project, current_user, @commit_params).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - ref = sanitized_new_branch_name.presence || @ref - redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(ref, file_path)) + redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) else flash[:alert] = result[:message] render :new @@ -48,22 +41,10 @@ class Projects::BlobController < Projects::ApplicationController end def update - result = Files::UpdateService. - new( - @project, - current_user, - params.merge(new_branch: sanitized_new_branch_name), - @ref, - @path - ).execute + result = Files::UpdateService.new(@project, current_user, @commit_params).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - - if from_merge_request - from_merge_request.reload_code - end - redirect_to after_edit_path else flash[:alert] = result[:message] @@ -80,12 +61,11 @@ class Projects::BlobController < Projects::ApplicationController end def destroy - result = Files::DeleteService.new(@project, current_user, params, @ref, @path).execute + result = Files::DeleteService.new(@project, current_user, @commit_params).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - redirect_to namespace_project_tree_path(@project.namespace, @project, - @ref) + redirect_to namespace_project_tree_path(@project.namespace, @project, @target_branch) else flash[:alert] = result[:message] render :show @@ -135,7 +115,6 @@ class Projects::BlobController < Projects::ApplicationController @id = params[:id] @ref, @path = extract_ref(@id) - rescue InvalidPathError not_found! end @@ -145,8 +124,8 @@ class Projects::BlobController < Projects::ApplicationController if from_merge_request diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) + "#file-path-#{hexdigest(@path)}" - elsif sanitized_new_branch_name.present? - namespace_project_blob_path(@project.namespace, @project, File.join(sanitized_new_branch_name, @path)) + elsif @target_branch.present? + namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path)) else namespace_project_blob_path(@project.namespace, @project, @id) end @@ -160,4 +139,25 @@ class Projects::BlobController < Projects::ApplicationController def sanitized_new_branch_name @new_branch ||= sanitize(strip_tags(params[:new_branch])) end + + def editor_variables + @current_branch = @ref + @target_branch = (sanitized_new_branch_name || @ref) + + @file_path = + if action_name.to_s == 'create' + File.join(@path, File.basename(params[:file_name])) + else + @path + end + + @commit_params = { + file_path: @file_path, + current_branch: @current_branch, + target_branch: @target_branch, + commit_message: params[:commit_message], + file_content: params[:content], + file_content_encoding: params[:encoding] + } + end end diff --git a/app/controllers/projects/compare_controller.rb b/app/controllers/projects/compare_controller.rb index c5f085c236f..d9b3adae95b 100644 --- a/app/controllers/projects/compare_controller.rb +++ b/app/controllers/projects/compare_controller.rb @@ -13,13 +13,8 @@ class Projects::CompareController < Projects::ApplicationController base_ref = Addressable::URI.unescape(params[:from]) @ref = head_ref = Addressable::URI.unescape(params[:to]) - compare_result = CompareService.new.execute( - current_user, - @project, - head_ref, - @project, - base_ref - ) + compare_result = CompareService.new. + execute(@project, head_ref, @project, base_ref) @commits = compare_result.commits @diffs = compare_result.diffs diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index d1265198318..f3054881daf 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -1,9 +1,7 @@ -require 'gitlab/satellite/satellite' - class Projects::MergeRequestsController < Projects::ApplicationController before_action :module_enabled before_action :merge_request, only: [ - :edit, :update, :show, :diffs, :commits, :automerge, :automerge_check, + :edit, :update, :show, :diffs, :commits, :merge, :merge_check, :ci_status, :toggle_subscription ] before_action :closes_issues, only: [:edit, :update, :show, :diffs, :commits] @@ -137,7 +135,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController end end - def automerge_check + def merge_check if @merge_request.unchecked? @merge_request.check_if_can_be_merged end @@ -147,11 +145,11 @@ class Projects::MergeRequestsController < Projects::ApplicationController render partial: "projects/merge_requests/widget/show.html.haml", layout: false end - def automerge + def merge return access_denied! unless @merge_request.can_be_merged_by?(current_user) - if @merge_request.automergeable? - AutoMergeWorker.perform_async(@merge_request.id, current_user.id, params) + if @merge_request.mergeable? + MergeWorker.perform_async(@merge_request.id, current_user.id, params) @status = true else @status = false diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 324d1795ab4..467b90861f9 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -41,8 +41,6 @@ class MergeRequest < ActiveRecord::Base delegate :commits, :diffs, :last_commit, :last_commit_short_sha, to: :merge_request_diff, prefix: nil - attr_accessor :should_remove_source_branch - # When this attribute is true some MR validation is ignored # It allows us to close or modify broken merge requests attr_accessor :allow_broken @@ -57,7 +55,7 @@ class MergeRequest < ActiveRecord::Base transition [:reopened, :opened] => :closed end - event :merge do + event :mark_as_merged do transition [:reopened, :opened, :locked] => :merged end @@ -205,7 +203,10 @@ class MergeRequest < ActiveRecord::Base end def check_if_can_be_merged - if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? + can_be_merged = + project.repository.can_be_merged?(source_sha, target_branch) + + if can_be_merged mark_as_mergeable else mark_as_unmergeable @@ -220,18 +221,6 @@ class MergeRequest < ActiveRecord::Base self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last end - def automerge!(current_user, commit_message = nil) - return unless automergeable? - - MergeRequests::AutoMergeService. - new(target_project, current_user). - execute(self, commit_message) - end - - def remove_source_branch? - self.should_remove_source_branch && !self.source_project.root_ref?(self.source_branch) && !self.for_fork? - end - def open? opened? || reopened? end @@ -240,11 +229,11 @@ class MergeRequest < ActiveRecord::Base title =~ /\A\[?WIP\]?:? /i end - def automergeable? + def mergeable? open? && !work_in_progress? && can_be_merged? end - def automerge_status + def gitlab_merge_status if work_in_progress? "work_in_progress" else @@ -271,14 +260,14 @@ class MergeRequest < ActiveRecord::Base # # see "git diff" def to_diff(current_user) - Gitlab::Satellite::MergeAction.new(current_user, self).diff_in_satellite + target_project.repository.diff_text(target_branch, source_sha) end # Returns the commit as a series of email patches. # # see "git format-patch" def to_patch(current_user) - Gitlab::Satellite::MergeAction.new(current_user, self).format_patch + target_project.repository.format_patch(target_branch, source_sha) end def hook_attrs @@ -429,4 +418,30 @@ class MergeRequest < ActiveRecord::Base "Open" end end + + def target_sha + @target_sha ||= target_project. + repository.commit(target_branch).sha + end + + def source_sha + commits.first.sha + end + + def fetch_ref + target_project.repository.fetch_ref( + source_project.repository.path_to_repo, + "refs/heads/#{source_branch}", + "refs/merge-requests/#{iid}/head" + ) + end + + def in_locked_state + begin + lock_mr + yield + ensure + unlock_mr if locked? + end + end end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index df1c2b78758..e317c8eac4d 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -16,9 +16,8 @@ require Rails.root.join("app/models/commit") class MergeRequestDiff < ActiveRecord::Base include Sortable - # Prevent store of diff - # if commits amount more then 200 - COMMITS_SAFE_SIZE = 200 + # Prevent store of diff if commits amount more then 500 + COMMITS_SAFE_SIZE = 500 attr_reader :commits, :diffs @@ -124,12 +123,12 @@ class MergeRequestDiff < ActiveRecord::Base if new_diffs.any? if new_diffs.size > Commit::DIFF_HARD_LIMIT_FILES self.state = :overflow_diff_files_limit - new_diffs = [] + new_diffs = new_diffs.first[Commit::DIFF_HARD_LIMIT_LINES] end if new_diffs.sum { |diff| diff.diff.lines.count } > Commit::DIFF_HARD_LIMIT_LINES self.state = :overflow_diff_lines_limit - new_diffs = [] + new_diffs = new_diffs.first[Commit::DIFF_HARD_LIMIT_LINES] end end @@ -160,12 +159,21 @@ class MergeRequestDiff < ActiveRecord::Base private def compare_result - @compare_result ||= CompareService.new.execute( - merge_request.author, - merge_request.source_project, - merge_request.source_branch, - merge_request.target_project, - merge_request.target_branch, - ) + @compare_result ||= + begin + # Update ref for merge request + merge_request.fetch_ref + + # Get latest sha of branch from source project + source_sha = merge_request.source_project.commit(source_branch).sha + + Gitlab::CompareResult.new( + Gitlab::Git::Compare.new( + merge_request.target_project.repository.raw_repository, + merge_request.target_branch, + source_sha, + ) + ) + end end end diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 30ffacadded..161a16ca61c 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -118,12 +118,11 @@ class Namespace < ActiveRecord::Base gitlab_shell.add_namespace(path_was) if gitlab_shell.mv_namespace(path_was, path) - # If repositories moved successfully we need to remove old satellites - # and send update instructions to users. + # If repositories moved successfully we need to + # send update instructions to users. # However we cannot allow rollback since we moved namespace dir # So we basically we mute exceptions in next actions begin - gitlab_shell.rm_satellites(path_was) send_update_instructions rescue # Returning false does not rollback after_* transaction but gives diff --git a/app/models/project.rb b/app/models/project.rb index 3dc1729e812..4628f478ca6 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -520,14 +520,6 @@ class Project < ActiveRecord::Base !repository.exists? || repository.empty? end - def ensure_satellite_exists - self.satellite.create unless self.satellite.exists? - end - - def satellite - @satellite ||= Gitlab::Satellite::Satellite.new(self) - end - def repo repository.raw end @@ -597,14 +589,11 @@ class Project < ActiveRecord::Base new_path_with_namespace = File.join(namespace_dir, path) if gitlab_shell.mv_repository(old_path_with_namespace, new_path_with_namespace) - # If repository moved successfully we need to remove old satellite - # and send update instructions to users. + # If repository moved successfully we need to send update instructions to users. # However we cannot allow rollback since we moved repository # So we basically we mute exceptions in next actions begin gitlab_shell.mv_repository("#{old_path_with_namespace}.wiki", "#{new_path_with_namespace}.wiki") - gitlab_shell.rm_satellites(old_path_with_namespace) - ensure_satellite_exists send_move_instructions reset_events_cache rescue @@ -702,7 +691,6 @@ class Project < ActiveRecord::Base def create_repository if forked? if gitlab_shell.fork_repository(forked_from_project.path_with_namespace, self.namespace.path) - ensure_satellite_exists true else errors.add(:base, 'Failed to fork repository via gitlab-shell') diff --git a/app/models/project_services/gitlab_ci_service.rb b/app/models/project_services/gitlab_ci_service.rb index 5aaa4e85cbc..ecdcd48ae60 100644 --- a/app/models/project_services/gitlab_ci_service.rb +++ b/app/models/project_services/gitlab_ci_service.rb @@ -74,6 +74,8 @@ class GitlabCiService < CiService else :error end + rescue Errno::ECONNREFUSED + :error end def fork_registration(new_project, private_token) @@ -103,6 +105,8 @@ class GitlabCiService < CiService if response.code == 200 and response["coverage"] response["coverage"] end + rescue Errno::ECONNREFUSED + nil end def build_page(sha, ref) diff --git a/app/models/repository.rb b/app/models/repository.rb index 24c32d90051..79b48ebfedf 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1,4 +1,9 @@ +require 'securerandom' + class Repository + class PreReceiveError < StandardError; end + class CommitError < StandardError; end + include Gitlab::ShellAdapter attr_accessor :raw_repository, :path_with_namespace, :project @@ -368,6 +373,89 @@ class Repository @root_ref ||= raw_repository.root_ref end + def commit_file(user, path, content, message, branch) + commit_with_hooks(user, branch) do |ref| + path[0] = '' if path[0] == '/' + + committer = user_to_comitter(user) + options = {} + options[:committer] = committer + options[:author] = committer + options[:commit] = { + message: message, + branch: ref, + } + + options[:file] = { + content: content, + path: path + } + + Gitlab::Git::Blob.commit(raw_repository, options) + end + end + + def remove_file(user, path, message, branch) + commit_with_hooks(user, branch) do |ref| + path[0] = '' if path[0] == '/' + + committer = user_to_comitter(user) + options = {} + options[:committer] = committer + options[:author] = committer + options[:commit] = { + message: message, + branch: ref + } + + options[:file] = { + path: path + } + + Gitlab::Git::Blob.remove(raw_repository, options) + end + end + + def user_to_comitter(user) + { + email: user.email, + name: user.name, + time: Time.now + } + end + + def can_be_merged?(source_sha, target_branch) + our_commit = rugged.branches[target_branch].target + their_commit = rugged.lookup(source_sha) + + if our_commit && their_commit + !rugged.merge_commits(our_commit, their_commit).conflicts? + else + false + end + end + + def merge(user, source_sha, target_branch, options = {}) + our_commit = rugged.branches[target_branch].target + their_commit = rugged.lookup(source_sha) + + raise "Invalid merge target" if our_commit.nil? + raise "Invalid merge source" if their_commit.nil? + + merge_index = rugged.merge_commits(our_commit, their_commit) + return false if merge_index.conflicts? + + commit_with_hooks(user, target_branch) do |ref| + actual_options = options.merge( + parents: [our_commit, their_commit], + tree: merge_index.write_tree(rugged), + update_ref: ref + ) + + Rugged::Commit.create(rugged, actual_options) + end + end + def merged_to_root_ref?(branch_name) branch_commit = commit(branch_name) root_ref_commit = commit(root_ref) @@ -412,6 +500,64 @@ class Repository ) end + def fetch_ref(source_path, source_ref, target_ref) + args = %W(git fetch #{source_path} #{source_ref}:#{target_ref}) + Gitlab::Popen.popen(args, path_to_repo) + end + + def commit_with_hooks(current_user, branch) + oldrev = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + branch + gl_id = Gitlab::ShellEnv.gl_id(current_user) + was_empty = empty? + + # Create temporary ref + random_string = SecureRandom.hex + tmp_ref = "refs/tmp/#{random_string}/head" + + unless was_empty + oldrev = find_branch(branch).target + rugged.references.create(tmp_ref, oldrev) + end + + # Make commit in tmp ref + newrev = yield(tmp_ref) + + unless newrev + raise CommitError.new('Failed to create commit') + end + + # Run GitLab pre-receive hook + pre_receive_hook = Gitlab::Git::Hook.new('pre-receive', path_to_repo) + status = pre_receive_hook.trigger(gl_id, oldrev, newrev, ref) + + if status + if was_empty + # Create branch + rugged.references.create(ref, newrev) + else + # Update head + current_head = find_branch(branch).target + + # Make sure target branch was not changed during pre-receive hook + if current_head == oldrev + rugged.references.update(ref, newrev) + else + raise CommitError.new('Commit was rejected because branch received new push') + end + end + + # Run GitLab post receive hook + post_receive_hook = Gitlab::Git::Hook.new('post-receive', path_to_repo) + status = post_receive_hook.trigger(gl_id, oldrev, newrev, ref) + else + # Remove tmp ref and return error to user + rugged.references.delete(tmp_ref) + + raise PreReceiveError.new('Commit was rejected by pre-receive hook') + end + end + private def cache diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 6d9ed345914..f00ec7408b6 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -31,6 +31,10 @@ class BaseService SystemHooksService.new end + def repository + project.repository + end + # Add an error to the specified model for restricted visibility levels def deny_visibility_level(model, denied_visibility_level = nil) denied_visibility_level ||= model.visibility_level diff --git a/app/services/compare_service.rb b/app/services/compare_service.rb index 6aa9df4b194..70f642baaaa 100644 --- a/app/services/compare_service.rb +++ b/app/services/compare_service.rb @@ -1,27 +1,28 @@ +require 'securerandom' + # Compare 2 branches for one repo or between repositories # and return Gitlab::CompareResult object that responds to commits and diffs class CompareService - def execute(current_user, source_project, source_branch, target_project, target_branch) - # Try to compare branches to get commits list and diffs - # - # Note: Use satellite only when need to compare between two repos - # because satellites are slower than operations on bare repo - if target_project == source_project - Gitlab::CompareResult.new( - Gitlab::Git::Compare.new( - target_project.repository.raw_repository, - target_branch, - source_branch, - ) + def execute(source_project, source_branch, target_project, target_branch) + source_sha = source_project.commit(source_branch).sha + + # If compare with other project we need to fetch ref first + unless target_project == source_project + random_string = SecureRandom.hex + + target_project.repository.fetch_ref( + source_project.repository.path_to_repo, + "refs/heads/#{source_branch}", + "refs/tmp/#{random_string}/head" ) - else - Gitlab::Satellite::CompareAction.new( - current_user, - target_project, - target_branch, - source_project, - source_branch - ).result end + + Gitlab::CompareResult.new( + Gitlab::Git::Compare.new( + target_project.repository.raw_repository, + target_branch, + source_sha, + ) + ) end end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index bd245100955..7aecee217d8 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -1,17 +1,75 @@ module Files class BaseService < ::BaseService - attr_reader :ref, :path + class ValidationError < StandardError; end - def initialize(project, user, params, ref, path = nil) - @project, @current_user, @params = project, user, params.dup - @ref = ref - @path = path + def execute + @current_branch = params[:current_branch] + @target_branch = params[:target_branch] + @commit_message = params[:commit_message] + @file_path = params[:file_path] + @file_content = if params[:file_content_encoding] == 'base64' + Base64.decode64(params[:file_content]) + else + params[:file_content] + end + + # Validate parameters + validate + + # Create new branch if it different from current_branch + if @target_branch != @current_branch + create_target_branch + end + + if sha = commit + success + else + error("Something went wrong. Your changes were not committed") + end + rescue Repository::CommitError, Repository::PreReceiveError, ValidationError => ex + error(ex.message) end private - def repository - project.repository + def current_branch + @current_branch ||= params[:current_branch] + end + + def target_branch + @target_branch ||= params[:target_branch] + end + + def raise_error(message) + raise ValidationError.new(message) + end + + def validate + allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch) + + unless allowed + raise_error("You are not allowed to push into this branch") + end + + unless project.empty_repo? + unless repository.branch_names.include?(@current_branch) + raise_error("You can only create files if you are on top of a branch") + end + + if @current_branch != @target_branch + if repository.branch_names.include?(@target_branch) + raise_error("Branch with such name already exists. You need to switch to this branch in order to make changes") + end + end + end + end + + def create_target_branch + result = CreateBranchService.new(project, current_user).execute(@target_branch, @current_branch) + + unless result[:status] == :success + raise_error("Something went wrong when we tried to create #{@target_branch} for you") + end end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 23833aa78ec..91d715b2d63 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,52 +1,30 @@ require_relative "base_service" module Files - class CreateService < BaseService - def execute - allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) + class CreateService < Files::BaseService + def commit + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) + end - unless allowed - return error("You are not allowed to create file in this branch") - end + def validate + super - file_name = File.basename(path) - file_path = path + file_name = File.basename(@file_path) unless file_name =~ Gitlab::Regex.file_name_regex - return error( + raise_error( 'Your changes could not be committed, because the file name ' + Gitlab::Regex.file_name_regex_message ) end - if project.empty_repo? - # everything is ok because repo does not have a commits yet - else - unless repository.branch_names.include?(ref) - return error("You can only create files if you are on top of a branch") - end - - blob = repository.blob_at_branch(ref, file_path) + unless project.empty_repo? + blob = repository.blob_at_branch(@current_branch, @file_path) if blob - return error("Your changes could not be committed, because file with such name exists") + raise_error("Your changes could not be committed, because file with such name exists") end end - - - new_file_action = Gitlab::Satellite::NewFileAction.new(current_user, project, ref, file_path) - created_successfully = new_file_action.commit!( - params[:content], - params[:commit_message], - params[:encoding], - params[:new_branch] - ) - - if created_successfully - success - else - error("Your changes could not be committed, because the file has been changed") - end end end end diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 1497a0f883b..27c881c3430 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -1,36 +1,9 @@ require_relative "base_service" module Files - class DeleteService < BaseService - def execute - allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) - - unless allowed - return error("You are not allowed to push into this branch") - end - - unless repository.branch_names.include?(ref) - return error("You can only create files if you are on top of a branch") - end - - blob = repository.blob_at_branch(ref, path) - - unless blob - return error("You can only edit text files") - end - - delete_file_action = Gitlab::Satellite::DeleteFileAction.new(current_user, project, ref, path) - - deleted_successfully = delete_file_action.commit!( - nil, - params[:commit_message] - ) - - if deleted_successfully - success - else - error("Your changes could not be committed, because the file has been changed") - end + class DeleteService < Files::BaseService + def commit + repository.remove_file(current_user, @file_path, @commit_message, @target_branch) end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 0724d3ae634..a20903c6f02 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -1,39 +1,9 @@ require_relative "base_service" module Files - class UpdateService < BaseService - def execute - allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) - - unless allowed - return error("You are not allowed to push into this branch") - end - - unless repository.branch_names.include?(ref) - return error("You can only create files if you are on top of a branch") - end - - blob = repository.blob_at_branch(ref, path) - - unless blob - return error("You can only edit text files") - end - - edit_file_action = Gitlab::Satellite::EditFileAction.new(current_user, project, ref, path) - edit_file_action.commit!( - params[:content], - params[:commit_message], - params[:encoding], - params[:new_branch] - ) - - success - rescue Gitlab::Satellite::CheckoutFailed => ex - error("Your changes could not be committed because ref '#{ref}' could not be checked out", 400) - rescue Gitlab::Satellite::CommitFailed => ex - error("Your changes could not be committed. Maybe there was nothing to commit?", 409) - rescue Gitlab::Satellite::PushFailed => ex - error("Your changes could not be committed. Maybe the file was changed by another process?", 409) + class UpdateService < Files::BaseService + def commit + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) end end end diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 5a2c97b08af..81535450ac1 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -10,16 +10,14 @@ class GitPushService # # Next, this method: # 1. Creates the push event - # 2. Ensures that the project satellite exists - # 3. Updates merge requests - # 4. Recognizes cross-references from commit messages - # 5. Executes the project's web hooks - # 6. Executes the project's services + # 2. Updates merge requests + # 3. Recognizes cross-references from commit messages + # 4. Executes the project's web hooks + # 5. Executes the project's services # def execute(project, user, oldrev, newrev, ref) @project, @user = project, user - project.ensure_satellite_exists project.repository.expire_cache if push_remove_branch?(ref, newrev) @@ -133,7 +131,8 @@ class GitPushService end def is_default_branch?(ref) - Gitlab::Git.branch_ref?(ref) && Gitlab::Git.ref_name(ref) == project.default_branch + Gitlab::Git.branch_ref?(ref) && + (Gitlab::Git.ref_name(ref) == project.default_branch || project.default_branch.nil?) end def commit_user(commit) diff --git a/app/services/merge_requests/auto_merge_service.rb b/app/services/merge_requests/auto_merge_service.rb deleted file mode 100644 index cdedf48b0c0..00000000000 --- a/app/services/merge_requests/auto_merge_service.rb +++ /dev/null @@ -1,30 +0,0 @@ -module MergeRequests - # AutoMergeService class - # - # Do git merge in satellite and in case of success - # mark merge request as merged and execute all hooks and notifications - # Called when you do merge via GitLab UI - class AutoMergeService < BaseMergeService - def execute(merge_request, commit_message) - merge_request.lock_mr - - if Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message) - merge_request.merge - - create_merge_event(merge_request, current_user) - create_note(merge_request) - notification_service.merge_mr(merge_request, current_user) - execute_hooks(merge_request, 'merge') - - true - else - merge_request.unlock_mr - false - end - rescue - merge_request.unlock_mr if merge_request.locked? - merge_request.mark_as_unmergeable - false - end - end -end diff --git a/app/services/merge_requests/base_merge_service.rb b/app/services/merge_requests/base_merge_service.rb deleted file mode 100644 index 9579573adf9..00000000000 --- a/app/services/merge_requests/base_merge_service.rb +++ /dev/null @@ -1,10 +0,0 @@ -module MergeRequests - class BaseMergeService < MergeRequests::BaseService - - private - - def create_merge_event(merge_request, current_user) - EventCreateService.new.merge_mr(merge_request, current_user) - end - end -end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 956480938c3..a9b29f9654d 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -12,12 +12,16 @@ module MergeRequests merge_request.target_project ||= (project.forked_from_project || project) merge_request.target_branch ||= merge_request.target_project.default_branch - unless merge_request.target_branch && merge_request.source_branch - return build_failed(merge_request, nil) + 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 compare_result = CompareService.new.execute( - current_user, merge_request.source_project, merge_request.source_branch, merge_request.target_project, @@ -40,7 +44,6 @@ module MergeRequests merge_request.compare_diffs = diffs elsif diffs == false - # satellite timeout return false merge_request.can_be_created = false merge_request.compare_failed = true end @@ -59,9 +62,6 @@ module MergeRequests end merge_request - - rescue Gitlab::Satellite::BranchesWithoutParent - return build_failed(merge_request, "Selected branches have no common commit so they cannot be merged.") end def build_failed(merge_request, message) diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 327ead4ff3f..98a67c0bc99 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -1,22 +1,47 @@ module MergeRequests # MergeService class # - # Mark existing merge request as merged - # and execute all hooks and notifications - # Called when you do merge via command line and push code - # to target branch - class MergeService < BaseMergeService + # Do git merge and in case of success + # mark merge request as merged and execute all hooks and notifications + # Executed when you do merge via GitLab UI + # + class MergeService < MergeRequests::BaseService + attr_reader :merge_request, :commit_message + def execute(merge_request, commit_message) - merge_request.merge + @commit_message = commit_message + @merge_request = merge_request + + unless @merge_request.mergeable? + return error('Merge request is not mergeable') + end + + merge_request.in_locked_state do + if commit + after_merge + success + else + error('Can not merge changes') + end + end + end - create_merge_event(merge_request, current_user) - create_note(merge_request) - notification_service.merge_mr(merge_request, current_user) - execute_hooks(merge_request, 'merge') + private + + def commit + committer = repository.user_to_comitter(current_user) + + options = { + message: commit_message, + author: committer, + committer: committer + } + + repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) + end - true - rescue - false + def after_merge + MergeRequests::PostMergeService.new(project, current_user).execute(merge_request) end end end diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb new file mode 100644 index 00000000000..aceb8cb9021 --- /dev/null +++ b/app/services/merge_requests/post_merge_service.rb @@ -0,0 +1,22 @@ +module MergeRequests + # PostMergeService class + # + # Mark existing merge request as merged + # and execute all hooks and notifications + # + class PostMergeService < MergeRequests::BaseService + def execute(merge_request) + merge_request.mark_as_merged + create_merge_event(merge_request, current_user) + create_note(merge_request) + notification_service.merge_mr(merge_request, current_user) + execute_hooks(merge_request, 'merge') + end + + private + + def create_merge_event(merge_request, current_user) + EventCreateService.new.merge_mr(merge_request, current_user) + end + end +end diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index d0648da049b..e903e48e3cd 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -33,9 +33,9 @@ module MergeRequests merge_requests.uniq.select(&:source_project).each do |merge_request| - MergeRequests::MergeService. + MergeRequests::PostMergeService. new(merge_request.target_project, @current_user). - execute(merge_request, nil) + execute(merge_request) end end diff --git a/app/services/projects/destroy_service.rb b/app/services/projects/destroy_service.rb index 403f419ec50..28872c89259 100644 --- a/app/services/projects/destroy_service.rb +++ b/app/services/projects/destroy_service.rb @@ -27,7 +27,6 @@ module Projects end end - project.satellite.destroy log_info("Project \"#{project.name}\" was removed") system_hook_service.execute_hooks_for(project, :destroy) true diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index f43c0ef70e9..550ed6897dd 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -33,9 +33,6 @@ module Projects raise TransferError.new("Project with same path in target namespace already exists") end - # Remove old satellite - project.satellite.destroy - # Apply new namespace id project.namespace = new_namespace project.save! @@ -51,9 +48,6 @@ module Projects # Move wiki repo also if present gitlab_shell.mv_repository("#{old_path}.wiki", "#{new_path}.wiki") - # Create a new satellite (reload project from DB) - Project.find(project.id).ensure_satellite_exists - # clear project cached events project.reset_events_cache diff --git a/app/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml index 96f188e4aa7..9c3e1703c89 100644 --- a/app/views/projects/blob/_editor.html.haml +++ b/app/views/projects/blob/_editor.html.haml @@ -12,8 +12,8 @@ \/ = text_field_tag 'file_name', params[:file_name], placeholder: "File name", required: true, class: 'form-control new-file-name' - .pull-right - = select_tag :encoding, options_for_select([ "base64", "text" ], "text"), class: 'form-control' + .pull-right + = select_tag :encoding, options_for_select([ "base64", "text" ], "text"), class: 'form-control' .file-content.code %pre.js-edit-mode-pane#editor diff --git a/app/views/projects/blob/new.html.haml b/app/views/projects/blob/new.html.haml index dac984f8c31..7c2a4fece94 100644 --- a/app/views/projects/blob/new.html.haml +++ b/app/views/projects/blob/new.html.haml @@ -6,11 +6,12 @@ = render 'shared/commit_message_container', params: params, placeholder: 'Add new file' - .form-group.branch - = label_tag 'branch', class: 'control-label' do - Branch - .col-sm-10 - = text_field_tag 'new_branch', @ref, class: "form-control" + - unless @project.empty_repo? + .form-group.branch + = label_tag 'branch', class: 'control-label' do + Branch + .col-sm-10 + = text_field_tag 'new_branch', @ref, class: "form-control" = hidden_field_tag 'content', '', id: 'file-content' = render 'projects/commit_button', ref: @ref, diff --git a/app/views/projects/merge_requests/_new_compare.html.haml b/app/views/projects/merge_requests/_new_compare.html.haml index ff9c0cdb283..7709330611a 100644 --- a/app/views/projects/merge_requests/_new_compare.html.haml +++ b/app/views/projects/merge_requests/_new_compare.html.haml @@ -35,7 +35,7 @@ - if @merge_request.compare_failed .alert.alert-danger %h4 Compare failed - %p We can't compare selected branches. It may be because of huge diff or satellite timeout. Please try again or select different branches. + %p We can't compare selected branches. It may be because of huge diff. Please try again or select different branches. - else .light-well .center diff --git a/app/views/projects/merge_requests/_new_submit.html.haml b/app/views/projects/merge_requests/_new_submit.html.haml index 633a54f3620..76f44211dac 100644 --- a/app/views/projects/merge_requests/_new_submit.html.haml +++ b/app/views/projects/merge_requests/_new_submit.html.haml @@ -24,7 +24,7 @@ = icon('history') Commits %span.badge= @commits.size - %li.diffs-tab + %li.diffs-tab.active = link_to url_for(params), data: {target: '#diffs', action: 'diffs', toggle: 'tab'} do = icon('list-alt') Changes @@ -33,7 +33,7 @@ .tab-content #commits.commits.tab-pane = render "projects/commits/commits", project: @project - #diffs.diffs.tab-pane + #diffs.diffs.tab-pane.active - if @diffs.present? = render "projects/diffs/diffs", diffs: @diffs, project: @project - elsif @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE diff --git a/app/views/projects/merge_requests/automerge.js.haml b/app/views/projects/merge_requests/merge.js.haml index 33321651e32..33321651e32 100644 --- a/app/views/projects/merge_requests/automerge.js.haml +++ b/app/views/projects/merge_requests/merge.js.haml diff --git a/app/views/projects/merge_requests/show/_commits.html.haml b/app/views/projects/merge_requests/show/_commits.html.haml index 3b7f283daf0..a71b181a6a5 100644 --- a/app/views/projects/merge_requests/show/_commits.html.haml +++ b/app/views/projects/merge_requests/show/_commits.html.haml @@ -1 +1 @@ -= render "projects/commits/commits", project: @merge_request.source_project += render "projects/commits/commits", project: @merge_request.project diff --git a/app/views/projects/merge_requests/show/_diffs.html.haml b/app/views/projects/merge_requests/show/_diffs.html.haml index 786b5f39063..626970f39be 100644 --- a/app/views/projects/merge_requests/show/_diffs.html.haml +++ b/app/views/projects/merge_requests/show/_diffs.html.haml @@ -1,5 +1,5 @@ - if @merge_request_diff.collected? - = render "projects/diffs/diffs", diffs: @merge_request.diffs, project: @merge_request.source_project + = render "projects/diffs/diffs", diffs: @merge_request.diffs, project: @merge_request.project - elsif @merge_request_diff.empty? .nothing-here-block Nothing to merge from #{@merge_request.source_branch} into #{@merge_request.target_branch} - else diff --git a/app/views/projects/merge_requests/widget/_open.html.haml b/app/views/projects/merge_requests/widget/_open.html.haml index 8c61e819374..0aad9bb3e88 100644 --- a/app/views/projects/merge_requests/widget/_open.html.haml +++ b/app/views/projects/merge_requests/widget/_open.html.haml @@ -3,8 +3,6 @@ .mr-widget-body - if @project.archived? = render 'projects/merge_requests/widget/open/archived' - - elsif !@project.satellite.exists? - = render 'projects/merge_requests/widget/open/no_satellite' - elsif @merge_request.commits.blank? = render 'projects/merge_requests/widget/open/nothing' - elsif @merge_request.branch_missing? diff --git a/app/views/projects/merge_requests/widget/_show.html.haml b/app/views/projects/merge_requests/widget/_show.html.haml index 263cab7a9e8..a489d4f9b24 100644 --- a/app/views/projects/merge_requests/widget/_show.html.haml +++ b/app/views/projects/merge_requests/widget/_show.html.haml @@ -11,10 +11,10 @@ var merge_request_widget; merge_request_widget = new MergeRequestWidget({ - url_to_automerge_check: "#{automerge_check_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", + url_to_automerge_check: "#{merge_check_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", check_enable: #{@merge_request.unchecked? ? "true" : "false"}, url_to_ci_check: "#{ci_status_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)}", ci_enable: #{@project.ci_service ? "true" : "false"}, - current_status: "#{@merge_request.automerge_status}", + current_status: "#{@merge_request.gitlab_merge_status}", }); diff --git a/app/views/projects/merge_requests/widget/open/_accept.html.haml b/app/views/projects/merge_requests/widget/open/_accept.html.haml index df20205de1c..b61e193fc42 100644 --- a/app/views/projects/merge_requests/widget/open/_accept.html.haml +++ b/app/views/projects/merge_requests/widget/open/_accept.html.haml @@ -1,4 +1,4 @@ -= form_for [:automerge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f| += form_for [:merge, @project.namespace.becomes(Namespace), @project, @merge_request], remote: true, method: :post, html: { class: 'accept-mr-form js-requires-input' } do |f| = hidden_field_tag :authenticity_token, form_authenticity_token .accept-merge-holder.clearfix.js-toggle-container .accept-action diff --git a/app/views/projects/merge_requests/widget/open/_no_satellite.html.haml b/app/views/projects/merge_requests/widget/open/_no_satellite.html.haml deleted file mode 100644 index 3718cfd8333..00000000000 --- a/app/views/projects/merge_requests/widget/open/_no_satellite.html.haml +++ /dev/null @@ -1,3 +0,0 @@ -%p - %span - %strong This repository does not have a satellite. Please ask an administrator to fix this issue! diff --git a/app/workers/auto_merge_worker.rb b/app/workers/auto_merge_worker.rb deleted file mode 100644 index a6dd73eee5f..00000000000 --- a/app/workers/auto_merge_worker.rb +++ /dev/null @@ -1,13 +0,0 @@ -class AutoMergeWorker - include Sidekiq::Worker - - sidekiq_options queue: :default - - def perform(merge_request_id, current_user_id, params) - params = params.with_indifferent_access - current_user = User.find(current_user_id) - merge_request = MergeRequest.find(merge_request_id) - merge_request.should_remove_source_branch = params[:should_remove_source_branch] - merge_request.automerge!(current_user, params[:commit_message]) - end -end diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb new file mode 100644 index 00000000000..6a8665c179a --- /dev/null +++ b/app/workers/merge_worker.rb @@ -0,0 +1,19 @@ +class MergeWorker + include Sidekiq::Worker + + sidekiq_options queue: :default + + def perform(merge_request_id, current_user_id, params) + params = params.with_indifferent_access + current_user = User.find(current_user_id) + merge_request = MergeRequest.find(merge_request_id) + + result = MergeRequests::MergeService.new(merge_request.target_project, current_user). + execute(merge_request, params[:commit_message]) + + if result[:status] == :success && params[:should_remove_source_branch].present? + DeleteBranchService.new(merge_request.source_project, current_user). + execute(merge_request.source_branch) + end + end +end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 94832872d13..b546f8777e1 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -27,7 +27,6 @@ class RepositoryImportWorker project.import_finish project.save - project.satellite.create unless project.satellite.exists? ProjectCacheWorker.perform_async(project.id) Gitlab::BitbucketImport::KeyDeleter.new(project).execute if project.import_type == 'bitbucket' end |