Merge pull request #750 from thelounge/xpaw/reduce-save

Avoid unnecessary disk writes when saving user

src/client.js

@@ -292,19 +292,15 @@ Client.prototype.setPassword = function(hash, callback) {
 		client.manager.updateUser(client.name, {
 			token: token,
 			password: hash
-		});
+		}, function(err) {
+			if (err) {
+				log.error("Failed to update password of", client.name, err);
+				return callback(false);
+			}
 
-		// re-read the hash off disk to ensure we use whatever is saved. this will
-		// prevent situations where the password failed to save properly and so
-		// a restart of the server would forget the change and use the old
-		// password again.
-		var user = client.manager.readUserConfig(client.name);
-		if (user.password === hash) {
 			client.config.password = hash;
-			callback(true);
+			return callback(true);
-		} else {
+		});
-			callback(false);
-		}
 	});
 };
 
@@ -464,23 +460,13 @@ Client.prototype.clientDetach = function(socketId) {
 	delete this.attachedClients[socketId];
 };
 
-var timer;
+Client.prototype.save = _.debounce(function SaveClient() {
-Client.prototype.save = function(force) {
-	var client = this;
-
 	if (Helper.config.public) {
 		return;
 	}
 
-	if (!force) {
+	const client = this;
-		clearTimeout(timer);
+	let json = {};
-		timer = setTimeout(function() {
-			client.save(true);
-		}, 1000);
-		return;
-	}
-
-	var json = {};
 	json.networks = this.networks.map(n => n.export());
 	client.manager.updateUser(client.name, json);
-};
+}, 1000, {maxWait: 10000});

src/clientManager.js

@@ -94,8 +94,8 @@ ClientManager.prototype.addUser = function(name, password) {
 	return true;
 };
 
-ClientManager.prototype.updateUser = function(name, opts) {
+ClientManager.prototype.updateUser = function(name, opts, callback) {
-	var users = this.getUsers();
+	const users = this.getUsers();
 	if (users.indexOf(name) === -1) {
 		return false;
 	}
@@ -103,19 +103,25 @@ ClientManager.prototype.updateUser = function(name, opts) {
 		return false;
 	}
 
-	var user = {};
+	let user = this.readUserConfig(name);
-	try {
+	const currentUser = JSON.stringify(user, null, "\t");
-		user = this.readUserConfig(name);
+	_.assign(user, opts);
-		_.assign(user, opts);
+	const newUser = JSON.stringify(user, null, "\t");
-		fs.writeFileSync(
+
-			Helper.getUserConfigPath(name),
+	// Do not touch the disk if object has not changed
-			JSON.stringify(user, null, "\t")
+	if (currentUser === newUser) {
-		);
+		return callback ? callback() : true;
-	} catch (e) {
-		log.error("Failed to update user", e);
-		return;
 	}
-	return true;
+
+	fs.writeFile(Helper.getUserConfigPath(name), newUser, (err) => {
+		if (err) {
+			log.error("Failed to update user", err);
+		}
+
+		if (callback) {
+			callback(err);
+		}
+	});
 };
 
 ClientManager.prototype.readUserConfig = function(name) {

src/server.js

@@ -273,9 +273,7 @@ function localAuth(client, user, password, callback) {
 		var hash = Helper.password.hash(password);
 
 		client.setPassword(hash, function(success) {
-			if (!success) {
+			if (success) {
-				log.error("Failed to update password of", client.name, "to match new security requirements");
-			} else {
 				log.info("User", client.name, "logged in and their hashed password has been updated to match new security requirements");
 			}
 		});