Save deleted players in the same file as player data

The tombstones lived in player_sync_deleted.json, written after
player_sync.json. A stop between the two writes lost the tombstone of a
fresh delete, and the next upload from an old copy brought the deleted
data back. Deleted players are now entries with deleted: true in
player_sync.json, so both are written in one safeOutputFile call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

plugins/player_sync/README.md

@@ -49,7 +49,7 @@ Importing and deleting both start a new epoch for the player.
 An instance holding a copy from an earlier epoch throws it away the next time the player joins, even if it is newer than what the controller has, and an upload from an earlier epoch is discarded.
 After an import the instance downloads the new data.
 After a delete the next join turns whatever the player has on that instance into the new synced inventory.
-Deleted players stay listed in `player_sync_deleted.json` in the database directory so their epoch survives a controller restart.
+Deleted players stay in `player_sync.json` as an entry with `deleted: true` so their epoch survives a controller restart.
 Both are refused while an instance holds the lock on the player, because the instance uploads its own copy when the player leaves and that would undo the change.
 Have the player leave first, or release the lock if the instance is stuck holding it.
 

plugins/player_sync/controller.ts

@@ -7,6 +7,9 @@ import path from "path";
 import * as lib from "@clusterio/lib";
 import * as msg from "./messages.js";
 
+// Left behind by deleted player data so instances can tell their copy is stale
+type Tombstone = { generation: number, epoch: number, deleted: true };
+
 async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) {
 	let databaseDirectory = config.get("controller.database_directory");
 	let itemsPath = path.resolve(databaseDirectory, "player_sync.json");
@@ -22,39 +25,48 @@ async function loadDatabase(config: lib.ControllerConfig, logger: lib.Logger) {
 			}
 		}
 	}
