Repository: go-kratos/kratos
Pull request #2094 (2022-06-10 15:36:24)

fix: kratos proto server cmd method name bugfix (#2089)
- add ucFirst function and unit test at cmd/kratos/internal/proto/server/server.go
- fix the [method name style bug ](https://github.com/go-kratos/kratos/issues/2089)

Casper-Mars: This would not solve the issue #2089 . Issue #2089 is about camel case and snake case.
raw34: > This would not solve the issue #2089 . Issue #2089 is about camel case and snake case.

this commit just fix the part2 problem, like this:


```
# proto
service OrderItems {
	rpc CreateOrderItems (CreateOrderItemsRequest) returns (CreateOrderItemsReply);
......
}
```

```
# wrong service
......

func (s *OrderItemsService) Createorderitems(ctx context.Context, req *pb.CreateOrderItemsRequest) (*pb.CreateOrderItemsReply, error) {
	return &pb.CreateOrderItemsReply{}, nil
}

.....
```

```
# right service

......

func (s *OrderItemsService) CreateOrderItems(ctx context.Context, req *pb.CreateOrderItemsRequest) (*pb.CreateOrderItemsReply, error) {
	return &pb.CreateOrderItemsReply{}, nil
}

.....
```


Casper-Mars: OH~Do you mean this PR is still working in progress?
raw34: 
> OH~Do you mean this PR is still working in progress?

the fact is i didn't find out the issue is about 2 problems, hah...

i can try to resolve the part1 problem some time later
codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2094](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (66bdcb4) into [main](https://codecov.io/gh/go-kratos/kratos/commit/123fc1e6c81a92d0cb9512904382b4e2c3993653?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (123fc1e) will **decrease** coverage by `0.13%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main    #2094      +/-   ##
==========================================
- Coverage   79.09%   78.95%   -0.14%     
==========================================
  Files          84       84              
  Lines        3683     3683              
==========================================
- Hits         2913     2908       -5     
- Misses        559      563       +4     
- Partials      211      212       +1     
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [selector/node/ewma/node.go](https://codecov.io/gh/go-kratos/kratos/pull/2094/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-c2VsZWN0b3Ivbm9kZS9ld21hL25vZGUuZ28=) | `83.51% <0.00%> (-3.30%)` | :arrow_down: |
| [internal/context/context.go](https://codecov.io/gh/go-kratos/kratos/pull/2094/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-aW50ZXJuYWwvY29udGV4dC9jb250ZXh0Lmdv) | `97.01% <0.00%> (-2.99%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [123fc1e...66bdcb4](https://codecov.io/gh/go-kratos/kratos/pull/2094?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).

shenqidebaozi: @raw34 https://github.com/songzhibin97/kratos/blob/4e100fcdeb4dd046f1430ecde1ca96fafb9584ff/cmd/kratos/internal/proto/server/server.go#L70
shenqidebaozi: It seems that the problem is caused by this modification
raw34: > It seems that the problem is caused by this modification

yes, it is. the modification fix another bug, but trigger this bug. and my commit can fix it.
shenqidebaozi: Has this PR been completed?

Patch:
diff --git a/cmd/kratos/internal/proto/server/server.go b/cmd/kratos/internal/proto/server/server.go
index 0ac7355f..dcc19790 100644
--- a/cmd/kratos/internal/proto/server/server.go
+++ b/cmd/kratos/internal/proto/server/server.go
@@ -7,15 +7,10 @@ import (
 	"path"
 	"strings"
 
-	"golang.org/x/text/cases"
-	"golang.org/x/text/language"
-
 	"github.com/emicklei/proto"
 	"github.com/spf13/cobra"
 )
 
-var caser = cases.Title(language.English)
-
 // CmdServer the service command.
 var CmdServer = &cobra.Command{
 	Use:   "server",
@@ -67,7 +62,7 @@ func run(cmd *cobra.Command, args []string) {
 					continue
 				}
 				cs.Methods = append(cs.Methods, &Method{
-					Service: s.Name, Name: caser.String(r.Name), Request: r.RequestType,
+					Service: s.Name, Name: ucFirst(r.Name), Request: r.RequestType,
 					Reply: r.ReturnsType, Type: getMethodType(r.StreamsRequest, r.StreamsReturns),
 				})
 			}
@@ -107,3 +102,11 @@ func getMethodType(streamsRequest, streamsReturns bool) MethodType {
 	}
 	return unaryType
 }
+
+func ucFirst(str string) string {
+	if str == "" {
+		return ""
+	}
+
+	return strings.ToUpper(str[:1]) + str[1:]
+}
diff --git a/contrib/config/apollo/apollo.go b/contrib/config/apollo/apollo.go
index 0d3b975e..a0a38b02 100644
--- a/contrib/config/apollo/apollo.go
+++ b/contrib/config/apollo/apollo.go
@@ -4,7 +4,6 @@ import (
 	"strings"
 
 	"github.com/go-kratos/kratos/v2/config"
-	"github.com/go-kratos/kratos/v2/encoding"
 	"github.com/go-kratos/kratos/v2/log"
 
 	"github.com/apolloconfig/agollo/v4"
@@ -104,61 +103,9 @@ func NewSource(opts ...Option) config.Source {
 	if err != nil {
 		panic(err)
 	}
-
 	return &apollo{client: client, opt: &op}
 }
 
-// genKey got the key of config.KeyValue pair.
-// eg: namespace.ext with subKey got namespace.subKey
-func genKey(ns, sub string) string {
-	arr := strings.Split(ns, ".")
-	if len(arr) < 1 {
-		return sub
-	}
-
-	if len(arr) == 1 {
-		if ns == "" {
-			return sub
-		}
-		return ns + "." + sub
-	}
-
-	return strings.Join(arr[:len(arr)-1], ".") + "." + sub
-}
-
-// resolve convert kv pair into one map[string]interface{} by split key into different
-// map level. such as: app.name = "application" => map[app][name] = "application"
-func resolve(key string, value interface{}, target map[string]interface{}) {
-	// expand key "aaa.bbb" into map[aaa]map[bbb]interface{}
-	keys := strings.Split(key, ".")
-	last := len(keys) - 1
-	cursor := target
-
-	for i, k := range keys {
-		if i == last {
-			cursor[k] = value
-			break
-		}
-
-		// not the last key, be deeper
-		v, ok := cursor[k]
-		if !ok {
-			// create a new map
-			deeper := make(map[string]interface{})
-			cursor[k] = deeper
-			cursor = deeper
-			continue
-		}
-
-		// current exists, then check existing value type, if it's not map
-		// that means duplicate keys, and at least one is not map instance.
-		if cursor, ok = v.(map[string]interface{}); !ok {
-			log.Warnf("duplicate key: %v\n", strings.Join(keys[:i+1], "."))
-			break
-		}
-	}
-}
-
 func format(ns string) string {
 	arr := strings.Split(ns, ".")
 	if len(arr) <= 1 {
@@ -173,26 +120,15 @@ func (e *apollo) load() []*config.KeyValue {
 	namespaces := strings.Split(e.opt.namespace, ",")
 
 	for _, ns := range namespaces {
-		next := map[string]interface{}{}
-		e.client.GetConfigCache(ns).Range(func(key, value interface{}) bool {
-			// all values are out properties format
-			resolve(genKey(ns, key.(string)), value, next)
-			return true
-		})
-
-		// serialize the namespace content KeyValue into bytes.
-		f := format(ns)
-		codec := encoding.GetCodec(f)
-		val, err := codec.Marshal(next)
+		value, err := e.client.GetConfigCache(ns).Get("content")
 		if err != nil {
-			log.Warnf("apollo could not handle namespace %s: %v", ns, err)
-			continue
+			log.Warnw("apollo get config failed", "err", err)
 		}
-
+		// serialize the namespace content KeyValue into bytes.
 		kv = append(kv, &config.KeyValue{
 			Key:    ns,
-			Value:  val,
-			Format: f,
+			Value:  []byte(value.(string)),
+			Format: format(ns),
 		})
 	}
 
diff --git a/contrib/config/apollo/parser.go b/contrib/config/apollo/parser.go
index e5a3cb8e..a449256f 100644
--- a/contrib/config/apollo/parser.go
+++ b/contrib/config/apollo/parser.go
@@ -1,10 +1,6 @@
 package apollo
 
 import (
-	"encoding/json"
-
-	"gopkg.in/yaml.v3"
-
 	"github.com/apolloconfig/agollo/v4/constant"
 	"github.com/apolloconfig/agollo/v4/extension"
 )
@@ -12,25 +8,13 @@ import (
 type jsonExtParser struct{}
 
 func (parser jsonExtParser) Parse(configContent interface{}) (map[string]interface{}, error) {
-	v, ok := configContent.(string)
-	if !ok {
-		return nil, nil
-	}
-	out := make(map[string]interface{}, 4)
-	err := json.Unmarshal([]byte(v), &out)
-	return out, err
+	return map[string]interface{}{"content": configContent}, nil
 }
 
 type yamlExtParser struct{}
 
 func (parser yamlExtParser) Parse(configContent interface{}) (out map[string]interface{}, err error) {
-	v, ok := configContent.(string)
-	if !ok {
-		return nil, nil
-	}
-	out = make(map[string]interface{}, 4)
-	err = yaml.Unmarshal([]byte(v), &out)
-	return
+	return map[string]interface{}{"content": configContent}, nil
 }
 
 func init() {
diff --git a/contrib/config/apollo/watcher.go b/contrib/config/apollo/watcher.go
index cd6a4178..04e02fbc 100644
--- a/contrib/config/apollo/watcher.go
+++ b/contrib/config/apollo/watcher.go
@@ -4,7 +4,6 @@ import (
 	"context"
 
 	"github.com/go-kratos/kratos/v2/config"
-	"github.com/go-kratos/kratos/v2/encoding"
 	"github.com/go-kratos/kratos/v2/log"
 
 	"github.com/apolloconfig/agollo/v4/storage"
@@ -16,28 +15,20 @@ type watcher struct {
 }
 
 type customChangeListener struct {
-	in chan<- []*config.KeyValue
+	in     chan<- []*config.KeyValue
+	apollo *apollo
 }
 
 func (c *customChangeListener) onChange(namespace string, changes map[string]*storage.ConfigChange) []*config.KeyValue {
 	kv := make([]*config.KeyValue, 0, 2)
-	next := make(map[string]interface{})
-
-	for key, change := range changes {
-		resolve(genKey(namespace, key), change.NewValue, next)
-	}
-
-	f := format(namespace)
-	codec := encoding.GetCodec(f)
-	val, err := codec.Marshal(next)
+	value, err := c.apollo.client.GetConfigCache(namespace).Get("content")
 	if err != nil {
-		log.Warnf("apollo could not handle namespace %s: %v", namespace, err)
-		return nil
+		log.Warnw("apollo get config failed", "err", err)
 	}
 	kv = append(kv, &config.KeyValue{
 		Key:    namespace,
-		Value:  val,
-		Format: f,
+		Value:  []byte(value.(string)),
+		Format: format(namespace),
 	})
 
 	return kv
@@ -56,7 +47,7 @@ func (c *customChangeListener) OnNewestChange(changeEvent *storage.FullChangeEve
 
 func newWatcher(a *apollo) (config.Watcher, error) {
 	changeCh := make(chan []*config.KeyValue)
-	listener := &customChangeListener{in: changeCh}
+	listener := &customChangeListener{in: changeCh, apollo: a}
 	a.client.AddChangeListener(listener)
 	return &watcher{
 		out: changeCh,


Test patch:
diff --git a/cmd/kratos/internal/proto/server/server_test.go b/cmd/kratos/internal/proto/server/server_test.go
new file mode 100644
index 00000000..a2cbeee6
--- /dev/null
+++ b/cmd/kratos/internal/proto/server/server_test.go
@@ -0,0 +1,52 @@
+package server
+
+import "testing"
+
+func Test_ucFirst(t *testing.T) {
+	type args struct {
+		str string
+	}
+	tests := []struct {
+		name string
+		args args
+		want string
+	}{
+		{
+			name: "ucFirst on lowercase words",
+			args: args{str: "helloworld"},
+			want: "Helloworld",
+		},
+		{
+			name: "ucFirst on uppercase words",
+			args: args{str: "HELLOWORLD"},
+			want: "HELLOWORLD",
+		},
+		{
+			name: "ucFirst on lowercase words with spaces",
+			args: args{str: "hello world"},
+			want: "Hello world",
+		},
+		{
+			name: "ucFirst on uppercase words with spaces",
+			args: args{str: "HELLO WORLD"},
+			want: "HELLO WORLD",
+		},
+		{
+			name: "ucFirst on Lower Camel Case words",
+			args: args{str: "helloWorld"},
+			want: "HelloWorld",
+		},
+		{
+			name: "ucFirst on Upper Camel Case words",
+			args: args{str: "HelloWorld"},
+			want: "HelloWorld",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := ucFirst(tt.args.str); got != tt.want {
+				t.Errorf("ucFirst() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}
diff --git a/contrib/config/apollo/apollo_test.go b/contrib/config/apollo/apollo_test.go
deleted file mode 100644
index 73e6f639..00000000
--- a/contrib/config/apollo/apollo_test.go
+++ /dev/null
@@ -1,189 +0,0 @@
-package apollo
-
-import (
-	"reflect"
-	"testing"
-)
-
-func Test_genKey(t *testing.T) {
-	type args struct {
-		ns  string
-		sub string
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			name: "case 1",
-			args: args{
-				ns:  "",
-				sub: "has_no_ns",
-			},
-			want: "has_no_ns",
-		},
-		{
-			name: "case 2",
-			args: args{
-				ns:  "ns.ext",
-				sub: "sub",
-			},
-			want: "ns.sub",
-		},
-		{
-			name: "case 3",
-			args: args{
-				ns:  "",
-				sub: "",
-			},
-			want: "",
-		},
-		{
-			name: "case 4",
-			args: args{
-				ns:  "ns.ext",
-				sub: "sub.sub2.sub3",
-			},
-			want: "ns.sub.sub2.sub3",
-		},
-		{
-			name: "case 5",
-			args: args{
-				ns:  "ns.more.ext",
-				sub: "sub.sub2.sub3",
-			},
-			want: "ns.more.sub.sub2.sub3",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := genKey(tt.args.ns, tt.args.sub); got != tt.want {
-				t.Errorf("genKey() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_format(t *testing.T) {
-	type args struct {
-		ns string
-	}
-	tests := []struct {
-		name string
-		args args
-		want string
-	}{
-		{
-			name: "case 0",
-			args: args{
-				ns: "ns.yaml",
-			},
-			want: "yaml",
-		},
-		{
-			name: "case 1",
-			args: args{
-				ns: "ns",
-			},
-			want: "json",
-		},
-		{
-			name: "case 2",
-			args: args{
-				ns: "ns.more.json",
-			},
-			want: "json",
-		},
-		{
-			name: "case 3",
-			args: args{
-				ns: "",
-			},
-			want: "json",
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := format(tt.args.ns); got != tt.want {
-				t.Errorf("format() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-func Test_convertProperties(t *testing.T) {
-	type args struct {
-		key    string
-		value  interface{}
-		target map[string]interface{}
-	}
-	tests := []struct {
-		name string
-		args args
-		want map[string]interface{}
-	}{
-		{
-			name: "case 0",
-			args: args{
-				key:    "application.name",
-				value:  "app name",
-				target: map[string]interface{}{},
-			},
-			want: map[string]interface{}{
-				"application": map[string]interface{}{
-					"name": "app name",
-				},
-			},
-		},
-		{
-			name: "case 1",
-			args: args{
-				key:    "application",
-				value:  []string{"1", "2", "3"},
-				target: map[string]interface{}{},
-			},
-			want: map[string]interface{}{
-				"application": []string{"1", "2", "3"},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			resolve(tt.args.key, tt.args.value, tt.args.target)
-			if !reflect.DeepEqual(tt.args.target, tt.want) {
-				t.Errorf("convertProperties() = %v, want %v", tt.args.target, tt.want)
-			}
-		})
-	}
-}
-
-func Test_convertProperties_duplicate(t *testing.T) {
-	target := map[string]interface{}{}
-	resolve("application.name", "name", target)
-	_, ok := target["application"]
-	if !reflect.DeepEqual(ok, true) {
-		t.Errorf("ok = %v, want %v", ok, true)
-	}
-	_, ok = target["application"].(map[string]interface{})["name"]
-	if !reflect.DeepEqual(ok, true) {
-		t.Errorf("ok = %v, want %v", ok, true)
-	}
-	if !reflect.DeepEqual(target["application"].(map[string]interface{})["name"], "name") {
-		t.Errorf("target[\"application\"][\"name\"] = %v, want %v", target["application"].(map[string]interface{})["name"], "name")
-	}
-
-	// cause duplicate, the oldest value will be kept
-	resolve("application.name.first", "first name", target)
-	_, ok = target["application"]
-	if !reflect.DeepEqual(ok, true) {
-		t.Errorf("ok = %v, want %v", ok, true)
-	}
-	_, ok = target["application"].(map[string]interface{})["name"]
-	if !reflect.DeepEqual(ok, true) {
-		t.Errorf("ok = %v, want %v", ok, true)
-	}
-	if !reflect.DeepEqual(target["application"].(map[string]interface{})["name"], "name") {
-		t.Errorf("target[\"application\"][\"name\"] = %v, want %v", target["application"].(map[string]interface{})["name"], "name")
-	}
-}