From aec66045ef1845fffa2c2f72dbd2dd219b76eb1a Mon Sep 17 00:00:00 2001 From: Audrius Butkevicius Date: Sat, 25 Aug 2018 11:36:10 +0100 Subject: [PATCH] lib/config: Rewrite pending notifications (fixes #2291) --- gui/default/index.html | 154 +++++++++--------- .../syncthing/core/syncthingController.js | 138 +++++++++++----- .../syncthing/device/editDeviceModalView.html | 2 +- .../device/globalChangesModalView.html | 4 +- .../settings/discardChangesConfirmation.html | 16 ++ .../syncthing/settings/settingsModalView.html | 84 ++++++++++ lib/config/config.go | 107 +++++++----- lib/config/config_test.go | 140 ++++++++++++++-- lib/config/deviceconfiguration.go | 64 ++++++-- lib/config/folderconfiguration.go | 28 ---- lib/config/observed.go | 26 +++ lib/config/testdata/ignoreddevices.xml | 4 +- lib/config/testdata/ignoredfolders.xml | 14 ++ lib/config/wrapper.go | 66 +++++++- lib/model/model.go | 13 +- 15 files changed, 630 insertions(+), 230 deletions(-) create mode 100644 gui/default/syncthing/settings/discardChangesConfirmation.html create mode 100644 lib/config/observed.go create mode 100644 lib/config/testdata/ignoredfolders.xml diff --git a/gui/default/index.html b/gui/default/index.html index c2a10c0eb..9aa7ea8a1 100644 --- a/gui/default/index.html +++ b/gui/default/index.html @@ -145,87 +145,90 @@ - +
-
-
-
-
-

- - New Device - {{ event.time | date:"yyyy-MM-dd HH:mm:ss" }} -

-
-
-

- - Device "{%name%}" ({%device%} at {%address%}) wants to connect. Add new device? - -

-
-
-
- - - + + +
+ + + +
+
+
+
+

+ + New Device + {{ pendingDevice.time | date:"yyyy-MM-dd HH:mm:ss" }} +

-
-
-
-
- - - -
-
-
-
-

-
- +
+

+ + Device "{%name%}" ({%device%} at {%address%}) wants to connect. Add new device? + +

+
+
+
+ +
- New Folder - Share Folder - {{ event.time | date:"yyyy-MM-dd HH:mm:ss" }} -

-
-
-

- - {%device%} wants to share folder "{%folder%}". - - - {%device%} wants to share folder "{%folderlabel%}" ({%folder%}). - - Share this folder? - Add new folder? -

-
-
-
- - - -
+ + +
+
+
+
+
+

+
+ +
+ New Folder + Share Folder + {{ pendingFolder.time | date:"yyyy-MM-dd HH:mm:ss" }} +

+
+
+

+ + {%device%} wants to share folder "{%folder%}". + + + {%device%} wants to share folder "{%folderlabel%}" ({%folder%}). + + Share this folder? + Add new folder? +

+
+
+
+ + + +
+
+
+
+
+
+
@@ -284,8 +287,6 @@
-
-
@@ -786,6 +787,7 @@ + diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js index e5759038e..dd6716753 100755 --- a/gui/default/syncthing/core/syncthingController.js +++ b/gui/default/syncthing/core/syncthingController.js @@ -28,9 +28,7 @@ angular.module('syncthing.core') $scope.model = {}; $scope.myID = ''; $scope.devices = []; - $scope.deviceRejections = {}; $scope.discoveryCache = {}; - $scope.folderRejections = {}; $scope.protocolChanged = false; $scope.reportData = {}; $scope.reportDataPreview = ''; @@ -260,14 +258,6 @@ angular.module('syncthing.core') } }); - $scope.$on(Events.DEVICE_REJECTED, function (event, arg) { - $scope.deviceRejections[arg.data.device] = arg; - }); - - $scope.$on(Events.FOLDER_REJECTED, function (event, arg) { - $scope.folderRejections[arg.data.folder + "-" + arg.data.device] = arg; - }); - $scope.$on(Events.CONFIG_SAVED, function (event, arg) { updateLocalConfig(arg.data); @@ -967,6 +957,7 @@ angular.module('syncthing.core') // loop through all devices var deviceCount = $scope.devices.length; + var pendingFolders = 0; for (var i = 0; i < $scope.devices.length; i++) { var status = $scope.deviceStatus({ deviceID:$scope.devices[i].deviceID @@ -982,10 +973,13 @@ angular.module('syncthing.core') deviceCount--; break; } + pendingFolders += $scope.devices[i].pendingFolders.length; } // enumerate notifications - if ($scope.openNoAuth || !$scope.configInSync || $scope.sizeOf($scope.deviceRejections) > 0 || $scope.sizeOf($scope.folderRejections) > 0 || $scope.errorList().length > 0 || !online) { + if ($scope.openNoAuth || !$scope.configInSync || $scope.errorList().length > 0 || !online || ( + !isEmptyObject($scope.config) && ($scope.config.pendingDevices.length > 0 || pendingFolders > 0) + )) { notifyCount++; } @@ -1025,6 +1019,14 @@ angular.module('syncthing.core') return matches[0].name; }; + $scope.friendlyNameFromID = function (deviceID) { + var match = $scope.findDevice(deviceID); + if (match) { + return $scope.deviceName(match); + } + return deviceID.substr(0, 6); + }; + $scope.findDevice = function (deviceID) { var matches = $scope.devices.filter(function (n) { return n.deviceID === deviceID; @@ -1178,8 +1180,24 @@ angular.module('syncthing.core') $scope.tmpOptions.upgrades = "candidate"; } $scope.tmpGUI = angular.copy($scope.config.gui); - $('#settings').modal().one('hidden.bs.modal', function () { + $scope.tmpRemoteIgnoredDevices = angular.copy($scope.config.remoteIgnoredDevices); + $scope.tmpDevices = angular.copy($scope.config.devices); + var settingsModal = $('#settings').modal(); + settingsModal.one('hidden.bs.modal', function () { + $('.nav-tabs a[href="#settings-general"]').tab('show'); window.location.hash = ""; + settingsModal.off('hide.bs.modal'); + }).on('hide.bs.modal', function (e) { + if ($scope.settingsModified()) { + $("#discard-changes-confirmation").modal().one('hidden.bs.modal', function() { + if (!$scope.settingsModified()) { + settingsModal.modal('hide'); + } + }); + e.preventDefault(); + e.stopImmediatePropagation(); + return false; + } }); }; @@ -1210,11 +1228,30 @@ angular.module('syncthing.core') return result; }; + $scope.settingsModified = function () { + // Options has artificial properties injected into the temp config. + // Need to recompute them before we can check equality + var options = angular.copy($scope.config.options); + options.deviceName = $scope.thisDevice().name; + options.upgrades = "none"; + if (options.autoUpgradeIntervalH > 0) { + options.upgrades = "stable"; + } + if (options.upgradeToPreReleases) { + options.upgrades = "candidate"; + } + var optionsEqual = angular.equals(options, $scope.tmpOptions); + var guiEquals = angular.equals($scope.config.gui, $scope.tmpGUI); + var ignoredDevicesEquals = angular.equals($scope.config.remoteIgnoredDevices, $scope.tmpRemoteIgnoredDevices); + var ignoredFoldersEquals = angular.equals($scope.config.devices, $scope.tmpDevices); + console.log("settings equals - options: " + optionsEqual + " gui: " + guiEquals + " ignDev: " + ignoredDevicesEquals + " ignFol: " + ignoredFoldersEquals); + return !optionsEqual || !guiEquals || !ignoredDevicesEquals || !ignoredFoldersEquals; + } + $scope.saveSettings = function () { // Make sure something changed - var changed = !angular.equals($scope.config.options, $scope.tmpOptions) || !angular.equals($scope.config.gui, $scope.tmpGUI); - var themeChanged = $scope.config.gui.theme !== $scope.tmpGUI.theme; - if (changed) { + if ($scope.settingsModified()) { + var themeChanged = $scope.config.gui.theme !== $scope.tmpGUI.theme; // Angular has issues with selects with numeric values, so we handle strings here. $scope.tmpOptions.urAccepted = parseInt($scope.tmpOptions._urAcceptedStr); // Check if auto-upgrade has been enabled or disabled. This @@ -1241,6 +1278,8 @@ angular.module('syncthing.core') $scope.thisDevice().name = $scope.tmpOptions.deviceName; $scope.config.options = angular.copy($scope.tmpOptions); $scope.config.gui = angular.copy($scope.tmpGUI); + $scope.config.remoteIgnoredDevices = angular.copy($scope.tmpRemoteIgnoredDevices); + $scope.config.devices = angular.copy($scope.tmpDevices); ['listenAddresses', 'globalAnnounceServers'].forEach(function (key) { $scope.config.options[key] = $scope.config.options["_" + key + "Str"].split(/[ ,]+/).map(function (x) { @@ -1367,11 +1406,6 @@ angular.module('syncthing.core') return n.deviceID !== $scope.currentDevice.deviceID; }); $scope.config.devices = $scope.devices; - // In case we later added the device manually, remove the ignoral - // record. - $scope.config.ignoredDevices = $scope.config.ignoredDevices.filter(function (id) { - return id !== $scope.currentDevice.deviceID; - }); for (var id in $scope.folders) { $scope.folders[id].devices = $scope.folders[id].devices.filter(function (n) { @@ -1385,7 +1419,6 @@ angular.module('syncthing.core') $scope.saveDevice = function () { $('#editDevice').modal('hide'); $scope.saveDeviceConfig($scope.currentDevice); - $scope.dismissDeviceRejection($scope.currentDevice.deviceID); }; $scope.saveDeviceConfig = function (deviceCfg) { @@ -1407,11 +1440,6 @@ angular.module('syncthing.core') $scope.devices.sort(deviceCompare); $scope.config.devices = $scope.devices; - // In case we are adding the device manually, remove the ignoral - // record. - $scope.config.ignoredDevices = $scope.config.ignoredDevices.filter(function (id) { - return id !== deviceCfg.deviceID; - }); for (var id in deviceCfg.selectedFolders) { if (deviceCfg.selectedFolders[id]) { @@ -1438,14 +1466,37 @@ angular.module('syncthing.core') $scope.saveConfig(); }; - $scope.dismissDeviceRejection = function (device) { - delete $scope.deviceRejections[device]; + $scope.ignoreDevice = function (pendingDevice) { + pendingDevice = angular.copy(pendingDevice); + // Bump time + pendingDevice.time = (new Date()).toISOString(); + $scope.config.remoteIgnoredDevices.push(pendingDevice); + $scope.saveConfig(); }; - $scope.ignoreRejectedDevice = function (device) { - $scope.config.ignoredDevices.push(device); - $scope.saveConfig(); - $scope.dismissDeviceRejection(device); + $scope.unignoreDeviceFromTemporaryConfig = function (ignoredDevice) { + $scope.tmpRemoteIgnoredDevices = $scope.tmpRemoteIgnoredDevices.filter(function (existingIgnoredDevice) { + return ignoredDevice.deviceID !== existingIgnoredDevice.deviceID; + }); + }; + + $scope.ignoredFoldersCountTmpConfig = function () { + var count = 0; + ($scope.tmpDevices || []).forEach(function (deviceCfg) { + count += deviceCfg.ignoredFolders.length; + }); + return count; + }; + + $scope.unignoreFolderFromTemporaryConfig = function (device, ignoredFolderID) { + for (var i = 0; i < $scope.tmpDevices.length; i++) { + if ($scope.tmpDevices[i].deviceID == device) { + $scope.tmpDevices[i].ignoredFolders = $scope.tmpDevices[i].ignoredFolders.filter(function (existingIgnoredFolder) { + return existingIgnoredFolder.id !== ignoredFolderID; + }); + return; + } + } }; $scope.otherDevices = function () { @@ -1565,6 +1616,7 @@ angular.module('syncthing.core') $('#folder-ignores textarea').focus(); } }).one('hidden.bs.modal', function () { + $('.nav-tabs a[href="#folder-general"]').tab('show'); window.location.hash = ""; }); }; @@ -1641,7 +1693,6 @@ angular.module('syncthing.core') }; $scope.addFolderAndShare = function (folder, folderLabel, device) { - $scope.dismissFolderRejection(folder, device); $scope.editingExisting = false; $scope.currentFolder = angular.copy($scope.folderDefaults); $scope.currentFolder.id = folder; @@ -1661,7 +1712,6 @@ angular.module('syncthing.core') }); $scope.config.folders = folderList($scope.folders); $scope.saveConfig(); - $scope.dismissFolderRejection(folder, device); }; $scope.saveFolder = function () { @@ -1749,14 +1799,18 @@ angular.module('syncthing.core') }); }; - $scope.dismissFolderRejection = function (folder, device) { - delete $scope.folderRejections[folder + "-" + device]; - }; + $scope.ignoreFolder = function (device, pendingFolder) { + pendingFolder = angular.copy(pendingFolder); + // Bump time + pendingFolder.time = (new Date()).toISOString(); - $scope.ignoreRejectedFolder = function (folder, device) { - $scope.config.ignoredFolders.push(folder); - $scope.saveConfig(); - $scope.dismissFolderRejection(folder, device); + for (var i = 0; i < $scope.devices.length; i++) { + if ($scope.devices[i].deviceID == device) { + $scope.devices[i].ignoredFolders.push(pendingFolder); + $scope.saveConfig(); + return; + } + } }; $scope.sharesFolder = function (folderCfg) { @@ -1788,7 +1842,7 @@ angular.module('syncthing.core') return folderID; } var label = $scope.folders[folderID].label; - return label.length > 0 ? label : folderID; + return label && label.length > 0 ? label : folderID; } $scope.deleteFolder = function (id) { diff --git a/gui/default/syncthing/device/editDeviceModalView.html b/gui/default/syncthing/device/editDeviceModalView.html index c100c5d28..6a0bb69ab 100644 --- a/gui/default/syncthing/device/editDeviceModalView.html +++ b/gui/default/syncthing/device/editDeviceModalView.html @@ -1,7 +1,7 @@
-
    +
    • General
    • Sharing
    • Advanced
      • diff --git a/gui/default/syncthing/device/globalChangesModalView.html b/gui/default/syncthing/device/globalChangesModalView.html index 83f7f4e6e..8ea6a8bce 100644 --- a/gui/default/syncthing/device/globalChangesModalView.html +++ b/gui/default/syncthing/device/globalChangesModalView.html @@ -28,8 +28,8 @@
- +
diff --git a/gui/default/syncthing/settings/discardChangesConfirmation.html b/gui/default/syncthing/settings/discardChangesConfirmation.html new file mode 100644 index 000000000..42859d8c1 --- /dev/null +++ b/gui/default/syncthing/settings/discardChangesConfirmation.html @@ -0,0 +1,16 @@ + +
+

+ You have unsaved changes. Do you really want to discard them? +

+
+
+ + + +
+ diff --git a/gui/default/syncthing/settings/settingsModalView.html b/gui/default/syncthing/settings/settingsModalView.html index 7ad111c4c..9f208efa6 100644 --- a/gui/default/syncthing/settings/settingsModalView.html +++ b/gui/default/syncthing/settings/settingsModalView.html @@ -5,6 +5,24 @@
  • General
  • GUI
  • Connections
    • +
  • + + +   + Ignored Devices +   + {{ tmpRemoteIgnoredDevices.length }} + +
    • +
  • + + +   + Ignored Folders +   + {{ ignoredFoldersCountTmpConfig() }} + +
    • @@ -228,6 +246,72 @@
    +
    +
    + You have no ignored devices. +
    + + + + + + + + + + + + + + + + + +
    Ignored atDeviceAddress
    {{ ignoredDevice.time | date:"yyyy-MM-dd HH:mm:ss" }} + {{ ignoredDevice.deviceID }} + {{ ignoredDevice.name }} + {{ ignoredDevice.address }} + +  Unignore + +
    +
    +
    +
    + +
    +
    + You have no ignored folders. +
    + + + + + + + + + + + + + + + + + + +
    Ignored atFolderDevice
    {{ ignoredFolder.time | date:"yyyy-MM-dd HH:mm:ss" }}{{ folderLabel(ignoredFolder.id) }} + {{ friendlyNameFromID(device.deviceID) }} + + +  Unignore + +
    +
    +
    +
    +
    diff --git a/lib/config/config.go b/lib/config/config.go index 2fcb6a747..0e497dde1 100644 --- a/lib/config/config.go +++ b/lib/config/config.go @@ -130,8 +130,8 @@ type Configuration struct { Devices []DeviceConfiguration `xml:"device" json:"devices"` GUI GUIConfiguration `xml:"gui" json:"gui"` Options OptionsConfiguration `xml:"options" json:"options"` - IgnoredDevices []protocol.DeviceID `xml:"ignoredDevice" json:"ignoredDevices"` - IgnoredFolders []string `xml:"ignoredFolder" json:"ignoredFolders"` + IgnoredDevices []ObservedDevice `xml:"remoteIgnoredDevice" json:"remoteIgnoredDevices"` + PendingDevices []ObservedDevice `xml:"pendingDevice" json:"pendingDevices"` XMLName xml.Name `xml:"configuration" json:"-"` MyID protocol.DeviceID `xml:"-" json:"-"` // Provided by the instantiator. @@ -157,12 +157,11 @@ func (cfg Configuration) Copy() Configuration { newCfg.GUI = cfg.GUI.Copy() // DeviceIDs are values - newCfg.IgnoredDevices = make([]protocol.DeviceID, len(cfg.IgnoredDevices)) + newCfg.IgnoredDevices = make([]ObservedDevice, len(cfg.IgnoredDevices)) copy(newCfg.IgnoredDevices, cfg.IgnoredDevices) - // FolderConfiguraion.ID is type string - newCfg.IgnoredFolders = make([]string, len(cfg.IgnoredFolders)) - copy(newCfg.IgnoredFolders, cfg.IgnoredFolders) + newCfg.PendingDevices = make([]ObservedDevice, len(cfg.PendingDevices)) + copy(newCfg.PendingDevices, cfg.PendingDevices) return newCfg } @@ -213,28 +212,11 @@ found: func (cfg *Configuration) clean() error { util.FillNilSlices(&cfg.Options) - // Initialize any empty slices - if cfg.Folders == nil { - cfg.Folders = []FolderConfiguration{} - } - if cfg.IgnoredDevices == nil { - cfg.IgnoredDevices = []protocol.DeviceID{} - } - if cfg.IgnoredFolders == nil { - cfg.IgnoredFolders = []string{} - } - if cfg.Options.AlwaysLocalNets == nil { - cfg.Options.AlwaysLocalNets = []string{} - } - if cfg.Options.UnackedNotificationIDs == nil { - cfg.Options.UnackedNotificationIDs = []string{} - } - // Prepare folders and check for duplicates. Duplicates are bad and // dangerous, can't currently be resolved in the GUI, and shouldn't // happen when configured by the GUI. We return with an error in that // situation. - seenFolders := make(map[string]struct{}) + existingFolders := make(map[string]*FolderConfiguration) for i := range cfg.Folders { folder := &cfg.Folders[i] folder.prepare() @@ -243,18 +225,10 @@ func (cfg *Configuration) clean() error { return fmt.Errorf("folder with empty ID in configuration") } - if _, ok := seenFolders[folder.ID]; ok { + if _, ok := existingFolders[folder.ID]; ok { return fmt.Errorf("duplicate folder ID %q in configuration", folder.ID) } - seenFolders[folder.ID] = struct{}{} - } - - // Remove ignored folders that are anyway part of the configuration. - for i := 0; i < len(cfg.IgnoredFolders); i++ { - if _, ok := seenFolders[cfg.IgnoredFolders[i]]; ok { - cfg.IgnoredFolders = append(cfg.IgnoredFolders[:i], cfg.IgnoredFolders[i+1:]...) - i-- // IgnoredFolders[i] now points to something else, so needs to be rechecked - } + existingFolders[folder.ID] = folder } cfg.Options.ListenAddresses = util.UniqueStrings(cfg.Options.ListenAddresses) @@ -330,25 +304,36 @@ func (cfg *Configuration) clean() error { // - free from duplicates // - sorted by ID cfg.Devices = ensureNoDuplicateDevices(cfg.Devices) - sort.Sort(DeviceConfigurationList(cfg.Devices)) + sort.Slice(cfg.Devices, func(a, b int) bool { + return cfg.Devices[a].DeviceID.Compare(cfg.Devices[b].DeviceID) == -1 + }) // Ensure that the folder list is sorted by ID - sort.Sort(FolderConfigurationList(cfg.Folders)) + sort.Slice(cfg.Folders, func(a, b int) bool { + return cfg.Folders[a].ID < cfg.Folders[b].ID + }) + // Ensure that in all folder configs // - any loose devices are not present in the wrong places // - there are no duplicate devices // - the versioning configuration parameter map is not nil + sharedFolders := make(map[protocol.DeviceID][]string, len(cfg.Devices)) for i := range cfg.Folders { cfg.Folders[i].Devices = ensureExistingDevices(cfg.Folders[i].Devices, existingDevices) cfg.Folders[i].Devices = ensureNoDuplicateFolderDevices(cfg.Folders[i].Devices) if cfg.Folders[i].Versioning.Params == nil { cfg.Folders[i].Versioning.Params = map[string]string{} } - sort.Sort(FolderDeviceConfigurationList(cfg.Folders[i].Devices)) + sort.Slice(cfg.Folders[i].Devices, func(a, b int) bool { + return cfg.Folders[i].Devices[a].DeviceID.Compare(cfg.Folders[i].Devices[b].DeviceID) == -1 + }) + for _, dev := range cfg.Folders[i].Devices { + sharedFolders[dev.DeviceID] = append(sharedFolders[dev.DeviceID], cfg.Folders[i].ID) + } } for i := range cfg.Devices { - cfg.Devices[i].prepare() + cfg.Devices[i].prepare(sharedFolders[cfg.Devices[i].DeviceID]) } // Very short reconnection intervals are annoying @@ -362,14 +347,39 @@ func (cfg *Configuration) clean() error { // The list of ignored devices should not contain any devices that have // been manually added to the config. - newIgnoredDevices := []protocol.DeviceID{} + var newIgnoredDevices []ObservedDevice + ignoredDevices := make(map[protocol.DeviceID]bool) for _, dev := range cfg.IgnoredDevices { - if !existingDevices[dev] { + if !existingDevices[dev.ID] { + ignoredDevices[dev.ID] = true newIgnoredDevices = append(newIgnoredDevices, dev) } } cfg.IgnoredDevices = newIgnoredDevices + // The list of pending devices should not contain devices that were added manually, nor should it contain + // ignored devices. + + // Sort by time, so that in case of duplicates latest "time" is used. + sort.Slice(cfg.PendingDevices, func(i, j int) bool { + return cfg.PendingDevices[i].Time.Before(cfg.PendingDevices[j].Time) + }) + + var newPendingDevices []ObservedDevice +nextPendingDevice: + for _, pendingDevice := range cfg.PendingDevices { + if !existingDevices[pendingDevice.ID] && !ignoredDevices[pendingDevice.ID] { + // Deduplicate + for _, existingPendingDevice := range newPendingDevices { + if existingPendingDevice.ID == pendingDevice.ID { + continue nextPendingDevice + } + } + newPendingDevices = append(newPendingDevices, pendingDevice) + } + } + cfg.PendingDevices = newPendingDevices + // Deprecated protocols are removed from the list of listeners and // device addresses. So far just kcp*. for _, prefix := range []string{"kcp"} { @@ -380,6 +390,23 @@ func (cfg *Configuration) clean() error { } } + // Initialize any empty slices + if cfg.Folders == nil { + cfg.Folders = []FolderConfiguration{} + } + if cfg.IgnoredDevices == nil { + cfg.IgnoredDevices = []ObservedDevice{} + } + if cfg.PendingDevices == nil { + cfg.PendingDevices = []ObservedDevice{} + } + if cfg.Options.AlwaysLocalNets == nil { + cfg.Options.AlwaysLocalNets = []string{} + } + if cfg.Options.UnackedNotificationIDs == nil { + cfg.Options.UnackedNotificationIDs = []string{} + } + return nil } diff --git a/lib/config/config_test.go b/lib/config/config_test.go index 74716692c..dd9946436 100644 --- a/lib/config/config_test.go +++ b/lib/config/config_test.go @@ -132,6 +132,8 @@ func TestDeviceConfig(t *testing.T) { Addresses: []string{"tcp://a"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, { DeviceID: device4, @@ -139,6 +141,8 @@ func TestDeviceConfig(t *testing.T) { Addresses: []string{"tcp://b"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, } expectedDeviceIDs := []protocol.DeviceID{device1, device4} @@ -230,16 +234,22 @@ func TestDeviceAddressesDynamic(t *testing.T) { DeviceID: device1, Addresses: []string{"dynamic"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device2: { DeviceID: device2, Addresses: []string{"dynamic"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device3: { DeviceID: device3, Addresses: []string{"dynamic"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device4: { DeviceID: device4, @@ -247,6 +257,8 @@ func TestDeviceAddressesDynamic(t *testing.T) { Addresses: []string{"dynamic"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, } @@ -269,18 +281,24 @@ func TestDeviceCompression(t *testing.T) { Addresses: []string{"dynamic"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device2: { DeviceID: device2, Addresses: []string{"dynamic"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device3: { DeviceID: device3, Addresses: []string{"dynamic"}, Compression: protocol.CompressNever, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device4: { DeviceID: device4, @@ -288,6 +306,8 @@ func TestDeviceCompression(t *testing.T) { Addresses: []string{"dynamic"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, } @@ -309,16 +329,22 @@ func TestDeviceAddressesStatic(t *testing.T) { DeviceID: device1, Addresses: []string{"tcp://192.0.2.1", "tcp://192.0.2.2"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device2: { DeviceID: device2, Addresses: []string{"tcp://192.0.2.3:6070", "tcp://[2001:db8::42]:4242"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device3: { DeviceID: device3, Addresses: []string{"tcp://[2001:db8::44]:4444", "tcp://192.0.2.4:6090"}, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, device4: { DeviceID: device4, @@ -326,6 +352,8 @@ func TestDeviceAddressesStatic(t *testing.T) { Addresses: []string{"dynamic"}, Compression: protocol.CompressMetadata, AllowedNetworks: []string{}, + IgnoredFolders: []ObservedFolder{}, + PendingFolders: []ObservedFolder{}, }, } @@ -560,8 +588,8 @@ func TestCopy(t *testing.T) { t.Error("Config should have changed") } if !bytes.Equal(bsOrig, bsCopy) { - //ioutil.WriteFile("a", bsOrig, 0644) - //ioutil.WriteFile("b", bsCopy, 0644) + // ioutil.WriteFile("a", bsOrig, 0644) + // ioutil.WriteFile("b", bsCopy, 0644) t.Error("Copy should be unchanged") } } @@ -697,7 +725,6 @@ func TestEmptyFolderPaths(t *testing.T) { func TestV14ListenAddressesMigration(t *testing.T) { tcs := [][3][]string{ - // Default listen plus default relays is now "default" { {"tcp://0.0.0.0:22000"}, @@ -710,7 +737,7 @@ func TestV14ListenAddressesMigration(t *testing.T) { // config to start with... { {"tcp://0.0.0.0:22000"}, // old listen addrs - {""}, // old relay addrs + {""}, // old relay addrs {"tcp://0.0.0.0:22000"}, // new listen addrs }, // Default listen plus non-default relays gets copied verbatim @@ -770,6 +797,44 @@ func TestIgnoredDevices(t *testing.T) { } } +func TestIgnoredFolders(t *testing.T) { + // Verify that ignored folder that are also present in the + // configuration are not in fact ignored. + // Also, verify that folders that are shared with a device are not ignored. + + wrapper, err := Load("testdata/ignoredfolders.xml", device1) + if err != nil { + t.Fatal(err) + } + + if wrapper.IgnoredFolder(device2, "folder1") { + t.Errorf("Device %v should not be ignored", device2) + } + if !wrapper.IgnoredFolder(device3, "folder1") { + t.Errorf("Device %v should be ignored", device3) + } + // Should be removed, hence not ignored. + if wrapper.IgnoredFolder(device4, "folder1") { + t.Errorf("Device %v should not be ignored", device4) + } + + if !wrapper.IgnoredFolder(device2, "folder2") { + t.Errorf("Device %v should not be ignored", device2) + } + if !wrapper.IgnoredFolder(device3, "folder2") { + t.Errorf("Device %v should be ignored", device3) + } + + // 2 for folder2, 1 for folder1, as non-existing device and device the folder is shared with is removed. + expectedIgnoredFolders := 3 + for _, dev := range wrapper.cfg.Devices { + expectedIgnoredFolders -= len(dev.IgnoredFolders) + } + if expectedIgnoredFolders != 0 { + t.Errorf("Left with %d ignored folders", expectedIgnoredFolders) + } +} + func TestGetDevice(t *testing.T) { // Verify that the Device() call does the right thing @@ -827,10 +892,43 @@ func TestIssue4219(t *testing.T) { // Adding a folder that was previously ignored should make it unignored. r := bytes.NewReader([]byte(`{ - "folders": [ - {"id": "abcd123"} + "devices": [ + { + "deviceID": "GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY", + "ignoredFolders": [ + { + "id": "t1" + }, + { + "id": "abcd123" + } + ] + }, + { + "deviceID": "LGFPDIT-7SKNNJL-VJZA4FC-7QNCRKA-CE753K7-2BW5QDK-2FOZ7FR-FEP57QJ", + "ignoredFolders": [ + { + "id": "t1" + }, + { + "id": "abcd123" + } + ] + } ], - "ignoredFolders": ["t1", "abcd123", "t2"] + "folders": [ + { + "id": "abcd123", + "devices":[ + {"deviceID": "GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY"} + ] + } + ], + "remoteIgnoredDevices": [ + { + "deviceID": "GYRZZQB-IRNPV4Z-T7TC52W-EQYJ3TT-FDQW6MW-DFLMU42-SSSU6EM-FBK2VAY" + } + ] }`)) cfg, err := ReadJSON(r, protocol.LocalDeviceID) @@ -838,19 +936,31 @@ func TestIssue4219(t *testing.T) { t.Fatal(err) } - if len(cfg.IgnoredFolders) != 2 { - t.Errorf("There should be two ignored folders, not %d", len(cfg.IgnoredFolders)) + if len(cfg.IgnoredDevices) != 0 { // 1 gets removed + t.Errorf("There should be zero ignored devices, not %d", len(cfg.IgnoredDevices)) + } + + ignoredFolders := 0 + for _, dev := range cfg.Devices { + ignoredFolders += len(dev.IgnoredFolders) + } + + if ignoredFolders != 3 { // 1 gets removed + t.Errorf("There should be three ignored folders, not %d", ignoredFolders) } w := Wrap("/tmp/cfg", cfg) - if !w.IgnoredFolder("t1") { - t.Error("Folder t1 should be ignored") + if !w.IgnoredFolder(device2, "t1") { + t.Error("Folder device2 t1 should be ignored") } - if !w.IgnoredFolder("t2") { - t.Error("Folder t2 should be ignored") + if !w.IgnoredFolder(device3, "t1") { + t.Error("Folder device3 t1 should be ignored") } - if w.IgnoredFolder("abcd123") { - t.Error("Folder abcd123 should not be ignored") + if w.IgnoredFolder(device2, "abcd123") { + t.Error("Folder device2 abcd123 should not be ignored") + } + if !w.IgnoredFolder(device3, "abcd123") { + t.Error("Folder device3 abcd123 should be ignored") } } diff --git a/lib/config/deviceconfiguration.go b/lib/config/deviceconfiguration.go index 2011a14bf..1315c863a 100644 --- a/lib/config/deviceconfiguration.go +++ b/lib/config/deviceconfiguration.go @@ -6,7 +6,11 @@ package config -import "github.com/syncthing/syncthing/lib/protocol" +import ( + "sort" + + "github.com/syncthing/syncthing/lib/protocol" +) type DeviceConfiguration struct { DeviceID protocol.DeviceID `xml:"id,attr" json:"deviceID"` @@ -22,6 +26,8 @@ type DeviceConfiguration struct { AutoAcceptFolders bool `xml:"autoAcceptFolders" json:"autoAcceptFolders"` MaxSendKbps int `xml:"maxSendKbps" json:"maxSendKbps"` MaxRecvKbps int `xml:"maxRecvKbps" json:"maxRecvKbps"` + IgnoredFolders []ObservedFolder `xml:"ignoredFolder" json:"ignoredFolders"` + PendingFolders []ObservedFolder `xml:"pendingFolder" json:"pendingFolders"` } func NewDeviceConfiguration(id protocol.DeviceID, name string) DeviceConfiguration { @@ -29,7 +35,7 @@ func NewDeviceConfiguration(id protocol.DeviceID, name string) DeviceConfigurati DeviceID: id, Name: name, } - d.prepare() + d.prepare(nil) return d } @@ -39,28 +45,64 @@ func (cfg DeviceConfiguration) Copy() DeviceConfiguration { copy(c.Addresses, cfg.Addresses) c.AllowedNetworks = make([]string, len(cfg.AllowedNetworks)) copy(c.AllowedNetworks, cfg.AllowedNetworks) + c.IgnoredFolders = make([]ObservedFolder, len(cfg.IgnoredFolders)) + copy(c.IgnoredFolders, cfg.IgnoredFolders) + c.PendingFolders = make([]ObservedFolder, len(cfg.PendingFolders)) + copy(c.PendingFolders, cfg.PendingFolders) return c } -func (cfg *DeviceConfiguration) prepare() { +func (cfg *DeviceConfiguration) prepare(sharedFolders []string) { if len(cfg.Addresses) == 0 || len(cfg.Addresses) == 1 && cfg.Addresses[0] == "" { cfg.Addresses = []string{"dynamic"} } if len(cfg.AllowedNetworks) == 0 { cfg.AllowedNetworks = []string{} } + + ignoredFolders := deduplicateObservedFoldersToMap(cfg.IgnoredFolders) + pendingFolders := deduplicateObservedFoldersToMap(cfg.PendingFolders) + + for id := range ignoredFolders { + delete(pendingFolders, id) + } + + for _, sharedFolder := range sharedFolders { + delete(ignoredFolders, sharedFolder) + delete(pendingFolders, sharedFolder) + } + + cfg.IgnoredFolders = sortedObservedFolderSlice(ignoredFolders) + cfg.PendingFolders = sortedObservedFolderSlice(pendingFolders) } -type DeviceConfigurationList []DeviceConfiguration - -func (l DeviceConfigurationList) Less(a, b int) bool { - return l[a].DeviceID.Compare(l[b].DeviceID) == -1 +func (cfg *DeviceConfiguration) IgnoredFolder(folder string) bool { + for _, ignoredFolder := range cfg.IgnoredFolders { + if ignoredFolder.ID == folder { + return true + } + } + return false } -func (l DeviceConfigurationList) Swap(a, b int) { - l[a], l[b] = l[b], l[a] +func sortedObservedFolderSlice(input map[string]ObservedFolder) []ObservedFolder { + output := make([]ObservedFolder, 0, len(input)) + for _, folder := range input { + output = append(output, folder) + } + sort.Slice(output, func(i, j int) bool { + return output[i].Time.Before(output[j].Time) + }) + return output } -func (l DeviceConfigurationList) Len() int { - return len(l) +func deduplicateObservedFoldersToMap(input []ObservedFolder) map[string]ObservedFolder { + output := make(map[string]ObservedFolder, len(input)) + for _, folder := range input { + if existing, ok := output[folder.ID]; !ok || existing.Time.Before(folder.Time) { + output[folder.ID] = folder + } + } + + return output } diff --git a/lib/config/folderconfiguration.go b/lib/config/folderconfiguration.go index 2f06b6951..fd3f5f04a 100644 --- a/lib/config/folderconfiguration.go +++ b/lib/config/folderconfiguration.go @@ -268,20 +268,6 @@ func (f *FolderConfiguration) SharedWith(device protocol.DeviceID) bool { return false } -type FolderDeviceConfigurationList []FolderDeviceConfiguration - -func (l FolderDeviceConfigurationList) Less(a, b int) bool { - return l[a].DeviceID.Compare(l[b].DeviceID) == -1 -} - -func (l FolderDeviceConfigurationList) Swap(a, b int) { - l[a], l[b] = l[b], l[a] -} - -func (l FolderDeviceConfigurationList) Len() int { - return len(l) -} - func (f *FolderConfiguration) CheckAvailableSpace(req int64) error { fs := f.Filesystem() usage, err := fs.Usage(".") @@ -296,17 +282,3 @@ func (f *FolderConfiguration) CheckAvailableSpace(req int64) error { } return fmt.Errorf("insufficient space in %v %v", fs.Type(), fs.URI()) } - -type FolderConfigurationList []FolderConfiguration - -func (l FolderConfigurationList) Len() int { - return len(l) -} - -func (l FolderConfigurationList) Less(a, b int) bool { - return l[a].ID < l[b].ID -} - -func (l FolderConfigurationList) Swap(a, b int) { - l[a], l[b] = l[b], l[a] -} diff --git a/lib/config/observed.go b/lib/config/observed.go new file mode 100644 index 000000000..afe2b0446 --- /dev/null +++ b/lib/config/observed.go @@ -0,0 +1,26 @@ +// Copyright (C) 2018 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +package config + +import ( + "time" + + "github.com/syncthing/syncthing/lib/protocol" +) + +type ObservedFolder struct { + Time time.Time `xml:"time,attr" json:"time"` + ID string `xml:"id,attr" json:"id"` + Label string `xml:"label,attr" json:"label"` +} + +type ObservedDevice struct { + Time time.Time `xml:"time,attr" json:"time"` + ID protocol.DeviceID `xml:"id,attr" json:"deviceID"` + Name string `xml:"name,attr" json:"name"` + Address string `xml:"address,attr" json:"address"` +} diff --git a/lib/config/testdata/ignoreddevices.xml b/lib/config/testdata/ignoreddevices.xml index f8d248895..e460726ce 100644 --- a/lib/config/testdata/ignoreddevices.xml +++ b/lib/config/testdata/ignoreddevices.xml @@ -5,6 +5,6 @@
    dynamic
     - AIR6LPZ-7K4PTTV-UXQSMUU-CPQ5YWH-OEDFIIQ-JUG777G-2YQXXR5-YD6AWQR - LGFPDIT-7SKNNJL-VJZA4FC-7QNCRKA-CE753K7-2BW5QDK-2FOZ7FR-FEP57QJ + + diff --git a/lib/config/testdata/ignoredfolders.xml b/lib/config/testdata/ignoredfolders.xml new file mode 100644 index 000000000..5d0c3a66d --- /dev/null +++ b/lib/config/testdata/ignoredfolders.xml @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/lib/config/wrapper.go b/lib/config/wrapper.go index 17c1f5fdf..4a666a70f 100644 --- a/lib/config/wrapper.go +++ b/lib/config/wrapper.go @@ -10,6 +10,7 @@ import ( "os" "path/filepath" "sync/atomic" + "time" "github.com/syncthing/syncthing/lib/events" "github.com/syncthing/syncthing/lib/fs" @@ -326,7 +327,7 @@ func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool { w.mut.Lock() defer w.mut.Unlock() for _, device := range w.cfg.IgnoredDevices { - if device == id { + if device.ID == id { return true } } @@ -335,15 +336,12 @@ func (w *Wrapper) IgnoredDevice(id protocol.DeviceID) bool { // IgnoredFolder returns whether or not share attempts for the given // folder should be silently ignored. -func (w *Wrapper) IgnoredFolder(folder string) bool { - w.mut.Lock() - defer w.mut.Unlock() - for _, nfolder := range w.cfg.IgnoredFolders { - if folder == nfolder { - return true - } +func (w *Wrapper) IgnoredFolder(device protocol.DeviceID, folder string) bool { + dev, ok := w.Device(device) + if !ok { + return false } - return false + return dev.IgnoredFolder(folder) } // Device returns the configuration for the given device and an "ok" bool. @@ -442,6 +440,56 @@ func (w *Wrapper) MyName() string { return cfg.Name } +func (w *Wrapper) AddOrUpdatePendingDevice(device protocol.DeviceID, name, address string) { + defer w.Save() + + w.mut.Lock() + defer w.mut.Unlock() + + for i := range w.cfg.PendingDevices { + if w.cfg.PendingDevices[i].ID == device { + w.cfg.PendingDevices[i].Time = time.Now() + w.cfg.PendingDevices[i].Name = name + w.cfg.PendingDevices[i].Address = address + return + } + } + + w.cfg.PendingDevices = append(w.cfg.PendingDevices, ObservedDevice{ + Time: time.Now(), + ID: device, + Name: name, + Address: address, + }) +} + +func (w *Wrapper) AddOrUpdatePendingFolder(id, label string, device protocol.DeviceID) { + defer w.Save() + + w.mut.Lock() + defer w.mut.Unlock() + + for _, dev := range w.cfg.Devices { + if dev.DeviceID == device { + for i := range dev.PendingFolders { + if dev.PendingFolders[i].ID == id { + dev.PendingFolders[i].Label = label + dev.PendingFolders[i].Time = time.Now() + return + } + } + dev.PendingFolders = append(dev.PendingFolders, ObservedFolder{ + Time: time.Now(), + ID: id, + Label: label, + }) + return + } + } + + panic("bug: adding pending folder for non-existing device") +} + // CheckHomeFreeSpace returns nil if the home disk has the required amount of // free space, or if home disk free space checking is disabled. func (w *Wrapper) CheckHomeFreeSpace() error { diff --git a/lib/model/model.go b/lib/model/model.go index fbad91b7d..dbbd18928 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -347,9 +347,7 @@ func (m *Model) tearDownFolderLocked(cfg config.FolderConfiguration) { // Must happen before stopping the folder service to abort ongoing // transmissions and thus allow timely service termination. for _, dev := range cfg.Devices { - if conn, ok := m.conn[dev.DeviceID]; ok { - closeRawConn(conn) - } + m.closeLocked(dev.DeviceID) } // Stop the services running for this folder and wait for them to finish @@ -935,10 +933,11 @@ func (m *Model) ClusterConfig(deviceID protocol.DeviceID, cm protocol.ClusterCon for _, folder := range cm.Folders { cfg, ok := m.cfg.Folder(folder.ID) if !ok || !cfg.SharedWith(deviceID) { - if m.cfg.IgnoredFolder(folder.ID) { + if deviceCfg.IgnoredFolder(folder.ID) { l.Infof("Ignoring folder %s from device %s since we are configured to", folder.Description(), deviceID) continue } + m.cfg.AddOrUpdatePendingFolder(folder.ID, folder.Label, deviceID) events.Default.Log(events.FolderRejected, map[string]string{ "folder": folder.ID, "folderLabel": folder.Label, @@ -1545,6 +1544,7 @@ func (m *Model) OnHello(remoteID protocol.DeviceID, addr net.Addr, hello protoco cfg, ok := m.cfg.Device(remoteID) if !ok { + m.cfg.AddOrUpdatePendingDevice(remoteID, hello.DeviceName, addr.String()) events.Default.Log(events.DeviceRejected, map[string]string{ "name": hello.DeviceName, "device": remoteID.String(), @@ -2701,6 +2701,11 @@ func (m *Model) CommitConfiguration(from, to config.Configuration) bool { continue } + // Ignored folder was removed, reconnect to retrigger the prompt. + if len(fromCfg.IgnoredFolders) > len(toCfg.IgnoredFolders) { + m.close(deviceID) + } + if toCfg.Paused { l.Infoln("Pausing", deviceID) m.close(deviceID)