Update remote happens through Gitaly onlyzj-is-anchestor

This change makes closes another migration! And remotes tests around it,
mostly due to stubbing this was needed. Also, these are tested on the
Gitaly side too:
- https://gitlab.com/gitlab-org/gitaly/issues/791
-
https://gitlab.com/gitlab-org/gitaly/blob/6dd74543f6af8ebcf22aa10e39da004f388fdda5/internal/service/repository/fetch_remote_test.go

Closes https://gitlab.com/gitlab-org/gitaly/issues/789

2 files changed, 29 insertions, 353 deletions

diff --git a/lib/gitlab/git/remote_mirror.rb b/lib/gitlab/git/remote_mirror.rb
index ebe46722890..e4743b4db0a 100644
--- a/lib/gitlab/git/remote_mirror.rb
+++ b/lib/gitlab/git/remote_mirror.rb
@@ -7,81 +7,8 @@ module Gitlab
       end
 
       def update(only_branches_matching: [])
-        @repository.gitaly_migrate(:remote_update_remote_mirror) do |is_enabled|
-          if is_enabled
-            gitaly_update(only_branches_matching)
-          else
-            rugged_update(only_branches_matching)
-          end
-        end
-      end
-
-      private
-
-      def gitaly_update(only_branches_matching)
-        @repository.gitaly_remote_client.update_remote_mirror(@ref_name, only_branches_matching)
-      end
-
-      def rugged_update(only_branches_matching)
-        local_branches = refs_obj(@repository.local_branches, only_refs_matching: only_branches_matching)
-        remote_branches = refs_obj(@repository.remote_branches(@ref_name), only_refs_matching: only_branches_matching)
-
-        updated_branches = changed_refs(local_branches, remote_branches)
-        push_branches(updated_branches.keys) if updated_branches.present?
-
-        delete_refs(local_branches, remote_branches)
-
-        local_tags = refs_obj(@repository.tags)
-        remote_tags = refs_obj(@repository.remote_tags(@ref_name))
-
-        updated_tags = changed_refs(local_tags, remote_tags)
-        @repository.push_remote_branches(@ref_name, updated_tags.keys) if updated_tags.present?
-
-        delete_refs(local_tags, remote_tags)
-      end
-
-      def refs_obj(refs, only_refs_matching: [])
-        refs.each_with_object({}) do |ref, refs|
-          next if only_refs_matching.present? && !only_refs_matching.include?(ref.name)
-
-          refs[ref.name] = ref
-        end
-      end
-
-      def changed_refs(local_refs, remote_refs)
-        local_refs.select do |ref_name, ref|
-          remote_ref = remote_refs[ref_name]
-
-          remote_ref.nil? || ref.dereferenced_target != remote_ref.dereferenced_target
-        end
-      end
-
-      def push_branches(branches)
-        default_branch, branches = branches.partition do |branch|
-          @repository.root_ref == branch
-        end
-
-        # Push the default branch first so it works fine when remote mirror is empty.
-        branches.unshift(*default_branch)
-
-        @repository.push_remote_branches(@ref_name, branches)
-      end
-
-      def delete_refs(local_refs, remote_refs)
-        refs = refs_to_delete(local_refs, remote_refs)
-
-        @repository.delete_remote_branches(@ref_name, refs.keys) if refs.present?
-      end
-
-      def refs_to_delete(local_refs, remote_refs)
-        default_branch_id = @repository.commit.id
-
-        remote_refs.select do |remote_ref_name, remote_ref|
-          next false if local_refs[remote_ref_name] # skip if branch or tag exist in local repo
-
-          remote_ref_id = remote_ref.dereferenced_target.try(:id)
-
-          remote_ref_id && @repository.rugged_is_ancestor?(remote_ref_id, default_branch_id)
+        @repository.wrapped_gitaly_errors do
+          @repository.gitaly_remote_client.update_remote_mirror(@ref_name, only_branches_matching)
         end
       end
     end
diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb
index 723cb374c37..065df4d4b0e 100644
--- a/spec/services/projects/update_remote_mirror_service_spec.rb
+++ b/spec/services/projects/update_remote_mirror_service_spec.rb
@@ -1,7 +1,8 @@
 require 'spec_helper'
 
 describe Projects::UpdateRemoteMirrorService do
-  let(:project) { create(:project, :repository) }
+  set(:project) { create(:project, :repository) }
+  let(:owner) { project.owner }
   let(:remote_project) { create(:forked_project_with_submodules) }
   let(:repository) { project.repository }
   let(:raw_repository) { repository.raw }
@@ -9,13 +10,11 @@ describe Projects::UpdateRemoteMirrorService do
 
   subject { described_class.new(project, project.creator) }
 
-  describe "#execute", :skip_gitaly_mock do
+  describe "#execute" do
     before do
-      create_branch(repository, 'existing-branch')
-      allow(raw_repository).to receive(:remote_tags) do
-        generate_tags(repository, 'v1.0.0', 'v1.1.0')
-      end
-      allow(raw_repository).to receive(:push_remote_branches).and_return(true)
+      repository.add_branch(owner, 'existing-branch', 'master')
+
+      allow(remote_mirror).to receive(:update_repository).and_return(true)
     end
 
     it "fetches the remote repository" do
@@ -34,307 +33,57 @@ describe Projects::UpdateRemoteMirrorService do
       expect(result[:status]).to eq(:success)
     end
 
-    describe 'Syncing branches' do
+    context 'when only syncing protected branches' do
       it "push all the branches the first time" do
         allow(repository).to receive(:fetch_remote)
 
-        expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, local_branch_names)
-
-        subject.execute(remote_mirror)
-      end
-
-      it "does not push anything is remote is up to date" do
-        allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, local_branch_names) }
-
-        expect(raw_repository).not_to receive(:push_remote_branches)
-
-        subject.execute(remote_mirror)
-      end
-
-      it "sync new branches" do
-        # call local_branch_names early so it is not called after the new branch has been created
-        current_branches = local_branch_names
-        allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, current_branches) }
-        create_branch(repository, 'my-new-branch')
-
-        expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['my-new-branch'])
-
-        subject.execute(remote_mirror)
-      end
-
-      it "sync updated branches" do
-        allow(repository).to receive(:fetch_remote) do
-          sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-          update_branch(repository, 'existing-branch')
-        end
-
-        expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['existing-branch'])
+        expect(remote_mirror).to receive(:update_repository).with({})
 
         subject.execute(remote_mirror)
       end
