diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/models/repository.rb | 110 | ||||
| -rw-r--r-- | app/services/files/base_service.rb | 2 | ||||
| -rw-r--r-- | app/services/files/create_dir_service.rb | 2 | ||||
| -rw-r--r-- | app/services/files/create_service.rb | 2 | ||||
| -rw-r--r-- | app/services/files/delete_service.rb | 2 | ||||
| -rw-r--r-- | app/services/files/update_service.rb | 4 | ||||
| -rw-r--r-- | doc/api/repository_files.md | 12 | ||||
| -rw-r--r-- | lib/api/files.rb | 12 | ||||
| -rw-r--r-- | lib/gitlab/git.rb | 8 | ||||
| -rw-r--r-- | spec/models/repository_spec.rb | 136 | ||||
| -rw-r--r-- | spec/requests/api/files_spec.rb | 75 |
12 files changed, 298 insertions, 68 deletions
diff --git a/CHANGELOG b/CHANGELOG index d133909748b..e9bd4198e67 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -92,6 +92,7 @@ v 8.12.0 (unreleased) - Fix missing flash messages on service edit page (airatshigapov) - Added project-specific enable/disable setting for LFS !5997 - Added group-specific enable/disable setting for LFS !6164 + - Add optional 'author' param when making commits. !5822 (dandunckelman) - Don't expose a user's token in the `/api/v3/user` API (!6047) - Remove redundant js-timeago-pending from user activity log (ClemMakesApps) - Ability to manage project issues, snippets, wiki, merge requests and builds access level diff --git a/app/models/repository.rb b/app/models/repository.rb index c69e5a22a69..772c62a4124 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -756,62 +756,59 @@ class Repository @root_ref ||= cache.fetch(:root_ref) { raw_repository.root_ref } end - def commit_dir(user, path, message, branch) + def commit_dir(user, path, message, branch, author_email: nil, author_name: nil) update_branch_with_hooks(user, branch) do |ref| - committer = user_to_committer(user) - options = {} - options[:committer] = committer - options[:author] = committer - - options[:commit] = { - message: message, - branch: ref, - update_ref: false, + options = { + commit: { + branch: ref, + message: message, + update_ref: false + } } + options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) + raw_repository.mkdir(path, options) end end - def commit_file(user, path, content, message, branch, update) + def commit_file(user, path, content, message, branch, update, author_email: nil, author_name: nil) update_branch_with_hooks(user, branch) do |ref| - committer = user_to_committer(user) - options = {} - options[:committer] = committer - options[:author] = committer - options[:commit] = { - message: message, - branch: ref, - update_ref: false, + options = { + commit: { + branch: ref, + message: message, + update_ref: false + }, + file: { + content: content, + path: path, + update: update + } } - options[:file] = { - content: content, - path: path, - update: update - } + options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) Gitlab::Git::Blob.commit(raw_repository, options) end end - def update_file(user, path, content, branch:, previous_path:, message:) + def update_file(user, path, content, branch:, previous_path:, message:, author_email: nil, author_name: nil) update_branch_with_hooks(user, branch) do |ref| - committer = user_to_committer(user) - options = {} - options[:committer] = committer - options[:author] = committer - options[:commit] = { - message: message, - branch: ref, - update_ref: false + options = { + commit: { + branch: ref, + message: message, + update_ref: false + }, + file: { + content: content, + path: path, + update: true + } } - options[:file] = { - content: content, - path: path, - update: true - } + options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) if previous_path && previous_path != path options[:file][:previous_path] = previous_path @@ -822,34 +819,39 @@ class Repository end end - def remove_file(user, path, message, branch) + def remove_file(user, path, message, branch, author_email: nil, author_name: nil) update_branch_with_hooks(user, branch) do |ref| - committer = user_to_committer(user) - options = {} - options[:committer] = committer - options[:author] = committer - options[:commit] = { - message: message, - branch: ref, - update_ref: false, + options = { + commit: { + branch: ref, + message: message, + update_ref: false + }, + file: { + path: path + } } - options[:file] = { - path: path - } + options.merge!(get_committer_and_author(user, email: author_email, name: author_name)) Gitlab::Git::Blob.remove(raw_repository, options) end end - def user_to_committer(user) + def get_committer_and_author(user, email: nil, name: nil) + committer = user_to_committer(user) + author = name && email ? Gitlab::Git::committer_hash(email: email, name: name) : committer + { - email: user.email, - name: user.name, - time: Time.now + author: author, + committer: committer } end + def user_to_committer(user) + Gitlab::Git::committer_hash(email: user.email, name: user.name) + end + def can_be_merged?(source_sha, target_branch) our_commit = rugged.branches[target_branch].target their_commit = rugged.lookup(source_sha) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index ea94818713b..e8465729d06 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -16,6 +16,8 @@ module Files params[:file_content] end @last_commit_sha = params[:last_commit_sha] + @author_email = params[:author_email] + @author_name = params[:author_name] # Validate parameters validate diff --git a/app/services/files/create_dir_service.rb b/app/services/files/create_dir_service.rb index 6107254a34e..d00d78cee7e 100644 --- a/app/services/files/create_dir_service.rb +++ b/app/services/files/create_dir_service.rb @@ -3,7 +3,7 @@ require_relative "base_service" module Files class CreateDirService < Files::BaseService def commit - repository.commit_dir(current_user, @file_path, @commit_message, @target_branch) + repository.commit_dir(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name) end def validate diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 8eaf6db8012..bf127843d55 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -3,7 +3,7 @@ require_relative "base_service" module Files class CreateService < Files::BaseService def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false) + repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch, false, author_email: @author_email, author_name: @author_name) end def validate diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 27c881c3430..8b27ad51789 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -3,7 +3,7 @@ require_relative "base_service" module Files class DeleteService < Files::BaseService def commit - repository.remove_file(current_user, @file_path, @commit_message, @target_branch) + repository.remove_file(current_user, @file_path, @commit_message, @target_branch, author_email: @author_email, author_name: @author_name) end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index 4fc3b640799..9e9b5b63f26 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -8,7 +8,9 @@ module Files repository.update_file(current_user, @file_path, @file_content, branch: @target_branch, previous_path: @previous_path, - message: @commit_message) + message: @commit_message, + author_email: @author_email, + author_name: @author_name) end private diff --git a/doc/api/repository_files.md b/doc/api/repository_files.md index fc3af5544de..1bc6a24e914 100644 --- a/doc/api/repository_files.md +++ b/doc/api/repository_files.md @@ -44,7 +44,7 @@ POST /projects/:id/repository/files ``` ```bash -curl --request POST --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&content=some%20content&commit_message=create%20a%20new%20file' +curl --request POST --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&author_email=author%40example.com&author_name=Firstname%20Lastname&content=some%20content&commit_message=create%20a%20new%20file' ``` Example response: @@ -61,6 +61,8 @@ Parameters: - `file_path` (required) - Full path to new file. Ex. lib/class.rb - `branch_name` (required) - The name of branch - `encoding` (optional) - 'text' or 'base64'. Text is default. +- `author_email` (optional) - Specify the commit author's email address +- `author_name` (optional) - Specify the commit author's name - `content` (required) - File content - `commit_message` (required) - Commit message @@ -71,7 +73,7 @@ PUT /projects/:id/repository/files ``` ```bash -curl --request PUT --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&content=some%20other%20content&commit_message=update%20file' +curl --request PUT --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&author_email=author%40example.com&author_name=Firstname%20Lastname&content=some%20other%20content&commit_message=update%20file' ``` Example response: @@ -88,6 +90,8 @@ Parameters: - `file_path` (required) - Full path to file. Ex. lib/class.rb - `branch_name` (required) - The name of branch - `encoding` (optional) - 'text' or 'base64'. Text is default. +- `author_email` (optional) - Specify the commit author's email address +- `author_name` (optional) - Specify the commit author's name - `content` (required) - New file content - `commit_message` (required) - Commit message @@ -107,7 +111,7 @@ DELETE /projects/:id/repository/files ``` ```bash -curl --request PUT --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&commit_message=delete%20file' +curl --request PUT --header 'PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK' 'https://gitlab.example.com/api/v3/projects/13083/repository/files?file_path=app/project.rb&branch_name=master&author_email=author%40example.com&author_name=Firstname%20Lastname&commit_message=delete%20file' ``` Example response: @@ -123,4 +127,6 @@ Parameters: - `file_path` (required) - Full path to file. Ex. lib/class.rb - `branch_name` (required) - The name of branch +- `author_email` (optional) - Specify the commit author's email address +- `author_name` (optional) - Specify the commit author's name - `commit_message` (required) - Commit message diff --git a/lib/api/files.rb b/lib/api/files.rb index c1d86f313b0..96510e651a3 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -11,14 +11,16 @@ module API target_branch: attrs[:branch_name], commit_message: attrs[:commit_message], file_content: attrs[:content], - file_content_encoding: attrs[:encoding] + file_content_encoding: attrs[:encoding], + author_email: attrs[:author_email], + author_name: attrs[:author_name] } end def commit_response(attrs) { file_path: attrs[:file_path], - branch_name: attrs[:branch_name], + branch_name: attrs[:branch_name] } end end @@ -96,7 +98,7 @@ module API authorize! :push_code, user_project required_attributes! [:file_path, :branch_name, :content, :commit_message] - attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding] + attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding, :author_email, :author_name] result = ::Files::CreateService.new(user_project, current_user, commit_params(attrs)).execute if result[:status] == :success @@ -122,7 +124,7 @@ module API authorize! :push_code, user_project required_attributes! [:file_path, :branch_name, :content, :commit_message] - attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding] + attrs = attributes_for_keys [:file_path, :branch_name, :content, :commit_message, :encoding, :author_email, :author_name] result = ::Files::UpdateService.new(user_project, current_user, commit_params(attrs)).execute if result[:status] == :success @@ -149,7 +151,7 @@ module API authorize! :push_code, user_project required_attributes! [:file_path, :branch_name, :commit_message] - attrs = attributes_for_keys [:file_path, :branch_name, :commit_message] + attrs = attributes_for_keys [:file_path, :branch_name, :commit_message, :author_email, :author_name] result = ::Files::DeleteService.new(user_project, current_user, commit_params(attrs)).execute if result[:status] == :success diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 7584efe4fa8..3ab99360206 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -18,6 +18,14 @@ module Gitlab end end + def committer_hash(email:, name:) + { + email: email, + name: name, + time: Time.now + } + end + def tag_name(ref) ref = ref.to_s if self.tag_ref?(ref) diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 94681004c96..db29f4d353b 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -16,6 +16,21 @@ describe Repository, models: true do merge_commit_id = repository.merge(user, merge_request, commit_options) repository.commit(merge_commit_id) end + let(:author_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:author_name) { FFaker::Name.name.chomp("\.") } describe '#branch_names_contains' do subject { repository.branch_names_contains(sample_commit.id) } @@ -132,7 +147,31 @@ describe Repository, models: true do end end - describe :commit_file do + describe "#commit_dir" do + it "commits a change that creates a new directory" do + expect do + repository.commit_dir(user, 'newdir', 'Create newdir', 'master') + end.to change { repository.commits('master').count }.by(1) + + newdir = repository.tree('master', 'newdir') + expect(newdir.path).to eq('newdir') + end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + expect do + repository.commit_dir(user, "newdir", "Add newdir", 'master', author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe "#commit_file" do it 'commits change to a file successfully' do expect do repository.commit_file(user, 'CHANGELOG', 'Changelog!', @@ -144,9 +183,23 @@ describe Repository, models: true do expect(blob.data).to eq('Changelog!') end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + expect do + repository.commit_file(user, "README", 'README!', 'Add README', + 'master', true, author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end - describe :update_file do + describe "#update_file" do it 'updates filename successfully' do expect do repository.update_file(user, 'NEWLICENSE', 'Copyright!', @@ -160,6 +213,85 @@ describe Repository, models: true do expect(files).not_to include('LICENSE') expect(files).to include('NEWLICENSE') end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.update_file(user, 'README', "Updated README!", + branch: 'master', + previous_path: 'README', + message: 'Update README', + author_email: author_email, + author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe "#remove_file" do + it 'removes file successfully' do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.remove_file(user, "README", "Remove README", 'master') + end.to change { repository.commits('master').count }.by(1) + + expect(repository.blob_at('master', 'README')).to be_nil + end + + context "when an author is specified" do + it "uses the given email/name to set the commit's author" do + repository.commit_file(user, "README", 'README!', 'Add README', 'master', true) + + expect do + repository.remove_file(user, "README", "Remove README", 'master', author_email: author_email, author_name: author_name) + end.to change { repository.commits('master').count }.by(1) + + last_commit = repository.commit + + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end + end + + describe '#get_committer_and_author' do + it 'returns the committer and author data' do + options = repository.get_committer_and_author(user) + expect(options[:committer][:email]).to eq(user.email) + expect(options[:author][:email]).to eq(user.email) + end + + context 'when the email/name are given' do + it 'returns an object containing the email/name' do + options = repository.get_committer_and_author(user, email: author_email, name: author_name) + expect(options[:author][:email]).to eq(author_email) + expect(options[:author][:name]).to eq(author_name) + end + end + + context 'when the email is given but the name is not' do + it 'returns the committer as the author' do + options = repository.get_committer_and_author(user, email: author_email) + expect(options[:author][:email]).to eq(user.email) + expect(options[:author][:name]).to eq(user.name) + end + end + + context 'when the name is given but the email is not' do + it 'returns nil' do + options = repository.get_committer_and_author(user, name: author_name) + expect(options[:author][:email]).to eq(user.email) + expect(options[:author][:name]).to eq(user.name) + end + end end describe "search_files" do diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 2d1213df8a7..050d0dd082d 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -5,6 +5,21 @@ describe API::API, api: true do let(:user) { create(:user) } let!(:project) { create(:project, namespace: user.namespace ) } let(:file_path) { 'files/ruby/popen.rb' } + let(:author_email) { FFaker::Internet.email } + + # I have to remove periods from the end of the name + # This happened when the user's name had a suffix (i.e. "Sr.") + # This seems to be what git does under the hood. For example, this commit: + # + # $ git commit --author='Foo Sr. <foo@example.com>' -m 'Where's my trailing period?' + # + # results in this: + # + # $ git show --pretty + # ... + # Author: Foo Sr <foo@example.com> + # ... + let(:author_name) { FFaker::Name.name.chomp("\.") } before { project.team << [user, :developer] } @@ -16,6 +31,7 @@ describe API::API, api: true do } get api("/projects/#{project.id}/repository/files", user), params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) expect(json_response['file_name']).to eq('popen.rb') @@ -25,6 +41,7 @@ describe API::API, api: true do it "returns a 400 bad request if no params given" do get api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -35,6 +52,7 @@ describe API::API, api: true do } get api("/projects/#{project.id}/repository/files", user), params + expect(response).to have_http_status(404) end end @@ -51,12 +69,17 @@ describe API::API, api: true do it "creates a new file in project repo" do post api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(201) expect(json_response['file_path']).to eq('newfile.rb') + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do post api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -65,8 +88,22 @@ describe API::API, api: true do and_return(false) post api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "creates a new file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name) + + post api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(201) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "PUT /projects/:id/repository/files" do @@ -81,14 +118,32 @@ describe API::API, api: true do it "updates existing file in project repo" do put api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do put api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "updates a file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name, content: "New content") + + put api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(200) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "DELETE /projects/:id/repository/files" do @@ -102,12 +157,17 @@ describe API::API, api: true do it "deletes existing file in project repo" do delete api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(user.email) + expect(last_commit.author_name).to eq(user.name) end it "returns a 400 bad request if no params given" do delete api("/projects/#{project.id}/repository/files", user) + expect(response).to have_http_status(400) end @@ -115,8 +175,22 @@ describe API::API, api: true do allow_any_instance_of(Repository).to receive(:remove_file).and_return(false) delete api("/projects/#{project.id}/repository/files", user), valid_params + expect(response).to have_http_status(400) end + + context "when specifying an author" do + it "removes a file with the specified author" do + valid_params.merge!(author_email: author_email, author_name: author_name) + + delete api("/projects/#{project.id}/repository/files", user), valid_params + + expect(response).to have_http_status(200) + last_commit = project.repository.commit.raw + expect(last_commit.author_email).to eq(author_email) + expect(last_commit.author_name).to eq(author_name) + end + end end describe "POST /projects/:id/repository/files with binary file" do @@ -143,6 +217,7 @@ describe API::API, api: true do it "remains unchanged" do get api("/projects/#{project.id}/repository/files", user), get_params + expect(response).to have_http_status(200) expect(json_response['file_path']).to eq(file_path) expect(json_response['file_name']).to eq(file_path) |