Update import locks to be properly atomic.

server/src/external/mongo/db.ts

@@ -164,7 +164,7 @@ const db = {
 	"bms-course-lookup": monkDB.get<BMSCourseDocument>("bms-course-lookup"),
 	"api-tokens": monkDB.get<APITokenDocument>("api-tokens"),
 	"orphan-scores": monkDB.get<OrphanScoreDocument>("orphan-scores"),
-	"import-locks": monkDB.get<ImportLockDocument>("import-locks"),
+	"import-locks": monkDB.get<{ userID: integer; locked: boolean }>("import-locks"),
 	tables: monkDB.get<TableDocument>("tables"),
 	"game-settings": monkDB.get<UGPTSettings>("game-settings"),
 	"game-stats-snapshots": monkDB.get<UserGameStatsSnapshot>("game-stats-snapshots"),

server/src/lib/score-import/framework/import-locks/lock.ts

@@ -2,33 +2,49 @@ import db from "external/mongo/db";
 import { integer } from "tachi-common";
 
 /**
- * Gets a users "import lock" if one exists. If one does not exist, it is set.
+ * If a user has no ongoing import, enable the import lock and return true.
+ * If a user has an ongoing import, return false.
+ *
  * @param userID - The user this import lock is for.
- * @returns If no lock exists for this user (and one was created), undefined is returned
- * If a lock exists for the user, the lock is returned.
+ * @returns True if the lock was set successfully, false if the user already
+ * has a lock.
  */
-export function GetOrSetUserLock(userID: integer) {
-	return db["import-locks"].findOneAndUpdate(
+export async function CheckAndSetOngoingImportLock(userID: integer) {
+	const lockExists = await db["import-locks"].findOne({
+		userID,
+	});
+
+	if (!lockExists) {
+		await db["import-locks"].insert({
+			userID,
+			locked: false,
+		});
+	}
+
+	const lockWasSet = await db["import-locks"].findOneAndUpdate(
 		{
 			userID,
+			locked: false,
 		},
 		{
-			$set: { userID },
-		},
-		{
-			upsert: true,
-			// this is marked as deprecated, but it shouldn't be, as returnDocument: "before"
-			// does nothing.
-			returnOriginal: true,
+			$set: { locked: true },
 		}
 	);
+
+	return !lockWasSet;
 }
 
 /**
- * Removes a users import lock.
+ * Disable a users import lock.
  */
-export function RemoveUserLock(userID: integer) {
-	return db["import-locks"].remove({
-		userID,
-	});
+export function UnsetOngoingImportLock(userID: integer) {
+	return db["import-locks"].findOneAndUpdate(
+		{
+			userID,
+			locked: true,
+		},
+		{
+			$set: { locked: false },
+		}
+	);
 }

server/src/lib/score-import/framework/score-importing/score-import-main.ts

@@ -19,7 +19,7 @@ import { InternalFailure } from "../common/converter-failures";
 import { CreateScoreLogger } from "../common/import-logger";
 import { ScorePlaytypeMap } from "../common/types";
 import { GetAndUpdateUsersGoals } from "../goals/goals";
-import { GetOrSetUserLock, RemoveUserLock } from "../import-locks/lock";
+import { CheckAndSetOngoingImportLock, UnsetOngoingImportLock } from "../import-locks/lock";
 import { UpdateUsersMilestones } from "../milestones/milestones";
 import { ProcessPBs } from "../pb/process-pbs";
 import { CreateSessions } from "../sessions/sessions";
@@ -50,17 +50,13 @@ export default async function ScoreImportMain<D, C>(
 		);
 	}
 
-	// in the event of any error, we remove the user lock.
 	try {
-		const lock = await GetOrSetUserLock(user.id);
+		const hasNoOngoingImport = await CheckAndSetOngoingImportLock(user.id);
 
-		if (lock) {
+		if (hasNoOngoingImport) {
 			// @danger
 			// Throwing away an import if the user already has one outgoing is *bad*, as in the case
-			// of degraded performance we might just start throwing scores away. This is obviously
-			// not great, but any other solution involves making a queue, which can't be done because
-			// InputParser is a very dynamic function that cannot be stored in redis or something.
-			//
+			// of degraded performance we might just start throwing scores away.
 			// Under normal circumstances, there is no scenario where a user would have two ongoing
 			// imports at the same time - even if they were using single-score imports on a 5 second
 			// chart, as each score import takes only around ~10-15milliseconds.
@@ -216,7 +212,7 @@ export default async function ScoreImportMain<D, C>(
 
 		return ImportDocument;
 	} finally {
-		await RemoveUserLock(user.id);
+		await UnsetOngoingImportLock(user.id);
 	}
 }
 

server/src/test-utils/mock-db/import-locks.json

@@ -1 +1,6 @@
-[]
+[
+	{
+		"userID": 1,
+		"locked": false
+	}
+]