diff --git a/sync3/ops.go b/sync3/ops.go index 45bb885..716f364 100644 --- a/sync3/ops.go +++ b/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 } diff --git a/sync3/ops_test.go b/sync3/ops_test.go index 269db96..ceacdff 100644 --- a/sync3/ops_test.go +++ b/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) diff --git a/testutils/list.go b/testutils/list.go new file mode 100644 index 0000000..e6fae2d --- /dev/null +++ b/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 +}