diff options
author | Douwe Maan <douwe@selenight.nl> | 2017-03-01 12:08:51 -0600 |
---|---|---|
committer | Douwe Maan <douwe@selenight.nl> | 2017-03-01 12:25:21 -0600 |
commit | 80543e0abd65a575bcc43f2482279af3980f4acc (patch) | |
tree | a9ca8ff8543f565bfd3a41a949f57ef39593b2b7 | |
parent | 981c730fdb3da1ada8d1b44d21e75a11175b3026 (diff) | |
download | gitlab-ce-80543e0abd65a575bcc43f2482279af3980f4acc.tar.gz |
Fix creating a file in an empty repo using the API
-rw-r--r-- | app/controllers/concerns/creates_commit.rb | 7 | ||||
-rw-r--r-- | app/models/repository.rb | 2 | ||||
-rw-r--r-- | app/services/files/base_service.rb | 16 | ||||
-rw-r--r-- | app/services/git_operation_service.rb | 37 | ||||
-rw-r--r-- | changelogs/unreleased/dm-fix-api-create-file-on-empty-repo.yml | 4 | ||||
-rw-r--r-- | spec/requests/api/files_spec.rb | 14 | ||||
-rw-r--r-- | spec/requests/api/v3/files_spec.rb | 14 |
7 files changed, 50 insertions, 44 deletions
diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index 2fe03020d2d..f897f828cec 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -4,7 +4,7 @@ module CreatesCommit def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) set_commit_variables - start_branch = @mr_target_branch unless initial_commit? + start_branch = @mr_target_branch commit_params = @commit_params.merge( start_project: @mr_target_project, start_branch: start_branch, @@ -117,11 +117,6 @@ module CreatesCommit @mr_source_branch = guess_mr_source_branch end - def initial_commit? - @mr_target_branch.nil? || - !@mr_target_project.repository.branch_exists?(@mr_target_branch) - end - def guess_mr_source_branch # XXX: Happens when viewing a commit without a branch. In this case, # @target_branch would be the default branch for @mr_source_project, diff --git a/app/models/repository.rb b/app/models/repository.rb index 0dbf246c3a4..d410d60dbad 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -995,6 +995,8 @@ class Repository end def with_repo_branch_commit(start_repository, start_branch_name) + return yield(nil) if start_repository.empty_repo? + branch_name_or_sha = if start_repository == self start_branch_name diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 31869c2f01e..3480cb70f67 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -58,16 +58,12 @@ module Files raise_error("You are not allowed to push into this branch") end - unless project.empty_repo? - unless @start_project.repository.branch_exists?(@start_branch) - raise_error('You can only create or edit files when you are on a branch') - end - - if different_branch? - if repository.branch_exists?(@target_branch) - raise_error('Branch with such name already exists. You need to switch to this branch in order to make changes') - end - end + if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch) + raise ValidationError, 'You can only create or edit files when you are on a branch' + end + + if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name) + raise ValidationError, 'Branch with such name already exists. You need to switch to this branch in order to make changes' end end diff --git a/app/services/git_operation_service.rb b/app/services/git_operation_service.rb index 27bcc047601..507e0a6680a 100644 --- a/app/services/git_operation_service.rb +++ b/app/services/git_operation_service.rb @@ -56,13 +56,14 @@ class GitOperationService start_project: repository.project, &block) - check_with_branch_arguments!( - branch_name, start_branch_name, start_project) + start_branch ||= branch_name + + verify_start_branch_exists!(start_project.repository, start_branch_name) update_branch_with_hooks(branch_name) do repository.with_repo_branch_commit( start_project.repository, - start_branch_name || branch_name, + start_branch_name, &block) end end @@ -150,30 +151,10 @@ class GitOperationService end end - def check_with_branch_arguments!( - branch_name, start_branch_name, start_project) - return if repository.branch_exists?(branch_name) - - if repository.project != start_project - unless start_branch_name - raise ArgumentError, - 'Should also pass :start_branch_name if' + - ' :start_project is different from current project' - end - - unless start_project.repository.branch_exists?(start_branch_name) - raise ArgumentError, - "Cannot find branch #{branch_name} nor" \ - " #{start_branch_name} from" \ - " #{start_project.path_with_namespace}" - end - elsif start_branch_name - unless repository.branch_exists?(start_branch_name) - raise ArgumentError, - "Cannot find branch #{branch_name} nor" \ - " #{start_branch_name} from" \ - " #{repository.project.path_with_namespace}" - end - end + def verify_start_branch_exists!(start_repository, start_branch_name) + return if start_repository.empty_repo? + return if start_repository.branch_exists?(start_branch_name) + + raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.path_with_namespace}" end end diff --git a/changelogs/unreleased/dm-fix-api-create-file-on-empty-repo.yml b/changelogs/unreleased/dm-fix-api-create-file-on-empty-repo.yml new file mode 100644 index 00000000000..7ac25c0a83e --- /dev/null +++ b/changelogs/unreleased/dm-fix-api-create-file-on-empty-repo.yml @@ -0,0 +1,4 @@ +--- +title: Fix creating a file in an empty repo using the API +merge_request: 9632 +author: diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb index 31b1aca6d73..91f8a35e045 100644 --- a/spec/requests/api/files_spec.rb +++ b/spec/requests/api/files_spec.rb @@ -147,6 +147,20 @@ describe API::Files, api: true do expect(last_commit.author_name).to eq(author_name) end end + + context 'when the repo is empty' do + let!(:project) { create(:project_empty_repo, namespace: user.namespace ) } + + 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 + end end describe "PUT /projects/:id/repository/files" do diff --git a/spec/requests/api/v3/files_spec.rb b/spec/requests/api/v3/files_spec.rb index 93637053626..dc5691a106f 100644 --- a/spec/requests/api/v3/files_spec.rb +++ b/spec/requests/api/v3/files_spec.rb @@ -148,6 +148,20 @@ describe API::V3::Files, api: true do expect(last_commit.author_name).to eq(author_name) end end + + context 'when the repo is empty' do + let!(:project) { create(:project_empty_repo, namespace: user.namespace ) } + + 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 + end end describe "PUT /projects/:id/repository/files" do |