Add tests

packages/controller/src/routes.ts

@@ -1,5 +1,5 @@
 import type { Application, Request, Response } from "express";
-import type Controller from "./Controller";
+import Controller from "./Controller";
 
 import busboy from "busboy";
 import crypto from "crypto";

packages/lib/src/config/validators.ts

@@ -52,7 +52,11 @@ export function integer(value: number) {
 	}
 }
 
-/** Value must be a valid file path, it does not need to exist */
+/**
+ * Value must be a valid file path, it does not need to exist
+ * On posix this will only error for non string values, which isn't very useful
+ * However, this could be expanded in the future to check system OS
+ */
 export function filePath(value: string) {
 	try {
 		path.resolve(value);

test/lib/config/classes.js

@@ -41,9 +41,49 @@ describe("lib/config/classes", function() {
 				"test.priv": { access: ["local"], type: "string", optional: true },
 				"test.cred": { credential: ["local"] },
 				"test.rdo": { readonly: ["local"] },
+				"test.base": { type: "number", initialValue: 1 },
+				"test.dependent": {
+					type: "number",
+					initialValue: 1,
+					dependsOn: ["test.base"],
+					validator(value, config) {
+						if (value < config.get("test.base")) {
+							throw new Error("dependent must be >= base");
+						}
+					},
+				},
+				"test.validated": {
+					type: "number",
+					optional: true,
+					validator(value) {
+						if (value !== null && value < 0) {
+							throw new Error("must be positive");
+						}
+					},
+				},
+				"test.objValidated": {
+					type: "object",
+					initialValue: {},
+					validator(value) {
+						if (value && value.bad !== undefined) {
+							throw new Error("bad key");
+						}
+					},
+				},
 			};
 		}
 
+		const testConfigDefault = Object.fromEntries(
+			Object.entries(TestConfig.fieldDefinitions).map(
+				([fieldName, fieldDef]) => ([
+					fieldName,
+					typeof fieldDef.initialValue === "function"
+						? fieldDef.initialValue()
+						: (fieldDef.initialValue ?? null),
+				])
+			)
+		);
+
 		describe("constructor", function() {
 			it("should construct an instance", function() {
 				let config = new TestConfig("local");
@@ -80,41 +120,37 @@ describe("lib/config/classes", function() {
 				const testInstance = new TestConfig("local", undefined, "filepath");
 				assert.equal(testInstance.filepath, "filepath");
 			});
+			it("should throw if a required field has no initialValue", function() {
+				this.skip(); // TODO known bug that was discovered but of of scope for #856
+				class BrokenConfig extends lib.Config {
+					static fieldDefinitions = {
+						"test.required": { type: "string" },
+					};
+				}
+
+				assert.throws(
+					() => new BrokenConfig("local"),
+				);
+			});
 		});
 
 		describe(".toJSON()", function() {
 			it("should serialize a basic config", function() {
-				let testInstance = new TestConfig("local");
-				assert.deepEqual(testInstance.toJSON(), {
-					"alpha.foo": null,
-					"beta.bar": {},
-					"test.enum": "b",
-					"test.test": null,
-					"test.func": 42,
-					"test.bool": false,
-					"test.json": {},
-					"test.rest": {},
-					"test.priv": null,
-					"test.cred": null,
-					"test.rdo": null,
-				});
+				const testInstance = new TestConfig("local");
+				assert.deepEqual(testInstance.toJSON(), testConfigDefault);
 			});
 		});
 
 		describe(".toRemote()", function() {
 			it("should leave out inaccessible fields", function() {
-				let testInstance = new TestConfig("local");
-				assert.deepEqual(testInstance.toRemote("remote"), {
-					"alpha.foo": null,
-					"beta.bar": {},
-					"test.enum": "b",
-					"test.test": null,
-					"test.func": 42,
-					"test.bool": false,
-					"test.json": {},
-					"test.rest": {},
-					"test.rdo": null,
-				});
+				const testInstance = new TestConfig("local");
+				const { "test.cred": _cred, "test.priv": _priv, ...expected } = testConfigDefault;
+				assert.deepEqual(testInstance.toRemote("remote"), expected);
+			});
+			it("should filter fields when filter argument is provided", function() {
+				const testInstance = new TestConfig("local");
+				const filtered = testInstance.toRemote("local", ["alpha.foo"]);
+				assert.deepEqual(filtered, { "alpha.foo": null });
 			});
 		});
 
@@ -165,6 +201,7 @@ describe("lib/config/classes", function() {
 
 			it("should preserve unknown fields when serialized back", function() {
 				let testFields = {
+					...testConfigDefault,
 					"extra.blah": true,
 					"extra.spam": "foobar",
 					"alpha.foo": "true",
@@ -175,10 +212,6 @@ describe("lib/config/classes", function() {
 					"test.bool": false,
 					"test.json": {},
 					"test.rest": {},
-					"test.priv": null,
-					"test.cred": null,
-					"test.rdo": null,
-					"test.alpha": null,
 					"test.beta": "decay",
 					"test.gamma": 99,
 				};
@@ -245,6 +278,7 @@ describe("lib/config/classes", function() {
 			});
 			it("should preserve unknown fields", async function() {
 				const testFields = {
+					...testConfigDefault,
 					"extra.blah": true,
 					"extra.spam": "foobar",
 					"alpha.foo": "true",
@@ -255,10 +289,6 @@ describe("lib/config/classes", function() {
 					"test.bool": false,
 					"test.json": {},
 					"test.rest": {},
-					"test.priv": null,
-					"test.cred": null,
-					"test.rdo": null,
-					"test.alpha": null,
 					"test.beta": "decay",
 					"test.gamma": 99,
 				};
@@ -347,19 +377,10 @@ describe("lib/config/classes", function() {
 					"beta.bar": { value: 30 },
 				}, false);
 				assert.deepEqual(testInstance.toJSON(), {
+					...testConfigDefault,
 					"extra.blah": true,
 					"extra.spam": "baz",
-					"alpha.foo": null,
 					"beta.bar": { value: 30 },
-					"test.bool": false,
-					"test.enum": "b",
-					"test.func": 42,
-					"test.json": {},
-					"test.rest": {},
-					"test.priv": null,
-					"test.cred": null,
-					"test.rdo": null,
-					"test.test": null,
 				});
 			});
 
@@ -709,6 +730,29 @@ describe("lib/config/classes", function() {
 				testInstance.set("test.enum", "b");
 				assert.deepEqual(testInstance.restartRequired, true, "restart required not set");
 			});
+			it("should run field validator successfully", function() {
+				testInstance.set("test.validated", 5);
+				assert.equal(testInstance.get("test.validated"), 5);
+			});
+
+			it("should throw if validator fails", function() {
+				assert.throws(
+					() => testInstance.set("test.validated", -1),
+					new lib.InvalidValue("Failed validation of test.validated: must be positive")
+				);
+			});
+			it("should validate dependent fields", function() {
+				testInstance.set("test.dependent", 10);
+				testInstance.set("test.base", 5);
+				assert.equal(testInstance.get("test.dependent"), 10);
+				assert.equal(testInstance.get("test.base"), 5);
+			});
+			it("should throw if dependent validation fails", function() {
+				assert.throws(
+					() => testInstance.set("test.base", 5),
+					new lib.InvalidValue("Failed validation of dependent test.dependent: dependent must be >= base")
+				);
+			});
 		});
 
 		describe(".setProp()", function() {
@@ -786,6 +830,130 @@ describe("lib/config/classes", function() {
 				testInstance.setProp("test.rest", "test", true);
 				assert.deepEqual(testInstance.restartRequired, true, "restart required not set");
 			});
+			it("should run validator when setting object property", function() {
+				testInstance.setProp("test.objValidated", "good", true);
+				assert.deepEqual(testInstance.get("test.objValidated"), { good: true });
+			});
+			it("should throw if object validator fails", function() {
+				assert.throws(
+					() => testInstance.setProp("test.objValidated", "bad", true),
+					new lib.InvalidValue("Failed validation of test.objValidated: bad key")
+				);
+			});
+		});
+
+		describe("get .staging", function() {
+			it("should create staging config and reuse it", function() {
+				const testInstance = new TestConfig("local");
+
+				const first = testInstance.staging;
+				const second = testInstance.staging;
+
+				assert.equal(first, second);
+			});
+			it("should return new instance if invalided", function() {
+				const testInstance = new TestConfig("local");
+
+				const first = testInstance.staging;
+				testInstance.set("alpha.foo", "value");
+				const second = testInstance.staging;
+
+				assert.notEqual(first, second);
+			});
+		});
+
+		describe(".stage()", function() {
+			let testInstance;
+			beforeEach(function() {
+				testInstance = new TestConfig("local");
+			});
+
+			it("should stage a value without committing", function() {
+				testInstance.stage("alpha.foo", "value");
+
+				assert.equal(testInstance.get("alpha.foo"), null);
+				assert.equal(testInstance.staging.get("alpha.foo"), "value");
+			});
+			it("should throw if field does not exist", function() {
+				assert.throws(
+					() => testInstance.stage("invalid", "value"),
+					new lib.InvalidField("No field named 'invalid'")
+				);
+			});
+		});
+
+		describe(".stageProp()", function() {
+			let testInstance;
+			beforeEach(function() {
+				testInstance = new TestConfig("local");
+			});
+
+			it("should stage object property change", function() {
+				testInstance.stageProp("test.json", "prop", true);
+
+				assert.deepEqual(testInstance.get("test.json"), {});
+				assert.deepEqual(testInstance.staging.get("test.json"), { prop: true });
+			});
+			it("should delete property when value undefined", function() {
+				testInstance.set("test.json", { a: 1 });
+
+				testInstance.stageProp("test.json", "a", undefined);
+
+				assert.deepEqual(testInstance.staging.get("test.json"), {});
+				assert.deepEqual(testInstance.get("test.json"), { a: 1 });
+			});
+			it("should throw if field is not object", function() {
+				assert.throws(
+					() => testInstance.stageProp("test.enum", "prop", true),
+					new lib.InvalidValue("Cannot set property on non-object field 'test.enum'")
+				);
+			});
+		});
+
+		describe(".commitStaging()", function() {
+			let testInstance;
+			beforeEach(function() {
+				testInstance = new TestConfig("local");
+			});
+
+			it("should do nothing if staging does not exist", function() {
+				testInstance.commitStaging();
+				assert.equal(testInstance.get("alpha.foo"), null);
+			});
+			it("should commit staged values", function() {
+				testInstance.stage("alpha.foo", "value");
+
+				assert.equal(testInstance.get("alpha.foo"), null);
+				testInstance.commitStaging();
+				assert.equal(testInstance.get("alpha.foo"), "value");
+			});
+			it("should throw if staged validation fails", function() {
+				testInstance.stage("test.validated", -1);
+
+				assert.throws(
+					() => testInstance.commitStaging(),
+					new lib.InvalidValue("Failed validation of test.validated: must be positive")
+				);
+			});
+			it("should commit stages values with dependent validation", function() {
+				assert.throws(() => testInstance.set("test.base", 10));
+				testInstance.stage("test.base", 10);
+				testInstance.stage("test.dependent", 15);
+				// The above ordering would error if set was used
+				testInstance.commitStaging();
+				assert.equal(testInstance.get("test.base"), 10);
+				assert.equal(testInstance.get("test.dependent"), 15);
+			});
+		});
+
+		describe(".revertStaging()", function() {
+			it("should discard staged changes", function() {
+				const testInstance = new TestConfig("local");
+				testInstance.stage("alpha.foo", "value");
+				testInstance.revertStaging();
+				assert.equal(testInstance.staging.get("alpha.foo"), null);
+				assert.equal(testInstance.get("alpha.foo"), null);
+			});
 		});
 	});
 });

test/lib/config/migrations.js

@@ -4,7 +4,7 @@ const assert = require("assert").strict;
 
 // Each migration test should indicate which version introduced the migration
 
-describe("lib/config/migrations", function() {
+describe("lib/config/definitions/migrations", function() {
 	describe("Controller Config", function() {
 		it("should migrate 'external_address' to 'public_url'", function() { // Alpha 19
 			const config = lib.ControllerConfig.fromJSON({

test/lib/config/validators.test.js

@@ -0,0 +1,245 @@
+"use strict";
+const assert = require("assert").strict;
+const lib = require("@clusterio/lib");
+const validators = lib.Config.validators;
+const { ControllerConfig, HostConfig, InstanceConfig, ControlConfig } = lib;
+
+describe("lib/config/validators", function() {
+	describe("all", function() {
+		it("should call all validators in order", function() {
+			let calls = [];
+
+			function v1(value, config) {
+				calls.push("v1");
+			}
+			function v2(value, config) {
+				calls.push("v2");
+			}
+
+			const combined = validators.all(v1, v2);
+			combined("value", {});
+
+			assert.deepEqual(calls, ["v1", "v2"], "validators were not called in order");
+		});
+
+		it("should propagate errors from validators", function() {
+			function v1() {}
+			function v2() {
+				throw new Error("failure");
+			}
+
+			const combined = validators.all(v1, v2);
+
+			assert.throws(
+				() => combined("value", {}),
+				/failure/,
+				"error from validator was not propagated"
+			);
+		});
+	});
+
+	describe("optional", function() {
+		it("should skip validation when value is null", function() {
+			let called = false;
+
+			function validator() {
+				called = true;
+			}
+
+			const wrapped = validators.optional(validator);
+			wrapped(null, {});
+
+			assert.equal(called, false, "validator should not have been called");
+		});
+
+		it("should run validator when value is not null", function() {
+			let called = false;
+
+			function validator() {
+				called = true;
+			}
+
+			const wrapped = validators.optional(validator);
+			wrapped("value", {});
+
+			assert.equal(called, true, "validator was not called");
+		});
+	});
+
+	describe("greaterThan", function() {
+		it("should allow values greater than min", function() {
+			const validator = validators.greaterThan(5);
+			assert.doesNotThrow(() => validator(6));
+		});
+
+		it("should throw if value equals min", function() {
+			const validator = validators.greaterThan(5);
+
+			assert.throws(
+				() => validator(5),
+				/Value must be greater than 5/,
+				"did not throw on equal value"
+			);
+		});
+
+		it("should throw if value is less than min", function() {
+			const validator = validators.greaterThan(5);
+
+			assert.throws(
+				() => validator(4),
+				/Value must be greater than 5/,
+				"did not throw on smaller value"
+			);
+		});
+	});
+
+	describe("greaterThanZero", function() {
+		it("should allow values greater than zero", function() {
+			assert.doesNotThrow(() => validators.greaterThanZero(1));
+		});
+
+		it("should throw on zero", function() {
+			assert.throws(
+				() => validators.greaterThanZero(0),
+				/Value must be greater than 0/
+			);
+		});
+
+		it("should throw on negative values", function() {
+			assert.throws(
+				() => validators.greaterThanZero(-1),
+				/Value must be greater than 0/
+			);
+		});
+	});
+
+	describe("greaterThanEqual", function() {
+		it("should allow values greater than min", function() {
+			const validator = validators.greaterThanEqual(5);
+			assert.doesNotThrow(() => validator(6));
+		});
+
+		it("should allow values equal to min", function() {
+			const validator = validators.greaterThanEqual(5);
+			assert.doesNotThrow(() => validator(5));
+		});
+
+		it("should throw if value is less than min", function() {
+			const validator = validators.greaterThanEqual(5);
+
+			assert.throws(
+				() => validator(4),
+				/Value must be greater than 5/,
+				"did not throw on smaller value"
+			);
+		});
+	});
+
+	describe("greaterThanEqualZero", function() {
+		it("should allow zero", function() {
+			assert.doesNotThrow(() => validators.greaterThanEqualZero(0));
+		});
+
+		it("should allow positive values", function() {
+			assert.doesNotThrow(() => validators.greaterThanEqualZero(1));
+		});
+
+		it("should throw on negative values", function() {
+			assert.throws(
+				() => validators.greaterThanEqualZero(-1),
+				/Value must be greater than 0/
+			);
+		});
+	});
+
+	describe("integer", function() {
+		it("should allow integer values", function() {
+			assert.doesNotThrow(() => validators.integer(10));
+		});
+
+		it("should throw on non-integer values", function() {
+			assert.throws(
+				() => validators.integer(1.5),
+				/Value must be an integer/,
+				"did not throw on float"
+			);
+		});
+	});
+
+	describe("filePath", function() {
+		it("should accept valid paths", function() {
+			assert.doesNotThrow(() => validators.filePath("test/file.txt"));
+			assert.doesNotThrow(() => validators.filePath("/absolute/path"));
+		});
+
+		it("should throw if value is not a string", function() {
+			// This is the only case that will cause path.resolve to throw on posix
+			assert.throws(
+				() => validators.filePath(null),
+				/Value must be a valid path/
+			);
+		});
+	});
+});
+
+describe("lib/config/definitions/validators", function() {
+	describe("Controller Config", function() {
+
+	});
+
+	describe("Host Config", function() {
+
+	});
+
+	describe("Instance Config", function() {
+		it("should validate factorio.version", function() {
+			const config = new InstanceConfig("controller");
+
+			// valid cases
+			assert.doesNotThrow(() => config.set("factorio.version", "latest"));
+			assert.doesNotThrow(() => config.set("factorio.version", "1.1"));
+			assert.doesNotThrow(() => config.set("factorio.version", "1.1.87"));
+
+			// invalid case
+			assert.throws(
+				() => config.set("factorio.version", "invalidValue"),
+				/Value must be be 'latest', or match X.Y, or match X.Y.Z/
+			);
+		});
+
+		it("should validate factorio.player_online_autosave_slots against autosave slots", function() {
+			const config = new InstanceConfig("controller", {
+				"factorio.settings": {
+					autosave_slots: 5,
+				},
+			});
+
+			// valid: equal
+			assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 5));
+
+			// valid: greater
+			assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 6));
+
+			// invalid: less than autosave_slots
+			assert.throws(
+				() => config.set("factorio.player_online_autosave_slots", 4),
+				/Value cannot be less than the number of autosave slots/
+			);
+		});
+
+		it("should allow player_online_autosave_slots when autosave_slots is not a number", function() {
+			const config = new InstanceConfig("controller", {
+				"factorio.settings": {
+					autosave_slots: null,
+				},
+			});
+
+			assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 0));
+			assert.doesNotThrow(() => config.set("factorio.player_online_autosave_slots", 1));
+		});
+	});
+
+	describe("Control Config", function() {
+
+	});
+});

test/mock.js

@@ -195,6 +195,10 @@ class MockController {
 		this.handles = new Map();
 	}
 
+	get authSecret() {
+		return Buffer.from(this.config.get("controller.auth_secret"), "base64");
+	}
+
 	handle(eventClass, handler) {
 		this.handles.set(eventClass, handler);
 	}
@@ -237,16 +241,15 @@ async function createInstancePlugin(InstancePluginClass, info) {
 	return plugin;
 }
 
-
 module.exports = {
-	MockLogger,
-	MockSocket,
-	MockConnector,
-	MockServer,
-	MockInstance,
-	MockHost,
-	MockControl,
-	MockController,
+	MockLogger: MockLogger,
+	MockSocket: MockSocket,
+	MockConnector: MockConnector,
+	MockServer: MockServer,
+	MockInstance: MockInstance,
+	MockHost: MockHost,
+	MockControl: MockControl,
+	MockController: MockController,
 
 	createControllerPlugin,
 	createInstancePlugin,