summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-06-02 10:41:36 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-06-02 10:41:36 +0000
commit5a1aa49b5533593dc4c6de82279fe44f5f15616c (patch)
treefb9711dc560490dbb422fcec95ff996a9e4ec7f6
parenta675bea2c1c1d5d6923cb97b8714eb72d4e4ff9b (diff)
parentd684b11054ea2b5577f5d843170759609227bf22 (diff)
downloadgitlab-ce-5a1aa49b5533593dc4c6de82279fe44f5f15616c.tar.gz
Merge branch 'web-editor-rugged' into 'master'
Create and edit files in web editor via rugged - [x] create file via rugged - [x] update file via rugged - [x] remove file via rugged - [ ] fix tests - [x] remove satellites code - [x] create activity event for new/edit file via rugged - [x] base64 support Part of https://dev.gitlab.org/gitlab/gitlabhq/issues/2300 See merge request !751
-rw-r--r--CHANGELOG1
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/models/repository.rb47
-rw-r--r--app/services/files/base_service.rb7
-rw-r--r--app/services/files/create_service.rb22
-rw-r--r--app/services/files/delete_service.rb15
-rw-r--r--app/services/files/update_service.rb20
-rw-r--r--app/services/git_push_service.rb3
-rw-r--r--app/views/projects/blob/_editor.html.haml4
-rw-r--r--lib/gitlab/satellite/files/delete_file_action.rb50
-rw-r--r--lib/gitlab/satellite/files/edit_file_action.rb68
-rw-r--r--lib/gitlab/satellite/files/file_action.rb25
-rw-r--r--lib/gitlab/satellite/files/new_file_action.rb67
-rw-r--r--spec/requests/api/files_spec.rb51
15 files changed, 104 insertions, 282 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 699824e9af4..870ab59afa5 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 f5082f626a8..78af7f5db69 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 cc373f5a0d7..bbc5639c84f 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 1b8c74028d9..1ca97017637 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 bd245100955..4d02752454e 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 23833aa78ec..0a80455bc6b 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 1497a0f883b..2281777604c 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 0724d3ae634..013cc1ee322 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 bdf36af02fd..cde65349d5c 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 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/lib/gitlab/satellite/files/delete_file_action.rb b/lib/gitlab/satellite/files/delete_file_action.rb
deleted file mode 100644
index 0d37b9dea85..00000000000
--- 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 3cb9c0b5ecb..00000000000
--- 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 6446b14568a..00000000000
--- 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 724dfa0d042..00000000000
--- 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 bab8888a631..15f547e128d 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