Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "verifyLinks" functionality of getRoomUpgradeHistory #3089

Merged
merged 1 commit into from
Jan 24, 2023
Merged

Fix "verifyLinks" functionality of getRoomUpgradeHistory #3089

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 119 additions & 21 deletions spec/unit/matrix-client.spec.ts
View file
Open in desktop
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
Open in desktop
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