Grpc fiber router integration (#4)

* update grpc config to be omitempty when not required * add negative test case for initCompoentFromConfig * minor refactoring to mr comments * minor refactoring to mr comments Co-authored-by: ningjie.lee <ningjie.lee@gojek.com>
gojek · Sep 6, 2022 · cc45f83 · cc45f83
1 parent ab99da9
commit cc45f83
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 34 deletions.
diff --git a/config/config.go b/config/config.go
@@ -67,15 +67,16 @@ func (d Duration) MarshalJSON() ([]byte, error) {
 }
 
 // Routes takes in an object of type Routes and returns a map of each route's ID and the route
-func (r Routes) Routes() map[string]fiber.Component {
+func (r Routes) Routes() (map[string]fiber.Component, error) {
 	routes := make(map[string]fiber.Component)
 	for _, routeConfig := range r {
-		if route, err := routeConfig.initComponent(); err != nil {
-		} else {
-			routes[route.ID()] = route
+		route, err := routeConfig.initComponent()
+		if err != nil {
+			return nil, err
 		}
+		routes[route.ID()] = route
 	}
-	return routes
+	return routes, nil
 }
 
 // MultiRouteConfig is used to parse the configuration for a MultiRouteComponent
@@ -112,7 +113,11 @@ func (c *RouterConfig) initComponent() (fiber.Component, error) {
 	default:
 		return nil, fmt.Errorf("unknown router type: [%s]", c.Type)
 	}
-	router.SetRoutes(c.Routes.Routes())
+	routes, err := c.Routes.Routes()
+	if err != nil {
+		return nil, err
+	}
+	router.SetRoutes(routes)
 
 	strategy, err := c.Strategy.Strategy()
 	if err != nil {
@@ -149,7 +154,12 @@ func (c *FanInConfig) FanIn() (fiber.FanIn, error) {
 
 func (c *CombinerConfig) initComponent() (fiber.Component, error) {
 	combiner := fiber.NewCombiner(c.ID)
-	combiner.SetRoutes(c.Routes.Routes())
+
+	routes, err := c.Routes.Routes()
+	if err != nil {
+		return nil, err
+	}
+	combiner.SetRoutes(routes)
 
 	fanIn, err := c.FanIn.FanIn()
 	if err != nil {
@@ -171,8 +181,12 @@ type ProxyConfig struct {
 	Endpoint string   `json:"endpoint" required:"true"`
 	Timeout  Duration `json:"timeout"`
 	Protocol string   `json:"protocol"`
-	Service  string   `json:"service"`
-	Method   string   `json:"method"`
+	GrpcConfig
+}
+
+type GrpcConfig struct {
+	Service string `json:"service,omitempty"`
+	Method  string `json:"method,omitempty"`
 }
 
 func (c *ProxyConfig) initComponent() (fiber.Component, error) {

diff --git a/config/config_test.go b/config/config_test.go
@@ -15,6 +15,7 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"google.golang.org/protobuf/types/dynamicpb"
 )
@@ -93,37 +94,47 @@ func TestFromConfig(t *testing.T) {
 	grpcProxy := fiber.NewProxy(nil, grpcCaller)
 
 	tests := []struct {
-		name       string
-		configPath string
-		want       fiber.Component
+		name              string
+		configPath        string
+		expectedComponent fiber.Component
+		expectedErrMsg    string
 	}{
 		{
-			name:       "http proxy",
-			configPath: "../internal/testdata/config/http_proxy.yaml",
-			want:       httpProxy,
+			name:              "http proxy",
+			configPath:        "../internal/testdata/config/http_proxy.yaml",
+			expectedComponent: httpProxy,
 		},
 		{
-			name:       "grpc proxy",
-			configPath: "../internal/testdata/config/grpc_proxy.yaml",
-			want:       grpcProxy,
+			name:              "grpc proxy",
+			configPath:        "../internal/testdata/config/grpc_proxy.yaml",
+			expectedComponent: grpcProxy,
+		},
+		{
+			name:           "grpc proxy",
+			configPath:     "../internal/testdata/config/invalid_grpc_proxy.yaml",
+			expectedErrMsg: "fiber: grpc dispatcher: missing config (endpoint/service/method)",
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			got, err := config.InitComponentFromConfig(tt.configPath)
-			assert.NoError(t, err)
-			assert.True(t,
-				cmp.Equal(tt.want, got,
-					cmpopts.IgnoreUnexported(grpc.ClientConn{}, dynamicpb.Message{}),
-					cmp.AllowUnexported(
-						fiber.BaseComponent{},
-						fiber.Proxy{},
-						fiber.Caller{},
-						fibergrpc.Dispatcher{},
-						fiberhttp.Dispatcher{}),
-				),
-				"config not equal to expected")
+			if tt.expectedErrMsg == "" {
+				require.NoError(t, err)
+				assert.True(t,
+					cmp.Equal(tt.expectedComponent, got,
+						cmpopts.IgnoreUnexported(grpc.ClientConn{}, dynamicpb.Message{}),
+						cmp.AllowUnexported(
+							fiber.BaseComponent{},
+							fiber.Proxy{},
+							fiber.Caller{},
+							fibergrpc.Dispatcher{},
+							fiberhttp.Dispatcher{}),
+					),
+					"config not equal to expected")
+			} else {
+				assert.Equal(t, tt.expectedErrMsg, err.Error())
+			}
 		})
 	}
 }
diff --git a/grpc/dispatcher.go b/grpc/dispatcher.go
@@ -92,7 +92,7 @@ func NewDispatcher(config DispatcherConfig) (*Dispatcher, error) {
 	if config.Endpoint == "" || config.Service == "" || config.Method == "" {
 		return nil, fiberError.ErrInvalidInput(
 			protocol.GRPC,
-			errors.New("grpc dispatcher: missing config (endpoint/service/method/response-proto)"))
+			errors.New("grpc dispatcher: missing config (endpoint/service/method)"))
 	}
 
 	conn, err := grpc.DialContext(context.Background(), config.Endpoint, grpc.WithTransportCredentials(insecure.NewCredentials()))

diff --git a/grpc/dispatcher_test.go b/grpc/dispatcher_test.go
@@ -97,7 +97,7 @@ func TestNewDispatcher(t *testing.T) {
 			expected: nil,
 			expectedErr: fiberError.ErrInvalidInput(
 				protocol.GRPC,
-				errors.New("grpc dispatcher: missing config (endpoint/service/method/response-proto)")),
+				errors.New("grpc dispatcher: missing config (endpoint/service/method)")),
 		},
 		{
 			name: "empty service",
@@ -108,7 +108,7 @@ func TestNewDispatcher(t *testing.T) {
 			expected: nil,
 			expectedErr: fiberError.ErrInvalidInput(
 				protocol.GRPC,
-				errors.New("grpc dispatcher: missing config (endpoint/service/method/response-proto)")),
+				errors.New("grpc dispatcher: missing config (endpoint/service/method)")),
 		},
 		{
 			name: "empty method",
@@ -119,7 +119,7 @@ func TestNewDispatcher(t *testing.T) {
 			expected: nil,
 			expectedErr: fiberError.ErrInvalidInput(
 				protocol.GRPC,
-				errors.New("grpc dispatcher: missing config (endpoint/service/method/response-proto)")),
+				errors.New("grpc dispatcher: missing config (endpoint/service/method)")),
 		},
 		{
 			name: "invalid endpoint",

diff --git a/internal/testdata/config/invalid_grpc_proxy.yaml b/internal/testdata/config/invalid_grpc_proxy.yaml
@@ -0,0 +1,5 @@
+type: PROXY
+id: proxy_name
+timeout: "20s"
+endpoint: "localhost:50555"
+protocol: "grpc"