You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by ba...@apache.org on 2020/08/09 12:48:18 UTC

[dubbo-go] 06/13: modify some comments and when parsing parameters occurred error, return error immediately

This is an automated email from the ASF dual-hosted git repository.

baze pushed a commit to branch 1.4
in repository https://gitbox.apache.org/repos/asf/dubbo-go.git

commit 79bd56cfefefb2431c36ab4ffecd81e37f3e13c7
Author: Patrick <dr...@foxmail.com>
AuthorDate: Wed Apr 1 23:38:08 2020 +0800

    modify some comments and when parsing parameters occurred error, return error immediately
---
 protocol/rest/client/client_impl/resty_client.go |   4 +-
 protocol/rest/client/rest_client.go              |   6 +-
 protocol/rest/server/rest_server.go              | 114 +++++++++++++----------
 3 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/protocol/rest/client/client_impl/resty_client.go b/protocol/rest/client/client_impl/resty_client.go
index bfd7445..b60f50a 100644
--- a/protocol/rest/client/client_impl/resty_client.go
+++ b/protocol/rest/client/client_impl/resty_client.go
@@ -40,11 +40,12 @@ func init() {
 	extension.SetRestClient(constant.DEFAULT_REST_CLIENT, NewRestyClient)
 }
 
-// A rest client implement by Resty
+// RestyClient a rest client implement by Resty
 type RestyClient struct {
 	client *resty.Client
 }
 
