diff options
author | John Cai <jcai@gitlab.com> | 2019-01-04 17:56:05 +0300 |
---|---|---|
committer | Jacob Vosmaer <jacob@gitlab.com> | 2019-01-04 17:56:05 +0300 |
commit | 3fef513cfcfd465f753e4f5f9649b4f9e7692597 (patch) | |
tree | 5e9b9d1e79037e1e5d4040bc53e2e949ada48707 | |
parent | c3dfc0b4d5cdcc24b15c05c5f32aa6f549991d0c (diff) |
Migrate writeref from using the ruby implementation to go
-rw-r--r-- | changelogs/unreleased/migrate-writeref.yml | 5 | ||||
-rw-r--r-- | internal/git/updateref/updateref.go | 4 | ||||
-rw-r--r-- | internal/git/updateref/updateref_test.go | 13 | ||||
-rw-r--r-- | internal/helper/error.go | 6 | ||||
-rw-r--r-- | internal/service/repository/write_ref.go | 43 |
5 files changed, 59 insertions, 12 deletions
diff --git a/changelogs/unreleased/migrate-writeref.yml b/changelogs/unreleased/migrate-writeref.yml new file mode 100644 index 000000000..adee7d3e1 --- /dev/null +++ b/changelogs/unreleased/migrate-writeref.yml @@ -0,0 +1,5 @@ +--- +title: Migrate writeref from using the ruby implementation to go +merge_request: 1008 +author: johncai +type: other diff --git a/internal/git/updateref/updateref.go b/internal/git/updateref/updateref.go index c61cc7e62..01376be52 100644 --- a/internal/git/updateref/updateref.go +++ b/internal/git/updateref/updateref.go @@ -40,8 +40,8 @@ func (u *Updater) Create(ref, value string) error { // Update commands the reference to be updated to point at the sha specified in // newvalue -func (u *Updater) Update(ref, newvalue string) error { - _, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00\x00", ref, newvalue) +func (u *Updater) Update(ref, newvalue, oldvalue string) error { + _, err := fmt.Fprintf(u.cmd, "update %s\x00%s\x00%s\x00", ref, newvalue, oldvalue) return err } diff --git a/internal/git/updateref/updateref_test.go b/internal/git/updateref/updateref_test.go index 1fb9aefb8..3de350b25 100644 --- a/internal/git/updateref/updateref_test.go +++ b/internal/git/updateref/updateref_test.go @@ -66,7 +66,7 @@ func TestUpdate(t *testing.T) { require.NotNil(t, commit, "%s does not exist in the test repository", ref) require.NotEqual(t, commit.Id, sha, "%s points to HEAD: %s in the test repository", ref, sha) - require.NoError(t, updater.Update(ref, sha)) + require.NoError(t, updater.Update(ref, sha, "")) require.NoError(t, updater.Wait()) // check the ref was updated @@ -74,6 +74,17 @@ func TestUpdate(t *testing.T) { require.NoError(t, logErr) require.NotNil(t, commit) require.Equal(t, commit.Id, sha, "reference was not updated") + + // since ref has been updated to HEAD, we know that it does not point to HEAD^. So, HEAD^ is an invalid "old value" for updating ref + parentCommit, err := log.GetCommit(ctx, testRepo, "HEAD^") + require.NoError(t, err) + require.Error(t, updater.Update(ref, parentCommit.Id, parentCommit.Id)) + + // check the ref was not updated + commit, logErr = log.GetCommit(ctx, testRepo, ref) + require.NoError(t, logErr) + require.NotNil(t, commit) + require.NotEqual(t, commit.Id, parentCommit.Id, "reference was updated when it shouldn't have been") } func TestDelete(t *testing.T) { diff --git a/internal/helper/error.go b/internal/helper/error.go index 4966b788d..738974760 100644 --- a/internal/helper/error.go +++ b/internal/helper/error.go @@ -17,6 +17,12 @@ func DecorateError(code codes.Code, err error) error { return err } +// ErrInternal wrappes err with codes.Internal, unless err is already a grpc error +func ErrInternal(err error) error { return DecorateError(codes.Internal, err) } + +// ErrInvalidArgument wrappes err with codes.InvalidArgument, unless err is already a grpc error +func ErrInvalidArgument(err error) error { return DecorateError(codes.InvalidArgument, err) } + // GrpcCode emulates the old grpc.Code function: it translates errors into codes.Code values. func GrpcCode(err error) codes.Code { if err == nil { diff --git a/internal/service/repository/write_ref.go b/internal/service/repository/write_ref.go index a70c15490..f9b193bac 100644 --- a/internal/service/repository/write_ref.go +++ b/internal/service/repository/write_ref.go @@ -6,28 +6,53 @@ import ( "gitlab.com/gitlab-org/gitaly-proto/go/gitalypb" "gitlab.com/gitlab-org/gitaly/internal/git" - "gitlab.com/gitlab-org/gitaly/internal/rubyserver" + "gitlab.com/gitlab-org/gitaly/internal/git/updateref" + "gitlab.com/gitlab-org/gitaly/internal/helper" "golang.org/x/net/context" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" ) func (s *server) WriteRef(ctx context.Context, req *gitalypb.WriteRefRequest) (*gitalypb.WriteRefResponse, error) { if err := validateWriteRefRequest(req); err != nil { - return nil, status.Errorf(codes.InvalidArgument, "WriteRef: %v", err) + return nil, helper.ErrInvalidArgument(err) } + if err := writeRef(ctx, req); err != nil { + return nil, helper.ErrInternal(err) + } + + return &gitalypb.WriteRefResponse{}, nil +} + +func writeRef(ctx context.Context, req *gitalypb.WriteRefRequest) error { + if string(req.Ref) == "HEAD" { + return updateSymbolicRef(ctx, req) + } + return updateRef(ctx, req) +} - client, err := s.RepositoryServiceClient(ctx) +func updateSymbolicRef(ctx context.Context, req *gitalypb.WriteRefRequest) error { + cmd, err := git.Command(ctx, req.GetRepository(), "symbolic-ref", string(req.GetRef()), string(req.GetRevision())) if err != nil { - return nil, err + return fmt.Errorf("error when creating symbolic-ref command: %v", err) } + if err = cmd.Wait(); err != nil { + return fmt.Errorf("error when running symbolic-ref command: %v", err) + } + return nil +} - clientCtx, err := rubyserver.SetHeaders(ctx, req.GetRepository()) +func updateRef(ctx context.Context, req *gitalypb.WriteRefRequest) error { + u, err := updateref.New(ctx, req.GetRepository()) if err != nil { - return nil, err + return fmt.Errorf("error when running creating new updater: %v", err) } + if err = u.Update(string(req.GetRef()), string(req.GetRevision()), string(req.GetOldRevision())); err != nil { + return fmt.Errorf("error when creating update-ref command: %v", err) + } + if err = u.Wait(); err != nil { + return fmt.Errorf("error when running update-ref command: %v", err) + } + return nil - return client.WriteRef(clientCtx, req) } func validateWriteRefRequest(req *gitalypb.WriteRefRequest) error { |