diff --git a/CHANGELOG b/CHANGELOG index 699824e9af464dcf8aa6dcc699ef3affdefa6769..870ab59afa5d9dd3b0a3f90b7469ee7c73747842 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -38,6 +38,7 @@ v 7.12.0 (unreleased) - 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) v 7.11.4 - Fix missing bullets when creating lists diff --git a/Gemfile b/Gemfile index f5082f626a8ba89f6ae7a08c7a241781b5e9f7ed..78af7f5db69a8c290e73a9c40ba881b5dc695973 100644 --- a/Gemfile +++ b/Gemfile @@ -45,7 +45,7 @@ gem "browser" # Extracting information from a git repository # Provide access to Gitlab::Git library -gem "gitlab_git", '~> 7.1.13' +gem "gitlab_git", '~> 7.2.2' # Ruby/Rack Git Smart-HTTP Server Handler # GitLab fork with a lot of changes (improved thread-safety, better memory usage etc) diff --git a/Gemfile.lock b/Gemfile.lock index cc373f5a0d7dd3e79bea5956d78b5a7a7e3d1e1a..bbc5639c84fa0735cbe8e78f4fd4c879bc791f4c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -225,7 +225,7 @@ GEM mime-types (~> 1.19) gitlab_emoji (0.1.0) gemojione (~> 2.0) - gitlab_git (7.1.13) + gitlab_git (7.2.2) activesupport (~> 4.0) charlock_holmes (~> 0.6) gitlab-linguist (~> 3.0) @@ -733,7 +733,7 @@ DEPENDENCIES gitlab-grack (~> 2.0.2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) - gitlab_git (~> 7.1.13) + gitlab_git (~> 7.2.2) gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) diff --git a/app/models/repository.rb b/app/models/repository.rb index 1b8c74028d9aa1f6d27bab3717f972a488566d3b..1ca970176379dce8350439fe2061b98552db59fb 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -370,8 +370,55 @@ 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 + private + def user_to_comitter(user) + { + email: user.email, + name: user.name, + time: Time.now + } + end + def cache @cache ||= RepositoryCache.new(path_with_namespace) end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index bd245100955887034435ac0fe1c0a0584eaec9a9..4d02752454eda8eed4fb32ca8183f105581de5bb 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -13,5 +13,12 @@ module Files def repository project.repository end + + 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) + end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 23833aa78ec81e14cea22f95933f5064f1aabd32..0a80455bc6bc4094d61ad220ffac295ac9d553e3 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 < BaseService + class CreateService < Files::BaseService def execute allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) @@ -33,16 +33,24 @@ module Files end end + content = + if params[:encoding] == 'base64' + Base64.decode64(params[:content]) + else + params[:content] + end - new_file_action = Gitlab::Satellite::NewFileAction.new(current_user, project, ref, file_path) - created_successfully = new_file_action.commit!( - params[:content], + sha = repository.commit_file( + current_user, + file_path, + content, params[:commit_message], - params[:encoding], - params[:new_branch] + params[:new_branch] || ref ) - if created_successfully + + if sha + after_commit(sha) success else error("Your changes could not be committed, because the file has been changed") diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 1497a0f883bcb32c5c85cdb3400a3960ea539de0..2281777604c84daf6d2243e008aeb5b606fe9a92 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 < BaseService + class DeleteService < Files::BaseService def execute allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) @@ -19,14 +19,15 @@ module Files 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] + sha = repository.remove_file( + current_user, + path, + params[:commit_message], + ref ) - if deleted_successfully + if sha + after_commit(sha) 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 0724d3ae6347b1150c0c40f81cb36fbdb7c9dea8..013cc1ee322209414f43f1d430eca16f0d6050a6 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 < BaseService + class UpdateService < Files::BaseService def execute allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) @@ -19,14 +19,22 @@ module Files 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], + 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[:encoding], - params[:new_branch] + 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) diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb index bdf36af02fdba0adee2e16f2e178ff2079bdef00..cde65349d5c2d016f1221c45ffa40e21eebec55a 100644 --- a/app/services/git_push_service.rb +++ b/app/services/git_push_service.rb @@ -127,7 +127,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/views/projects/blob/_editor.html.haml b/app/views/projects/blob/_editor.html.haml index 96f188e4aa7ab644487318d53faf2a021a4053b7..9c3e1703c893148dc66df5cf6e31ccaa684104c8 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 deleted file mode 100644 index 0d37b9dea8502e765aed57cfedfea514f29dcc9e..0000000000000000000000000000000000000000 --- a/lib/gitlab/satellite/files/delete_file_action.rb +++ /dev/null @@ -1,50 +0,0 @@ -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 deleted file mode 100644 index 3cb9c0b5ecbceed9529812162d235befca60ffb7..0000000000000000000000000000000000000000 --- a/lib/gitlab/satellite/files/edit_file_action.rb +++ /dev/null @@ -1,68 +0,0 @@ -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 deleted file mode 100644 index 6446b14568a5de087d3e0cbbf91506dacf24fa74..0000000000000000000000000000000000000000 --- a/lib/gitlab/satellite/files/file_action.rb +++ /dev/null @@ -1,25 +0,0 @@ -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 deleted file mode 100644 index 724dfa0d042e1a80376a40dad4a34967998fdf5e..0000000000000000000000000000000000000000 --- a/lib/gitlab/satellite/files/new_file_action.rb +++ /dev/null @@ -1,67 +0,0 @@ -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 bab8888a631277b9c79360405c3fa5bdf6c50ac2..15f547e128d3ec993dc982ebf49dbe23f0b107a6 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -49,10 +49,6 @@ describe API::API, api: true do } 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') @@ -63,9 +59,9 @@ describe API::API, api: true do expect(response.status).to eq(400) end - it "should return a 400 if satellite fails to create file" do - Gitlab::Satellite::NewFileAction.any_instance.stub( - commit!: false, + it "should return a 400 if editor fails to create file" do + Repository.any_instance.stub( + commit_file: false, ) post api("/projects/#{project.id}/repository/files", user), valid_params @@ -84,10 +80,6 @@ describe API::API, api: true do } 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) @@ -97,35 +89,6 @@ 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 @@ -138,10 +101,6 @@ describe API::API, api: true do } 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) @@ -153,8 +112,8 @@ 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, + Repository.any_instance.stub( + remove_file: false, ) delete api("/projects/#{project.id}/repository/files", user), valid_params