From 9a1ad8deec9cf1c3d35c85f599c041f581a7b782 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 10:48:02 +0200 Subject: [PATCH 01/14] Revert "Merge branch 'refactor-can-be-merge' into 'master'" This reverts commit 459e6d346768d9d8fceaee00bf0870b8e7c4ed9a, reversing changes made to 804168e1def1204af712febb229f41a3865f085f. Signed-off-by: Dmitriy Zaporozhets --- app/models/merge_request.rb | 8 +++++++- app/models/repository.rb | 9 --------- spec/models/repository_spec.rb | 14 -------------- 3 files changed, 7 insertions(+), 24 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 1ef76d1670..5a48fe6628 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -209,7 +209,13 @@ class MergeRequest < ActiveRecord::Base if for_fork? Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? else - project.repository.can_be_merged?(source_branch, target_branch) + rugged = project.repository.rugged + our_commit = rugged.branches[target_branch].target + their_commit = rugged.branches[source_branch].target + + if our_commit && their_commit + !rugged.merge_commits(our_commit, their_commit).conflicts? + end end if can_be_merged diff --git a/app/models/repository.rb b/app/models/repository.rb index 277a917859..dae19b9ddc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -411,15 +411,6 @@ class Repository } end - def can_be_merged?(source_branch, target_branch) - our_commit = rugged.branches[target_branch].target - their_commit = rugged.branches[source_branch].target - - if our_commit && their_commit - !rugged.merge_commits(our_commit, their_commit).conflicts? - end - end - def search_files(query, ref) offset = 2 args = %W(git grep -i -n --before-context #{offset} --after-context #{offset} #{query} #{ref || root_ref}) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index d25351b0f0..0927cde61a 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -34,20 +34,6 @@ describe Repository do end end - describe :can_be_merged? do - context 'mergeable branches' do - subject { repository.can_be_merged?('feature', 'master') } - - it { is_expected.to be_truthy } - end - - context 'non-mergeable branches' do - subject { repository.can_be_merged?('feature_conflict', 'feature') } - - it { is_expected.to be_falsey } - end - end - describe "search_files" do let(:results) { repository.search_files('feature', 'master') } subject { results } From 2258572b80bf9b07ffb4795639b489e7f7f346e7 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 10:50:39 +0200 Subject: [PATCH 02/14] Revert "Merge branches inside one repository using rugged instead of satellites" This reverts commit d24c40ec219d76e01e2fab5f6ebf431adae91bdd. --- app/models/merge_request.rb | 15 +----- app/models/repository.rb | 2 + .../merge_requests/auto_merge_service.rb | 51 ++----------------- 3 files changed, 6 insertions(+), 62 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 5a48fe6628..324d1795ab 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -205,20 +205,7 @@ class MergeRequest < ActiveRecord::Base end def check_if_can_be_merged - can_be_merged = - if for_fork? - Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? - else - rugged = project.repository.rugged - our_commit = rugged.branches[target_branch].target - their_commit = rugged.branches[source_branch].target - - if our_commit && their_commit - !rugged.merge_commits(our_commit, their_commit).conflicts? - end - end - - if can_be_merged + if Gitlab::Satellite::MergeAction.new(self.author, self).can_be_merged? mark_as_mergeable else mark_as_unmergeable diff --git a/app/models/repository.rb b/app/models/repository.rb index dae19b9ddc..29bccbbaf5 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -403,6 +403,8 @@ class Repository Gitlab::Git::Blob.remove(raw_repository, options) end + private + def user_to_comitter(user) { email: user.email, diff --git a/app/services/merge_requests/auto_merge_service.rb b/app/services/merge_requests/auto_merge_service.rb index db824d452d..cdedf48b0c 100644 --- a/app/services/merge_requests/auto_merge_service.rb +++ b/app/services/merge_requests/auto_merge_service.rb @@ -5,20 +5,17 @@ module MergeRequests # mark merge request as merged and execute all hooks and notifications # Called when you do merge via GitLab UI class AutoMergeService < BaseMergeService - attr_reader :merge_request, :commit_message - def execute(merge_request, commit_message) - @commit_message = commit_message - @merge_request = merge_request - merge_request.lock_mr - if merge! + 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 @@ -29,47 +26,5 @@ module MergeRequests merge_request.mark_as_unmergeable false end - - def merge! - if merge_request.for_fork? - Gitlab::Satellite::MergeAction.new(current_user, merge_request).merge!(commit_message) - else - # Merge local branches using rugged instead of satellites - if sha = commit - after_commit(sha, merge_request.target_branch) - - if merge_request.remove_source_branch? - DeleteBranchService.new(merge_request.source_project, current_user).execute(merge_request.source_branch) - end - - true - else - false - end - end - end - - def commit - committer = repository.user_to_comitter(current_user) - - options = { - message: commit_message, - author: committer, - committer: committer - } - - repository.merge(merge_request.source_branch, merge_request.target_branch, options) - end - - def after_commit(sha, branch) - commit = repository.commit(sha) - full_ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch}" - old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA - GitPushService.new.execute(project, current_user, old_sha, sha, full_ref) - end - - def repository - project.repository - end end end From 52d10d73ea60649c79ef7804eb0a43953ff473b2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:12:35 +0200 Subject: [PATCH 03/14] Fix conflict issue Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index 29bccbbaf5..dae19b9ddc 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -403,8 +403,6 @@ class Repository Gitlab::Git::Blob.remove(raw_repository, options) end - private - def user_to_comitter(user) { email: user.email, From 8e134f20ccdf8fbbaea2466c0b26e8d51a82fdaa Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:40:39 +0200 Subject: [PATCH 04/14] Remove changelog items Signed-off-by: Dmitriy Zaporozhets --- CHANGELOG | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 99ee87b897..15718aab72 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,6 +2,51 @@ Please view this file on the master branch, on stable branches it's out of date. v 7.13.4 - Allow users to send abuse reports +v 7.14.0 (unreleased) + - Show incompatible projects in Bitbucket import status (Stan Hu) + - Fix coloring of diffs on MR Discussion-tab (Gert Goet) + - Fix "Network" and "Graphs" pages for branches with encoded slashes (Stan Hu) + - Fix errors deleting and creating branches with encoded slashes (Stan Hu) + - Always add current user to autocomplete controller to support filter by "Me" (Stan Hu) + - Fix multi-line syntax highlighting (Stan Hu) + - Fix network graph when branch name has single quotes (Stan Hu) + - Add "Confirm user" button in user admin page (Stan Hu) + - Upgrade gitlab_git to version 7.2.6 to fix Error 500 when creating network graphs (Stan Hu) + - Add support for Unicode filenames in relative links (Hiroyuki Sato) + - Fix URL used for refreshing notes if relative_url is present (Bartłomiej Święcki) + - Fix commit data retrieval when branch name has single quotes (Stan Hu) + - Check that project was actually created rather than just validated in import:repos task (Stan Hu) + - Fix full screen mode for snippet comments (Daniel Gerhardt) + - Fix 404 error in files view after deleting the last file in a repository (Stan Hu) + - Fix the "Reload with full diff" URL button (Stan Hu) + - Fix label read access for unauthenticated users (Daniel Gerhardt) + - Fix access to disabled features for unauthenticated users (Daniel Gerhardt) + - Fix OAuth provider bug where GitLab would not go return to the redirect_uri after sign-in (Stan Hu) + - Fix file upload dialog for comment editing (Daniel Gerhardt) + - Set OmniAuth full_host parameter to ensure redirect URIs are correct (Stan Hu) + - Return comments in created order in merge request API (Stan Hu) + - Expire Rails cache entries after two weeks to prevent endless Redis growth + - Add support for destroying project milestones (Stan Hu) + - Add fetch command to the MR page. + - Allow custom backup archive permissions + - Add fetch command to the MR page + - Add project star and fork count, group avatar URL and user/group web URL attributes to API + - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. + - Add fetch command to the MR page. + - Show who last edited a comment if it wasn't the original author + - Send notification to all participants when MR is merged. + - Add ability to manage user email addresses via the API. + - Show buttons to add license, changelog and contribution guide if they're missing. + - Tweak project page buttons. + - Disabled autocapitalize and autocorrect on login field (Daryl Chan) + - Mention group and project name in creation, update and deletion notices (Achilleas Pipinellis) + - Update gravatar link on profile page to link to configured gravatar host (Ben Bodenmiller) + - Remove redis-store TTL monkey patch + - Add support for CI skipped status + - Fetch code from forks to refs/merge-requests/:id/head when merge request created + - Remove comments and email addresses when publicly exposing ssh keys (Zeger-Jan van de Weg) + - Cache all events + v 7.13.3 - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. @@ -142,7 +187,6 @@ v 7.12.0 - Add SAML support as an omniauth provider - Allow to configure a URL to show after sign out - Add an option to automatically sign-in with an Omniauth provider - - Better performance for web editor (switched from satellites to rugged) - GitLab CI service sends .gitlab-ci.yml in each push call - When remove project - move repository and schedule it removal - Improve group removing logic From 60b3d2b704e7dfc415c04fdb14ccf56034509069 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:49:09 +0200 Subject: [PATCH 05/14] Revert "Refactor web editor" This reverts commit dfccb06dda344819989fa8d6a9a3c56c5ca0b65f. Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/blob_controller.rb | 60 ++++++++-------- app/services/files/base_service.rb | 77 ++------------------- app/services/files/create_service.rb | 50 ++++++++++--- app/services/files/delete_service.rb | 32 ++++++++- app/services/files/update_service.rb | 42 ++++++++++- app/views/projects/blob/new.html.haml | 11 ++- 6 files changed, 152 insertions(+), 120 deletions(-) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 100d3d3b31..b762518d37 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -13,20 +13,27 @@ 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 :require_branch_head, only: [:edit, :update] - before_action :editor_variables, except: [:show, :preview, :diff] before_action :after_edit_path, only: [:edit, :update] + before_action :require_branch_head, only: [:edit, :update] def new commit unless @repository.empty? end def create - result = Files::CreateService.new(@project, current_user, @commit_params).execute + 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 if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) + ref = sanitized_new_branch_name.presence || @ref + redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(ref, file_path)) else flash[:alert] = result[:message] render :new @@ -41,10 +48,22 @@ class Projects::BlobController < Projects::ApplicationController end def update - result = Files::UpdateService.new(@project, current_user, @commit_params).execute + result = Files::UpdateService. + new( + @project, + current_user, + params.merge(new_branch: sanitized_new_branch_name), + @ref, + @path + ).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] @@ -61,11 +80,12 @@ class Projects::BlobController < Projects::ApplicationController end def destroy - result = Files::DeleteService.new(@project, current_user, @commit_params).execute + result = Files::DeleteService.new(@project, current_user, params, @ref, @path).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - redirect_to namespace_project_tree_path(@project.namespace, @project, @target_branch) + redirect_to namespace_project_tree_path(@project.namespace, @project, + @ref) else flash[:alert] = result[:message] render :show @@ -115,6 +135,7 @@ class Projects::BlobController < Projects::ApplicationController @id = params[:id] @ref, @path = extract_ref(@id) + rescue InvalidPathError not_found! end @@ -124,8 +145,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 @target_branch.present? - namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path)) + elsif sanitized_new_branch_name.present? + namespace_project_blob_path(@project.namespace, @project, File.join(sanitized_new_branch_name, @path)) else namespace_project_blob_path(@project.namespace, @project, @id) end @@ -139,25 +160,4 @@ 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/services/files/base_service.rb b/app/services/files/base_service.rb index 646784f2d9..4d02752454 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -1,34 +1,11 @@ module Files class BaseService < ::BaseService - class ValidationError < StandardError; end + attr_reader :ref, :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 - after_commit(sha, @target_branch) - success - else - error("Something went wrong. Your changes were not committed") - end - rescue ValidationError => ex - error(ex.message) + def initialize(project, user, params, ref, path = nil) + @project, @current_user, @params = project, user, params.dup + @ref = ref + @path = path end private @@ -37,51 +14,11 @@ module Files project.repository end - def after_commit(sha, branch) + def after_commit(sha) commit = repository.commit(sha) - full_ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch}" + full_ref = 'refs/heads/' + (params[:new_branch] || ref) old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA GitPushService.new.execute(project, current_user, old_sha, sha, full_ref) end - - 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 91d715b2d6..0a80455bc6 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -2,29 +2,59 @@ 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) - end + def execute + allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) - def validate - super + unless allowed + return error("You are not allowed to create file in this branch") + end - file_name = File.basename(@file_path) + file_name = File.basename(path) + file_path = path unless file_name =~ Gitlab::Regex.file_name_regex - raise_error( + return error( 'Your changes could not be committed, because the file name ' + Gitlab::Regex.file_name_regex_message ) end - unless project.empty_repo? - blob = repository.blob_at_branch(@current_branch, @file_path) + 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) if blob - raise_error("Your changes could not be committed, because file with such name exists") + return error("Your changes could not be committed, because file with such name exists") end end + + content = + if params[:encoding] == 'base64' + Base64.decode64(params[:content]) + else + params[:content] + end + + sha = repository.commit_file( + current_user, + file_path, + content, + params[:commit_message], + params[:new_branch] || ref + ) + + + if sha + after_commit(sha) + 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 27c881c343..2281777604 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -2,8 +2,36 @@ require_relative "base_service" module Files class DeleteService < Files::BaseService - def commit - repository.remove_file(current_user, @file_path, @commit_message, @target_branch) + 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 + + sha = repository.remove_file( + current_user, + path, + params[:commit_message], + ref + ) + + if sha + after_commit(sha) + 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/update_service.rb b/app/services/files/update_service.rb index a20903c6f0..013cc1ee32 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -2,8 +2,46 @@ require_relative "base_service" module Files class UpdateService < Files::BaseService - def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) + 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 + + content = + if params[:encoding] == 'base64' + Base64.decode64(params[:content]) + else + params[:content] + end + + sha = repository.commit_file( + current_user, + path, + content, + params[:commit_message], + params[:new_branch] || ref + ) + + after_commit(sha) + 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) end end end diff --git a/app/views/projects/blob/new.html.haml b/app/views/projects/blob/new.html.haml index 7c2a4fece9..dac984f8c3 100644 --- a/app/views/projects/blob/new.html.haml +++ b/app/views/projects/blob/new.html.haml @@ -6,12 +6,11 @@ = render 'shared/commit_message_container', params: params, placeholder: 'Add new file' - - 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" + .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, From fac581b170bebb88a7082a1c141b69bc8fed897f Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:49:29 +0200 Subject: [PATCH 06/14] Revert "Make web editor work correctly after switch from satellites" This reverts commit 435f680b897b892103fa157d4699dbb6d9ecf758. --- app/services/files/base_service.rb | 3 +-- app/services/files/create_service.rb | 2 +- app/services/files/delete_service.rb | 2 +- app/services/files/update_service.rb | 2 +- app/services/git_push_service.rb | 3 +-- 5 files changed, 5 insertions(+), 7 deletions(-) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 4d02752454..29013be0f9 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -17,8 +17,7 @@ module Files def after_commit(sha) commit = repository.commit(sha) full_ref = 'refs/heads/' + (params[:new_branch] || ref) - old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA - GitPushService.new.execute(project, current_user, old_sha, sha, full_ref) + GitPushService.new.execute(project, current_user, commit.parent_id, sha, full_ref) end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 0a80455bc6..bafc3565da 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -1,7 +1,7 @@ require_relative "base_service" module Files - class CreateService < Files::BaseService + class CreateService < BaseService def execute allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 2281777604..fabcdc1964 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -1,7 +1,7 @@ require_relative "base_service" module Files - class DeleteService < Files::BaseService + class DeleteService < BaseService def execute allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 013cc1ee32..c972f8322b 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -1,7 +1,7 @@ require_relative "base_service" module Files - class UpdateService < Files::BaseService + class UpdateService < BaseService def execute allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index 3511392d1d..5a2c97b08a 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -133,8 +133,7 @@ class GitPushService end def is_default_branch?(ref) - Gitlab::Git.branch_ref?(ref) && - (Gitlab::Git.ref_name(ref) == project.default_branch || project.default_branch.nil?) + Gitlab::Git.branch_ref?(ref) && Gitlab::Git.ref_name(ref) == project.default_branch end def commit_user(commit) From f22f7a6f516b7d4febbeb0c8ab95e0336aa5c5f5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:50:43 +0200 Subject: [PATCH 07/14] Revert "Create activity event and execute hooks on web editor commit" This reverts commit 3d416f1682c5e6a6ac1ea7013f66bbd0d23b452c. --- app/services/files/base_service.rb | 6 ------ app/services/files/create_service.rb | 5 ++--- app/services/files/update_service.rb | 3 +-- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 29013be0f9..bd24510095 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -13,11 +13,5 @@ module Files def repository project.repository end - - def after_commit(sha) - commit = repository.commit(sha) - full_ref = 'refs/heads/' + (params[:new_branch] || ref) - GitPushService.new.execute(project, current_user, commit.parent_id, sha, full_ref) - end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index bafc3565da..3516cf30db 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -40,7 +40,7 @@ module Files params[:content] end - sha = repository.commit_file( + created_successfully = repository.commit_file( current_user, file_path, content, @@ -49,8 +49,7 @@ module Files ) - if sha - after_commit(sha) + if created_successfully success else error("Your changes could not be committed, because the file has been changed") diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index c972f8322b..4d7ac3b750 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -26,7 +26,7 @@ module Files params[:content] end - sha = repository.commit_file( + repository.commit_file( current_user, path, content, @@ -34,7 +34,6 @@ module Files params[:new_branch] || ref ) - after_commit(sha) success rescue Gitlab::Satellite::CheckoutFailed => ex error("Your changes could not be committed because ref '#{ref}' could not be checked out", 400) From 3ffd63888dbb02cc8b2a4da951a25ebc5d68cfe5 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:50:48 +0200 Subject: [PATCH 08/14] Revert "Use rugged in web editor for base64 encoding" This reverts commit 541133197be098732cdc14a12aa059e21cac3d71. --- app/services/files/create_service.rb | 31 +++++++++++++++------------- app/services/files/update_service.rb | 31 +++++++++++++++------------- 2 files changed, 34 insertions(+), 28 deletions(-) diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 3516cf30db..21065f7151 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -33,20 +33,23 @@ module Files end end - content = - if params[:encoding] == 'base64' - Base64.decode64(params[:content]) - else - params[:content] - end - - created_successfully = repository.commit_file( - current_user, - file_path, - content, - params[:commit_message], - params[:new_branch] || ref - ) + if params[:encoding] == 'base64' + 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] + ) + else + created_successfully = repository.commit_file( + current_user, + file_path, + params[:content], + params[:commit_message], + params[:new_branch] || ref + ) + end if created_successfully diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 4d7ac3b750..5efd43d16c 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -19,20 +19,23 @@ module Files return error("You can only edit text files") end - content = - if params[:encoding] == 'base64' - Base64.decode64(params[:content]) - else - params[:content] - end - - repository.commit_file( - current_user, - path, - content, - params[:commit_message], - params[:new_branch] || ref - ) + if params[:encoding] == 'base64' + 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] + ) + else + repository.commit_file( + current_user, + path, + params[:content], + params[:commit_message], + params[:new_branch] || ref + ) + end success rescue Gitlab::Satellite::CheckoutFailed => ex From aeec708f53864f3af12c9669f212ad6737ff4121 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:51:26 +0200 Subject: [PATCH 09/14] Revert "Fix adding new file to empty repo" This reverts commit 27a158506e033acd7195acf91995c1574e122832. Signed-off-by: Dmitriy Zaporozhets --- app/services/files/create_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 21065f7151..c0cf595632 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -42,7 +42,7 @@ module Files params[:new_branch] ) else - created_successfully = repository.commit_file( + created_successfull = repository.commit_file( current_user, file_path, params[:content], From b1958385cd30394c13ec5df300ace41394c82881 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:52:33 +0200 Subject: [PATCH 10/14] Revert "Create and edit files in web editor via rugged" This reverts commit 734a4ba87de7bc8cf152c5bc7f93ba04210b282d. Signed-off-by: Dmitriy Zaporozhets --- app/models/repository.rb | 47 ---------------------------- app/services/files/create_service.rb | 24 +++++--------- app/services/files/update_service.rb | 24 +++++--------- 3 files changed, 14 insertions(+), 81 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index dae19b9ddc..1b03cd848c 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -364,53 +364,6 @@ class Repository @root_ref ||= raw_repository.root_ref end - def commit_file(user, path, content, message, 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 - - def remove_file(user, path, message, 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 - - def user_to_comitter(user) - { - email: user.email, - name: user.name, - time: Time.now - } - end - def search_files(query, ref) offset = 2 args = %W(git grep -i -n --before-context #{offset} --after-context #{offset} #{query} #{ref || root_ref}) diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index c0cf595632..23833aa78e 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -33,24 +33,14 @@ module Files end end - if params[:encoding] == 'base64' - 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] - ) - else - created_successfull = repository.commit_file( - current_user, - file_path, - params[:content], - params[:commit_message], - params[:new_branch] || ref - ) - 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 diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 5efd43d16c..0724d3ae63 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -19,23 +19,13 @@ module Files return error("You can only edit text files") end - if params[:encoding] == 'base64' - 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] - ) - else - repository.commit_file( - current_user, - path, - params[:content], - params[:commit_message], - params[:new_branch] || ref - ) - 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 From d4bf7848cbc1a00bc00d27f24e08d7a38900189b Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 13:04:38 +0200 Subject: [PATCH 11/14] Revert "Merge branch 'web-editor-rugged' into 'master'" This reverts commit 5a1aa49b5533593dc4c6de82279fe44f5f15616c, reversing changes made to a675bea2c1c1d5d6923cb97b8714eb72d4e4ff9b. Signed-off-by: Dmitriy Zaporozhets --- app/services/files/delete_service.rb | 13 ++-- app/views/projects/blob/_editor.html.haml | 4 +- .../satellite/files/delete_file_action.rb | 50 ++++++++++++++ .../satellite/files/edit_file_action.rb | 68 +++++++++++++++++++ lib/gitlab/satellite/files/file_action.rb | 25 +++++++ lib/gitlab/satellite/files/new_file_action.rb | 67 ++++++++++++++++++ spec/requests/api/files_spec.rb | 52 ++++++++++++-- 7 files changed, 266 insertions(+), 13 deletions(-) create mode 100644 lib/gitlab/satellite/files/delete_file_action.rb create mode 100644 lib/gitlab/satellite/files/edit_file_action.rb create mode 100644 lib/gitlab/satellite/files/file_action.rb create mode 100644 lib/gitlab/satellite/files/new_file_action.rb diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index fabcdc1964..1497a0f883 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -19,15 +19,14 @@ module Files return error("You can only edit text files") end - sha = repository.remove_file( - current_user, - path, - params[:commit_message], - ref + delete_file_action = Gitlab::Satellite::DeleteFileAction.new(current_user, project, ref, path) + + deleted_successfully = delete_file_action.commit!( + nil, + params[:commit_message] ) - if sha - after_commit(sha) + if deleted_successfully success else error("Your changes could not be committed, because the file has been changed") diff --git a/app/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml index 9c3e1703c8..96f188e4aa 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/lib/gitlab/satellite/files/delete_file_action.rb b/lib/gitlab/satellite/files/delete_file_action.rb new file mode 100644 index 0000000000..0d37b9dea8 --- /dev/null +++ b/lib/gitlab/satellite/files/delete_file_action.rb @@ -0,0 +1,50 @@ +require_relative 'file_action' + +module Gitlab + module Satellite + class DeleteFileAction < FileAction + # Deletes file and creates a new commit for it + # + # Returns false if committing the change fails + # Returns false if pushing from the satellite to bare repo failed or was rejected + # Returns true otherwise + def commit!(content, commit_message) + in_locked_and_timed_satellite do |repo| + prepare_satellite!(repo) + + # create target branch in satellite at the corresponding commit from bare repo + repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}") + + # update the file in the satellite's working dir + file_path_in_satellite = File.join(repo.working_dir, file_path) + + # Prevent relative links + unless safe_path?(file_path_in_satellite) + Gitlab::GitLogger.error("FileAction: Relative path not allowed") + return false + end + + File.delete(file_path_in_satellite) + + # add removed file + repo.remove(file_path_in_satellite) + + # commit the changes + # will raise CommandFailed when commit fails + repo.git.commit(raise: true, timeout: true, a: true, m: commit_message) + + + # push commit back to bare repo + # will raise CommandFailed when push fails + repo.git.push({ raise: true, timeout: true }, :origin, ref) + + # everything worked + true + end + rescue Grit::Git::CommandFailed => ex + Gitlab::GitLogger.error(ex.message) + false + end + end + end +end diff --git a/lib/gitlab/satellite/files/edit_file_action.rb b/lib/gitlab/satellite/files/edit_file_action.rb new file mode 100644 index 0000000000..3cb9c0b5ec --- /dev/null +++ b/lib/gitlab/satellite/files/edit_file_action.rb @@ -0,0 +1,68 @@ +require_relative 'file_action' + +module Gitlab + module Satellite + # GitLab server-side file update and commit + class EditFileAction < FileAction + # Updates the files content and creates a new commit for it + # + # Returns false if the ref has been updated while editing the file + # Returns false if committing the change fails + # Returns false if pushing from the satellite to bare repo failed or was rejected + # Returns true otherwise + def commit!(content, commit_message, encoding, new_branch = nil) + in_locked_and_timed_satellite do |repo| + prepare_satellite!(repo) + + # create target branch in satellite at the corresponding commit from bare repo + begin + repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}") + rescue Grit::Git::CommandFailed => ex + log_and_raise(CheckoutFailed, ex.message) + end + + # update the file in the satellite's working dir + file_path_in_satellite = File.join(repo.working_dir, file_path) + + # Prevent relative links + unless safe_path?(file_path_in_satellite) + Gitlab::GitLogger.error("FileAction: Relative path not allowed") + return false + end + + # Write file + write_file(file_path_in_satellite, content, encoding) + + # commit the changes + # will raise CommandFailed when commit fails + begin + repo.git.commit(raise: true, timeout: true, a: true, m: commit_message) + rescue Grit::Git::CommandFailed => ex + log_and_raise(CommitFailed, ex.message) + end + + + target_branch = new_branch.present? ? "#{ref}:#{new_branch}" : ref + + # push commit back to bare repo + # will raise CommandFailed when push fails + begin + repo.git.push({ raise: true, timeout: true }, :origin, target_branch) + rescue Grit::Git::CommandFailed => ex + log_and_raise(PushFailed, ex.message) + end + + # everything worked + true + end + end + + private + + def log_and_raise(errorClass, message) + Gitlab::GitLogger.error(message) + raise(errorClass, message) + end + end + end +end diff --git a/lib/gitlab/satellite/files/file_action.rb b/lib/gitlab/satellite/files/file_action.rb new file mode 100644 index 0000000000..6446b14568 --- /dev/null +++ b/lib/gitlab/satellite/files/file_action.rb @@ -0,0 +1,25 @@ +module Gitlab + module Satellite + class FileAction < Action + attr_accessor :file_path, :ref + + def initialize(user, project, ref, file_path) + super user, project + @file_path = file_path + @ref = ref + end + + def safe_path?(path) + File.absolute_path(path) == path + end + + def write_file(abs_file_path, content, file_encoding = 'text') + if file_encoding == 'base64' + File.open(abs_file_path, 'wb') { |f| f.write(Base64.decode64(content)) } + else + File.open(abs_file_path, 'w') { |f| f.write(content) } + end + end + end + end +end diff --git a/lib/gitlab/satellite/files/new_file_action.rb b/lib/gitlab/satellite/files/new_file_action.rb new file mode 100644 index 0000000000..724dfa0d04 --- /dev/null +++ b/lib/gitlab/satellite/files/new_file_action.rb @@ -0,0 +1,67 @@ +require_relative 'file_action' + +module Gitlab + module Satellite + class NewFileAction < FileAction + # Updates the files content and creates a new commit for it + # + # Returns false if the ref has been updated while editing the file + # Returns false if committing the change fails + # Returns false if pushing from the satellite to bare repo failed or was rejected + # Returns true otherwise + def commit!(content, commit_message, encoding, new_branch = nil) + in_locked_and_timed_satellite do |repo| + prepare_satellite!(repo) + + # create target branch in satellite at the corresponding commit from bare repo + current_ref = + if @project.empty_repo? + # skip this step if we want to add first file to empty repo + Satellite::PARKING_BRANCH + else + repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}") + ref + end + + file_path_in_satellite = File.join(repo.working_dir, file_path) + dir_name_in_satellite = File.dirname(file_path_in_satellite) + + # Prevent relative links + unless safe_path?(file_path_in_satellite) + Gitlab::GitLogger.error("FileAction: Relative path not allowed") + return false + end + + # Create dir if not exists + FileUtils.mkdir_p(dir_name_in_satellite) + + # Write file + write_file(file_path_in_satellite, content, encoding) + + # add new file + repo.add(file_path_in_satellite) + + # commit the changes + # will raise CommandFailed when commit fails + repo.git.commit(raise: true, timeout: true, a: true, m: commit_message) + + target_branch = if new_branch.present? && !@project.empty_repo? + "#{ref}:#{new_branch}" + else + "#{current_ref}:#{ref}" + end + + # push commit back to bare repo + # will raise CommandFailed when push fails + repo.git.push({ raise: true, timeout: true }, :origin, target_branch) + + # everything worked + true + end + rescue Grit::Git::CommandFailed => ex + Gitlab::GitLogger.error(ex.message) + false + end + end + end +end diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 6c7860511e..346f1e29d4 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -49,6 +49,10 @@ describe API::API, api: true do end it "should create a new file in project repo" do + Gitlab::Satellite::NewFileAction.any_instance.stub( + commit!: true, + ) + post api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(201) expect(json_response['file_path']).to eq('newfile.rb') @@ -59,9 +63,10 @@ describe API::API, api: true do expect(response.status).to eq(400) end - it "should return a 400 if editor fails to create file" do - allow_any_instance_of(Repository).to receive(:commit_file). - and_return(false) + it "should return a 400 if satellite fails to create file" do + Gitlab::Satellite::NewFileAction.any_instance.stub( + commit!: false, + ) post api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) @@ -79,6 +84,10 @@ describe API::API, api: true do end it "should update existing file in project repo" do + Gitlab::Satellite::EditFileAction.any_instance.stub( + commit!: true, + ) + put api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(200) expect(json_response['file_path']).to eq(file_path) @@ -88,6 +97,35 @@ describe API::API, api: true do put api("/projects/#{project.id}/repository/files", user) expect(response.status).to eq(400) end + + it 'should return a 400 if the checkout fails' do + Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) + .and_raise(Gitlab::Satellite::CheckoutFailed) + + put api("/projects/#{project.id}/repository/files", user), valid_params + expect(response.status).to eq(400) + + ref = valid_params[:branch_name] + expect(response.body).to match("ref '#{ref}' could not be checked out") + end + + it 'should return a 409 if the file was not modified' do + Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) + .and_raise(Gitlab::Satellite::CommitFailed) + + put api("/projects/#{project.id}/repository/files", user), valid_params + expect(response.status).to eq(409) + expect(response.body).to match("Maybe there was nothing to commit?") + end + + it 'should return a 409 if the push fails' do + Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) + .and_raise(Gitlab::Satellite::PushFailed) + + put api("/projects/#{project.id}/repository/files", user), valid_params + expect(response.status).to eq(409) + expect(response.body).to match("Maybe the file was changed by another process?") + end end describe "DELETE /projects/:id/repository/files" do @@ -100,6 +138,10 @@ describe API::API, api: true do end it "should delete existing file in project repo" do + Gitlab::Satellite::DeleteFileAction.any_instance.stub( + commit!: true, + ) + delete api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(200) expect(json_response['file_path']).to eq(file_path) @@ -111,7 +153,9 @@ describe API::API, api: true do end it "should return a 400 if satellite fails to create file" do - allow_any_instance_of(Repository).to receive(:remove_file).and_return(false) + Gitlab::Satellite::DeleteFileAction.any_instance.stub( + commit!: false, + ) delete api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) From 18db872bc504805a33ef3a99708d22c61e822fcc Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 13:29:35 +0200 Subject: [PATCH 12/14] Revert "Fix editing files via API" This reverts commit 7bde6ae540bab5c93a83bfe26102674adba0eab5. --- lib/api/files.rb | 50 +++++++++++++++++++++++------------------------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/lib/api/files.rb b/lib/api/files.rb index c7b30cf2f0..e0ea6d7dd1 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -3,26 +3,6 @@ module API class Files < Grape::API before { authenticate! } - helpers do - def commit_params(attrs) - { - file_path: attrs[:file_path], - current_branch: attrs[:branch_name], - target_branch: attrs[:branch_name], - commit_message: attrs[:commit_message], - file_content: attrs[:content], - file_content_encoding: attrs[:encoding] - } - end - - def commit_response(attrs) - { - file_path: attrs[:file_path], - branch_name: attrs[:branch_name], - } - end - end - resource :projects do # Get file from repository # File content is Base64 encoded @@ -93,11 +73,17 @@ module API required_attributes! [:file_path, :branch_name, :content, :commit_message] attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding] - result = ::Files::CreateService.new(user_project, current_user, commit_params(attrs)).execute + branch_name = attrs.delete(:branch_name) + file_path = attrs.delete(:file_path) + result = ::Files::CreateService.new(user_project, current_user, attrs, branch_name, file_path).execute if result[:status] == :success status(201) - commit_response(attrs) + + { + file_path: file_path, + branch_name: branch_name + } else render_api_error!(result[:message], 400) end @@ -119,11 +105,17 @@ module API required_attributes! [:file_path, :branch_name, :content, :commit_message] attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding] - result = ::Files::UpdateService.new(user_project, current_user, commit_params(attrs)).execute + branch_name = attrs.delete(:branch_name) + file_path = attrs.delete(:file_path) + result = ::Files::UpdateService.new(user_project, current_user, attrs, branch_name, file_path).execute if result[:status] == :success status(200) - commit_response(attrs) + + { + file_path: file_path, + branch_name: branch_name + } else http_status = result[:http_status] || 400 render_api_error!(result[:message], http_status) @@ -146,11 +138,17 @@ module API required_attributes! [:file_path, :branch_name, :commit_message] attrs = attributes_for_keys [:file_path, :branch_name, :commit_message] - result = ::Files::DeleteService.new(user_project, current_user, commit_params(attrs)).execute + branch_name = attrs.delete(:branch_name) + file_path = attrs.delete(:file_path) + result = ::Files::DeleteService.new(user_project, current_user, attrs, branch_name, file_path).execute if result[:status] == :success status(200) - commit_response(attrs) + + { + file_path: file_path, + branch_name: branch_name + } else render_api_error!(result[:message], 400) end From f181bc6e1e0cd5281361fab5a11bd499024c79b1 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 13:47:37 +0200 Subject: [PATCH 13/14] Fix file api tests Signed-off-by: Dmitriy Zaporozhets --- spec/requests/api/files_spec.rb | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 346f1e29d4..1d14a1729c 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -49,9 +49,7 @@ describe API::API, api: true do end it "should create a new file in project repo" do - Gitlab::Satellite::NewFileAction.any_instance.stub( - commit!: true, - ) + expect_any_instance_of(Gitlab::Satellite::NewFileAction).to receive(:commit!).and_return(true) post api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(201) @@ -64,9 +62,7 @@ describe API::API, api: true do end it "should return a 400 if satellite fails to create file" do - Gitlab::Satellite::NewFileAction.any_instance.stub( - commit!: false, - ) + expect_any_instance_of(Gitlab::Satellite::NewFileAction).to receive(:commit!).and_return(false) post api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) @@ -84,9 +80,7 @@ describe API::API, api: true do end it "should update existing file in project repo" do - Gitlab::Satellite::EditFileAction.any_instance.stub( - commit!: true, - ) + expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_return(true) put api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(200) @@ -99,8 +93,7 @@ describe API::API, api: true do end it 'should return a 400 if the checkout fails' do - Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) - .and_raise(Gitlab::Satellite::CheckoutFailed) + expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::CheckoutFailed) put api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) @@ -110,8 +103,7 @@ describe API::API, api: true do end it 'should return a 409 if the file was not modified' do - Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) - .and_raise(Gitlab::Satellite::CommitFailed) + expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::CommitFailed) put api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(409) @@ -119,8 +111,7 @@ describe API::API, api: true do end it 'should return a 409 if the push fails' do - Gitlab::Satellite::EditFileAction.any_instance.stub(:commit!) - .and_raise(Gitlab::Satellite::PushFailed) + expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::PushFailed) put api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(409) @@ -138,10 +129,7 @@ describe API::API, api: true do end it "should delete existing file in project repo" do - Gitlab::Satellite::DeleteFileAction.any_instance.stub( - commit!: true, - ) - + expect_any_instance_of(Gitlab::Satellite::DeleteFileAction).to receive(:commit!).and_return(true) delete api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(200) expect(json_response['file_path']).to eq(file_path) @@ -153,9 +141,7 @@ describe API::API, api: true do end it "should return a 400 if satellite fails to create file" do - Gitlab::Satellite::DeleteFileAction.any_instance.stub( - commit!: false, - ) + expect_any_instance_of(Gitlab::Satellite::DeleteFileAction).to receive(:commit!).and_return(false) delete api("/projects/#{project.id}/repository/files", user), valid_params expect(response.status).to eq(400) From f4f6d60ca8a9528db92490e2fc893491b54ac0b9 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Wed, 12 Aug 2015 14:17:26 +0300 Subject: [PATCH 14/14] update changelog --- CHANGELOG | 49 ++++--------------------------------------------- 1 file changed, 4 insertions(+), 45 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 15718aab72..0acc3f110f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,52 +1,11 @@ Please view this file on the master branch, on stable branches it's out of date. + +v 7.13.5 + - Satellites reverted + v 7.13.4 - Allow users to send abuse reports -v 7.14.0 (unreleased) - - Show incompatible projects in Bitbucket import status (Stan Hu) - - Fix coloring of diffs on MR Discussion-tab (Gert Goet) - - Fix "Network" and "Graphs" pages for branches with encoded slashes (Stan Hu) - - Fix errors deleting and creating branches with encoded slashes (Stan Hu) - - Always add current user to autocomplete controller to support filter by "Me" (Stan Hu) - - Fix multi-line syntax highlighting (Stan Hu) - - Fix network graph when branch name has single quotes (Stan Hu) - - Add "Confirm user" button in user admin page (Stan Hu) - - Upgrade gitlab_git to version 7.2.6 to fix Error 500 when creating network graphs (Stan Hu) - - Add support for Unicode filenames in relative links (Hiroyuki Sato) - - Fix URL used for refreshing notes if relative_url is present (Bartłomiej Święcki) - - Fix commit data retrieval when branch name has single quotes (Stan Hu) - - Check that project was actually created rather than just validated in import:repos task (Stan Hu) - - Fix full screen mode for snippet comments (Daniel Gerhardt) - - Fix 404 error in files view after deleting the last file in a repository (Stan Hu) - - Fix the "Reload with full diff" URL button (Stan Hu) - - Fix label read access for unauthenticated users (Daniel Gerhardt) - - Fix access to disabled features for unauthenticated users (Daniel Gerhardt) - - Fix OAuth provider bug where GitLab would not go return to the redirect_uri after sign-in (Stan Hu) - - Fix file upload dialog for comment editing (Daniel Gerhardt) - - Set OmniAuth full_host parameter to ensure redirect URIs are correct (Stan Hu) - - Return comments in created order in merge request API (Stan Hu) - - Expire Rails cache entries after two weeks to prevent endless Redis growth - - Add support for destroying project milestones (Stan Hu) - - Add fetch command to the MR page. - - Allow custom backup archive permissions - - Add fetch command to the MR page - - Add project star and fork count, group avatar URL and user/group web URL attributes to API - - Fix bug causing Bitbucket importer to crash when OAuth application had been removed. - - Add fetch command to the MR page. - - Show who last edited a comment if it wasn't the original author - - Send notification to all participants when MR is merged. - - Add ability to manage user email addresses via the API. - - Show buttons to add license, changelog and contribution guide if they're missing. - - Tweak project page buttons. - - Disabled autocapitalize and autocorrect on login field (Daryl Chan) - - Mention group and project name in creation, update and deletion notices (Achilleas Pipinellis) - - Update gravatar link on profile page to link to configured gravatar host (Ben Bodenmiller) - - Remove redis-store TTL monkey patch - - Add support for CI skipped status - - Fetch code from forks to refs/merge-requests/:id/head when merge request created - - Remove comments and email addresses when publicly exposing ssh keys (Zeger-Jan van de Weg) - - Cache all events - v 7.13.3 - Fix bug causing Bitbucket importer to crash when OAuth application had been removed.