bugfix: add torture test for list delta ops

- Randomly move elements 10,000 times in a sliding window.
- Fixed a bug as a result which would cause the algorithm to
  fail to issue a DELETE/INSERT when the room was _inserted_
  to the very end of the window range, due to it misfiring
  with the logic to not issue operations for no-op moves.

sync3/ops.go

@@ -74,7 +74,7 @@ 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 {
+		if toRoomID == roomID && listFromIndex == listToIndex && listOp == ListOpChange && wasInsideRange {
 			continue // no-op move
 		}
 

sync3/ops_test.go

@@ -3,7 +3,10 @@ package sync3
 import (
 	"bytes"
 	"encoding/json"
+	"math/rand"
 	"testing"
+
+	"github.com/matrix-org/sync-v3/testutils"
 )
 
 func TestCalculateListOps_BasicOperations(t *testing.T) {
@@ -490,6 +493,77 @@ func TestCalculateListOps_MultipleWindowOperations(t *testing.T) {
 	}
 }
 
+func TestCalculateListOps_TortureSingleWindow_Move(t *testing.T) {
+	rand.Seed(42)
+	ranges := SliceRanges{{0, 5}}
+	inRangeIDs := map[string]bool{
+		"a": true, "b": true, "c": true, "d": true, "e": true, "f": true,
+	}
+	for i := 0; i < 10000; i++ {
+		before := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i"}
+		after, roomID, fromIndex, toIndex := testutils.MoveRandomElement(before)
+		t.Logf("move %s from %v to %v -> %v", roomID, fromIndex, toIndex, after)
+
+		sl := newStringList(before)
+		sl.sortedRoomIDs = after
+		gotOps, gotSubs := CalculateListOps(&RequestList{
+			Ranges: ranges,
+		}, sl, roomID, ListOpChange)
+
+		for _, sub := range gotSubs {
+			if inRangeIDs[sub] {
+				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] {
+			// the swap happens outside the range, so we expect nothing
+			if len(gotOps) != 0 {
+				t.Errorf("CalculateLisOps: swap outside range got ops wanted none: %+v", gotOps)
+			}
+			continue
+		}
+		if fromIndex == toIndex {
+			// we want 0 ops
+			if len(gotOps) > 0 {
+				t.Errorf("CalculateLisOps: from==to got ops wanted none: %+v", gotOps)
+			}
+			continue
+		}
+		if len(gotOps) != 2 {
+			t.Fatalf("CalculateLisOps: wanted 2 ops, got %+v", gotOps)
+			continue
+		}
+		if int64(fromIndex) > ranges[0][1] {
+			// should inject a different room at the start of the range
+			fromIndex = int(ranges[0][1])
+		}
+		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]
+		}
+		assertSingleOp(t, gotOps[1], "INSERT", toIndex, roomID)
+	}
+}
+
+func assertSingleOp(t *testing.T, op ResponseOp, opName string, index int, optRoomID string) {
+	t.Helper()
+	singleOp, ok := op.(*ResponseOpSingle)
+	if !ok {
+		t.Errorf("op was not a single operation, got %+v", op)
+	}
+	if singleOp.Operation != opName {
+		t.Errorf("op name got %v want %v", singleOp.Operation, opName)
+	}
+	if *singleOp.Index != index {
+		t.Errorf("op index got %v want %v", *singleOp.Index, index)
+	}
+	if optRoomID != "" && singleOp.RoomID != optRoomID {
+		t.Errorf("op room ID got %v want %v", singleOp.RoomID, optRoomID)
+	}
+}
+
 func assertEqualOps(t *testing.T, name string, gotOps, wantOps []ResponseOp) {
 	t.Helper()
 	got, err := json.Marshal(gotOps)

testutils/list.go

@@ -0,0 +1,36 @@
+package testutils
+
+import "math/rand"
+
+// Moves a random element in `before`. Returns a new array with the element moved, the item moved,
+// and which index positions it was moved from/to.
+func MoveRandomElement(before []string) (after []string, item string, fromIndex, toIndex int) {
+	// randomly choose an element to move
+	fromIndex = rand.Intn(len(before))
+	item = before[fromIndex]
+	// randomly choose a destination location
+	toIndex = rand.Intn(len(before))
+	// create the after array
+	for j := range before {
+		if j == fromIndex {
+			if j == toIndex {
+				after = append(after, before[j]) // no-op move
+			}
+			continue // skip over before item
+		}
+		if j == toIndex {
+			if j > fromIndex {
+				// we've skipped item already, need to make up for it now
+				after = append(after, before[toIndex])
+				after = append(after, item)
+			} else {
+				// we will skip over item, need to make up for it now
+				after = append(after, item)
+				after = append(after, before[toIndex])
+			}
+			continue
+		}
+		after = append(after, before[j])
+	}
+	return
+}