From 561fb2f052494dce3ddb50689763381cf39cad28 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Toma=C5=BE=20Jerman?= <tomaz.jerman121@gmail.com>
Date: Wed, 2 Dec 2020 09:25:08 +0100
Subject: [PATCH] Fix envoy settings from multiple sources

---
 pkg/envoy/store/settings.go                | 77 ++++++++++------------
 tests/envoy/settings_test.go               | 55 ++++++++++++++++
 tests/envoy/testdata/settings/source1.yaml |  3 +
 tests/envoy/testdata/settings/source2.yaml |  3 +
 4 files changed, 96 insertions(+), 42 deletions(-)
 create mode 100644 tests/envoy/settings_test.go
 create mode 100644 tests/envoy/testdata/settings/source1.yaml
 create mode 100644 tests/envoy/testdata/settings/source2.yaml

diff --git a/pkg/envoy/store/settings.go b/pkg/envoy/store/settings.go
index 2f13ff173..675c3dab9 100644
--- a/pkg/envoy/store/settings.go
+++ b/pkg/envoy/store/settings.go
@@ -14,80 +14,73 @@ type (
 	settingsState struct {
 		cfg *EncoderConfig
 
-		res *resource.Settings
-		ss  types.SettingValueSet
+		res types.SettingValueSet
+		exs types.SettingValueSet
 	}
 )
 
+var (
+	// gSettingsState will aggregate all of the setting resources.
+	gSettingsState *settingsState = nil
+)
+
 func NewSettingsState(res *resource.Settings, cfg *EncoderConfig) resourceState {
 	return &settingsState{
 		cfg: cfg,
 
-		res: res,
+		res: res.Res,
 	}
 }
 
 func (n *settingsState) Prepare(ctx context.Context, s store.Storer, state *envoy.ResourceState) (err error) {
-	// Preload settings
-	n.ss, _, err = store.SearchSettings(ctx, s, types.SettingsFilter{})
-	if err == store.ErrNotFound {
-		n.ss = make(types.SettingValueSet, 0, len(n.res.Res))
-	} else if err != nil {
-		return err
-	}
+	// Init global state
+	if gSettingsState == nil {
+		gSettingsState = &settingsState{
+			cfg: n.cfg,
+		}
 
-	// Default values
-	for _, s := range n.res.Res {
-		if s.UpdatedAt.IsZero() {
-			s.UpdatedAt = time.Now()
+		// Preload settings
+		gSettingsState.exs, _, err = store.SearchSettings(ctx, s, types.SettingsFilter{})
+		if err == store.ErrNotFound {
+			gSettingsState.exs = make(types.SettingValueSet, 0, len(n.res))
+		} else if err != nil {
+			return err
 		}
 	}
 
-	// Nothing else to do.
-	// Settings can't conflict either.
+	// Default values
+	for _, s := range n.res {
+		if s.UpdatedAt.IsZero() {
+			s.UpdatedAt = time.Now()
+		}
+
+		gSettingsState.res = append(gSettingsState.res, s)
+	}
 
 	return nil
 }
 
 func (n *settingsState) Encode(ctx context.Context, s store.Storer, state *envoy.ResourceState) (err error) {
-	ss := make(types.SettingValueSet, 0, len(n.res.Res))
+	ss := make(types.SettingValueSet, 0, len(n.res))
 
-	for _, ns := range n.res.Res {
-		os := n.ss.First(ns.Name)
+	for _, ns := range n.res {
+		os := n.exs.First(ns.Name)
 		if os != nil {
 			// Update existing setting
 			switch n.cfg.OnExisting {
-			case Skip:
+			case Skip,
+				MergeLeft:
 				ss = append(ss, os)
 
-			case Replace:
+			case Replace,
+				MergeRight:
 				ss = append(ss, ns)
-
-			case MergeLeft:
-				ss = append(ss, mergeSettings(os, ns))
-
-			case MergeRight:
-				ss = append(ss, mergeSettings(ns, os))
 			}
 		} else {
 			// Create fresh setting
 			ss = append(ss, ns)
 		}
 	}
-	err = store.TruncateSettings(ctx, s)
-	if err != nil {
-		return err
-	}
-	return store.CreateSetting(ctx, s, ss...)
-}
 
-// mergeSettings merges b into a, prioritising a
-func mergeSettings(a, b *types.SettingValue) *types.SettingValue {
-	c := *a
-
-	if len(c.Value) <= 0 {
-		c.Value = b.Value
-	}
-
-	return &c
+	return store.UpsertSetting(ctx, s, ss...)
 }
diff --git a/tests/envoy/settings_test.go b/tests/envoy/settings_test.go
new file mode 100644
index 000000000..71dd24374
--- /dev/null
+++ b/tests/envoy/settings_test.go
@@ -0,0 +1,55 @@
+package envoy
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cortezaproject/corteza-server/store"
+	"github.com/cortezaproject/corteza-server/system/types"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSettings(t *testing.T) {
+	var (
+		ctx    = context.Background()
+		s, err = initStore(ctx)
+	)
+	if err != nil {
+		t.Fatalf("failed to init sqlite in-memory db: %v", err)
+	}
+
+	prepare := func(ctx context.Context, s store.Storer, t *testing.T, suite string) (*require.Assertions, error) {
+		req := require.New(t)
+
+		nn, err := dd(ctx, suite)
+		req.NoError(err)
+
+		return req, encode(ctx, s, nn)
+	}
+
+	// Prepare
+	s, err = initStore(ctx)
+	err = ce(
+		err,
+
+		s.TruncateSettings(ctx),
+	)
+	if err != nil {
+		t.Fatal(err.Error())
+	}
+
+	req, err := prepare(ctx, s, t, "settings")
+	req.NoError(err)
+
+	t.Run("settings", func(t *testing.T) {
+		ss, _, err := store.SearchSettings(ctx, s, types.SettingsFilter{})
+		req.NoError(err)
+		req.NotNil(ss)
+		req.Len(ss, 4)
+
+		req.Equal("s1.opt.1", ss[0].Name)
+		req.Equal("s1.opt.2", ss[1].Name)
+		req.Equal("s2.opt.1", ss[2].Name)
+		req.Equal("s2.opt.2", ss[3].Name)
+	})
+}
diff --git a/tests/envoy/testdata/settings/source1.yaml b/tests/envoy/testdata/settings/source1.yaml
new file mode 100644
index 000000000..8e36334f0
--- /dev/null
+++ b/tests/envoy/testdata/settings/source1.yaml
@@ -0,0 +1,3 @@
+settings:
+  s1.opt.1: s1 opt1
+  s1.opt.2: s1 opt2
diff --git a/tests/envoy/testdata/settings/source2.yaml b/tests/envoy/testdata/settings/source2.yaml
new file mode 100644
index 000000000..fc9da6821
--- /dev/null
+++ b/tests/envoy/testdata/settings/source2.yaml
@@ -0,0 +1,3 @@
+settings:
+  s2.opt.1: s2 opt1
+  s2.opt.2: s2 opt2