Skip to content

Commit

Permalink
Add tests for various forms of rich replies
Browse files Browse the repository at this point in the history
Signed-off-by: Tadeusz „tadzik” Sośnierz <[email protected]>
  • Loading branch information
Half-Shot authored and tadzik committed Mar 21, 2024
1 parent e060290 commit 19bf636
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 16 deletions.
1 change: 1 addition & 0 deletions changelog.d/1799.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add tests for various forms of rich replies.
138 changes: 138 additions & 0 deletions spec/e2e/replies.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
import { TestIrcServer } from "matrix-org-irc";
import { IrcBridgeE2ETest } from "../util/e2e-test";
import { describe, expect, it } from "@jest/globals";


describe('Reply handling', () => {
let testEnv: IrcBridgeE2ETest;
async function setupTestEnv(shortReplyTresholdSeconds: number) {
testEnv = await IrcBridgeE2ETest.createTestEnv({
matrixLocalparts: [TestIrcServer.generateUniqueNick("alice"), TestIrcServer.generateUniqueNick("charlie")],
ircNicks: ['bob'],
traceToFile: true,
shortReplyTresholdSeconds,
});
await testEnv.setUp();
}
afterEach(() => {
return testEnv?.tearDown();
});

it('should use short and long reply formats, depending on elapsed time', async () => {
await setupTestEnv(1);

const channel = `#${TestIrcServer.generateUniqueNick("test")}`;
const { homeserver } = testEnv;
const [alice, charlie] = homeserver.users.map(u => u.client);
const { bob } = testEnv.ircTest.clients;

await bob.join(channel);

const adminRoomId = await testEnv.createAdminRoomHelper(alice);
const cRoomId = await testEnv.joinChannelHelper(alice, adminRoomId, channel);
await charlie.joinRoom(cRoomId);

const bobUserId = `@irc_${bob.nick}:${homeserver.domain}`;
await alice.waitForRoomEvent(
{eventType: 'm.room.member', sender: bobUserId, stateKey: bobUserId, roomId: cRoomId}
);

// first message is always a bit delayed, so let's send&await it ahead of time before we get to testing
let bridgedMessage = bob.waitForEvent('message', 10000);
await alice.sendText(cRoomId, "warming up...");
await bridgedMessage;

const originalMessageBody = "Original message";
bridgedMessage = bob.waitForEvent('message', 10000);
const originalMessageId = await alice.sendText(cRoomId, originalMessageBody);
await bridgedMessage;

bridgedMessage = bob.waitForEvent('message', 10000);
await charlie.replyText(cRoomId, {
event_id: originalMessageId,
}, "Short reply");
let ircMessage = await bridgedMessage;

expect(ircMessage[2]).toContain("Short reply");
expect(ircMessage[2]).not.toContain("Original message");

await new Promise(r => setTimeout(r, 1000));

bridgedMessage = bob.waitForEvent('message', 10000);
await charlie.replyText(cRoomId, {
event_id: originalMessageId,
}, "Long reply");
ircMessage = await bridgedMessage;

expect(ircMessage[2]).toContain("Long reply");
expect(ircMessage[2]).toContain("Original message");
});
it('should not leak the contents of messages to new joiners', async () => {
await setupTestEnv(0);

const channel = `#${TestIrcServer.generateUniqueNick("test")}`;
const { homeserver, ircBridge } = testEnv;
const [alice, charlie] = homeserver.users.map(u => u.client);
const { bob } = testEnv.ircTest.clients;

// Create the channel
await bob.join(channel);

const adminRoomId = await testEnv.createAdminRoomHelper(alice);
const cRoomId = await testEnv.joinChannelHelper(alice, adminRoomId, channel);
const roomName = await alice.getRoomStateEvent(cRoomId, 'm.room.name', '');
expect(roomName.name).toEqual(channel);

// And finally wait for bob to appear.
const bobUserId = `@irc_${bob.nick}:${homeserver.domain}`;
await alice.waitForRoomEvent(
{eventType: 'm.room.member', sender: bobUserId, stateKey: bobUserId, roomId: cRoomId}
);

// Send some messages
const aliceMsg = bob.waitForEvent('message', 10000);
const bobMsg = alice.waitForRoomEvent(
{eventType: 'm.room.message', sender: bobUserId, roomId: cRoomId}
);
const aliceMsgBody = "Hello bib!";
const aliceEventId = alice.sendText(cRoomId, aliceMsgBody);
await aliceMsg;
bob.say(channel, "Hi alice!");
await bobMsg;

// Now charlie joins and tries to reply to alice.
await charlie.joinRoom(cRoomId);
const charlieMsgIrcPromise = bob.waitForEvent('message', 10000);
await charlie.replyText(cRoomId, {
event_id: await aliceEventId,
}, "Sneaky reply to a message I have not seen");

// Safety check to ensure that we're using the long form reply format.
expect(ircBridge.config.ircService.matrixHandler?.shortReplyTresholdSeconds).toBe(0);
// The long form reply format should not contain alice's message.
const charlieIrcMessage = (await charlieMsgIrcPromise)[2];
expect(charlieIrcMessage).not.toContain(aliceMsgBody);

// Now check that alice can reply, as they have been in the room long enough.
const aliceReplyMsgPromise = bob.waitForEvent('message', 10000);
await alice.replyText(cRoomId, {
event_id: await aliceEventId,
}, "Oh sorry, I meant bob!");
expect((await aliceReplyMsgPromise)[2]).toContain(aliceMsgBody);

// restart the bridge, effectively marking members as "been here forever"
await testEnv.recreateBridge();
await testEnv.setUp();
const postRestartAliceMsg = bob.waitForEvent('message', 10000);
const postRestartAliceMsgBody = "Hello post-restart world!";
const postRestartAliceEventId = alice.sendText(cRoomId, postRestartAliceMsgBody);
await postRestartAliceMsg;

const postRestartCharlieMsg = bob.waitForEvent('message', 10000);
await charlie.replyText(cRoomId, {
event_id: await postRestartAliceEventId,
}, "Hello alice!");
const postRestartCharlieMsgBody = (await postRestartCharlieMsg)[2];
expect(postRestartCharlieMsgBody).toContain(postRestartAliceMsgBody);
});
});
18 changes: 9 additions & 9 deletions spec/integ/matrix-to-irc.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ describe("Matrix-to-IRC message bridging", function() {
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$original:bar.com",
origin_server_ts: 1_000,
origin_server_ts: Date.now(),
type: "m.room.message"
});
const p = env.ircMock._whenClient(roomMapping.server, testUser.nick, "say",
Expand Down Expand Up @@ -274,7 +274,7 @@ describe("Matrix-to-IRC message bridging", function() {
},
sender: testUser.id,
room_id: roomMapping.roomId,
origin_server_ts: 2_000,
origin_server_ts: Date.now(),
type: "m.room.message"
});
await p;
Expand All @@ -290,7 +290,7 @@ describe("Matrix-to-IRC message bridging", function() {
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$original:bar.com",
origin_server_ts: 1_000,
origin_server_ts: Date.now(),
type: "m.room.message"
});
const p = env.ircMock._whenClient(roomMapping.server, testUser.nick, "say",
Expand Down Expand Up @@ -320,7 +320,7 @@ describe("Matrix-to-IRC message bridging", function() {
},
sender: testUser.id,
room_id: roomMapping.roomId,
origin_server_ts: 1_000_000,
origin_server_ts: Date.now() + 1_000_000,
type: "m.room.message"
});
await p;
Expand Down Expand Up @@ -381,7 +381,7 @@ describe("Matrix-to-IRC message bridging", function() {
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$original:bar.com",
origin_server_ts: 1_000,
origin_server_ts: Date.now(),
type: "m.room.message"
});
const p = env.ircMock._whenClient(roomMapping.server, testUser.nick, "say",
Expand Down Expand Up @@ -411,7 +411,7 @@ describe("Matrix-to-IRC message bridging", function() {
},
sender: testUser.id,
room_id: roomMapping.roomId,
origin_server_ts: 1_000_000,
origin_server_ts: Date.now() + 1_000_000,
type: "m.room.message"
});
await p;
Expand Down Expand Up @@ -465,7 +465,7 @@ describe("Matrix-to-IRC message bridging", function() {
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$first:bar.com",
origin_server_ts: 1_000,
origin_server_ts: Date.now(),
type: "m.room.message"
})

