From c719f1b926bacdd635f5e1e6d339dfffa6eaf19c Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Fri, 29 Jul 2022 09:57:58 -0400 Subject: [PATCH] Share code for tracking config via state events (#418) * Share code for tracking config via state events This should prevent forgetting to add state event handlers in new or existing connection types. * Address review feedback * Mark GitLabRepoConnection as an IConnection * Remove unused imports * Tighten typing for JiraProjectConnectionState --- changelog.d/418.misc | 1 + src/Connections/CommandConnection.ts | 17 ++++++++++++----- src/Connections/GithubRepo.ts | 12 ++++++------ src/Connections/GitlabRepo.ts | 17 ++++++++--------- src/Connections/IConnection.ts | 1 + src/Connections/JiraProject.ts | 23 +++++++++++------------ src/Connections/SetupConnection.ts | 9 ++++++++- 7 files changed, 47 insertions(+), 33 deletions(-) create mode 100644 changelog.d/418.misc diff --git a/changelog.d/418.misc b/changelog.d/418.misc new file mode 100644 index 00000000..9ad601b0 --- /dev/null +++ b/changelog.d/418.misc @@ -0,0 +1 @@ +Refactor the way room state is tracked for room-specific configuration, to increase code reuse. diff --git a/src/Connections/CommandConnection.ts b/src/Connections/CommandConnection.ts index fb82529f..4ec19acb 100644 --- a/src/Connections/CommandConnection.ts +++ b/src/Connections/CommandConnection.ts @@ -3,33 +3,40 @@ import LogWrapper from "../LogWrapper"; import { MatrixClient } from "matrix-bot-sdk"; import { MatrixMessageContent, MatrixEvent } from "../MatrixEvent"; import { BaseConnection } from "./BaseConnection"; -import { PermissionCheckFn } from "."; +import { IConnectionState, PermissionCheckFn } from "."; const log = new LogWrapper("CommandConnection"); /** * Connection class that handles commands for a given connection. Should be used * by connections expecting to handle user input. */ -export abstract class CommandConnection extends BaseConnection { +export abstract class CommandConnection extends BaseConnection { protected enabledHelpCategories?: string[]; protected includeTitlesInHelp?: boolean; constructor( roomId: string, stateKey: string, canonicalStateType: string, + protected state: StateType, private readonly botClient: MatrixClient, private readonly botCommands: BotCommands, private readonly helpMessage: HelpFunction, - protected readonly stateCommandPrefix: string, + protected readonly defaultCommandPrefix: string, protected readonly serviceName?: string, ) { super(roomId, stateKey, canonicalStateType); - } + } protected get commandPrefix() { - return this.stateCommandPrefix + " "; + return (this.state.commandPrefix || this.defaultCommandPrefix) + " "; } + public async onStateUpdate(stateEv: MatrixEvent) { + this.state = this.validateConnectionState(stateEv.content); + } + + protected abstract validateConnectionState(content: unknown): StateType; + public async onMessageEvent(ev: MatrixEvent, checkPermission: PermissionCheckFn) { const commandResult = await handleCommand( ev.sender, ev.content.body, this.botCommands, this,checkPermission, diff --git a/src/Connections/GithubRepo.ts b/src/Connections/GithubRepo.ts index 4e7b5a32..ce7f0e38 100644 --- a/src/Connections/GithubRepo.ts +++ b/src/Connections/GithubRepo.ts @@ -36,7 +36,6 @@ interface IQueryRoomOpts { export interface GitHubRepoConnectionOptions extends IConnectionState { ignoreHooks?: AllowedEventsNames[], - commandPrefix?: string; showIssueRoomLink?: boolean; prDiff?: { enabled: boolean; @@ -212,7 +211,7 @@ function compareEmojiStrings(e0: string, e1: string, e0Index = 0) { * Handles rooms connected to a GitHub repo. */ @Connection -export class GitHubRepoConnection extends CommandConnection implements IConnection { +export class GitHubRepoConnection extends CommandConnection implements IConnection { static validateState(state: Record, isExistingState = false): GitHubRepoConnectionState { const validator = new Ajv().compile(ConnectionStateSchema); @@ -368,7 +367,7 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti constructor(roomId: string, private readonly as: Appservice, - private state: GitHubRepoConnectionState, + state: GitHubRepoConnectionState, private readonly tokenStore: UserTokenStore, stateKey: string, private readonly githubInstance: GithubInstance, @@ -378,10 +377,11 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti roomId, stateKey, GitHubRepoConnection.CanonicalEventType, + state, as.botClient, GitHubRepoConnection.botCommands, GitHubRepoConnection.helpMessage, - state.commandPrefix || "!gh", + "!gh", "github", ); } @@ -415,8 +415,8 @@ export class GitHubRepoConnection extends CommandConnection implements IConnecti return this.state.priority || super.priority; } - public async onStateUpdate(stateEv: MatrixEvent) { - this.state = stateEv.content as GitHubRepoConnectionState; + protected validateConnectionState(content: unknown) { + return content as GitHubRepoConnectionState; } public isInterestedInStateEvent(eventType: string, stateKey: string) { diff --git a/src/Connections/GitlabRepo.ts b/src/Connections/GitlabRepo.ts index 1c24f73d..40e10138 100644 --- a/src/Connections/GitlabRepo.ts +++ b/src/Connections/GitlabRepo.ts @@ -3,13 +3,13 @@ import { UserTokenStore } from "../UserTokenStore"; import { Appservice, StateEvent } from "matrix-bot-sdk"; import { BotCommands, botCommand, compileBotCommands } from "../BotCommands"; -import { MatrixEvent, MatrixMessageContent } from "../MatrixEvent"; +import { MatrixMessageContent } from "../MatrixEvent"; import markdown from "markdown-it"; import LogWrapper from "../LogWrapper"; import { BridgeConfigGitLab, GitLabInstance } from "../Config/Config"; import { IGitLabWebhookMREvent, IGitLabWebhookNoteEvent, IGitLabWebhookPushEvent, IGitLabWebhookReleaseEvent, IGitLabWebhookTagPushEvent, IGitLabWebhookWikiPageEvent } from "../Gitlab/WebhookTypes"; import { CommandConnection } from "./CommandConnection"; -import { Connection, IConnectionState, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection"; +import { Connection, IConnection, IConnectionState, InstantiateConnectionOpts, ProvisionConnectionOpts } from "./IConnection"; import { GetConnectionsResponseItem } from "../provisioning/api"; import { ErrCode, ApiError, ValidatorApiError } from "../api" import { AccessLevel } from "../Gitlab/Types"; @@ -19,7 +19,6 @@ export interface GitLabRepoConnectionState extends IConnectionState { instance: string; path: string; ignoreHooks?: AllowedEventsNames[], - commandPrefix?: string; pushTagsRegex?: string, includingLabels?: string[]; excludingLabels?: string[]; @@ -130,7 +129,7 @@ export interface GitLabTargetFilter { * Handles rooms connected to a GitLab repo. */ @Connection -export class GitLabRepoConnection extends CommandConnection { +export class GitLabRepoConnection extends CommandConnection implements IConnection { static readonly CanonicalEventType = "uk.half-shot.matrix-hookshot.gitlab.repository"; static readonly LegacyCanonicalEventType = "uk.half-shot.matrix-github.gitlab.repository"; @@ -277,17 +276,18 @@ export class GitLabRepoConnection extends CommandConnection { constructor(roomId: string, stateKey: string, private readonly as: Appservice, - private state: GitLabRepoConnectionState, + state: GitLabRepoConnectionState, private readonly tokenStore: UserTokenStore, private readonly instance: GitLabInstance) { super( roomId, stateKey, GitLabRepoConnection.CanonicalEventType, + state, as.botClient, GitLabRepoConnection.botCommands, GitLabRepoConnection.helpMessage, - state.commandPrefix || "!gl", + "!gl", "gitlab", ) if (!state.path || !state.instance) { @@ -303,9 +303,8 @@ export class GitLabRepoConnection extends CommandConnection { return this.state.priority || super.priority; } - public async onStateUpdate(stateEv: MatrixEvent) { - const state = stateEv.content as GitLabRepoConnectionState; - this.state = state; + protected validateConnectionState(content: unknown) { + return content as GitLabRepoConnectionState; } public isInterestedInStateEvent(eventType: string, stateKey: string) { diff --git a/src/Connections/IConnection.ts b/src/Connections/IConnection.ts index 910d6d91..8a73d1b4 100644 --- a/src/Connections/IConnection.ts +++ b/src/Connections/IConnection.ts @@ -14,6 +14,7 @@ export type PermissionCheckFn = (service: string, level: BridgePermissionLevel) export interface IConnectionState { priority?: number; + commandPrefix?: string; } export interface IConnection { diff --git a/src/Connections/JiraProject.ts b/src/Connections/JiraProject.ts index 53a2afb6..64867a1b 100644 --- a/src/Connections/JiraProject.ts +++ b/src/Connections/JiraProject.ts @@ -7,7 +7,7 @@ import markdownit from "markdown-it"; import { generateJiraWebLinkFromIssue } from "../Jira"; import { JiraProject } from "../Jira/Types"; import { botCommand, BotCommands, compileBotCommands } from "../BotCommands"; -import { MatrixEvent, MatrixMessageContent } from "../MatrixEvent"; +import { MatrixMessageContent } from "../MatrixEvent"; import { CommandConnection } from "./CommandConnection"; import { UserTokenStore } from "../UserTokenStore"; import { CommandError, NotLoggedInError } from "../errors"; @@ -19,13 +19,12 @@ const JiraAllowedEvents: JiraAllowedEventsNames[] = ["issue.created"]; export interface JiraProjectConnectionState extends IConnectionState { // legacy field, prefer url id?: string; - url?: string; + url: string; events?: JiraAllowedEventsNames[], - commandPrefix?: string; } -function validateJiraConnectionState(state: JiraProjectConnectionState) { - const {url, commandPrefix, events, priority} = state as JiraProjectConnectionState; +function validateJiraConnectionState(state: unknown): JiraProjectConnectionState { + const {url, commandPrefix, events, priority} = state as Partial; if (url === undefined) { throw new ApiError("Expected a 'url' property", ErrCode.BadValue); } @@ -50,7 +49,7 @@ const md = new markdownit(); * Handles rooms connected to a Jira project. */ @Connection -export class JiraProjectConnection extends CommandConnection implements IConnection { +export class JiraProjectConnection extends CommandConnection implements IConnection { static readonly CanonicalEventType = "uk.half-shot.matrix-hookshot.jira.project"; @@ -83,7 +82,7 @@ export class JiraProjectConnection extends CommandConnection implements IConnect if (!jiraResourceClient) { throw new ApiError("User is not authenticated with this JIRA instance", ErrCode.ForbiddenUser); } - const connection = new JiraProjectConnection(roomId, as, data, validData.url, tokenStore); + const connection = new JiraProjectConnection(roomId, as, validData, validData.url, tokenStore); log.debug(`projectKey for ${validData.url} is ${connection.projectKey}`); if (!connection.projectKey) { throw Error('Expected projectKey to be defined'); @@ -151,17 +150,18 @@ export class JiraProjectConnection extends CommandConnection implements IConnect constructor(roomId: string, private readonly as: Appservice, - private state: JiraProjectConnectionState, + state: JiraProjectConnectionState, stateKey: string, private readonly tokenStore: UserTokenStore,) { super( roomId, stateKey, JiraProjectConnection.CanonicalEventType, + state, as.botClient, JiraProjectConnection.botCommands, JiraProjectConnection.helpMessage, - state.commandPrefix || "!jira", + "!jira", "jira" ); if (state.url) { @@ -178,9 +178,8 @@ export class JiraProjectConnection extends CommandConnection implements IConnect return JiraProjectConnection.EventTypes.includes(eventType) && this.stateKey === stateKey; } - public async onStateUpdate(event: MatrixEvent) { - const validatedConfig = validateJiraConnectionState(event.content as JiraProjectConnectionState); - this.state = validatedConfig; + protected validateConnectionState(content: unknown) { + return validateJiraConnectionState(content); } public async onJiraIssueCreated(data: JiraIssueEvent) { diff --git a/src/Connections/SetupConnection.ts b/src/Connections/SetupConnection.ts index a66204e5..3386e066 100644 --- a/src/Connections/SetupConnection.ts +++ b/src/Connections/SetupConnection.ts @@ -12,7 +12,7 @@ import { URL } from "url"; import { SetupWidget } from "../Widgets/SetupWidget"; import { AdminRoom } from "../AdminRoom"; import { GitLabRepoConnection } from "./GitlabRepo"; -import { ProvisionConnectionOpts } from "./IConnection"; +import { IConnectionState, ProvisionConnectionOpts } from "./IConnection"; import LogWrapper from "../LogWrapper"; const md = new markdown(); const log = new LogWrapper("SetupConnection"); @@ -34,6 +34,11 @@ export class SetupConnection extends CommandConnection { return this.provisionOpts.as; } + protected validateConnectionState(content: unknown) { + log.warn("SetupConnection has no state to be validated"); + return content as IConnectionState; + } + constructor(public readonly roomId: string, private readonly provisionOpts: ProvisionConnectionOpts, private readonly getOrCreateAdminRoom: (userId: string) => Promise,) { @@ -41,6 +46,8 @@ export class SetupConnection extends CommandConnection { roomId, "", "", + // TODO Consider storing room-specific config in state. + {}, provisionOpts.as.botClient, SetupConnection.botCommands, SetupConnection.helpMessage,