You've already forked matrix-js-sdk
mirror of
https://github.com/matrix-org/matrix-js-sdk.git
synced 2025-11-26 17:03:12 +03:00
sliding sync: handle lone DELETE and INSERT operations
If you leave a room you can get a lone DELETE op. If you join a room you can get a lone INSERT op. Up until now, we've assumed these operations happen at the ends of the list (e.g [0] or [length-1]) which is not guaranteed as it depends on the sort order (e.g sort alphabetically and join a room called 'D'). In this scenario, the indexes would not be tracked correctly. Fixed with integration tests.
This commit is contained in:
@@ -558,6 +558,153 @@ describe("SlidingSync", () => {
|
|||||||
await httpBackend.flushAllExpected();
|
await httpBackend.flushAllExpected();
|
||||||
await responseProcessed;
|
await responseProcessed;
|
||||||
await listPromise;
|
await listPromise;
|
||||||
|
});
|
||||||
|
|
||||||
|
// this refers to a set of operations where the end result is no change.
|
||||||
|
it("should handle net zero operations correctly", async () => {
|
||||||
|
const indexToRoomId = {
|
||||||
|
0: roomB,
|
||||||
|
1: roomC,
|
||||||
|
};
|
||||||
|
expect(slidingSync.getListData(0).roomIndexToRoomId).toEqual(indexToRoomId);
|
||||||
|
httpBackend.when("POST", syncUrl).respond(200, {
|
||||||
|
pos: "f",
|
||||||
|
// currently the list is [B,C] so we will insert D then immediately delete it
|
||||||
|
lists: [{
|
||||||
|
count: 500,
|
||||||
|
ops: [
|
||||||
|
{
|
||||||
|
op: "DELETE", index: 2,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
op: "INSERT", index: 0, room_id: roomA,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
op: "DELETE", index: 0,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
count: 50,
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
const listPromise = listenUntil(slidingSync, "SlidingSync.List",
|
||||||
|
(listIndex, joinedCount, roomIndexToRoomId) => {
|
||||||
|
expect(listIndex).toEqual(0);
|
||||||
|
expect(joinedCount).toEqual(500);
|
||||||
|
expect(roomIndexToRoomId).toEqual(indexToRoomId);
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
const responseProcessed = listenUntil(slidingSync, "SlidingSync.Lifecycle", (state) => {
|
||||||
|
return state === SlidingSyncState.Complete;
|
||||||
|
});
|
||||||
|
await httpBackend.flushAllExpected();
|
||||||
|
await responseProcessed;
|
||||||
|
await listPromise;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle deletions correctly", async () => {
|
||||||
|
expect(slidingSync.getListData(0).roomIndexToRoomId).toEqual({
|
||||||
|
0: roomB,
|
||||||
|
1: roomC,
|
||||||
|
});
|
||||||
|
httpBackend.when("POST", syncUrl).respond(200, {
|
||||||
|
pos: "g",
|
||||||
|
lists: [{
|
||||||
|
count: 499,
|
||||||
|
ops: [
|
||||||
|
{
|
||||||
|
op: "DELETE", index: 0,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
count: 50,
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
const listPromise = listenUntil(slidingSync, "SlidingSync.List",
|
||||||
|
(listIndex, joinedCount, roomIndexToRoomId) => {
|
||||||
|
expect(listIndex).toEqual(0);
|
||||||
|
expect(joinedCount).toEqual(499);
|
||||||
|
expect(roomIndexToRoomId).toEqual({
|
||||||
|
0: roomC,
|
||||||
|
});
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
const responseProcessed = listenUntil(slidingSync, "SlidingSync.Lifecycle", (state) => {
|
||||||
|
return state === SlidingSyncState.Complete;
|
||||||
|
});
|
||||||
|
await httpBackend.flushAllExpected();
|
||||||
|
await responseProcessed;
|
||||||
|
await listPromise;
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle insertions correctly", async () => {
|
||||||
|
expect(slidingSync.getListData(0).roomIndexToRoomId).toEqual({
|
||||||
|
0: roomC,
|
||||||
|
});
|
||||||
|
httpBackend.when("POST", syncUrl).respond(200, {
|
||||||
|
pos: "h",
|
||||||
|
lists: [{
|
||||||
|
count: 500,
|
||||||
|
ops: [
|
||||||
|
{
|
||||||
|
op: "INSERT", index: 1, room_id: roomA,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
count: 50,
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
let listPromise = listenUntil(slidingSync, "SlidingSync.List",
|
||||||
|
(listIndex, joinedCount, roomIndexToRoomId) => {
|
||||||
|
expect(listIndex).toEqual(0);
|
||||||
|
expect(joinedCount).toEqual(500);
|
||||||
|
expect(roomIndexToRoomId).toEqual({
|
||||||
|
0: roomC,
|
||||||
|
1: roomA,
|
||||||
|
});
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
let responseProcessed = listenUntil(slidingSync, "SlidingSync.Lifecycle", (state) => {
|
||||||
|
return state === SlidingSyncState.Complete;
|
||||||
|
});
|
||||||
|
await httpBackend.flushAllExpected();
|
||||||
|
await responseProcessed;
|
||||||
|
await listPromise;
|
||||||
|
|
||||||
|
httpBackend.when("POST", syncUrl).respond(200, {
|
||||||
|
pos: "h",
|
||||||
|
lists: [{
|
||||||
|
count: 501,
|
||||||
|
ops: [
|
||||||
|
{
|
||||||
|
op: "INSERT", index: 1, room_id: roomB,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
count: 50,
|
||||||
|
}],
|
||||||
|
});
|
||||||
|
listPromise = listenUntil(slidingSync, "SlidingSync.List",
|
||||||
|
(listIndex, joinedCount, roomIndexToRoomId) => {
|
||||||
|
expect(listIndex).toEqual(0);
|
||||||
|
expect(joinedCount).toEqual(501);
|
||||||
|
expect(roomIndexToRoomId).toEqual({
|
||||||
|
0: roomC,
|
||||||
|
1: roomB,
|
||||||
|
2: roomA,
|
||||||
|
});
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
responseProcessed = listenUntil(slidingSync, "SlidingSync.Lifecycle", (state) => {
|
||||||
|
return state === SlidingSyncState.Complete;
|
||||||
|
});
|
||||||
|
await httpBackend.flushAllExpected();
|
||||||
|
await responseProcessed;
|
||||||
|
await listPromise;
|
||||||
slidingSync.stop();
|
slidingSync.stop();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -530,6 +530,65 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
|
|||||||
this.emit(SlidingSyncEvent.Lifecycle, state, resp, err);
|
this.emit(SlidingSyncEvent.Lifecycle, state, resp, err);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private shiftRight(listIndex: number, hi: number, low: number) {
|
||||||
|
// l h
|
||||||
|
// 0,1,2,3,4 <- before
|
||||||
|
// 0,1,2,2,3 <- after, hi is deleted and low is duplicated
|
||||||
|
for (let i = hi; i > low; i--) {
|
||||||
|
if (this.lists[listIndex].isIndexInRange(i)) {
|
||||||
|
this.lists[listIndex].roomIndexToRoomId[i] =
|
||||||
|
this.lists[listIndex].roomIndexToRoomId[
|
||||||
|
i - 1
|
||||||
|
];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private shiftLeft(listIndex: number, hi: number, low: number) {
|
||||||
|
// l h
|
||||||
|
// 0,1,2,3,4 <- before
|
||||||
|
// 0,1,3,4,4 <- after, low is deleted and hi is duplicated
|
||||||
|
for (let i = low; i < hi; i++) {
|
||||||
|
if (this.lists[listIndex].isIndexInRange(i)) {
|
||||||
|
this.lists[listIndex].roomIndexToRoomId[i] =
|
||||||
|
this.lists[listIndex].roomIndexToRoomId[
|
||||||
|
i + 1
|
||||||
|
];
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private removeEntry(listIndex: number, index: number) {
|
||||||
|
// work out the max index
|
||||||
|
let max = -1;
|
||||||
|
for (const n in this.lists[listIndex].roomIndexToRoomId) {
|
||||||
|
if (Number(n) > max) {
|
||||||
|
max = Number(n);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (max < 0 || index > max) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Everything higher than the gap needs to be shifted left.
|
||||||
|
this.shiftLeft(listIndex, max, index);
|
||||||
|
delete this.lists[listIndex].roomIndexToRoomId[max];
|
||||||
|
}
|
||||||
|
|
||||||
|
private addEntry(listIndex: number, index: number) {
|
||||||
|
// work out the max index
|
||||||
|
let max = -1;
|
||||||
|
for (const n in this.lists[listIndex].roomIndexToRoomId) {
|
||||||
|
if (Number(n) > max) {
|
||||||
|
max = Number(n);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (max < 0 || index > max) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
// Everything higher than the gap needs to be shifted right, +1 so we don't delete the highest element
|
||||||
|
this.shiftRight(listIndex, max+1, index);
|
||||||
|
}
|
||||||
|
|
||||||
private processListOps(list: ListResponse, listIndex: number): void {
|
private processListOps(list: ListResponse, listIndex: number): void {
|
||||||
let gapIndex = -1;
|
let gapIndex = -1;
|
||||||
list.ops.forEach((op: Operation) => {
|
list.ops.forEach((op: Operation) => {
|
||||||
@@ -537,6 +596,10 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
|
|||||||
case "DELETE": {
|
case "DELETE": {
|
||||||
logger.debug("DELETE", listIndex, op.index, ";");
|
logger.debug("DELETE", listIndex, op.index, ";");
|
||||||
delete this.lists[listIndex].roomIndexToRoomId[op.index];
|
delete this.lists[listIndex].roomIndexToRoomId[op.index];
|
||||||
|
if (gapIndex !== -1) {
|
||||||
|
// we already have a DELETE operation to process, so process it.
|
||||||
|
this.removeEntry(listIndex, gapIndex);
|
||||||
|
}
|
||||||
gapIndex = op.index;
|
gapIndex = op.index;
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
@@ -551,20 +614,9 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
|
|||||||
if (this.lists[listIndex].roomIndexToRoomId[op.index]) {
|
if (this.lists[listIndex].roomIndexToRoomId[op.index]) {
|
||||||
// something is in this space, shift items out of the way
|
// something is in this space, shift items out of the way
|
||||||
if (gapIndex < 0) {
|
if (gapIndex < 0) {
|
||||||
logger.debug(
|
// we haven't been told where to shift from, so make way for a new room entry.
|
||||||
"cannot work out where gap is, INSERT without previous DELETE! List: ",
|
this.addEntry(listIndex, op.index);
|
||||||
listIndex,
|
} else if (gapIndex > op.index) {
|
||||||
);
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
// 0,1,2,3 index
|
|
||||||
// [A,B,C,D]
|
|
||||||
// DEL 3
|
|
||||||
// [A,B,C,_]
|
|
||||||
// INSERT E 0
|
|
||||||
// [E,A,B,C]
|
|
||||||
// gapIndex=3, op.index=0
|
|
||||||
if (gapIndex > op.index) {
|
|
||||||
// the gap is further down the list, shift every element to the right
|
// the gap is further down the list, shift every element to the right
|
||||||
// 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:
|
||||||
// [A,B,C,_] gapIndex=3, op.index=0
|
// [A,B,C,_] gapIndex=3, op.index=0
|
||||||
@@ -572,26 +624,13 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
|
|||||||
// [A,B,B,C] i=2
|
// [A,B,B,C] i=2
|
||||||
// [A,A,B,C] i=1
|
// [A,A,B,C] i=1
|
||||||
// Terminate. We'll assign into op.index next.
|
// Terminate. We'll assign into op.index next.
|
||||||
for (let i = gapIndex; i > op.index; i--) {
|
this.shiftRight(listIndex, gapIndex, op.index);
|
||||||
if (this.lists[listIndex].isIndexInRange(i)) {
|
|
||||||
this.lists[listIndex].roomIndexToRoomId[i] =
|
|
||||||
this.lists[listIndex].roomIndexToRoomId[
|
|
||||||
i - 1
|
|
||||||
];
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else if (gapIndex < op.index) {
|
} else if (gapIndex < op.index) {
|
||||||
// the gap is further up the list, shift every element to the left
|
// the gap is further up the list, shift every element to the left
|
||||||
// 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
|
||||||
for (let i = gapIndex; i < op.index; i++) {
|
this.shiftLeft(listIndex, op.index, gapIndex);
|
||||||
if (this.lists[listIndex].isIndexInRange(i)) {
|
|
||||||
this.lists[listIndex].roomIndexToRoomId[i] =
|
|
||||||
this.lists[listIndex].roomIndexToRoomId[
|
|
||||||
i + 1
|
|
||||||
];
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
gapIndex = -1; // forget the gap, we don't need it anymore.
|
||||||
}
|
}
|
||||||
this.lists[listIndex].roomIndexToRoomId[op.index] = op.room_id;
|
this.lists[listIndex].roomIndexToRoomId[op.index] = op.room_id;
|
||||||
break;
|
break;
|
||||||
@@ -631,6 +670,11 @@ export class SlidingSync extends TypedEventEmitter<SlidingSyncEvent, SlidingSync
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
if (gapIndex !== -1) {
|
||||||
|
// we already have a DELETE operation to process, so process it
|
||||||
|
// Everything higher than the gap needs to be shifted left.
|
||||||
|
this.removeEntry(listIndex, gapIndex);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
Reference in New Issue
Block a user