From bef37e27cb6be9fd72387f45d37d11a7653f6787 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bar=C4=B1=C5=9F=20Soner=20U=C5=9Fakl=C4=B1?=
 <barisusakli@gmail.com>
Date: Thu, 18 Jun 2020 23:16:48 -0400
Subject: [PATCH] fix: test lock for user create (#8415)

* fix: test lock for user create

* fix: redis hdel with undefined

* feat: add test for undefined key in deleteObjectFields
---
 src/database/redis/hash.js |  6 +++++-
 src/user/create.js         | 24 ++++++++++++++++++++++--
 test/database/hash.js      |  4 ++++
 test/user.js               | 34 ++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 3 deletions(-)

diff --git a/src/database/redis/hash.js b/src/database/redis/hash.js
index afa2b7e016..d5636cdfe7 100644
--- a/src/database/redis/hash.js
+++ b/src/database/redis/hash.js
@@ -154,7 +154,11 @@ module.exports = function (module) {
 	};
 
 	module.deleteObjectFields = async function (key, fields) {
-		if (!Array.isArray(fields) || !fields.length) {
+		if (!key || !Array.isArray(fields) || !fields.length) {
+			return;
+		}
+		fields = fields.filter(Boolean);
+		if (!fields.length) {
 			return;
 		}
 		await module.client.async.hdel(key, fields);
diff --git a/src/user/create.js b/src/user/create.js
index 8724472cfe..a205851323 100644
--- a/src/user/create.js
+++ b/src/user/create.js
@@ -14,10 +14,30 @@ module.exports = function (User) {
 		if (data.email !== undefined) {
 			data.email = String(data.email).trim();
 		}
-		const timestamp = data.timestamp || Date.now();
 
 		await User.isDataValid(data);
 
+		try {
+			await lock(data.username, '[[error:username-taken]]');
+			if (data.email) {
+				await lock(data.email, '[[error:email-taken]]');
+			}
+			return await create(data);
+		} finally {
+			await db.deleteObjectFields('locks', [data.username, data.email]);
+		}
+	};
+
+	async function lock(value, error) {
+		const count = await db.incrObjectField('locks', value);
+		if (count > 1) {
+			throw new Error(error);
+		}
+	}
+
+	async function create(data) {
+		const timestamp = data.timestamp || Date.now();
+
 		let userData = {
 			username: data.username,
 			userslug: data.userslug,
@@ -92,7 +112,7 @@ module.exports = function (User) {
 		}
 		plugins.fireHook('action:user.create', { user: userData, data: data });
 		return userData.uid;
-	};
+	}
 
 	async function storePassword(uid, password) {
 		if (!password) {
diff --git a/test/database/hash.js b/test/database/hash.js
index b6aba5852f..8d94a23b6b 100644
--- a/test/database/hash.js
+++ b/test/database/hash.js
@@ -435,6 +435,10 @@ describe('Hash methods', function () {
 			});
 		});
 
+		it('should not error if one of the fields is undefined', async function () {
+			await db.deleteObjectFields('someKey', ['best', undefined]);
+		});
+
 		it('should not error if field is null', function (done) {
 			db.deleteObjectField('someKey', null, function (err) {
 				assert.ifError(err);
diff --git a/test/user.js b/test/user.js
index fa0b271404..c122d7f6b5 100644
--- a/test/user.js
+++ b/test/user.js
@@ -100,6 +100,40 @@ describe('User', function () {
 				done();
 			});
 		});
+
+		it('should error if username is already taken', async function () {
+			let err;
+			async function tryCreate(data) {
+				try {
+					return await User.create(data);
+				} catch (_err) {
+					err = _err;
+				}
+			}
+
+			await Promise.all([
+				tryCreate({ username: 'dupe1' }),
+				tryCreate({ username: 'dupe1' }),
+			]);
+			assert.strictEqual(err.message, '[[error:username-taken]]');
+		});
+
+		it('should error if email is already taken', async function () {
+			let err;
+			async function tryCreate(data) {
+				try {
+					return await User.create(data);
+				} catch (_err) {
+					err = _err;
+				}
+			}
+
+			await Promise.all([
+				tryCreate({ username: 'notdupe1', email: 'dupe@dupe.com' }),
+				tryCreate({ username: 'notdupe2', email: 'dupe@dupe.com' }),
+			]);
+			assert.strictEqual(err.message, '[[error:email-taken]]');
+		});
 	});
 
 	describe('.uniqueUsername()', function () {