Skip to content

Commit

Permalink
Fix bug in getRoomUpgradeHistory's verifyLinks functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@andybalaam
andybalaam committed Jan 24, 2023
1 parent addb2ca commit 3a8991e
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 22 deletions.
140 changes: 119 additions & 21 deletions spec/unit/matrix-client.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2205,7 +2205,7 @@ describe("MatrixClient", function () {
"creator": "@daryl:alexandria.example.com",
"m.federate": true,
"predecessor": {
event_id: "spec_is_not_clear_what_id_this_is",
event_id: "id_of_last_event",
room_id: predecessorRoomId,
},
"room_version": "9",
Expand Down Expand Up @@ -2278,34 +2278,34 @@ describe("MatrixClient", function () {
});

describe("getRoomUpgradeHistory", () => {
function createRoomHistory(): [Room, Room, Room, Room] {
/**
* Create a chain of room history with create events and tombstones.
*
* @param creates include create events (default=true)
* @param tombstones include tomstone events (default=true)
* @returns 4 rooms chained together with tombstones and create
* events, in order from oldest to latest.
*/
function createRoomHistory(creates = true, tombstones = true): [Room, Room, Room, Room] {
const room1 = new Room("room1", client, "@carol:alexandria.example.com");
const room2 = new Room("room2", client, "@daryl:alexandria.example.com");
const room3 = new Room("room3", client, "@rick:helicopter.example.com");
const room4 = new Room("room4", client, "@michonne:hawthorne.example.com");

room1.addLiveEvents([tombstoneEvent(room2.roomId, room1.roomId)], {});
room2.addLiveEvents([roomCreateEvent(room2.roomId, room1.roomId)]);

room2.addLiveEvents([tombstoneEvent(room3.roomId, room2.roomId)], {});
room3.addLiveEvents([roomCreateEvent(room3.roomId, room2.roomId)]);
if (creates) {
room2.addLiveEvents([roomCreateEvent(room2.roomId, room1.roomId)]);
room3.addLiveEvents([roomCreateEvent(room3.roomId, room2.roomId)]);
room4.addLiveEvents([roomCreateEvent(room4.roomId, room3.roomId)]);
}

room3.addLiveEvents([tombstoneEvent(room4.roomId, room3.roomId)], {});
room4.addLiveEvents([roomCreateEvent(room4.roomId, room3.roomId)]);
if (tombstones) {
room1.addLiveEvents([tombstoneEvent(room2.roomId, room1.roomId)], {});
room2.addLiveEvents([tombstoneEvent(room3.roomId, room2.roomId)], {});
room3.addLiveEvents([tombstoneEvent(room4.roomId, room3.roomId)], {});
}

mocked(store.getRoom).mockImplementation((roomId: string) => {
switch (roomId) {
case "room1":
return room1;
case "room2":
return room2;
case "room3":
return room3;
case "room4":
return room4;
default:
return null;
}
return { room1, room2, room3, room4 }[roomId] || null;
});

return [room1, room2, room3, room4];
Expand Down Expand Up @@ -2334,6 +2334,49 @@ describe("MatrixClient", function () {
]);
});

it("Returns the predecessors of this room (with verify links)", () => {
const [room1, room2, room3, room4] = createRoomHistory();
const verifyLinks = true;
const history = client.getRoomUpgradeHistory(room4.roomId, verifyLinks);
expect(history.map((room) => room.roomId)).toEqual([
room1.roomId,
room2.roomId,
room3.roomId,
room4.roomId,
]);
});

it("With verify links, rejects predecessors that don't point forwards", () => {
// Given successors point back with create events, but
// predecessors do not point forwards with tombstones
const [, , , room4] = createRoomHistory(true, false);

// When I ask for history with verifyLinks on
const verifyLinks = true;
const history = client.getRoomUpgradeHistory(room4.roomId, verifyLinks);

// Then the predecessors are not included in the history
expect(history.map((room) => room.roomId)).toEqual([room4.roomId]);
});

it("Without verify links, includes predecessors that don't point forwards", () => {
// Given successors point back with create events, but
// predecessors do not point forwards with tombstones
const [room1, room2, room3, room4] = createRoomHistory(true, false);

// When I ask for history with verifyLinks off
const verifyLinks = false;
const history = client.getRoomUpgradeHistory(room4.roomId, verifyLinks);

// Then the predecessors are included in the history
expect(history.map((room) => room.roomId)).toEqual([
room1.roomId,
room2.roomId,
room3.roomId,
room4.roomId,
]);
});

it("Returns the subsequent rooms", () => {
const [room1, room2, room3, room4] = createRoomHistory();
const history = client.getRoomUpgradeHistory(room1.roomId);
Expand All @@ -2345,6 +2388,49 @@ describe("MatrixClient", function () {
]);
});

it("Returns the subsequent rooms (with verify links)", () => {
const [room1, room2, room3, room4] = createRoomHistory();
const verifyLinks = true;
const history = client.getRoomUpgradeHistory(room1.roomId, verifyLinks);
expect(history.map((room) => room.roomId)).toEqual([
room1.roomId,
room2.roomId,
room3.roomId,
room4.roomId,
]);
});

it("With verify links, rejects successors that don't point backwards", () => {
// Given predecessors point forwards with tombstones, but
// successors do not point back with create events.
const [room1, , ,] = createRoomHistory(false, true);

// When I ask for history with verifyLinks on
const verifyLinks = true;
const history = client.getRoomUpgradeHistory(room1.roomId, verifyLinks);

// Then the successors are not included in the history
expect(history.map((room) => room.roomId)).toEqual([room1.roomId]);
});

it("Without verify links, includes predecessors that don't point forwards", () => {
// Given predecessors point forwards with tombstones, but
// successors do not point back with create events.
const [room1, room2, room3, room4] = createRoomHistory(false, true);

// When I ask for history with verifyLinks off
const verifyLinks = false;
const history = client.getRoomUpgradeHistory(room1.roomId, verifyLinks);

// Then the successors are included in the history
expect(history.map((room) => room.roomId)).toEqual([
room1.roomId,
room2.roomId,
room3.roomId,
room4.roomId,
]);
});

it("Returns the predecessors and subsequent rooms", () => {
const [room1, room2, room3, room4] = createRoomHistory();
const history = client.getRoomUpgradeHistory(room3.roomId);
Expand All @@ -2355,6 +2441,18 @@ describe("MatrixClient", function () {
room4.roomId,
]);
});

it("Returns the predecessors and subsequent rooms (with verify links)", () => {
const [room1, room2, room3, room4] = createRoomHistory();
const verifyLinks = true;
const history = client.getRoomUpgradeHistory(room3.roomId, verifyLinks);
expect(history.map((room) => room.roomId)).toEqual([
room1.roomId,
room2.roomId,
room3.roomId,
room4.roomId,
]);
});
});
});
});
4 changes: 3 additions & 1 deletion src/client.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4996,6 +4996,7 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
const upgradeHistory = [currentRoom];

// Work backwards first, looking at create events.
let successorRoomId = currentRoom.roomId;
let createEvent = currentRoom.currentState.getStateEvents(EventType.RoomCreate, "");
while (createEvent) {
const predecessor = createEvent.getContent()["predecessor"];
Expand All @@ -5006,13 +5007,14 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
if (verifyLinks) {
const tombstone = refRoom.currentState.getStateEvents(EventType.RoomTombstone, "");

if (!tombstone || tombstone.getContent()["replacement_room"] !== refRoom.roomId) {
if (!tombstone || tombstone.getContent()["replacement_room"] !== successorRoomId) {
break;
}
}

// Insert at the front because we're working backwards from the currentRoom
upgradeHistory.splice(0, 0, refRoom);
successorRoomId = refRoom.roomId;
createEvent = refRoom.currentState.getStateEvents(EventType.RoomCreate, "");
} else {
// No further create events to look at
Expand Down

0 comments on commit 3a8991e

Please sign in to comment.