diff --git a/src/Bridge.ts b/src/Bridge.ts index 0d15728f..4a7ddad0 100644 --- a/src/Bridge.ts +++ b/src/Bridge.ts @@ -789,7 +789,7 @@ export class Bridge { try { await ( new SetupConnection( - roomId, this.as, this.tokenStore, this.config, + roomId, this.as, this.tokenStore, this.storage, this.config, this.getOrCreateAdminRoom.bind(this), this.github, ) diff --git a/src/ConnectionManager.ts b/src/ConnectionManager.ts index 5256b2e9..98f159e2 100644 --- a/src/ConnectionManager.ts +++ b/src/ConnectionManager.ts @@ -96,7 +96,7 @@ export class ConnectionManager { if (!this.config.checkPermission(userId, "github", BridgePermissionLevel.manageConnections)) { throw new ApiError('User is not permitted to provision connections for GitHub', ErrCode.ForbiddenUser); } - const res = await GitHubRepoConnection.provisionConnection(roomId, userId, data, this.as, this.tokenStore, this.github, this.config.github); + const res = await GitHubRepoConnection.provisionConnection(roomId, userId, data, this.as, this.tokenStore, this.github, this.config.github, this.storage); await this.as.botIntent.underlyingClient.sendStateEvent(roomId, GitHubRepoConnection.CanonicalEventType, res.connection.stateKey, res.stateEventContent); this.push(res.connection); return res.connection; @@ -145,7 +145,7 @@ export class ConnectionManager { throw Error('GitHub is not configured'); } this.assertStateAllowed(state, "github"); - return new GitHubRepoConnection(roomId, this.as, state.content, this.tokenStore, state.stateKey, this.github, this.config.github); + return new GitHubRepoConnection(roomId, this.as, state.content, this.tokenStore, this.storage, state.stateKey, this.github, this.config.github); } if (GitHubDiscussionConnection.EventTypes.includes(state.type)) { diff --git a/src/Connections/GithubRepo.ts b/src/Connections/GithubRepo.ts index 179c3fde..f399bf26 100644 --- a/src/Connections/GithubRepo.ts +++ b/src/Connections/GithubRepo.ts @@ -21,6 +21,7 @@ import { BridgeConfigGitHub } from "../Config/Config"; import { ApiError, ErrCode } from "../api"; import { PermissionCheckFn } from "."; import { MinimalGitHubIssue, MinimalGitHubRepo } from "../libRs"; +import { IBridgeStorageProvider } from "../Stores/StorageProvider"; const log = new LogWrapper("GitHubRepoConnection"); const md = new markdown(); @@ -48,6 +49,7 @@ export interface GitHubRepoConnectionOptions extends IConnectionState { newIssue?: { labels: string[]; }; + useThreads?: boolean; } export interface GitHubRepoConnectionState extends GitHubRepoConnectionOptions { org: string; @@ -150,7 +152,7 @@ function validateState(state: Record): GitHubRepoConnectionStat */ export class GitHubRepoConnection extends CommandConnection implements IConnection { static async provisionConnection(roomId: string, userId: string, data: Record, as: Appservice, - tokenStore: UserTokenStore, githubInstance: GithubInstance, config: BridgeConfigGitHub) { + tokenStore: UserTokenStore, githubInstance: GithubInstance, config: BridgeConfigGitHub, store: IBridgeStorageProvider) { const validData = validateState(data); const octokit = await tokenStore.getOctokitForUser(userId); if (!octokit) { @@ -183,7 +185,7 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti const stateEventKey = `${validData.org}/${validData.repo}`; return { stateEventContent: validData, - connection: new GitHubRepoConnection(roomId, as, validData, tokenStore, stateEventKey, githubInstance, config), + connection: new GitHubRepoConnection(roomId, as, validData, tokenStore, store, stateEventKey, githubInstance, config), } } @@ -281,6 +283,7 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti private readonly as: Appservice, private state: GitHubRepoConnectionState, private readonly tokenStore: UserTokenStore, + private readonly store: IBridgeStorageProvider, stateKey: string, private readonly githubInstance: GithubInstance, private readonly config: BridgeConfigGitHub, @@ -297,6 +300,32 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti ); } + protected async setThreadForRemoteId(eventId: string, remoteId: string|number) { + // Store regardless of configuration, in case the user wants to turn it on. + await this.store.setEventIdForRemoteId(`${this.connectionId}/${remoteId}`, eventId); + log.debug(`Stored thread eventId for ${remoteId} as ${eventId}`); + } + + protected async getThreadForRemoteId(remoteId: string|number, renderInTimeline = false): Promise|undefined> { + const parentEventId = this.state.useThreads && await this.store.getEventIdForRemoteId(`${this.connectionId}/${remoteId}`); + log.debug(`Found ${parentEventId || "no eventId"} for ${remoteId}`); + if (!parentEventId) { + return; + } + + return { + "m.relates_to": { + rel_type: renderInTimeline ? undefined : "m.thread", + event_id: parentEventId, + // Needed to prevent clients from showing these as actual replies + is_falling_back: true, + "m.in_reply_to": { + event_id: parentEventId, + } + }, + } + } + public get hotlinkIssues() { const cfg = this.config.defaultOptions?.hotlinkIssues || this.state.hotlinkIssues; if (cfg === false) { @@ -582,13 +611,14 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } const content = emoji.emojify(message); const labels = FormatUtil.formatLabels(event.issue.labels?.map(l => ({ name: l.name, description: l.description || undefined, color: l.color || undefined }))); - await this.as.botIntent.sendEvent(this.roomId, { + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content + (labels.plain.length > 0 ? ` with labels ${labels.plain}`: ""), formatted_body: md.renderInline(content) + (labels.html.length > 0 ? ` with labels ${labels.html}`: ""), format: "org.matrix.custom.html", ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.issue), }); + await this.setThreadForRemoteId(eventId, event.issue.id); } public async onIssueStateChange(event: IssuesEditedEvent|IssuesReopenedEvent|IssuesClosedEvent) { @@ -628,13 +658,18 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } } const content = `**${event.sender.login}** ${state} issue [${orgRepoName}#${event.issue.number}](${event.issue.html_url}): "${emoji.emojify(event.issue.title)}"${withComment}`; - await this.as.botIntent.sendEvent(this.roomId, { + const thread = await this.getThreadForRemoteId(event.issue.id, true); + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", + ...thread, ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.issue), }); + if (!thread) { + this.setThreadForRemoteId(eventId, event.issue.id); + } } public async onIssueEdited(event: IssuesEditedEvent) { @@ -647,13 +682,18 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti log.info(`onIssueEdited ${this.roomId} ${this.org}/${this.repo} #${event.issue.number}`); const orgRepoName = event.repository.full_name; const content = `**${event.sender.login}** edited issue [${orgRepoName}#${event.issue.number}](${event.issue.html_url}): "${emoji.emojify(event.issue.title)}"`; - await this.as.botIntent.sendEvent(this.roomId, { + const thread = await this.getThreadForRemoteId(event.issue.id); + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", + ...thread, ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.issue), }); + if (!thread) { + this.setThreadForRemoteId(eventId, event.issue.id); + } } public async onIssueLabeled(event: IssuesLabeledEvent) { @@ -678,12 +718,15 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti const orgRepoName = event.repository.full_name; const {plain, html} = FormatUtil.formatLabels(event.issue.labels?.map(l => ({ name: l.name, description: l.description || undefined, color: l.color || undefined }))); const content = `**${event.sender.login}** labeled issue [${orgRepoName}#${event.issue.number}](${event.issue.html_url}): "${emoji.emojify(event.issue.title)}"`; - this.as.botIntent.sendEvent(this.roomId, { - msgtype: "m.notice", - body: content + (plain.length > 0 ? ` with labels ${plain}`: ""), - formatted_body: md.renderInline(content) + (html.length > 0 ? ` with labels ${html}`: ""), - format: "org.matrix.custom.html", - ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.issue), + this.getThreadForRemoteId(event.issue.id).then((thread) => { + return this.as.botIntent.sendEvent(this.roomId, { + msgtype: "m.notice", + body: content + (plain.length > 0 ? ` with labels ${plain}`: ""), + formatted_body: md.renderInline(content) + (html.length > 0 ? ` with labels ${html}`: ""), + format: "org.matrix.custom.html", + ...thread, + ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.issue), + }) }).catch(ex => { log.error('Failed to send onIssueLabeled message', ex); }); @@ -736,14 +779,19 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } const content = emoji.emojify(`**${event.sender.login}** ${verb} a new PR [${orgRepoName}#${event.pull_request.number}](${event.pull_request.html_url}): "${event.pull_request.title}"`); const labels = FormatUtil.formatLabels(event.pull_request.labels?.map(l => ({ name: l.name, description: l.description || undefined, color: l.color || undefined }))); - await this.as.botIntent.sendEvent(this.roomId, { + const thread = await this.getThreadForRemoteId(event.pull_request.id); + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content + (labels.plain.length > 0 ? ` with labels ${labels}`: "") + diffContent, formatted_body: md.renderInline(content) + (labels.html.length > 0 ? ` with labels ${labels.html}`: "") + diffContentHtml, format: "org.matrix.custom.html", + ...thread, ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.pull_request), ...FormatUtil.getPartialBodyForGitHubPR(event.repository, event.pull_request), }); + if (!thread) { + this.setThreadForRemoteId(eventId, event.pull_request.id); + } } public async onPRReadyForReview(event: PullRequestReadyForReviewEvent) { @@ -759,14 +807,18 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } const orgRepoName = event.repository.full_name; const content = emoji.emojify(`**${event.sender.login}** has marked [${orgRepoName}#${event.pull_request.number}](${event.pull_request.html_url}) as ready to review "${event.pull_request.title}"`); - await this.as.botIntent.sendEvent(this.roomId, { + const thread = await this.getThreadForRemoteId(event.pull_request.id, true); + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", - // TODO: Fix types. + ...thread, ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.pull_request), }); + if (!thread) { + this.setThreadForRemoteId(eventId, event.pull_request.id); + } } public async onPRReviewed(event: PullRequestReviewSubmittedEvent) { @@ -792,14 +844,18 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti return; } const content = emoji.emojify(`**${event.sender.login}** ${emojiForReview} ${event.review.state.toLowerCase()} [${orgRepoName}#${event.pull_request.number}](${event.pull_request.html_url}) "${event.pull_request.title}"`); - await this.as.botIntent.sendEvent(this.roomId, { + const thread = await this.getThreadForRemoteId(event.pull_request.id, true); + const eventId = await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", - // TODO: Fix types. + ...thread, ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.pull_request), }); + if (!thread) { + this.setThreadForRemoteId(eventId, event.pull_request.id); + } } public async onPRClosed(event: PullRequestClosedEvent) { @@ -838,11 +894,13 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti } const content = emoji.emojify(`**${event.sender.login}** ${verb} PR [${orgRepoName}#${event.pull_request.number}](${event.pull_request.html_url}): "${event.pull_request.title}"${withComment}`); + const thread = await this.getThreadForRemoteId(event.pull_request.id, true); await this.as.botIntent.sendEvent(this.roomId, { msgtype: "m.notice", body: content, formatted_body: md.renderInline(content), format: "org.matrix.custom.html", + ...thread, // TODO: Fix types. ...FormatUtil.getPartialBodyForGithubIssue(event.repository, event.pull_request), }); diff --git a/src/Connections/SetupConnection.ts b/src/Connections/SetupConnection.ts index 2293f002..da13b3b8 100644 --- a/src/Connections/SetupConnection.ts +++ b/src/Connections/SetupConnection.ts @@ -13,6 +13,7 @@ import { FigmaFileConnection } from "./FigmaFileConnection"; import { URL } from "url"; import { SetupWidget } from "../Widgets/SetupWidget"; import { AdminRoom } from "../AdminRoom"; +import { IBridgeStorageProvider } from "../Stores/StorageProvider"; const md = new markdown(); /** @@ -27,6 +28,7 @@ export class SetupConnection extends CommandConnection { constructor(public readonly roomId: string, private readonly as: Appservice, private readonly tokenStore: UserTokenStore, + private readonly store: IBridgeStorageProvider, private readonly config: BridgeConfig, private readonly getOrCreateAdminRoom: (userId: string) => Promise, private readonly githubInstance?: GithubInstance,) { @@ -73,7 +75,7 @@ export class SetupConnection extends CommandConnection { throw new CommandError("Invalid GitHub url", "The GitHub url you entered was not valid."); } const [, org, repo] = urlParts; - const res = await GitHubRepoConnection.provisionConnection(this.roomId, userId, {org, repo}, this.as, this.tokenStore, this.githubInstance, this.config.github); + const res = await GitHubRepoConnection.provisionConnection(this.roomId, userId, {org, repo}, this.as, this.tokenStore, this.githubInstance, this.config.github, this.store); await this.as.botClient.sendStateEvent(this.roomId, GitHubRepoConnection.CanonicalEventType, url, res.stateEventContent); await this.as.botClient.sendNotice(this.roomId, `Room configured to bridge ${org}/${repo}`); } diff --git a/src/Stores/MemoryStorageProvider.ts b/src/Stores/MemoryStorageProvider.ts index 6f4cd7fb..281dc957 100644 --- a/src/Stores/MemoryStorageProvider.ts +++ b/src/Stores/MemoryStorageProvider.ts @@ -2,6 +2,7 @@ import { MemoryStorageProvider as MSP } from "matrix-bot-sdk"; import { IBridgeStorageProvider } from "./StorageProvider"; import { IssuesGetResponseData } from "../Github/Types"; import { ProvisionSession } from "matrix-appservice-bridge"; +import QuickLRU from "@alloc/quick-lru"; export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider { private issues: Map = new Map(); @@ -10,6 +11,9 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider private figmaCommentIds: Map = new Map(); private widgetSessions: Map = new Map(); + // Set to a suitably large, but bounded size. + private eventIdForRemoteId = new QuickLRU({ maxSize: 5000 }); + constructor() { super(); } @@ -54,16 +58,26 @@ export class MemoryStorageProvider extends MSP implements IBridgeStorageProvider public async getSessionForToken(token: string) { return this.widgetSessions.get(token) || null; } + public async createSession(session: ProvisionSession) { this.widgetSessions.set(session.token, session); } + public async deleteSession(token: string) { this.widgetSessions.delete(token); } + public async deleteAllSessions(userId: string) { [...this.widgetSessions.values()] .filter(s => s.userId === userId) .forEach(s => this.widgetSessions.delete(s.token)); } + public async getEventIdForRemoteId(remoteId: string) { + return this.eventIdForRemoteId.get(remoteId) ?? null; + } + + public async setEventIdForRemoteId(remoteId: string, eventId: string) { + this.eventIdForRemoteId.set(remoteId, eventId); + } } diff --git a/src/Stores/RedisStorageProvider.ts b/src/Stores/RedisStorageProvider.ts index bb982d5c..9815762c 100644 --- a/src/Stores/RedisStorageProvider.ts +++ b/src/Stores/RedisStorageProvider.ts @@ -15,9 +15,12 @@ const GH_ISSUES_KEY = "gh.issues"; const GH_ISSUES_LAST_COMMENT_KEY = "gh.issues.last_comment"; const GH_ISSUES_REVIEW_DATA_KEY = "gh.issues.review_data"; const FIGMA_EVENT_COMMENT_ID = "figma.comment_event_id"; +const REMOTE_EVENTS_HASH = "remote_events"; +const REMOTE_EVENTS_LRU = "remote_events_lru"; const COMPLETED_TRANSACTIONS_EXPIRE_AFTER = 24 * 60 * 60; // 24 hours const ISSUES_EXPIRE_AFTER = 7 * 24 * 60 * 60; // 7 days const ISSUES_LAST_COMMENT_EXPIRE_AFTER = 14 * 24 * 60 * 60; // 7 days +const REMOTE_EVENTS_MAX_SIZE = 5000; const WIDGET_TOKENS = "widgets.tokens."; @@ -155,4 +158,23 @@ export class RedisStorageProvider implements IBridgeStorageProvider { token = await this.redis.spop(`${WIDGET_USER_TOKENS}${userId}`); } } + + async getEventIdForRemoteId(remoteId: string): Promise { + const result = await this.redis.hget(REMOTE_EVENTS_HASH, remoteId); + if (result) { + await this.redis.zadd(REMOTE_EVENTS_LRU, Date.now(), remoteId); + } + return result; + } + + async setEventIdForRemoteId(remoteId: string, eventId: string): Promise { + await this.redis.hset(REMOTE_EVENTS_HASH, remoteId, eventId); + await this.redis.zadd(REMOTE_EVENTS_LRU, Date.now(), remoteId); + const lruCount = await this.redis.zcount(REMOTE_EVENTS_LRU, "-inf", "+inf"); + if (lruCount <= REMOTE_EVENTS_MAX_SIZE) { + return; + } + const popped = await this.redis.zpopmin(REMOTE_EVENTS_LRU, lruCount - REMOTE_EVENTS_MAX_SIZE); + await this.redis.zrem(REMOTE_EVENTS_HASH, popped); + } } diff --git a/src/Stores/StorageProvider.ts b/src/Stores/StorageProvider.ts index 52ff39da..aaa61160 100644 --- a/src/Stores/StorageProvider.ts +++ b/src/Stores/StorageProvider.ts @@ -11,4 +11,6 @@ export interface IBridgeStorageProvider extends IAppserviceStorageProvider, ISto getPRReviewData(repo: string, issueNumber: string, scope?: string): Promise; setFigmaCommentEventId(roomId: string, figmaCommentId: string, eventId: string): Promise; getFigmaCommentEventId(roomId: string, figmaCommentId: string): Promise; + setEventIdForRemoteId(remoteId: string, eventId: string): Promise; + getEventIdForRemoteId(remoteId: string): Promise; } \ No newline at end of file