Repository: go-kratos/kratos
Pull request #2100 (2022-06-14 04:01:45)

fix(config): apollo get the original configuration file directly
<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need to mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a semantic commits format title, such as `<type>[optional scope]: <description>`, see: https://go-kratos.dev/docs/community/contribution#type
5. at the same time, please note that similar work should be submitted in one PR as far as possible to reduce the workload of reviewers. Do not split a work into multiple PR unless it should.
-->

<!--
🎉 感谢您向 Kratos 发送 PR 请求！以下是一些提示：
如果这是你第一次为 Kratos 贡献，请阅读我们的贡献指南：https://go-kratos.dev/en/docs/community/contribution/
2、确保您已经为您的 PR 添加或运行了适当的测试和lint，请在提交PR之前使用“make lint”和“make test”，使用“make clean”整理您的 go.mod。
3、如果请购单未完成，您可能需要将其标记为 WIP（在制品）PR 或 draft PR
4、请使用语义提交格式标题，如“<类型>[可选范围]：<说明>`，请参阅：https://go-kratos.dev/docs/community/contribution#type
5. 同时请注意，同类的工作请尽量在一个PR中提交，以减轻 review 者的工作负担，不要把一项工作拆分成很多个PR，除非它应该这样做。
-->

<!--
-->

#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->


#### Which issue(s) this PR fixes (resolves / be part of):
<!--
* Automatically closes linked issue when PR is merged.
* If your PR is not fully resolved the issue, please use `part of #<issue number>` instead.

Usage: `fixes/resolves #<issue number>`, or `fixes/resolves (paste link of issue)`.
-->


#### Other special notes for the reviewers:
<!--
* Somethings that need extra attention for the reviewers
* Some additional notes, TODO list, etc.
-->


codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/2100?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#2100](https://codecov.io/gh/go-kratos/kratos/pull/2100?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (d3891fe) 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.05%`.
> The diff coverage is `n/a`.

```diff
@@            Coverage Diff             @@
##             main    #2100      +/-   ##
==========================================
- Coverage   79.09%   79.03%   -0.06%     
==========================================
  Files          84       84              
  Lines        3683     3683              
==========================================
- Hits         2913     2911       -2     
- Misses        559      561       +2     
  Partials      211      211              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/2100?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [internal/context/context.go](https://codecov.io/gh/go-kratos/kratos/pull/2100/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/2100?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/2100?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [123fc1e...d3891fe](https://codecov.io/gh/go-kratos/kratos/pull/2100?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).


Patch:
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/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")
-	}
-}