diff --git a/client_example/main.go b/client_example/main.go index ab97da9bebc..e564c9627c5 100644 --- a/client_example/main.go +++ b/client_example/main.go @@ -83,8 +83,20 @@ func main() { callGetAll(settingsClient) // Merge applies multiple settings values at once. - log.Println("calling Merge(`{\"foo\": \"bar\", \"daemon\":{\"port\":\"422\"}}`)") - callMerge(settingsClient) + log.Println("calling Merge()") + callMerge(settingsClient, `{"foo": {"value": "bar"}, "daemon":{"port":"422"}, "board_manager": {"additional_urls":["https://example.com"]}}`) + + log.Println("calling GetAll()") + callGetAll(settingsClient) + + log.Println("calling Merge()") + callMerge(settingsClient, `{"foo": {} }`) + + log.Println("calling GetAll()") + callGetAll(settingsClient) + + log.Println("calling Merge()") + callMerge(settingsClient, `{"foo": "bar" }`) // Get the value of the foo key. log.Println("calling GetValue(foo)") @@ -262,11 +274,10 @@ func callUnsetProxy(client settings.SettingsClient) { } } -func callMerge(client settings.SettingsClient) { - bulkSettings := `{"foo": "bar", "daemon":{"port":"422"}}` +func callMerge(client settings.SettingsClient, jsonData string) { _, err := client.Merge(context.Background(), &settings.RawData{ - JsonData: bulkSettings, + JsonData: jsonData, }) if err != nil { diff --git a/commands/daemon/settings.go b/commands/daemon/settings.go index a64f6c2a272..d2e714d5108 100644 --- a/commands/daemon/settings.go +++ b/commands/daemon/settings.go @@ -19,6 +19,8 @@ import ( "context" "encoding/json" "errors" + "fmt" + "strings" "github.com/arduino/arduino-cli/configuration" rpc "github.com/arduino/arduino-cli/rpc/settings" @@ -40,6 +42,31 @@ func (s *SettingsService) GetAll(ctx context.Context, req *rpc.GetAllRequest) (* return nil, err } +// mapper converts a map of nested maps to a map of scalar values. +// For example: +// {"foo": "bar", "daemon":{"port":"420"}, "sketch": {"always_export_binaries": "true"}} +// would convert to: +// {"foo": "bar", "daemon.port":"420", "sketch.always_export_binaries": "true"} +func mapper(toMap map[string]interface{}) map[string]interface{} { + res := map[string]interface{}{} + for k, v := range toMap { + switch data := v.(type) { + case map[string]interface{}: + for mK, mV := range mapper(data) { + // Concatenate keys + res[fmt.Sprintf("%s.%s", k, mK)] = mV + } + // This is done to avoid skipping keys containing empty maps + if len(data) == 0 { + res[k] = map[string]interface{}{} + } + default: + res[k] = v + } + } + return res +} + // Merge applies multiple settings values at once. func (s *SettingsService) Merge(ctx context.Context, req *rpc.RawData) (*rpc.MergeResponse, error) { var toMerge map[string]interface{} @@ -47,8 +74,13 @@ func (s *SettingsService) Merge(ctx context.Context, req *rpc.RawData) (*rpc.Mer return nil, err } - if err := configuration.Settings.MergeConfigMap(toMerge); err != nil { - return nil, err + mapped := mapper(toMerge) + + // Set each value individually. + // This is done because Viper ignores empty strings or maps when + // using the MergeConfigMap function. + for k, v := range mapped { + configuration.Settings.Set(k, v) } return &rpc.MergeResponse{}, nil @@ -61,7 +93,17 @@ func (s *SettingsService) GetValue(ctx context.Context, req *rpc.GetValueRequest key := req.GetKey() value := &rpc.Value{} - if !configuration.Settings.InConfig(key) { + // Check if settings key actually existing, we don't use Viper.InConfig() + // since that doesn't check for keys formatted like daemon.port or those set + // with Viper.Set(). This way we check for all existing settings for sure. + keyExists := false + for _, k := range configuration.Settings.AllKeys() { + if k == key || strings.HasPrefix(k, key) { + keyExists = true + break + } + } + if !keyExists { return nil, errors.New("key not found in settings") } diff --git a/commands/daemon/settings_test.go b/commands/daemon/settings_test.go index 5941321ecae..98c01653497 100644 --- a/commands/daemon/settings_test.go +++ b/commands/daemon/settings_test.go @@ -48,12 +48,38 @@ func TestGetAll(t *testing.T) { } func TestMerge(t *testing.T) { - bulkSettings := `{"foo": "bar", "daemon":{"port":"420"}}` - _, err := svc.Merge(context.Background(), &rpc.RawData{JsonData: bulkSettings}) - require.Nil(t, err) + // Verify defaults + require.Equal(t, "50051", configuration.Settings.GetString("daemon.port")) + require.Equal(t, "", configuration.Settings.GetString("foo")) + require.Equal(t, false, configuration.Settings.GetBool("sketch.always_export_binaries")) + + bulkSettings := `{"foo": "bar", "daemon":{"port":"420"}, "sketch": {"always_export_binaries": "true"}}` + res, err := svc.Merge(context.Background(), &rpc.RawData{JsonData: bulkSettings}) + require.NotNil(t, res) + require.NoError(t, err) require.Equal(t, "420", configuration.Settings.GetString("daemon.port")) require.Equal(t, "bar", configuration.Settings.GetString("foo")) + require.Equal(t, true, configuration.Settings.GetBool("sketch.always_export_binaries")) + + bulkSettings = `{"foo":"", "daemon": {}, "sketch": {"always_export_binaries": "false"}}` + res, err = svc.Merge(context.Background(), &rpc.RawData{JsonData: bulkSettings}) + require.NotNil(t, res) + require.NoError(t, err) + + require.Equal(t, "50051", configuration.Settings.GetString("daemon.port")) + require.Equal(t, "", configuration.Settings.GetString("foo")) + require.Equal(t, false, configuration.Settings.GetBool("sketch.always_export_binaries")) + + bulkSettings = `{"daemon": {"port":""}}` + res, err = svc.Merge(context.Background(), &rpc.RawData{JsonData: bulkSettings}) + require.NotNil(t, res) + require.NoError(t, err) + + require.Equal(t, "", configuration.Settings.GetString("daemon.port")) + // Verifies other values are not changed + require.Equal(t, "", configuration.Settings.GetString("foo")) + require.Equal(t, false, configuration.Settings.GetBool("sketch.always_export_binaries")) reset() } @@ -61,8 +87,32 @@ func TestMerge(t *testing.T) { func TestGetValue(t *testing.T) { key := &rpc.GetValueRequest{Key: "daemon"} resp, err := svc.GetValue(context.Background(), key) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, `{"port":"50051"}`, resp.GetJsonData()) + + key = &rpc.GetValueRequest{Key: "daemon.port"} + resp, err = svc.GetValue(context.Background(), key) + require.NoError(t, err) + require.Equal(t, `"50051"`, resp.GetJsonData()) +} + +func TestGetMergedValue(t *testing.T) { + // Verifies value is not set + key := &rpc.GetValueRequest{Key: "foo"} + res, err := svc.GetValue(context.Background(), key) + require.Nil(t, res) + require.Error(t, err, "Error getting settings value") + + // Merge value + bulkSettings := `{"foo": "bar"}` + _, err = svc.Merge(context.Background(), &rpc.RawData{JsonData: bulkSettings}) + require.NoError(t, err) + + // Verifies value is correctly returned + key = &rpc.GetValueRequest{Key: "foo"} + res, err = svc.GetValue(context.Background(), key) + require.NoError(t, err) + require.Equal(t, `"bar"`, res.GetJsonData()) } func TestGetValueNotFound(t *testing.T) {