ui: rework tunnel adding logic and fix bugs

2 files changed, 45 insertions, 23 deletions

diff --git a/ui/tunnelspage.go b/ui/tunnelspage.go
index 44150267..e3053186 100644
--- a/ui/tunnelspage.go
+++ b/ui/tunnelspage.go
@@ -11,6 +11,7 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"time"
 
@@ -192,34 +193,55 @@ func (tp *TunnelsPage) importFiles(paths []string) {
 	}
 
 	if lastErr != nil || unparsedConfigs == nil {
-		walk.MsgBox(tp.Form(), "Error", fmt.Sprintf("Could not parse some files: %v", lastErr), walk.MsgBoxIconWarning)
+		walk.MsgBox(tp.Form(), "Error", fmt.Sprintf("Could not import selected configuration: %v", lastErr), walk.MsgBoxIconWarning)
 		return
 	}
 
-	var configs []*conf.Config
+	// Add in reverse order so that the first one is selected.
+	sort.Slice(unparsedConfigs, func(i, j int) bool {
+		//TODO: use proper tunnel string sorting/comparison algorithm, as the other comments indicate too.
+		return strings.Compare(unparsedConfigs[i].Name, unparsedConfigs[j].Name) > 0
+	})
+
+	existingTunnelList, err := service.IPCClientTunnels()
+	if err != nil {
+		walk.MsgBox(tp.Form(), "Error", fmt.Sprintf("Could not enumerate existing tunnels: %v", lastErr), walk.MsgBoxIconWarning)
+		return
+	}
+	existingLowerTunnels := make(map[string]bool, len(existingTunnelList))
+	for _, tunnel := range existingTunnelList {
+		existingLowerTunnels[strings.ToLower(tunnel.Name)] = true
+	}
 
+	configCount := 0
 	for _, unparsedConfig := range unparsedConfigs {
+		if existingLowerTunnels[strings.ToLower(unparsedConfig.Name)] {
+			lastErr = fmt.Errorf("The tunnel \"%s\" already exists", unparsedConfig.Name)
+			continue
+		}
 		config, err := conf.FromWgQuick(unparsedConfig.Config, unparsedConfig.Name)
 		if err != nil {
 			lastErr = err
 			continue
 		}
-		service.IPCClientNewTunnel(config)
-		configs = append(configs, config)
+		_, err = service.IPCClientNewTunnel(config)
+		if err != nil {
+			lastErr = err
+			continue
+		}
+		configCount++
 	}
 
-	m, n := len(configs), len(unparsedConfigs)
+	m, n := configCount, len(unparsedConfigs)
 	switch {
 	case n == 1 && m != n:
-		walk.MsgBox(tp.Form(), "Error", fmt.Sprintf("Could not parse some files: %v", lastErr), walk.MsgBoxIconWarning)
+		walk.MsgBox(tp.Form(), "Error", fmt.Sprintf("Unable to import configuration: %v", lastErr), walk.MsgBoxIconWarning)
 	case n == 1 && m == n:
 		// TODO: Select tunnel in the list
 	case m == n:
 		walk.MsgBox(tp.Form(), "Imported tunnels", fmt.Sprintf("Imported %d tunnels", m), walk.MsgBoxOK)
 	case m != n:
 		walk.MsgBox(tp.Form(), "Imported tunnels", fmt.Sprintf("Imported %d of %d tunnels", m, n), walk.MsgBoxIconWarning)
-	default:
-		panic("unreachable case")
 	}
 }
 
diff --git a/ui/tunnelsview.go b/ui/tunnelsview.go
index 58816d09..15dc22ca 100644
--- a/ui/tunnelsview.go
+++ b/ui/tunnelsview.go
@@ -7,6 +7,7 @@ package ui
 
 import (
 	"sort"
+	"strings"
 
 	"github.com/lxn/walk"
 	"golang.zx2c4.com/wireguard/windows/service"
@@ -176,34 +177,33 @@ func (tv *TunnelsView) onTunnelsChange() {
 				}
 			}
 		}
-		tunnelsAdded := 0
-		lastTunnelName := ""
+		didAdd := false
+		firstTunnelName := ""
 		for tunnel := range newTunnels {
 			if !oldTunnels[tunnel] {
+				//TODO: use proper tunnel string sorting/comparison algorithm, as the other comments indicate too.
+				if len(firstTunnelName) == 0 || strings.Compare(firstTunnelName, tunnel.Name) > 0 {
+					firstTunnelName = tunnel.Name
+				}
 				tv.model.tunnels = append(tv.model.tunnels, tunnel)
-				tunnelsAdded++
-				lastTunnelName = tunnel.Name
+				didAdd = true
 			}
 		}
-		if tunnelsAdded > 0 {
+		if didAdd {
 			tv.model.PublishRowsReset()
 			tv.model.Sort(tv.model.SortedColumn(), tv.model.SortOrder())
-			// If adding a tunnel for the first time when the previously were none, select it
 			if len(tv.SelectedIndexes()) == 0 {
-				tv.selectTunnel(lastTunnelName)
+				tv.selectTunnel(firstTunnelName)
 			}
 		}
 	})
 }
 
-// Tiny helper to select a tunnel by name.
 func (tv *TunnelsView) selectTunnel(tunnelName string) {
-	tv.Synchronize(func() {
-		for i, tunnel := range tv.model.tunnels {
-			if tunnel.Name == tunnelName {
-				tv.SetCurrentIndex(i)
-				break
-			}
+	for i, tunnel := range tv.model.tunnels {
+		if tunnel.Name == tunnelName {
+			tv.SetCurrentIndex(i)
+			break
 		}
-	})
+	}
 }