Merge branch '791-server-implementation-mirrorservice-updateremotemirror' into 'master'

Resolve "Server Implementation MirrorService::UpdateRemoteMirror"

Closes #791

See merge request gitlab-org/gitaly!544

6 files changed, 214 insertions, 4 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0857add54..3ea0299a4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 UNRELEASED
 
+- Implement RemoteService.UpdateRemoteMirror RPC
+  https://gitlab.com/gitlab-org/gitaly/merge_requests/544
 - Implement OperationService.UserCommitFiles RPC
   https://gitlab.com/gitlab-org/gitaly/merge_requests/516
 - Use grpc-go 1.9.1
diff --git a/internal/service/remote/update_remote_mirror.go b/internal/service/remote/update_remote_mirror.go
index b58fd7013..db2ada3ce 100644
--- a/internal/service/remote/update_remote_mirror.go
+++ b/internal/service/remote/update_remote_mirror.go
@@ -1,10 +1,71 @@
 package remote
 
 import (
+	"fmt"
+
 	pb "gitlab.com/gitlab-org/gitaly-proto/go"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/rubyserver"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
-func (*server) UpdateRemoteMirror(pb.RemoteService_UpdateRemoteMirrorServer) error {
-	return helper.Unimplemented
+func (s *server) UpdateRemoteMirror(stream pb.RemoteService_UpdateRemoteMirrorServer) error {
+	firstRequest, err := stream.Recv()
+	if err != nil {
+		return err
+	}
+
+	if err = validateUpdateRemoteMirrorRequest(firstRequest); err != nil {
+		return status.Errorf(codes.InvalidArgument, "UpdateRemoteMirror: %v", err)
+	}
+
+	ctx := stream.Context()
+	client, err := s.RemoteServiceClient(ctx)
+	if err != nil {
+		return err
+	}
+
+	clientCtx, err := rubyserver.SetHeaders(ctx, firstRequest.GetRepository())
+	if err != nil {
+		return err
+	}
+
+	rubyStream, err := client.UpdateRemoteMirror(clientCtx)
+	if err != nil {
+		return err
+	}
+
+	if err := rubyStream.Send(firstRequest); err != nil {
+		return err
+	}
+
+	err = rubyserver.Proxy(func() error {
+		request, err := stream.Recv()
+		if err != nil {
+			return err
+		}
+		return rubyStream.Send(request)
+	})
+
+	if err != nil {
+		return err
+	}
+
+	response, err := rubyStream.CloseAndRecv()
+	if err != nil {
+		return err
+	}
+
+	return stream.SendAndClose(response)
+}
+
+func validateUpdateRemoteMirrorRequest(req *pb.UpdateRemoteMirrorRequest) error {
+	if req.GetRepository() == nil {
+		return fmt.Errorf("empty Repository")
+	}
+	if req.GetRefName() == "" {
+		return fmt.Errorf("empty RefName")
+	}
+
+	return nil
 }
diff --git a/internal/service/remote/update_remote_mirror_test.go b/internal/service/remote/update_remote_mirror_test.go
new file mode 100644
index 000000000..07ed5eee4
--- /dev/null
+++ b/internal/service/remote/update_remote_mirror_test.go
@@ -0,0 +1,125 @@
+package remote
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	pb "gitlab.com/gitlab-org/gitaly-proto/go"
+	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
+	"golang.org/x/net/context"
+	"google.golang.org/grpc/codes"
+)
+
+func TestSuccessfulUpdateRemoteMirrorRequest(t *testing.T) {
+	server, serverSocketPath := runRemoteServiceServer(t)
+	defer server.Stop()
+
+	client, conn := NewRemoteClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, testRepoPath, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	_, mirrorPath, mirrorCleanupFn := testhelper.NewTestRepo(t)
+	defer mirrorCleanupFn()
+
+	remoteName := "remote_mirror_1"
+
+	// Preconditions
+	testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "tag", "v0.0.1", "master") // I needed another tag for the tests
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "remote", "add", remoteName, mirrorPath)
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "fetch", remoteName)
+
+	// Updates
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "new-branch", "60ecb67744cb56576c30214ff52294f8ce2def98")                    // Add branch
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "ignored-branch", "60ecb67744cb56576c30214ff52294f8ce2def98")                // Add branch not matching branch list
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "update-ref", "refs/heads/empty-branch", "0b4bc9a49b562e85de7cc9e834518ea6828729b9")   // Update branch
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "branch", "-D", "not-merged-branch")                                                   // Delete branch
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", "new-tag", "60ecb67744cb56576c30214ff52294f8ce2def98")                          // Add tag
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", "-fam", "Overriding tag", "v1.0.0", "0b4bc9a49b562e85de7cc9e834518ea6828729b9") // Update tag
+	testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "tag", "-d", "v0.0.1")                                                                 // Delete tag
+
+	newTagOid := string(testhelper.MustRunCommand(t, nil, "git", "-C", testRepoPath, "rev-parse", "v1.0.0"))
+	newTagOid = strings.TrimSpace(newTagOid)
+	require.NotEqual(t, newTagOid, "f4e6814c3e4e7a0de82a9e7cd20c626cc963a2f8") // Sanity check that the tag did in fact change
+
+	ctx, cancel := testhelper.Context()
+	defer cancel()
+
+	firstRequest := &pb.UpdateRemoteMirrorRequest{
+		Repository:           testRepo,
+		RefName:              remoteName,
+		OnlyBranchesMatching: nil,
+	}
+	matchingRequest1 := &pb.UpdateRemoteMirrorRequest{
+		OnlyBranchesMatching: [][]byte{[]byte("new-branch"), []byte("empty-branch")},
+	}
+	matchingRequest2 := &pb.UpdateRemoteMirrorRequest{
+		OnlyBranchesMatching: [][]byte{[]byte("not-merged-branch"), []byte("matcher-without-matches")},
+	}
+
+	stream, err := client.UpdateRemoteMirror(ctx)
+	require.NoError(t, err)
+	require.NoError(t, stream.Send(firstRequest))
+	require.NoError(t, stream.Send(matchingRequest1))
+	require.NoError(t, stream.Send(matchingRequest2))
+
+	_, err = stream.CloseAndRecv()
+	require.NoError(t, err)
+
+	mirrorRefs := string(testhelper.MustRunCommand(t, nil, "git", "-C", mirrorPath, "for-each-ref"))
+
+	require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/heads/new-branch")
+	require.NotContains(t, mirrorRefs, "refs/heads/ignored-branch")
+	require.Contains(t, mirrorRefs, "0b4bc9a49b562e85de7cc9e834518ea6828729b9 commit\trefs/heads/empty-branch")
+	require.NotContains(t, mirrorRefs, "refs/heads/not-merged-branch")
+	require.Contains(t, mirrorRefs, "60ecb67744cb56576c30214ff52294f8ce2def98 commit\trefs/tags/new-tag")
+	require.Contains(t, mirrorRefs, newTagOid+" tag\trefs/tags/v1.0.0")
+	require.NotContains(t, mirrorRefs, "refs/tags/v0.0.1")
+}
+
+func TestFailedUpdateRemoteMirrorRequestDueToValidation(t *testing.T) {
+	server, serverSocketPath := runRemoteServiceServer(t)
+	defer server.Stop()
+
+	client, conn := NewRemoteClient(t, serverSocketPath)
+	defer conn.Close()
+
+	testRepo, _, cleanupFn := testhelper.NewTestRepo(t)
+	defer cleanupFn()
+
+	testCases := []struct {
+		desc    string
+		request *pb.UpdateRemoteMirrorRequest
+	}{
+		{
+			desc: "empty Repository",
+			request: &pb.UpdateRemoteMirrorRequest{
+				Repository: nil,
+				RefName:    "remote_mirror_1",
+			},
+		},
+		{
+			desc: "empty RefName",
+			request: &pb.UpdateRemoteMirrorRequest{
+				Repository: testRepo,
+				RefName:    "",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.desc, func(t *testing.T) {
+			ctx, cancel := context.WithCancel(context.Background())
+			defer cancel()
+
+			stream, err := client.UpdateRemoteMirror(ctx)
+			require.NoError(t, err)
+			require.NoError(t, stream.Send(tc.request))
+
+			_, err = stream.CloseAndRecv()
+			testhelper.AssertGrpcError(t, err, codes.InvalidArgument, tc.desc)
+		})
+	}
+}
diff --git a/ruby/lib/gitaly_server/remote_service.rb b/ruby/lib/gitaly_server/remote_service.rb
index 6bbcbba58..f5aac8ed3 100644
--- a/ruby/lib/gitaly_server/remote_service.rb
+++ b/ruby/lib/gitaly_server/remote_service.rb
@@ -35,6 +35,22 @@ module GitalyServer
       end
     end
 
+    def update_remote_mirror(call)
+      bridge_exceptions do
+        request_enum = call.each_remote_read
+        first_request = request_enum.next
+        repo = Gitlab::Git::Repository.from_gitaly(first_request.repository, call)
+        only_branches_matching = first_request.only_branches_matching.to_a
+
+        only_branches_matching += request_enum.flat_map(&:only_branches_matching)
+
+        remote_mirror = Gitlab::Git::RemoteMirror.new(repo, first_request.ref_name)
+        remote_mirror.update(only_branches_matching: only_branches_matching)
+
+        Gitaly::UpdateRemoteMirrorResponse.new
+      end
+    end
+
     private
 
     def parse_refmaps(refmaps)
diff --git a/ruby/lib/gitlab/git.rb b/ruby/lib/gitlab/git.rb
index cf0f7fc7b..094464e03 100644
--- a/ruby/lib/gitlab/git.rb
+++ b/ruby/lib/gitlab/git.rb
@@ -47,11 +47,12 @@ module Gitlab
           gitaly_repository,
           GitalyServer.repo_path(call),
           GitalyServer.gl_repository(call),
+          Gitlab::Git::GitlabProjects.from_gitaly(gitaly_repository, call),
           GitalyServer.repo_alt_dirs(call)
         )
       end
 
-      def initialize(gitaly_repository, path, gl_repository, combined_alt_dirs="")
+      def initialize(gitaly_repository, path, gl_repository, gitlab_projects, combined_alt_dirs="")
         @gitaly_repository = gitaly_repository
 
         alt_dirs = combined_alt_dirs
@@ -62,6 +63,7 @@ module Gitlab
         @relative_path = gitaly_repository.relative_path
         @path = path
         @gl_repository = gl_repository
+        @gitlab_projects = gitlab_projects
         @rugged = Rugged::Repository.new(path, alternates: alt_dirs)
         @attributes = Gitlab::Git::Attributes.new(path)
       end
diff --git a/ruby/lib/gitlab/gitaly_client.rb b/ruby/lib/gitlab/gitaly_client.rb
index 20fafcb8c..e6b45c9aa 100644
--- a/ruby/lib/gitlab/gitaly_client.rb
+++ b/ruby/lib/gitlab/gitaly_client.rb
@@ -13,6 +13,10 @@ module Gitlab
         whitelist = [:fetch_ref, :fetch_internal]
         yield whitelist.include?(args.first)
       end
+
+      def allow_n_plus_1_calls
+        yield
+      end
     end
   end
 end