diff --git a/gateway/controller/land.go b/gateway/controller/land.go index 83a840c4..cf30fa91 100644 --- a/gateway/controller/land.go +++ b/gateway/controller/land.go @@ -2,6 +2,7 @@ package controller import ( "context" + "errors" "fmt" "time" @@ -13,6 +14,15 @@ import ( "go.uber.org/zap" ) +// ErrInvalidRequest is returned when the request fails validation. +// This error should be mapped to codes.InvalidArgument at the gRPC layer. +var ErrInvalidRequest = errors.New("invalid request") + +// IsInvalidRequest returns true if any error in the error chain is ErrInvalidRequest. +func IsInvalidRequest(err error) bool { + return errors.Is(err, ErrInvalidRequest) +} + // LandController handles land business logic for the gateway type LandController struct { logger *zap.Logger @@ -40,6 +50,17 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan c.metricsScope.Counter("land_request_count").Inc(1) + // Validate required fields. + if req.Queue == "" { + return nil, fmt.Errorf("LandController requires the request to have a queue name specified: %w", ErrInvalidRequest) + } + if req.Change == nil || req.Change.Source == "" { + return nil, fmt.Errorf("LandController requires the request to have a change source specified: %w", ErrInvalidRequest) + } + if len(req.Change.GetIds()) == 0 { + return nil, fmt.Errorf("LandController requires the request to have at least one change ID specified: %w", ErrInvalidRequest) + } + change := entity.Change{ Source: req.Change.GetSource(), IDs: req.Change.GetIds(), diff --git a/gateway/controller/land_test.go b/gateway/controller/land_test.go index 46b78a53..6a598c32 100644 --- a/gateway/controller/land_test.go +++ b/gateway/controller/land_test.go @@ -184,3 +184,91 @@ func TestLand_CounterDomainIncludesQueue(t *testing.T) { require.NoError(t, err) assert.Equal(t, "request/my-queue", capturedDomain) } + +func TestLand_ReturnsErrorOnEmptyQueue(t *testing.T) { + store := &mockStorage{requestStore: &mockRequestStore{ + createFunc: func(ctx context.Context, request entity.Request) error { + return nil + }, + }} + cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) { + return 1, nil + }} + controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "", + Change: &pb.Change{Source: "github", Ids: []string{"123"}}, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.True(t, IsInvalidRequest(err)) +} + +func TestLand_ReturnsErrorOnEmptyChangeSource(t *testing.T) { + store := &mockStorage{requestStore: &mockRequestStore{ + createFunc: func(ctx context.Context, request entity.Request) error { + return nil + }, + }} + cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) { + return 1, nil + }} + controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "test-queue", + Change: &pb.Change{Source: "", Ids: []string{"123"}}, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.True(t, IsInvalidRequest(err)) +} + +func TestLand_ReturnsErrorOnNilChange(t *testing.T) { + store := &mockStorage{requestStore: &mockRequestStore{ + createFunc: func(ctx context.Context, request entity.Request) error { + return nil + }, + }} + cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) { + return 1, nil + }} + controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "test-queue", + Change: nil, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.True(t, IsInvalidRequest(err)) +} + +func TestLand_ReturnsErrorOnEmptyChangeIDs(t *testing.T) { + store := &mockStorage{requestStore: &mockRequestStore{ + createFunc: func(ctx context.Context, request entity.Request) error { + return nil + }, + }} + cnt := &mockCounter{nextFunc: func(ctx context.Context, domain string) (int64, error) { + return 1, nil + }} + controller := NewLandController(zap.NewNop(), tally.NoopScope, store, cnt) + ctx := context.Background() + + req := &pb.LandRequest{ + Queue: "test-queue", + Change: &pb.Change{Source: "github", Ids: []string{}}, + } + _, err := controller.Land(ctx, req) + + require.Error(t, err) + assert.True(t, IsInvalidRequest(err)) +}