From 76098d7e766ad074eb6278ee487410f1f02817c3 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 14 Mar 2023 21:20:25 +0100 Subject: [PATCH 1/4] Fix incorrect typing of dehydrated networks and channels Client and ClientManager deal with both 'dehydrated' channels/networks (ie. directly from JSON configuration) and the 'rehydrated' ones (classes, with socket objects, message arrays, etc.). However, because their attributes are similar, both types were used interchangeably, which becomes an issue when splitting Client's configuration loading into smaller methods. --- server/client.ts | 10 +++++++--- server/clientManager.ts | 3 ++- server/models/chan.ts | 7 +++++++ server/models/network.ts | 30 +++++++++++++++++++++++++++++- 4 files changed, 45 insertions(+), 5 deletions(-) diff --git a/server/client.ts b/server/client.ts index 66a408c0..049c0744 100644 --- a/server/client.ts +++ b/server/client.ts @@ -15,7 +15,7 @@ import inputs from "./plugins/inputs"; import PublicClient from "./plugins/packages/publicClient"; import SqliteMessageStorage from "./plugins/messageStorage/sqlite"; import TextFileMessageStorage from "./plugins/messageStorage/text"; -import Network, {IgnoreListItem, NetworkWithIrcFramework} from "./models/network"; +import Network, {IgnoreListItem, NetworkConfig, NetworkWithIrcFramework} from "./models/network"; import ClientManager from "./clientManager"; import {MessageStorage, SearchQuery, SearchResponse} from "./plugins/messageStorage/types"; @@ -96,7 +96,7 @@ class Client { [socketId: string]: {token: string; openChannel: number}; }; config!: UserConfig & { - networks?: Network[]; + networks?: NetworkConfig[]; }; id!: number; idMsg!: number; @@ -112,7 +112,11 @@ class Client { fileHash!: string; - constructor(manager: ClientManager, name?: string, config = {} as UserConfig) { + constructor( + manager: ClientManager, + name?: string, + config = {} as UserConfig & {networks: NetworkConfig[]} + ) { _.merge(this, { awayMessage: "", lastActiveChannel: -1, diff --git a/server/clientManager.ts b/server/clientManager.ts index 705fa432..d32081e3 100644 --- a/server/clientManager.ts +++ b/server/clientManager.ts @@ -7,6 +7,7 @@ import path from "path"; import Auth from "./plugins/auth"; import Client, {UserConfig} from "./client"; import Config from "./config"; +import {NetworkConfig} from "./models/network"; import WebPush from "./plugins/webpush"; import log from "./log"; import {Server} from "socket.io"; @@ -283,7 +284,7 @@ class ClientManager { try { const data = fs.readFileSync(userPath, "utf-8"); - return JSON.parse(data) as UserConfig; + return JSON.parse(data) as UserConfig & {networks: NetworkConfig[]}; } catch (e: any) { // eslint-disable-next-line @typescript-eslint/restrict-template-expressions log.error(`Failed to read user ${colors.bold(name)}: ${e}`); diff --git a/server/models/chan.ts b/server/models/chan.ts index 65a4067a..82be2152 100644 --- a/server/models/chan.ts +++ b/server/models/chan.ts @@ -33,6 +33,13 @@ export type FilteredChannel = Chan & { totalMessages: number; }; +export type ChanConfig = { + name: string; + key?: string; + muted?: boolean; + type?: string; +}; + class Chan { id: number; messages: Msg[]; diff --git a/server/models/network.ts b/server/models/network.ts index a47757f5..269a415e 100644 --- a/server/models/network.ts +++ b/server/models/network.ts @@ -1,7 +1,7 @@ import _ from "lodash"; import {v4 as uuidv4} from "uuid"; import IrcFramework, {Client as IRCClient} from "irc-framework"; -import Chan, {Channel, ChanType} from "./chan"; +import Chan, {ChanConfig, Channel, ChanType} from "./chan"; import Msg, {MessageType} from "./msg"; import Prefix from "./prefix"; import Helper, {Hostmask} from "../helper"; @@ -67,6 +67,34 @@ export type NetworkWithIrcFramework = Network & { }; }; +export type NetworkConfig = { + nick: string; + name: string; + host: string; + port: number; + tls: boolean; + userDisconnected: boolean; + rejectUnauthorized: boolean; + password: string; + awayMessage: string; + commands: any[]; + username: string; + realname: string; + leaveMessage: string; + sasl: string; + saslAccount: string; + saslPassword: string; + channels: ChanConfig[]; + uuid: string; + proxyHost: string; + proxyPort: number; + proxyUsername: string; + proxyPassword: string; + proxyEnabled: boolean; + highlightRegex?: string; + ignoreList: any[]; +}; + class Network { nick: string; name: string; From a049a01aeb2b09edaaf46411bb764c14a607b343 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 14 Mar 2023 21:24:06 +0100 Subject: [PATCH 2/4] Client: move socket connection out of the constructor It will make it easier to write tests for what used to be in the connect() method --- server/client.ts | 33 +++++++++++++++++++++++--------- server/clientManager.ts | 1 + server/plugins/inputs/connect.ts | 2 +- server/server.ts | 3 ++- test/tests/customhighlights.ts | 1 + 5 files changed, 29 insertions(+), 11 deletions(-) diff --git a/server/client.ts b/server/client.ts index 049c0744..c34ab0e3 100644 --- a/server/client.ts +++ b/server/client.ts @@ -180,8 +180,16 @@ class Client { this.registerPushSubscription(session, session.pushSubscription, true); } }); + } - (client.config.networks || []).forEach((network) => client.connect(network, true)); + connect() { + const client = this; + + if (client.networks.length !== 0) { + throw new Error(`${client.name} is already connected`); + } + + (client.config.networks || []).forEach((network) => client.connectToNetwork(network, true)); // Networks are stored directly in the client object // We don't need to keep it in the config object @@ -192,7 +200,7 @@ class Client { // Networks are created instantly, but to reduce server load on startup // We randomize the IRC connections and channel log loading - let delay = manager.clients.length * 500; + let delay = client.manager.clients.length * 500; client.networks.forEach((network) => { setTimeout(() => { network.channels.forEach((channel) => channel.loadMessages(client, network)); @@ -205,7 +213,7 @@ class Client { delay += 1000 + Math.floor(Math.random() * 1000); }); - client.fileHash = manager.getDataToSave(client).newHash; + client.fileHash = client.manager.getDataToSave(client).newHash; } } @@ -242,12 +250,10 @@ class Client { return false; } - connect(args: Record, isStartup = false) { + networkFromConfig(args: Record): Network { const client = this; - let channels: Chan[] = []; - // Get channel id for lobby before creating other channels for nicer ids - const lobbyChannelId = client.idChan++; + let channels: Chan[] = []; if (Array.isArray(args.channels)) { let badName = false; @@ -295,7 +301,7 @@ class Client { } // TODO; better typing for args - const network = new Network({ + return new Network({ uuid: args.uuid, name: String( args.name || (Config.values.lockNetwork ? Config.values.defaults.name : "") || "" @@ -323,6 +329,15 @@ class Client { proxyUsername: String(args.proxyUsername || ""), proxyPassword: String(args.proxyPassword || ""), }); + } + + connectToNetwork(args: Record, isStartup = false) { + const client = this; + + // Get channel id for lobby before creating other channels for nicer ids + const lobbyChannelId = client.idChan++; + + const network = this.networkFromConfig(args); // Set network lobby channel id network.getLobby().id = lobbyChannelId; @@ -363,7 +378,7 @@ class Client { if (!isStartup) { client.save(); - channels.forEach((channel) => channel.loadMessages(client, network)); + network.channels.forEach((channel) => channel.loadMessages(client, network)); } } diff --git a/server/clientManager.ts b/server/clientManager.ts index d32081e3..85055d88 100644 --- a/server/clientManager.ts +++ b/server/clientManager.ts @@ -145,6 +145,7 @@ class ClientManager { } } else { client = new Client(this, name, userConfig); + client.connect(); this.clients.push(client); } diff --git a/server/plugins/inputs/connect.ts b/server/plugins/inputs/connect.ts index 8ba60c20..85262e57 100644 --- a/server/plugins/inputs/connect.ts +++ b/server/plugins/inputs/connect.ts @@ -39,7 +39,7 @@ const input: PluginInputHandler = function (network, chan, cmd, args) { } const host = args[0]; - this.connect({host, port, tls}); + this.connectToNetwork({host, port, tls}); return true; }; diff --git a/server/server.ts b/server/server.ts index e5b005dd..5ecdea92 100644 --- a/server/server.ts +++ b/server/server.ts @@ -485,7 +485,7 @@ function initializeClient( data.commands = null; data.ignoreList = null; - client.connect(data); + client.connectToNetwork(data); } }); @@ -948,6 +948,7 @@ function performAuthentication(this: Socket, data) { if (Config.values.public) { client = new Client(manager!); + client.connect(); manager!.clients.push(client); socket.on("disconnect", function () { diff --git a/test/tests/customhighlights.ts b/test/tests/customhighlights.ts index d9a8cfd4..30f84c01 100644 --- a/test/tests/customhighlights.ts +++ b/test/tests/customhighlights.ts @@ -27,6 +27,7 @@ describe("Custom highlights", function () { }, } as any ); + client.connect(); logInfoStub.restore(); expect(userLoadedLog).to.equal("User test loaded\n"); From d58fb845651fe2859313c05a80cdcdebc27a8c68 Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Tue, 14 Mar 2023 21:24:19 +0100 Subject: [PATCH 3/4] Fix test wording --- test/models/network.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/network.ts b/test/models/network.ts index 039866e8..4cc63f00 100644 --- a/test/models/network.ts +++ b/test/models/network.ts @@ -30,7 +30,7 @@ describe("Network", function () { expect(network1.uuid).to.not.equal(network2.uuid); }); - it("lobby should be at the top", function () { + it("should keep the lobby at the top", function () { const network = new Network({ name: "Super Nice Network", channels: [ From 320075e376eecc0843f57b2f9b3207f8f245930e Mon Sep 17 00:00:00 2001 From: Val Lorentz Date: Wed, 15 Mar 2023 10:49:56 +0100 Subject: [PATCH 4/4] Remove override of UserConfig --- server/client.ts | 11 +++-------- server/clientManager.ts | 2 +- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/server/client.ts b/server/client.ts index c34ab0e3..d5ffe84e 100644 --- a/server/client.ts +++ b/server/client.ts @@ -78,6 +78,7 @@ export type UserConfig = { hostname?: string; isSecure?: boolean; }; + networks?: NetworkConfig[]; }; export type Mention = { @@ -95,9 +96,7 @@ class Client { attachedClients!: { [socketId: string]: {token: string; openChannel: number}; }; - config!: UserConfig & { - networks?: NetworkConfig[]; - }; + config!: UserConfig; id!: number; idMsg!: number; idChan!: number; @@ -112,11 +111,7 @@ class Client { fileHash!: string; - constructor( - manager: ClientManager, - name?: string, - config = {} as UserConfig & {networks: NetworkConfig[]} - ) { + constructor(manager: ClientManager, name?: string, config = {} as UserConfig) { _.merge(this, { awayMessage: "", lastActiveChannel: -1, diff --git a/server/clientManager.ts b/server/clientManager.ts index 85055d88..78e94d18 100644 --- a/server/clientManager.ts +++ b/server/clientManager.ts @@ -285,7 +285,7 @@ class ClientManager { try { const data = fs.readFileSync(userPath, "utf-8"); - return JSON.parse(data) as UserConfig & {networks: NetworkConfig[]}; + return JSON.parse(data) as UserConfig; } catch (e: any) { // eslint-disable-next-line @typescript-eslint/restrict-template-expressions log.error(`Failed to read user ${colors.bold(name)}: ${e}`);