lib/config, lib/model: Commit auto-accepted folders all at once (#6684)

lib/api/mocked_config_test.go

@@ -102,6 +102,10 @@ func (c *mockedConfig) SetFolder(fld config.FolderConfiguration) (config.Waiter,
 	return noopWaiter{}, nil
 }
 
+func (c *mockedConfig) SetFolders(folders []config.FolderConfiguration) (config.Waiter, error) {
+	return noopWaiter{}, nil
+}
+
 func (c *mockedConfig) Device(id protocol.DeviceID) (config.DeviceConfiguration, bool) {
 	return config.DeviceConfiguration{}, false
 }

lib/config/wrapper.go

@@ -73,6 +73,7 @@ type Wrapper interface {
 	Folders() map[string]FolderConfiguration
 	FolderList() []FolderConfiguration
 	SetFolder(fld FolderConfiguration) (Waiter, error)
+	SetFolders(folders []FolderConfiguration) (Waiter, error)
 
 	Device(id protocol.DeviceID) (DeviceConfiguration, bool)
 	Devices() map[protocol.DeviceID]DeviceConfiguration
@@ -96,7 +97,6 @@ type wrapper struct {
 
 	waiter    Waiter // Latest ongoing config change
 	deviceMap map[protocol.DeviceID]DeviceConfiguration
-	folderMap map[string]FolderConfiguration
 	subs      []Committer
 	mut       sync.Mutex
 
@@ -196,7 +196,6 @@ func (w *wrapper) replaceLocked(to Configuration) (Waiter, error) {
 
 	w.cfg = to
 	w.deviceMap = nil
-	w.folderMap = nil
 
 	w.waiter = w.notifyListeners(from.Copy(), to.Copy())
 
@@ -288,13 +287,11 @@ func (w *wrapper) RemoveDevice(id protocol.DeviceID) (Waiter, error) {
 func (w *wrapper) Folders() map[string]FolderConfiguration {
 	w.mut.Lock()
 	defer w.mut.Unlock()
-	if w.folderMap == nil {
-		w.folderMap = make(map[string]FolderConfiguration, len(w.cfg.Folders))
-		for _, fld := range w.cfg.Folders {
-			w.folderMap[fld.ID] = fld.Copy()
-		}
+	folderMap := make(map[string]FolderConfiguration, len(w.cfg.Folders))
+	for _, fld := range w.cfg.Folders {
+		folderMap[fld.ID] = fld.Copy()
 	}
-	return w.folderMap
+	return folderMap
 }
 
 // FolderList returns a slice of folders.
@@ -307,19 +304,30 @@ func (w *wrapper) FolderList() []FolderConfiguration {
 // SetFolder adds a new folder to the configuration, or overwrites an existing
 // folder with the same ID.
 func (w *wrapper) SetFolder(fld FolderConfiguration) (Waiter, error) {
+	return w.SetFolders([]FolderConfiguration{fld})
+}
+
+// SetFolders adds new folders to the configuration, or overwrites existing
+// folders with the same ID.
+func (w *wrapper) SetFolders(folders []FolderConfiguration) (Waiter, error) {
 	w.mut.Lock()
 	defer w.mut.Unlock()
 
 	newCfg := w.cfg.Copy()
 
-	for i := range newCfg.Folders {
-		if newCfg.Folders[i].ID == fld.ID {
-			newCfg.Folders[i] = fld
-			return w.replaceLocked(newCfg)
+	inds := make(map[string]int, len(w.cfg.Folders))
+	for i, folder := range newCfg.Folders {
+		inds[folder.ID] = i
+	}
+	filtered := folders[:0]
+	for _, folder := range folders {
+		if i, ok := inds[folder.ID]; ok {
+			newCfg.Folders[i] = folder
+		} else {
+			filtered = append(filtered, folder)
 		}
 	}
-
-	newCfg.Folders = append(newCfg.Folders, fld)
+	newCfg.Folders = append(newCfg.Folders, filtered...)
 
 	return w.replaceLocked(newCfg)
 }

lib/model/model.go

@@ -964,8 +964,19 @@ func (m *model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon
 
 	// Needs to happen outside of the fmut, as can cause CommitConfiguration
 	if deviceCfg.AutoAcceptFolders {
+		changedFolders := make([]config.FolderConfiguration, 0, len(cm.Folders))
 		for _, folder := range cm.Folders {
-			changed = m.handleAutoAccepts(deviceCfg, folder) || changed
+			if fcfg, fchanged := m.handleAutoAccepts(deviceCfg, folder); fchanged {
+				changedFolders = append(changedFolders, fcfg)
+			}
+		}
+		if len(changedFolders) > 0 {
+			// Need to wait for the waiter, as this calls CommitConfiguration,
+			// which sets up the folder and as we return from this call,
+			// ClusterConfig starts poking at m.folderFiles and other things
+			// that might not exist until the config is committed.
+			w, _ := m.cfg.SetFolders(changedFolders)
+			w.Wait()
 		}
 	}
 
@@ -1246,7 +1257,7 @@ func (m *model) handleDeintroductions(introducerCfg config.DeviceConfiguration,
 
 // handleAutoAccepts handles adding and sharing folders for devices that have
 // AutoAcceptFolders set to true.
-func (m *model) handleAutoAccepts(deviceCfg config.DeviceConfiguration, folder protocol.Folder) bool {
+func (m *model) handleAutoAccepts(deviceCfg config.DeviceConfiguration, folder protocol.Folder) (config.FolderConfiguration, bool) {
 	if cfg, ok := m.cfg.Folder(folder.ID); !ok {
 		defaultPath := m.cfg.Options().DefaultFolderPath
 		defaultPathFs := fs.NewFilesystem(fs.FilesystemTypeBasic, defaultPath)
@@ -1263,32 +1274,24 @@ func (m *model) handleAutoAccepts(deviceCfg config.DeviceConfiguration, folder p
 			fcfg.Devices = append(fcfg.Devices, config.FolderDeviceConfiguration{
 				DeviceID: deviceCfg.DeviceID,
 			})
-			// Need to wait for the waiter, as this calls CommitConfiguration,
-			// which sets up the folder and as we return from this call,
-			// ClusterConfig starts poking at m.folderFiles and other things
-			// that might not exist until the config is committed.
-			w, _ := m.cfg.SetFolder(fcfg)
-			w.Wait()
 
 			l.Infof("Auto-accepted %s folder %s at path %s", deviceCfg.DeviceID, folder.Description(), fcfg.Path)
-			return true
+			return fcfg, true
 		}
 		l.Infof("Failed to auto-accept folder %s from %s due to path conflict", folder.Description(), deviceCfg.DeviceID)
-		return false
+		return config.FolderConfiguration{}, false
 	} else {
 		for _, device := range cfg.DeviceIDs() {
 			if device == deviceCfg.DeviceID {
 				// Already shared nothing todo.
-				return false
+				return config.FolderConfiguration{}, false
 			}
 		}
 		cfg.Devices = append(cfg.Devices, config.FolderDeviceConfiguration{
 			DeviceID: deviceCfg.DeviceID,
 		})
-		w, _ := m.cfg.SetFolder(cfg)
-		w.Wait()
 		l.Infof("Shared %s with %s due to auto-accept", folder.ID, deviceCfg.DeviceID)
-		return true
+		return cfg, true
 	}
 }