summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-08-11 13:04:38 +0200
committerValery Sizov <vsv2711@gmail.com>2015-08-12 13:53:01 +0300
commitd4bf7848cbc1a00bc00d27f24e08d7a38900189b (patch)
treecf066dbf41e3a433e91c1119cefa20f95324fe33
parentb1958385cd30394c13ec5df300ace41394c82881 (diff)
downloadgitlab-ce-d4bf7848cbc1a00bc00d27f24e08d7a38900189b.tar.gz
Revert "Merge branch 'web-editor-rugged' into 'master'"
This reverts commit 5a1aa49b5533593dc4c6de82279fe44f5f15616c, reversing changes made to a675bea2c1c1d5d6923cb97b8714eb72d4e4ff9b. Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>
-rw-r--r--app/services/files/delete_service.rb13
-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.rb52
7 files changed, 266 insertions, 13 deletions
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index fabcdc19648..1497a0f883b 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 9c3e1703c89..96f188e4aa7 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 00000000000..0d37b9dea85
--- /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 00000000000..3cb9c0b5ecb
--- /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 00000000000..6446b14568a
--- /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 00000000000..724dfa0d042
--- /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 6c7860511e8..346f1e29d48 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)