1
0
mirror of https://github.com/matrix-org/matrix-js-sdk.git synced 2025-11-26 17:03:12 +03:00

bugfix: allow subtly different DELETE/INSERT semantics

In sliding sync, with an empty list, it is possible for the proxy
to send back DELETE 0, INSERT 0 !room which has the net result of
`[!room]`. Previously, the JS SDK would not handle this correctly.
Now it does. With tests.
This commit is contained in:
Kegan Dougal
2022-10-25 13:07:53 +01:00
parent 8f10c0d921
commit 24a9562b07
2 changed files with 91 additions and 1 deletions

View File

@@ -806,6 +806,91 @@ describe("SlidingSync", () => {
await listPromise; await listPromise;
slidingSync.stop(); 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", () => { describe("transaction IDs", () => {

View File

@@ -641,8 +641,12 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
// starting at the gap so we can just shift each element in turn // starting at the gap so we can just shift each element in turn
this.shiftLeft(listIndex, op.index, gapIndex); 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; this.lists[listIndex].roomIndexToRoomId[op.index] = op.room_id;
break; break;
} }
@@ -686,6 +690,7 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
// Everything higher than the gap needs to be shifted left. // Everything higher than the gap needs to be shifted left.
this.removeEntry(listIndex, gapIndex); this.removeEntry(listIndex, gapIndex);
} }
console.log("post-process", listIndex, JSON.stringify(this.lists[listIndex].roomIndexToRoomId));
} }
/** /**