From 96764856962117bbca34a2e6930ca0c495b9d478 Mon Sep 17 00:00:00 2001 From: Nikita Kryuchkov Date: Tue, 15 Sep 2020 19:30:37 +0300 Subject: [PATCH 1/2] UpdateAppArg removes app argument if its value is empty --- pkg/visor/visorconfig/v1.go | 44 ++++++--- pkg/visor/visorconfig/v1_test.go | 158 +++++++++++++++++++++++++++++++ 2 files changed, 188 insertions(+), 14 deletions(-) create mode 100644 pkg/visor/visorconfig/v1_test.go diff --git a/pkg/visor/visorconfig/v1.go b/pkg/visor/visorconfig/v1.go index 63b4ef872..85bd18953 100644 --- a/pkg/visor/visorconfig/v1.go +++ b/pkg/visor/visorconfig/v1.go @@ -125,6 +125,7 @@ func (v1 *V1) UpdateAppAutostart(launch *launcher.Launcher, appName string, auto } // UpdateAppArg updates the cli flag of the specified app config and also within the launcher. +// It removes argName from app args if value is an empty string. // The updated config gets flushed to file if there are any changes. func (v1 *V1) UpdateAppArg(launch *launcher.Launcher, appName, argName, value string) error { v1.mu.Lock() @@ -132,34 +133,49 @@ func (v1 *V1) UpdateAppArg(launch *launcher.Launcher, appName, argName, value st conf := v1.Launcher - configChanged := true + configChanged := updateArg(conf, appName, argName, value) + + if !configChanged { + return nil + } + + launch.ResetConfig(launcher.Config{ + VisorPK: v1.PK, + Apps: conf.Apps, + ServerAddr: conf.ServerAddr, + }) + + return v1.flush(v1) +} + +func updateArg(conf *V1Launcher, appName, argName, value string) bool { + configChanged := false + for i := range conf.Apps { if conf.Apps[i].Name == appName { configChanged = true argChanged := false - for j := range conf.Apps[i].Args { + l := len(conf.Apps[i].Args) + for j := 0; j < l; j++ { if conf.Apps[i].Args[j] == argName && j+1 < len(conf.Apps[i].Args) { - conf.Apps[i].Args[j+1] = value + if value == "" { + conf.Apps[i].Args = append(conf.Apps[i].Args[:j], conf.Apps[i].Args[j+2:]...) + l -= 2 + j-- + } else { + conf.Apps[i].Args[j+1] = value + } argChanged = true break } } + if !argChanged { conf.Apps[i].Args = append(conf.Apps[i].Args, argName, value) } } } - if !configChanged { - return nil - } - - launch.ResetConfig(launcher.Config{ - VisorPK: v1.PK, - Apps: conf.Apps, - ServerAddr: conf.ServerAddr, - }) - - return v1.flush(v1) + return configChanged } diff --git a/pkg/visor/visorconfig/v1_test.go b/pkg/visor/visorconfig/v1_test.go new file mode 100644 index 000000000..96f484bd7 --- /dev/null +++ b/pkg/visor/visorconfig/v1_test.go @@ -0,0 +1,158 @@ +package visorconfig + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/skycoin/skywire/pkg/app/launcher" +) + +func Test_updateArg(t *testing.T) { + type args struct { + conf *V1Launcher + appName string + argName string + value string + } + tests := []struct { + name string + args args + wantResult bool + wantConf *V1Launcher + }{ + { + name: "Case 1", + args: args{ + conf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-passcode", "1234"}, + }, + }, + }, + appName: "skysocks-client", + argName: "-passcode", + value: "4321", + }, + wantResult: true, + wantConf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-passcode", "4321"}, + }, + }, + }, + }, + { + name: "Case 2", + args: args{ + conf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-passcode", "1234"}, + }, + }, + }, + appName: "skysocks-client", + argName: "-passcode", + value: "", + }, + wantResult: true, + wantConf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{}, + }, + }, + }, + }, + { + name: "Case 3", + args: args{ + conf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, + }, + }, + }, + appName: "skysocks-client", + argName: "-passcode", + value: "", + }, + wantResult: true, + wantConf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-test", "abc"}, + }, + }, + }, + }, + { + name: "Case 4", + args: args{ + conf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, + }, + }, + }, + appName: "skysocks-client", + argName: "-arg1", + value: "678", + }, + wantResult: true, + wantConf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-passcode", "1234", "-test", "abc", "-arg1", "678"}, + }, + }, + }, + }, + { + name: "Case 5", + args: args{ + conf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, + }, + }, + }, + appName: "unknown", + argName: "-arg1", + value: "678", + }, + wantResult: false, + wantConf: &V1Launcher{ + Apps: []launcher.AppConfig{ + { + Name: "skysocks-client", + Args: []string{"-t", "-passcode", "1234", "-test", "abc"}, + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := updateArg(tt.args.conf, tt.args.appName, tt.args.argName, tt.args.value) + assert.Equal(t, result, tt.wantResult) + assert.EqualValues(t, tt.args.conf, tt.wantConf) + }) + } +} From 8a60f90dedd22b332a10dfe23664ab1bfebbd4b0 Mon Sep 17 00:00:00 2001 From: Nikita Kryuchkov Date: Tue, 15 Sep 2020 19:35:04 +0300 Subject: [PATCH 2/2] Fix ineffectual assignment --- pkg/visor/visorconfig/v1.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/visor/visorconfig/v1.go b/pkg/visor/visorconfig/v1.go index 85bd18953..29fe3f7b8 100644 --- a/pkg/visor/visorconfig/v1.go +++ b/pkg/visor/visorconfig/v1.go @@ -161,7 +161,6 @@ func updateArg(conf *V1Launcher, appName, argName, value string) bool { if conf.Apps[i].Args[j] == argName && j+1 < len(conf.Apps[i].Args) { if value == "" { conf.Apps[i].Args = append(conf.Apps[i].Args[:j], conf.Apps[i].Args[j+2:]...) - l -= 2 j-- } else { conf.Apps[i].Args[j+1] = value