Merge pull request #2801 from matrix-org/kegan/ss-api-changes

Sliding sync: add include_old_rooms; remove is_tombstoned

Author: kegsay

spec/integ/sliding-sync.spec.ts

@@ -806,6 +806,91 @@ describe("SlidingSync", () => {
             await listPromise;
             slidingSync.stop();
         });
+
+        // Regression test to make sure things like DELETE 0 INSERT 0 work correctly and we don't
+        // end up losing room IDs.
+        it("should handle insertions with a spurious DELETE correctly", async () => {
+            slidingSync = new SlidingSync(proxyBaseUrl, [
+                {
+                    ranges: [[0, 20]],
+                },
+            ], {}, client!, 1);
+            // initially start with nothing
+            httpBackend!.when("POST", syncUrl).respond(200, {
+                pos: "a",
+                lists: [{
+                    count: 0,
+                    ops: [],
+                }],
+            });
+            slidingSync.start();
+            await httpBackend!.flushAllExpected();
+            expect(slidingSync.getListData(0)!.roomIndexToRoomId).toEqual({});
+
+            // insert a room
+            httpBackend!.when("POST", syncUrl).respond(200, {
+                pos: "b",
+                lists: [{
+                    count: 1,
+                    ops: [
+                        {
+                            op: "DELETE", index: 0,
+                        },
+                        {
+                            op: "INSERT", index: 0, room_id: roomA,
+                        },
+                    ],
+                }],
+            });
+            await httpBackend!.flushAllExpected();
+            expect(slidingSync.getListData(0)!.roomIndexToRoomId).toEqual({
+                0: roomA,
+            });
+
+            // insert another room
+            httpBackend!.when("POST", syncUrl).respond(200, {
+                pos: "c",
+                lists: [{
+                    count: 1,
+                    ops: [
+                        {
+                            op: "DELETE", index: 1,
+                        },
+                        {
+                            op: "INSERT", index: 0, room_id: roomB,
+                        },
+                    ],
+                }],
+            });
+            await httpBackend!.flushAllExpected();
+            expect(slidingSync.getListData(0)!.roomIndexToRoomId).toEqual({
+                0: roomB,
+                1: roomA,
+            });
+
+            // insert a final room
+            httpBackend!.when("POST", syncUrl).respond(200, {
+                pos: "c",
+                lists: [{
+                    count: 1,
+                    ops: [
+                        {
+                            op: "DELETE", index: 2,
+                        },
+                        {
+                            op: "INSERT", index: 0, room_id: roomC,
+                        },
+                    ],
+                }],
+            });
+            await httpBackend!.flushAllExpected();
+            expect(slidingSync.getListData(0)!.roomIndexToRoomId).toEqual({
+                0: roomC,
+                1: roomB,
+                2: roomA,
+            });
+            slidingSync.stop();
+        });
     });
 
     describe("transaction IDs", () => {

src/sliding-sync.ts

@@ -33,6 +33,7 @@ const BUFFER_PERIOD_MS = 10 * 1000;
 export interface MSC3575RoomSubscription {
     required_state?: string[][];
     timeline_limit?: number;
+    include_old_rooms?: MSC3575RoomSubscription;
 }
 
 /**
@@ -42,7 +43,6 @@ export interface MSC3575Filter {
     is_dm?: boolean;
     is_encrypted?: boolean;
     is_invite?: boolean;
-    is_tombstoned?: boolean;
     room_name_like?: string;
     room_types?: string[];
     not_room_types?: string[];
@@ -641,8 +641,12 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
                             // starting at the gap so we can just shift each element in turn
                             this.shiftLeft(listIndex, op.index, gapIndex);
                         }
-                        gapIndex = -1; // forget the gap, we don't need it anymore.
                     }
+                    // forget the gap, we don't need it anymore. This is outside the check for
+                    // a room being present in this index position because INSERTs always universally
+                    // forget the gap, not conditionally based on the presence of a room in the INSERT
+                    // position. Without this, DELETE 0; INSERT 0; would do the wrong thing.
+                    gapIndex = -1;
                     this.lists[listIndex].roomIndexToRoomId[op.index] = op.room_id;
                     break;
                 }