Merge branch 'dm-fix-api-create-file-on-empty-repo' into 'master'

Fix creating a file in an empty repo using the API

Closes #28626

See merge request !9632

6 files changed, 37 insertions, 40 deletions

diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1a6418a4491..5085e5f436c 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1075,6 +1075,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 0a25f56d24c..0f31f4d7cbb 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, "A branch called #{@branch_name} already exists. Switch to that 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..ed6ea638235 100644
--- a/app/services/git_operation_service.rb
+++ b/app/services/git_operation_service.rb
@@ -56,12 +56,16 @@ class GitOperationService
     start_project: repository.project,
     &block)
 
-    check_with_branch_arguments!(
-      branch_name, start_branch_name, start_project)
+    start_repository = start_project.repository
+    start_branch_name = nil if start_repository.empty_repo?
+
+    if start_branch_name && !start_repository.branch_exists?(start_branch_name)
+      raise ArgumentError, "Cannot find branch #{start_branch_name} in #{start_repository.path_with_namespace}"
+    end
 
     update_branch_with_hooks(branch_name) do
       repository.with_repo_branch_commit(
-        start_project.repository,
+        start_repository,
         start_branch_name || branch_name,
         &block)
     end
@@ -149,31 +153,4 @@ class GitOperationService
       repository.raw_repository.autocrlf = :input
     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
-  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/factories/projects.rb b/spec/factories/projects.rb
index c80b09e9b9d..3d8b7e879c7 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -39,6 +39,10 @@ FactoryGirl.define do
     trait :empty_repo do
       after(:create) do |project|
         project.create_repository
+
+        # We delete hooks so that gitlab-shell will not try to authenticate with
+        # an API that isn't running
+        FileUtils.rm_r(File.join(project.repository_storage_path, "#{project.path_with_namespace}.git", 'hooks'))
       end
     end
 
diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb
index 5e26e779366..07061548320 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