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.

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

Types of Cli and HOWTO.md improvements #224

Merged
merged 10 commits into from
Sep 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion HOWTO.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ an application service, listen and handle incoming Matrix requests and expose a
Open up `index.js` and add this at the bottom of the file:
```javascript
var Cli = require("matrix-appservice-bridge").Cli;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I wonder if we should promote good practices and use const/let

var Bridge = require("matrix-appservice-bridge").Bridge;
var Bridge = require("matrix-appservice-bridge").Bridge; // we will use this later
var AppServiceRegistration = require("matrix-appservice-bridge").AppServiceRegistration;

new Cli({
Expand Down Expand Up @@ -244,6 +244,7 @@ var AppServiceRegistration = require("matrix-appservice-bridge").AppServiceRegis
new Cli({
registrationPath: "slack-registration.yaml",
generateRegistration: function(reg, callback) {
reg.setId(AppServiceRegistration.generateToken());
reg.setHomeserverToken(AppServiceRegistration.generateToken());
reg.setAppServiceToken(AppServiceRegistration.generateToken());
reg.setSenderLocalpart("slackbot");
Expand Down
2 changes: 2 additions & 0 deletions changelog.d/224.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Types: Make some options of Cli optional
Small corrections to HOWTO.md
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"version": "1.13.2",
"description": "Bridging infrastructure for Matrix Application Services",
"main": "lib/index.js",
"types": "lib/index.d.ts",
"scripts": {
"build": "tsc --project tsconfig.json",
"prepare": "npm run build",
Expand Down
92 changes: 57 additions & 35 deletions src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import { RemoteUser } from "./models/users/remote";
import BridgeInternalError = unstable.BridgeInternalError;
import wrapError = unstable.wrapError;
import EventNotHandledError = unstable.EventNotHandledError;
import EventUnknownError = unstable.EventUnknownError;
import { ThirdpartyProtocolResponse, ThirdpartyLocationResponse, ThirdpartyUserResponse } from "./thirdparty";
import { RemoteRoom } from "./models/rooms/remote";
import { Registry } from "prom-client";
Expand All @@ -58,15 +57,15 @@ const INTENT_CULL_CHECK_PERIOD_MS = 1000 * 60; // once per minute
const INTENT_CULL_EVICT_AFTER_MS = 1000 * 60 * 15; // 15 minutes

export interface WeakEvent extends Record<string, unknown> {
event_id: string;
room_id: string;
event_id: string; // eslint-disable-line camelcase
room_id: string; // eslint-disable-line camelcase
sender: string;
content: Record<string,unknown>;
content: Record<string, unknown>;
unsigned: {
age: number;
}
origin_server_ts: number;
state_key: string;
origin_server_ts: number; // eslint-disable-line camelcase
state_key: string; // eslint-disable-line camelcase
type: string;
}

Expand Down Expand Up @@ -103,12 +102,14 @@ interface BridgeOpts {
* not supplied, no users will be provisioned on user queries. Provisioned users
* will automatically be stored in the associated `userStore`.
*/
onUserQuery?: (matrixUser: MatrixUser) => PossiblePromise<{name?: string, url?: string, remote?: RemoteUser}|null|void>;
onUserQuery?: (matrixUser: MatrixUser) =>
PossiblePromise<{name?: string, url?: string, remote?: RemoteUser}|null|void>;
/**
* The bridge will invoke this function when queried via onAliasQuery. If
* not supplied, no rooms will be provisioned on alias queries. Provisioned rooms
* will automatically be stored in the associated `roomStore`. */
onAliasQuery?: (alias: string, aliasLocalpart: string) => PossiblePromise<{creationOpts: Record<string, unknown>, remote?: RemoteRoom}|null|void>;
onAliasQuery?: (alias: string, aliasLocalpart: string) =>
PossiblePromise<{creationOpts: Record<string, unknown>, remote?: RemoteRoom}|null|void>;
/**
* The bridge will invoke this function when a room has been created
* via onAliasQuery.
Expand Down Expand Up @@ -253,7 +254,7 @@ export class Bridge {
private queue: EventQueue;
private intentBackingStore: IntentBackingStore;
private prevRequestPromise: Promise<unknown>;
private readonly onLog: (message: string, isError : boolean) => void;
private readonly onLog: (message: string, isError: boolean) => void;

private intentLastAccessedTimeout: NodeJS.Timeout|null = null;
private botIntent?: Intent;
Expand Down Expand Up @@ -403,15 +404,15 @@ export class Bridge {
this.eventStore = eventStore as EventBridgeStore;
}

/**
* Run the bridge (start listening)
* @param port The port to listen on.
* @param config Configuration options
* @param appServiceInstance The AppService instance to attach to.
* If not provided, one will be created.
* @param hostname Optional hostname to bind to. (e.g. 0.0.0.0)
* @return A promise resolving when the bridge is ready
*/
/**
* Run the bridge (start listening)
* @param port The port to listen on.
* @param config Configuration options
* @param appServiceInstance The AppService instance to attach to.
* If not provided, one will be created.
* @param hostname Optional hostname to bind to. (e.g. 0.0.0.0)
* @return A promise resolving when the bridge is ready
*/
public async run<T>(port: number, config: T, appServiceInstance?: AppService, hostname?: string, backlog = 10) {
// Load the registration file into an AppServiceRegistration object.
if (typeof this.opts.registration === "string") {
Expand All @@ -422,7 +423,7 @@ export class Bridge {
}
this.registration = registration;
}
else{
else {
this.registration = this.opts.registration;
}

Expand Down Expand Up @@ -520,7 +521,7 @@ export class Bridge {
this.roomLinkValidator?.readRuleFile(req.query.filename as string|undefined);
res.status(200).send("Success");
}
else {
else {
res.status(404).send("RoomLinkValidator not in use");
}
}
Expand All @@ -532,9 +533,9 @@ export class Bridge {
}
}

// Set a timer going which will periodically remove Intent objects to prevent
// them from accumulating too much. Removal is based on access time (calls to
// getIntent). Intents expire after `INTENT_CULL_EVICT_AFTER_MS` of not being called.
// Set a timer going which will periodically remove Intent objects to prevent
// them from accumulating too much. Removal is based on access time (calls to
// getIntent). Intents expire after `INTENT_CULL_EVICT_AFTER_MS` of not being called.
private setupIntentCulling() {
if (this.intentLastAccessedTimeout) {
clearTimeout(this.intentLastAccessedTimeout);
Expand Down Expand Up @@ -589,7 +590,7 @@ export class Bridge {
const result = await getProtocolFunc(protocol);
res.status(200).json(result);
}
catch (ex) {
catch (ex) {
respondErr(ex, res)
}
},
Expand Down Expand Up @@ -618,7 +619,7 @@ export class Bridge {
const result = await getLocationFunc(protocol, req.query as Record<string, string[]|string>);
res.status(200).json(result);
}
catch (ex) {
catch (ex) {
respondErr(ex, res)
}
},
Expand Down Expand Up @@ -647,7 +648,7 @@ export class Bridge {
const result = await parseLocationFunc(alias);
res.status(200).json(result);
}
catch (ex) {
catch (ex) {
respondErr(ex, res)
}
},
Expand Down Expand Up @@ -676,7 +677,7 @@ export class Bridge {
const result = await getUserFunc(protocol, req.query as Record<string, string[]|string>);
res.status(200).json(result);
}
catch (ex) {
catch (ex) {
respondErr(ex, res)
}
}
Expand Down Expand Up @@ -705,7 +706,7 @@ export class Bridge {
const result = await parseUserFunc(userid);
res.status(200).json(result);
}
catch (ex) {
catch (ex) {
respondErr(ex, res)
}
},
Expand All @@ -723,7 +724,12 @@ export class Bridge {
* @param {Bridge~appServicePathHandler} opts.handler Function to handle requests
* to this endpoint.
*/
public addAppServicePath(opts: {method: "GET"|"PUT"|"POST"|"DELETE", checkToken?: boolean, path: string, handler: (req: ExRequest, respose: ExResponse, next: NextFunction) => void}) {
public addAppServicePath(opts: {
method: "GET"|"PUT"|"POST"|"DELETE",
checkToken?: boolean,
path: string,
handler: (req: ExRequest, respose: ExResponse, next: NextFunction) => void,
}) {
// TODO(paul): This is gut-wrenching into the AppService instance itself.
// Maybe an API on that object would be good?
const app: Application = (this.appservice as any).app;
Expand Down Expand Up @@ -874,7 +880,10 @@ export class Bridge {
* @param provisionedUser Provisioning information.
* @return Resolved when provisioned.
*/
public async provisionUser(matrixUser: MatrixUser, provisionedUser?: {name?: string, url?: string, remote?: RemoteUser}) {
public async provisionUser(
matrixUser: MatrixUser,
provisionedUser?: {name?: string, url?: string, remote?: RemoteUser}
) {
if (!this.clientFactory) {
throw Error('Cannot call getIntent before calling .run()');
}
Expand Down Expand Up @@ -925,14 +934,14 @@ export class Bridge {
}
if (!this.botIntent) {
throw Error('botIntent is not ready yet');
return;
}
const aliasLocalpart = alias.split(":")[0].substring(1);
const provisionedRoom = await this.opts.controller.onAliasQuery(alias, aliasLocalpart);
if (!provisionedRoom) {
// Not provisioning room.
throw Error("Not provisioning room for this alias");
}
// eslint-disable-next-line camelcase
const createRoomResponse: {room_id: string} = await this.botClient.createRoom(
provisionedRoom.creationOpts
);
Expand Down Expand Up @@ -972,6 +981,7 @@ export class Bridge {
if (this.roomUpgradeHandler && this.appServiceBot) {
// m.room.tombstone is the event that signals a room upgrade.
if (event.type === "m.room.tombstone" && isCanonicalState && this.roomUpgradeHandler) {
// eslint-disable-next-line camelcase
this.roomUpgradeHandler.onTombstone({...event, content: event.content as {replacement_room: string}});
if (this.opts.roomUpgradeOpts.consumeEvent) {
return null;
Expand Down Expand Up @@ -1051,6 +1061,7 @@ export class Bridge {
this.opts.controller.onEvent(data.request, data.context);
}

// eslint-disable-next-line camelcase
private async getBridgeContext(event: {sender: string, type: string, state_key: string, room_id: string}) {
if (this.opts.disableContext) {
return null;
Expand All @@ -1069,6 +1080,7 @@ export class Bridge {
return context.get(this.roomStore, this.userStore);
}

// eslint-disable-next-line camelcase
private handleEventError(event: {room_id: string, event_id: string}, error: EventNotHandledError) {
if (!this.botIntent) {
throw Error('Cannot call handleEventError before calling .run()');
Expand Down Expand Up @@ -1200,24 +1212,34 @@ export class Bridge {

}

function loadDatabase<T extends BridgeStore>(path: string, cls: new (db: Datastore) => T) {
function loadDatabase<T extends BridgeStore>(path: string, Cls: new (db: Datastore) => T) {
const defer = deferPromise<T>();
var db = new Datastore({
const db = new Datastore({
filename: path,
autoload: true,
onload: function(err) {
if (err) {
defer.reject(err);
}
else {
defer.resolve(new cls(db));
defer.resolve(new Cls(db));
}
}
});
return defer.promise;
}

function retryAlgorithm(event: unknown, attempts: number, err: {httpStatus: number, cors?: string, name: string, data?: { retry_after_ms: number }}) {
function retryAlgorithm(
event: unknown,
attempts: number,
err: {
httpStatus: number,
cors?: string,
name: string,
// eslint-disable-next-line camelcase
data?: { retry_after_ms: number },
}
) {
if (err.httpStatus === 400 || err.httpStatus === 403 || err.httpStatus === 401) {
// client error; no amount of retrying with save you now.
return -1;
Expand Down
39 changes: 28 additions & 11 deletions src/components/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,24 @@ interface CliOpts<ConfigType extends Record<string, unknown>> {
schema: string|Record<string, unknown>;
defaults: Record<string, unknown>;
};
registrationPath: string;
registrationPath?: string;
enableRegistration?: boolean;
enableLocalpart?: boolean;
port?: number;
noUrl?: boolean;
}

interface VettedCliOpts<ConfigType extends Record<string, unknown>> {
run: (port: number, config: ConfigType | null, registration: AppServiceRegistration | null) => void;
onConfigChanged?: (config: ConfigType) => void,
generateRegistration?: (reg: AppServiceRegistration, cb: (finalReg: AppServiceRegistration) => void) => void;
bridgeConfig?: {
affectsRegistration?: boolean;
schema: string | Record<string, unknown>;
defaults: Record<string, unknown>;
};
registrationPath: string;
enableRegistration: boolean;
enableLocalpart: boolean;
port: number;
noUrl?: boolean;
Expand All @@ -56,6 +72,7 @@ export class Cli<ConfigType extends Record<string, unknown>> {
public static DEFAULT_FILENAME = "registration.yaml";
private bridgeConfig: ConfigType|null = null;
private args: CliArgs|null = null;
private opts: VettedCliOpts<ConfigType>;

/**
* @constructor
Expand All @@ -80,25 +97,25 @@ export class Cli<ConfigType extends Record<string, unknown>> {
* file to. Users can overwrite this with -f.
* @param opts.enableLocalpart Enable '--localpart [-l]'. Default: false.
*/
constructor(private opts: CliOpts<ConfigType>) {
if (this.opts.enableRegistration === undefined) {
this.opts.enableRegistration = true;
}

if (!this.opts.run || typeof this.opts.run !== "function") {
constructor(opts: CliOpts<ConfigType>) {
if (!opts.run || typeof opts.run !== "function") {
throw new Error("Requires 'run' function.");
}

if (this.opts.enableRegistration && !this.opts.generateRegistration) {
if (opts.enableRegistration && !opts.generateRegistration) {
throw new Error(
"Registration generation is enabled but no " +
"'generateRegistration' function has been provided"
);
}
this.opts.enableLocalpart = Boolean(this.opts.enableLocalpart);

this.opts.registrationPath = this.opts.registrationPath || Cli.DEFAULT_FILENAME;
this.opts.port = this.opts.port || Cli.DEFAULT_PORT;
this.opts = {
...opts,
enableRegistration: typeof opts.enableRegistration === 'boolean' ? opts.enableRegistration : true,
enableLocalpart: Boolean(opts.enableLocalpart),
registrationPath: opts.registrationPath || Cli.DEFAULT_FILENAME,
port: opts.port || Cli.DEFAULT_PORT,
};
}
/**
* Get the parsed arguments. Only set after run is called and arguments parsed.
Expand Down