You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by GitBox <gi...@apache.org> on 2022/05/08 14:32:46 UTC
[GitHub] [dubbo-go] sanxun0325 commented on a diff in pull request #1855: feat: Rest protocol Support
sanxun0325 commented on code in PR #1855:
URL: https://github.com/apache/dubbo-go/pull/1855#discussion_r866873701
##########
config/rest_config.go:
##########
@@ -32,6 +43,8 @@ type RestConsumerConfig struct {
Produces string `default:"application/json" yaml:"rest_produces" json:"rest_produces,omitempty" property:"rest_produces"`
Consumes string `default:"application/json" yaml:"rest_consumes" json:"rest_consumes,omitempty" property:"rest_consumes"`
RestServiceConfigsMap map[string]*RestServiceConfig `yaml:"references" json:"references,omitempty" property:"references"`
+
Review Comment:
空行可以删掉
##########
protocol/rest/config/reader/rest_config_reader.go:
##########
@@ -61,7 +61,8 @@ func (cr *RestConfigReader) ReadConsumerConfig(reader *bytes.Buffer) error {
restConsumerServiceConfigMap := make(map[string]*config.RestServiceConfig, len(restConsumerConfig.RestServiceConfigsMap))
for key, rc := range restConsumerConfig.RestServiceConfigsMap {
rc.Client = getNotEmptyStr(rc.Client, restConsumerConfig.Client, constant.DefaultRestClient)
- rc.RestMethodConfigsMap = initMethodConfigMap(rc, restConsumerConfig.Consumes, restConsumerConfig.Produces)
+ //初始化每个方法的配置
Review Comment:
用英文注释吧
##########
common/proxy/proxy.go:
##########
@@ -140,6 +140,7 @@ func DefaultProxyImplementFunc(p *Proxy, v common.RPCService) {
}
} else { // only return error
replyEmptyFlag = true
+ //reply = reflect.New(outs[0])
Review Comment:
加一行注释的原因是？
##########
config/rest_config.go:
##########
@@ -92,6 +199,14 @@ func (c *RestServiceConfig) UnmarshalYAML(unmarshal func(interface{}) error) err
return nil
}
+type RestCommonConfig struct {
+ Path string `yaml:"path" json:"rest_path,omitempty" property:"rest_path"`
+ MethodType string `yaml:"method" json:"rest_method,omitempty" property:"rest_method"`
+ QueryParams string `yaml:"query_params" json:"rest_query_params,omitempty" property:"rest_query_params"`
+ PathParams string `yaml:"path_params" json:"rest_path_params,omitempty" property:"rest_query_params"`
+
Review Comment:
空行
##########
config/config_resolver.go:
##########
@@ -52,6 +52,7 @@ func GetConfigResolver(conf *loaderConf) *koanf.Koanf {
switch conf.suffix {
case "yaml", "yml":
+ //err = k.Load(rawbytes.Provider(bytes), yaml.Parser())
Review Comment:
同上
##########
config/rest_config.go:
##########
@@ -46,12 +59,107 @@ func (c *RestConsumerConfig) UnmarshalYAML(unmarshal func(interface{}) error) er
return nil
}
+// pi todo refactor
+func (c *RestConsumerConfig) Init(rc *RootConfig) error {
+ restConsumerConfig := rc.RestConsumer
+
+ if restConsumerConfig == nil {
+ logger.Debugf("[rest-protocol] no consumer config")
+ return nil
+ }
+
+ restConsumerServiceConfigMap := make(map[string]*RestServiceConfig, len(restConsumerConfig.RestServiceConfigsMap))
+ for key, rc := range restConsumerConfig.RestServiceConfigsMap {
+ rc.Client = getNotEmptyStr(rc.Client, restConsumerConfig.Client, constant.DefaultRestClient)
+ rc.RestMethodConfigs = initMethodConfigMap(rc, restConsumerConfig.Consumes, restConsumerConfig.Produces)
+ restConsumerServiceConfigMap[key] = rc
+ }
+
+ SetRestConsumerServiceConfigMap(restConsumerServiceConfigMap)
+
+ return nil
+}
+
+// initProviderRestConfig ...
+func initMethodConfigMap(rc *RestServiceConfig, consumes string, produces string) map[string]*RestMethodConfig {
+ mcm := make(map[string]*RestMethodConfig, len(rc.RestMethodConfigs))
+ for _, mc := range rc.RestMethodConfigs {
+ mc.InterfaceName = rc.InterfaceName
+ mc.Path = rc.Path + mc.Path
+ mc.Consumes = getNotEmptyStr(mc.Consumes, rc.Consumes, consumes)
+ mc.Produces = getNotEmptyStr(mc.Produces, rc.Produces, produces)
+ mc.MethodType = getNotEmptyStr(mc.MethodType, rc.MethodType)
+ mc = transformMethodConfig(mc)
+ mcm[mc.MethodName] = mc
+ }
+ return mcm
+}
+
+// transformMethodConfig
+func transformMethodConfig(methodConfig *RestMethodConfig) *RestMethodConfig {
+ if len(methodConfig.PathParamsMap) == 0 && len(methodConfig.PathParams) > 0 {
+ paramsMap, err := parseParamsString2Map(methodConfig.PathParams)
+ if err != nil {
+ logger.Warnf("[RestConfig] Path Param parse error:%v", err)
+ } else {
+ methodConfig.PathParamsMap = paramsMap
+ }
+ }
+ if len(methodConfig.QueryParamsMap) == 0 && len(methodConfig.QueryParams) > 0 {
+ paramsMap, err := parseParamsString2Map(methodConfig.QueryParams)
+ if err != nil {
+ logger.Warnf("[RestConfig] Query Param parse error:%v", err)
+ } else {
+ methodConfig.QueryParamsMap = paramsMap
+ }
+ }
+ if len(methodConfig.HeadersMap) == 0 && len(methodConfig.Headers) > 0 {
+ headersMap, err := parseParamsString2Map(methodConfig.Headers)
+ if err != nil {
+ logger.Warnf("[RestConfig] Header parse error:%v", err)
+ } else {
+ methodConfig.HeadersMap = headersMap
+ }
+ }
+ return methodConfig
+}
+
+// transform a string to a map
+// for example:
+// string "0:id,1:name" => map [0:id,1:name]
+func parseParamsString2Map(params string) (map[int]string, error) {
+ m := make(map[int]string, 8)
+ for _, p := range strings.Split(params, ",") {
+ pa := strings.Split(p, ":")
+ key, err := strconv.Atoi(pa[0])
+ if err != nil {
+ return nil, err
+ }
+ m[key] = pa[1]
+ }
+ return m, nil
+}
+
+// function will return first not empty string ..
+func getNotEmptyStr(args ...string) string {
Review Comment:
getNotEmptyStr 这个func 在[rest_config_reader.go](https://github.com/apache/dubbo-go/pull/1855/files#diff-d33851599c1bda8da989a7716e0992742b67bd7c4a36113f0f0edfb181753110)已经有了，看起来是通用的，放在能共用的地方吧
##########
protocol/rest/config/reader/rest_config_reader.go:
##########
@@ -114,16 +115,26 @@ func getNotEmptyStr(args ...string) string {
// transformMethodConfig
func transformMethodConfig(methodConfig *config.RestMethodConfig) *config.RestMethodConfig {
- if len(methodConfig.PathParamsMap) == 0 && len(methodConfig.PathParams) > 0 {
- paramsMap, err := parseParamsString2Map(methodConfig.PathParams)
+ if len(methodConfig.PathParamsMap) == 0 && len(methodConfig.RestCommonConfig.PathParams) > 0 {
Review Comment:
methodConfig.RestCommonConfig 我看在下面判断了是否等于nil， 在这里直接使用会出现nil情况么
##########
protocol/rest/server/rest_server.go:
##########
@@ -234,15 +237,18 @@ func assembleArgsFromBody(methodConfig *rest_config.RestMethodConfig, argsTypes
}
}
if err := req.ReadEntity(&ni); err != nil {
- return perrors.Errorf("[Go restful] Read body entity error, error is %v", perrors.WithStack(err))
+ // pi todo improve the fault tolerance so just logger warn
+ //return perrors.Errorf("[Go restful] Read body entity error, error is %v", perrors.WithStack(err))
Review Comment:
没用的注释和 todo 删掉吧
##########
protocol/rest/client/client_impl/resty_client.go:
##########
@@ -32,15 +32,12 @@ import (
)
import (
- "dubbo.apache.org/dubbo-go/v3/common/constant"
- "dubbo.apache.org/dubbo-go/v3/common/extension"
"dubbo.apache.org/dubbo-go/v3/protocol/rest/client"
)
-func init() {
- extension.SetRestClient(constant.DefaultRestClient, NewRestyClient)
-}
-
+//func init() {
Review Comment:
注释的如果没用就直接删掉吧
##########
protocol/rest/rest_protocol.go:
##########
@@ -27,19 +27,33 @@ import (
"dubbo.apache.org/dubbo-go/v3/common/constant"
"dubbo.apache.org/dubbo-go/v3/common/extension"
"dubbo.apache.org/dubbo-go/v3/common/logger"
+ "dubbo.apache.org/dubbo-go/v3/config"
"dubbo.apache.org/dubbo-go/v3/protocol"
"dubbo.apache.org/dubbo-go/v3/protocol/rest/client"
- _ "dubbo.apache.org/dubbo-go/v3/protocol/rest/client/client_impl"
- rest_config "dubbo.apache.org/dubbo-go/v3/protocol/rest/config"
- _ "dubbo.apache.org/dubbo-go/v3/protocol/rest/config/reader"
+ "dubbo.apache.org/dubbo-go/v3/protocol/rest/client/client_impl"
"dubbo.apache.org/dubbo-go/v3/protocol/rest/server"
- _ "dubbo.apache.org/dubbo-go/v3/protocol/rest/server/server_impl"
)
var restProtocol *RestProtocol
const REST = "rest"
+func init() {
+ SetRestServer(constant.DefaultRestServer, server.NewGoRestfulServer)
+}
+
+func init() {
+ extension.SetRestClient(constant.DefaultRestClient, client_impl.NewRestyClient)
Review Comment:
多个init func合并到一个里面吧
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org