Merge branch 'id-refactor-handlers' into 'master'

Remove handler functions which aren't used

See merge request gitlab-org/gitlab-shell!349

go/internal/handler/exec.go

@@ -9,17 +9,10 @@ import (
 	"gitlab.com/gitlab-org/gitaly/client"
 
 	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
-	"gitlab.com/gitlab-org/gitlab-shell/go/internal/logger"
 	"gitlab.com/gitlab-org/labkit/tracing"
 	"google.golang.org/grpc"
 )
 
-// GitalyHandlerFuncWithJSON implementations are responsible for deserializing
-// the request JSON into a GRPC request message, making an appropriate Gitaly
-// call with the request, using the provided client, and returning the exit code
-// or error from the Gitaly call.
-type GitalyHandlerFuncWithJSON func(ctx context.Context, client *grpc.ClientConn, requestJSON string) (int32, error)
-
 // GitalyHandlerFunc implementations are responsible for making
 // an appropriate Gitaly call using the provided client and context
 // and returning an error from the Gitaly call.
@@ -38,22 +31,6 @@ type GitalyCommand struct {
 	Token       string
 }
 
-// RunGitalyCommand provides a bootstrap for Gitaly commands executed
-// through GitLab-Shell. It ensures that logging, tracing and other
-// common concerns are configured before executing the `handler`.
-// RunGitalyCommand will handle errors internally and call
-// `os.Exit()` on completion. This method will never return to
-// the caller.
-func RunGitalyCommand(handler GitalyHandlerFuncWithJSON) {
-	exitCode, err := internalRunGitalyCommand(os.Args, handler)
-
-	if err != nil {
-		logger.Fatal("error: %v", err)
-	}
-
-	os.Exit(exitCode)
-}
-
 // RunGitalyCommand provides a bootstrap for Gitaly commands executed
 // through GitLab-Shell. It ensures that logging, tracing and other
 // common concerns are configured before executing the `handler`.
@@ -71,42 +48,6 @@ func (gc *GitalyCommand) RunGitalyCommand(handler GitalyHandlerFunc) error {
 	return err
 }
 
