From d7fc3aaec4fce86fa01e556461cd2d6f7775fa2e Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:53:13 +0000 Subject: [PATCH 1/4] Replace bootstrap methods with async factories Controller.bootstrap and Host.bootstrap returned constructor arguments that the caller had to spread into new Controller/Host, and DatastoreProvider.bootstrap did the same for datastores. They are replaced by Controller.fromConfig, Host.fromConfig and KeyValueDatastore/SubscribableDatastore.fromProvider, which load the data and return the finished instance. Closes #1126 Co-Authored-By: Claude Opus 5.5 --- packages/controller/controller.ts | 11 +++--- packages/controller/src/Controller.ts | 50 +++++++++++++++++---------- packages/host/host.ts | 3 +- packages/host/src/Host.ts | 14 +++++--- packages/lib/src/datastore.ts | 15 +++++--- test/controller/Controller.js | 32 +++++++++++++++++ test/host/Host.test.js | 15 ++++++++ test/lib/datastore.js | 50 ++++++++++++++++----------- 8 files changed, 133 insertions(+), 57 deletions(-) diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index a2efeb12..bc57c1db 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -147,15 +147,15 @@ async function handleBootstrapCommand( path.join(databaseDirectory, "roles.json"), ); - const roles = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const roles = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "roles.json"), lib.Role.fromJSON.bind(lib.Role), - ).bootstrap()); + )); - const users = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const users = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "users.json"), UserRecord.fromJSON.bind(UserRecord), - ).bootstrap()); + )); const userManager = new UserManager(users, roles, controllerConfig); @@ -474,13 +474,12 @@ async function startup() { return; } - controller = new Controller( + controller = await Controller.fromConfig( clusterLogger, pluginInfos, controllerConfig, Boolean(args.canRestart), Boolean(args.recovery), - ...await Controller.bootstrap(controllerConfig) ); // Refuse to start if there are no users loaded diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 84404f28..45a16a00 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -148,7 +148,14 @@ export default class Controller { } }); - static async bootstrap(config: lib.ControllerConfig) { + /** Create a controller with its datastores and mod store loaded from disk */ + static async fromConfig( + clusterLogger: winston.Logger, + pluginInfos: lib.PluginNodeEnvInfo[], + config: lib.ControllerConfig, + canRestart = false, + recoveryMode = false, + ) { let databaseDirectory = config.get("controller.database_directory"); await fs.mkdir(databaseDirectory, { recursive: true }); @@ -158,23 +165,23 @@ export default class Controller { path.join(databaseDirectory, "roles.json"), ); - const systems = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const systems = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "systems.json"), lib.SystemInfo.fromJSON.bind(lib.SystemInfo), this.migrateSystems, - ).bootstrap()); + )); - const hosts = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const hosts = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "hosts.json"), HostRecord.fromJSON.bind(HostRecord), this.migrateHosts, this.finaliseHosts, - ).bootstrap()); + )); - const instances = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const instances = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "instances.json"), InstanceRecord.fromJSON.bind(InstanceRecord), this.migrateInstances, this.finaliseInstances, - ).bootstrap()); + )); // Save plugin versions written while loading for (const instance of instances.values()) { if (instance.config.dirty) { @@ -182,33 +189,33 @@ export default class Controller { } } - const saves = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const saves = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "saves.json"), lib.SaveDetails.fromJSON.bind(lib.SaveDetails) - ).bootstrap()); + )); - const modPacks = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const modPacks = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "mod-packs.json"), lib.ModPack.fromJSON.bind(lib.ModPack), this.migrateModPacks, - ).bootstrap()); + )); - const roles = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const roles = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "roles.json"), lib.Role.fromJSON.bind(lib.Role), this.migrateRoles, - ).bootstrap()); + )); - const users = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const users = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "users.json"), UserRecord.fromJSON.bind(UserRecord), undefined, this.finaliseUsers, - ).bootstrap()); + )); - const notes = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( + const notes = await lib.SubscribableDatastore.fromProvider(new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "notes.json"), lib.Note.fromJSON.bind(lib.Note), - ).bootstrap()); + )); let modsDirectory = config.get("controller.mods_directory"); await fs.mkdir(modsDirectory, { recursive: true }); @@ -223,7 +230,12 @@ export default class Controller { lib.Role.ensureDefaultAdminRole(roles); lib.Role.ensureDefaultPlayerRole(roles); - return [ + return new this( + clusterLogger, + pluginInfos, + config, + canRestart, + recoveryMode, systems, hosts, instances, @@ -233,7 +245,7 @@ export default class Controller { roles, users, notes, - ] as const; + ); } constructor( diff --git a/packages/host/host.ts b/packages/host/host.ts index da2ae862..566232ca 100755 --- a/packages/host/host.ts +++ b/packages/host/host.ts @@ -233,13 +233,12 @@ async function startHost() { } let hostConnector = new HostConnector(hostConfig, pluginInfos); - host = new Host( + host = await Host.fromConfig( hostConnector, hostConfig, pluginInfos, Boolean(args.canRestart), Boolean(args.recovery), - ...await Host.bootstrap(hostConfig) ); // Handle interrupts diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index f5ce4ad3..8d89c75a 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -329,12 +329,18 @@ export default class Host extends lib.Link { "_warning": "Changes to this file will be overwritten by the controller's copy.", }; - static async bootstrap(hostConfig: lib.HostConfig) { + /** Create a host with its mod store loaded from disk */ + static async fromConfig( + connector: HostConnector, + hostConfig: lib.HostConfig, + pluginInfos: lib.PluginNodeEnvInfo[], + canRestart = false, + recoveryMode = false, + ) { const modsDirectory = hostConfig.get("host.mods_directory"); await fs.mkdir(modsDirectory, { recursive: true }); - return [ - await lib.ModStore.fromDirectory(modsDirectory), - ] as const; + const modStore = await lib.ModStore.fromDirectory(modsDirectory); + return new this(connector, hostConfig, pluginInfos, canRestart, recoveryMode, modStore); } constructor( diff --git a/packages/lib/src/datastore.ts b/packages/lib/src/datastore.ts index 23c294e0..c5f58282 100644 --- a/packages/lib/src/datastore.ts +++ b/packages/lib/src/datastore.ts @@ -17,11 +17,6 @@ export abstract class DatastoreProvider< > { abstract save(data: Map): Promise abstract load(): Promise> - - // Load data from the source and return it, this should be passed to the constructor of a datastore - async bootstrap() { - return [this, await this.load()] as const; - } } // Store all data in memory, used as a default and should be avoided @@ -251,6 +246,11 @@ export class KeyValueDatastore< K extends DatastoreKey, V extends DatastoreValue, > extends Datastore { + // Create a datastore populated with the data loaded from the provider + static async fromProvider(provider: DatastoreProvider) { + return new this(provider, await provider.load()); + } + // Set the value in the datastore, be careful of race conditions if you await any functions before calling set set(key: K, value: V) { this.data.set(key, value); @@ -316,6 +316,11 @@ export class SubscribableDatastore< this.lastUpdatedAtMs = SubscribableDatastore.maxUpdatedAtMs(this.data.values()); } + // Create a datastore populated with the data loaded from the provider + static async fromProvider(provider: DatastoreProvider) { + return new this(provider, await provider.load()); + } + async load() { await super.load(); this.lastUpdatedAtMs = SubscribableDatastore.maxUpdatedAtMs(this.data.values()); diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 7294ba94..76eec466 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -57,6 +57,38 @@ describe("controller/src/Controller", function() { assert(list.check(ip), `${ip} missing from list`); } } + describe(".fromConfig()", function() { + let dir; + beforeEach(async function() { + dir = await fs.mkdtemp(path.join(os.tmpdir(), "clusterio-controller-")); + }); + afterEach(async function() { + await fs.rm(dir, { recursive: true, force: true }); + }); + it("should load datastores and mods from disk", async function() { + const databaseDirectory = path.join(dir, "database"); + const modsDirectory = path.join(dir, "mods"); + await fs.mkdir(databaseDirectory); + await fs.writeFile(path.join(databaseDirectory, "notes.json"), JSON.stringify([ + { resourceType: "instance", resourceId: "100", content: "hello" }, + ])); + const config = new ControllerConfig("controller", { + "controller.version": controllerVersion, + "controller.database_directory": databaseDirectory, + "controller.mods_directory": modsDirectory, + }); + + const loaded = await Controller.fromConfig({}, [], config, true, false); + assert(loaded instanceof Controller); + assert.equal(loaded.canRestart, true); + assert.equal(loaded.recoveryMode, false); + assert.equal(loaded.notes.get("instance/100").content, "hello"); + assert.equal(loaded.modStore.modsDirectory, modsDirectory); + assert(loaded.roles.size > 0, "default roles were not added"); + assert(loaded.modPacks.size > 0, "default mod packs were not added"); + await fs.access(modsDirectory); + }); + }); describe(".loadJsonObject()", function() { let dir; before(async function() { diff --git a/test/host/Host.test.js b/test/host/Host.test.js index e442bc76..9bcfb32c 100644 --- a/test/host/Host.test.js +++ b/test/host/Host.test.js @@ -62,6 +62,21 @@ describe("host/src/Host", function() { }); }); describe("class Host", function() { + describe(".fromConfig()", function() { + const modsDirectory = path.join("temp", "test", "host_from_config", "mods"); + beforeEach(async function() { + await fs.rm(path.dirname(modsDirectory), { recursive: true, force: true }); + }); + it("should create the mods directory and load the mod store", async function() { + const hostConfig = new lib.HostConfig("host", { "host.mods_directory": modsDirectory }); + const host = await Host.fromConfig(new HostConnector(hostConfig, []), hostConfig, [], true, false); + assert(host instanceof Host); + assert.equal(host.canRestart, true); + assert.equal(host.recoveryMode, false); + assert.equal(host.modStore.modsDirectory, modsDirectory); + await fs.access(modsDirectory); + }); + }); describe(".handleSyncUserListsEvent()", function() { const hostAddress = lib.Address.fromShorthand({ hostId: 1 }); const instanceAddress = lib.Address.fromShorthand({ instanceId: 1 }); diff --git a/test/lib/datastore.js b/test/lib/datastore.js index 322450aa..4c829e76 100644 --- a/test/lib/datastore.js +++ b/test/lib/datastore.js @@ -50,13 +50,6 @@ describe("lib/datastore", function() { it("can be constructed", function() { assert.deepEqual(datastoreProvider.methodCalls[0], ["constructor"]); }); - - it("can be bootstrapped", async function() { - const rtn = await datastoreProvider.bootstrap(); - assert.equal(rtn[0], datastoreProvider); - assert.equal(rtn[1], datastoreProvider.value); - assert.deepEqual(datastoreProvider.methodCalls[1], ["load"]); - }); }); describe("class MemoryDatastoreProvider", function() { @@ -70,13 +63,13 @@ describe("lib/datastore", function() { describe("constructor", function() { it("has a default value", async function() { - const [, value] = await new lib.MemoryDatastoreProvider().bootstrap(); + const value = await new lib.MemoryDatastoreProvider().load(); assert.notEqual(value, undefined); assert.notEqual(value, null); }); it("can have a value given", async function() { const expectedValue = new Map([["foo", "bar"]]); - const [, value] = await new lib.MemoryDatastoreProvider(expectedValue).bootstrap(); + const value = await new lib.MemoryDatastoreProvider(expectedValue).load(); assert.deepEqual(value, expectedValue); }); }); @@ -116,7 +109,7 @@ describe("lib/datastore", function() { it("has default values", async function() { datastoreProvider = new lib.JsonDatastoreProvider(dataPath); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData)); }); it("can accept a fromJson method", async function() { @@ -125,7 +118,7 @@ describe("lib/datastore", function() { json => [json] ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData.map(([k, v]) => [k, [v]]))); }); it("can accept a migration method", async function() { @@ -135,7 +128,7 @@ describe("lib/datastore", function() { rawJson => ({ c: "foo", ...rawJson }) ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map([...initialData, ["c", "foo"]])); }); it("can accept a finalise method", async function() { @@ -146,7 +139,7 @@ describe("lib/datastore", function() { obj => [obj] ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData.map(([k, v]) => [k, [v]]))); }); }); @@ -210,7 +203,7 @@ describe("lib/datastore", function() { it("has default values", async function() { datastoreProvider = new lib.JsonIdDatastoreProvider(dataPath); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData)); }); it("can accept a fromJson method", async function() { @@ -219,7 +212,7 @@ describe("lib/datastore", function() { json => ({...json, foo: true}) ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData.map(v => ([v[0], {...v[1], foo: true}])))); }); it("can accept a migration method", async function() { @@ -229,7 +222,7 @@ describe("lib/datastore", function() { rawJson => ([{ id: "c" }, ...rawJson]) ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map([...initialData, ["c", { id: "c" }]])); }); it("can accept a finalise method", async function() { @@ -240,7 +233,7 @@ describe("lib/datastore", function() { obj => ({...obj, foo: true}) ); - const [, value] = await datastoreProvider.bootstrap(); + const value = await datastoreProvider.load(); assert.deepEqual(value, new Map(initialData.map(v => ([v[0], {...v[1], foo: true}])))); }); }); @@ -306,10 +299,6 @@ describe("lib/datastore", function() { datastore = new MockDatastore(datastoreProvider, datastoreProvider.value); assert.deepEqual(datastore.getData(), datastoreProvider.value); }); - it("accepts the return of bootstrap", async function() { - datastore = new MockDatastore(...await datastoreProvider.bootstrap()); - assert.deepEqual(datastore.getData(), datastoreProvider.value); - }); }); describe("getFilePath", function() { @@ -478,6 +467,15 @@ describe("lib/datastore", function() { datastore.on("update", updates => eventsRaised.push(updates)); }); + describe("fromProvider", function() { + it("loads data from the provider", async function() { + datastore = await lib.KeyValueDatastore.fromProvider(datastoreProvider); + assert(datastore instanceof lib.KeyValueDatastore); + assert.equal(datastore.get("foo"), "fooValue"); + assert.deepEqual(datastoreProvider.methodCalls[1], ["load"]); + }); + }); + describe("set", function() { it("can set a value", function() { datastore.set("test", "testValue"); @@ -571,6 +569,16 @@ describe("lib/datastore", function() { datastore.on("update", updates => eventsRaised.push(updates)); }); + describe("fromProvider", function() { + it("loads data from the provider", async function() { + datastore = await lib.SubscribableDatastore.fromProvider(datastoreProvider); + assert(datastore instanceof lib.SubscribableDatastore); + assert.equal(datastore.get("foo").id, "foo"); + assert.equal(datastore.lastUpdatedAtMs, 10); + assert.deepEqual(datastoreProvider.methodCalls[1], ["load"]); + }); + }); + function forceFuture() { const future = {id: "future"}; for (let i = 1; i < 100; i++) { From ee097bfa6f02674ab572061526437052874b27a0 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:08:41 +0000 Subject: [PATCH 2/4] Test that fromConfig saves instances changed while loading Co-Authored-By: Claude Opus 5.5 --- test/controller/Controller.js | 40 +++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 76eec466..8a8b5edf 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -14,6 +14,7 @@ import { addPluginConfigFields, renamePermissionPrefix, } from "@clusterio/lib"; +import * as lib from "@clusterio/lib"; import "../setup_logging.js"; class MockEvent {} @@ -88,6 +89,45 @@ describe("controller/src/Controller", function() { assert(loaded.modPacks.size > 0, "default mod packs were not added"); await fs.access(modsDirectory); }); + describe("with a plugin version change", function() { + before(function() { + addPluginConfigFields([{ name: "from_config_plugin", version: "1.2.0", instanceEntrypoint: "x" }]); + }); + afterEach(function() { + lib.Config.pluginVersions.clear(); + }); + after(function() { + for (const Config of [lib.ControllerConfig, lib.HostConfig, lib.InstanceConfig]) { + for (const name of Object.keys(Config.fieldDefinitions)) { + if (name.startsWith("from_config_plugin.")) { + delete Config.fieldDefinitions[name]; + } + } + } + }); + it("should save instances whose config changed while loading", async function() { + const databaseDirectory = path.join(dir, "database"); + const instancesPath = path.join(databaseDirectory, "instances.json"); + await fs.mkdir(databaseDirectory); + const instanceConfig = new InstanceConfig("controller", { + "instance.id": 7, "from_config_plugin.version": "1.0.0", + }); + await fs.writeFile(instancesPath, JSON.stringify([ + { config: instanceConfig.toJSON(), status: "unassigned" }, + ])); + lib.Config.pluginVersions.set("from_config_plugin", "1.2.0"); + const config = new ControllerConfig("controller", { + "controller.version": controllerVersion, + "controller.database_directory": databaseDirectory, + "controller.mods_directory": path.join(dir, "mods"), + }); + + const loaded = await Controller.fromConfig({}, [], config); + await loaded.instances.records.save(); + const [saved] = JSON.parse(await fs.readFile(instancesPath, "utf8")); + assert.equal(saved.config["from_config_plugin.version"], "1.2.0"); + }); + }); }); describe(".loadJsonObject()", function() { let dir; From 3a81cfd94bbf45a25fbb133c8aae9849d4aaf6de Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:05:32 +0000 Subject: [PATCH 3/4] Order fromConfig arguments config first Controller.fromConfig and Host.fromConfig now take the config, then the plugin infos, then the logger or connector. Co-Authored-By: Claude Opus 5.5 --- packages/controller/controller.ts | 4 ++-- packages/controller/src/Controller.ts | 4 ++-- packages/host/host.ts | 2 +- packages/host/src/Host.ts | 2 +- test/controller/Controller.js | 4 ++-- test/host/Host.test.js | 2 +- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index bc57c1db..3c95468a 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -475,9 +475,9 @@ async function startup() { } controller = await Controller.fromConfig( - clusterLogger, - pluginInfos, controllerConfig, + pluginInfos, + clusterLogger, Boolean(args.canRestart), Boolean(args.recovery), ); diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 45a16a00..d0a12720 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -150,9 +150,9 @@ export default class Controller { /** Create a controller with its datastores and mod store loaded from disk */ static async fromConfig( - clusterLogger: winston.Logger, - pluginInfos: lib.PluginNodeEnvInfo[], config: lib.ControllerConfig, + pluginInfos: lib.PluginNodeEnvInfo[], + clusterLogger: winston.Logger, canRestart = false, recoveryMode = false, ) { diff --git a/packages/host/host.ts b/packages/host/host.ts index 566232ca..db2a9357 100755 --- a/packages/host/host.ts +++ b/packages/host/host.ts @@ -234,9 +234,9 @@ async function startHost() { let hostConnector = new HostConnector(hostConfig, pluginInfos); host = await Host.fromConfig( - hostConnector, hostConfig, pluginInfos, + hostConnector, Boolean(args.canRestart), Boolean(args.recovery), ); diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index 8d89c75a..c90b0c5e 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -331,9 +331,9 @@ export default class Host extends lib.Link { /** Create a host with its mod store loaded from disk */ static async fromConfig( - connector: HostConnector, hostConfig: lib.HostConfig, pluginInfos: lib.PluginNodeEnvInfo[], + connector: HostConnector, canRestart = false, recoveryMode = false, ) { diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 8a8b5edf..35a0cf4b 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -79,7 +79,7 @@ describe("controller/src/Controller", function() { "controller.mods_directory": modsDirectory, }); - const loaded = await Controller.fromConfig({}, [], config, true, false); + const loaded = await Controller.fromConfig(config, [], {}, true, false); assert(loaded instanceof Controller); assert.equal(loaded.canRestart, true); assert.equal(loaded.recoveryMode, false); @@ -122,7 +122,7 @@ describe("controller/src/Controller", function() { "controller.mods_directory": path.join(dir, "mods"), }); - const loaded = await Controller.fromConfig({}, [], config); + const loaded = await Controller.fromConfig(config, [], {}); await loaded.instances.records.save(); const [saved] = JSON.parse(await fs.readFile(instancesPath, "utf8")); assert.equal(saved.config["from_config_plugin.version"], "1.2.0"); diff --git a/test/host/Host.test.js b/test/host/Host.test.js index 9bcfb32c..c8a96a8f 100644 --- a/test/host/Host.test.js +++ b/test/host/Host.test.js @@ -69,7 +69,7 @@ describe("host/src/Host", function() { }); it("should create the mods directory and load the mod store", async function() { const hostConfig = new lib.HostConfig("host", { "host.mods_directory": modsDirectory }); - const host = await Host.fromConfig(new HostConnector(hostConfig, []), hostConfig, [], true, false); + const host = await Host.fromConfig(hostConfig, [], new HostConnector(hostConfig, []), true, false); assert(host instanceof Host); assert.equal(host.canRestart, true); assert.equal(host.recoveryMode, false); From abb04f32586cb234a25576ff2549bdafa3be5b8e Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Sat, 24 Oct 2026 00:11:22 +0000 Subject: [PATCH 4/4] Create FactorioServer and Instance through async factories FactorioServer.fromOptions finds the Factorio install and generates the RCON password before constructing, so _version and _dataDir are never null and setTargetVersion and init are gone. checkForUpdates is now a module function called before fromOptions. Instance.fromConfig resolves the version, downloads Factorio if needed and builds the server, then constructs the instance with it. It runs before the instance is connected, so the starting and stopped statuses and the version requests go through the host link. FactorioVersionsRequest and LatestReleasesRequest now accept host as a source, and the host tracks starting instances so a second start during a download is rejected. Plugin loading stays a separate loadPlugins step after the instance is connected. Co-Authored-By: Claude Opus 5.5 --- packages/host/src/Host.ts | 30 ++- packages/host/src/Instance.ts | 131 ++++++++----- packages/host/src/server.ts | 192 ++++++++++--------- packages/lib/src/data/messages_controller.ts | 4 +- test/host/Host.test.js | 23 +++ test/host/Instance.js | 77 +++++++- test/host/export.js | 3 +- test/host/server.js | 103 +++++----- test/host/server_stop.js | 5 +- test/integration/export.js | 3 +- test/integration/routing.js | 3 +- test/integration/server.js | 11 +- 12 files changed, 364 insertions(+), 221 deletions(-) diff --git a/packages/host/src/Host.ts b/packages/host/src/Host.ts index c90b0c5e..c0757f83 100644 --- a/packages/host/src/Host.ts +++ b/packages/host/src/Host.ts @@ -322,6 +322,8 @@ export default class Host extends lib.Link { _startup = true; _disconnecting = false; _shuttingDown = false; + /** Instances waiting on {@link Instance.fromConfig} */ + _startingInstances = new Set(); startedAtMs = Date.now(); @@ -764,7 +766,7 @@ export default class Host extends lib.Link { this.send( new lib.InstanceStatusChangedEvent( instanceId, - instanceConnection ? instanceConnection.instance.status : "stopped", + instanceConnection ? instanceConnection.instance.status : this._unconnectedStatus(instanceId), instanceConnection ? instanceConnection.instance.server.gamePort : this.gamePort(instanceId), instanceConnection ? instanceConnection.instance.server.version @@ -880,6 +882,11 @@ export default class Host extends lib.Link { } } + /** Status of an instance without a connection */ + _unconnectedStatus(instanceId: number) { + return this._startingInstances.has(instanceId) ? "starting" as const : "stopped" as const; + } + /** * Initialize and connect an unloaded instance * @@ -888,22 +895,27 @@ export default class Host extends lib.Link { */ async _connectInstance(instanceId: number) { let unloadedInstance = this.getRequestInstance(instanceId); - if (this.instanceConnections.has(instanceId)) { + if (this.instanceConnections.has(instanceId) || this._startingInstances.has(instanceId)) { throw new lib.RequestError(`Instance with ID ${instanceId} is running`); } let hostAddress = new lib.Address(lib.Address.host, this.config.get("host.id")); let instanceAddress = new lib.Address(lib.Address.instance, instanceId); let [connectionClient, connectionServer] = lib.VirtualConnector.makePair(instanceAddress, hostAddress); - let instance = new Instance( - this, connectionClient, - unloadedInstance.path, this.config.get("host.factorio_directory"), - unloadedInstance.config - ); + let instance: Instance; + this._startingInstances.add(instanceId); + try { + instance = await Instance.fromConfig( + unloadedInstance.config, this, connectionClient, + unloadedInstance.path, this.config.get("host.factorio_directory"), + ); + } finally { + this._startingInstances.delete(instanceId); + } let instanceConnection = new InstanceConnection(connectionServer, this, instance); this.instanceConnections.set(instanceId, instanceConnection); - await instance.init(this.pluginInfos); + await instance.loadPlugins(this.pluginInfos); return instanceConnection; } @@ -1259,7 +1271,7 @@ export default class Host extends lib.Link { let instanceConnection = this.instanceConnections.get(instanceId); list.push(new lib.HostInstanceUpdate( instance.config.toRemote("controller"), - instanceConnection ? instanceConnection.instance.status : "stopped", + instanceConnection ? instanceConnection.instance.status : this._unconnectedStatus(instanceId), instanceConnection ? instanceConnection.instance.server.gamePort : this.gamePort(instanceId), instanceConnection ? instanceConnection.instance.server.version : undefined, )); diff --git a/packages/host/src/Instance.ts b/packages/host/src/Instance.ts index 4402c268..bda1d0c4 100644 --- a/packages/host/src/Instance.ts +++ b/packages/host/src/Instance.ts @@ -10,7 +10,7 @@ const execAsync = util.promisify(exec); // internal libraries import * as lib from "@clusterio/lib"; -import { FactorioServer } from "./server.js"; +import { FactorioServer, checkForUpdates } from "./server.js"; import { loadModules, patch } from "./patch.js"; import { exportData } from "./export.js"; import type Host from "./Host.js"; @@ -32,6 +32,13 @@ const scriptCommands = [ "/silent-command", "/sc", ]; +function instanceLogger(config: lib.InstanceConfig) { + return lib.logger.child({ + instance_id: config.get("instance.id"), + instance_name: config.get("instance.name"), + }); +} + // Statuses the instance passes through on its way to running or stopped. const transitoryStatuses: ReadonlySet = new Set([ "starting", "stopping", "creating_save", "exporting_data", @@ -191,8 +198,8 @@ export default class Instance extends lib.Link { host: Host, connector: lib.VirtualConnector, dir: string, - factorioDir: string, - instanceConfig: lib.InstanceConfig + instanceConfig: lib.InstanceConfig, + server: FactorioServer, ) { super(connector); this._host = host; @@ -202,10 +209,7 @@ export default class Instance extends lib.Link { this.id = this.config.get("instance.id"); - this.logger = lib.logger.child({ - instance_id: this.id, - instance_name: this.name, - }); + this.logger = instanceLogger(instanceConfig); this.hooks = new InstanceHooks(this.logger); @@ -225,28 +229,7 @@ export default class Instance extends lib.Link { }; this.config.on("fieldChanged", this._configFieldChanged); - let serverOptions = { - logger: this.logger, - // TODO: factorio.version is not validated - version: this.config.get("factorio.version") as lib.TargetVersion, - executablePath: this.config.get("factorio.executable_path") ?? undefined, - gamePort: this.config.get("factorio.game_port") ?? host.assignGamePort(this.id), - rconPort: this.config.get("factorio.rcon_port") ?? undefined, - rconPassword: this.config.get("factorio.rcon_password") ?? undefined, - enableLuaUdp: this.config.get("factorio.enable_lua_udp"), - luaUdpPort: this.config.get("factorio.lua_udp_port") ?? undefined, - enableWhitelist: this.config.get("factorio.enable_whitelist"), - enableAuthserverBans: this.config.get("factorio.enable_authserver_bans"), - verboseLogging: this.config.get("factorio.verbose_logging"), - consoleLogging: this.config.get("factorio.console_logging"), - stripPaths: this.config.get("factorio.strip_paths"), - maxConcurrentCommands: this.config.get("factorio.max_concurrent_commands"), - shutdownTimeoutMs: this.config.get("factorio.shutdown_timeout") * 1000, - }; - - this.server = new FactorioServer( - factorioDir, this._dir, serverOptions - ); + this.server = server; this.server.on("output", (parsed, line) => { this.logger.log("server", { message: line, instance_id: this.id, parsed }); @@ -670,20 +653,22 @@ end`.replace(/\r?\n/g, " "); this._saveStats().catch(err => this.logger.error(`Error saving stats:\n${err.stack}`)); } - async _loadStats() { + static async _readStats(statsPath: string) { let instanceStats; try { - instanceStats = JSON.parse(await fs.readFile(this.path("instance-stats.json"), "utf8")); + instanceStats = JSON.parse(await fs.readFile(statsPath, "utf8")); } catch (err: any) { if (err.code === "ENOENT") { - return; + return { playerStats: new Map(), playerAutosaveSlot: 1 }; } throw err; } - this.playerStats = new Map(instanceStats["players"].map( - ([id, stats]: [number, Static]) => [id, new lib.PlayerStats(stats)]) - ); - this._playerAutosaveSlot = instanceStats["player_autosave_slot"] || 1; + return { + playerStats: new Map(instanceStats["players"].map( + ([id, stats]: [string, Static]) => [id, new lib.PlayerStats(stats)] + )), + playerAutosaveSlot: (instanceStats["player_autosave_slot"] as number) || 1, + }; } async _saveStats() { @@ -694,21 +679,35 @@ end`.replace(/\r?\n/g, " "); await lib.safeOutputFile(this.path("instance-stats.json"), content); } - async init(pluginInfos: lib.PluginNodeEnvInfo[]) { - // After changing the status this function is responsible for calling notifyExit if it throws. - this.notifyStatus("starting"); + /** + * Create an instance with a Factorio server, downloading Factorio if needed + * + * The host reports the starting status for the instance while this runs. + */ + static async fromConfig( + config: lib.InstanceConfig, + host: Host, + connector: lib.VirtualConnector, + dir: string, + factorioDir: string, + ) { + const id = config.get("instance.id"); + // TODO: factorio.version is not validated + const targetVersion = config.get("factorio.version") as lib.TargetVersion; + const gamePort = config.get("factorio.game_port") ?? await host.assignFreeGamePort(id); + host.send(new lib.InstanceStatusChangedEvent(id, "starting", gamePort, targetVersion)); + + let server: FactorioServer; + let stats: Awaited>; try { - if (this.config.get("factorio.game_port") === null) { - this.server.gamePort = await this._host.assignFreeGamePort(this.id); - } - const factorioVersions = await this.sendTo("controller", new lib.FactorioVersionsRequest()); - await this._loadStats(); + const factorioVersions = await host.sendTo("controller", new lib.FactorioVersionsRequest()); + stats = await Instance._readStats(path.join(dir, "instance-stats.json")); // Resolve a release channel target (e.g. "stable") to a concrete // version using the controller's cached latest-releases data, so the // existing download/lookup logic can fetch and run it. - const targetVersion = this.config.get("factorio.version") as string; + let version = targetVersion; if (lib.isReleaseChannel(targetVersion)) { - const releases = await this.sendTo("controller", new lib.LatestReleasesRequest()); + const releases = await host.sendTo("controller", new lib.LatestReleasesRequest()); const resolved = lib.resolveReleaseChannel(releases, targetVersion); if (!resolved) { throw new Error( @@ -716,16 +715,46 @@ end`.replace(/\r?\n/g, " "); "latest-releases unavailable and no cached copy on the controller." ); } - this.server.setTargetVersion(resolved); + version = resolved; } - await this.server.checkForUpdates(factorioVersions); - await this.server.init(); + const logger = instanceLogger(config); + await checkForUpdates(factorioDir, version, factorioVersions, logger); + server = await FactorioServer.fromOptions(factorioDir, dir, { + logger, + version, + executablePath: config.get("factorio.executable_path") ?? undefined, + gamePort, + rconPort: config.get("factorio.rcon_port") ?? undefined, + rconPassword: config.get("factorio.rcon_password") ?? undefined, + enableLuaUdp: config.get("factorio.enable_lua_udp"), + luaUdpPort: config.get("factorio.lua_udp_port") ?? undefined, + enableWhitelist: config.get("factorio.enable_whitelist"), + enableAuthserverBans: config.get("factorio.enable_authserver_bans"), + verboseLogging: config.get("factorio.verbose_logging"), + consoleLogging: config.get("factorio.console_logging"), + stripPaths: config.get("factorio.strip_paths"), + maxConcurrentCommands: config.get("factorio.max_concurrent_commands"), + shutdownTimeoutMs: config.get("factorio.shutdown_timeout") * 1000, + }); } catch (err) { - this.notifyExit(); - await this.sendSaveListUpdate(); + host.send(new lib.InstanceStatusChangedEvent(id, "stopped", gamePort, targetVersion)); + await host.sendSaveListUpdate(id, path.join(dir, "saves")); throw err; } + const instance = new this(host, connector, dir, config, server); + instance._status = "starting"; + instance.playerStats = stats.playerStats; + instance._playerAutosaveSlot = stats.playerAutosaveSlot; + return instance; + } + + /** + * Load the instance plugins and report the instance as initialised + * + * Must be called once the instance is connected to the host. + */ + async loadPlugins(pluginInfos: lib.PluginNodeEnvInfo[]) { // load plugins const plugins: Record = {}; for (let pluginInfo of pluginInfos) { diff --git a/packages/host/src/server.ts b/packages/host/src/server.ts index 5cedf332..df3e85f7 100644 --- a/packages/host/src/server.ts +++ b/packages/host/src/server.ts @@ -185,6 +185,73 @@ async function downloadAndExtractTar(url: string, targetDir: string) { // Factorio downloads in progress by target directory, shared by every server in the process const downloadsInProgress = new Map>(); +/** + * Download the newest Factorio version matching the target if not installed + * + * Call before {@link FactorioServer.fromOptions} for it to find the new version. + * + * @param factorioDir - Path to Factorio installation dirs. + * @param targetVersion - Version to look for, or "latest". + * @param availableVersions - Versions available for download. + * @param logger - Logger to report the download on. + */ +export async function checkForUpdates( + factorioDir: string, + targetVersion: lib.TargetVersion, + availableVersions: lib.ExternalFactorioVersion[], + logger: lib.Logger = lib.logger, +) { + + const latestVersion = availableVersions + .filter(v => ( + targetVersion === "latest" || v.version === targetVersion || v.version.startsWith(targetVersion) + )) + .sort((v1, v2) => versionOrder(v1.version, v2.version)) + [0]; + + if (!latestVersion) { + return; + } + + // A version that is still being extracted can already look installed + const dst = path.join(factorioDir, latestVersion.version); + const installedVersions = await listFactorioVersions(factorioDir); + if (!installedVersions.versions.has(latestVersion.version) || downloadsInProgress.has(dst)) { + const v = latestVersion.version; + // Can't download if not linux or this is a direct install + if (installedVersions.direct) { + logger.info( + `A newer version of factorio is available (${v}) but cannot be downloaded automatically ` + + `because ${factorioDir} is a direct (single-version) Factorio install. Use a versioned ` + + `layout, where ${factorioDir} contains a subdirectory per version, to enable automatic ` + + "downloads." + ); + return; + } + if (process.platform !== "linux") { + logger.info( + `A newer version of factorio is available (${v}) but must be manually downloaded ` + + "(automatic downloads are only supported on Linux)." + ); + return; + } + + // Download the newer version, or wait for another server already downloading it + let download = downloadsInProgress.get(dst); + if (download) { + logger.info(`A newer version of factorio is available (${v}), waiting for download...`); + } else { + logger.info(`A newer version of factorio is available (${v}), starting download...`); + download = downloadAndExtractTar(latestVersion.headlessUrl, dst).finally(() => { + downloadsInProgress.delete(dst); + }); + downloadsInProgress.set(dst, download); + } + await download; + logger.info(`Downloaded factorio version ${v}`); + } +} + /** * Find factorio data directory of the given version * @@ -670,9 +737,8 @@ export class FactorioServer extends events.EventEmitter { _factorioDir: string; _writeDir: string; - // Resolved in init - _version: lib.FullVersion | null = null; - _dataDir: string | null = null; + _version: lib.FullVersion; + _dataDir: string; // Due to inconsistencies in the factorio api, we must manually watch the whitelist // https://forums.factorio.com/viewtopic.php?t=123673 @@ -680,8 +746,7 @@ export class FactorioServer extends events.EventEmitter { _whitelist = new Set(); _logger: lib.Logger; - _targetVersion: lib.TargetVersion; - _state: "new" | "init" | "create" | "running" | "stopping" = "new"; + _state: "init" | "create" | "running" | "stopping" = "init"; _server: child_process.ChildProcessWithoutNullStreams | null = null; _rconClient: Rcon | null= null; _rconReady = false; @@ -697,27 +762,36 @@ export class FactorioServer extends events.EventEmitter { /** - * Create a Factorio server interface + * Create a Factorio server interface for the version found by {@link fromOptions} * * @param factorioDir - Directory of the Factorio install(s). * @param writeDir - Directory to write runtime data to. - * @param options - Optional parameters. + * @param dataDir - Data directory of the Factorio version to run. + * @param version - Factorio version to run. + * @param options - Optional parameters, `version` is ignored. */ - constructor(factorioDir: string, writeDir: string, options: FactorioServerOptions) { + constructor( + factorioDir: string, + writeDir: string, + dataDir: string, + version: lib.FullVersion, + options: FactorioServerOptions & { rconPassword: string }, + ) { super(); this._factorioDir = factorioDir; this._writeDir = writeDir; + this._dataDir = dataDir; + this._version = version; this._logger = options.logger || lib.logger; - this._targetVersion = options.version || "latest"; this.executablePath = options.executablePath; /** UDP port used for hosting the Factorio game server on */ this.gamePort = options.gamePort || randomDynamicPort(); /** TCP port used for RCON on the Factorio game server */ this.rconPort = options.rconPort || randomDynamicPort(); /** Password used for RCON on the Factorio game server */ - this.rconPassword = options.rconPassword as string; // init will generate one if not there + this.rconPassword = options.rconPassword; /** Enable UDP messaging with the Factorio game server */ this.enableLuaUdp = options.enableLuaUdp || false; /** UDP port the Factorio game server receives Lua UDP messages on */ @@ -886,7 +960,7 @@ export class FactorioServer extends events.EventEmitter { // Factorio before 2.1.10 crashes when a packet is received on a headless server _checkLuaUdpVersion() { - if (this.enableLuaUdp && lib.integerFullVersion(this._version!) < lib.integerFullVersion("2.1.10")) { + if (this.enableLuaUdp && lib.integerFullVersion(this._version) < lib.integerFullVersion("2.1.10")) { throw new Error("Lua UDP requires Factorio 2.1.10 or later"); } } @@ -1075,94 +1149,25 @@ export class FactorioServer extends events.EventEmitter { } /** - * Override the target version of Factorio to run + * Create a Factorio server interface for an installed version * - * Used to substitute a concrete version for a release channel target - * (e.g. "stable" or "experimental") before {@link checkForUpdates} and - * {@link init} run. Must be called before the server is initialized. - */ - setTargetVersion(version: lib.TargetVersion) { - this._check(["new"]); - this._targetVersion = version; - } - - /** - * Initialize class instance - * - * Must be called before instances of this class can be used. + * @param factorioDir - Directory of the Factorio install(s). + * @param writeDir - Directory to write runtime data to. + * @param options - Optional parameters. * @throws {Error} if the requested version of Factorio was not found. */ - async init() { - this._check(["new"]); - [this._dataDir, this._version] = await findVersion(this._factorioDir, this._targetVersion); - this.rconPassword = this.rconPassword as string | undefined ?? await generatePassword(10); - this._state = "init"; - } - - /** - * Check of updated versions to download - * - * Should be called before init for the new version to be detected during init - */ - async checkForUpdates(availableVersions: lib.ExternalFactorioVersion[]) { - const targetVersion = this._targetVersion; - const latestVersion = availableVersions - .filter(v => ( - targetVersion === "latest" || v.version === targetVersion || v.version.startsWith(targetVersion) - )) - .sort((v1, v2) => versionOrder(v1.version, v2.version)) - [0]; - - if (!latestVersion) { - return; - } - - // A version that is still being extracted can already look installed - const dst = path.join(this._factorioDir, latestVersion.version); - const installedVersions = await listFactorioVersions(this._factorioDir); - if (!installedVersions.versions.has(latestVersion.version) || downloadsInProgress.has(dst)) { - const v = latestVersion.version; - // Can't download if not linux or this is a direct install - if (installedVersions.direct) { - this._logger.info( - `A newer version of factorio is available (${v}) but cannot be downloaded automatically ` + - `because ${this._factorioDir} is a direct (single-version) Factorio install. Use a versioned ` + - `layout, where ${this._factorioDir} contains a subdirectory per version, to enable automatic ` + - "downloads." - ); - return; - } - if (process.platform !== "linux") { - this._logger.info( - `A newer version of factorio is available (${v}) but must be manually downloaded ` + - "(automatic downloads are only supported on Linux)." - ); - return; - } - - // Download the newer version, or wait for another server already downloading it - let download = downloadsInProgress.get(dst); - if (download) { - this._logger.info(`A newer version of factorio is available (${v}), waiting for download...`); - } else { - this._logger.info(`A newer version of factorio is available (${v}), starting download...`); - download = downloadAndExtractTar(latestVersion.headlessUrl, dst).finally(() => { - downloadsInProgress.delete(dst); - }); - downloadsInProgress.set(dst, download); - } - await download; - this._logger.info(`Downloaded factorio version ${v}`); - } + static async fromOptions(factorioDir: string, writeDir: string, options: FactorioServerOptions) { + const [dataDir, version] = await findVersion(factorioDir, options.version || "latest"); + const rconPassword = options.rconPassword ?? await generatePassword(10); + return new this(factorioDir, writeDir, dataDir, version, { ...options, rconPassword }); } /** * The version of Factorio in use. This will be the actual version if - * "latest" (the default) was specified as `factorioVersion` to the constructor. - * This will be undefined before the server is initialized, or if init failed. + * "latest" (the default) was passed to {@link fromOptions}. */ - get version(): lib.FullVersion | undefined { - return this._version ?? undefined; + get version(): lib.FullVersion { + return this._version; } /** @@ -1681,14 +1686,13 @@ export class FactorioServer extends events.EventEmitter { * Return path in data directory * * Creates a path using path.join with the given parts that's relative to - * the data directory of the Factorio server. Not valid before init has - * been called. + * the data directory of the Factorio server. * * @param parts - Extra parts to add to the data path. * @returns Data directory path. */ dataPath(...parts: string[]) { - return path.join(this._dataDir!, ...parts); + return path.join(this._dataDir, ...parts); } /** diff --git a/packages/lib/src/data/messages_controller.ts b/packages/lib/src/data/messages_controller.ts index 732ff875..fa6f106d 100644 --- a/packages/lib/src/data/messages_controller.ts +++ b/packages/lib/src/data/messages_controller.ts @@ -432,7 +432,7 @@ export class DebugWsMessageEvent { export class FactorioVersionsRequest { declare ["constructor"]: typeof FactorioVersionsRequest; static type = "request" as const; - static src = ["control", "instance"] as const; + static src = ["control", "host", "instance"] as const; static dst = "controller" as const; static permission = "core.external.get_factorio_versions" as const; @@ -454,7 +454,7 @@ export class FactorioVersionsRequest { export class LatestReleasesRequest { declare ["constructor"]: typeof LatestReleasesRequest; static type = "request" as const; - static src = ["control", "instance"] as const; + static src = ["control", "host", "instance"] as const; static dst = "controller" as const; static permission = "core.external.get_latest_releases" as const; diff --git a/test/host/Host.test.js b/test/host/Host.test.js index c8a96a8f..26f4d569 100644 --- a/test/host/Host.test.js +++ b/test/host/Host.test.js @@ -77,6 +77,29 @@ describe("host/src/Host", function() { await fs.access(modsDirectory); }); }); + describe("._connectInstance()", function() { + const originalFromConfig = Instance.fromConfig; + afterEach(function() { + Instance.fromConfig = originalFromConfig; + }); + it("should reject a second start while the instance is starting", async function() { + const hostConfig = new lib.HostConfig("host"); + const host = new Host(new HostConnector(hostConfig, []), hostConfig, []); + const instanceConfig = new lib.InstanceConfig("host", { "instance.id": 7 }); + host.assignedInstances.set(7, { path: "dir", config: instanceConfig }); + let fail; + Instance.fromConfig = () => new Promise((resolve, reject) => { fail = reject; }); + + const first = host._connectInstance(7); + await assert.rejects(host._connectInstance(7), /Instance with ID 7 is running/); + assert.equal(host._unconnectedStatus(7), "starting"); + + fail(new Error("start failed")); + await assert.rejects(first, /start failed/); + assert.equal(host._unconnectedStatus(7), "stopped"); + assert.equal(host.instanceConnections.has(7), false); + }); + }); describe(".handleSyncUserListsEvent()", function() { const hostAddress = lib.Address.fromShorthand({ hostId: 1 }); const instanceAddress = lib.Address.fromShorthand({ instanceId: 1 }); diff --git a/test/host/Instance.js b/test/host/Instance.js index 9d4562a5..2c39a80e 100644 --- a/test/host/Instance.js +++ b/test/host/Instance.js @@ -1,4 +1,5 @@ import assert from "node:assert/strict"; +import fs from "node:fs/promises"; import path from "node:path"; import * as lib from "@clusterio/lib"; @@ -12,6 +13,79 @@ const addr = lib.Address.fromShorthand; describe("class Instance", function() { + describe(".fromConfig()", function() { + const factorioDir = path.join("test", "file", "factorio"); + const dir = path.join("temp", "test", "instance_from_config"); + let host; + let sent; + let saveListUpdates; + let requests; + function makeConfig(fields) { + return new lib.InstanceConfig("host", { "instance.id": 5, "instance.name": "foo", ...fields }); + } + function fromConfig(config) { + const connector = new MockConnector(addr({ instanceId: 5 }), addr({ hostId: 1 })); + return Instance.fromConfig(config, host, connector, dir, factorioDir); + } + beforeEach(async function() { + await fs.rm(dir, { recursive: true, force: true }); + await fs.mkdir(dir, { recursive: true }); + sent = []; + saveListUpdates = []; + requests = []; + host = { + assignFreeGamePort: async () => 34100, + send(event) { sent.push(event); }, + async sendTo(dst, request) { + requests.push(request.constructor.name); + if (request instanceof lib.FactorioVersionsRequest) { + return []; + } + if (request instanceof lib.LatestReleasesRequest) { + return { stable: { headless: "0.1.1" } }; + } + throw new Error(`Unexpected request ${request.constructor.name}`); + }, + async sendSaveListUpdate(instanceId, savesDir) { saveListUpdates.push([instanceId, savesDir]); }, + }; + }); + + it("should create an instance with a server for the installed version", async function() { + const instance = await fromConfig(makeConfig({})); + assert.equal(instance.status, "starting"); + assert.equal(instance.server.version, "0.1.2"); + assert.equal(instance.server.gamePort, 34100); + assert.deepEqual(sent, [new lib.InstanceStatusChangedEvent(5, "starting", 34100, "latest")]); + assert.deepEqual(requests, ["FactorioVersionsRequest"]); + }); + it("should use the configured game port", async function() { + const instance = await fromConfig(makeConfig({ "factorio.game_port": 34200 })); + assert.equal(instance.server.gamePort, 34200); + }); + it("should resolve a release channel through the controller", async function() { + const instance = await fromConfig(makeConfig({ "factorio.version": "stable" })); + assert.equal(instance.server.version, "0.1.1"); + assert.deepEqual(requests, ["FactorioVersionsRequest", "LatestReleasesRequest"]); + }); + it("should load player stats", async function() { + await fs.writeFile(path.join(dir, "instance-stats.json"), JSON.stringify({ + players: [["player", { join_count: 2 }]], + player_autosave_slot: 3, + })); + const instance = await fromConfig(makeConfig({})); + assert.equal(instance.playerStats.get("player").joinCount, 2); + assert.equal(instance._playerAutosaveSlot, 3); + }); + it("should report the instance stopped if the version is not installed", async function() { + await assert.rejects(fromConfig(makeConfig({ "factorio.version": "0.1.5" })), /0\.1\.5/); + assert.deepEqual(sent, [ + new lib.InstanceStatusChangedEvent(5, "starting", 34100, "0.1.5"), + new lib.InstanceStatusChangedEvent(5, "stopped", 34100, "0.1.5"), + ]); + assert.deepEqual(saveListUpdates, [[5, path.join(dir, "saves")]]); + }); + }); + let src; let dst = addr({ hostId: 1 }); let instance; @@ -21,8 +95,7 @@ describe("class Instance", function() { instanceConfig.set("instance.name", "foo"); src = addr({ instanceId: instanceConfig.get("instance.id") }); connector = new MockConnector(src, dst); - instance = new Instance({ assignGamePort: () => 1 }, connector, "dir", "factorioDir", instanceConfig); - instance.server = new MockServer(); + instance = new Instance({}, connector, "dir", instanceConfig, new MockServer()); }); describe(".name", function() { diff --git a/test/host/export.js b/test/host/export.js index 48c9eaea..43109532 100644 --- a/test/host/export.js +++ b/test/host/export.js @@ -9,8 +9,7 @@ describe("host/src/export", function() { let testServer; before(async function() { let writePath = path.join("temp", "test", "server"); - testServer = new FactorioServer(path.join("test", "file", "factorio"), writePath, {}); - await testServer.init(); + testServer = await FactorioServer.fromOptions(path.join("test", "file", "factorio"), writePath, {}); }); describe("exportLocale()", function() { diff --git a/test/host/server.js b/test/host/server.js index 9934b575..20d2ae5c 100644 --- a/test/host/server.js +++ b/test/host/server.js @@ -223,7 +223,10 @@ describe("host/server", function() { describe("class FactorioServer", function() { let writePath = path.join("temp", "test", "server"); - let server = new hostServer.FactorioServer(path.join("test", "file", "factorio"), writePath, {}); + let server; + before(async function() { + server = await hostServer.FactorioServer.fromOptions(path.join("test", "file", "factorio"), writePath, {}); + }); describe("constructor()", function() { it("should handle dashes in write path with strapPaths enabled", function() { @@ -231,28 +234,37 @@ describe("host/server", function() { new hostServer.FactorioServer( path.join("test", "file", "factorio"), path.join("temp", "test", "server-1"), - { stripPaths: true } + path.join("test", "file", "factorio", "0.1.2", "data"), + "0.1.2", + { stripPaths: true, rconPassword: "" } ); }); }); - describe(".setTargetVersion()", function() { - it("should override the target version before init", function() { - const fresh = new hostServer.FactorioServer( - path.join("test", "file", "factorio"), writePath, {} + describe(".fromOptions()", function() { + it("should resolve the requested version", async function() { + const found = await hostServer.FactorioServer.fromOptions( + path.join("test", "file", "factorio"), writePath, { version: "0.1.1" } ); - fresh.setTargetVersion("0.1.5"); - assert.equal(fresh._targetVersion, "0.1.5"); + assert.equal(found.version, "0.1.1"); + assert.equal(found.dataPath(), path.join("test", "file", "factorio", "0.1.1", "data")); + assert.equal(found._state, "init"); }); - }); - - describe(".init()", function() { - it("should not throw on first call", async function() { - await server.init(); + it("should generate an RCON password if none is given", async function() { + assert.equal(typeof server.rconPassword, "string"); + assert.equal(server.rconPassword.length, 10); + const given = await hostServer.FactorioServer.fromOptions( + path.join("test", "file", "factorio"), writePath, { rconPassword: "secret" } + ); + assert.equal(given.rconPassword, "secret"); }); - - it("should throw if called twice", async function() { - await assert.rejects(server.init(), new Error("Expected state new but state is init")); + it("should reject if the version is not installed", async function() { + await assert.rejects( + hostServer.FactorioServer.fromOptions( + path.join("test", "file", "factorio"), writePath, { version: "0.1.5" } + ), + /Unable to find Factorio version 0.1.5/ + ); }); }); @@ -314,7 +326,7 @@ describe("host/server", function() { describe(".handle()", function() { it("should log validation errors from ipc handlers", async function() { let logged = []; - let ipcServer = new hostServer.FactorioServer( + let ipcServer = await hostServer.FactorioServer.fromOptions( path.join("test", "file", "factorio"), writePath, { logger: { error: msg => { logged.push(msg); } } } ); @@ -571,7 +583,7 @@ describe("host/server", function() { }); }); - describe(".checkForUpdates()", function() { + describe("checkForUpdates()", function() { let _fetch; let fetchCalledWith; let _platform = process.platform; @@ -589,7 +601,6 @@ describe("host/server", function() { }); afterEach(function() { global.fetch = _fetch; - server._factorioDir = path.join("test", "file", "factorio"); Object.defineProperty(process, "platform", { value: _platform, }); @@ -602,18 +613,17 @@ describe("host/server", function() { ]) { /* eslint-disable no-loop-func */ describe(suite, function() { + const factorioDir = path.join("test", "file", "factorio"); + // A target that matches the 0.1.5 download but not the installed 0.1.1 + const newerTarget = target === "0.1.1" ? "0.1.5" : target; it("should do nothing when there are no versions", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target; - await server.checkForUpdates([]); + await hostServer.checkForUpdates(factorioDir, target, []); assert.equal(fetchCalledWith, null); }); if (target !== "latest") { it("should do nothing when no version matches", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target; - await server.checkForUpdates([{ + await hostServer.checkForUpdates(factorioDir, target, [{ stable: true, version: "0.2.1", headlessUrl: "test1", @@ -627,9 +637,7 @@ describe("host/server", function() { }); } it("should do nothing when there is no newer version", async function() { - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target; - await server.checkForUpdates([{ + await hostServer.checkForUpdates(factorioDir, target, [{ stable: true, version: "0.1.1", headlessUrl: "test1", @@ -642,9 +650,7 @@ describe("host/server", function() { assert.equal(fetchCalledWith, null); }); it("should do nothing for direct installs", async function() { - server._factorioDir = path.join("test", "file", "factorio", "0.1.1"); - server._targetVersion = target === "0.1.1" ? "0.1.5" : target; - await server.checkForUpdates([{ + await hostServer.checkForUpdates(path.join(factorioDir, "0.1.1"), newerTarget, [{ stable: true, version: "0.1.5", headlessUrl: "test1", @@ -662,12 +668,10 @@ describe("host/server", function() { }); it("should do nothing when on windows", async function() { let logLine = null; - server._logger = { info: line => { logLine = line; } }; + const logger = { info: line => { logLine = line; } }; Object.defineProperty(process, "platform", { value: "win32" }); - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target === "0.1.1" ? "0.1.5" : target; - await server.checkForUpdates([{ + await hostServer.checkForUpdates(factorioDir, newerTarget, [{ stable: true, version: "0.1.5", headlessUrl: "test1", @@ -679,7 +683,7 @@ describe("host/server", function() { stable: false, version: "0.1.0", headlessUrl: "test3", - }]); + }], logger); assert.equal(fetchCalledWith, null); assert.ok(logLine !== null); @@ -687,12 +691,10 @@ describe("host/server", function() { }); it("should do attempt to download on linux", async function() { let logLine = null; - server._logger = { info: line => { logLine = line; } }; + const logger = { info: line => { logLine = line; } }; Object.defineProperty(process, "platform", { value: "linux" }); - server._factorioDir = path.join("test", "file", "factorio"); - server._targetVersion = target === "0.1.1" ? "0.1.5" : target; - await assert.rejects(server.checkForUpdates([{ + await assert.rejects(hostServer.checkForUpdates(factorioDir, newerTarget, [{ stable: true, version: "0.1.5", headlessUrl: "test1", @@ -704,7 +706,7 @@ describe("host/server", function() { stable: false, version: "0.1.0", headlessUrl: "test3", - }]), new Error("Failed to fetch test1: -1 Fetch called")); + }], logger), new Error("Failed to fetch test1: -1 Fetch called")); assert.equal(fetchCalledWith, "test1"); assert.ok(logLine !== null); @@ -716,7 +718,7 @@ describe("host/server", function() { describe("concurrent downloads", function() { const versions = [{ stable: true, version: "0.1.5", headlessUrl: "test1" }]; - // Holds the mocked download open until release() and resolves waiting once a server waits for it + // Holds the mocked download open until release() and resolves waiting once a check waits for it function holdDownload(factorioDir) { const held = { fetches: 0 }; const release = new Promise(resolve => { held.release = resolve; }); @@ -731,7 +733,7 @@ describe("host/server", function() { let waited; held.waiting = new Promise(resolve => { waited = resolve; }); const logger = { info: line => line.endsWith("waiting for download...") && waited() }; - held.servers = [0, 1].map(() => new hostServer.FactorioServer(factorioDir, writePath, { logger })); + held.check = () => hostServer.checkForUpdates(factorioDir, "latest", versions, logger); Object.defineProperty(process, "platform", { value: "linux" }); return held; } @@ -739,7 +741,7 @@ describe("host/server", function() { it("should download once when servers check at the same time", async function() { const held = holdDownload(path.join("test", "file", "factorio")); let settled = 0; - const checks = held.servers.map(s => s.checkForUpdates(versions).finally(() => { settled += 1; })); + const checks = [0, 1].map(() => held.check().finally(() => { settled += 1; })); await held.waiting; assert.equal(held.fetches, 1); @@ -754,14 +756,14 @@ describe("host/server", function() { await fs.rm(factorioDir, { force: true, recursive: true }); await fs.mkdir(factorioDir, { recursive: true }); const held = holdDownload(factorioDir); - const first = held.servers[0].checkForUpdates(versions); + const first = held.check(); await held.fetching; // The extraction has written the changelog but has not finished const dataDir = path.join(factorioDir, "0.1.5", "data"); await fs.mkdir(dataDir, { recursive: true }); await fs.writeFile(path.join(dataDir, "changelog.txt"), "Version: 0.1.5\n"); - const second = held.servers[1].checkForUpdates(versions); + const second = held.check(); await held.waiting; held.release(); @@ -776,12 +778,11 @@ describe("host/server", function() { this.skip(); } - server._factorioDir = path.join("temp", "test", "factorioDownload"); - server._targetVersion = "latest"; + const factorioDir = path.join("temp", "test", "factorioDownload"); global.fetch = _fetch; - await fs.rm(server._factorioDir, { force: true, recursive: true, maxRetries: 10 }); - await fs.mkdir(server._factorioDir, { recursive: true }); - await server.checkForUpdates([{ + await fs.rm(factorioDir, { force: true, recursive: true, maxRetries: 10 }); + await fs.mkdir(factorioDir, { recursive: true }); + await hostServer.checkForUpdates(factorioDir, "latest", [{ stable: true, version: "2.0.73", headlessUrl: "https://www.factorio.com/get-download/2.0.73/headless/linux64", diff --git a/test/host/server_stop.js b/test/host/server_stop.js index 8cae2ac4..14871ae1 100644 --- a/test/host/server_stop.js +++ b/test/host/server_stop.js @@ -15,7 +15,10 @@ class FakeProcess extends events.EventEmitter { } function createStartedServer() { - const server = new FactorioServer(path.join("test", "file", "factorio"), path.join("temp", "test", "server"), {}); + const server = new FactorioServer( + path.join("test", "file", "factorio"), path.join("temp", "test", "server"), + path.join("test", "file", "factorio", "0.1.2", "data"), "0.1.2", { rconPassword: "" }, + ); server._state = "running"; server._rconReady = false; server._server = new FakeProcess(); diff --git a/test/integration/export.js b/test/integration/export.js index 7e6b0d45..da7362c5 100644 --- a/test/integration/export.js +++ b/test/integration/export.js @@ -12,8 +12,7 @@ describe("Integration of lib/factorio/export", function() { requiresFactorio(this); it("returns a nested Map with base game locale information", async function() { let writePath = path.join("temp", "test", "server"); - let testServer = new FactorioServer(factorioDir, writePath, {}); - await testServer.init(); + let testServer = await FactorioServer.fromOptions(factorioDir, writePath, {}); let locale = await _exportLocale(testServer, new Map(), new Map(), ["base"], "en"); diff --git a/test/integration/routing.js b/test/integration/routing.js index 3aa2a9c9..6a0103bd 100644 --- a/test/integration/routing.js +++ b/test/integration/routing.js @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import * as lib from "@clusterio/lib"; import { Host, Instance, InstanceConnection } from "@clusterio/host"; import { ControlConnection, Controller, HostConnection, InstanceRecord } from "@clusterio/controller"; +import { MockServer } from "../mock.js"; // Although there are no imports, the file has side-effects such as "before" which can effect tests import "./index.js"; @@ -39,7 +40,7 @@ function connectInstance(controller, host, instanceId) { instanceConfig.set("instance.assigned_host", host.config.get("host.id")); host.assignedInstances.set(instanceId, { path: "invalid", config: instanceConfig }); controller.instances.records.set(new InstanceRecord(instanceConfig, "stopped")); - const instance = new Instance(host, instanceSide, "invalid", "invalid", instanceConfig); + const instance = new Instance(host, instanceSide, "invalid", instanceConfig, new MockServer()); const instanceConnection = new InstanceConnection(hostSide, host, instance); host.instanceConnections.set(instanceId, instanceConnection); return instance; diff --git a/test/integration/server.js b/test/integration/server.js index e1f5b369..2251a6d0 100644 --- a/test/integration/server.js +++ b/test/integration/server.js @@ -40,12 +40,14 @@ describe("Integration of host/src/server", function() { describe("class FactorioServer", function() { let writePath = path.join("temp", "test", "integration"); - let server = new FactorioServer(factorioDir, writePath, {}); - // Let Factorio bind an OS-assigned UDP port instead of guessing a free one. - server.gamePort = 0; + let server; let logFile; before(async function() { + server = await FactorioServer.fromOptions(factorioDir, writePath, {}); + // Let Factorio bind an OS-assigned UDP port instead of guessing a free one. + server.gamePort = 0; + // Delete result from previous run of these tests await fs.rm(writePath, { force: true, recursive: true, maxRetries: 10 }); @@ -63,9 +65,6 @@ describe("Integration of host/src/server", function() { } logFile.write(`${JSON.stringify(output)}\n`); }); - - // Initialize sever. - await server.init(); }); after(function() {