Merge pull request #3589 from thelounge/xpaw/user-fs

Optimize user file updates

src/client.js

@@ -54,7 +54,6 @@ function Client(manager, name, config = {}) {
 		idMsg: 1,
 		name: name,
 		networks: [],
-		sockets: manager.sockets,
 		manager: manager,
 		messageStorage: [],
 		highlightRegex: null,
@@ -62,6 +61,9 @@ function Client(manager, name, config = {}) {
 
 	const client = this;
 
+	client.config.log = Boolean(client.config.log);
+	client.config.password = String(client.config.password);
+
 	if (!Helper.config.public && client.config.log) {
 		if (Helper.config.messageStorage.includes("sqlite")) {
 			client.messageStorage.push(new MessageStorage(client));
@@ -129,6 +131,8 @@ function Client(manager, name, config = {}) {
 
 			delay += 1000 + Math.floor(Math.random() * 1000);
 		});
+
+		client.fileHash = manager.getDataToSave(client).newHash;
 	}
 }
 
@@ -140,8 +144,8 @@ Client.prototype.createChannel = function(attr) {
 };
 
 Client.prototype.emit = function(event, data) {
-	if (this.sockets !== null) {
+	if (this.manager !== null) {
-		this.sockets.in(this.id).emit(event, data);
+		this.manager.sockets.in(this.id).emit(event, data);
 	}
 };
 
@@ -315,28 +319,23 @@ Client.prototype.updateSession = function(token, ip, request) {
 		agent: friendlyAgent,
 	});
 
-	client.manager.updateUser(client.name, {
+	client.save();
-		sessions: client.config.sessions,
-	});
 };
 
 Client.prototype.setPassword = function(hash, callback) {
 	const client = this;
 
-	client.manager.updateUser(
+	const oldHash = client.config.password;
-		client.name,
+	client.config.password = hash;
-		{
+	client.manager.saveUser(client, function(err) {
-			password: hash,
+		if (err) {
-		},
+			// If user file fails to write, reset it back
-		function(err) {
+			client.config.password = oldHash;
-			if (err) {
+			return callback(false);
-				return callback(false);
-			}
-
-			client.config.password = hash;
-			return callback(true);
 		}
-	);
+
+		return callback(true);
+	});
 };
 
 Client.prototype.input = function(data) {
@@ -574,7 +573,7 @@ Client.prototype.names = function(data) {
 };
 
 Client.prototype.quit = function(signOut) {
-	const sockets = this.sockets.sockets;
+	const sockets = this.manager.sockets.sockets;
 	const room = sockets.adapter.rooms[this.id];
 
 	if (room && room.sockets) {
@@ -661,9 +660,7 @@ Client.prototype.registerPushSubscription = function(session, subscription, noSa
 	session.pushSubscription = data;
 
 	if (!noSave) {
-		this.manager.updateUser(this.name, {
+		this.save();
-			sessions: this.config.sessions,
-		});
 	}
 
 	return data;
@@ -671,9 +668,7 @@ Client.prototype.registerPushSubscription = function(session, subscription, noSa
 
 Client.prototype.unregisterPushSubscription = function(token) {
 	this.config.sessions[token].pushSubscription = null;
-	this.manager.updateUser(this.name, {
+	this.save();
-		sessions: this.config.sessions,
-	});
 };
 
 Client.prototype.save = _.debounce(
@@ -683,10 +678,8 @@ Client.prototype.save = _.debounce(
 		}
 
 		const client = this;
-		const json = {};
+		client.manager.saveUser(client);
-		json.networks = this.networks.map((n) => n.export());
-		client.manager.updateUser(client.name, json);
 	},
-	1000,
+	5000,
-	{maxWait: 10000}
+	{maxWait: 20000}
 );

src/clientManager.js

@@ -3,6 +3,7 @@
 const _ = require("lodash");
 const log = require("./log");
 const colors = require("chalk");
+const crypto = require("crypto");
 const fs = require("fs");
 const path = require("path");
 const Client = require("./client");
@@ -134,10 +135,6 @@ ClientManager.prototype.addUser = function(name, password, enableLog) {
 	const user = {
 		password: password || "",
 		log: enableLog,
-		networks: [],
-		sessions: {},
-		clientSettings: {},
-		browser: {},
 	};
 
 	try {
@@ -179,27 +176,39 @@ ClientManager.prototype.addUser = function(name, password, enableLog) {
 	return true;
 };
 
-ClientManager.prototype.updateUser = function(name, opts, callback) {
+ClientManager.prototype.getDataToSave = function(client) {
-	const user = readUserConfig(name);
+	const json = Object.assign({}, client.config, {
+		networks: client.networks.map((n) => n.export()),
+	});
+	const newUser = JSON.stringify(json, null, "\t");
+	const newHash = crypto
+		.createHash("sha256")
+		.update(newUser)
+		.digest("hex");
 
-	if (!user) {
+	return {newUser, newHash};
-		return callback ? callback(true) : false;
+};
+
+ClientManager.prototype.saveUser = function(client, callback) {
+	const {newUser, newHash} = this.getDataToSave(client);
+
+	// Do not write to disk if the exported data hasn't actually changed
+	if (client.fileHash === newHash) {
+		return;
 	}
 
-	const currentUser = JSON.stringify(user, null, "\t");
+	const pathReal = Helper.getUserConfigPath(client.name);
-	_.assign(user, opts);
+	const pathTemp = pathReal + ".tmp";
-	const newUser = JSON.stringify(user, null, "\t");
-
-	// Do not touch the disk if object has not changed
-	if (currentUser === newUser) {
-		return callback ? callback() : true;
-	}
 
 	try {
-		fs.writeFileSync(Helper.getUserConfigPath(name), newUser);
+		// Write to a temp file first, in case the write fails
+		// we do not lose the original file (for example when disk is full)
+		fs.writeFileSync(pathTemp, newUser);
+		fs.renameSync(pathTemp, pathReal);
+
 		return callback ? callback() : true;
 	} catch (e) {
-		log.error(`Failed to update user ${colors.green(name)} (${e})`);
+		log.error(`Failed to update user ${colors.green(client.name)} (${e})`);
 
 		if (callback) {
 			callback(e);

src/command-line/users/reset.js

@@ -30,8 +30,10 @@ program
 			return;
 		}
 
-		const file = Helper.getUserConfigPath(name);
+		const pathReal = Helper.getUserConfigPath(name);
-		const user = require(file);
+		const pathTemp = pathReal + ".tmp";
+		const user = JSON.parse(fs.readFileSync(pathReal, "utf-8"));
+
 		log.prompt(
 			{
 				text: "Enter new password:",
@@ -44,7 +46,14 @@ program
 
 				user.password = Helper.password.hash(password);
 				user.sessions = {};
-				fs.writeFileSync(file, JSON.stringify(user, null, "\t"));
+
+				const newUser = JSON.stringify(user, null, "\t");
+
+				// Write to a temp file first, in case the write fails
+				// we do not lose the original file (for example when disk is full)
+				fs.writeFileSync(pathTemp, newUser);
+				fs.renameSync(pathTemp, pathReal);
+
 				log.info(`Successfully reset password for ${colors.bold(name)}.`);
 			}
 		);

src/server.js

@@ -591,9 +591,7 @@ function initializeClient(socket, client, token, lastMessage, openChannel) {
 					value: newSetting.value,
 				});
 
-				client.manager.updateUser(client.name, {
+				client.save();
-					clientSettings: client.config.clientSettings,
-				});
 
 				if (newSetting.name === "highlights") {
 					client.compileCustomHighlights();
@@ -630,9 +628,7 @@ function initializeClient(socket, client, token, lastMessage, openChannel) {
 
 		delete client.config.sessions[tokenToSignOut];
 
-		client.manager.updateUser(client.name, {
+		client.save();
-			sessions: client.config.sessions,
-		});
 
 		_.map(client.attachedClients, (attachedClient, socketId) => {
 			if (attachedClient.token !== tokenToSignOut) {
@@ -664,15 +660,19 @@ function initializeClient(socket, client, token, lastMessage, openChannel) {
 		socket.emit("commands", inputs.getCommands());
 	};
 
-	if (!Helper.config.public && token === null) {
+	if (Helper.config.public) {
+		sendInitEvent(null);
+	} else if (token === null) {
 		client.generateToken((newToken) => {
-			client.attachedClients[socket.id].token = token = client.calculateTokenHash(newToken);
+			token = client.calculateTokenHash(newToken);
+			client.attachedClients[socket.id].token = token;
 
 			client.updateSession(token, getClientIp(socket), socket.request);
 
 			sendInitEvent(newToken);
 		});
 	} else {
+		client.updateSession(token, getClientIp(socket), socket.request);
 		sendInitEvent(null);
 	}
 }
@@ -734,16 +734,9 @@ function performAuthentication(data) {
 	let client;
 	let token = null;
 
-	const finalInit = () => {
+	const finalInit = () =>
 		initializeClient(socket, client, token, data.lastMessage || -1, data.openChannel);
 
-		if (!Helper.config.public) {
-			client.manager.updateUser(client.name, {
-				browser: client.config.browser,
-			});
-		}
-	};
-
 	const initClient = () => {
 		// Configuration does not change during runtime of TL,
 		// and the client listens to this event only once
@@ -827,8 +820,6 @@ function performAuthentication(data) {
 		if (Object.prototype.hasOwnProperty.call(client.config.sessions, providedToken)) {
 			token = providedToken;
 
-			client.updateSession(providedToken, getClientIp(socket), socket.request);
-
 			return authCallback(true);
 		}
 	}

test/tests/customhighlights.js

@@ -13,6 +13,12 @@ describe("Custom highlights", function() {
 	const client = new Client(
 		{
 			clients: [],
+			getDataToSave() {
+				return {
+					newUser: "",
+					newHash: "",
+				};
+			},
 		},
 		"test",
 		{