From 6d5685c5f559f84179b4f2bad667b795c94a37fe Mon Sep 17 00:00:00 2001 From: Carlos Henrique Guardao Gandarez Date: Sun, 28 Nov 2021 16:39:43 -0300 Subject: [PATCH] Revert to use gopkg.in/ini.v1 when saving config file --- go.mod | 2 +- go.sum | 2 + pkg/backoff/backoff.go | 44 ++-- pkg/backoff/backoff_internal_test.go | 60 +---- pkg/backoff/testdata/multiline.cfg | 8 - pkg/backoff/testdata/multiline_expected.cfg | 8 - pkg/config/config_test.go | 43 +++- .../testdata/wakatime-multiline-expected.cfg | 13 ++ pkg/ini/ini.go | 220 ------------------ 9 files changed, 83 insertions(+), 317 deletions(-) delete mode 100644 pkg/backoff/testdata/multiline.cfg delete mode 100644 pkg/backoff/testdata/multiline_expected.cfg create mode 100644 pkg/config/testdata/wakatime-multiline-expected.cfg delete mode 100644 pkg/ini/ini.go diff --git a/go.mod b/go.mod index 82684c09..35579447 100644 --- a/go.mod +++ b/go.mod @@ -17,7 +17,7 @@ require ( github.com/stretchr/testify v1.7.0 github.com/yookoala/realpath v1.0.0 go.etcd.io/bbolt v1.3.5 - gopkg.in/ini.v1 v1.62.0 + gopkg.in/ini.v1 v1.65.0 ) require ( diff --git a/go.sum b/go.sum index 259d3917..a627a6af 100644 --- a/go.sum +++ b/go.sum @@ -680,6 +680,8 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/ini.v1 v1.62.0 h1:duBzk771uxoUuOlyRLkHsygud9+5lrlGjdFBb4mSKDU= gopkg.in/ini.v1 v1.62.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= +gopkg.in/ini.v1 v1.65.0 h1:B2//IEITFk89S+Nl2tozBeqUvFEpUAY6daarSlrx8jU= +gopkg.in/ini.v1 v1.65.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7/go.mod h1:JAlM8MvJe8wmxCU4Bli9HhUf9+ttbYbLASfIpnQbh74= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= diff --git a/pkg/backoff/backoff.go b/pkg/backoff/backoff.go index dda994c3..77132ec3 100644 --- a/pkg/backoff/backoff.go +++ b/pkg/backoff/backoff.go @@ -7,9 +7,8 @@ import ( "time" "github.com/wakatime/wakatime-cli/pkg/api" - "github.com/wakatime/wakatime-cli/pkg/config" + ini "github.com/wakatime/wakatime-cli/pkg/config" "github.com/wakatime/wakatime-cli/pkg/heartbeat" - "github.com/wakatime/wakatime-cli/pkg/ini" "github.com/wakatime/wakatime-cli/pkg/log" "github.com/spf13/viper" @@ -35,12 +34,12 @@ type Config struct { // WithBackoff initializes and returns a heartbeat handle option, which // can be used in a heartbeat processing pipeline to prevent trying to send // a heartbeat when the api is unresponsive. -func WithBackoff(c Config) heartbeat.HandleOption { +func WithBackoff(config Config) heartbeat.HandleOption { return func(next heartbeat.Handle) heartbeat.Handle { return func(hh []heartbeat.Heartbeat) ([]heartbeat.Result, error) { log.Debugln("execute heartbeat backoff algorithm") - if shouldBackoff(c.Retries, c.At) { + if shouldBackoff(config.Retries, config.At) { return nil, api.Err("won't send heartbeat due to backoff") } @@ -49,16 +48,16 @@ func WithBackoff(c Config) heartbeat.HandleOption { log.Debugf("incrementing backoff due to error") // error response, increment backoff - if updateErr := updateBackoffSettings(c.V, c.Retries+1, time.Now()); updateErr != nil { + if updateErr := updateBackoffSettings(config.V, config.Retries+1, time.Now()); updateErr != nil { log.Warnf("failed to update backoff settings: %s", updateErr) } return nil, err } - if !c.At.IsZero() { + if !config.At.IsZero() { // success response, reset backoff - if resetErr := updateBackoffSettings(c.V, 0, time.Time{}); resetErr != nil { + if resetErr := updateBackoffSettings(config.V, 0, time.Time{}); resetErr != nil { log.Warnf("failed to reset backoff settings: %s", resetErr) } } @@ -87,28 +86,25 @@ func shouldBackoff(retries int, at time.Time) bool { } func updateBackoffSettings(v *viper.Viper, retries int, at time.Time) error { - keys := map[ini.Key]string{ - { - Section: "internal", - Name: "backoff_retries", - }: strconv.Itoa(retries), - { - Section: "internal", - Name: "backoff_at", - }: "", + w, err := ini.NewIniWriter(v, ini.FilePath) + if err != nil { + return fmt.Errorf("failed to parse config file: %s", err) + } + + keyValue := map[string]string{ + "backoff_retries": strconv.Itoa(retries), + "backoff_at": "", } if !at.IsZero() { - keys[ini.Key{ - Section: "internal", - Name: "backoff_at", - }] = at.Format(config.DateFormat) + keyValue["backoff_at"] = at.Format(ini.DateFormat) + } else { + keyValue["backoff_at"] = "" } - iniFile, err := config.InternalFilePath(v) - if err != nil { - return fmt.Errorf("error getting filepath: %s", err) + if err := w.Write("internal", keyValue); err != nil { + return fmt.Errorf("failed to write to internal config file: %s", err) } - return ini.SetKeys(iniFile, keys) + return nil } diff --git a/pkg/backoff/backoff_internal_test.go b/pkg/backoff/backoff_internal_test.go index 248974ec..dd9f83de 100644 --- a/pkg/backoff/backoff_internal_test.go +++ b/pkg/backoff/backoff_internal_test.go @@ -2,12 +2,10 @@ package backoff import ( "os" - "strings" "testing" "time" "github.com/wakatime/wakatime-cli/pkg/config" - "github.com/wakatime/wakatime-cli/pkg/ini" "github.com/spf13/viper" "github.com/stretchr/testify/assert" @@ -52,16 +50,16 @@ func TestUpdateBackoffSettings(t *testing.T) { err = updateBackoffSettings(v, 2, at) require.NoError(t, err) - writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { + ini, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { assert.Equal(t, v, vp) return tmpFile.Name(), nil }) require.NoError(t, err) - backoffAt := writer.File.Section("internal").Key("backoff_at").MustTimeFormat(config.DateFormat) + backoffAt := ini.File.Section("internal").Key("backoff_at").MustTimeFormat(config.DateFormat) assert.WithinDuration(t, time.Now(), backoffAt, 15*time.Second) - assert.Equal(t, "2", writer.File.Section("internal").Key("backoff_retries").String()) + assert.Equal(t, "2", ini.File.Section("internal").Key("backoff_retries").String()) } func TestUpdateBackoffSettings_NotInBackoff(t *testing.T) { @@ -78,58 +76,12 @@ func TestUpdateBackoffSettings_NotInBackoff(t *testing.T) { err = updateBackoffSettings(v, 0, time.Time{}) require.NoError(t, err) - writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { + ini, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { assert.Equal(t, v, vp) return tmpFile.Name(), nil }) require.NoError(t, err) - assert.Empty(t, writer.File.Section("internal").Key("backoff_at").String()) - assert.Equal(t, "0", writer.File.Section("internal").Key("backoff_retries").String()) -} - -func TestUpdateBackoffSettings_NoMultilineSideEffects(t *testing.T) { - v := viper.New() - - tmpFile, err := os.CreateTemp(os.TempDir(), "wakatime") - require.NoError(t, err) - - defer os.Remove(tmpFile.Name()) - - v.Set("config", tmpFile.Name()) - v.Set("internal-config", tmpFile.Name()) - - copyFile(t, "testdata/multiline.cfg", tmpFile.Name()) - - err = updateBackoffSettings(v, 0, time.Time{}) - require.NoError(t, err) - - writer, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { - assert.Equal(t, v, vp) - return tmpFile.Name(), nil - }) - require.NoError(t, err) - - assert.Equal(t, "\none\ntwo", writer.File.Section("settings").Key("ignore").String()) - assert.Empty(t, writer.File.Section("internal").Key("backoff_at").String()) - assert.Equal(t, "0", writer.File.Section("internal").Key("backoff_retries").String()) - - value := ini.GetKey(tmpFile.Name(), ini.Key{Section: "settings", Name: "ignore"}) - assert.Equal(t, "\n one\n two", value) - - actual, err := os.ReadFile(tmpFile.Name()) - require.NoError(t, err) - - expected, err := os.ReadFile("testdata/multiline_expected.cfg") - require.NoError(t, err) - - assert.Equal(t, strings.ReplaceAll(string(expected), "\r", ""), string(actual)) -} - -func copyFile(t *testing.T, source, destination string) { - input, err := os.ReadFile(source) - require.NoError(t, err) - - err = os.WriteFile(destination, input, 0600) - require.NoError(t, err) + assert.Empty(t, ini.File.Section("internal").Key("backoff_at").String()) + assert.Equal(t, "0", ini.File.Section("internal").Key("backoff_retries").String()) } diff --git a/pkg/backoff/testdata/multiline.cfg b/pkg/backoff/testdata/multiline.cfg deleted file mode 100644 index 9999176f..00000000 --- a/pkg/backoff/testdata/multiline.cfg +++ /dev/null @@ -1,8 +0,0 @@ -[settings] -ignore = - one - two -debug = true -[internal] -backoff_retries = 1 -backoff_at = 2021-09-05T12:17:21-07:00 \ No newline at end of file diff --git a/pkg/backoff/testdata/multiline_expected.cfg b/pkg/backoff/testdata/multiline_expected.cfg deleted file mode 100644 index cc250f94..00000000 --- a/pkg/backoff/testdata/multiline_expected.cfg +++ /dev/null @@ -1,8 +0,0 @@ -[settings] -ignore = - one - two -debug = true -[internal] -backoff_retries = 0 -backoff_at = \ No newline at end of file diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index d7daf025..c7cc5740 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -39,10 +39,10 @@ func TestReadInConfig_Multiline(t *testing.T) { require.NoError(t, err) ignoreConfig := strings.ReplaceAll(vipertools.GetString(v, "settings.ignore"), "\r", "") - assert.Equal(t, "\nCOMMIT_EDITMSG$\nPULLREQ_EDITMSG$\nMERGE_MSG$\nTAG_EDITMSG$", ignoreConfig) + assert.Equal(t, "\n COMMIT_EDITMSG$\n PULLREQ_EDITMSG$\n MERGE_MSG$\n TAG_EDITMSG$", ignoreConfig) gitConfig := strings.ReplaceAll(vipertools.GetString(v, "git.submodules_disabled"), "\r", "") - assert.Equal(t, "\n.*secret.*\nfix.*", gitConfig) + assert.Equal(t, "\n .*secret.*\n fix.*", gitConfig) } func TestReadInConfig_Multiple(t *testing.T) { @@ -238,6 +238,37 @@ func TestWrite(t *testing.T) { } } +func TestWrite_NoMultilineSideEffects(t *testing.T) { + multilineOption := viper.IniLoadOptions(ini.LoadOptions{AllowPythonMultilineValues: true}) + v := viper.NewWithOptions(multilineOption) + + tmpFile, err := os.CreateTemp(os.TempDir(), "wakatime") + require.NoError(t, err) + + defer os.Remove(tmpFile.Name()) + + v.Set("config", tmpFile.Name()) + + copyFile(t, "testdata/wakatime-multiline.cfg", tmpFile.Name()) + + w, err := config.NewIniWriter(v, func(vp *viper.Viper) (string, error) { + assert.Equal(t, v, vp) + return tmpFile.Name(), nil + }) + require.NoError(t, err) + + err = w.Write("settings", map[string]string{"debug": "true"}) + require.NoError(t, err) + + actual, err := os.ReadFile(tmpFile.Name()) + require.NoError(t, err) + + expected, err := os.ReadFile("testdata/wakatime-multiline-expected.cfg") + require.NoError(t, err) + + assert.Equal(t, strings.ReplaceAll(string(expected), "\r", ""), string(actual)) +} + func TestWriteErr(t *testing.T) { w := config.IniWriter{} @@ -246,3 +277,11 @@ func TestWriteErr(t *testing.T) { assert.Equal(t, "got undefined wakatime config file instance", err.Error()) } + +func copyFile(t *testing.T, source, destination string) { + input, err := os.ReadFile(source) + require.NoError(t, err) + + err = os.WriteFile(destination, input, 0600) + require.NoError(t, err) +} diff --git a/pkg/config/testdata/wakatime-multiline-expected.cfg b/pkg/config/testdata/wakatime-multiline-expected.cfg new file mode 100644 index 00000000..879da467 --- /dev/null +++ b/pkg/config/testdata/wakatime-multiline-expected.cfg @@ -0,0 +1,13 @@ +[settings] +ignore = """ + COMMIT_EDITMSG$ + PULLREQ_EDITMSG$ + MERGE_MSG$ + TAG_EDITMSG$""" +debug = true + +[git] +submodules_disabled = """ + .*secret.* + fix.*""" + diff --git a/pkg/ini/ini.go b/pkg/ini/ini.go deleted file mode 100644 index 8c9c66dd..00000000 --- a/pkg/ini/ini.go +++ /dev/null @@ -1,220 +0,0 @@ -package ini - -import ( - "bufio" - "fmt" - "os" - "strings" -) - -// Key holds a single INI section and key. -type Key struct { - // Section is an INI section for this key - Section string - // Name is the key name of an INI setting - Name string -} - -// GetKey returns the value for given INI Key. -func GetKey(iniFile string, key Key) string { - keys := []Key{key} - - return GetKeys(iniFile, keys)[key] -} - -// GetKeys returns the values for given INI Keys. -func GetKeys(iniFile string, keys []Key) map[Key]string { - result := map[Key]string{} - finding := map[Key]bool{} - found := map[Key]bool{} - - for _, key := range keys { - finding[key] = true - } - - fh, err := os.Open(iniFile) - if err != nil { - return result - } - - defer fh.Close() - - scanner := bufio.NewScanner(fh) - scanner.Split(bufio.ScanLines) - - var ( - currentSection string - currentKey Key - multiline []string - ) - - for scanner.Scan() { - line := scanner.Text() - - if len(multiline) > 0 { - if isMultiline(line) { - multiline = append(multiline, line) - continue - } - - if finding[currentKey] { - result[currentKey] = strings.Join(multiline, "\n") - found[currentKey] = true - - // return early if we've already found all they keys - if len(found) == len(finding) { - return result - } - } - - multiline = []string{} - } - - if isSection(line) { - currentSection = getSectionName(line) - continue - } - - split := strings.SplitN(line, "=", 2) - if len(split) != 2 { - continue - } - - possibleKey := getPossibleKeyName(split) - if possibleKey == "" { - continue - } - - currentKey = Key{ - Section: currentSection, - Name: possibleKey, - } - - multiline = append(multiline, strings.TrimLeft(strings.TrimLeft(split[1], " "), "\t")) - } - - if len(multiline) > 0 && finding[currentKey] { - result[currentKey] = strings.Join(multiline, "\n") - found[currentKey] = true - } - - // key not found in INI file, return empty string - return result -} - -// SetKey saves the value for given INI Key. -func SetKey(iniFile string, key Key, value string) error { - keys := map[Key]string{key: value} - return SetKeys(iniFile, keys) -} - -// SetKeys saves multiple values for given INI Keys. -func SetKeys(iniFile string, keys map[Key]string) error { - if len(keys) == 0 { - return nil - } - - fh, err := os.Open(iniFile) - if err != nil { - return fmt.Errorf("failed to open ini file %q: %s", iniFile, err) - } - - scanner := bufio.NewScanner(fh) - scanner.Split(bufio.ScanLines) - - var ( - lines []string - currentSection string - ) - - found := map[Key]bool{} - - for scanner.Scan() { - line := scanner.Text() - - if isSection(line) { // nolint:nestif - for key, value := range keys { - if found[key] { - continue - } - - if currentSection == key.Section { - found[key] = true - - lines = append(lines, key.Name+" = "+value) - - break - } - } - - currentSection = getSectionName(line) - } else { - localFound := false - - for key, value := range keys { - if found[key] { - continue - } - - if currentSection == key.Section { - split := strings.SplitN(line, "=", 2) - if len(split) == 2 { - possibleKey := getPossibleKeyName(split) - if possibleKey == key.Name { - found[key] = true - localFound = true - lines = append(lines, key.Name+" = "+value) - break - } - } - } - } - - if localFound { - continue - } - } - - lines = append(lines, line) - } - - for key, value := range keys { - if found[key] { - continue - } - - if currentSection != key.Section { - lines = append(lines, "["+key.Section+"]") - - currentSection = key.Section - } - - lines = append(lines, key.Name+" = "+value) - } - - fh.Close() - - output := strings.Join(lines, "\n") - - return os.WriteFile(iniFile, []byte(output), 0644) // nolint:gosec -} - -func isSection(line string) bool { - return strings.HasPrefix(line, "[") && strings.HasSuffix(line, "]") && len(line) > 2 -} - -func isMultiline(line string) bool { - return strings.HasPrefix(line, " ") || strings.HasPrefix(line, "\t") -} - -func getSectionName(line string) string { - return strings.TrimSuffix(strings.TrimPrefix(line, "["), "]") -} - -func getPossibleKeyName(splitLine []string) string { - if len(splitLine) == 0 { - return "" - } - - return strings.TrimRight(strings.TrimRight(splitLine[0], " "), "\t") -}