summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRubén Dávila <ruben@gitlab.com>2017-12-15 23:24:12 -0500
committerRubén Dávila <ruben@gitlab.com>2017-12-20 01:24:53 -0500
commitc210ddab9289e29fadc9a5a0462ffbe864af736c (patch)
treef64ad586aba389319be6e07d14b117f0bf048761
parent81331fa77e181d193f444c34443d6ef0c5b01ed6 (diff)
downloadgitlab-ce-c210ddab9289e29fadc9a5a0462ffbe864af736c.tar.gz
Check if file has been modified for each action provided.
When commiting multiple files we're now checking if any of those files has been modified by another commit and we're rejecting the new commit in this case.
-rw-r--r--app/services/files/multi_service.rb12
-rw-r--r--changelogs/unreleased/15922-validate-file-status-when-commiting-multiple-files.yml5
-rw-r--r--doc/api/commits.md1
-rw-r--r--spec/services/files/multi_service_spec.rb45
4 files changed, 61 insertions, 2 deletions
diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb
index bfacc462847..eb2773733b1 100644
--- a/app/services/files/multi_service.rb
+++ b/app/services/files/multi_service.rb
@@ -20,6 +20,7 @@ module Files
params[:actions].each do |action|
validate_action!(action)
+ validate_file_status(action)
end
end
@@ -28,5 +29,16 @@ module Files
raise_error("Unknown action '#{action[:action]}'")
end
end
+
+ def validate_file_status(action)
+ return unless action[:last_commit_id]
+
+ current_commit = Gitlab::Git::Commit.last_for_path(
+ @start_project.repository, @start_branch, action[:file_path])
+
+ if current_commit.sha != action[:last_commit_id]
+ raise_error("The file has changed since you started editing it: #{action[:file_path]}")
+ end
+ end
end
end
diff --git a/changelogs/unreleased/15922-validate-file-status-when-commiting-multiple-files.yml b/changelogs/unreleased/15922-validate-file-status-when-commiting-multiple-files.yml
new file mode 100644
index 00000000000..db2bd6e692b
--- /dev/null
+++ b/changelogs/unreleased/15922-validate-file-status-when-commiting-multiple-files.yml
@@ -0,0 +1,5 @@
+---
+title: 'Validate file status when commiting multiple files'
+merge_request: 15922
+author:
+type: added
diff --git a/doc/api/commits.md b/doc/api/commits.md
index 5a4a8d888b3..7de08d230dd 100644
--- a/doc/api/commits.md
+++ b/doc/api/commits.md
@@ -84,6 +84,7 @@ POST /projects/:id/repository/commits
| `previous_path` | string | no | Original full path to the file being moved. Ex. `lib/class1.rb` |
| `content` | string | no | File content, required for all except `delete`. Optional for `move` |
| `encoding` | string | no | `text` or `base64`. `text` is default. |
+| `last_commit_id` | string | no | Last known file commit id |
```bash
PAYLOAD=$(cat << 'JSON'
diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb
index 34457ee796a..6e11f126f14 100644
--- a/spec/services/files/multi_service_spec.rb
+++ b/spec/services/files/multi_service_spec.rb
@@ -6,14 +6,20 @@ describe Files::MultiService do
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:branch_name) { project.default_branch }
+ let(:file_path) { 'files/ruby/popen.rb' }
let(:action) { 'update' }
+ let!(:original_commit_id) do
+ Gitlab::Git::Commit.last_for_path(project.repository, branch_name, file_path).sha
+ end
+
let(:actions) do
[
{
action: action,
- file_path: 'files/ruby/popen.rb',
- content: 'New content'
+ file_path: file_path,
+ content: 'New content',
+ last_commit_id: original_commit_id
}
]
end
@@ -50,5 +56,40 @@ describe Files::MultiService do
expect(results[:message]).to match(/Unknown action/)
end
end
+
+ describe 'Updating files' do
+ context 'when the file has been previously updated' do
+ before do
+ update_file(file_path)
+ end
+
+ it 'rejects the commit' do
+ results = subject.execute
+
+ expect(results[:status]).to eq(:error)
+ expect(results[:message]).to match(file_path)
+ end
+ end
+
+ context 'when the file have not been modified' do
+ it 'accepts the commit' do
+ results = subject.execute
+
+ expect(results[:status]).to eq(:success)
+ end
+ end
+ end
+ end
+
+ def update_file(path)
+ params = {
+ file_path: path,
+ start_branch: branch_name,
+ branch_name: branch_name,
+ commit_message: 'Update file',
+ file_content: 'New content'
+ }
+
+ Files::UpdateService.new(project, user, params).execute
end
end