optimize: grpc methodName & http router (dapr#289)

* fix/typo: modify state annotation Signed-off-by: 1046102779 <[email protected]> * bugfix&optimize: grpc router bug&http invalid router Signed-off-by: 1046102779 <[email protected]> * bugfix&optimize: grpc router bug&http invalid router Signed-off-by: 1046102779 <[email protected]>
back-in-sunset · Jun 10, 2022 · a1648db · a1648db
1 parent d204f69
commit a1648db
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 23 deletions.
diff --git a/service/grpc/invoke.go b/service/grpc/invoke.go
@@ -27,9 +27,14 @@ import (
 
 // AddServiceInvocationHandler appends provided service invocation handler with its method to the service.
 func (s *Server) AddServiceInvocationHandler(method string, fn cc.ServiceInvocationHandler) error {
-	if method == "" {
+	if method == "" || method == "/" {
 		return fmt.Errorf("servie name required")
 	}
+
+	if method[0] == '/' {
+		method = method[1:]
+	}
+
 	if fn == nil {
 		return fmt.Errorf("invocation handler required")
 	}

diff --git a/service/grpc/invoke_test.go b/service/grpc/invoke_test.go
@@ -46,6 +46,10 @@ func TestInvokeErrors(t *testing.T) {
 	server := getTestServer()
 	err := server.AddServiceInvocationHandler("", nil)
 	assert.Error(t, err)
+
+	err = server.AddServiceInvocationHandler("/", nil)
+	assert.Error(t, err)
+
 	err = server.AddServiceInvocationHandler("test", nil)
 	assert.Error(t, err)
 }
@@ -90,7 +94,7 @@ func TestInvoke(t *testing.T) {
 	ctx := context.Background()
 
 	server := getTestServer()
-	err := server.AddServiceInvocationHandler(methodName, testInvokeHandler)
+	err := server.AddServiceInvocationHandler("/"+methodName, testInvokeHandler)
 	assert.Nil(t, err)
 
 	err = server.AddServiceInvocationHandler(methodNameWithError, testInvokeHandlerWithError)

diff --git a/service/http/invoke.go b/service/http/invoke.go
@@ -24,9 +24,10 @@ import (
 
 // AddServiceInvocationHandler appends provided service invocation handler with its route to the service.
 func (s *Server) AddServiceInvocationHandler(route string, fn common.ServiceInvocationHandler) error {
-	if route == "" {
+	if route == "" || route == "/" {
 		return fmt.Errorf("service route required")
 	}
+
 	if fn == nil {
 		return fmt.Errorf("invocation handler required")
 	}

diff --git a/service/http/invoke_test.go b/service/http/invoke_test.go
@@ -30,15 +30,18 @@ import (
 
 func TestInvocationHandlerWithoutHandler(t *testing.T) {
 	s := newServer("", nil)
-	err := s.AddServiceInvocationHandler("/", nil)
+	err := s.AddServiceInvocationHandler("/hello", nil)
 	assert.Errorf(t, err, "expected error adding event handler")
+
+	err = s.AddServiceInvocationHandler("/", nil)
+	assert.Errorf(t, err, "expected error adding event handler, invalid router")
 }
 
 func TestInvocationHandlerWithToken(t *testing.T) {
-	data := `{"name": "test", "data": hellow}`
+	data := `{"name": "test", "data": hello}`
 	_ = os.Setenv(common.AppAPITokenEnvVar, "app-dapr-token")
 	s := newServer("", nil)
-	err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
+	err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
 		if in == nil || in.Data == nil || in.ContentType == "" {
 			err = errors.New("nil input")
 			return
@@ -50,11 +53,11 @@ func TestInvocationHandlerWithToken(t *testing.T) {
 		}
 		return
 	})
-	assert.NoErrorf(t, err, "error adding event handler")
+	assert.NoErrorf(t, err, "adding event handler success")
 
 	// forbbiden.
-	req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(data))
-	assert.NoErrorf(t, err, "error creating request")
+	req, err := http.NewRequest(http.MethodPost, "/hello", strings.NewReader(data))
+	assert.NoErrorf(t, err, "creating request success")
 	req.Header.Set("Content-Type", "application/json")
 
 	resp := httptest.NewRecorder()
@@ -70,9 +73,9 @@ func TestInvocationHandlerWithToken(t *testing.T) {
 }
 
 func TestInvocationHandlerWithData(t *testing.T) {
-	data := `{"name": "test", "data": hellow}`
+	data := `{"name": "test", "data": hello}`
 	s := newServer("", nil)
-	err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
+	err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
 		if in == nil || in.Data == nil || in.ContentType == "" {
 			err = errors.New("nil input")
 			return
@@ -84,42 +87,42 @@ func TestInvocationHandlerWithData(t *testing.T) {
 		}
 		return
 	})
-	assert.NoErrorf(t, err, "error adding event handler")
+	assert.NoErrorf(t, err, "adding event handler success")
 
-	req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(data))
-	assert.NoErrorf(t, err, "error creating request")
+	req, err := http.NewRequest(http.MethodPost, "/hello", strings.NewReader(data))
+	assert.NoErrorf(t, err, "creating request success")
 	req.Header.Set("Content-Type", "application/json")
 
 	resp := httptest.NewRecorder()
 	s.mux.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
 
 	b, err := ioutil.ReadAll(resp.Body)
-	assert.NoErrorf(t, err, "error reading response body")
+	assert.NoErrorf(t, err, "reading response body success")
 	assert.Equal(t, data, string(b))
 }
 
 func TestInvocationHandlerWithoutInputData(t *testing.T) {
 	s := newServer("", nil)
-	err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
+	err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
 		if in == nil || in.Data != nil {
 			err = errors.New("nil input")
 			return
 		}
 		return &common.Content{}, nil
 	})
-	assert.NoErrorf(t, err, "error adding event handler")
+	assert.NoErrorf(t, err, "adding event handler success")
 
-	req, err := http.NewRequest(http.MethodPost, "/", nil)
-	assert.NoErrorf(t, err, "error creating request")
+	req, err := http.NewRequest(http.MethodPost, "/hello", nil)
+	assert.NoErrorf(t, err, "creating request success")
 	req.Header.Set("Content-Type", "application/json")
 
 	resp := httptest.NewRecorder()
 	s.mux.ServeHTTP(resp, req)
 	assert.Equal(t, http.StatusOK, resp.Code)
 
 	b, err := ioutil.ReadAll(resp.Body)
-	assert.NoErrorf(t, err, "error reading response body")
+	assert.NoErrorf(t, err, "reading response body success")
 	assert.NotNil(t, b)
 	assert.Equal(t, "", string(b))
 }
@@ -132,13 +135,13 @@ func TestInvocationHandlerWithInvalidRoute(t *testing.T) {
 	s := newServer("", nil)
 
 	err := s.AddServiceInvocationHandler("no-slash", emptyInvocationFn)
-	assert.NoErrorf(t, err, "error adding no slash route event handler")
+	assert.NoErrorf(t, err, "adding no slash route event handler success")
 
 	err = s.AddServiceInvocationHandler("", emptyInvocationFn)
 	assert.Errorf(t, err, "expected error from adding no route event handler")
 
 	err = s.AddServiceInvocationHandler("/a", emptyInvocationFn)
-	assert.NoErrorf(t, err, "error adding event handler")
+	assert.NoErrorf(t, err, "adding event handler success")
 
 	makeEventRequest(t, s, "/b", "", http.StatusNotFound)
 }
@@ -151,7 +154,7 @@ func TestInvocationHandlerWithError(t *testing.T) {
 	s := newServer("", nil)
 
 	err := s.AddServiceInvocationHandler("/error", errorInvocationFn)
-	assert.NoErrorf(t, err, "error adding error event handler")
+	assert.NoErrorf(t, err, "adding error event handler success")
 
 	makeEventRequest(t, s, "/error", "", http.StatusInternalServerError)
 }