From 958a948456d1a0c3c97bb60e8759e8f9f5578ac8 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Fri, 30 Dec 2022 16:40:12 +0100 Subject: [PATCH] sqlite: Remove client from sqlitestorage The only reason we accepted a client was that so we have access to the next message id when we need it. So let's accept an id provider function instead. --- server/client.ts | 2 +- server/models/chan.ts | 2 +- server/plugins/messageStorage/sqlite.ts | 23 ++++++++++------------- server/plugins/messageStorage/types.d.ts | 2 +- test/plugins/sqlite.ts | 16 ++++++++-------- 5 files changed, 21 insertions(+), 24 deletions(-) diff --git a/server/client.ts b/server/client.ts index 1af16751..33abf2ab 100644 --- a/server/client.ts +++ b/server/client.ts @@ -138,7 +138,7 @@ class Client { if (!Config.values.public && client.config.log) { if (Config.values.messageStorage.includes("sqlite")) { - client.messageProvider = new SqliteMessageStorage(client); + client.messageProvider = new SqliteMessageStorage(client.name); client.messageStorage.push(client.messageProvider); } diff --git a/server/models/chan.ts b/server/models/chan.ts index 1298353c..8fd68ed7 100644 --- a/server/models/chan.ts +++ b/server/models/chan.ts @@ -287,7 +287,7 @@ class Chan { } client.messageProvider - .getMessages(network, this) + .getMessages(network, this, () => client.idMsg++) .then((messages) => { if (messages.length === 0) { if (network.irc!.network.cap.isEnabled("znc.in/playback")) { diff --git a/server/plugins/messageStorage/sqlite.ts b/server/plugins/messageStorage/sqlite.ts index 698f7c82..7538da8e 100644 --- a/server/plugins/messageStorage/sqlite.ts +++ b/server/plugins/messageStorage/sqlite.ts @@ -5,7 +5,6 @@ import path from "path"; 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, SearchableMessageStorage} from "./types"; @@ -49,17 +48,17 @@ class SqliteMessageStorage implements SearchableMessageStorage { isEnabled: boolean; database!: Database; initDone: Deferred; - client: Client; + userName: string; - constructor(client: Client) { - this.client = client; + constructor(userName: string) { + this.userName = userName; this.isEnabled = false; this.initDone = new Deferred(); } async _enable() { const logsPath = Config.getUserLogsPath(); - const sqlitePath = path.join(logsPath, `${this.client.name}.sqlite3`); + const sqlitePath = path.join(logsPath, `${this.userName}.sqlite3`); try { await fs.mkdir(logsPath, {recursive: true}); @@ -186,13 +185,11 @@ class SqliteMessageStorage implements SearchableMessageStorage { ]); } - /** - * 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 channel Channel - Channel object for which to load messages for - */ - async getMessages(network: Network, channel: Channel): Promise { + async getMessages( + network: Network, + channel: Channel, + nextID: () => number + ): Promise { await this.initDone.promise; if (!this.isEnabled || Config.values.maxHistory === 0) { @@ -215,7 +212,7 @@ class SqliteMessageStorage implements SearchableMessageStorage { msg.type = row.type; const newMsg = new Msg(msg); - newMsg.id = this.client.idMsg++; + newMsg.id = nextID(); return newMsg; }); diff --git a/server/plugins/messageStorage/types.d.ts b/server/plugins/messageStorage/types.d.ts index acaa041a..2ead5575 100644 --- a/server/plugins/messageStorage/types.d.ts +++ b/server/plugins/messageStorage/types.d.ts @@ -16,7 +16,7 @@ interface MessageStorage { deleteChannel(network: Network, channel: Channel): Promise; - getMessages(network: Network, channel: Channel): Promise; + getMessages(network: Network, channel: Channel, nextID: () => number): Promise; canProvideMessages(): boolean; } diff --git a/test/plugins/sqlite.ts b/test/plugins/sqlite.ts index 4476faf0..16c832f1 100644 --- a/test/plugins/sqlite.ts +++ b/test/plugins/sqlite.ts @@ -6,7 +6,6 @@ import util from "../util"; import Msg, {MessageType} from "../../server/models/msg"; import Config from "../../server/config"; import MessageStorage from "../../server/plugins/messageStorage/sqlite"; -import Client from "../../server/client"; describe("SQLite Message Storage", function () { // Increase timeout due to unpredictable I/O on CI services @@ -17,10 +16,7 @@ describe("SQLite Message Storage", function () { let store: MessageStorage; before(function (done) { - store = new MessageStorage({ - name: "testUser", - idMsg: 1, - } as Client); + store = new MessageStorage("testUser"); // Delete database file from previous test run if (fs.existsSync(expectedPath)) { @@ -47,7 +43,7 @@ describe("SQLite Message Storage", function () { it("should resolve an empty array when disabled", async function () { store.isEnabled = false; - const messages = await store.getMessages(null as any, null as any); + const messages = await store.getMessages(null as any, null as any, null as any); expect(messages).to.be.empty; store.isEnabled = true; }); @@ -106,13 +102,15 @@ describe("SQLite Message Storage", function () { }); it("should retrieve previously stored message", async function () { + let msgid = 0; const messages = await store.getMessages( { uuid: "this-is-a-network-guid", } as any, { name: "#thisisaCHANNEL", - } as any + } as any, + () => msgid++ ); expect(messages).to.have.lengthOf(1); const msg = messages[0]; @@ -138,9 +136,11 @@ describe("SQLite Message Storage", function () { ); } + let msgId = 0; const messages = await store.getMessages( {uuid: "retrieval-order-test-network"} as any, - {name: "#channel"} as any + {name: "#channel"} as any, + () => msgId++ ); expect(messages).to.have.lengthOf(2); expect(messages.map((i_1) => i_1.text)).to.deep.equal(["msg 198", "msg 199"]);