-
-      context 'when push only protected branches option is set' do
-        let(:unprotected_branch_name) { 'existing-branch' }
-        let(:protected_branch_name) do
-          project.repository.branch_names.find { |n| n != unprotected_branch_name }
-        end
-        let!(:protected_branch) do
-          create(:protected_branch, project: project, name: protected_branch_name)
-        end
-
-        before do
-          project.reload
-          remote_mirror.only_protected_branches = true
-        end
-
-        it "sync updated protected branches" do
-          allow(repository).to receive(:fetch_remote) do
-            sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-            update_branch(repository, protected_branch_name)
-          end
-
-          expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, [protected_branch_name])
-
-          subject.execute(remote_mirror)
-        end
-
-        it 'does not sync unprotected branches' do
-          allow(repository).to receive(:fetch_remote) do
-            sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-            update_branch(repository, unprotected_branch_name)
-          end
-
-          expect(raw_repository).not_to receive(:push_remote_branches).with(remote_mirror.remote_name, [unprotected_branch_name])
-
-          subject.execute(remote_mirror)
-        end
-      end
-
-      context 'when branch exists in local and remote repo' do
-        context 'when it has diverged' do
-          it 'syncs branches' do
-            allow(repository).to receive(:fetch_remote) do
-              sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-              update_remote_branch(repository, remote_mirror.remote_name, 'markdown')
-            end
-
-            expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['markdown'])
-
-            subject.execute(remote_mirror)
-          end
-        end
-      end
-
-      describe 'for delete' do
-        context 'when branch exists in local and remote repo' do
-          it 'deletes the branch from remote repo' do
-            allow(repository).to receive(:fetch_remote) do
-              sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-              delete_branch(repository, 'existing-branch')
-            end
-
-            expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['existing-branch'])
-
-            subject.execute(remote_mirror)
-          end
-        end
-
-        context 'when push only protected branches option is set' do
-          before do
-            remote_mirror.only_protected_branches = true
-          end
-
-          context 'when branch exists in local and remote repo' do
-            let!(:protected_branch_name) { local_branch_names.first }
-
-            before do
-              create(:protected_branch, project: project, name: protected_branch_name)
-              project.reload
-            end
-
-            it 'deletes the protected branch from remote repo' do
-              allow(repository).to receive(:fetch_remote) do
-                sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-                delete_branch(repository, protected_branch_name)
-              end
-
-              expect(raw_repository).not_to receive(:delete_remote_branches).with(remote_mirror.remote_name, [protected_branch_name])
-
-              subject.execute(remote_mirror)
-            end
-
-            it 'does not delete the unprotected branch from remote repo' do
-              allow(repository).to receive(:fetch_remote) do
-                sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-                delete_branch(repository, 'existing-branch')
-              end
-
-              expect(raw_repository).not_to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['existing-branch'])
-
-              subject.execute(remote_mirror)
-            end
-          end
-
-          context 'when branch only exists on remote repo' do
-            let!(:protected_branch_name) { 'remote-branch' }
-
-            before do
-              create(:protected_branch, project: project, name: protected_branch_name)
-            end
-
-            context 'when it has diverged' do
-              it 'does not delete the remote branch' do
-                allow(repository).to receive(:fetch_remote) do
-                  sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-
-                  rev = repository.find_branch('markdown').dereferenced_target
-                  create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', rev.id)
-                end
-
-                expect(raw_repository).not_to receive(:delete_remote_branches)
-
-                subject.execute(remote_mirror)
-              end
-            end
-
-            context 'when it has not diverged' do
-              it 'deletes the remote branch' do
-                allow(repository).to receive(:fetch_remote) do
-                  sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-
-                  masterrev = repository.find_branch('master').dereferenced_target
-                  create_remote_branch(repository, remote_mirror.remote_name, protected_branch_name, masterrev.id)
-                end
-
-                expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, [protected_branch_name])
-
-                subject.execute(remote_mirror)
-              end
-            end
-          end
-        end
-
-        context 'when branch only exists on remote repo' do
-          context 'when it has diverged' do
-            it 'does not delete the remote branch' do
-              allow(repository).to receive(:fetch_remote) do
-                sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-
-                rev = repository.find_branch('markdown').dereferenced_target
-                create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', rev.id)
-              end
-
-              expect(raw_repository).not_to receive(:delete_remote_branches)
-
-              subject.execute(remote_mirror)
-            end
-          end
-
-          context 'when it has not diverged' do
-            it 'deletes the remote branch' do
-              allow(repository).to receive(:fetch_remote) do
-                sync_remote(repository, remote_mirror.remote_name, local_branch_names)
-
-                masterrev = repository.find_branch('master').dereferenced_target
-                create_remote_branch(repository, remote_mirror.remote_name, 'remote-branch', masterrev.id)
-              end
-
-              expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['remote-branch'])
-
-              subject.execute(remote_mirror)
-            end
-          end
-        end
-      end
     end
 
-    describe 'Syncing tags' do
-      before do
-        allow(repository).to receive(:fetch_remote) { sync_remote(repository, remote_mirror.remote_name, local_branch_names) }
+    context 'when push only protected branches option is set' do
+      let(:unprotected_branch_name) { 'existing-branch' }
+      let(:protected_branch_name) do
+        project.repository.branch_names.find { |n| n != unprotected_branch_name }
       end
