bugfix: fix a bug where move updates could go missing between windows

If a room moved from one window range to another window range, and the
index of the destination was the leading edge of a different window,
this would trip up the code into thinking it was a no-op move and hence
not issue a DELETE/INSERT for the 2nd window, even though it was in fact
needed. For example:
```
   w1           w2
[0,1,2] 3,4,5 [6,7,8]

Move 1 to 6 turns the list into:
[0,2,3] 4,5,6 [1,7,8]

which should be the operations:
DELETE 1, INSERT 2 (val=3)
DELETE 6, INSERT 6 (val=1)

but because DELETE/INSERT both have the same index value, and the target
room is the updated room, we thought this was the same as when you have:

[0,1,2] 3,4,5

Move 0 to 0

which should no-op.
```

Fixed by ensuring that we also check that there is only 1 move operation.
If there are >1 move operations then we are moving between lists and should
include the DELETE/INSERT operation with the same index. This could manifest
itself in updated rooms spontaneously disappearing and/or neighbouring rooms
being duplicated.

sync3/ops.go

@@ -74,7 +74,10 @@ func CalculateListOps(reqList *RequestList, list List, roomID string, listOp Lis
 		listToIndex := listFromTo[1]
 		wasUpdatedRoomInserted := listToIndex == toIndex
 		toRoomID := list.Get(listToIndex)
-		if toRoomID == roomID && listFromIndex == listToIndex && listOp == ListOpChange && wasInsideRange {
+		if toRoomID == roomID && listFromIndex == listToIndex && listOp == ListOpChange && wasInsideRange && len(listFromTos) == 1 {
+			// DELETE/INSERT have the same index, we're INSERTing the room that was updated, it was a Change not Add/Delete, it
+			// was previously inside the window AND there's just 1 move operation = it's moving to and from the same index so
+			// don't send an update.
 			continue // no-op move
 		}
 

sync3/ops_test.go

@@ -598,6 +598,7 @@ func TestCalculateListOps_TortureSingleWindowMiddle_Move(t *testing.T) {
 				t.Errorf("CalculateListOps: got sub %v but it was already in the range", sub)
 			}
 		}
+
 		if (int64(fromIndex) > ranges[0][1] && int64(toIndex) > ranges[0][1]) || (int64(fromIndex) < ranges[0][0] && int64(toIndex) < ranges[0][0]) {
 			// the swap happens outside the range, so we expect nothing
 			if len(gotOps) != 0 {
@@ -650,9 +651,8 @@ func TestCalculateListOps_TortureSingleWindowMiddle_Move(t *testing.T) {
 	}
 }
 
-/*
-func TestCalculateListOps_TortureMultipleWindows_Move(t *testing.T) {
-	rand.Seed(41)
+func TestCalculateListOpsTortureMultipleWindowsMove(t *testing.T) {
+	rand.Seed(40)
 	ranges := SliceRanges{{0, 5}, {9, 11}}
 	inRangeIDs := map[string]bool{
 		"a": true, "b": true, "c": true, "d": true, "e": true, "f": true, "j": true, "k": true, "l": true,
@@ -667,8 +667,6 @@ func TestCalculateListOps_TortureMultipleWindows_Move(t *testing.T) {
 		gotOps, gotSubs := CalculateListOps(&RequestList{
 			Ranges: ranges,
 		}, sl, roomID, ListOpChange)
-		b, _ := json.Marshal(gotOps)
-		fmt.Println(string(b))
 		for _, sub := range gotSubs {
 			if inRangeIDs[sub] {
 				t.Fatalf("CalculateListOps: got sub %v but it was already in the range", sub)
@@ -704,39 +702,48 @@ func TestCalculateListOps_TortureMultipleWindows_Move(t *testing.T) {
 			continue
 		}
 
-			if int64(fromIndex) > ranges[0][1] {
-				// should inject a different room at the start of the range
-				fromIndex = int(ranges[0][1])
-			} else if int64(fromIndex) < ranges[0][0] {
-				// should inject a different room at the start of the range
-				fromIndex = int(ranges[0][0])
-			}
-			assertSingleOp(t, gotOps[0], "DELETE", fromIndex, "")
-			if int64(toIndex) > ranges[0][1] {
-				// should inject a different room at the end of the range
-				toIndex = int(ranges[0][1])
-				roomID = after[toIndex]
-			} else if int64(toIndex) < ranges[0][0] {
-				// should inject a different room at the end of the range
-				toIndex = int(ranges[0][0])
-				roomID = after[toIndex]
-			}
-			if !inRangeIDs[roomID] {
-				// it should now be in-range
-				inRange := false
-				for _, sub := range gotSubs {
-					if sub == roomID {
-						inRange = true
-						break
-					}
-				}
-				if !inRange {
-					t.Errorf("got subs %v missing room %v", gotSubs, roomID)
+		if wantOps == 4 {
+			continue // TODO: find a way to check multi DELETE/INSERT
+		}
+		rng := ranges[0]
+		if insideSecondWindow {
+			rng = ranges[1]
+		}
+
+		if int64(fromIndex) > rng[1] {
+			// should inject a different room at the start of the range
+			fromIndex = int(rng[1])
+		} else if int64(fromIndex) < rng[0] {
+			// should inject a different room at the start of the range
+			fromIndex = int(rng[0])
+		}
+		assertSingleOp(t, gotOps[0], "DELETE", fromIndex, "")
+
+		if int64(toIndex) > rng[1] {
+			// should inject a different room at the end of the range
+			toIndex = int(rng[1])
+			roomID = after[toIndex]
+		} else if int64(toIndex) < rng[0] {
+			// should inject a different room at the end of the range
+			toIndex = int(rng[0])
+			roomID = after[toIndex]
+		}
+		if !inRangeIDs[roomID] {
+			// it should now be in-range
+			inRange := false
+			for _, sub := range gotSubs {
+				if sub == roomID {
+					inRange = true
+					break
 				}
 			}
-			assertSingleOp(t, gotOps[1], "INSERT", toIndex, roomID)
+			if !inRange {
+				t.Errorf("got subs %v missing room %v", gotSubs, roomID)
+			}
+		}
+		assertSingleOp(t, gotOps[1], "INSERT", toIndex, roomID)
 	}
-} */
+}
 
 func assertSingleOp(t *testing.T, op ResponseOp, opName string, index int, optRoomID string) {
 	t.Helper()