-// internalRunGitalyCommand runs Gitaly's command by particular Gitaly address and token
-func internalRunGitalyCommand(args []string, handler GitalyHandlerFuncWithJSON) (int, error) {
-	if len(args) != 3 {
-		return 1, fmt.Errorf("expected 2 arguments, got %v", args)
-	}
-
-	cfg, err := config.New()
-	if err != nil {
-		return 1, err
-	}
-
-	if err := logger.Configure(cfg); err != nil {
-		return 1, err
-	}
-
-	gc := &GitalyCommand{
-		Config:      cfg,
-		ServiceName: args[0],
-		Address:     args[1],
-		Token:       os.Getenv("GITALY_TOKEN"),
-	}
-	requestJSON := string(args[2])
-
-	gitalyConn, err := getConn(gc)
-
-	if err != nil {
-		return 1, err
-	}
-
-	exitCode, err := handler(gitalyConn.ctx, gitalyConn.conn, requestJSON)
-
-	gitalyConn.close()
-
-	return int(exitCode), err
-}
-
 func getConn(gc *GitalyCommand) (*GitalyConn, error) {
 	if gc.Address == "" {
 		return nil, fmt.Errorf("no gitaly_address given")

go/internal/handler/exec_test.go

@@ -2,102 +2,41 @@ package handler
 
 import (
 	"context"
-	"fmt"
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitlab-shell/go/internal/testhelper"
 	"google.golang.org/grpc"
+
+	"gitlab.com/gitlab-org/gitlab-shell/go/internal/config"
 )
 
-func TestInteralRunHandler(t *testing.T) {
-	type testCase struct {
-		name    string
-		args    []string
-		handler func(context.Context, *grpc.ClientConn, string) (int32, error)
-		want    int
-		wantErr bool
-	}
+func makeHandler(t *testing.T, err error) func(context.Context, *grpc.ClientConn) (int32, error) {
+	return func(ctx context.Context, client *grpc.ClientConn) (int32, error) {
+		require.NotNil(t, ctx)
+		require.NotNil(t, client)
 
-	var currentTest *testCase
-	makeHandler := func(r1 int32, r2 error) func(context.Context, *grpc.ClientConn, string) (int32, error) {
-		return func(ctx context.Context, client *grpc.ClientConn, requestJSON string) (int32, error) {
-			require.NotNil(t, ctx)
-			require.NotNil(t, client)
-			require.Equal(t, currentTest.args[2], requestJSON)
-			return r1, r2
-		}
+		return 0, err
 	}
-	tests := []testCase{
-		{
-			name:    "expected",
-			args:    []string{"test", "tcp://localhost:9999", "{}"},
-			handler: makeHandler(0, nil),
-			want:    0,
-			wantErr: false,
-		},
-		{
-			name:    "handler_error",
-			args:    []string{"test", "tcp://localhost:9999", "{}"},
-			handler: makeHandler(0, fmt.Errorf("error")),
-			want:    0,
-			wantErr: true,
-		},
-		{
-			name:    "handler_exitcode",
-			args:    []string{"test", "tcp://localhost:9999", "{}"},
-			handler: makeHandler(1, nil),
-			want:    1,
-			wantErr: false,
-		},
-		{
-			name:    "handler_error_exitcode",
-			args:    []string{"test", "tcp://localhost:9999", "{}"},
-			handler: makeHandler(1, fmt.Errorf("error")),
-			want:    1,
-			wantErr: true,
-		},
-		{
-			name:    "too_few_arguments",
-			args:    []string{"test"},
-			handler: makeHandler(10, nil),
-			want:    1,
-			wantErr: true,
-		},
-		{
-			name:    "too_many_arguments",
-			args:    []string{"test", "1", "2", "3"},
-			handler: makeHandler(10, nil),
-			want:    1,
-			wantErr: true,
-		},
-		{
-			name:    "empty_gitaly_address",
-			args:    []string{"test", "", "{}"},
-			handler: makeHandler(10, nil),
-			want:    1,
-			wantErr: true,
-		},
+}
+
+func TestRunGitalyCommand(t *testing.T) {
+	cmd := GitalyCommand{
+		Config:  &config.Config{},
+		Address: "tcp://localhost:9999",
 	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			currentTest = &tt
-			defer func() {
-				currentTest = nil
-			}()
 
-			done, err := testhelper.PrepareTestRootDir()
-			defer done()
-			require.NoError(t, err)
+	err := cmd.RunGitalyCommand(makeHandler(t, nil))
+	require.NoError(t, err)
 
-			got, err := internalRunGitalyCommand(tt.args, tt.handler)
-			if tt.wantErr {
-				require.Error(t, err)
-			} else {
-				require.NoError(t, err)
-			}
+	expectedErr := errors.New("error")
+	err = cmd.RunGitalyCommand(makeHandler(t, expectedErr))
+	require.Equal(t, err, expectedErr)
+}
 
-			require.Equal(t, tt.want, got)
-		})
-	}
+func TestMissingGitalyAddress(t *testing.T) {
+	cmd := GitalyCommand{Config: &config.Config{}}
+
+	err := cmd.RunGitalyCommand(makeHandler(t, nil))
+	require.EqualError(t, err, "no gitaly_address given")
 }

go/internal/handler/receive_pack.go

-package handler
-
-import (
-	"context"
-	"os"
-
-	"gitlab.com/gitlab-org/gitaly/client"
-	pb "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
-	"google.golang.org/grpc"
-)
-
-// ReceivePack issues a Gitaly receive-pack rpc to the provided address
-func ReceivePack(ctx context.Context, conn *grpc.ClientConn, request *pb.SSHReceivePackRequest) (int32, error) {
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
-	return client.ReceivePack(ctx, conn, os.Stdin, os.Stdout, os.Stderr, request)
-}

go/internal/handler/upload_archive.go

-package handler
-
-import (
-	"context"
-	"os"
-
-	"gitlab.com/gitlab-org/gitaly/client"
-	pb "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
-	"google.golang.org/grpc"
-)
-
-// UploadArchive issues a Gitaly upload-archive rpc to the provided address
-func UploadArchive(ctx context.Context, conn *grpc.ClientConn, request *pb.SSHUploadArchiveRequest) (int32, error) {
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
-	return client.UploadArchive(ctx, conn, os.Stdin, os.Stdout, os.Stderr, request)
-}

go/internal/handler/upload_pack.go

-package handler
-
-import (
-	"context"
-	"os"
-
-	"gitlab.com/gitlab-org/gitaly/client"
-	pb "gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
-	"google.golang.org/grpc"
-)
-
-// UploadPack issues a Gitaly upload-pack rpc to the provided address
-func UploadPack(ctx context.Context, conn *grpc.ClientConn, request *pb.SSHUploadPackRequest) (int32, error) {
-	ctx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
-	return client.UploadPack(ctx, conn, os.Stdin, os.Stdout, os.Stderr, request)
-}