Merge pull request #1154 from bbassie/fix/deleted-exports

Handle deleted export files

packages/controller/src/Controller.ts

@@ -226,6 +226,8 @@ export default class Controller {
 		await fs.mkdir(modsDirectory, { recursive: true });
 		const modStore = await lib.ModStore.fromDirectory(modsDirectory);
 
+		await this.validateExports(modPacks, "static");
+
 		// Add default mod packs
 		if (modPacks.size === 0) {
 			modPacks.setMany(lib.ModPack.defaultModPacks);
@@ -876,6 +878,37 @@ export default class Controller {
 		});
 	}
 
+	/** Remove export manifests that reference files missing from staticDirectory */
+	static async validateExports(modPacks: lib.SubscribableDatastore<lib.ModPack>, staticDirectory: string) {
+		const invalid: lib.ModPack[] = [];
+		for (const modPack of modPacks.valuesMutable()) {
+			if (!modPack.exportManifest) {
+				continue;
+			}
+			const missing: string[] = [];
+			for (const file of Object.values(modPack.exportManifest.assets)) {
+				const valid = path.basename(file) === file
+					&& await fs.stat(path.join(staticDirectory, file)).then(s => s.isFile(), () => false);
+				if (!valid) {
+					missing.push(file);
+				}
+			}
+			if (missing.length) {
+				Controller.dropExport(modPack, `missing ${missing.join(", ")}`);
+				invalid.push(modPack);
+			}
+		}
+		if (invalid.length) {
+			modPacks.setMany(invalid);
+		}
+	}
+
+	/** Clear the export manifest of a mutable mod pack, caller must save it */
+	static dropExport(modPack: lib.ModPack, reason: string) {
+		logger.warn(`Removing export data of mod pack ${modPack.name} (id ${modPack.id}): ${reason}`);
+		modPack.exportManifest = undefined;
+	}
+
 	static migrateRoles(rawJson: unknown[]): Static<typeof lib.Role.jsonSchema>[] {
 		const serialized = rawJson as Static<typeof lib.Role.jsonSchema>[];
 		// migrate: host token permissions were removed with host pairing in 2.1
@@ -984,6 +1017,7 @@ export default class Controller {
 			express.static(path.join(import.meta.dirname, "..", "..", "web", "static"), staticOptions)
 		);
 		app.use("/static", express.static("static", staticOptions)); // Used for data export files
+		app.use("/static", routes.missingExportAsset);
 
 		// Add API routes
 		routes.addRouteHandlers(app);

packages/controller/src/routes.ts

@@ -394,6 +394,30 @@ async function uploadExport(req: Request, res: Response) {
 	res.sendStatus(200);
 }
 
+// Reached when no static file matched, drops exports whose files were removed
+export function missingExportAsset(req: Request, res: Response, next: any) {
+	const file = req.path.slice(1);
+	if (!["GET", "HEAD"].includes(req.method) || !file) {
+		next();
+		return;
+	}
+	const modPacks = res.app.locals.controller.modPacks;
+	const affected = [...modPacks.valuesMutable()].filter(
+		modPack => modPack.exportManifest && Object.values(modPack.exportManifest.assets).includes(file)
+	);
+	if (!affected.length) {
+		next();
+		return;
+	}
+	for (const modPack of affected) {
+		Controller.dropExport(modPack, `${file} was removed from disk`);
+	}
+	modPacks.setMany(affected);
+	res.status(404).type("text/plain").send(
+		`Export data file ${file} is missing, run export-data on an instance to regenerate it`
+	);
+}
+
 type ProxyStreamEvents = {
 	"error": [ err: any ],
 	"close": [],

packages/web_ui/src/components/ModPackViewPage.tsx

@@ -1108,7 +1108,9 @@ function useExportedAsset(modPack: lib.ModPack | undefined, asset: "settings"|"l
 	}
 	useEffect(() => {
 		async function load() {
+			const empty = asset === "locale" ? new Map() : {};
 			if (!assetFilename) {
+				setAssetData(empty);
 				return;
 			}
 
@@ -1117,7 +1119,14 @@ function useExportedAsset(modPack: lib.ModPack | undefined, asset: "settings"|"l
 				let data = await response.json();
 				setAssetData(asset === "locale" ? new Map(data) : data);
 			} else {
-				logger.error(`Error loading mod pack asset "${asset}" (${response.status}): ${await response.text()}`);
+				if (response.status === 404) {
+					logger.warn(`Mod pack asset "${asset}" is missing on the controller`);
+				} else {
+					logger.error(
+						`Error loading mod pack asset "${asset}" (${response.status}): ${await response.text()}`
+					);
+				}
+				setAssetData(empty);
 			}
 		}
 

packages/web_ui/src/store/export_locale_stores.ts

@@ -24,19 +24,24 @@ class ExportLocaleStore {
 		let response = await fetch(`${staticRoot}${cacheEntry.localePath}`);
 		if (response.ok) {
 			cacheEntry.data = new Map(await response.json() as ExportLocale);
-			for (const callback of cacheEntry.callbacks) {
-				callback();
-			}
 		} else {
-			notify(
-				`Failed to locale for mod pack ${modPackId}, server returned: ` +
-				`${response.status} ${response.statusText}`
-			);
+			if (response.status === 404) {
+				logger.warn(`Locale for mod pack ${modPackId} is missing on the controller`);
+			} else {
+				notify(
+					`Failed to locale for mod pack ${modPackId}, server returned: ` +
+					`${response.status} ${response.statusText}`
+				);
+			}
+			cacheEntry.data = new Map();
+		}
+		for (const callback of cacheEntry.callbacks) {
+			callback();
 		}
 	}
 
 	subscribe(modPackId: number | undefined, localePath: string | undefined, callback: () => void) {
-		if (modPackId === undefined || !localePath) {
+		if (modPackId === undefined) {
 			return () => {};
 		}
 

packages/web_ui/src/store/export_prototype_metadata_store.ts

@@ -28,10 +28,14 @@ class ExportPrototypeMetadataStore {
 
 		const response = await fetch(`${staticRoot}${cacheEntry.metadataPath}`);
 		if (!response.ok) {
-			notify(
-				`Failed to fetch prototype metadata for mod pack ${modPackId}, server returned: ` +
-				`${response.status} ${response.statusText}`
-			);
+			if (response.status === 404) {
+				logger.warn(`Prototype metadata for mod pack ${modPackId} is missing on the controller`);
+			} else {
+				notify(
+					`Failed to fetch prototype metadata for mod pack ${modPackId}, server returned: ` +
+					`${response.status} ${response.statusText}`
+				);
+			}
 			// eslint-disable-next-line node/no-sync
 			cacheEntry.styleSheet.replaceSync("");
 			cacheEntry.metadata = new Map();

test/controller/Controller.js

@@ -198,6 +198,43 @@ describe("controller/src/Controller", function() {
 				});
 			});
 		});
+		describe(".validateExports()", function() {
+			let dir;
+			before(async function() {
+				dir = await fs.mkdtemp(path.join(os.tmpdir(), "clusterio-exports-"));
+				await fs.writeFile(path.join(dir, "locale.abc.json"), "[]");
+				await fs.writeFile(path.join(dir, "metadata.abc.json"), "{}");
+			});
+			after(async function() {
+				await fs.rm(dir, { recursive: true, force: true });
+			});
+			it("should drop export manifests with missing or invalid files", async function() {
+				const provider = new lib.MemoryDatastoreProvider();
+				const modPacks = new lib.SubscribableDatastore(provider, new Map());
+				const packs = [
+					{ id: 1, assets: { locale: "locale.abc.json", metadata: "metadata.abc.json" } },
+					{ id: 2, assets: { locale: "locale.abc.json", metadata: "metadata.def.json" } },
+					{ id: 3, assets: { locale: "../locale.abc.json" } },
+					{ id: 4 },
+				].map(({ id, assets }) => {
+					const modPack = ModPack.fromJSON({ id, name: `pack ${id}` });
+					if (assets) {
+						modPack.exportManifest = new lib.ExportManifest(assets);
+					}
+					return modPack;
+				});
+				modPacks.setMany(packs);
+				const updates = [];
+				modPacks.on("update", updated => updates.push(...updated));
+
+				await Controller.validateExports(modPacks, dir);
+				assert.deepEqual(modPacks.get(1).exportManifest.assets, packs[0].exportManifest.assets);
+				assert.equal(modPacks.get(2).exportManifest, undefined);
+				assert.equal(modPacks.get(3).exportManifest, undefined);
+				assert.equal(modPacks.get(4).exportManifest, undefined);
+				assert.deepEqual(updates.map(modPack => modPack.id), [2, 3]);
+			});
+		});
 		describe(".loadJsonObject()", function() {
 			let dir;
 			before(async function() {

test/controller/routes.js

@@ -1,11 +1,15 @@
 import assert from "node:assert/strict";
 import events from "node:events";
+import fs from "node:fs/promises";
 import http from "node:http";
-
+import os from "node:os";
+import path from "node:path";
 import jwt from "jsonwebtoken";
+import express from "express";
 
-import { SubscribableDatastore, wait, expositionContentType } from "@clusterio/lib";
+import * as lib from "@clusterio/lib";
 import { Controller, HostPairingManager, HostRecord } from "@clusterio/controller";
+
 import * as routes from "@clusterio/controller/dist/node/src/routes.js";
 import * as mock from "../mock.js";
 
@@ -51,7 +55,7 @@ describe("controller/src/routes", function() {
 			stream = await routes.createProxyStream(controller.app);
 			responses = await Promise.all([
 				(async function() {
-					await wait(100);
+					await lib.wait(100);
 					return await fetch(`${endpoint}/${stream.id}`, {
 						method: "PUT", body: "test content",
 					});
@@ -67,7 +71,7 @@ describe("controller/src/routes", function() {
 					method: "PUT", body: "test content",
 				}),
 				(async function() {
-					await wait(100);
+					await lib.wait(100);
 					return await fetch(`${endpoint}/${stream.id}`);
 				}()),
 			]);
@@ -114,7 +118,7 @@ describe("controller/src/routes", function() {
 				let response = await fetch(endpoint, { headers });
 				assert.equal(response.status, 200);
 				const contentType = response.headers.get("content-type");
-				for (const part of expositionContentType.split("; ")) {
+				for (const part of lib.expositionContentType.split("; ")) {
 					assert(contentType.includes(part), `expected ${part} in content type ${contentType}`);
 				}
 				assert(/^# HELP /m.test(await response.text()), "expected exposition text");
@@ -124,7 +128,7 @@ describe("controller/src/routes", function() {
 			let calls = 0;
 			controller.hooks.metrics.attach("test", async () => {
 				calls += 1;
-				await wait(100);
+				await lib.wait(100);
 			});
 			const headers = { "X-Access-Token": testToken };
 			let responses = await Promise.all([
@@ -141,6 +145,42 @@ describe("controller/src/routes", function() {
 			assert.equal(calls, 2);
 		});
 	});
+	describe("missingExportAsset()", function() {
+		let dir;
+		beforeEach(async function() {
+			dir = await fs.mkdtemp(path.join(os.tmpdir(), "clusterio-static-"));
+			await fs.writeFile(path.join(dir, "locale.abc.json"), "[]");
+			const provider = new lib.MemoryDatastoreProvider();
+			controller.modPacks = new lib.SubscribableDatastore(provider, new Map());
+			const modPack = lib.ModPack.fromJSON({ id: 1, name: "test" });
+			modPack.exportManifest = new lib.ExportManifest({
+				locale: "locale.abc.json", metadata: "metadata.abc.json",
+			});
+			controller.modPacks.set(modPack);
+			controller.app.use("/static", express.static(dir));
+			controller.app.use("/static", routes.missingExportAsset);
+		});
+		afterEach(async function() {
+			await fs.rm(dir, { recursive: true, force: true });
+		});
+		it("should serve export files that exist", async function() {
+			const response = await fetch(`http://localhost:${port}/static/locale.abc.json`);
+			assert.equal(response.status, 200);
+			assert(controller.modPacks.get(1).exportManifest);
+		});
+		it("should drop the export and explain when a file is missing", async function() {
+			const response = await fetch(`http://localhost:${port}/static/metadata.abc.json`);
+			assert.equal(response.status, 404);
+			assert.match(await response.text(), /metadata\.abc\.json is missing/);
+			assert.equal(controller.modPacks.get(1).exportManifest, undefined);
+		});
+		it("should pass through files not in an export", async function() {
+			const response = await fetch(`http://localhost:${port}/static/other.js`);
+			assert.equal(response.status, 404);
+			assert.doesNotMatch(await response.text(), /export-data/);
+			assert(controller.modPacks.get(1).exportManifest);
+		});
+	});
 	describe("stripStaticPrefix()", function() {
 		it("should remove a leading static/", function() {
 			assert.equal(routes.stripStaticPrefix("static/main.abc.js"), "main.abc.js");
@@ -218,7 +258,7 @@ describe("controller/src/routes", function() {
 		beforeEach(function() {
 			endpoint = `http://localhost:${port}/api/host-pairing`;
 			// The mock controller lacks what pairing needs, use the real implementations.
-			controller.hosts = new SubscribableDatastore();
+			controller.hosts = new lib.SubscribableDatastore();
 			controller.hostPairing = new HostPairingManager(controller);
 			controller.generateHostToken = Controller.prototype.generateHostToken;
 			controller.verifyHostToken = Controller.prototype.verifyHostToken;
@@ -227,8 +267,8 @@ describe("controller/src/routes", function() {
 			playerToken = controller.users.signUserToken(controller.users.getByName("player"));
 		});
 
-		function post(path, body, headers = {}) {
-			return fetch(`${endpoint}${path}`, {
+		function post(urlPath, body, headers = {}) {
+			return fetch(`${endpoint}${urlPath}`, {
 				method: "POST",
 				headers: { "Content-Type": "application/json", ...headers },
 				body: typeof body === "string" || body === undefined ? body : JSON.stringify(body),
@@ -453,7 +493,7 @@ describe("controller/src/routes", function() {
 			it("should not be accepted in the URL", async function() {
 				const { requestId } = await createRequest();
 				const id = encodeURIComponent(requestId);
-				for (const [method, path] of [
+				for (const [method, urlPath] of [
 					["GET", `/poll?requestId=${id}`],
 					["GET", `/poll/${id}`],
 					["POST", `/poll/${id}`],
@@ -461,8 +501,8 @@ describe("controller/src/routes", function() {
 					["GET", `/${id}`],
 					["POST", `/cancel/${id}`],
 				]) {
-					const response = await fetch(`${endpoint}${path}`, { method });
-					assert.equal(response.status, 404, `${method} ${path}`);
+					const response = await fetch(`${endpoint}${urlPath}`, { method });
+					assert.equal(response.status, 404, `${method} ${urlPath}`);
 				}
 				await assertResponse(await post("/poll", { requestId }), 202, { status: "pending" });
 			});