Refactor render package

Makefile

@@ -28,7 +28,7 @@ $(NAME):
 	$(GO) build $(MODULE)
 
 test:
-	$(GO) test ./...
+	$(GO) test -race ./...
 
 gox-build:
 	rm -rf out

config/config.go

@@ -349,13 +349,6 @@ func ReadConfig(filename string) (*Config, error) {
 		}
 	}
 
-	if cfg.ClickHouse.DataTableLegacy != "" {
-		cfg.DataTable = append(cfg.DataTable, DataTable{
-			Table:      cfg.ClickHouse.DataTableLegacy,
-			RollupConf: cfg.ClickHouse.RollupConfLegacy,
-		})
-	}
-
 	err = IndexUseReversesValidate(cfg.ClickHouse.IndexUseReverses)
 	if err != nil {
 		return nil, err
@@ -373,53 +366,9 @@ func ReadConfig(filename string) (*Config, error) {
 		}
 	}
 
-	for i := 0; i < len(cfg.DataTable); i++ {
-		if cfg.DataTable[i].TargetMatchAny != "" {
-			r, err := regexp.Compile(cfg.DataTable[i].TargetMatchAny)
-			if err != nil {
-				return nil, err
-			}
-			cfg.DataTable[i].TargetMatchAnyRegexp = r
-		}
-
-		if cfg.DataTable[i].TargetMatchAll != "" {
-			r, err := regexp.Compile(cfg.DataTable[i].TargetMatchAll)
-			if err != nil {
-				return nil, err
-			}
-			cfg.DataTable[i].TargetMatchAllRegexp = r
-		}
-
-		rdp := cfg.DataTable[i].RollupDefaultPrecision
-		rdf := cfg.DataTable[i].RollupDefaultFunction
-		if cfg.DataTable[i].RollupConf == "auto" || cfg.DataTable[i].RollupConf == "" {
-			table := cfg.DataTable[i].Table
-			if cfg.DataTable[i].RollupAutoTable != "" {
-				table = cfg.DataTable[i].RollupAutoTable
-			}
-
-			cfg.DataTable[i].Rollup, err = rollup.NewAuto(cfg.ClickHouse.Url, table, time.Minute, rdp, rdf)
-		} else if cfg.DataTable[i].RollupConf == "none" {
-			cfg.DataTable[i].Rollup, err = rollup.NewDefault(rdp, rdf)
-		} else {
-			cfg.DataTable[i].Rollup, err = rollup.NewXMLFile(cfg.DataTable[i].RollupConf, rdp, rdf)
-		}
-
-		if err != nil {
-			return nil, err
-		}
-
-		if len(cfg.DataTable[i].Context) == 0 {
-			cfg.DataTable[i].ContextMap = knownDataTableContext
-		} else {
-			cfg.DataTable[i].ContextMap = make(map[string]bool)
-			for _, c := range cfg.DataTable[i].Context {
-				if !knownDataTableContext[c] {
-					return nil, fmt.Errorf("unknown context %#v", c)
-				}
-				cfg.DataTable[i].ContextMap[c] = true
-			}
-		}
+	err = cfg.ProcessDataTables()
+	if err != nil {
+		return nil, err
 	}
 
 	// compute prometheus external url
@@ -443,3 +392,64 @@ func ReadConfig(filename string) (*Config, error) {
 
 	return cfg, nil
 }
+
+// ProcessDataTables checks if legacy `data`-table config is used, compiles regexps for `target-match-any` and `target-match-all`
+// parameters, sets the rollup configuration and proper context.
+func (c *Config) ProcessDataTables() (err error) {
+	if c.ClickHouse.DataTableLegacy != "" {
+		c.DataTable = append(c.DataTable, DataTable{
+			Table:      c.ClickHouse.DataTableLegacy,
+			RollupConf: c.ClickHouse.RollupConfLegacy,
+		})
+	}
+
+	for i := 0; i < len(c.DataTable); i++ {
+		if c.DataTable[i].TargetMatchAny != "" {
+			r, err := regexp.Compile(c.DataTable[i].TargetMatchAny)
+			if err != nil {
+				return err
+			}
+			c.DataTable[i].TargetMatchAnyRegexp = r
+		}
+
+		if c.DataTable[i].TargetMatchAll != "" {
+			r, err := regexp.Compile(c.DataTable[i].TargetMatchAll)
+			if err != nil {
+				return err
+			}
+			c.DataTable[i].TargetMatchAllRegexp = r
+		}
+
+		rdp := c.DataTable[i].RollupDefaultPrecision
+		rdf := c.DataTable[i].RollupDefaultFunction
+		if c.DataTable[i].RollupConf == "auto" || c.DataTable[i].RollupConf == "" {
+			table := c.DataTable[i].Table
+			if c.DataTable[i].RollupAutoTable != "" {
+				table = c.DataTable[i].RollupAutoTable
+			}
+
+			c.DataTable[i].Rollup, err = rollup.NewAuto(c.ClickHouse.Url, table, time.Minute, rdp, rdf)
+		} else if c.DataTable[i].RollupConf == "none" {
+			c.DataTable[i].Rollup, err = rollup.NewDefault(rdp, rdf)
+		} else {
+			c.DataTable[i].Rollup, err = rollup.NewXMLFile(c.DataTable[i].RollupConf, rdp, rdf)
+		}
+
+		if err != nil {
+			return err
+		}
+
+		if len(c.DataTable[i].Context) == 0 {
+			c.DataTable[i].ContextMap = knownDataTableContext
+		} else {
+			c.DataTable[i].ContextMap = make(map[string]bool)
+			for _, ctx := range c.DataTable[i].Context {
+				if !knownDataTableContext[ctx] {
+					return fmt.Errorf("unknown context %#v", ctx)
+				}
+				c.DataTable[i].ContextMap[ctx] = true
+			}
+		}
+	}
+	return nil
+}

config/config_test.go

@@ -0,0 +1,215 @@
+package config
+
+import (
+	"fmt"
+	"io/fs"
+	"regexp"
+	"regexp/syntax"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestProcessDataTables(t *testing.T) {
+	type in struct {
+		table       DataTable
+		tableLegacy string
+	}
+	type out struct {
+		tables []DataTable
+		err    error
+	}
+	type ctx map[string]bool
+
+	regexpCompileWrapper := func(re string) *regexp.Regexp {
+		r, _ := regexp.Compile(re)
+		return r
+	}
+
+	tests := []struct {
+		name string
+		in   in
+		out  out
+	}{
+		{
+			name: "legacy table only",
+			in: in{
+				tableLegacy: "graphite.data",
+			},
+			out: out{
+				[]DataTable{
+					{
+						Table:      "graphite.data",
+						RollupConf: "auto",
+						ContextMap: ctx{"graphite": true, "prometheus": true},
+					},
+				},
+				nil,
+			},
+		},
+		{
+			name: "legacy and normal tables",
+			in: in{
+				table:       DataTable{Table: "graphite.new_data"},
+				tableLegacy: "graphite.data",
+			},
+			out: out{
+				[]DataTable{
+					{
+						Table:      "graphite.new_data",
+						ContextMap: ctx{"graphite": true, "prometheus": true},
+					},
+					{
+						Table:      "graphite.data",
+						RollupConf: "auto",
+						ContextMap: ctx{"graphite": true, "prometheus": true},
+					},
+				},
+				nil,
+			},
+		},
+		{
+			name: "fail to compile TargetMatchAll",
+			in: in{
+				table: DataTable{Table: "graphite.data", TargetMatchAll: "[2223"},
+			},
+			out: out{
+				[]DataTable{{Table: "graphite.data", TargetMatchAll: "[2223"}},
+				&syntax.Error{Code: syntax.ErrMissingBracket, Expr: "[2223"},
+			},
+		},
+		{
+			name: "fail to compile TargetMatchAny",
+			in: in{
+				table: DataTable{Table: "graphite.data", TargetMatchAny: "[2223"},
+			},
+			out: out{
+				[]DataTable{{Table: "graphite.data", TargetMatchAny: "[2223"}},
+				&syntax.Error{Code: syntax.ErrMissingBracket, Expr: "[2223"},
+			},
+		},
+		{
+			name: "fail to compile TargetMatchAny",
+			in: in{
+				table: DataTable{Table: "graphite.data", TargetMatchAny: "[2223"},
+			},
+			out: out{
+				[]DataTable{{Table: "graphite.data", TargetMatchAny: "[2223"}},
+				&syntax.Error{Code: syntax.ErrMissingBracket, Expr: "[2223"},
+			},
+		},
+		{
+			name: "fail to read xml rollup",
+			in: in{
+				table: DataTable{Table: "graphite.data", RollupConf: "/some/file/that/does/not/hopefully/exists/on/the/disk"},
+			},
+			out: out{
+				[]DataTable{{Table: "graphite.data", RollupConf: "/some/file/that/does/not/hopefully/exists/on/the/disk"}},
+				&fs.PathError{Op: "open", Path: "/some/file/that/does/not/hopefully/exists/on/the/disk", Err: syscall.ENOENT},
+			},
+		},
+		{
+			name: "unknown context",
+			in: in{
+				table: DataTable{Table: "graphite.data", Context: []string{"unexpected"}},
+			},
+			out: out{
+				[]DataTable{
+					{
+						Table:      "graphite.data",
+						Context:    []string{"unexpected"},
+						ContextMap: ctx{},
+					},
+				},
+				fmt.Errorf("unknown context \"unexpected\""),
+			},
+		},
+		{
+			name: "check all works",
+			in: in{
+				table: DataTable{
+					Table:                  "graphite.data",
+					Reverse:                true,
+					TargetMatchAll:         "^.*[asdf][.].*",
+					TargetMatchAny:         "^.*{a|s|d|f}[.].*",
+					RollupConf:             "none",
+					RollupDefaultFunction:  "any",
+					RollupDefaultPrecision: 61,
+					RollupUseReverted:      true,
+					Context:                []string{"prometheus"},
+				},
+				tableLegacy: "table",
+			},
+			out: out{
+				[]DataTable{
+					{
+						Table:                  "graphite.data",
+						Reverse:                true,
+						TargetMatchAll:         "^.*[asdf][.].*",
+						TargetMatchAny:         "^.*{a|s|d|f}[.].*",
+						TargetMatchAllRegexp:   regexpCompileWrapper("^.*[asdf][.].*"),
+						TargetMatchAnyRegexp:   regexpCompileWrapper("^.*{a|s|d|f}[.].*"),
+						RollupConf:             "none",
+						RollupDefaultFunction:  "any",
+						RollupDefaultPrecision: 61,
+						RollupUseReverted:      true,
+						Context:                []string{"prometheus"},
+						ContextMap:             ctx{"prometheus": true},
+					},
+					{
+						Table:      "table",
+						RollupConf: "auto",
+						ContextMap: ctx{"graphite": true, "prometheus": true},
+					},
+				},
+				nil,
+			},
+		},
+		{
+			name: "unknown context",
+			in: in{
+				table: DataTable{Table: "graphite.data", Context: []string{"unexpected"}},
+			},
+			out: out{
+				[]DataTable{
+					{
+						Table:      "graphite.data",
+						Context:    []string{"unexpected"},
+						ContextMap: ctx{},
+					},
+				},
+				fmt.Errorf("unknown context \"unexpected\""),
+			},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			cfg := New()
+			if test.in.table.Table != "" {
+				cfg.DataTable = []DataTable{test.in.table}
+			}
+			if test.in.tableLegacy != "" {
+				cfg.ClickHouse.DataTableLegacy = test.in.tableLegacy
+			}
+			err := cfg.ProcessDataTables()
+			if err != nil {
+				assert.Equal(t, test.out.err, err)
+				return
+			}
+			assert.Equal(t, len(test.out.tables), len(cfg.DataTable))
+			// it's difficult to check rollup.Rollup because Rules.updated field
+			// We explicitly don't check it here
+			for i := range cfg.DataTable {
+				test.out.tables[i].Rollup = nil
+				cfg.DataTable[i].Rollup = nil
+			}
+			assert.Equal(t, test.out.tables, cfg.DataTable)
+		})
+	}
+}
+
+func TestKnownDataTableContext(t *testing.T) {
+	assert.Equal(t, map[string]bool{ContextGraphite: true, ContextPrometheus: true}, knownDataTableContext)
+}

go.mod

@@ -15,7 +15,7 @@ require (
 	github.com/pkg/errors v0.8.1
 	github.com/prometheus/common v0.4.1
 	github.com/prometheus/prometheus v1.8.2-0.20190814100549-343d8d75fd76
-	github.com/stretchr/testify v1.3.0
+	github.com/stretchr/testify v1.7.0
 	go.uber.org/atomic v1.4.0 // indirect; prometheus?
 	go.uber.org/multierr v1.1.0 // indirect; prometheus?
 	go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee // indirect

go.sum

@@ -400,10 +400,13 @@ github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasO
 github.com/spf13/pflag v1.0.3 h1:zPAT6CGy6wXeQ7NtTnaTerfKOsV6V6F8agHXFiazDkg=
 github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnInEg4=
 github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.1.1 h1:2vfRuCMp5sSVIDSqO8oNnWJq7mPa6KVP3iPIwFBuy8A=
 github.com/stretchr/objx v0.1.1/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
+github.com/stretchr/testify v1.7.0 h1:nwc3DEeHmmLAfoZucVR881uASk0Mfjw8xYJ99tb5CcY=
+github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
 github.com/xlab/treeprint v0.0.0-20180616005107-d6fb6747feb6/go.mod h1:ce1O1j6UtZfjr22oyGxGLbauSBp2YVXpARAosm7dHBg=
 go.opencensus.io v0.20.1/go.mod h1:6WKK9ahsWS3RSO+PY9ZHZUfv2irvY6gN279GOPZjmmk=
 go.opencensus.io v0.20.2 h1:NAfh7zF0/3/HqtMvJNZ/RFrSlCE6ZTlHmKfhL/Dm1Jk=
@@ -517,6 +520,8 @@ gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bl
 gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c h1:dUUwHk2QECo/6vqA44rthZ8ie2QXMNeKRTHCNY2nXvo=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20180728063816-88497007e858/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 k8s.io/api v0.0.0-20190409021203-6e4e0e4f393b h1:aBGgKJUM9Hk/3AE8WaZIApnTxG35kbuQba2w+SXqezo=