-
-      context 'when there are not tags to push' do
-        it 'does not try to push tags' do
-          allow(repository).to receive(:remote_tags) { {} }
-          allow(repository).to receive(:tags) { [] }
-
-          expect(repository).not_to receive(:push_tags)
-
-          subject.execute(remote_mirror)
-        end
+      let!(:protected_branch) do
+        create(:protected_branch, project: project, name: protected_branch_name)
       end
 
-      context 'when there are some tags to push' do
-        it 'pushes tags to remote' do
-          allow(raw_repository).to receive(:remote_tags) { {} }
-
-          expect(raw_repository).to receive(:push_remote_branches).with(remote_mirror.remote_name, ['v1.0.0', 'v1.1.0'])
-
-          subject.execute(remote_mirror)
-        end
+      before do
+        project.reload
+        remote_mirror.only_protected_branches = true
       end
 
-      context 'when there are some tags to delete' do
-        it 'deletes tags from remote' do
-          remote_tags = generate_tags(repository, 'v1.0.0', 'v1.1.0')
-          allow(raw_repository).to receive(:remote_tags) { remote_tags }
-
-          repository.rm_tag(create(:user), 'v1.0.0')
-
-          expect(raw_repository).to receive(:delete_remote_branches).with(remote_mirror.remote_name, ['v1.0.0'])
+      it "sync updated protected branches" do
+        allow(repository).to receive(:fetch_remote)
+        expect(remote_mirror).to receive(:update_repository).with(only_branches_matching: ["'test'"])
 
-          subject.execute(remote_mirror)
-        end
+        subject.execute(remote_mirror)
       end
     end
   end
 
-  def create_branch(repository, branch_name)
-    rugged = repository.rugged
-    masterrev = repository.find_branch('master').dereferenced_target
-    parentrev = repository.commit(masterrev).parent_id
-
-    rugged.references.create("refs/heads/#{branch_name}", parentrev)
-
-    repository.expire_branches_cache
-  end
-
-  def create_remote_branch(repository, remote_name, branch_name, source_id)
-    rugged = repository.rugged
-
-    rugged.references.create("refs/remotes/#{remote_name}/#{branch_name}", source_id)
-  end
-
   def sync_remote(repository, remote_name, local_branch_names)
-    rugged = repository.rugged
-
     local_branch_names.each do |branch|
-      target = repository.find_branch(branch).try(:dereferenced_target)
-      rugged.references.create("refs/remotes/#{remote_name}/#{branch}", target.id) if target
+      commit = repository.commit(branch)
+      repository.write_ref("refs/remotes/#{remote_name}/#{branch}", commit.id) if commit
     end
   end
 
   def update_remote_branch(repository, remote_name, branch)
-    rugged = repository.rugged
-    masterrev = repository.find_branch('master').dereferenced_target.id
+    masterrev = repository.commit('master').id
 
-    rugged.references.create("refs/remotes/#{remote_name}/#{branch}", masterrev, force: true)
+    repository.write_ref("refs/remotes/#{remote_name}/#{branch}", masterrev, force: true)
     repository.expire_branches_cache
   end
 
   def update_branch(repository, branch)
-    rugged = repository.rugged
-    masterrev = repository.find_branch('master').dereferenced_target.id
-
-    # Updated existing branch
-    rugged.references.create("refs/heads/#{branch}", masterrev, force: true)
-    repository.expire_branches_cache
-  end
-
-  def delete_branch(repository, branch)
-    rugged = repository.rugged
+    masterrev = repository.commit('master').id
 
-    rugged.references.delete("refs/heads/#{branch}")
+    repository.write_ref("refs/heads/#{branch}", masterrev, force: true)
     repository.expire_branches_cache
   end