Expand All @@ -484,7 +484,7 @@ describe("Matrix-to-IRC message bridging", function() {
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$second:bar.com",
origin_server_ts: 1_000_000,
origin_server_ts: Date.now() + 1_000_000,
type: "m.room.message"
});

Expand Down Expand Up @@ -517,7 +517,7 @@ describe("Matrix-to-IRC message bridging", function() {
},
sender: testUser.id,
room_id: roomMapping.roomId,
origin_server_ts: 2_000_000,
origin_server_ts: Date.now() + 2_000_000,
type: "m.room.message"
});

Expand Down
7 changes: 7 additions & 0 deletions spec/util/e2e-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { expect } from "@jest/globals";
import dns from 'node:dns';
import fs from "node:fs/promises";
import { WriteStream, createWriteStream } from "node:fs";
import { DEFAULTS as MatrixHandlerDefaults } from "../../src/bridge/MatrixHandler";
// Needed to make tests work on GitHub actions. Node 17+ defaults
// to IPv6, and the homerunner domain resolves to IPv6, but the
// runtime doesn't actually support IPv6 🤦
Expand All @@ -22,12 +23,14 @@ const DEFAULT_PORT = parseInt(process.env.IRC_TEST_PORT ?? '6667', 10);
const DEFAULT_ADDRESS = process.env.IRC_TEST_ADDRESS ?? "127.0.0.1";
const IRCBRIDGE_TEST_REDIS_URL = process.env.IRCBRIDGE_TEST_REDIS_URL;


