From cebc6d069fa609de918881854414768fadc87fed Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 11:03:49 +0200 Subject: [PATCH 01/12] sqlite: error if sqlite isn't enabled but search() is called When we assert that something can't possibly happen, we better error out rather than jugging on with no error ;) --- server/plugins/messageStorage/sqlite.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index f2a3b526..568d5256 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -213,10 +213,12 @@ class SqliteMessageStorage implements ISqliteMessageStorage { }) as Promise; } - search(query: SearchQuery): Promise { + search(query: SearchQuery): Promise { if (!this.isEnabled) { // this should never be hit as messageProvider is checked in client.search() - return Promise.resolve([]); + return Promise.reject( + "search called but sqlite provider not enabled. This is a programming error" + ); } // Using the '@' character to escape '%' and '_' in patterns. From bea4545abffe738dfeb025b36817490c1b5fa61d Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sun, 2 Oct 2022 11:00:52 +0200 Subject: [PATCH 02/12] don't call search on a disabled msg provider A provider might be available, but not functional (broken migration invalid configuration or what have you). Don't try to call search in this case. --- server/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/client.ts b/server/client.ts index 39b8bacb..eec9ad90 100644 --- a/server/client.ts +++ b/server/client.ts @@ -619,7 +619,7 @@ class Client { } search(query: SearchQuery) { - if (this.messageProvider === undefined) { + if (!this.messageProvider?.isEnabled) { return Promise.resolve({ results: [], target: "", From f6b292107ee4e627562d170babcb272cfa102a1e Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 11:06:57 +0200 Subject: [PATCH 03/12] sqlite: move export to bottom of the file This makes it easier to see what's getting exported, rather than if it's interspersed randomly in the middle of the file --- server/plugins/messageStorage/sqlite.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 568d5256..a7f293e3 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -268,8 +268,6 @@ class SqliteMessageStorage implements ISqliteMessageStorage { } } -export default SqliteMessageStorage; - // TODO: type any function parseSearchRowsToMessages(id: number, rows: any[]) { const messages: Msg[] = []; @@ -287,3 +285,5 @@ function parseSearchRowsToMessages(id: number, rows: any[]) { return messages; } + +export default SqliteMessageStorage; From e62b169a6abab4b2a0df34a5da21c92136ba3790 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 11:16:52 +0200 Subject: [PATCH 04/12] sqlite: fix docstring --- server/plugins/messageStorage/sqlite.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index a7f293e3..193cf6dc 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -174,8 +174,8 @@ class SqliteMessageStorage implements ISqliteMessageStorage { /** * Load messages for given channel on a given network and resolve a promise with loaded messages. * - * @param Network network - Network object where the channel is - * @param Chan channel - Channel object for which to load messages for + * @param network Network - Network object where the channel is + * @param channel Channel - Channel object for which to load messages for */ getMessages(network: Network, channel: Channel) { if (!this.isEnabled || Config.values.maxHistory === 0) { From 89ee5373643d1c5cb664401de745109bf7bcb77c Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 13:41:52 +0200 Subject: [PATCH 05/12] sqlite: add run helper function Extract the serialization logic into a single place and consistently log errors to the console rather than a fire and forget approach. --- server/plugins/messageStorage/sqlite.ts | 67 +++++++++++++++---------- 1 file changed, 40 insertions(+), 27 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 193cf6dc..dda5e814 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -63,7 +63,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { this.database = new sqlite3.Database(sqlitePath); this.database.serialize(() => { - schema.forEach((line) => this.database.run(line)); + schema.forEach((line) => this.run(line)); this.database.get( "SELECT value FROM options WHERE name = 'schema_version'", @@ -74,13 +74,10 @@ class SqliteMessageStorage implements ISqliteMessageStorage { // New table if (row === undefined) { - this.database.serialize(() => - this.database.run( - "INSERT INTO options (name, value) VALUES ('schema_version', ?)", - currentSchemaVersion - ) + this.run( + "INSERT INTO options (name, value) VALUES ('schema_version', ?)", + currentSchemaVersion ); - return; } @@ -100,11 +97,9 @@ class SqliteMessageStorage implements ISqliteMessageStorage { `sqlite messages schema version is out of date (${storedSchemaVersion} < ${currentSchemaVersion}). Running migrations if any.` ); - this.database.serialize(() => - this.database.run( - "UPDATE options SET value = ? WHERE name = 'schema_version'", - currentSchemaVersion - ) + this.run( + "UPDATE options SET value = ? WHERE name = 'schema_version'", + currentSchemaVersion ); } ); @@ -145,15 +140,13 @@ class SqliteMessageStorage implements ISqliteMessageStorage { return newMsg; }, {}); - this.database.serialize(() => - this.database.run( - "INSERT INTO messages(network, channel, time, type, msg) VALUES(?, ?, ?, ?, ?)", - network.uuid, - channel.name.toLowerCase(), - msg.time.getTime(), - msg.type, - JSON.stringify(clonedMsg) - ) + this.run( + "INSERT INTO messages(network, channel, time, type, msg) VALUES(?, ?, ?, ?, ?)", + network.uuid, + channel.name.toLowerCase(), + msg.time.getTime(), + msg.type, + JSON.stringify(clonedMsg) ); } @@ -162,12 +155,10 @@ class SqliteMessageStorage implements ISqliteMessageStorage { return; } - this.database.serialize(() => - this.database.run( - "DELETE FROM messages WHERE network = ? AND channel = ?", - network.uuid, - channel.name.toLowerCase() - ) + this.run( + "DELETE FROM messages WHERE network = ? AND channel = ?", + network.uuid, + channel.name.toLowerCase() ); } @@ -266,6 +257,28 @@ class SqliteMessageStorage implements ISqliteMessageStorage { canProvideMessages() { return this.isEnabled; } + + private run(stmt: string, ...params: any[]) { + this.serialize_run(stmt, params).catch((err) => + log.error(`failed to run ${stmt}`, String(err)) + ); + } + + private serialize_run(stmt: string, params: any[]): Promise { + return new Promise((resolve, reject) => { + this.database.serialize(() => { + this.database.run(stmt, params, (err) => { + + if (err) { + reject(err); + return; + } + + resolve(); + }); + }); + }); + } } // TODO: type any From cc3302e8743633b3b87e15fb54a964510b2466d1 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 14:21:29 +0200 Subject: [PATCH 06/12] sqlite: create serialize_fetchall helper function That puts all the serialization logic into one place and allows us to use async / promises --- server/plugins/messageStorage/sqlite.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index dda5e814..1a9222dd 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -279,6 +279,21 @@ class SqliteMessageStorage implements ISqliteMessageStorage { }); }); } + + private serialize_fetchall(stmt: string, ...params: any[]): Promise { + return new Promise((resolve, reject) => { + this.database.serialize(() => { + this.database.all(stmt, params, (err, rows) => { + if (err) { + reject(err); + return; + } + + resolve(rows); + }); + }); + }); + } } // TODO: type any From ee8223c2006ad31fc746824b495125b321da4bf8 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 14:55:35 +0200 Subject: [PATCH 07/12] sqlite: use serialize_fetchall in getMessages --- server/plugins/messageStorage/sqlite.ts | 39 ++++++++++--------------- 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 1a9222dd..2e2d8c9e 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -168,7 +168,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { * @param network Network - Network object where the channel is * @param channel Channel - Channel object for which to load messages for */ - getMessages(network: Network, channel: Channel) { + async getMessages(network: Network, channel: Channel): Promise { if (!this.isEnabled || Config.values.maxHistory === 0) { return Promise.resolve([]); } @@ -176,32 +176,23 @@ class SqliteMessageStorage implements ISqliteMessageStorage { // If unlimited history is specified, load 100k messages const limit = Config.values.maxHistory < 0 ? 100000 : Config.values.maxHistory; - return new Promise((resolve, reject) => { - this.database.serialize(() => - this.database.all( - "SELECT msg, type, time FROM messages WHERE network = ? AND channel = ? ORDER BY time DESC LIMIT ?", - [network.uuid, channel.name.toLowerCase(), limit], - (err, rows) => { - if (err) { - return reject(err); - } + const rows = await this.serialize_fetchall( + "SELECT msg, type, time FROM messages WHERE network = ? AND channel = ? ORDER BY time DESC LIMIT ?", + network.uuid, + channel.name.toLowerCase(), + limit + ); - resolve( - rows.reverse().map((row) => { - const msg = JSON.parse(row.msg); - msg.time = row.time; - msg.type = row.type; + return rows.reverse().map((row: any) => { + const msg = JSON.parse(row.msg); + msg.time = row.time; + msg.type = row.type; - const newMsg = new Msg(msg); - newMsg.id = this.client.idMsg++; + const newMsg = new Msg(msg); + newMsg.id = this.client.idMsg++; - return newMsg; - }) - ); - } - ) - ); - }) as Promise; + return newMsg; + }) as Message[]; } search(query: SearchQuery): Promise { From 5e1cbe32f95aca776fe4dff550a0c8c369460417 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Sat, 27 Aug 2022 15:18:22 +0200 Subject: [PATCH 08/12] sqlite: use serialize_fetchall in search --- server/plugins/messageStorage/sqlite.ts | 30 ++++++++++--------------- 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 2e2d8c9e..c9d2ec34 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -195,10 +195,10 @@ class SqliteMessageStorage implements ISqliteMessageStorage { }) as Message[]; } - search(query: SearchQuery): Promise { + async search(query: SearchQuery): Promise { if (!this.isEnabled) { // this should never be hit as messageProvider is checked in client.search() - return Promise.reject( + throw new Error( "search called but sqlite provider not enabled. This is a programming error" ); } @@ -227,22 +227,16 @@ class SqliteMessageStorage implements ISqliteMessageStorage { query.offset = parseInt(query.offset as string, 10) || 0; params.push(String(query.offset)); - return new Promise((resolve, reject) => { - this.database.all(select, params, (err, rows) => { - if (err) { - reject(err); - } else { - const response: SearchResponse = { - searchTerm: query.searchTerm, - target: query.channelName, - networkUuid: query.networkUuid, - offset: query.offset as number, - results: parseSearchRowsToMessages(query.offset as number, rows).reverse(), - }; - resolve(response); - } - }); - }); + const rows = await this.serialize_fetchall(select, ...params); + const response: SearchResponse = { + searchTerm: query.searchTerm, + target: query.channelName, + networkUuid: query.networkUuid, + offset: query.offset as number, + results: parseSearchRowsToMessages(query.offset as number, rows).reverse(), + }; + + return response; } canProvideMessages() { From f04a06682d3690b571dc0b9720baa79b687b9465 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Mon, 29 Aug 2022 07:47:33 +0200 Subject: [PATCH 09/12] extract migrations --- server/plugins/messageStorage/sqlite.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index c9d2ec34..6b46e8b5 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -62,6 +62,11 @@ class SqliteMessageStorage implements ISqliteMessageStorage { this.isEnabled = true; this.database = new sqlite3.Database(sqlitePath); + + this.run_migrations() + } + + private run_migrations() { this.database.serialize(() => { schema.forEach((line) => this.run(line)); From bbe81bb2fa9001762df90c1a267afa0239ebb7c7 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Mon, 10 Oct 2022 21:43:05 +0200 Subject: [PATCH 10/12] sqlite: add serialize_get --- server/plugins/messageStorage/sqlite.ts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 6b46e8b5..ed242aea 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -284,6 +284,24 @@ class SqliteMessageStorage implements ISqliteMessageStorage { }); }); } + + private serialize_get(stmt: string, ...params: any[]): Promise { + const log_id = this.stmt_id(); + return new Promise((resolve, reject) => { + this.database.serialize(() => { + this.database.get(stmt, params, (err, row) => { + log.debug(log_id, "callback", stmt); + + if (err) { + reject(err); + return; + } + + resolve(row); + }); + }); + }); + } } // TODO: type any From f068fd429012c47648faf8c4d751f972062709bd Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Mon, 10 Oct 2022 21:43:30 +0200 Subject: [PATCH 11/12] sqlite: convert migrations to async This removes quite a bunch of indention and callbacks --- server/plugins/messageStorage/sqlite.ts | 77 ++++++++++++------------- 1 file changed, 36 insertions(+), 41 deletions(-) diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index ed242aea..2641a466 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -63,52 +63,47 @@ class SqliteMessageStorage implements ISqliteMessageStorage { this.database = new sqlite3.Database(sqlitePath); - this.run_migrations() + this.run_migrations().catch((err) => { + log.error("Migration failed", String(err)); + this.isEnabled = false; + }); } - private run_migrations() { - this.database.serialize(() => { - schema.forEach((line) => this.run(line)); + async run_migrations() { + for (const stmt of schema) { + await this.serialize_run(stmt, []); + } - this.database.get( - "SELECT value FROM options WHERE name = 'schema_version'", - (err, row) => { - if (err) { - return log.error(`Failed to retrieve schema version: ${err.toString()}`); - } + const version = await this.serialize_get( + "SELECT value FROM options WHERE name = 'schema_version'" + ); - // New table - if (row === undefined) { - this.run( - "INSERT INTO options (name, value) VALUES ('schema_version', ?)", - currentSchemaVersion - ); - return; - } - - const storedSchemaVersion = parseInt(row.value, 10); - - if (storedSchemaVersion === currentSchemaVersion) { - return; - } - - if (storedSchemaVersion > currentSchemaVersion) { - return log.error( - `sqlite messages schema version is higher than expected (${storedSchemaVersion} > ${currentSchemaVersion}). Is The Lounge out of date?` - ); - } - - log.info( - `sqlite messages schema version is out of date (${storedSchemaVersion} < ${currentSchemaVersion}). Running migrations if any.` - ); - - this.run( - "UPDATE options SET value = ? WHERE name = 'schema_version'", - currentSchemaVersion - ); - } + if (version === undefined) { + // new table + await this.serialize_run( + "INSERT INTO options (name, value) VALUES ('schema_version', ?)", + [currentSchemaVersion] ); - }); + return; + } + + const storedSchemaVersion = parseInt(version.value, 10); + + if (storedSchemaVersion === currentSchemaVersion) { + return; + } + + if (storedSchemaVersion > currentSchemaVersion) { + throw `sqlite messages schema version is higher than expected (${storedSchemaVersion} > ${currentSchemaVersion}). Is The Lounge out of date?`; + } + + log.info( + `sqlite messages schema version is out of date (${storedSchemaVersion} < ${currentSchemaVersion}). Running migrations if any.` + ); + + await this.serialize_run("UPDATE options SET value = ? WHERE name = 'schema_version'", [ + currentSchemaVersion, + ]); } close(callback?: (error?: Error | null) => void) { From d62dd3e62d106009cbded2fd9af13fe9fae35ae5 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Tue, 1 Nov 2022 22:51:40 +0100 Subject: [PATCH 12/12] messageStorage: convert to async Message stores are more complicated that a sync "fire and forget" API allows for. For starters, non trivial stores (say sqlite) can fail during init and we want to be able to catch that. Second, we really need to be able to run migrations and such, which may block (and fail) the activation of the store. On the plus side, this pushes error handling to the caller rather than the stores, which is a good thing as that allows us to eventually push this to the client in the UI, rather than just logging it in the server on stdout --- server/client.ts | 6 +- server/helper.ts | 15 +++ server/models/chan.ts | 2 +- server/plugins/messageStorage/sqlite.ts | 81 ++++++------ server/plugins/messageStorage/text.ts | 59 +++------ server/plugins/messageStorage/types.d.ts | 13 +- test/plugins/sqlite.ts | 153 +++++++++-------------- 7 files changed, 142 insertions(+), 187 deletions(-) diff --git a/server/client.ts b/server/client.ts index eec9ad90..cac35532 100644 --- a/server/client.ts +++ b/server/client.ts @@ -147,7 +147,7 @@ class Client { } for (const messageStorage of client.messageStorage) { - messageStorage.enable(); + messageStorage.enable().catch((e) => log.error(e)); } } @@ -614,7 +614,7 @@ class Client { } for (const messageStorage of this.messageStorage) { - messageStorage.deleteChannel(target.network, target.chan); + messageStorage.deleteChannel(target.network, target.chan).catch((e) => log.error(e)); } } @@ -767,7 +767,7 @@ class Client { }); for (const messageStorage of this.messageStorage) { - messageStorage.close(); + messageStorage.close().catch((e) => log.error(e)); } } diff --git a/server/helper.ts b/server/helper.ts index bee5120f..bd5be4b8 100644 --- a/server/helper.ts +++ b/server/helper.ts @@ -23,6 +23,7 @@ const Helper = { parseHostmask, compareHostmask, compareWithWildcard, + catch_to_error, password: { hash: passwordHash, @@ -183,3 +184,17 @@ function compareWithWildcard(a: string, b: string) { const re = new RegExp(`^${user_regex}$`, "i"); // case insensitive return re.test(b); } + +function catch_to_error(prefix: string, err: any): Error { + let msg: string; + + if (err instanceof Error) { + msg = err.message; + } else if (typeof err === "string") { + msg = err; + } else { + msg = err.toString(); + } + + return new Error(`${prefix}: ${msg}`); +} diff --git a/server/models/chan.ts b/server/models/chan.ts index e23955fa..1298353c 100644 --- a/server/models/chan.ts +++ b/server/models/chan.ts @@ -260,7 +260,7 @@ class Chan { } for (const messageStorage of client.messageStorage) { - messageStorage.index(target.network, targetChannel, msg); + messageStorage.index(target.network, targetChannel, msg).catch((e) => log.error(e)); } } loadMessages(client: Client, network: Network) { diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 2641a466..2ec4d094 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -2,11 +2,12 @@ import type {Database} from "sqlite3"; import log from "../../log"; import path from "path"; -import fs from "fs"; +import fs from "fs/promises"; import Config from "../../config"; import Msg, {Message} from "../../models/msg"; import Client from "../../client"; import Chan, {Channel} from "../../models/chan"; +import Helper from "../../helper"; import type { SearchResponse, SearchQuery, @@ -47,26 +48,26 @@ class SqliteMessageStorage implements ISqliteMessageStorage { this.isEnabled = false; } - enable() { + async enable() { const logsPath = Config.getUserLogsPath(); const sqlitePath = path.join(logsPath, `${this.client.name}.sqlite3`); try { - fs.mkdirSync(logsPath, {recursive: true}); - } catch (e: any) { - log.error("Unable to create logs directory", String(e)); - - return; + await fs.mkdir(logsPath, {recursive: true}); + } catch (e) { + throw Helper.catch_to_error("Unable to create logs directory", e); } this.isEnabled = true; this.database = new sqlite3.Database(sqlitePath); - this.run_migrations().catch((err) => { - log.error("Migration failed", String(err)); + try { + await this.run_migrations(); + } catch (e) { this.isEnabled = false; - }); + throw Helper.catch_to_error("Migration failed", e); + } } async run_migrations() { @@ -106,25 +107,26 @@ class SqliteMessageStorage implements ISqliteMessageStorage { ]); } - close(callback?: (error?: Error | null) => void) { + async close() { if (!this.isEnabled) { return; } this.isEnabled = false; - this.database.close((err) => { - if (err) { - log.error(`Failed to close sqlite database: ${err.message}`); - } + return new Promise((resolve, reject) => { + this.database.close((err) => { + if (err) { + reject(`Failed to close sqlite database: ${err.message}`); + return; + } - if (callback) { - callback(err); - } + resolve(); + }); }); } - index(network: Network, channel: Chan, msg: Msg) { + async index(network: Network, channel: Chan, msg: Msg) { if (!this.isEnabled) { return; } @@ -140,26 +142,27 @@ class SqliteMessageStorage implements ISqliteMessageStorage { return newMsg; }, {}); - this.run( + await this.serialize_run( "INSERT INTO messages(network, channel, time, type, msg) VALUES(?, ?, ?, ?, ?)", - network.uuid, - channel.name.toLowerCase(), - msg.time.getTime(), - msg.type, - JSON.stringify(clonedMsg) + [ + network.uuid, + channel.name.toLowerCase(), + msg.time.getTime(), + msg.type, + JSON.stringify(clonedMsg), + ] ); } - deleteChannel(network: Network, channel: Channel) { + async deleteChannel(network: Network, channel: Channel) { if (!this.isEnabled) { return; } - this.run( - "DELETE FROM messages WHERE network = ? AND channel = ?", + await this.serialize_run("DELETE FROM messages WHERE network = ? AND channel = ?", [ network.uuid, - channel.name.toLowerCase() - ); + channel.name.toLowerCase(), + ]); } /** @@ -170,7 +173,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { */ async getMessages(network: Network, channel: Channel): Promise { if (!this.isEnabled || Config.values.maxHistory === 0) { - return Promise.resolve([]); + return []; } // If unlimited history is specified, load 100k messages @@ -183,7 +186,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { limit ); - return rows.reverse().map((row: any) => { + return rows.reverse().map((row: any): Message => { const msg = JSON.parse(row.msg); msg.time = row.time; msg.type = row.type; @@ -192,7 +195,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { newMsg.id = this.client.idMsg++; return newMsg; - }) as Message[]; + }); } async search(query: SearchQuery): Promise { @@ -243,17 +246,10 @@ class SqliteMessageStorage implements ISqliteMessageStorage { return this.isEnabled; } - private run(stmt: string, ...params: any[]) { - this.serialize_run(stmt, params).catch((err) => - log.error(`failed to run ${stmt}`, String(err)) - ); - } - private serialize_run(stmt: string, params: any[]): Promise { return new Promise((resolve, reject) => { this.database.serialize(() => { this.database.run(stmt, params, (err) => { - if (err) { reject(err); return; @@ -265,7 +261,7 @@ class SqliteMessageStorage implements ISqliteMessageStorage { }); } - private serialize_fetchall(stmt: string, ...params: any[]): Promise { + private serialize_fetchall(stmt: string, ...params: any[]): Promise { return new Promise((resolve, reject) => { this.database.serialize(() => { this.database.all(stmt, params, (err, rows) => { @@ -281,12 +277,9 @@ class SqliteMessageStorage implements ISqliteMessageStorage { } private serialize_get(stmt: string, ...params: any[]): Promise { - const log_id = this.stmt_id(); return new Promise((resolve, reject) => { this.database.serialize(() => { this.database.get(stmt, params, (err, row) => { - log.debug(log_id, "callback", stmt); - if (err) { reject(err); return; diff --git a/server/plugins/messageStorage/text.ts b/server/plugins/messageStorage/text.ts index fbd2abc3..3190890c 100644 --- a/server/plugins/messageStorage/text.ts +++ b/server/plugins/messageStorage/text.ts @@ -1,9 +1,8 @@ /* eslint-disable @typescript-eslint/restrict-template-expressions */ -import fs from "fs"; +import fs from "fs/promises"; import path from "path"; import filenamify from "filenamify"; -import log from "../../log"; import Config from "../../config"; import {MessageStorage} from "./types"; import Client from "../../client"; @@ -20,19 +19,17 @@ class TextFileMessageStorage implements MessageStorage { this.isEnabled = false; } - enable() { + // eslint-disable-next-line @typescript-eslint/require-await + async enable() { this.isEnabled = true; } - close(callback: () => void) { + // eslint-disable-next-line @typescript-eslint/require-await + async close() { this.isEnabled = false; - - if (callback) { - callback(); - } } - index(network: Network, channel: Channel, msg: Message) { + async index(network: Network, channel: Channel, msg: Message) { if (!this.isEnabled) { return; } @@ -44,10 +41,9 @@ class TextFileMessageStorage implements MessageStorage { ); try { - fs.mkdirSync(logPath, {recursive: true}); - } catch (e: any) { - log.error("Unable to create logs directory", String(e)); - return; + await fs.mkdir(logPath, {recursive: true}); + } catch (e) { + throw new Error(`Unable to create logs directory: ${e}`); } let line = `[${msg.time.toISOString()}] `; @@ -106,35 +102,18 @@ class TextFileMessageStorage implements MessageStorage { line += "\n"; - fs.appendFile( - path.join(logPath, TextFileMessageStorage.getChannelFileName(channel)), - line, - (e) => { - if (e) { - log.error("Failed to write user log", e.message); - } - } - ); + try { + await fs.appendFile( + path.join(logPath, TextFileMessageStorage.getChannelFileName(channel)), + line + ); + } catch (e) { + throw new Error(`Failed to write user log: ${e}`); + } } - deleteChannel() { - /* TODO: Truncating text logs is disabled, until we figure out some UI for it - if (!this.isEnabled) { - return; - } - - const logPath = path.join( - Config.getUserLogsPath(), - this.client.name, - TextFileMessageStorage.getNetworkFolderName(network), - TextFileMessageStorage.getChannelFileName(channel) - ); - - fs.truncate(logPath, 0, (e) => { - if (e) { - log.error("Failed to truncate user log", e); - } - });*/ + async deleteChannel() { + // Not implemented for text log files } getMessages() { diff --git a/server/plugins/messageStorage/types.d.ts b/server/plugins/messageStorage/types.d.ts index 6dfa20db..7b68f929 100644 --- a/server/plugins/messageStorage/types.d.ts +++ b/server/plugins/messageStorage/types.d.ts @@ -9,13 +9,13 @@ interface MessageStorage { client: Client; isEnabled: boolean; - enable(): void; + enable(): Promise; - close(callback?: () => void): void; + close(): Promise; - index(network: Network, channel: Channel, msg: Message): void; + index(network: Network, channel: Channel, msg: Message): Promise; - deleteChannel(network: Network, channel: Channel); + deleteChannel(network: Network, channel: Channel): Promise; getMessages(network: Network, channel: Channel): Promise; @@ -30,12 +30,11 @@ export type SearchQuery = { }; export type SearchResponse = - | (Omit & { + | Omit & { results: Message[]; target: string; offset: number; - }) - | []; + }; type SearchFunction = (query: SearchQuery) => Promise; diff --git a/test/plugins/sqlite.ts b/test/plugins/sqlite.ts index 00211339..7019829d 100644 --- a/test/plugins/sqlite.ts +++ b/test/plugins/sqlite.ts @@ -37,18 +37,16 @@ describe("SQLite Message Storage", function () { fs.rmdir(path.join(Config.getHomePath(), "logs"), done); }); - it("should resolve an empty array when disabled", function () { - return store.getMessages(null as any, null as any).then((messages) => { - expect(messages).to.be.empty; - }); + it("should resolve an empty array when disabled", async function () { + const messages = await store.getMessages(null as any, null as any); + expect(messages).to.be.empty; }); - it("should create database file", function () { + it("should create database file", async function () { expect(store.isEnabled).to.be.false; expect(fs.existsSync(expectedPath)).to.be.false; - store.enable(); - + await store.enable(); expect(store.isEnabled).to.be.true; }); @@ -90,8 +88,8 @@ describe("SQLite Message Storage", function () { ); }); - it("should store a message", function () { - store.index( + it("should store a message", async function () { + await store.index( { uuid: "this-is-a-network-guid", } as any, @@ -105,35 +103,30 @@ describe("SQLite Message Storage", function () { ); }); - it("should retrieve previously stored message", function () { - return store - .getMessages( - { - uuid: "this-is-a-network-guid", - } as any, - { - name: "#thisisaCHANNEL", - } as any - ) - .then((messages) => { - expect(messages).to.have.lengthOf(1); - - const msg = messages[0]; - - expect(msg.text).to.equal("Hello from sqlite world!"); - expect(msg.type).to.equal(MessageType.MESSAGE); - expect(msg.time.getTime()).to.equal(123456789); - }); + it("should retrieve previously stored message", async function () { + const messages = await store.getMessages( + { + uuid: "this-is-a-network-guid", + } as any, + { + name: "#thisisaCHANNEL", + } as any + ); + expect(messages).to.have.lengthOf(1); + const msg = messages[0]; + expect(msg.text).to.equal("Hello from sqlite world!"); + expect(msg.type).to.equal(MessageType.MESSAGE); + expect(msg.time.getTime()).to.equal(123456789); }); - it("should retrieve latest LIMIT messages in order", function () { + it("should retrieve latest LIMIT messages in order", async function () { const originalMaxHistory = Config.values.maxHistory; try { Config.values.maxHistory = 2; for (let i = 0; i < 200; ++i) { - store.index( + await store.index( {uuid: "retrieval-order-test-network"} as any, {name: "#channel"} as any, new Msg({ @@ -143,60 +136,47 @@ describe("SQLite Message Storage", function () { ); } - return store - .getMessages( - {uuid: "retrieval-order-test-network"} as any, - {name: "#channel"} as any - ) - .then((messages) => { - expect(messages).to.have.lengthOf(2); - expect(messages.map((i) => i.text)).to.deep.equal(["msg 198", "msg 199"]); - }); + const messages = await store.getMessages( + {uuid: "retrieval-order-test-network"} as any, + {name: "#channel"} as any + ); + expect(messages).to.have.lengthOf(2); + expect(messages.map((i_1) => i_1.text)).to.deep.equal(["msg 198", "msg 199"]); } finally { Config.values.maxHistory = originalMaxHistory; } }); - it("should search messages", function () { + it("should search messages", async function () { const originalMaxHistory = Config.values.maxHistory; try { Config.values.maxHistory = 2; - return store - .search({ - searchTerm: "msg", - networkUuid: "retrieval-order-test-network", - } as any) - .then((messages) => { - // @ts-expect-error Property 'results' does not exist on type '[]'. - expect(messages.results).to.have.lengthOf(100); + const search = await store.search({ + searchTerm: "msg", + networkUuid: "retrieval-order-test-network", + } as any); + expect(search.results).to.have.lengthOf(100); + const expectedMessages: string[] = []; - const expectedMessages: string[] = []; + for (let i = 100; i < 200; ++i) { + expectedMessages.push(`msg ${i}`); + } - for (let i = 100; i < 200; ++i) { - expectedMessages.push(`msg ${i}`); - } - - // @ts-expect-error Property 'results' does not exist on type '[]'. - expect(messages.results.map((i) => i.text)).to.deep.equal(expectedMessages); - }); + expect(search.results.map((i_1) => i_1.text)).to.deep.equal(expectedMessages); } finally { Config.values.maxHistory = originalMaxHistory; } }); - it("should search messages with escaped wildcards", function () { - function assertResults(query, expected) { - return store - .search({ - searchTerm: query, - networkUuid: "this-is-a-network-guid2", - } as any) - .then((messages) => { - // @ts-expect-error Property 'results' does not exist on type '[]'. - expect(messages.results.map((i) => i.text)).to.deep.equal(expected); - }); + it("should search messages with escaped wildcards", async function () { + async function assertResults(query: string, expected: string[]) { + const search = await store.search({ + searchTerm: query, + networkUuid: "this-is-a-network-guid2", + } as any); + expect(search.results.map((i) => i.text)).to.deep.equal(expected); } const originalMaxHistory = Config.values.maxHistory; @@ -204,7 +184,7 @@ describe("SQLite Message Storage", function () { try { Config.values.maxHistory = 3; - store.index( + await store.index( {uuid: "this-is-a-network-guid2"} as any, {name: "#channel"} as any, new Msg({ @@ -213,7 +193,7 @@ describe("SQLite Message Storage", function () { } as any) ); - store.index( + await store.index( {uuid: "this-is-a-network-guid2"} as any, {name: "#channel"} as any, new Msg({ @@ -222,7 +202,7 @@ describe("SQLite Message Storage", function () { } as any) ); - store.index( + await store.index( {uuid: "this-is-a-network-guid2"} as any, {name: "#channel"} as any, new Msg({ @@ -231,32 +211,21 @@ describe("SQLite Message Storage", function () { } as any) ); - return ( - store - .getMessages( - {uuid: "this-is-a-network-guid2"} as any, - {name: "#channel"} as any - ) - // .getMessages() waits for store.index() transactions to commit - .then(() => assertResults("foo", ["foo % bar _ baz", "foo bar x baz"])) - .then(() => assertResults("%", ["foo % bar _ baz"])) - .then(() => assertResults("foo % bar ", ["foo % bar _ baz"])) - .then(() => assertResults("_", ["foo % bar _ baz"])) - .then(() => assertResults("bar _ baz", ["foo % bar _ baz"])) - .then(() => assertResults("%%", [])) - .then(() => assertResults("@%", [])) - .then(() => assertResults("@", ["bar @ baz"])) - ); + await assertResults("foo", ["foo % bar _ baz", "foo bar x baz"]); + await assertResults("%", ["foo % bar _ baz"]); + await assertResults("foo % bar ", ["foo % bar _ baz"]); + await assertResults("_", ["foo % bar _ baz"]); + await assertResults("bar _ baz", ["foo % bar _ baz"]); + await assertResults("%%", []); + await assertResults("@%", []); + await assertResults("@", ["bar @ baz"]); } finally { Config.values.maxHistory = originalMaxHistory; } }); - it("should close database", function (done) { - store.close((err) => { - expect(err).to.be.null; - expect(fs.existsSync(expectedPath)).to.be.true; - done(); - }); + it("should close database", async function () { + await store.close(); + expect(fs.existsSync(expectedPath)).to.be.true; }); });