diff options
| -rw-r--r-- | GITALY_SERVER_VERSION | 2 | ||||
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | Gemfile.lock | 4 | ||||
| -rw-r--r-- | Gemfile.rails4.lock | 4 | ||||
| -rw-r--r-- | app/services/files/multi_service.rb | 11 | ||||
| -rw-r--r-- | changelogs/unreleased/51083-fix-move-api.yml | 5 | ||||
| -rw-r--r-- | doc/api/commits.md | 2 | ||||
| -rw-r--r-- | lib/gitlab/gitaly_client/operation_service.rb | 3 | ||||
| -rw-r--r-- | spec/services/files/multi_service_spec.rb | 39 |
9 files changed, 55 insertions, 17 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 9084fa2f716..26aaba0e866 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.1.0 +1.2.0 @@ -432,7 +432,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 1.1.0', require: 'gitaly' +gem 'gitaly-proto', '~> 1.2.0', require: 'gitaly' gem 'grpc', '~> 1.15.0' gem 'google-protobuf', '~> 3.6' diff --git a/Gemfile.lock b/Gemfile.lock index 96b453344a1..d7ef3b114d3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -273,7 +273,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.1.0) + gitaly-proto (1.2.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-default_value_for (3.1.1) @@ -1006,7 +1006,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.1.0) + gitaly-proto (~> 1.2.0) github-markup (~> 1.7.0) gitlab-default_value_for (~> 3.1.1) gitlab-markup (~> 1.6.5) diff --git a/Gemfile.rails4.lock b/Gemfile.rails4.lock index 1289a28b719..ea248dfdabe 100644 --- a/Gemfile.rails4.lock +++ b/Gemfile.rails4.lock @@ -272,7 +272,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (1.1.0) + gitaly-proto (1.2.0) grpc (~> 1.0) github-markup (1.7.0) gitlab-markup (1.6.5) @@ -998,7 +998,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 1.1.0) + gitaly-proto (~> 1.2.0) github-markup (~> 1.7.0) gitlab-markup (~> 1.6.5) gitlab-sidekiq-fetcher diff --git a/app/services/files/multi_service.rb b/app/services/files/multi_service.rb index c9d3ee31d82..927634c2159 100644 --- a/app/services/files/multi_service.rb +++ b/app/services/files/multi_service.rb @@ -8,6 +8,7 @@ module Files transformer = Lfs::FileTransformer.new(project, @branch_name) actions = actions_after_lfs_transformation(transformer, params[:actions]) + actions = transform_move_actions(actions) commit_actions!(actions) end @@ -26,6 +27,16 @@ module Files end end + # When moving a file, `content: nil` means "use the contents of the previous + # file", while `content: ''` means "move the file and set it to empty" + def transform_move_actions(actions) + actions.map do |action| + action[:infer_content] = true if action[:content].nil? + + action + end + end + def commit_actions!(actions) repository.multi_action( current_user, diff --git a/changelogs/unreleased/51083-fix-move-api.yml b/changelogs/unreleased/51083-fix-move-api.yml new file mode 100644 index 00000000000..8838f6f267e --- /dev/null +++ b/changelogs/unreleased/51083-fix-move-api.yml @@ -0,0 +1,5 @@ +--- +title: 'Commits API: Preserve file content in move operations if unspecified' +merge_request: 23387 +author: +type: fixed diff --git a/doc/api/commits.md b/doc/api/commits.md index 7d9b52ec24f..6c16216429d 100644 --- a/doc/api/commits.md +++ b/doc/api/commits.md @@ -87,7 +87,7 @@ POST /projects/:id/repository/commits | `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`. Only considered for `move` action. | -| `content` | string | no | File content, required for all except `delete` and `chmod`. Optional for `move` | +| `content` | string | no | File content, required for all except `delete`, `chmod`, and `move`. Move actions that do not specify `content` will preserve the existing file content, and any other value of `content` will overwrite the file content. | | `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. | diff --git a/lib/gitlab/gitaly_client/operation_service.rb b/lib/gitlab/gitaly_client/operation_service.rb index c32c2c0b2fb..22d2d149e65 100644 --- a/lib/gitlab/gitaly_client/operation_service.rb +++ b/lib/gitlab/gitaly_client/operation_service.rb @@ -385,7 +385,8 @@ module Gitlab file_path: encode_binary(action[:file_path]), previous_path: encode_binary(action[:previous_path]), base64_content: action[:encoding] == 'base64', - execute_filemode: !!action[:execute_filemode] + execute_filemode: !!action[:execute_filemode], + infer_content: !!action[:infer_content] ) rescue RangeError raise ArgumentError, "Unknown action '#{action[:action]}'" diff --git a/spec/services/files/multi_service_spec.rb b/spec/services/files/multi_service_spec.rb index 5f3c8e82715..84c48d63c64 100644 --- a/spec/services/files/multi_service_spec.rb +++ b/spec/services/files/multi_service_spec.rb @@ -122,26 +122,47 @@ describe Files::MultiService do let(:action) { 'move' } let(:new_file_path) { 'files/ruby/new_popen.rb' } + let(:result) { subject.execute } + let(:blob) { repository.blob_at_branch(branch_name, new_file_path) } + context 'when original file has been updated' do before do update_file(original_file_path) end it 'rejects the commit' do - results = subject.execute - - expect(results[:status]).to eq(:error) - expect(results[:message]).to match(original_file_path) + expect(result[:status]).to eq(:error) + expect(result[:message]).to match(original_file_path) end end - context 'when original file have not been updated' do + context 'when original file has not been updated' do it 'moves the file' do - results = subject.execute - blob = project.repository.blob_at_branch(branch_name, new_file_path) - - expect(results[:status]).to eq(:success) + expect(result[:status]).to eq(:success) expect(blob).to be_present + expect(blob.data).to eq(file_content) + end + + context 'when content is nil' do + let(:file_content) { nil } + + it 'moves the existing content untouched' do + original_content = repository.blob_at_branch(branch_name, original_file_path).data + + expect(result[:status]).to eq(:success) + expect(blob).to be_present + expect(blob.data).to eq(original_content) + end + end + + context 'when content is an empty string' do + let(:file_content) { '' } + + it 'moves the file and empties it' do + expect(result[:status]).to eq(:success) + expect(blob).not_to be_nil + expect(blob.data).to eq('') + end end end end |