Skip to content

Commit a1648db

Browse files
10461027791046102779
authored
optimize: grpc methodName & http router (dapr#289)
* fix/typo: modify state annotation Signed-off-by: 1046102779 <seachen@tencent.com> * bugfix&optimize: grpc router bug&http invalid router Signed-off-by: 1046102779 <seachen@tencent.com> * bugfix&optimize: grpc router bug&http invalid router Signed-off-by: 1046102779 <seachen@tencent.com>
1 parent d204f69 commit a1648db

File tree

4 files changed

+36
-23
lines changed

4 files changed

+36
-23
lines changed

‎service/grpc/invoke.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,14 @@ import (
2727

2828
// AddServiceInvocationHandler appends provided service invocation handler with its method to the service.
2929
func (s *Server) AddServiceInvocationHandler(method string, fn cc.ServiceInvocationHandler) error {
30-
if method == "" {
30+
if method == "" || method == "/" {
3131
return fmt.Errorf("servie name required")
3232
}
33+
34+
if method[0] == '/' {
35+
method = method[1:]
36+
}
37+
3338
if fn == nil {
3439
return fmt.Errorf("invocation handler required")
3540
}

‎service/grpc/invoke_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,10 @@ func TestInvokeErrors(t *testing.T) {
4646
server := getTestServer()
4747
err := server.AddServiceInvocationHandler("", nil)
4848
assert.Error(t, err)
49+
50+
err = server.AddServiceInvocationHandler("/", nil)
51+
assert.Error(t, err)
52+
4953
err = server.AddServiceInvocationHandler("test", nil)
5054
assert.Error(t, err)
5155
}
@@ -90,7 +94,7 @@ func TestInvoke(t *testing.T) {
9094
ctx := context.Background()
9195

9296
server := getTestServer()
93-
err := server.AddServiceInvocationHandler(methodName, testInvokeHandler)
97+
err := server.AddServiceInvocationHandler("/"+methodName, testInvokeHandler)
9498
assert.Nil(t, err)
9599

96100
err = server.AddServiceInvocationHandler(methodNameWithError, testInvokeHandlerWithError)

‎service/http/invoke.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ import (
2424

2525
// AddServiceInvocationHandler appends provided service invocation handler with its route to the service.
2626
func (s *Server) AddServiceInvocationHandler(route string, fn common.ServiceInvocationHandler) error {
27-
if route == "" {
27+
if route == "" || route == "/" {
2828
return fmt.Errorf("service route required")
2929
}
30+
3031
if fn == nil {
3132
return fmt.Errorf("invocation handler required")
3233
}

‎service/http/invoke_test.go

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,15 +30,18 @@ import (
3030

3131
func TestInvocationHandlerWithoutHandler(t *testing.T) {
3232
s := newServer("", nil)
33-
err := s.AddServiceInvocationHandler("/", nil)
33+
err := s.AddServiceInvocationHandler("/hello", nil)
3434
assert.Errorf(t, err, "expected error adding event handler")
35+
36+
err = s.AddServiceInvocationHandler("/", nil)
37+
assert.Errorf(t, err, "expected error adding event handler, invalid router")
3538
}
3639

3740
func TestInvocationHandlerWithToken(t *testing.T) {
38-
data := `{"name": "test", "data": hellow}`
41+
data := `{"name": "test", "data": hello}`
3942
_ = os.Setenv(common.AppAPITokenEnvVar, "app-dapr-token")
4043
s := newServer("", nil)
41-
err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
44+
err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
4245
if in == nil || in.Data == nil || in.ContentType == "" {
4346
err = errors.New("nil input")
4447
return
@@ -50,11 +53,11 @@ func TestInvocationHandlerWithToken(t *testing.T) {
5053
}
5154
return
5255
})
53-
assert.NoErrorf(t, err, "error adding event handler")
56+
assert.NoErrorf(t, err, "adding event handler success")
5457

5558
// forbbiden.
56-
req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(data))
57-
assert.NoErrorf(t, err, "error creating request")
59+
req, err := http.NewRequest(http.MethodPost, "/hello", strings.NewReader(data))
60+
assert.NoErrorf(t, err, "creating request success")
5861
req.Header.Set("Content-Type", "application/json")
5962

6063
resp := httptest.NewRecorder()
@@ -70,9 +73,9 @@ func TestInvocationHandlerWithToken(t *testing.T) {
7073
}
7174

7275
func TestInvocationHandlerWithData(t *testing.T) {
73-
data := `{"name": "test", "data": hellow}`
76+
data := `{"name": "test", "data": hello}`
7477
s := newServer("", nil)
75-
err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
78+
err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
7679
if in == nil || in.Data == nil || in.ContentType == "" {
7780
err = errors.New("nil input")
7881
return
@@ -84,42 +87,42 @@ func TestInvocationHandlerWithData(t *testing.T) {
8487
}
8588
return
8689
})
87-
assert.NoErrorf(t, err, "error adding event handler")
90+
assert.NoErrorf(t, err, "adding event handler success")
8891

89-
req, err := http.NewRequest(http.MethodPost, "/", strings.NewReader(data))
90-
assert.NoErrorf(t, err, "error creating request")
92+
req, err := http.NewRequest(http.MethodPost, "/hello", strings.NewReader(data))
93+
assert.NoErrorf(t, err, "creating request success")
9194
req.Header.Set("Content-Type", "application/json")
9295

9396
resp := httptest.NewRecorder()
9497
s.mux.ServeHTTP(resp, req)
9598
assert.Equal(t, http.StatusOK, resp.Code)
9699

97100
b, err := ioutil.ReadAll(resp.Body)
98-
assert.NoErrorf(t, err, "error reading response body")
101+
assert.NoErrorf(t, err, "reading response body success")
99102
assert.Equal(t, data, string(b))
100103
}
101104

102105
func TestInvocationHandlerWithoutInputData(t *testing.T) {
103106
s := newServer("", nil)
104-
err := s.AddServiceInvocationHandler("/", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
107+
err := s.AddServiceInvocationHandler("/hello", func(ctx context.Context, in *common.InvocationEvent) (out *common.Content, err error) {
105108
if in == nil || in.Data != nil {
106109
err = errors.New("nil input")
107110
return
108111
}
109112
return &common.Content{}, nil
110113
})
111-
assert.NoErrorf(t, err, "error adding event handler")
114+
assert.NoErrorf(t, err, "adding event handler success")
112115

113-
req, err := http.NewRequest(http.MethodPost, "/", nil)
114-
assert.NoErrorf(t, err, "error creating request")
116+
req, err := http.NewRequest(http.MethodPost, "/hello", nil)
117+
assert.NoErrorf(t, err, "creating request success")
115118
req.Header.Set("Content-Type", "application/json")
116119

117120
resp := httptest.NewRecorder()
118121
s.mux.ServeHTTP(resp, req)
119122
assert.Equal(t, http.StatusOK, resp.Code)
120123

121124
b, err := ioutil.ReadAll(resp.Body)
122-
assert.NoErrorf(t, err, "error reading response body")
125+
assert.NoErrorf(t, err, "reading response body success")
123126
assert.NotNil(t, b)
124127
assert.Equal(t, "", string(b))
125128
}
@@ -132,13 +135,13 @@ func TestInvocationHandlerWithInvalidRoute(t *testing.T) {
132135
s := newServer("", nil)
133136

134137
err := s.AddServiceInvocationHandler("no-slash", emptyInvocationFn)
135-
assert.NoErrorf(t, err, "error adding no slash route event handler")
138+
assert.NoErrorf(t, err, "adding no slash route event handler success")
136139

137140
err = s.AddServiceInvocationHandler("", emptyInvocationFn)
138141
assert.Errorf(t, err, "expected error from adding no route event handler")
139142

140143
err = s.AddServiceInvocationHandler("/a", emptyInvocationFn)
141-
assert.NoErrorf(t, err, "error adding event handler")
144+
assert.NoErrorf(t, err, "adding event handler success")
142145

143146
makeEventRequest(t, s, "/b", "", http.StatusNotFound)
144147
}
@@ -151,7 +154,7 @@ func TestInvocationHandlerWithError(t *testing.T) {
151154
s := newServer("", nil)
152155

153156
err := s.AddServiceInvocationHandler("/error", errorInvocationFn)
154-
assert.NoErrorf(t, err, "error adding error event handler")
157+
assert.NoErrorf(t, err, "adding error event handler success")
155158

156159
makeEventRequest(t, s, "/error", "", http.StatusInternalServerError)
157160
}

0 commit comments

Comments
 (0)