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