From 271776d4aa25a23b6f58c6befa94a240e61d4120 Mon Sep 17 00:00:00 2001 From: Jacopo Date: Sun, 23 Sep 2018 12:48:29 +0200 Subject: [PATCH] Adds chmod action to POST /projects/:id/repository/commits API With this action the user can update the execute_filemode of a given file in the repository. --- GITALY_SERVER_VERSION | 2 +- app/services/files/multi_service.rb | 2 +- ...832-adds-chdmod-to-commits-actions-api.yml | 5 ++ doc/api/commits.md | 12 ++- lib/api/commits.rb | 21 ++++- lib/gitlab/gitaly_client/operation_service.rb | 3 +- spec/requests/api/commits_spec.rb | 80 ++++++++++++++++--- spec/services/files/multi_service_spec.rb | 36 ++++++++- 8 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 changelogs/unreleased/43832-adds-chdmod-to-commits-actions-api.yml diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 61825a7bf03..ee1e4d2aee5 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.121.0 +0.122.0 diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index 08088f8c592..c9d3ee31d82 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -2,7 +2,7 @@ module Files class MultiService < Files::BaseService - UPDATE_FILE_ACTIONS = %w(update move delete).freeze + UPDATE_FILE_ACTIONS = %w(update move delete chmod).freeze def create_commit! transformer = Lfs::FileTransformer.new(project, @branch_name) diff --git a/changelogs/unreleased/43832-adds-chdmod-to-commits-actions-api.yml b/changelogs/unreleased/43832-adds-chdmod-to-commits-actions-api.yml new file mode 100644 index 00000000000..cea1436ae8b --- /dev/null +++ b/changelogs/unreleased/43832-adds-chdmod-to-commits-actions-api.yml @@ -0,0 +1,5 @@ +--- +title: Allows to chmod file with commits API +merge_request: 21866 +author: Jacopo Beschi @jacopo-beschi +type: added diff --git a/doc/api/commits.md b/doc/api/commits.md index 624ed529009..5ff1e1f60e0 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -83,12 +83,13 @@ POST /projects/:id/repository/commits | `actions[]` Attribute | Type | Required | Description | | --------------------- | ---- | -------- | ----------- | -| `action` | string | yes | The action to perform, `create`, `delete`, `move`, `update` | +| `action` | string | yes | The action to perform, `create`, `delete`, `move`, `update`, `chmod`| | `file_path` | string | yes | Full path to the file. Ex. `lib/class.rb` | -| `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` | +| `previous_path` | string | no | Original full path to the file being moved. Ex. `lib/class1.rb`. Only considered for `move` action. | +| `content` | string | no | File content, required for all except `delete` and `chmod`. Optional for `move` | | `encoding` | string | no | `text` or `base64`. `text` is default. | | `last_commit_id` | string | no | Last known file commit id. Will be only considered in update, move and delete actions. | +| `execute_filemode` | boolean | no | When `true/false` enables/disables the execute flag on the file. Only considered for `chmod` action. | ```bash PAYLOAD=$(cat << 'JSON' @@ -115,6 +116,11 @@ PAYLOAD=$(cat << 'JSON' "action": "update", "file_path": "foo/bar5", "content": "new content" + }, + { + "action": "chmod", + "file_path": "foo/bar5", + "execute_filemode": true } ] } diff --git a/lib/api/commits.rb b/lib/api/commits.rb index fcaff35459e..5aeffc8fb99 100644 --- a/lib/api/commits.rb +++ b/lib/api/commits.rb @@ -73,7 +73,26 @@ module API params do requires :branch, type: String, desc: 'Name of the branch to commit into. To create a new branch, also provide `start_branch`.', allow_blank: false requires :commit_message, type: String, desc: 'Commit message' - requires :actions, type: Array[Hash], desc: 'Actions to perform in commit' + requires :actions, type: Array, desc: 'Actions to perform in commit' do + requires :action, type: String, desc: 'The action to perform, `create`, `delete`, `move`, `update`, `chmod`', values: %w[create update move delete chmod].freeze + requires :file_path, type: String, desc: 'Full path to the file. Ex. `lib/class.rb`' + given action: ->(action) { action == 'move' } do + requires :previous_path, type: String, desc: 'Original full path to the file being moved. Ex. `lib/class1.rb`' + end + given action: ->(action) { %w[create move].include? action } do + optional :content, type: String, desc: 'File content' + end + given action: ->(action) { action == 'update' } do + requires :content, type: String, desc: 'File content' + end + optional :encoding, type: String, desc: '`text` or `base64`', default: 'text', values: %w[text base64] + given action: ->(action) { %w[update move delete].include? action } do + optional :last_commit_id, type: String, desc: 'Last known file commit id' + end + given action: ->(action) { action == 'chmod' } do + requires :execute_filemode, type: Boolean, desc: 'When `true/false` enables/disables the execute flag on the file.' + end + end optional :start_branch, type: String, desc: 'Name of the branch to start the new commit from' optional :author_email, type: String, desc: 'Author email for commit' optional :author_name, type: String, desc: 'Author name for commit' diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index 54c78fdb680..0f148614b20 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -333,7 +333,8 @@ module Gitlab action: action[:action].upcase.to_sym, file_path: encode_binary(action[:file_path]), previous_path: encode_binary(action[:previous_path]), - base64_content: action[:encoding] == 'base64' + base64_content: action[:encoding] == 'base64', + execute_filemode: !!action[:execute_filemode] ) rescue RangeError raise ArgumentError, "Unknown action '#{action[:action]}'" diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index d5b31610dad..f3fb88474a4 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -238,7 +238,7 @@ describe API::Commits do describe 'create' do let(:message) { 'Created file' } - let!(:invalid_c_params) do + let(:invalid_c_params) do { branch: 'master', commit_message: message, @@ -251,7 +251,7 @@ describe API::Commits do ] } end - let!(:valid_c_params) do + let(:valid_c_params) do { branch: 'master', commit_message: message, @@ -264,7 +264,7 @@ describe API::Commits do ] } end - let!(:valid_utf8_c_params) do + let(:valid_utf8_c_params) do { branch: 'master', commit_message: message, @@ -315,7 +315,7 @@ describe API::Commits do describe 'delete' do let(:message) { 'Deleted file' } - let!(:invalid_d_params) do + let(:invalid_d_params) do { branch: 'markdown', commit_message: message, @@ -327,7 +327,7 @@ describe API::Commits do ] } end - let!(:valid_d_params) do + let(:valid_d_params) do { branch: 'markdown', commit_message: message, @@ -356,7 +356,7 @@ describe API::Commits do describe 'move' do let(:message) { 'Moved file' } - let!(:invalid_m_params) do + let(:invalid_m_params) do { branch: 'feature', commit_message: message, @@ -370,7 +370,7 @@ describe API::Commits do ] } end - let!(:valid_m_params) do + let(:valid_m_params) do { branch: 'feature', commit_message: message, @@ -401,7 +401,7 @@ describe API::Commits do describe 'update' do let(:message) { 'Updated file' } - let!(:invalid_u_params) do + let(:invalid_u_params) do { branch: 'master', commit_message: message, @@ -414,7 +414,7 @@ describe API::Commits do ] } end - let!(:valid_u_params) do + let(:valid_u_params) do { branch: 'master', commit_message: message, @@ -442,9 +442,57 @@ describe API::Commits do end end + describe 'chmod' do + let(:message) { 'Chmod +x file' } + let(:file_path) { 'files/ruby/popen.rb' } + let(:execute_filemode) { true } + let(:params) do + { + branch: 'master', + commit_message: message, + actions: [ + { + action: 'chmod', + file_path: file_path, + execute_filemode: execute_filemode + } + ] + } + end + + it 'responds with success' do + post api(url, user), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['title']).to eq(message) + end + + context 'when execute_filemode is false' do + let(:execute_filemode) { false } + + it 'responds with success' do + post api(url, user), params + + expect(response).to have_gitlab_http_status(201) + expect(json_response['title']).to eq(message) + end + end + + context "when the file doesn't exists" do + let(:file_path) { 'foo/bar.baz' } + + it "responds with 400" do + post api(url, user), params + + expect(response).to have_gitlab_http_status(400) + expect(json_response['message']).to eq("A file with this name doesn't exist") + end + end + end + describe 'multiple operations' do let(:message) { 'Multiple actions' } - let!(:invalid_mo_params) do + let(:invalid_mo_params) do { branch: 'master', commit_message: message, @@ -468,11 +516,16 @@ describe API::Commits do action: 'update', file_path: 'foo/bar.baz', content: 'puts 8' + }, + { + action: 'chmod', + file_path: 'files/ruby/popen.rb', + execute_filemode: true } ] } end - let!(:valid_mo_params) do + let(:valid_mo_params) do { branch: 'master', commit_message: message, @@ -496,6 +549,11 @@ describe API::Commits do action: 'update', file_path: 'files/ruby/popen.rb', content: 'puts 8' + }, + { + action: 'chmod', + file_path: 'files/ruby/popen.rb', + execute_filemode: true } ] } diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb index 3bdedaf3770..5f3c8e82715 100644 --- a/spec/services/files/multi_service_spec.rb +++ b/spec/services/files/multi_service_spec.rb @@ -11,6 +11,7 @@ describe Files::MultiService do let(:new_file_path) { 'files/ruby/popen.rb' } let(:file_content) { 'New content' } let(:action) { 'update' } + let(:commit_message) { 'Update File' } let!(:original_commit_id) do Gitlab::Git::Commit.last_for_path(project.repository, branch_name, original_file_path).sha @@ -30,7 +31,7 @@ describe Files::MultiService do let(:commit_params) do { - commit_message: "Update File", + commit_message: commit_message, branch_name: branch_name, start_branch: branch_name, actions: actions @@ -84,6 +85,39 @@ describe Files::MultiService do end end + describe 'changing execute_filemode of a file' do + let(:commit_message) { 'Chmod +x file' } + let(:file_path) { original_file_path } + let(:default_action) do + { + action: 'chmod', + file_path: file_path, + execute_filemode: true + } + end + + it 'accepts the commit' do + results = subject.execute + + expect(results[:status]).to eq(:success) + end + + it 'updates the execute_filemode of the file' do + expect { subject.execute }.to change { repository.blob_at_branch(branch_name, file_path).mode }.from('100644').to('100755') + end + + context "when the file doesn't exists" do + let(:file_path) { 'files/wrong_path.rb' } + + it 'rejects the commit' do + results = subject.execute + + expect(results[:status]).to eq(:error) + expect(results[:message]).to eq("A file with this name doesn't exist") + end + end + end + context 'when moving a file' do let(:action) { 'move' } let(:new_file_path) { 'files/ruby/new_popen.rb' }