diff --git a/lib/model/model.go b/lib/model/model.go index 5527ca664..d57310889 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -1096,15 +1096,17 @@ func (m *Model) handleIntroductions(introducerCfg config.DeviceConfiguration, cm foldersDevices := make(folderDeviceSet) for _, folder := range cm.Folders { - // We don't have this folder, skip. - if _, ok := m.folderDevices[folder.ID]; !ok { - continue - } - // Adds devices which we do not have, but the introducer has // for the folders that we have in common. Also, shares folders // with devices that we have in common, yet are currently not sharing // the folder. + + fcfg, ok := m.cfg.Folder(folder.ID) + if !ok { + // Don't have this folder, carry on. + continue + } + nextDevice: for _, device := range folder.Devices { // No need to share with self. @@ -1118,22 +1120,29 @@ func (m *Model) handleIntroductions(introducerCfg config.DeviceConfiguration, cm // The device is currently unknown. Add it to the config. m.introduceDevice(device, introducerCfg) changed = true - } - - for _, er := range m.deviceFolders[device.ID] { - if er == folder.ID { - // We already share the folder with this device, so - // nothing to do. - continue nextDevice + } else { + for _, dev := range fcfg.DeviceIDs() { + if dev == device.ID { + // We already share the folder with this device, so + // nothing to do. + continue nextDevice + } } } // We don't yet share this folder with this device. Add the device // to sharing list of the folder. l.Infof("Sharing folder %s with %v (vouched for by introducer %v)", folder.Description(), device.ID, introducerCfg.DeviceID) - m.shareFolderWithDeviceLocked(device.ID, folder.ID, introducerCfg.DeviceID) + fcfg.Devices = append(fcfg.Devices, config.FolderDeviceConfiguration{ + DeviceID: device.ID, + IntroducedBy: introducerCfg.DeviceID, + }) changed = true } + + if changed { + m.cfg.SetFolder(fcfg) + } } return foldersDevices, changed @@ -1195,7 +1204,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 { - if _, ok := m.cfg.Folder(folder.ID); !ok { + if cfg, ok := m.cfg.Folder(folder.ID); !ok { defaultPath := m.cfg.Options().DefaultFolderPath defaultPathFs := fs.NewFilesystem(fs.FilesystemTypeBasic, defaultPath) for _, path := range []string{folder.Label, folder.ID} { @@ -1204,7 +1213,9 @@ func (m *Model) handleAutoAccepts(deviceCfg config.DeviceConfiguration, folder p } fcfg := config.NewFolderConfiguration(m.id, folder.ID, folder.Label, fs.FilesystemTypeBasic, filepath.Join(defaultPath, path)) - + 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 @@ -1212,29 +1223,26 @@ func (m *Model) handleAutoAccepts(deviceCfg config.DeviceConfiguration, folder p w, _ := m.cfg.SetFolder(fcfg) w.Wait() - // This needs to happen under a lock. - m.fmut.Lock() - w = m.shareFolderWithDeviceLocked(deviceCfg.DeviceID, folder.ID, protocol.DeviceID{}) - m.fmut.Unlock() - w.Wait() l.Infof("Auto-accepted %s folder %s at path %s", deviceCfg.DeviceID, folder.Description(), fcfg.Path) return true } l.Infof("Failed to auto-accept folder %s from %s due to path conflict", folder.Description(), deviceCfg.DeviceID) return false + } else { + for _, device := range cfg.DeviceIDs() { + if device == deviceCfg.DeviceID { + // Already shared nothing todo. + return 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 } - - // Folder does not exist yet. - if m.folderSharedWith(folder.ID, deviceCfg.DeviceID) { - return false - } - - m.fmut.Lock() - w := m.shareFolderWithDeviceLocked(deviceCfg.DeviceID, folder.ID, protocol.DeviceID{}) - m.fmut.Unlock() - w.Wait() - l.Infof("Shared %s with %s due to auto-accept", folder.ID, deviceCfg.DeviceID) - return true } func (m *Model) introduceDevice(device protocol.Device, introducerCfg config.DeviceConfiguration) { @@ -1265,19 +1273,6 @@ func (m *Model) introduceDevice(device protocol.Device, introducerCfg config.Dev m.cfg.SetDevice(newDeviceCfg) } -func (m *Model) shareFolderWithDeviceLocked(deviceID protocol.DeviceID, folder string, introducer protocol.DeviceID) config.Waiter { - m.deviceFolders[deviceID] = append(m.deviceFolders[deviceID], folder) - m.folderDevices.set(deviceID, folder) - - folderCfg := m.cfg.Folders()[folder] - folderCfg.Devices = append(folderCfg.Devices, config.FolderDeviceConfiguration{ - DeviceID: deviceID, - IntroducedBy: introducer, - }) - w, _ := m.cfg.SetFolder(folderCfg) - return w -} - // Closed is called when a connection has been closed func (m *Model) Closed(conn protocol.Connection, err error) { device := conn.ID() diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 6414b2f6a..110b1d9f3 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -60,6 +60,9 @@ func init() { defaultConfig = config.Wrap("/tmp/test", _defaultConfig) defaultAutoAcceptCfg = config.Configuration{ Devices: []config.DeviceConfiguration{ + { + DeviceID: protocol.LocalDeviceID, // self + }, { DeviceID: device1, AutoAcceptFolders: true, @@ -110,7 +113,9 @@ func newState(cfg config.Configuration) (*config.Wrapper, *Model) { m := NewModel(wcfg, protocol.LocalDeviceID, "syncthing", "dev", db, nil) for _, folder := range cfg.Folders { - m.AddFolder(folder) + if !folder.Paused { + m.AddFolder(folder) + } } m.ServeBackground() m.AddConnection(&fakeConnection{id: device1}, protocol.HelloResult{}) @@ -990,7 +995,9 @@ func TestIntroducer(t *testing.T) { func TestAutoAcceptRejected(t *testing.T) { // Nothing happens if AutoAcceptFolders not set tcfg := defaultAutoAcceptCfg.Copy() - tcfg.Devices[0].AutoAcceptFolders = false + for i := range tcfg.Devices { + tcfg.Devices[i].AutoAcceptFolders = false + } wcfg, m := newState(tcfg) id := srand.String(8) defer os.RemoveAll(filepath.Join("testdata", id)) @@ -1058,6 +1065,7 @@ func TestAutoAcceptExistingFolder(t *testing.T) { id := srand.String(8) idOther := srand.String(8) // To check that path does not get changed. defer os.RemoveAll(filepath.Join("testdata", id)) + defer os.RemoveAll(filepath.Join("testdata", idOther)) tcfg := defaultAutoAcceptCfg.Copy() tcfg.Folders = []config.FolderConfiguration{ @@ -1218,6 +1226,115 @@ func TestAutoAcceptFallsBackToID(t *testing.T) { } } +func TestAutoAcceptPausedWhenFolderConfigChanged(t *testing.T) { + // Existing folder + id := srand.String(8) + idOther := srand.String(8) // To check that path does not get changed. + defer os.RemoveAll(filepath.Join("testdata", id)) + defer os.RemoveAll(filepath.Join("testdata", idOther)) + + tcfg := defaultAutoAcceptCfg.Copy() + fcfg := config.NewFolderConfiguration(protocol.LocalDeviceID, id, "", fs.FilesystemTypeBasic, filepath.Join("testdata", idOther)) + fcfg.Paused = true + // The order of devices here is wrong (cfg.clean() sorts them), which will cause the folder to restart. + // Because of the restart, folder gets removed from m.deviceFolder, which means that generateClusterConfig will not panic. + // This wasn't an issue before, yet keeping the test case to prove that it still isn't. + fcfg.Devices = append(fcfg.Devices, config.FolderDeviceConfiguration{ + DeviceID: device1, + }) + tcfg.Folders = []config.FolderConfiguration{fcfg} + wcfg, m := newState(tcfg) + if _, ok := wcfg.Folder(id); !ok || m.folderSharedWith(id, device1) { + t.Error("missing folder, or shared", id) + } + if _, ok := m.folderRunners[id]; ok { + t.Fatal("folder running?") + } + + m.ClusterConfig(device1, protocol.ClusterConfig{ + Folders: []protocol.Folder{ + { + ID: id, + Label: id, + }, + }, + }) + m.generateClusterConfig(device1) + + if fcfg, ok := wcfg.Folder(id); !ok { + t.Error("missing folder") + } else if fcfg.Path != filepath.Join("testdata", idOther) { + t.Error("folder path changed") + } else { + for _, dev := range fcfg.DeviceIDs() { + if dev == device1 { + return + } + } + t.Error("device missing") + } + + if _, ok := m.folderDevices[id]; ok { + t.Error("folder started") + } +} + +func TestAutoAcceptPausedWhenFolderConfigNotChanged(t *testing.T) { + // Existing folder + id := srand.String(8) + idOther := srand.String(8) // To check that path does not get changed. + defer os.RemoveAll(filepath.Join("testdata", id)) + defer os.RemoveAll(filepath.Join("testdata", idOther)) + + tcfg := defaultAutoAcceptCfg.Copy() + fcfg := config.NewFolderConfiguration(protocol.LocalDeviceID, id, "", fs.FilesystemTypeBasic, filepath.Join("testdata", idOther)) + fcfg.Paused = true + // The new folder is exactly the same as the one constructed by handleAutoAccept, which means + // the folder will not be restarted (even if it's paused), yet handleAutoAccept used to add the folder + // to m.deviceFolders which had caused panics when calling generateClusterConfig, as the folder + // did not have a file set. + fcfg.Devices = append([]config.FolderDeviceConfiguration{ + { + DeviceID: device1, + }, + }, fcfg.Devices...) // Need to ensure this device order to avoid folder restart. + tcfg.Folders = []config.FolderConfiguration{fcfg} + wcfg, m := newState(tcfg) + if _, ok := wcfg.Folder(id); !ok || m.folderSharedWith(id, device1) { + t.Error("missing folder, or shared", id) + } + if _, ok := m.folderRunners[id]; ok { + t.Fatal("folder running?") + } + + m.ClusterConfig(device1, protocol.ClusterConfig{ + Folders: []protocol.Folder{ + { + ID: id, + Label: id, + }, + }, + }) + m.generateClusterConfig(device1) + + if fcfg, ok := wcfg.Folder(id); !ok { + t.Error("missing folder") + } else if fcfg.Path != filepath.Join("testdata", idOther) { + t.Error("folder path changed") + } else { + for _, dev := range fcfg.DeviceIDs() { + if dev == device1 { + return + } + } + t.Error("device missing") + } + + if _, ok := m.folderDevices[id]; ok { + t.Error("folder started") + } +} + func changeIgnores(t *testing.T, m *Model, expected []string) { arrEqual := func(a, b []string) bool { if len(a) != len(b) {