Fix gRPC interface function to merge configs

client_example/main.go

@@ -83,9 +83,12 @@ func main() {
 	callGetAll(settingsClient)
 
 	// Merge applies multiple settings values at once.
-	log.Println("calling Merge(`{\"foo\": \"bar\", \"daemon\":{\"port\":\"422\"}}`)")
+	log.Println("calling Merge()")
 	callMerge(settingsClient)
 
+	log.Println("calling GetAll()")
+	callGetAll(settingsClient)
+
 	// Get the value of the foo key.
 	log.Println("calling GetValue(foo)")
 	callGetValue(settingsClient)
@@ -263,7 +266,7 @@ func callUnsetProxy(client settings.SettingsClient) {
 }
 
 func callMerge(client settings.SettingsClient) {
-	bulkSettings := `{"foo": "bar", "daemon":{"port":"422"}}`
+	bulkSettings := `{"foo": "bar", "daemon":{"port":"422"}, "board_manager": {"additional_urls":["https://example.com"]}}`
 	_, err := client.Merge(context.Background(),
 		&settings.RawData{
 			JsonData: bulkSettings,

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,15 +42,46 @@ 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 v.(type) {
+		case map[string]interface{}:
+			data := v.(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{}
 	if err := json.Unmarshal([]byte(req.GetJsonData()), &toMerge); err != nil {
 		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 +94,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")
 	}
 

commands/daemon/settings_test.go

@@ -48,21 +48,68 @@ 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"))
 
 	reset()
 }
 
 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) {