interface Opts {
matrixLocalparts?: string[];
ircNicks?: string[];
timeout?: number;
config?: Partial<BridgeConfig>,
traceToFile?: boolean,
shortReplyTresholdSeconds?: number,
}

const traceFilePath = '.e2e-traces';
Expand Down Expand Up @@ -235,6 +238,10 @@ export class IrcBridgeE2ETest {
ircHandler: {
powerLevelGracePeriodMs: 0,
},
matrixHandler: {
...MatrixHandlerDefaults,
shortReplyTresholdSeconds: opts.shortReplyTresholdSeconds ?? 0,
},
servers: {
localhost: {
...IrcServer.DEFAULT_CONFIG,
Expand Down
37 changes: 30 additions & 7 deletions src/bridge/MatrixHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { trackChannelAndCreateRoom } from "./RoomCreation";
import { renderTemplate } from "../util/Template";
import { trimString } from "../util/TrimString";
import { messageDiff } from "../util/MessageDiff";
import QuickLRU = require("quick-lru");

async function reqHandler(req: BridgeRequest, promise: PromiseLike<unknown>|void) {
try {
Expand Down Expand Up @@ -52,7 +53,7 @@ export interface MatrixHandlerConfig {
truncatedMessageTemplate: string;
}

const DEFAULTS: MatrixHandlerConfig = {
export const DEFAULTS: MatrixHandlerConfig = {
eventCacheSize: 4096,
replySourceMaxLength: 32,
shortReplyTresholdSeconds: 5 * 60,
Expand Down Expand Up @@ -106,6 +107,7 @@ export interface OnMemberEventData {
state_key: string;
type: string;
event_id: string;
origin_server_ts?: number;
content: {
displayname?: string;
membership: string;
Expand Down Expand Up @@ -135,6 +137,11 @@ export class MatrixHandler {
private adminHandler: AdminRoomHandler;
private config: MatrixHandlerConfig = DEFAULTS;

private memberJoinDefaultTs = Date.now();
private memberJoinTs = new QuickLRU<string, number>({
maxSize: 8192,
});

constructor(
private readonly ircBridge: IrcBridge,
config: MatrixHandlerConfig|undefined,
Expand Down Expand Up @@ -386,6 +393,9 @@ export class MatrixHandler {
* @param {Object} event : The Matrix member event.
*/
private _onMemberEvent(req: BridgeRequest, event: OnMemberEventData) {
if (event.origin_server_ts) {
this.memberJoinTs.set(`${event.room_id}/${event.state_key}`, event.origin_server_ts);
}
this.memberTracker?.onEvent(event);
}

Expand Down Expand Up @@ -1039,7 +1049,7 @@ export class MatrixHandler {
// special handling for replies (and threads)
if (event.content["m.relates_to"] && event.content["m.relates_to"]["m.in_reply_to"]) {
const eventId = event.content["m.relates_to"]["m.in_reply_to"].event_id;
const reply = await this.textForReplyEvent(event, eventId, ircRoom);
const reply = await this.textForReplyEvent(req, event, eventId, ircRoom);
if (reply !== null) {
ircAction.text = reply.formatted;
cacheBody = reply.reply;
Expand Down Expand Up @@ -1260,8 +1270,10 @@ export class MatrixHandler {
await this.ircBridge.getMatrixUser(ircUser);
}

private async textForReplyEvent(event: MatrixMessageEvent, replyEventId: string, ircRoom: IrcRoom):
Promise<{formatted: string; reply: string}|null> {
private async textForReplyEvent(
req: BridgeRequest, event: MatrixMessageEvent, replyEventId: string, ircRoom: IrcRoom
): Promise<{formatted: string; reply: string}|null> {
const bridgeIntent = this.ircBridge.getAppServiceBridge().getIntent();
// strips out the quotation of the original message, if needed
const replyText = (body: string): string => {
const REPLY_REGEX = /> <(.*?)>(.*?)\n\n([\s\S]*)/;
Expand All @@ -1285,7 +1297,7 @@ export class MatrixHandler {
if (!cachedEvent) {
// Fallback to fetching from the homeserver.
try {
const eventContent = await this.ircBridge.getAppServiceBridge().getIntent().getEvent(
const eventContent = await bridgeIntent.getEvent(
event.room_id, replyEventId
);
rplName = eventContent.sender;
Expand Down Expand Up @@ -1317,6 +1329,17 @@ export class MatrixHandler {
rplSource = cachedEvent.body;
}

const senderJoinTs = this.memberJoinTs.get(`${event.room_id}/${event.sender}`) ?? this.memberJoinDefaultTs;
if (senderJoinTs > cachedEvent.timestamp) {
// User joined AFTER the event was sent (or left and joined, but we can't distinguish that).
// Do not treat as a reply.
req.log.warn(`User ${event.sender} attempted to reply to an event before they were joined`);
return {
formatted: rplText,
reply: rplText,
};
}

// Get the first non-blank line from the source.
const lines = rplSource.split('\n').filter((line) => !/^\s*$/.test(line))
if (lines.length > 0) {
Expand Down Expand Up @@ -1347,8 +1370,8 @@ export class MatrixHandler {
}

let replyTemplate: string;
const tresholdMs = (this.config.shortReplyTresholdSeconds) * 1000;
if (rplSource && event.origin_server_ts - cachedEvent.timestamp > tresholdMs) {
const thresholdMs = (this.config.shortReplyTresholdSeconds) * 1000;
if (rplSource && event.origin_server_ts - cachedEvent.timestamp > thresholdMs) {
replyTemplate = this.config.longReplyTemplate;
}
else {
Expand Down

0 comments on commit 19bf636

Please sign in to comment.