Merge branch 'ee_stable_candidate' into '7-13-stable-ee'

7-13-5-stable-ee candidate

See merge request !476

lib/gitlab/satellite/files/new_file_action.rb

+require_relative 'file_action'
+
+module Gitlab
+  module Satellite
+    class NewFileAction < FileAction
+      # Updates the files content and creates a new commit for it
+      #
+      # Returns false if the ref has been updated while editing the file
+      # Returns false if committing the change fails
+      # Returns false if pushing from the satellite to bare repo failed or was rejected
+      # Returns true otherwise
+      def commit!(content, commit_message, encoding, new_branch = nil)
+        in_locked_and_timed_satellite do |repo|
+          prepare_satellite!(repo)
+
+          # create target branch in satellite at the corresponding commit from bare repo
+          current_ref =
+            if @project.empty_repo?
+              # skip this step if we want to add first file to empty repo
+              Satellite::PARKING_BRANCH
+            else
+              repo.git.checkout({ raise: true, timeout: true, b: true }, ref, "origin/#{ref}")
+              ref
+            end
+
+          file_path_in_satellite = File.join(repo.working_dir, file_path)
+          dir_name_in_satellite = File.dirname(file_path_in_satellite)
+
+          # Prevent relative links
+          unless safe_path?(file_path_in_satellite)
+            Gitlab::GitLogger.error("FileAction: Relative path not allowed")
+            return false
+          end
+
+          # Create dir if not exists
+          FileUtils.mkdir_p(dir_name_in_satellite)
+
+          # Write file
+          write_file(file_path_in_satellite, content, encoding)
+
+          # add new file
+          repo.add(file_path_in_satellite)
+
+          # commit the changes
+          # will raise CommandFailed when commit fails
+          repo.git.commit(raise: true, timeout: true, a: true, m: commit_message)
+
+          target_branch = if new_branch.present? && !@project.empty_repo?
+                            "#{ref}:#{new_branch}"
+                          else
+                            "#{current_ref}:#{ref}"
+                          end
+
+          # push commit back to bare repo
+          # will raise CommandFailed when push fails
+          repo.git.push({ raise: true, timeout: true }, :origin, target_branch)
+
+          # everything worked
+          true
+        end
+      rescue Grit::Git::CommandFailed => ex
+        Gitlab::GitLogger.error(ex.message)
+        false
+      end
+    end
+  end
+end

spec/lib/gitlab/github_import/client_spec.rb

@@ -5,7 +5,8 @@
   let(:client) { Gitlab::GithubImport::Client.new(token) }
   before do
-    Gitlab.config.omniauth.providers << OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "github")
+    github_provider = OpenStruct.new(app_id: "asd123", app_secret: "asd123", name: "github", args: { "client_options" => {} })
+    allow(Gitlab.config.omniauth).to receive(:providers).and_return([github_provider])
   end
   it 'all OAuth2 client options are symbols' do

spec/models/repository_spec.rb

@@ -34,20 +34,6 @@
     end
   end
-  describe :can_be_merged? do
-    context 'mergeable branches' do
-      subject { repository.can_be_merged?('feature', 'master') }
-
-      it { is_expected.to be_truthy }
-    end
-
-    context 'non-mergeable branches' do
-      subject { repository.can_be_merged?('feature_conflict', 'feature') }
-
-      it { is_expected.to be_falsey }
-    end
-  end
-
   describe "search_files" do
     let(:results) { repository.search_files('feature', 'master') }
     subject { results }

spec/requests/api/files_spec.rb

@@ -49,6 +49,8 @@
     end
     it "should create a new file in project repo" do
+      expect_any_instance_of(Gitlab::Satellite::NewFileAction).to receive(:commit!).and_return(true)
+
       post api("/projects/#{project.id}/repository/files", user), valid_params
       expect(response.status).to eq(201)
       expect(json_response['file_path']).to eq('newfile.rb')
@@ -59,9 +61,8 @@
       expect(response.status).to eq(400)
     end
-    it "should return a 400 if editor fails to create file" do
-      allow_any_instance_of(Repository).to receive(:commit_file).
-        and_return(false)
+    it "should return a 400 if satellite fails to create file" do
+      expect_any_instance_of(Gitlab::Satellite::NewFileAction).to receive(:commit!).and_return(false)
       post api("/projects/#{project.id}/repository/files", user), valid_params
       expect(response.status).to eq(400)
@@ -79,6 +80,8 @@
     end
     it "should update existing file in project repo" do
+      expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_return(true)
+
       put api("/projects/#{project.id}/repository/files", user), valid_params
       expect(response.status).to eq(200)
       expect(json_response['file_path']).to eq(file_path)
@@ -88,6 +91,32 @@
       put api("/projects/#{project.id}/repository/files", user)
       expect(response.status).to eq(400)
     end
+
+    it 'should return a 400 if the checkout fails' do
+      expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::CheckoutFailed)
+
+      put api("/projects/#{project.id}/repository/files", user), valid_params
+      expect(response.status).to eq(400)
+
+      ref = valid_params[:branch_name]
+      expect(response.body).to match("ref '#{ref}' could not be checked out")
+    end
+
+    it 'should return a 409 if the file was not modified' do
+      expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::CommitFailed)
+
+      put api("/projects/#{project.id}/repository/files", user), valid_params
+      expect(response.status).to eq(409)
+      expect(response.body).to match("Maybe there was nothing to commit?")
+    end
+
+    it 'should return a 409 if the push fails' do
+      expect_any_instance_of(Gitlab::Satellite::EditFileAction).to receive(:commit!).and_raise(Gitlab::Satellite::PushFailed)
+
+      put api("/projects/#{project.id}/repository/files", user), valid_params
+      expect(response.status).to eq(409)
+      expect(response.body).to match("Maybe the file was changed by another process?")
+    end
   end
   describe "DELETE /projects/:id/repository/files" do
@@ -100,6 +129,7 @@
     end
     it "should delete existing file in project repo" do
+      expect_any_instance_of(Gitlab::Satellite::DeleteFileAction).to receive(:commit!).and_return(true)
       delete api("/projects/#{project.id}/repository/files", user), valid_params
       expect(response.status).to eq(200)
       expect(json_response['file_path']).to eq(file_path)
@@ -111,7 +141,7 @@
     end
     it "should return a 400 if satellite fails to create file" do
-      allow_any_instance_of(Repository).to receive(:remove_file).and_return(false)
+      expect_any_instance_of(Gitlab::Satellite::DeleteFileAction).to receive(:commit!).and_return(false)
       delete api("/projects/#{project.id}/repository/files", user), valid_params
       expect(response.status).to eq(400)