From 076cd772b74c0421138336464974bce1bbd810a4 Mon Sep 17 00:00:00 2001 From: Shu Kutsuzawa Date: Sat, 27 Mar 2021 00:12:26 +0900 Subject: [PATCH] fix: Refactor config (#68) --- cmd/config/create.go | 7 +- config/config.go | 75 +++--- config/config_test.go | 562 +++++++++++++++++------------------------- go.mod | 1 + go.sum | 2 + 5 files changed, 270 insertions(+), 377 deletions(-) diff --git a/cmd/config/create.go b/cmd/config/create.go index bd2e630..e7ac11e 100644 --- a/cmd/config/create.go +++ b/cmd/config/create.go @@ -26,17 +26,18 @@ The new config has only launcher-version and launcher-image.`, return err } - config, err := configNew(path) + c, err := configNew(path) if err != nil { return err } - err = config.AddEntry(name) + defaultEntry := config.DefaultEntry() + err = c.AddEntry(name, defaultEntry) if err != nil { return err } - err = config.Save() + err = c.Save() if err != nil { return err } diff --git a/config/config.go b/config/config.go index 8e9bbae..a71fc5f 100644 --- a/config/config.go +++ b/config/config.go @@ -6,21 +6,22 @@ import ( "path/filepath" "github.com/go-yaml/yaml" + "github.com/mitchellh/mapstructure" ) // Launcher is launcher entity struct type Launcher struct { - Version string `yaml:"version"` - Image string `yaml:"image"` + Version string `yaml:"version" mapstructure:"launcher-version"` + Image string `yaml:"image" mapstructure:"launcher-image"` } // Entry is entity struct of sd-local config type Entry struct { - APIURL string `yaml:"api-url"` - StoreURL string `yaml:"store-url"` - Token string `yaml:"token"` - UUID string `yaml:"UUID"` - Launcher Launcher `yaml:"launcher"` + APIURL string `yaml:"api-url" mapstructure:"api-url"` + StoreURL string `yaml:"store-url" mapstructure:"store-url"` + Token string `yaml:"token" mapstructure:"token"` + UUID string `yaml:"UUID" mapstructure:"uuid"` + Launcher Launcher `yaml:"launcher" mapstructure:",squash"` } // Config is a set of sd-local config entities @@ -30,6 +31,20 @@ type Config struct { filePath string `yaml:"-"` } +// DefaultEntry describes the initial value of an entry +func DefaultEntry() *Entry { + return &Entry{ + APIURL: "", + StoreURL: "", + Token: "", + UUID: "", + Launcher: Launcher{ + Version: "stable", + Image: "screwdrivercd/launcher", + }, + } +} + func create(configPath string) error { _, err := os.Stat(configPath) // if file exists return nil @@ -50,7 +65,7 @@ func create(configPath string) error { err = yaml.NewEncoder(file).Encode(Config{ Entries: map[string]*Entry{ - "default": newEntry(), + "default": DefaultEntry(), }, Current: "default", }) @@ -61,15 +76,6 @@ func create(configPath string) error { return nil } -func newEntry() *Entry { - return &Entry{ - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, - } -} - // New returns parsed config func New(configPath string) (Config, error) { err := create(configPath) @@ -93,20 +99,20 @@ func New(configPath string) (Config, error) { } if c.Entries == nil { - c.Entries = make(map[string]*Entry, 0) + c.Entries = make(map[string]*Entry) } return c, nil } // AddEntry create new Entry and add it to Config -func (c *Config) AddEntry(name string) error { +func (c *Config) AddEntry(name string, entry *Entry) error { _, exist := c.Entries[name] if exist { return fmt.Errorf("config `%s` already exists", name) } - c.Entries[name] = newEntry() + c.Entries[name] = entry return nil } @@ -163,31 +169,36 @@ func (c *Config) Save() error { // Set preserve sd-local config with new value. func (e *Entry) Set(key, value string) error { + // Update the receiver(*Entry) with the args `key` and `value` as follows. + // 1. Encode current entry to empty map + // 2. Check map key found (error handring for unknown key) and set value + // 3. Update current entry by map + var m map[string]interface{} + if err := mapstructure.Decode(e, &m); err != nil { + return err + } + if _, ok := m[key]; !ok { + return fmt.Errorf("invalid key %s", key) + } + + // To preserve compatibility switch key { - case "api-url": - e.APIURL = value - case "store-url": - e.StoreURL = value - case "token": - e.Token = value case "launcher-version": if value == "" { value = "stable" } - e.Launcher.Version = value case "launcher-image": if value == "" { value = "screwdrivercd/launcher" } - e.Launcher.Image = value case "uuid": if value == "" { value = "-" } - e.UUID = value - default: - return fmt.Errorf("invalid key %s", key) } - + m[key] = value + if err := mapstructure.Decode(m, &e); err != nil { + return err + } return nil } diff --git a/config/config_test.go b/config/config_test.go index 6e27376..22ff21e 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -11,29 +11,44 @@ import ( "time" "github.com/go-yaml/yaml" + "github.com/mitchellh/mapstructure" "github.com/stretchr/testify/assert" ) var testDir string = "./testdata" +func dummyEntry() *Entry { + return &Entry{ + APIURL: "api-url", + StoreURL: "store-api-url", + Token: "dummy_token", + Launcher: Launcher{ + Version: "latest", + Image: "screwdrivercd/launcher", + }, + } +} + +// dummyConfig is eqeual to ./testdata/successConfig +func dummyConfig() Config { + return Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), + }, + Current: "default", + } +} + func TestCreateConfig(t *testing.T) { - t.Run("success", func(t *testing.T) { + t.Run("success to init config", func(t *testing.T) { rand.Seed(time.Now().UnixNano()) cnfPath := filepath.Join(testDir, fmt.Sprintf("%vconfig", rand.Int())) defer os.Remove(cnfPath) expect := Config{ Entries: map[string]*Entry{ - "default": { - APIURL: "", - StoreURL: "", - Token: "", - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, - }, + "default": DefaultEntry(), }, Current: "default", } @@ -53,15 +68,7 @@ func TestCreateConfig(t *testing.T) { expect := Config{ Entries: map[string]*Entry{ - "default": { - APIURL: "", - StoreURL: "", - Token: "", - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, - }, + "default": DefaultEntry(), }, Current: "default", } @@ -86,21 +93,8 @@ func TestNewConfig(t *testing.T) { t.Fatal(err) } - testConfig := Config{ - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - }, - }, - Current: "default", - filePath: cnfPath, - } + testConfig := dummyConfig() + testConfig.filePath = cnfPath assert.Nil(t, err) assert.Equal(t, testConfig, actual) @@ -116,256 +110,175 @@ func TestNewConfig(t *testing.T) { } func TestConfigEntry(t *testing.T) { - t.Run("success", func(t *testing.T) { - config := Config{ - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - }, - }, - Current: "default", - } - - testEntry := &Entry{ - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - } - - actual, err := config.Entry("default") - if err != nil { - t.Fatal(err) - } - - assert.Nil(t, err) - assert.Equal(t, testEntry, actual) - }) + cases := map[string]struct { + current string + expectEntry *Entry + expectErr error + }{ + "success": { + current: "default", + expectEntry: dummyEntry(), + expectErr: nil, + }, + "failed": { + current: "doesnotexist", + expectEntry: &Entry{}, + expectErr: fmt.Errorf("config `doesnotexist` does not exist"), + }, + } - t.Run("failure by invalid current", func(t *testing.T) { - config := Config{ - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + for name, test := range cases { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() + config := Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), }, - }, - Current: "doesnotexist", - } - - _, err := config.Entry(config.Current) + Current: "default", + } + actual, err := config.Entry(test.current) - assert.Equal(t, "config `doesnotexist` does not exist", err.Error()) - }) + assert.Equal(t, test.expectErr, err) + assert.Equal(t, test.expectEntry, actual) + }) + } } func TestConfigAddEntry(t *testing.T) { - t.Run("success", func(t *testing.T) { - config := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - }, - }, - } - - err := config.AddEntry("test") - if err != nil { - t.Fatal(err) - } - - expected := Config{ - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - }, - "test": { - APIURL: "", - StoreURL: "", - Token: "", - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, + cases := map[string]struct { + addedEntryName string + expectConfig Config + expectErr error + }{ + "successfully added a test entry": { + addedEntryName: "test", + expectConfig: Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), + "test": DefaultEntry(), }, + Current: "default", }, - Current: "default", - } - - assert.Equal(t, expected, config) - }) - - t.Run("failure by the name that exists", func(t *testing.T) { - config := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + expectErr: nil, + }, + "failure by the name that exists": { + addedEntryName: "default", + expectConfig: Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), }, + Current: "default", }, - } + expectErr: fmt.Errorf("config `default` already exists"), + }, + } - err := config.AddEntry("default") - assert.Equal(t, "config `default` already exists", err.Error()) - }) + for name, test := range cases { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() + config := dummyConfig() + err := config.AddEntry(test.addedEntryName, DefaultEntry()) + assert.Equal(t, test.expectErr, err) + assert.Equal(t, test.expectConfig, config) + }) + } } func TestConfigDeleteEntry(t *testing.T) { - t.Run("success", func(t *testing.T) { - config := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, - }, - "test": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + cases := map[string]struct { + deletedEntryName string + expectConfig Config + expectErr error + }{ + "successfully deleted a test entry": { + deletedEntryName: "test", + expectConfig: Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), }, + Current: "default", }, - } - - err := config.DeleteEntry("test") - if err != nil { - t.Fatal(err) - } - - expected := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + expectErr: nil, + }, + "failure by the name that does not exist": { + deletedEntryName: "doesnotexist", + expectConfig: Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), + "test": DefaultEntry(), }, + Current: "default", }, - } - - assert.Equal(t, expected, config) - }) - - t.Run("failure", func(t *testing.T) { - config := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + expectErr: fmt.Errorf("config `doesnotexist` does not exist"), + }, + "failure by trying to delete current entry": { + deletedEntryName: "default", + expectConfig: Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), + "test": DefaultEntry(), }, + Current: "default", }, - } + expectErr: fmt.Errorf("config `default` is current config"), + }, + } - err := config.DeleteEntry("test") - assert.Equal(t, "config `test` does not exist", err.Error()) - }) + for name, test := range cases { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() - t.Run("failure by trying to delete current entry", func(t *testing.T) { - config := Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{ - Version: "latest", - Image: "screwdrivercd/launcher", - }, + config := Config{ + Entries: map[string]*Entry{ + "default": dummyEntry(), + "test": DefaultEntry(), }, - }, - } + Current: "default", + } - err := config.DeleteEntry("default") - assert.Equal(t, "config `default` is current config", err.Error()) - }) + err := config.DeleteEntry(test.deletedEntryName) + assert.Equal(t, test.expectErr, err) + assert.Equal(t, test.expectConfig, config) + }) + } } func TestConfigSetCurrent(t *testing.T) { - testConfig := func() Config { - return Config{ - Current: "default", - Entries: map[string]*Entry{ - "default": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{}, - }, - "test": { - APIURL: "api-url", - StoreURL: "store-api-url", - Token: "dummy_token", - Launcher: Launcher{}, - }, - }, - filePath: "/home/user/.sdlocal/config", - } + cases := map[string]struct { + setEntryName string + expectCurrent string + expectErr error + }{ + "success to set": { + setEntryName: "test", + expectCurrent: "test", + expectErr: nil, + }, + "failure to set": { + setEntryName: "doesnotexist", + expectCurrent: "default", + expectErr: fmt.Errorf("config `doesnotexist` does not exist"), + }, } - t.Run("success", func(t *testing.T) { - c := testConfig() - c.SetCurrent("test") - assert.Equal(t, "test", c.Current) - }) - t.Run("failure", func(t *testing.T) { - c := testConfig() - err := c.SetCurrent("unknownconfig") - - assert.Equal(t, "config `unknownconfig` does not exist", err.Error()) - }) + for name, test := range cases { + test := test + t.Run(name, func(t *testing.T) { + config := Config{ + Current: "default", + Entries: map[string]*Entry{ + "default": dummyEntry(), + "test": DefaultEntry(), + }, + } + err := config.SetCurrent(test.setEntryName) + assert.Equal(t, test.expectErr, err) + assert.Equal(t, test.expectCurrent, config.Current) + }) + } } func TestConfigSave(t *testing.T) { @@ -419,108 +332,73 @@ func TestConfigSave(t *testing.T) { } func TestSetEntry(t *testing.T) { - testCases := []struct { - name string - setting map[string]string - expectEntry Entry + type setting struct { + key string + value string + } + + cases := map[string]struct { + input setting + expectValue interface{} + expectErr error }{ - { - - name: "success", - setting: map[string]string{ - "api-url": "example-api.com", - "store-url": "example-store.com", - "token": "dummy-token", - "launcher-version": "1.0.0", - "launcher-image": "alpine", - "invalidKey": "invalidValue", - }, - expectEntry: Entry{ - APIURL: "example-api.com", - StoreURL: "example-store.com", - Token: "dummy-token", - Launcher: Launcher{ - Version: "1.0.0", - Image: "alpine", - }, + "set api-url": { + input: setting{ + key: "api-url", + value: "example-api-url", }, + expectValue: "example-api-url", }, - { - name: "success override", - setting: map[string]string{ - "api-url": "override-example-api.com", - "store-url": "override-example-store.com", - "token": "override-dummy-token", - "launcher-version": "override-1.0.0", - "launcher-image": "override-alpine", - "invalidKey": "override-invalidValue", - }, - expectEntry: Entry{ - APIURL: "override-example-api.com", - StoreURL: "override-example-store.com", - Token: "override-dummy-token", - Launcher: Launcher{ - Version: "override-1.0.0", - Image: "override-alpine", - }, + "set launcher-version": { + input: setting{ + key: "launcher-version", + value: "example-version", }, + expectValue: "example-version", }, - { - name: "used default value", - setting: map[string]string{ - "api-url": "override-example-api.com", - "store-url": "override-example-store.com", - "token": "override-dummy-token", - "launcher-version": "", - "launcher-image": "", - "invalidKey": "override-invalidValue", + "set empty to launcher-version": { + input: setting{ + key: "launcher-version", + value: "", }, - expectEntry: Entry{ - APIURL: "override-example-api.com", - StoreURL: "override-example-store.com", - Token: "override-dummy-token", - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, + expectValue: "stable", + }, + "set empty to launcher-image": { + input: setting{ + key: "launcher-image", + value: "", }, + expectValue: "screwdrivercd/launcher", }, - { - name: "invalid key", - setting: map[string]string{ - "api-url": "override-example-api.com", - "store-url": "override-example-store.com", - "token": "override-dummy-token", - "launcher-version": "", - "launcher-image": "", - "invalidKey": "invalidValue", + "set empty to uuid": { + input: setting{ + key: "uuid", + value: "", }, - expectEntry: Entry{ - APIURL: "override-example-api.com", - StoreURL: "override-example-store.com", - Token: "override-dummy-token", - Launcher: Launcher{ - Version: "stable", - Image: "screwdrivercd/launcher", - }, + expectValue: "-", + }, + "set invalid-key": { + input: setting{ + key: "invalid-key", + value: "invalid-value", }, + expectValue: nil, + expectErr: fmt.Errorf("invalid key invalid-key"), }, } - for _, tt := range testCases { - t.Run(tt.name, func(t *testing.T) { + for name, test := range cases { + test := test + t.Run(name, func(t *testing.T) { + t.Parallel() - e := &Entry{} + e := DefaultEntry() + err := e.Set(test.input.key, test.input.value) + assert.Equal(t, test.expectErr, err) - for key, val := range tt.setting { - err := e.Set(key, val) - if key == "invalidKey" { - assert.NotNil(t, err) - } else { - assert.Nil(t, err) - } - } - assert.Equal(t, tt.expectEntry, *e) + var m map[string]interface{} + mapstructure.Decode(e, &m) + assert.Equal(t, test.expectValue, m[test.input.key]) }) } } diff --git a/go.mod b/go.mod index 6c6e3dc..242f1d8 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect github.com/kr/pretty v0.2.0 // indirect github.com/mitchellh/go-homedir v1.1.0 + github.com/mitchellh/mapstructure v1.4.1 github.com/rhysd/go-github-selfupdate v1.2.2 github.com/sirupsen/logrus v1.5.0 github.com/spf13/cobra v0.0.7 diff --git a/go.sum b/go.sum index fde77fa..95e8969 100644 --- a/go.sum +++ b/go.sum @@ -84,6 +84,8 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5 github.com/mitchellh/go-homedir v1.1.0 h1:lukF9ziXFxDFPkA1vsr5zpc1XuPDn/wFntq5mG+4E0Y= github.com/mitchellh/go-homedir v1.1.0/go.mod h1:SfyaCUpYCn1Vlf4IUYiD9fPX4A5wJrkLzIz1N1q0pr0= github.com/mitchellh/mapstructure v1.1.2/go.mod h1:FVVH3fgwuzCH5S8UJGiWEs2h04kUh9fWfEaFds41c1Y= +github.com/mitchellh/mapstructure v1.4.1 h1:CpVNEelQCZBooIPDn+AR3NpivK/TIKU8bDxdASFVQag= +github.com/mitchellh/mapstructure v1.4.1/go.mod h1:bFUtVrKA4DC2yAKiSyO/QUcy7e+RRV2QTWOzhPopBRo= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= github.com/oklog/ulid v1.3.1/go.mod h1:CirwcVhetQ6Lv90oh/F+FBtV6XMibvdAFo93nm5qn4U= github.com/onsi/ginkgo v1.6.0 h1:Ix8l273rp3QzYgXSR+c8d1fTG7UPgYkOSELPhiY/YGw=