+// NewRestyClient a constructor of RestyClient
 func NewRestyClient(restOption *client.RestOptions) client.RestClient {
 	client := resty.New()
 	client.SetTransport(
@@ -66,6 +67,7 @@ func NewRestyClient(restOption *client.RestOptions) client.RestClient {
 	}
 }
 
+// Do send request by RestyClient
 func (rc *RestyClient) Do(restRequest *client.RestClientRequest, res interface{}) error {
 	req := rc.client.R()
 	req.Header = restRequest.Header
diff --git a/protocol/rest/client/rest_client.go b/protocol/rest/client/rest_client.go
index 47d17c6..d63c5e0 100644
--- a/protocol/rest/client/rest_client.go
+++ b/protocol/rest/client/rest_client.go
@@ -22,13 +22,13 @@ import (
 	"time"
 )
 
-// Some rest options
+// RestOptions
 type RestOptions struct {
 	RequestTimeout time.Duration
 	ConnectTimeout time.Duration
 }
 
-// Client request
+// RestClientRequest
 type RestClientRequest struct {
 	Header      http.Header
 	Location    string
@@ -39,7 +39,7 @@ type RestClientRequest struct {
 	Body        interface{}
 }
 
-// User can implement this client interface to send request
+// RestClient user can implement this client interface to send request
 type RestClient interface {
 	Do(request *RestClientRequest, res interface{}) error
 }
diff --git a/protocol/rest/server/rest_server.go b/protocol/rest/server/rest_server.go
index 60f0dab..60cac9a 100644
--- a/protocol/rest/server/rest_server.go
+++ b/protocol/rest/server/rest_server.go
@@ -19,6 +19,7 @@ package server
 
 import (
 	"context"
+	"errors"
 	"net/http"
 	"reflect"
 	"strconv"
@@ -37,6 +38,8 @@ import (
 	rest_config "github.com/apache/dubbo-go/protocol/rest/config"
 )
 
+const parseParameterErrorStr = "An error occurred while parsing parameters on the server"
+
 type RestServer interface {
 	// start rest server
 	Start(url common.URL)
@@ -50,19 +53,19 @@ type RestServer interface {
 
 // RestServerRequest interface
 type RestServerRequest interface {
-	// Get the Ptr of http.Request
+	// RawRequest get the Ptr of http.Request
 	RawRequest() *http.Request
-	// Get the path parameter by name
+	// PathParameter get the path parameter by name
 	PathParameter(name string) string
-	// Get the map of the path parameters
+	// PathParameters get the map of the path parameters
 	PathParameters() map[string]string
-	// Get the query parameter by name
+	// QueryParameter get the query parameter by name
 	QueryParameter(name string) string
-	// Get the map of query parameters
+	// QueryParameters get the map of query parameters
 	QueryParameters(name string) []string
-	// Get the body parameter of name
+	// BodyParameter get the body parameter of name
 	BodyParameter(name string) (string, error)
-	// Get the header parameter of name
+	// HeaderParameter get the header parameter of name
 	HeaderParameter(name string) string
 	// ReadEntity checks the Accept header and reads the content into the entityPointer.
 	ReadEntity(entityPointer interface{}) error
@@ -72,12 +75,13 @@ type RestServerRequest interface {
 type RestServerResponse interface {
 	http.ResponseWriter
 	// WriteError writes the http status and the error string on the response. err can be nil.
-	// Return an error if writing was not succesful.
+	// Return an error if writing was not successful.
 	WriteError(httpStatus int, err error) (writeErr error)
 	// WriteEntity marshals the value using the representation denoted by the Accept Header.
 	WriteEntity(value interface{}) error
 }
 
+// GetRouteFunc
 // A route function will be invoked by http server
 func GetRouteFunc(invoker protocol.Invoker, methodConfig *rest_config.RestMethodConfig) func(req RestServerRequest, resp RestServerResponse) {
 	return func(req RestServerRequest, resp RestServerResponse) {
@@ -92,9 +96,16 @@ func GetRouteFunc(invoker protocol.Invoker, methodConfig *rest_config.RestMethod
 		replyType := method.ReplyType()
 		if (len(argsTypes) == 1 || len(argsTypes) == 2 && replyType == nil) &&
 			argsTypes[0].String() == "[]interface {}" {
-			args = getArgsInterfaceFromRequest(req, methodConfig)
+			args, err = getArgsInterfaceFromRequest(req, methodConfig)
 		} else {
-			args = getArgsFromRequest(req, argsTypes, methodConfig)
+			args, err = getArgsFromRequest(req, argsTypes, methodConfig)
+		}
+		if err != nil {
+			logger.Errorf("[Go Restful] parsing parameters error:%v", err)
+			err = resp.WriteError(http.StatusInternalServerError, errors.New(parseParameterErrorStr))
+			if err != nil {
+				logger.Errorf("[Go Restful] WriteErrorString error:%v", err)
+			}
 		}
 		result := invoker.Invoke(context.Background(), invocation.NewRPCInvocation(methodConfig.MethodName, args, make(map[string]string)))
 		if result.Error() != nil {
@@ -111,9 +122,9 @@ func GetRouteFunc(invoker protocol.Invoker, methodConfig *rest_config.RestMethod
 	}
 }
 
-// when service function like GetUser(req []interface{}, rsp *User) error
+// getArgsInterfaceFromRequest when service function like GetUser(req []interface{}, rsp *User) error
 // use this method to get arguments
-func getArgsInterfaceFromRequest(req RestServerRequest, methodConfig *rest_config.RestMethodConfig) []interface{} {
+func getArgsInterfaceFromRequest(req RestServerRequest, methodConfig *rest_config.RestMethodConfig) ([]interface{}, error) {
 	argsMap := make(map[int]interface{}, 8)
 	maxKey := 0
 	for k, v := range methodConfig.PathParamsMap {
@@ -145,10 +156,10 @@ func getArgsInterfaceFromRequest(req RestServerRequest, methodConfig *rest_confi
 		}
 		m := make(map[string]interface{})
 		// TODO read as a slice
-		if err := req.ReadEntity(&m); err != nil {
-			logger.Warnf("[Go restful] Read body entity as map[string]interface{} error:%v", perrors.WithStack(err))
-		} else {
+		if err := req.ReadEntity(&m); err == nil {
 			argsMap[methodConfig.Body] = m
+		} else {
+			return nil, perrors.Errorf("[Go restful] Read body entity as map[string]interface{} error:%v", err)
 		}
 	}
 	args := make([]interface{}, maxKey+1)
@@ -157,30 +168,37 @@ func getArgsInterfaceFromRequest(req RestServerRequest, methodConfig *rest_confi
 			args[k] = v
 		}
 	}
-	return args
+	return args, nil
 }
 
-// get arguments from server.RestServerRequest
-func getArgsFromRequest(req RestServerRequest, argsTypes []reflect.Type, methodConfig *rest_config.RestMethodConfig) []interface{} {
+// getArgsFromRequest get arguments from server.RestServerRequest
+func getArgsFromRequest(req RestServerRequest, argsTypes []reflect.Type, methodConfig *rest_config.RestMethodConfig) ([]interface{}, error) {
 	argsLength := len(argsTypes)
 	args := make([]interface{}, argsLength)
 	for i, t := range argsTypes {
 		args[i] = reflect.Zero(t).Interface()
 	}
-	assembleArgsFromPathParams(methodConfig, argsLength, argsTypes, req, args)
-	assembleArgsFromQueryParams(methodConfig, argsLength, argsTypes, req, args)
-	assembleArgsFromBody(methodConfig, argsTypes, req, args)
-	assembleArgsFromHeaders(methodConfig, req, argsLength, argsTypes, args)
-	return args
+	if err := assembleArgsFromPathParams(methodConfig, argsLength, argsTypes, req, args); err != nil {
+		return nil, err
+	}
+	if err := assembleArgsFromQueryParams(methodConfig, argsLength, argsTypes, req, args); err != nil {
+		return nil, err
+	}
+	if err := assembleArgsFromBody(methodConfig, argsTypes, req, args); err != nil {
+		return nil, err
+	}
+	if err := assembleArgsFromHeaders(methodConfig, req, argsLength, argsTypes, args); err != nil {
+		return nil, err
+	}
+	return args, nil
 }
 
-// assemble arguments from headers
-func assembleArgsFromHeaders(methodConfig *rest_config.RestMethodConfig, req RestServerRequest, argsLength int, argsTypes []reflect.Type, args []interface{}) {
+// assembleArgsFromHeaders assemble arguments from headers
+func assembleArgsFromHeaders(methodConfig *rest_config.RestMethodConfig, req RestServerRequest, argsLength int, argsTypes []reflect.Type, args []interface{}) error {
 	for k, v := range methodConfig.HeadersMap {
 		param := req.HeaderParameter(v)
 		if k < 0 || k >= argsLength {
-			logger.Errorf("[Go restful] Header param parse error, the args:%v doesn't exist", k)
-			continue
+			return perrors.Errorf("[Go restful] Header param parse error, the args:%v doesn't exist", k)
 		}
 		t := argsTypes[k]
 		if t.Kind() == reflect.Ptr {
@@ -189,13 +207,14 @@ func assembleArgsFromHeaders(methodConfig *rest_config.RestMethodConfig, req Res
 		if t.Kind() == reflect.String {
 			args[k] = param
 		} else {
-			logger.Errorf("[Go restful] Header param parse error, the args:%v of type isn't string", k)
+			return perrors.Errorf("[Go restful] Header param parse error, the args:%v of type isn't string", k)
 		}
 	}
+	return nil
 }
 
-// assemble arguments from body
-func assembleArgsFromBody(methodConfig *rest_config.RestMethodConfig, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) {
+// assembleArgsFromBody assemble arguments from body
+func assembleArgsFromBody(methodConfig *rest_config.RestMethodConfig, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) error {
 	if methodConfig.Body >= 0 && methodConfig.Body < len(argsTypes) {
 		t := argsTypes[methodConfig.Body]
 		kind := t.Kind()
@@ -213,16 +232,17 @@ func assembleArgsFromBody(methodConfig *rest_config.RestMethodConfig, argsTypes
 				ni = n.Interface()
 			}
 		}
-		if err := req.ReadEntity(&ni); err != nil {
-			logger.Errorf("[Go restful] Read body entity error:%v", err)
-		} else {
+		if err := req.ReadEntity(&ni); err == nil {
 			args[methodConfig.Body] = ni
+		} else {
+			return perrors.Errorf("[Go restful] Read body entity error, error is %v", perrors.WithStack(err))
 		}
 	}
+	return nil
 }
 
-// assemble arguments from query params
-func assembleArgsFromQueryParams(methodConfig *rest_config.RestMethodConfig, argsLength int, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) {
+// assembleArgsFromQueryParams assemble arguments from query params
+func assembleArgsFromQueryParams(methodConfig *rest_config.RestMethodConfig, argsLength int, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) error {
 	var (
 		err   error
 		param interface{}
@@ -230,8 +250,7 @@ func assembleArgsFromQueryParams(methodConfig *rest_config.RestMethodConfig, arg
 	)
 	for k, v := range methodConfig.QueryParamsMap {
 		if k < 0 || k >= argsLength {
-			logger.Errorf("[Go restful] Query param parse error, the args:%v doesn't exist", k)
-			continue
+			return perrors.Errorf("[Go restful] Query param parse error, the args:%v doesn't exist", k)
 		}
 		t := argsTypes[k]
 		kind := t.Kind()
@@ -252,19 +271,18 @@ func assembleArgsFromQueryParams(methodConfig *rest_config.RestMethodConfig, arg
 		} else if kind == reflect.Int64 {
 			param, err = strconv.ParseInt(req.QueryParameter(v), 10, 64)
 		} else {
-			logger.Errorf("[Go restful] Query param parse error, the args:%v of type isn't int or string or slice", k)
-			continue
+			return perrors.Errorf("[Go restful] Query param parse error, the args:%v of type isn't int or string or slice", k)
 		}
 		if err != nil {
-			logger.Errorf("[Go restful] Query param parse error, error is %v", err)
-			continue
+			return perrors.Errorf("[Go restful] Query param parse error, error:%v", perrors.WithStack(err))
 		}
 		args[k] = param
 	}
+	return nil
 }
 
-// assemble arguments from path params
-func assembleArgsFromPathParams(methodConfig *rest_config.RestMethodConfig, argsLength int, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) {
+// assembleArgsFromPathParams assemble arguments from path params
+func assembleArgsFromPathParams(methodConfig *rest_config.RestMethodConfig, argsLength int, argsTypes []reflect.Type, req RestServerRequest, args []interface{}) error {
 	var (
 		err   error
 		param interface{}
@@ -272,8 +290,7 @@ func assembleArgsFromPathParams(methodConfig *rest_config.RestMethodConfig, args
 	)
 	for k, v := range methodConfig.PathParamsMap {
 		if k < 0 || k >= argsLength {
-			logger.Errorf("[Go restful] Path param parse error, the args:%v doesn't exist", k)
-			continue
+			return perrors.Errorf("[Go restful] Path param parse error, the args:%v doesn't exist", k)
 		}
 		t := argsTypes[k]
 		kind := t.Kind()
@@ -292,13 +309,12 @@ func assembleArgsFromPathParams(methodConfig *rest_config.RestMethodConfig, args
 		} else if kind == reflect.String {
 			param = req.PathParameter(v)
 		} else {
-			logger.Warnf("[Go restful] Path param parse error, the args:%v of type isn't int or string", k)
-			continue
+			return perrors.Errorf("[Go restful] Path param parse error, the args:%v of type isn't int or string", k)
 		}
 		if err != nil {
-			logger.Errorf("[Go restful] Path param parse error, error is %v", err)
-			continue
+			return perrors.Errorf("[Go restful] Path param parse error, error is %v", perrors.WithStack(err))
 		}
 		args[k] = param
 	}
+	return nil
 }