lib/model: Clean up generateClusterConfig, fix spurious test failure by sorting

lib/model/model.go

@@ -1958,7 +1958,12 @@ func (m *Model) generateClusterConfig(device protocol.DeviceID) protocol.Cluster
 	var message protocol.ClusterConfig
 
 	m.fmut.RLock()
-	for _, folder := range m.deviceFolders[device] {
+	// The list of folders in the message is sorted, so we always get the
+	// same order.
+	folders := m.deviceFolders[device]
+	sort.Strings(folders)
+
+	for _, folder := range folders {
 		folderCfg := m.cfg.Folders()[folder]
 		fs := m.folderFiles[folder]
 
@@ -1971,12 +1976,8 @@ func (m *Model) generateClusterConfig(device protocol.DeviceID) protocol.Cluster
 			DisableTempIndexes: folderCfg.DisableTempIndexes,
 		}
 
-		for device := range m.folderDevices[folder] {
-			// DeviceID is a value type, but with an underlying array. Copy it
-			// so we don't grab aliases to the same array later on in device[:]
-			device := device
-			// TODO: Set read only bit when relevant, and when we have per device
-			// access controls.
+		// Devices are sorted, so we always get the same order.
+		for _, device := range m.folderDevices.sortedDevices(folder) {
 			deviceCfg := m.cfg.Devices()[device]
 
 			var indexID protocol.IndexID
@@ -2604,3 +2605,13 @@ func (s folderDeviceSet) hasDevice(dev protocol.DeviceID) bool {
 	}
 	return false
 }
+
+// sortedDevices returns the list of devices for a given folder, sorted
+func (s folderDeviceSet) sortedDevices(folder string) []protocol.DeviceID {
+	devs := make([]protocol.DeviceID, 0, len(s[folder]))
+	for dev := range s[folder] {
+		devs = append(devs, dev)
+	}
+	sort.Sort(protocol.DeviceIDs(devs))
+	return devs
+}

lib/model/model_test.go

@@ -445,13 +445,13 @@ func TestClusterConfig(t *testing.T) {
 		t.Errorf("Incorrect number of devices %d != 2", l)
 	}
 	if id := r.Devices[0].ID; id != device1 {
-		t.Errorf("Incorrect device ID %x != %x", id, device1)
+		t.Errorf("Incorrect device ID %s != %s", id, device1)
 	}
 	if !r.Devices[0].Introducer {
 		t.Error("Device1 should be flagged as Introducer")
 	}
 	if id := r.Devices[1].ID; id != device2 {
-		t.Errorf("Incorrect device ID %x != %x", id, device2)
+		t.Errorf("Incorrect device ID %s != %s", id, device2)
 	}
 	if r.Devices[1].Introducer {
 		t.Error("Device2 should not be flagged as Introducer")
@@ -465,13 +465,13 @@ func TestClusterConfig(t *testing.T) {
 		t.Errorf("Incorrect number of devices %d != 2", l)
 	}
 	if id := r.Devices[0].ID; id != device1 {
-		t.Errorf("Incorrect device ID %x != %x", id, device1)
+		t.Errorf("Incorrect device ID %s != %s", id, device1)
 	}
 	if !r.Devices[0].Introducer {
 		t.Error("Device1 should be flagged as Introducer")
 	}
 	if id := r.Devices[1].ID; id != device2 {
-		t.Errorf("Incorrect device ID %x != %x", id, device2)
+		t.Errorf("Incorrect device ID %s != %s", id, device2)
 	}
 	if r.Devices[1].Introducer {
 		t.Error("Device2 should not be flagged as Introducer")

lib/protocol/deviceid.go

@@ -203,3 +203,18 @@ func untypeoify(s string) string {
 	s = strings.Replace(s, "8", "B", -1)
 	return s
 }
+
+// DeviceIDs is a sortable slice of DeviceID
+type DeviceIDs []DeviceID
+
+func (l DeviceIDs) Len() int {
+	return len(l)
+}
+
+func (l DeviceIDs) Less(a, b int) bool {
+	return l[a].Compare(l[b]) == -1
+}
+
+func (l DeviceIDs) Swap(a, b int) {
+	l[a], l[b] = l[b], l[a]
+}