-	return await loadMap<IpcPlayerData>(itemsPath, logger);
-}
-
-async function loadMap<V>(file: string, logger: lib.Logger): Promise<Map<string, V>> {
-	logger.verbose(`Loading ${file}`);
+	let entries: [string, IpcPlayerData | Tombstone][] = [];
+	logger.verbose(`Loading ${itemsPath}`);
 	try {
-		let content = await fs.readFile(file, { encoding: "utf8" });
-		return new Map(JSON.parse(content));
+		let content = await fs.readFile(itemsPath, { encoding: "utf8" });
+		entries = JSON.parse(content);
 
 	} catch (err: any) {
-		if (err.code === "ENOENT") {
-			logger.verbose(`Creating new ${path.basename(file)}`);
-			return new Map();
+		if (err.code !== "ENOENT") {
+			throw err;
 		}
-		throw err;
+		logger.verbose("Creating new player data database");
 	}
+
+	let playerDatastore = new Map<string, IpcPlayerData>();
+	let tombstones = new Map<string, Tombstone>();
+	for (let [name, entry] of entries) {
+		if ("deleted" in entry) {
+			tombstones.set(name, entry);
+		} else {
+			playerDatastore.set(name, entry);
+		}
+	}
+	return { playerDatastore, tombstones };
 }
 
-async function saveMap(file: string, map: Map<string, unknown>, logger: lib.Logger) {
+// Tombstones share the file with the player data so a delete is written in one go
+async function saveDatabase(
+	controllerConfig: lib.ControllerConfig,
+	playerDatastore: Map<string, IpcPlayerData>,
+	tombstones: Map<string, Tombstone>,
+	logger: lib.Logger,
+) {
+	let file = path.resolve(controllerConfig.get("controller.database_directory"), "player_sync.json");
 	logger.verbose(`writing ${file}`);
-	await lib.safeOutputFile(file, JSON.stringify(Array.from(map)));
+	let content = JSON.stringify([...playerDatastore, ...tombstones]);
+	await lib.safeOutputFile(file, content);
 }
 
-// Left behind by deleted player data so instances can tell their copy is stale
-type Tombstone = { generation: number, epoch: number };
-
 export default async function(context: ControllerPluginContext) {
 	const { controller, logger, plugin } = context;
 	const acquiredPlayers = new Map<string, { instanceId: number, expiresMs?: number }>();
-	const databaseDirectory = controller.config.get("controller.database_directory");
-	const playerDatastore: Map<string, IpcPlayerData> = await loadDatabase(controller.config, logger);
-	const tombstonesPath = path.resolve(databaseDirectory, "player_sync_deleted.json");
-	const tombstones = await loadMap<Tombstone>(tombstonesPath, logger);
+	const { playerDatastore, tombstones } = await loadDatabase(controller.config, logger);
 	let playerDatastoreDirty = false;
 
 	// Generation and epoch instances should compare their copy against
@@ -130,8 +142,7 @@ export default async function(context: ControllerPluginContext) {
 	controller.hooks.save.attach(plugin.name, async () => {
 		if (playerDatastoreDirty) {
 			playerDatastoreDirty = false;
-			await saveMap(path.resolve(databaseDirectory, "player_sync.json"), playerDatastore, logger);
-			await saveMap(tombstonesPath, tombstones, logger);
+			await saveDatabase(controller.config, playerDatastore, tombstones, logger);
 		}
 	});
 
@@ -292,7 +303,7 @@ export default async function(context: ControllerPluginContext) {
 		checkNotAcquired(playerName);
 
 		let { generation, epoch } = storedVersion(playerName);
-		tombstones.set(playerName, { generation, epoch: epoch + 1 });
+		tombstones.set(playerName, { generation, epoch: epoch + 1, deleted: true });
 		playerDatastore.delete(playerName);
 		playerDatastoreDirty = true;
 		logger.info(`Deleted player data for ${playerName}`);

plugins/player_sync/test/controller.js

@@ -104,9 +104,9 @@ describe("player_sync controller", function() {
 			const response = await handler(msg.AcquireRequest)(new msg.AcquireRequest(instanceId, playerName));
 			return response.status === "acquired";
 		}
-		async function saved(file = "player_sync.json") {
+		async function saved() {
 			await controller.hooks.save.invoke();
-			return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, file), "utf8")));
+			return new Map(JSON.parse(await fs.readFile(path.join(databaseDir, "player_sync.json"), "utf8")));
 		}
 		async function upload(instanceId, data) {
 			await handler(msg.UploadRequest)(new msg.UploadRequest(instanceId, data.name, data));
@@ -217,7 +217,8 @@ describe("player_sync controller", function() {
 			it("should delete stored data", async function() {
 				await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored"));
 				await del("stored");
-				assert(!(await saved()).has("stored"));
+				const response = await handler(msg.GetPlayerDataRequest)(new msg.GetPlayerDataRequest("stored"));
+				assert.equal(response.playerData, null);
 			});
 			it("should keep the generation and bump the epoch", async function() {
 				await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored"));
@@ -225,7 +226,8 @@ describe("player_sync controller", function() {
 				assert.deepEqual(
 					await acquireResponse(2, "stored"), new msg.AcquireResponse("acquired", 3, false, undefined, 1),
 				);
-				assert.deepEqual((await saved("player_sync_deleted.json")).get("stored"), { generation: 3, epoch: 1 });
+				// In the same file as the player data so the delete and its tombstone are written together
+				assert.deepEqual((await saved()).get("stored"), { generation: 3, epoch: 1, deleted: true });
 
 				controller = await load();
 				assert.deepEqual(
@@ -236,10 +238,9 @@ describe("player_sync controller", function() {
 				await handler(msg.ForceReleaseRequest)(new msg.ForceReleaseRequest("stored"));
 				await del("stored");
 				await upload(1, playerData("stored", 11));
-				assert(!(await saved()).has("stored"));
+				assert((await saved()).get("stored").deleted);
 				await upload(2, playerData("stored", 4, { epoch: 1 }));
 				assert.deepEqual((await saved()).get("stored"), playerData("stored", 4, { epoch: 1 }));
-				assert(!(await saved("player_sync_deleted.json")).has("stored"));
 
 				// The other instance downloads the baseline instead of uploading its stale copy
 				await upload(1, playerData("stored", 11));