Add Delete command to gRPC (#2212)

* Add DeleteRequest and DeleteResponse to gRPC

* Add Delete command to settings

* Add TestDelete to settings_test.go

* Remove temporary directory when TestWrite ends

* Refactor `config delete` command using gRPC function call

Author: MatteoPologruto

commands/daemon/settings.go

@@ -144,3 +144,36 @@ func (s *SettingsService) Write(ctx context.Context, req *rpc.WriteRequest) (*rp
 	}
 	return &rpc.WriteResponse{}, nil
 }
+
+// Delete removes a key from the config file
+func (s *SettingsService) Delete(ctx context.Context, req *rpc.DeleteRequest) (*rpc.DeleteResponse, error) {
+	toDelete := req.GetKey()
+
+	// 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
+	keys := []string{}
+	for _, k := range configuration.Settings.AllKeys() {
+		if !strings.HasPrefix(k, toDelete) {
+			keys = append(keys, k)
+			continue
+		}
+		keyExists = true
+	}
+
+	if !keyExists {
+		return nil, errors.New(tr("key not found in settings"))
+	}
+
+	// Override current settings to delete the key
+	updatedSettings := configuration.Init("")
+	for _, k := range keys {
+		updatedSettings.Set(k, configuration.Settings.Get(k))
+	}
+	configPath := configuration.Settings.ConfigFileUsed()
+	updatedSettings.SetConfigFile(configPath)
+	configuration.Settings = updatedSettings
+
+	return &rpc.DeleteResponse{}, nil
+}

commands/daemon/settings_test.go

@@ -142,7 +142,9 @@ func TestWrite(t *testing.T) {
 	require.NoError(t, err)
 
 	tempDir := paths.TempDir()
-	testFolder, _ := tempDir.MkTempDir("testdata")
+	testFolder, err := tempDir.MkTempDir("testdata")
+	require.NoError(t, err)
+	defer testFolder.RemoveAll()
 
 	// Verifies config files doesn't exist
 	configFile := testFolder.Join("arduino-cli.yml")
@@ -157,3 +159,18 @@ func TestWrite(t *testing.T) {
 	// We don't verify the content since we expect config library, Viper, to work
 	require.True(t, configFile.Exist())
 }
+
+func TestDelete(t *testing.T) {
+	_, err := svc.Delete(context.Background(), &rpc.DeleteRequest{
+		Key: "doesnotexist",
+	})
+	require.Error(t, err)
+
+	_, err = svc.Delete(context.Background(), &rpc.DeleteRequest{
+		Key: "network",
+	})
+	require.NoError(t, err)
+
+	_, err = svc.GetValue(context.Background(), &rpc.GetValueRequest{Key: "network"})
+	require.Error(t, err)
+}

commands/daemon/testdata/arduino-cli.yml

@@ -14,3 +14,6 @@ logging:
   file: ""
   format: text
   level: info
+
+network:
+  proxy: "123"

internal/cli/config/delete.go

@@ -17,13 +17,13 @@ package config
 
 import (
 	"os"
-	"strings"
 
+	"github.com/arduino/arduino-cli/commands/daemon"
 	"github.com/arduino/arduino-cli/configuration"
 	"github.com/arduino/arduino-cli/internal/cli/feedback"
+	"github.com/arduino/arduino-cli/rpc/cc/arduino/cli/settings/v1"
 	"github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
-	"github.com/spf13/viper"
 )
 
 func initDeleteCommand() *cobra.Command {
@@ -47,26 +47,13 @@ func runDeleteCommand(cmd *cobra.Command, args []string) {
 	logrus.Info("Executing `arduino-cli config delete`")
 	toDelete := args[0]
 
-	keys := []string{}
-	exists := false
-	for _, v := range configuration.Settings.AllKeys() {
-		if !strings.HasPrefix(v, toDelete) {
-			keys = append(keys, v)
-			continue
-		}
-		exists = true
+	svc := daemon.SettingsService{}
+	_, err := svc.Delete(cmd.Context(), &settings.DeleteRequest{Key: toDelete})
+	if err != nil {
+		feedback.Fatal(tr("Cannot delete the key %[1]s: %[2]v", toDelete, err), feedback.ErrGeneric)
 	}
-
-	if !exists {
-		feedback.Fatal(tr("Settings key doesn't exist"), feedback.ErrGeneric)
-	}
-
-	updatedSettings := viper.New()
-	for _, k := range keys {
-		updatedSettings.Set(k, configuration.Settings.Get(k))
-	}
-
-	if err := updatedSettings.WriteConfigAs(configuration.Settings.ConfigFileUsed()); err != nil {
-		feedback.Fatal(tr("Can't write config file: %v", err), feedback.ErrGeneric)
+	_, err = svc.Write(cmd.Context(), &settings.WriteRequest{FilePath: configuration.Settings.ConfigFileUsed()})
+	if err != nil {
+		feedback.Fatal(tr("Cannot write the file %[1]s: %[2]v", configuration.Settings.ConfigFileUsed(), err), feedback.ErrGeneric)
 	}
 }

internal/integrationtest/config/config_test.go

@@ -302,7 +302,7 @@ func TestAddRemoveSetDeleteOnUnexistingKey(t *testing.T) {
 
 	_, stderr, err = cli.Run("config", "delete", "some.key", "--config-file", "arduino-cli.yaml")
 	require.Error(t, err)
-	require.Contains(t, string(stderr), "Settings key doesn't exist")
+	require.Contains(t, string(stderr), "Cannot delete the key some.key: key not found in settings\n")
 }
 
 func TestAddSingleArgument(t *testing.T) {

rpc/cc/arduino/cli/settings/v1/settings.pb.go

@@ -36,6 +36,9 @@ service SettingsService {
 
   // Writes to file settings currently stored in memory
   rpc Write(WriteRequest) returns (WriteResponse);
+
+  // Deletes an entry and rewrites the file settings
+  rpc Delete(DeleteRequest) returns (DeleteResponse);
 }
 
 message GetAllResponse {
@@ -78,4 +81,11 @@ message WriteRequest {
   string file_path = 1;
 }
 
-message WriteResponse {}
+message WriteResponse {}
+
+message DeleteRequest {
+  // The key of the setting to delete.
+  string key = 1;
+}
+
+message DeleteResponse {}