From aec0fc7a1e63f56b1a4440d4f6872abdbc0b0912 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:33:27 +0000 Subject: [PATCH 1/2] Let plugins migrate the data core stores for them Plugins had no way to update the config fields and permissions core stores for them, so renaming inventory_sync needed migrations in core packages. A plugin can now declare a migrationEntrypoint. It's loaded on the controller and hosts before configs and roles are read, for every plugin in the plugin list, and attaches handlers to sync hooks for the controller, host and instance configs and roles. lib gets helpers to rename config fields and permissions, and each config gets a hidden .version field set after load so migrations can tell which version saved the data. Co-Authored-By: Claude Opus 5.5 --- docs/writing-plugins.md | 54 ++++++++++++++++++ packages/controller/controller.ts | 8 ++- packages/controller/src/Controller.ts | 17 +++++- packages/host/host.ts | 6 ++ packages/lib/browser.ts | 1 + packages/lib/index.ts | 2 + packages/lib/src/SyncHook.ts | 69 ++++++++++++++++++++++ packages/lib/src/config/classes.ts | 13 ++++- packages/lib/src/config/definitions.ts | 37 ++++++++++++ packages/lib/src/data/Role.ts | 4 ++ packages/lib/src/migrations.ts | 79 ++++++++++++++++++++++++++ packages/lib/src/plugin.ts | 18 +++++- packages/lib/src/plugin_loader.ts | 40 +++++++++++++ packages/lib/tsconfig.browser.json | 1 + test/controller/Controller.js | 13 ++++- test/lib/SyncHook.js | 52 +++++++++++++++++ test/lib/config/migrations.js | 65 +++++++++++++++++++++ test/lib/migrations.js | 50 ++++++++++++++++ test/lib/plugin_loader.js | 40 +++++++++++++ 19 files changed, 562 insertions(+), 7 deletions(-) create mode 100644 packages/lib/src/SyncHook.ts create mode 100644 packages/lib/src/migrations.ts create mode 100644 test/lib/SyncHook.js create mode 100644 test/lib/migrations.js diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md index bf878972..093d98e7 100644 --- a/docs/writing-plugins.md +++ b/docs/writing-plugins.md @@ -11,6 +11,7 @@ Each entrypoint of a plugin exports a function that attaches handlers to hooks c - [Logging Messages](#logging-messages) - [Plugin Configuration](#plugin-configuration) - [Plugin Permissions](#plugin-permissions) +- [Migrating Stored Data](#migrating-stored-data) - [Communicating with Factorio](#communicating-with-factorio) - [Defining Link Messages](#defining-link-messages) - [Sending Link Messages](#sending-link-messages) @@ -98,6 +99,11 @@ The following properties are recognized: This is an optional paramater. A plugin can be made that only runs on the clusterioctl side. +**migrationEntrypoint**: + Path to a Node.js module relative to the plugin directory which exports the migration entrypoint function for this plugin. + This is an optional parameter. + See [Migrating Stored Data](#migrating-stored-data) + **features**: Array of features the plugin needs from the instances it is loaded on, `"SavePatching"` and/or `"ScriptCommands"`. See [Handling Invalid Configuration](#handling-invalid-configuration) @@ -310,6 +316,54 @@ Permissions declared this way are registered when the plugin is loaded. The older `lib.definePermission()` function still works, but a plugin should not use both for the same permission name. +## Migrating Stored Data + +Clusterio stores the config fields and permissions of plugins, so a plugin that renames them needs to migrate the stored values or users lose what they had set. +This is done from the migration entrypoint, which is loaded on the controller and hosts before any config or role is read, for all plugins in the plugin list including those disabled with `load_plugin`. +It is passed a context with `hooks` and attaches handlers for the data it needs to migrate: + +```js +import * as lib from "@clusterio/lib"; + +export default function(context) { + const { hooks, plugin } = context; + // Plugin was renamed from frobber in 2.0.0 + const renameFields = config => lib.renameConfigPrefix(config, "frobber.", "foo_frobber."); + hooks.controllerConfig.attach(plugin.name, renameFields); + hooks.hostConfig.attach(plugin.name, renameFields); + hooks.instanceConfig.attach(plugin.name, renameFields); + hooks.role.attach(plugin.name, role => lib.renamePermissionPrefix(role, "frobber.", "foo_frobber.")); +} +``` + +The handlers get the serialized config or role after the core migrations and return the migrated version. +They run synchronously, a handler that throws stops the controller or host from starting. +`lib.renameConfigField`, `lib.renameConfigPrefix`, `lib.renamePermission` and `lib.renamePermissionPrefix` cover the common case of renaming. + +Handlers run every time the data is loaded and must leave data that is already migrated unchanged. +For changes that can't be repeated, each config has a hidden `.version` field with the version of the plugin it was last loaded with, which is updated after the migrations have run: + +```js +hooks.instanceConfig.attach(plugin.name, config => { + const version = config["foo_frobber.version"]; + // The default delay went from 1 to 2 in 1.2.0, update configs still on the old default + if ( + version && lib.integerFullVersion(version) < lib.integerFullVersion("1.2.0") + && config["foo_frobber.delay"] === 1 + ) { + config["foo_frobber.delay"] = 2; + } + return config; +}); +``` + +The version field is missing in configs saved before the plugin was loaded with Clusterio versions that track it. +Roles have no version. + +Data the plugin stores itself, like files in the database directory or values in the save, is up to the plugin to migrate from its other entrypoints. +Modules don't need migrating, save patching replaces the files of every module when an instance starts. + + ## Communicating with Factorio For pushing data into Factorio there's RCON, which lets you send arbitrary Lua commands to invoke whatever code you want in the game. diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index ba863ae9..8057bd77 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -419,6 +419,9 @@ async function initialize(): Promise { lib.registerPluginMessages(pluginInfos); lib.registerPluginPermissions(pluginInfos); lib.addPluginConfigFields(pluginInfos); + if (!args.recovery) { + await lib.loadPluginMigrations(pluginInfos); + } const [controllerConfig, controllerConfigLock] = await lib.loadConfigFromArgs( args, @@ -427,6 +430,9 @@ async function initialize(): Promise { ); controllerConfig.set("controller.version", packageConfig.version); // Allows tracking last loaded version + if (!args.recovery) { + lib.setPluginVersions(controllerConfig, pluginInfos); + } if (!controllerConfig.get("controller.auth_secret")) { logger.info("Generating new controller authentication secret"); @@ -477,7 +483,7 @@ async function startup() { controllerConfig, Boolean(args.canRestart), Boolean(args.recovery), - ...await Controller.bootstrap(controllerConfig) + ...await Controller.bootstrap(controllerConfig, args.recovery ? [] : pluginInfos) ); // Refuse to start if there are no users loaded diff --git a/packages/controller/src/Controller.ts b/packages/controller/src/Controller.ts index 454b96ef..edfbac06 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -148,7 +148,15 @@ export default class Controller { } }); - static async bootstrap(config: lib.ControllerConfig) { + /** + * Load the controller datastores + * + * @param config - Controller config. + * @param pluginInfos - + * Plugins whose migrations ran, their versions are saved to the + * instance configs. + */ + static async bootstrap(config: lib.ControllerConfig, pluginInfos: lib.PluginNodeEnvInfo[] = []) { let databaseDirectory = config.get("controller.database_directory"); await fs.mkdir(databaseDirectory, { recursive: true }); @@ -175,6 +183,11 @@ export default class Controller { InstanceRecord.fromJSON.bind(InstanceRecord), this.migrateInstances, this.finaliseInstances, ).bootstrap()); + for (const instance of instances.values()) { + if (lib.setPluginVersions(instance.config, pluginInfos)) { + instances.set(instance); + } + } const saves = new lib.SubscribableDatastore(...await new lib.JsonIdDatastoreProvider( path.join(databaseDirectory, "saves.json"), @@ -822,7 +835,7 @@ export default class Controller { } return permission; }); - return json; + return lib.Role.pluginMigrations.reduce(json); }); } diff --git a/packages/host/host.ts b/packages/host/host.ts index 8d31e071..f6270dec 100755 --- a/packages/host/host.ts +++ b/packages/host/host.ts @@ -177,6 +177,9 @@ async function startHost() { lib.registerPluginMessages(pluginInfos); lib.registerPluginPermissions(pluginInfos); lib.addPluginConfigFields(pluginInfos); + if (!args.recovery) { + await lib.loadPluginMigrations(pluginInfos); + } const [hostConfig, hostConfigLock] = await lib.loadConfigFromArgs( args, @@ -185,6 +188,9 @@ async function startHost() { ); hostConfig.set("host.version", packageConfig.version); // Allows tracking last loaded version + if (!args.recovery) { + lib.setPluginVersions(hostConfig, pluginInfos); + } if (command === "config") { await lib.handleConfigCommand(args, hostConfig, hostConfigLock); diff --git a/packages/lib/browser.ts b/packages/lib/browser.ts index 153a0370..121c0103 100644 --- a/packages/lib/browser.ts +++ b/packages/lib/browser.ts @@ -1,5 +1,6 @@ export * from "./src/api"; export * from "./src/AsyncHook"; +export * from "./src/SyncHook"; export * from "./src/config"; export * from "./src/data"; export * from "./src/errors"; diff --git a/packages/lib/index.ts b/packages/lib/index.ts index 109851f2..f45e5554 100644 --- a/packages/lib/index.ts +++ b/packages/lib/index.ts @@ -37,6 +37,8 @@ export * from "./src/load_plugin_list.js"; export * from "./src/rce_ops.js"; export * from "./src/LockFile.js"; export * from "./src/AsyncHook.js"; +export * from "./src/SyncHook.js"; +export * from "./src/migrations.js"; export { default as ExponentialBackoff } from "./src/ExponentialBackoff.js"; export { default as ModStore } from "./src/ModStore.js"; diff --git a/packages/lib/src/SyncHook.ts b/packages/lib/src/SyncHook.ts new file mode 100644 index 00000000..784d2aea --- /dev/null +++ b/packages/lib/src/SyncHook.ts @@ -0,0 +1,69 @@ +import { PluginError } from "./errors.js"; + +export type SyncHookHandler = (value: Value) => Value; + +/** + * Hook that passes a value through each attached handler in turn. + * + * Handlers run synchronously in the order they were attached, for places + * that can't wait on a promise such as the Config constructor. + */ +export class SyncHook { + private _handlers = new Map>(); + + get size() { + return this._handlers.size; + } + + get attached() { + return this._handlers.keys(); + } + + /** + * Attach a handler to this hook. + * + * An error is thrown if another handler with the same name is already attached. + */ + attach(name: string, handler: SyncHookHandler) { + if (this._handlers.has(name)) { + throw new Error(`Handler with name ${name} is already attached`); + } + this._handlers.set(name, handler); + } + + /** + * Remove a previously attached handler. + * + * If no handler exists, then no action is performed. + */ + detach(name: string) { + this._handlers.delete(name); + } + + /** + * Remove all attached handlers. + */ + clear() { + this._handlers.clear(); + } + + /** + * Pass a value through all attached handlers. + * + * Each handler gets the value returned by the one before it. Errors are + * rethrown as a PluginError named after the handler that threw. + * + * @param value - Value given to the first handler. + * @returns Value returned by the last handler. + */ + reduce(value: Value) { + for (const [name, handler] of this._handlers) { + try { + value = handler(value); + } catch (err: any) { + throw new PluginError(name, err instanceof Error ? err : new Error(String(err))); + } + } + return value; + } +} diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index cfc78837..eb20af68 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -10,6 +10,7 @@ import { StringEnum } from "../data/composites.js"; import { safeOutputFile } from "../file_ops.js"; import * as validators from "./validators.js"; import { RequestError } from "../errors.js"; +import type { SyncHook } from "../SyncHook.js"; const ConfigLocation = StringEnum(["controller", "host", "control"]); export type ConfigLocation = Static; @@ -224,6 +225,12 @@ export class Config< return config; } + /** + * Migrations attached by plugins, run after {@link Config.migrations}. + * Declared by each subclass that plugins can add fields to. + */ + declare static pluginMigrations?: SyncHook; + /** Defines the default access locations for the config class */ static defaultAccess: ConfigLocation[] = ["controller", "host", "control"]; @@ -269,7 +276,11 @@ export class Config< ) as Fields; if (fields) { - this.update(this.constructor.migrations(fields), false, location); + let migrated = this.constructor.migrations(fields); + if (this.constructor.pluginMigrations) { + migrated = this.constructor.pluginMigrations.reduce(migrated); + } + this.update(migrated, false, location); } } diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index 1938075d..a81c44ef 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -4,6 +4,7 @@ import * as validators from "./validators.js"; import type { PluginNodeEnvInfo, PluginWebpackEnvInfo } from "../plugin.js"; import { Static } from "@sinclair/typebox"; import { isTargetVersion } from "../data/version.js"; +import { SyncHook } from "../SyncHook.js"; type configFromJSON = (...args: Parameters) => T; type configFromFile = (...args: Parameters) => Promise; @@ -42,6 +43,7 @@ export interface ControllerConfigFields { "controller.allow_plugin_updates": boolean; "controller.allow_plugin_install": boolean; [key: `${string}.load_plugin`]: boolean; + [key: `${string}.version`]: string; } /** @@ -51,6 +53,7 @@ export interface ControllerConfigFields { export class ControllerConfig extends classes.Config { declare static fromJSON: configFromJSON; declare static fromFile: configFromFile; + static pluginMigrations = new SyncHook(); static migrations(config: Static) { if (config.hasOwnProperty("controller.external_address")) { config["controller.public_url"] = config["controller.external_address"]; @@ -334,6 +337,7 @@ export interface HostConfigFields { "host.allow_plugin_updates": boolean; "host.allow_plugin_install": boolean; [key: `${string}.load_plugin`]: boolean; + [key: `${string}.version`]: string; } /** @@ -343,6 +347,7 @@ export interface HostConfigFields { export class HostConfig extends classes.Config { declare static fromJSON: configFromJSON; declare static fromFile: configFromFile; + static pluginMigrations = new SyncHook(); static fieldDefinitions: classes.ConfigDefs = { "host.version": { type: "string", @@ -468,6 +473,7 @@ export interface InstanceConfigFields { "instance.auto_start": boolean; "instance.exclude_from_start_all": boolean; [key: `${string}.load_plugin`]: boolean; + [key: `${string}.version`]: string; "factorio.version": string; "factorio.executable_path": string | null; @@ -500,6 +506,7 @@ export interface InstanceConfigFields { export class InstanceConfig extends classes.Config { declare static fromJSON: configFromJSON; declare static fromFile: configFromFile; + static pluginMigrations = new SyncHook(); static migrations(config: Static) { function boolToEnableDisable(name: string) { if (config.hasOwnProperty(name) && typeof config[name] === "boolean") { @@ -805,6 +812,14 @@ export function addPluginFieldDefinitions( Config.fieldDefinitions[`${pluginInfo.name}.load_plugin`] = fieldDef; + // Version of the plugin the config was last loaded with, see setPluginVersions + Config.fieldDefinitions[`${pluginInfo.name}.version`] = { + type: "string", + initialValue: "version" in pluginInfo ? pluginInfo.version : "", + readonly: ["controller", "host"], + hidden: true, + }; + // Merge in other fields from the plugin const fields = pluginInfo[kind]; if (fields) { @@ -829,3 +844,25 @@ export function addPluginConfigFields(pluginInfos: PluginNodeEnvInfo[] | PluginW } } } + +/** + * Set the version field of each plugin to the version that was loaded + * + * Done after loading a config so the plugin migrations can compare the + * stored version against the one now running. + * + * @param config - Config to update. + * @param pluginInfos - Plugins loaded. + * @returns true if any version changed. + */ +export function setPluginVersions(config: classes.Config, pluginInfos: PluginNodeEnvInfo[]) { + let changed = false; + for (const pluginInfo of pluginInfos) { + const name = `${pluginInfo.name}.version`; + if (config.constructor.fieldDefinitions[name] && config.get(name) !== pluginInfo.version) { + config.set(name, pluginInfo.version); + changed = true; + } + } + return changed; +} diff --git a/packages/lib/src/data/Role.ts b/packages/lib/src/data/Role.ts index 954d5def..cc8c8df6 100644 --- a/packages/lib/src/data/Role.ts +++ b/packages/lib/src/data/Role.ts @@ -1,5 +1,6 @@ import { Static, Type } from "@sinclair/typebox"; import { permissions as globalPermissions } from "../permissions.js"; +import { SyncHook } from "../SyncHook.js"; interface RoleCollection { get(id: number): Role; @@ -31,6 +32,9 @@ export default class Role { is_deleted: Type.Optional(Type.Boolean()), }); + /** Migrations attached by plugins, run on roles loaded by the controller */ + static pluginMigrations = new SyncHook>(); + static fromJSON(json: Static) { return new this( json.id, diff --git a/packages/lib/src/migrations.ts b/packages/lib/src/migrations.ts new file mode 100644 index 00000000..b28a8033 --- /dev/null +++ b/packages/lib/src/migrations.ts @@ -0,0 +1,79 @@ +/** + * Helpers for plugin migrations + * @module lib/migrations + */ +import type { Static } from "@sinclair/typebox"; +import type { ConfigSchema } from "./config/classes.js"; +import type Role from "./data/Role.js"; + +type SerializedRole = Static; + +/** + * Rename a field in a serialized config + * + * The value is moved if the old field exists and the new one doesn't. The + * old field is removed either way. + * + * @param config - Serialized config to update. + * @param from - Current name of the field. + * @param to - New name of the field. + * @returns The config passed in. + */ +export function renameConfigField(config: ConfigSchema, from: string, to: string) { + if (Object.hasOwn(config, from)) { + if (!Object.hasOwn(config, to)) { + config[to] = config[from]; + } + delete config[from]; + } + return config; +} + +/** + * Rename all fields starting with a prefix in a serialized config + * + * @example + * lib.renameConfigPrefix(config, "inventory_sync.", "player_sync."); + * + * @param config - Serialized config to update. + * @param from - Current prefix of the fields. + * @param to - New prefix of the fields. + * @returns The config passed in. + */ +export function renameConfigPrefix(config: ConfigSchema, from: string, to: string) { + for (const name of Object.keys(config)) { + if (name.startsWith(from)) { + renameConfigField(config, name, to + name.slice(from.length)); + } + } + return config; +} + +/** + * Rename a permission in a serialized role + * + * @param role - Serialized role to update. + * @param from - Current name of the permission. + * @param to - New name of the permission. + * @returns The role passed in. + */ +export function renamePermission(role: SerializedRole, from: string, to: string) { + return renamePermissionsWith(role, name => (name === from ? to : name)); +} + +/** + * Rename all permissions starting with a prefix in a serialized role + * + * @param role - Serialized role to update. + * @param from - Current prefix of the permissions. + * @param to - New prefix of the permissions. + * @returns The role passed in. + */ +export function renamePermissionPrefix(role: SerializedRole, from: string, to: string) { + return renamePermissionsWith(role, name => (name.startsWith(from) ? to + name.slice(from.length) : name)); +} + +function renamePermissionsWith(role: SerializedRole, rename: (name: string) => string) { + role.permissions = [...new Set(role.permissions.map(rename))]; + return role; +} diff --git a/packages/lib/src/plugin.ts b/packages/lib/src/plugin.ts index 1a00eb2f..9ff338ab 100644 --- a/packages/lib/src/plugin.ts +++ b/packages/lib/src/plugin.ts @@ -2,10 +2,12 @@ * Plugin interfaces and utilities. * @module lib/plugin */ +import type { Static } from "@sinclair/typebox"; import type { Logger } from "./logging.js"; -import type { FieldDefinition } from "./config/index.js"; +import type { ConfigSchema, FieldDefinition } from "./config/index.js"; import type { PermissionDefinition } from "./permissions.js"; -import type { PlayerStats } from "./data/index.js"; +import type { PlayerStats, Role } from "./data/index.js"; +import type { SyncHook } from "./SyncHook.js"; export const PluginFeatureFlags = [ /** The plugin requires module code to be patched into the save */ @@ -33,6 +35,8 @@ export type PluginDeclaration = { webEntrypoint?: string; ctlConfigFields?: Record; + migrationEntrypoint?: string; + features?: (typeof PluginFeatureFlags)[number][]; messages?: any[]; @@ -114,6 +118,16 @@ export type PluginLoadContext< plugin: Info; }; +/** Hooks for migrating data that Clusterio stores on behalf of plugins */ +export type MigrationHooks = { + controllerConfig: SyncHook; + hostConfig: SyncHook; + instanceConfig: SyncHook; + role: SyncHook>; +}; + +export type MigrationContext = PluginLoadContext<{ hooks: MigrationHooks }>; + export type PluginClass< Context extends object, Info extends PluginDeclaration, diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index d0b8c400..f4ec5276 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -10,6 +10,8 @@ import * as libErrors from "./errors.js"; import * as libPlugin from "./plugin.js"; import { type Logger, logger } from "./logging.js"; import { loadPluginEntrypoint, loadPluginClass } from "./loadPlugin.js"; +import { ControllerConfig, HostConfig, InstanceConfig } from "./config/definitions.js"; +import Role from "./data/Role.js"; /** @@ -90,6 +92,44 @@ export async function loadPluginInfos(pluginList: Map) { return plugins; } +/** + * Load the migration entrypoints of plugins + * + * Must be called before configs and roles are loaded for the migrations + * to run on them. Loaded for every plugin, including those disabled with + * load_plugin. + * + * @param pluginInfos - Plugins to load migrations for. + */ +export async function loadPluginMigrations(pluginInfos: libPlugin.PluginNodeEnvInfo[]) { + const hooks: libPlugin.MigrationHooks = { + controllerConfig: ControllerConfig.pluginMigrations, + hostConfig: HostConfig.pluginMigrations, + instanceConfig: InstanceConfig.pluginMigrations, + role: Role.pluginMigrations, + }; + + for (const pluginInfo of pluginInfos) { + if (!pluginInfo.migrationEntrypoint) { + continue; + } + + try { + const module = await import(pathToFileURL( + path.join(path.dirname(pluginInfo.packagePath), pluginInfo.migrationEntrypoint) + ).href); + const context: libPlugin.MigrationContext = { + hooks, + plugin: pluginInfo, + logger: logger.child({ plugin: pluginInfo.name }), + }; + await loadPluginEntrypoint(pluginInfo, "migration", context, module); + } catch (err: any) { + throw new libErrors.PluginError(pluginInfo.name, err); + } + } +} + export async function loadPlugin< Context extends { logger: Logger }, Class extends libPlugin.PluginClass, diff --git a/packages/lib/tsconfig.browser.json b/packages/lib/tsconfig.browser.json index 4b6c539b..b351f335 100644 --- a/packages/lib/tsconfig.browser.json +++ b/packages/lib/tsconfig.browser.json @@ -5,6 +5,7 @@ "package.json", "./src/api.ts", "./src/AsyncHook.ts", + "./src/SyncHook.ts", "./src/config/*.ts", "./src/data/*.ts", "./src/external/*.ts", diff --git a/test/controller/Controller.js b/test/controller/Controller.js index 5ca508ad..7294ba94 100644 --- a/test/controller/Controller.js +++ b/test/controller/Controller.js @@ -11,7 +11,7 @@ import { fileURLToPath } from "node:url"; import { ControllerConfig, Address, RequestError, InstanceConfig, SystemInfo, Role, ModPack, AccountUpdateEvent, - addPluginConfigFields, + addPluginConfigFields, renamePermissionPrefix, } from "@clusterio/lib"; import "../setup_logging.js"; @@ -714,6 +714,17 @@ describe("controller/src/Controller", function() { { permissions: ["core.instance.save.subscribe"] }, ]); }); + it("runs plugin role migrations", function() { + Role.pluginMigrations.attach("test", role => ( + renamePermissionPrefix(role, "old_name.", "new_name.") + )); + try { + const result = Controller.migrateRoles([{ permissions: ["foo", "old_name.view"] }]); + assert.deepEqual(result, [{ permissions: ["foo", "new_name.view"] }]); + } finally { + Role.pluginMigrations.clear(); + } + }); it("does nothing for upto date data", function() { const role = new Role( 0, "n", "d", new Set("p1", "p2"), 0, false diff --git a/test/lib/SyncHook.js b/test/lib/SyncHook.js new file mode 100644 index 00000000..414c1a0b --- /dev/null +++ b/test/lib/SyncHook.js @@ -0,0 +1,52 @@ +import assert from "node:assert/strict"; + +import * as lib from "@clusterio/lib"; + + +describe("lib/SyncHook", function() { + describe("class SyncHook", function() { + let hook; + beforeEach(function() { + hook = new lib.SyncHook(); + }); + + it("should track attached handlers", function() { + assert.equal(hook.size, 0); + hook.attach("alpha", value => value); + hook.attach("beta", value => value); + assert.equal(hook.size, 2); + assert.deepEqual([...hook.attached], ["alpha", "beta"]); + }); + it("should reject duplicate handler names", function() { + hook.attach("alpha", value => value); + assert.throws( + () => hook.attach("alpha", value => value), + new Error("Handler with name alpha is already attached") + ); + }); + it("should detach and clear handlers", function() { + hook.attach("alpha", value => value); + hook.attach("beta", value => value); + hook.detach("alpha"); + assert.deepEqual([...hook.attached], ["beta"]); + hook.clear(); + assert.equal(hook.size, 0); + }); + it("should pass the value through handlers in attach order", function() { + hook.attach("alpha", value => [...value, "alpha"]); + hook.attach("beta", value => [...value, "beta"]); + assert.deepEqual(hook.reduce([]), ["alpha", "beta"]); + }); + it("should return the value unchanged with no handlers", function() { + const value = {}; + assert.equal(hook.reduce(value), value); + }); + it("should rethrow errors as a PluginError named after the handler", function() { + hook.attach("alpha", () => { throw new Error("failed"); }); + assert.throws( + () => hook.reduce(1), + err => err instanceof lib.PluginError && err.pluginName === "alpha" && err.original.message === "failed" + ); + }); + }); +}); diff --git a/test/lib/config/migrations.js b/test/lib/config/migrations.js index f93a10fd..4861683e 100644 --- a/test/lib/config/migrations.js +++ b/test/lib/config/migrations.js @@ -45,4 +45,69 @@ describe("lib/config/definitions/migrations", function() { describe("Ctl Config", function() { }); + describe("Plugin migrations", function() { + const pluginInfo = { + name: "test_plugin", + version: "1.2.0", + instanceEntrypoint: "instance.js", + instanceConfigFields: { + "test_plugin.delay": { type: "number", initialValue: 1 }, + }, + }; + before(function() { + lib.addPluginConfigFields([pluginInfo]); + }); + afterEach(function() { + lib.InstanceConfig.pluginMigrations.clear(); + }); + after(function() { + for (const Config of [lib.ControllerConfig, lib.HostConfig, lib.InstanceConfig]) { + for (const name of Object.keys(Config.fieldDefinitions)) { + if (name.startsWith("test_plugin.")) { + delete Config.fieldDefinitions[name]; + } + } + } + }); + + it("should run attached migrations on load", function() { + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => ( + lib.renameConfigPrefix(config, "old_plugin.", "test_plugin.") + )); + const config = lib.InstanceConfig.fromJSON({ + "old_plugin.load_plugin": false, + "old_plugin.delay": 5, + }, "controller"); + assert.equal(config.get("test_plugin.load_plugin"), false); + assert.equal(config.get("test_plugin.delay"), 5); + assert.equal(config.toJSON()["old_plugin.delay"], undefined); + }); + it("should run plugin migrations after core migrations", function() { + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { + assert.equal(config["factorio.sync_adminlist"], "enabled"); + return config; + }); + lib.InstanceConfig.fromJSON({ "factorio.sync_adminlist": true }, "controller"); + }); + it("should give migrations the version the config was saved with", function() { + let seen; + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { + seen = config["test_plugin.version"]; + return config; + }); + const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); + assert.equal(seen, "1.1.0"); + assert.equal(config.get("test_plugin.version"), "1.1.0"); + }); + it("should default the version of new configs to the loaded plugin version", function() { + const config = new lib.InstanceConfig("controller"); + assert.equal(config.get("test_plugin.version"), "1.2.0"); + }); + it("should update versions with setPluginVersions", function() { + const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); + assert.equal(lib.setPluginVersions(config, [pluginInfo]), true); + assert.equal(config.get("test_plugin.version"), "1.2.0"); + assert.equal(lib.setPluginVersions(config, [pluginInfo]), false); + }); + }); }); diff --git a/test/lib/migrations.js b/test/lib/migrations.js new file mode 100644 index 00000000..d51c208c --- /dev/null +++ b/test/lib/migrations.js @@ -0,0 +1,50 @@ +import assert from "node:assert/strict"; + +import * as lib from "@clusterio/lib"; + + +describe("lib/migrations", function() { + describe("renameConfigField()", function() { + it("should move the value to the new name", function() { + const config = { "foo.bar": 1, "foo.other": 2 }; + assert.equal(lib.renameConfigField(config, "foo.bar", "foo.bar_baz"), config); + assert.deepEqual(config, { "foo.bar_baz": 1, "foo.other": 2 }); + }); + it("should keep an existing value under the new name", function() { + const config = { "foo.bar": 1, "foo.bar_baz": 2 }; + lib.renameConfigField(config, "foo.bar", "foo.bar_baz"); + assert.deepEqual(config, { "foo.bar_baz": 2 }); + }); + it("should do nothing if the old name is missing", function() { + const config = { "foo.bar_baz": 2 }; + lib.renameConfigField(config, "foo.bar", "foo.bar_baz"); + assert.deepEqual(config, { "foo.bar_baz": 2 }); + }); + }); + describe("renameConfigPrefix()", function() { + it("should rename all fields with the prefix", function() { + const config = { "old.load_plugin": false, "old.value": 1, "old_other.value": 2 }; + lib.renameConfigPrefix(config, "old.", "new."); + assert.deepEqual(config, { "new.load_plugin": false, "new.value": 1, "old_other.value": 2 }); + }); + }); + describe("renamePermission()", function() { + it("should rename the permission", function() { + const role = { permissions: ["foo.run", "foo.view"] }; + assert.equal(lib.renamePermission(role, "foo.run", "foo.frobnicate"), role); + assert.deepEqual(role.permissions, ["foo.frobnicate", "foo.view"]); + }); + it("should not duplicate a permission the role already has", function() { + const role = { permissions: ["foo.run", "foo.frobnicate"] }; + lib.renamePermission(role, "foo.run", "foo.frobnicate"); + assert.deepEqual(role.permissions, ["foo.frobnicate"]); + }); + }); + describe("renamePermissionPrefix()", function() { + it("should rename all permissions with the prefix", function() { + const role = { permissions: ["old.view", "old_other.view", "core.admin"] }; + lib.renamePermissionPrefix(role, "old.", "new."); + assert.deepEqual(role.permissions, ["new.view", "old_other.view", "core.admin"]); + }); + }); +}); diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index ec08f515..c1b4c740 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -110,6 +110,46 @@ describe("lib/plugin_loader", function() { ); }); }); + describe("loadPluginMigrations()", function() { + const baseDir = path.resolve("temp", "test", "plugin", "migration_plugin"); + before(async function() { + await fs.mkdir(baseDir, { recursive: true }); + await fs.writeFile(path.join(baseDir, "migrations.js"), ` + export default function(context) { + context.hooks.controllerConfig.attach(context.plugin.name, config => { + config.migratedBy = context.plugin.name; + return config; + }); + } + `); + await fs.writeFile(path.join(baseDir, "throwing.js"), ` + export default function() { throw new Error("load failed"); } + `); + }); + afterEach(function() { + lib.ControllerConfig.pluginMigrations.clear(); + }); + + function info(entrypoint) { + return { + name: "test", + packagePath: path.join(baseDir, "package.json"), + migrationEntrypoint: entrypoint, + }; + } + + it("should attach migrations from the migration entrypoint", async function() { + await lib.loadPluginMigrations([info("migrations.js"), { name: "other" }]); + assert.deepEqual([...lib.ControllerConfig.pluginMigrations.attached], ["test"]); + assert.deepEqual(lib.ControllerConfig.pluginMigrations.reduce({}), { migratedBy: "test" }); + }); + it("should reject with a PluginError if the entrypoint throws", async function() { + await assert.rejects( + lib.loadPluginMigrations([info("throwing.js")]), + err => err instanceof lib.PluginError && err.pluginName === "test" + ); + }); + }); describe("loadPlugin()", function() { const baseDir = path.join("temp", "test", "plugin"); const functionPlugin = path.resolve(baseDir, "function_plugin"); From e455d17bb7450a9977593d25efdb7e8fe8225ba4 Mon Sep 17 00:00:00 2001 From: bbassie <17990055+bbassie@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:37:03 +0000 Subject: [PATCH 2/2] Write plugin versions while loading configs The staging copy made by Config.set() went through the constructor, so plugin migrations ran a second time on the live fields. A version-gated migration was applied twice when setPluginVersions() changed the version. Staging now copies the fields with update() instead. A config saved without .version got the running version as its default, so setPluginVersions() saw no change and nothing saved the stamp. The constructor now writes the registered plugin versions after the migrations run and marks the config dirty when that changes a loaded value. Controller.bootstrap() saves the instances that came out dirty, and setPluginVersions() is gone. Restructure the SyncHook and plugin config tests by method, and suggest the controller config version for role migrations in the docs. Co-Authored-By: Claude Opus 5.5 --- docs/writing-plugins.md | 24 +++++- packages/controller/controller.ts | 5 +- packages/controller/src/Controller.ts | 13 +-- packages/host/host.ts | 3 - packages/lib/src/config/classes.ts | 35 ++++++-- packages/lib/src/config/definitions.ts | 25 +----- packages/lib/src/plugin_loader.ts | 5 +- test/lib/SyncHook.js | 81 ++++++++++-------- test/lib/config/migrations.js | 110 +++++++++++++++++-------- test/lib/plugin_loader.js | 6 ++ 10 files changed, 188 insertions(+), 119 deletions(-) diff --git a/docs/writing-plugins.md b/docs/writing-plugins.md index 093d98e7..423b283e 100644 --- a/docs/writing-plugins.md +++ b/docs/writing-plugins.md @@ -357,8 +357,28 @@ hooks.instanceConfig.attach(plugin.name, config => { }); ``` -The version field is missing in configs saved before the plugin was loaded with Clusterio versions that track it. -Roles have no version. +The version field is missing or empty in configs saved before Clusterio tracked it, and in configs last loaded in recovery mode. + +Roles have no version of their own. +They are stored on the controller, so use the version in the controller config, which is loaded before the roles: + +```js +let controllerVersion; +hooks.controllerConfig.attach(plugin.name, config => { + controllerVersion ??= config["foo_frobber.version"] ?? ""; + return config; +}); +hooks.role.attach(plugin.name, role => { + // foo_frobber.edit was split out of foo_frobber.view in 1.2.0 + if ( + controllerVersion && lib.integerFullVersion(controllerVersion) < lib.integerFullVersion("1.2.0") + && role.permissions.includes("foo_frobber.view") + ) { + role.permissions.push("foo_frobber.edit"); + } + return role; +}); +``` Data the plugin stores itself, like files in the database directory or values in the save, is up to the plugin to migrate from its other entrypoints. Modules don't need migrating, save patching replaces the files of every module when an instance starts. diff --git a/packages/controller/controller.ts b/packages/controller/controller.ts index 8057bd77..1f7134a1 100644 --- a/packages/controller/controller.ts +++ b/packages/controller/controller.ts @@ -430,9 +430,6 @@ async function initialize(): Promise { ); controllerConfig.set("controller.version", packageConfig.version); // Allows tracking last loaded version - if (!args.recovery) { - lib.setPluginVersions(controllerConfig, pluginInfos); - } if (!controllerConfig.get("controller.auth_secret")) { logger.info("Generating new controller authentication secret"); @@ -483,7 +480,7 @@ async function startup() { controllerConfig, Boolean(args.canRestart), Boolean(args.recovery), - ...await Controller.bootstrap(controllerConfig, args.recovery ? [] : pluginInfos) + ...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 edfbac06..c973b63d 100644 --- a/packages/controller/src/Controller.ts +++ b/packages/controller/src/Controller.ts @@ -148,15 +148,7 @@ export default class Controller { } }); - /** - * Load the controller datastores - * - * @param config - Controller config. - * @param pluginInfos - - * Plugins whose migrations ran, their versions are saved to the - * instance configs. - */ - static async bootstrap(config: lib.ControllerConfig, pluginInfos: lib.PluginNodeEnvInfo[] = []) { + static async bootstrap(config: lib.ControllerConfig) { let databaseDirectory = config.get("controller.database_directory"); await fs.mkdir(databaseDirectory, { recursive: true }); @@ -183,8 +175,9 @@ export default class Controller { InstanceRecord.fromJSON.bind(InstanceRecord), this.migrateInstances, this.finaliseInstances, ).bootstrap()); + // Save plugin versions written while loading for (const instance of instances.values()) { - if (lib.setPluginVersions(instance.config, pluginInfos)) { + if (instance.config.dirty) { instances.set(instance); } } diff --git a/packages/host/host.ts b/packages/host/host.ts index f6270dec..a130b5c8 100755 --- a/packages/host/host.ts +++ b/packages/host/host.ts @@ -188,9 +188,6 @@ async function startHost() { ); hostConfig.set("host.version", packageConfig.version); // Allows tracking last loaded version - if (!args.recovery) { - lib.setPluginVersions(hostConfig, pluginInfos); - } if (command === "config") { await lib.handleConfigCommand(args, hostConfig, hostConfigLock); diff --git a/packages/lib/src/config/classes.ts b/packages/lib/src/config/classes.ts index eb20af68..d793bced 100644 --- a/packages/lib/src/config/classes.ts +++ b/packages/lib/src/config/classes.ts @@ -231,6 +231,12 @@ export class Config< */ declare static pluginMigrations?: SyncHook; + /** + * Versions of the plugins whose migrations are attached, keyed by + * plugin name. Written to `.version` after the migrations run. + */ + static pluginVersions = new Map(); + /** Defines the default access locations for the config class */ static defaultAccess: ConfigLocation[] = ["controller", "host", "control"]; @@ -241,10 +247,15 @@ export class Config< /** Set to true during validators to prevent edits */ private _readonly = false; // TODO replace with "using config.asReadonly()" /** Set to true when a field in the config is changed. */ - private dirty = false; + private _dirty = false; /** Set to true when a 'restart required' field in the config is changed. */ restartRequired = false; + /** True if the config has changes that have not been saved. */ + get dirty() { + return this._dirty; + } + /** * Create a new instance of the given config * @@ -282,6 +293,16 @@ export class Config< } this.update(migrated, false, location); } + + for (const [pluginName, version] of this.constructor.pluginVersions) { + const name = `${pluginName}.version` as keyof Fields & string; + if (name in this.fields && this.fields[name] !== version) { + this.fields[name] = version as Fields[typeof name]; + if (fields) { + this._dirty = true; + } + } + } } get filepath() { @@ -289,7 +310,7 @@ export class Config< } set filepath(filepath: string | undefined) { - this.dirty = true; + this._dirty = true; this._filepath = filepath; } @@ -300,7 +321,9 @@ export class Config< // TODO replace with "using config.startStating()" get staging(): Config { if (this._staging) { return this._staging.config; } - const config = new this.constructor(this.location, this.fields); + // Copied with update so migrations don't run again on the live fields + const config = new this.constructor(this.location); + config.update(this.fields, false); const changes = new Set(); config.on("fieldChanged", name => changes.add(name)); this._staging = { config, changes }; @@ -454,7 +477,7 @@ export class Config< if (def.restartRequired) { this.restartRequired = true; } - this.dirty = true; + this._dirty = true; this.emit("fieldChanged", name, value, prev); } } @@ -476,14 +499,14 @@ export class Config< * @param force - Set to true to save the file even if no changes have been made. */ async save(force = false) { - if (!force && !this.dirty) { + if (!force && !this._dirty) { return; } if (!this.filepath) { throw new Error("Cannot save config which has no filepath"); } await safeOutputFile(this.filepath, JSON.stringify(this, null, "\t"), { mode: 0o600 }); - this.dirty = false; + this._dirty = false; } static jsonSchema = ConfigSchema; diff --git a/packages/lib/src/config/definitions.ts b/packages/lib/src/config/definitions.ts index a81c44ef..671eec66 100644 --- a/packages/lib/src/config/definitions.ts +++ b/packages/lib/src/config/definitions.ts @@ -812,10 +812,10 @@ export function addPluginFieldDefinitions( Config.fieldDefinitions[`${pluginInfo.name}.load_plugin`] = fieldDef; - // Version of the plugin the config was last loaded with, see setPluginVersions + // Version of the plugin the config was last loaded with, see Config.pluginVersions Config.fieldDefinitions[`${pluginInfo.name}.version`] = { type: "string", - initialValue: "version" in pluginInfo ? pluginInfo.version : "", + initialValue: "", readonly: ["controller", "host"], hidden: true, }; @@ -845,24 +845,3 @@ export function addPluginConfigFields(pluginInfos: PluginNodeEnvInfo[] | PluginW } } -/** - * Set the version field of each plugin to the version that was loaded - * - * Done after loading a config so the plugin migrations can compare the - * stored version against the one now running. - * - * @param config - Config to update. - * @param pluginInfos - Plugins loaded. - * @returns true if any version changed. - */ -export function setPluginVersions(config: classes.Config, pluginInfos: PluginNodeEnvInfo[]) { - let changed = false; - for (const pluginInfo of pluginInfos) { - const name = `${pluginInfo.name}.version`; - if (config.constructor.fieldDefinitions[name] && config.get(name) !== pluginInfo.version) { - config.set(name, pluginInfo.version); - changed = true; - } - } - return changed; -} diff --git a/packages/lib/src/plugin_loader.ts b/packages/lib/src/plugin_loader.ts index f4ec5276..8194ce7c 100644 --- a/packages/lib/src/plugin_loader.ts +++ b/packages/lib/src/plugin_loader.ts @@ -10,6 +10,7 @@ import * as libErrors from "./errors.js"; import * as libPlugin from "./plugin.js"; import { type Logger, logger } from "./logging.js"; import { loadPluginEntrypoint, loadPluginClass } from "./loadPlugin.js"; +import { Config } from "./config/classes.js"; import { ControllerConfig, HostConfig, InstanceConfig } from "./config/definitions.js"; import Role from "./data/Role.js"; @@ -97,7 +98,8 @@ export async function loadPluginInfos(pluginList: Map) { * * Must be called before configs and roles are loaded for the migrations * to run on them. Loaded for every plugin, including those disabled with - * load_plugin. + * load_plugin. Also registers the plugin versions written to each config + * after the migrations run. * * @param pluginInfos - Plugins to load migrations for. */ @@ -110,6 +112,7 @@ export async function loadPluginMigrations(pluginInfos: libPlugin.PluginNodeEnvI }; for (const pluginInfo of pluginInfos) { + Config.pluginVersions.set(pluginInfo.name, pluginInfo.version); if (!pluginInfo.migrationEntrypoint) { continue; } diff --git a/test/lib/SyncHook.js b/test/lib/SyncHook.js index 414c1a0b..04e3c560 100644 --- a/test/lib/SyncHook.js +++ b/test/lib/SyncHook.js @@ -10,43 +10,56 @@ describe("lib/SyncHook", function() { hook = new lib.SyncHook(); }); - it("should track attached handlers", function() { - assert.equal(hook.size, 0); - hook.attach("alpha", value => value); - hook.attach("beta", value => value); - assert.equal(hook.size, 2); - assert.deepEqual([...hook.attached], ["alpha", "beta"]); + describe(".attach()", function() { + it("should track attached handlers", function() { + assert.equal(hook.size, 0); + hook.attach("alpha", value => value); + hook.attach("beta", value => value); + assert.equal(hook.size, 2); + assert.deepEqual([...hook.attached], ["alpha", "beta"]); + }); + it("should reject duplicate handler names", function() { + hook.attach("alpha", value => value); + assert.throws( + () => hook.attach("alpha", value => value), + new Error("Handler with name alpha is already attached") + ); + }); }); - it("should reject duplicate handler names", function() { - hook.attach("alpha", value => value); - assert.throws( - () => hook.attach("alpha", value => value), - new Error("Handler with name alpha is already attached") - ); + describe(".detach()", function() { + it("should remove the named handler", function() { + hook.attach("alpha", value => value); + hook.attach("beta", value => value); + hook.detach("alpha"); + assert.deepEqual([...hook.attached], ["beta"]); + }); }); - it("should detach and clear handlers", function() { - hook.attach("alpha", value => value); - hook.attach("beta", value => value); - hook.detach("alpha"); - assert.deepEqual([...hook.attached], ["beta"]); - hook.clear(); - assert.equal(hook.size, 0); + describe(".clear()", function() { + it("should remove all handlers", function() { + hook.attach("alpha", value => value); + hook.attach("beta", value => value); + hook.clear(); + assert.equal(hook.size, 0); + }); }); - it("should pass the value through handlers in attach order", function() { - hook.attach("alpha", value => [...value, "alpha"]); - hook.attach("beta", value => [...value, "beta"]); - assert.deepEqual(hook.reduce([]), ["alpha", "beta"]); - }); - it("should return the value unchanged with no handlers", function() { - const value = {}; - assert.equal(hook.reduce(value), value); - }); - it("should rethrow errors as a PluginError named after the handler", function() { - hook.attach("alpha", () => { throw new Error("failed"); }); - assert.throws( - () => hook.reduce(1), - err => err instanceof lib.PluginError && err.pluginName === "alpha" && err.original.message === "failed" - ); + describe(".reduce()", function() { + it("should pass the value through handlers in attach order", function() { + hook.attach("alpha", value => [...value, "alpha"]); + hook.attach("beta", value => [...value, "beta"]); + assert.deepEqual(hook.reduce([]), ["alpha", "beta"]); + }); + it("should return the value unchanged with no handlers", function() { + const value = {}; + assert.equal(hook.reduce(value), value); + }); + it("should rethrow errors as a PluginError named after the handler", function() { + hook.attach("alpha", () => { throw new Error("failed"); }); + assert.throws( + () => hook.reduce(1), + err => err instanceof lib.PluginError && err.pluginName === "alpha" + && err.original.message === "failed" + ); + }); }); }); }); diff --git a/test/lib/config/migrations.js b/test/lib/config/migrations.js index 4861683e..72447983 100644 --- a/test/lib/config/migrations.js +++ b/test/lib/config/migrations.js @@ -45,7 +45,7 @@ describe("lib/config/definitions/migrations", function() { describe("Ctl Config", function() { }); - describe("Plugin migrations", function() { + describe("Plugin Config", function() { const pluginInfo = { name: "test_plugin", version: "1.2.0", @@ -59,6 +59,7 @@ describe("lib/config/definitions/migrations", function() { }); afterEach(function() { lib.InstanceConfig.pluginMigrations.clear(); + lib.Config.pluginVersions.clear(); }); after(function() { for (const Config of [lib.ControllerConfig, lib.HostConfig, lib.InstanceConfig]) { @@ -70,44 +71,81 @@ describe("lib/config/definitions/migrations", function() { } }); - it("should run attached migrations on load", function() { - lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => ( - lib.renameConfigPrefix(config, "old_plugin.", "test_plugin.") - )); - const config = lib.InstanceConfig.fromJSON({ - "old_plugin.load_plugin": false, - "old_plugin.delay": 5, - }, "controller"); - assert.equal(config.get("test_plugin.load_plugin"), false); - assert.equal(config.get("test_plugin.delay"), 5); - assert.equal(config.toJSON()["old_plugin.delay"], undefined); - }); - it("should run plugin migrations after core migrations", function() { - lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { - assert.equal(config["factorio.sync_adminlist"], "enabled"); - return config; + describe("static .pluginMigrations", function() { + it("should run on load", function() { + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => ( + lib.renameConfigPrefix(config, "old_plugin.", "test_plugin.") + )); + const config = lib.InstanceConfig.fromJSON({ + "old_plugin.load_plugin": false, + "old_plugin.delay": 5, + }, "controller"); + assert.equal(config.get("test_plugin.load_plugin"), false); + assert.equal(config.get("test_plugin.delay"), 5); + assert.equal(config.toJSON()["old_plugin.delay"], undefined); }); - lib.InstanceConfig.fromJSON({ "factorio.sync_adminlist": true }, "controller"); - }); - it("should give migrations the version the config was saved with", function() { - let seen; - lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { - seen = config["test_plugin.version"]; - return config; + it("should run after core migrations", function() { + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { + assert.equal(config["factorio.sync_adminlist"], "enabled"); + return config; + }); + lib.InstanceConfig.fromJSON({ "factorio.sync_adminlist": true }, "controller"); + }); + it("should see the version the config was saved with", function() { + let seen; + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { + seen = config["test_plugin.version"]; + return config; + }); + lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); + assert.equal(seen, "1.1.0"); + }); + it("should not run again when the config is changed", function() { + lib.Config.pluginVersions.set("test_plugin", "1.2.0"); + let calls = 0; + lib.InstanceConfig.pluginMigrations.attach("test_plugin", config => { + calls += 1; + if (config["test_plugin.version"] === "1.1.0") { + config["test_plugin.delay"] *= 60; + } + return config; + }); + const config = lib.InstanceConfig.fromJSON({ + "test_plugin.version": "1.1.0", + "test_plugin.delay": 2, + }, "controller"); + config.set("instance.name", "changed"); + assert.equal(config.get("test_plugin.delay"), 120); + assert.equal(calls, 1); }); - const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); - assert.equal(seen, "1.1.0"); - assert.equal(config.get("test_plugin.version"), "1.1.0"); }); - it("should default the version of new configs to the loaded plugin version", function() { - const config = new lib.InstanceConfig("controller"); - assert.equal(config.get("test_plugin.version"), "1.2.0"); - }); - it("should update versions with setPluginVersions", function() { - const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); - assert.equal(lib.setPluginVersions(config, [pluginInfo]), true); - assert.equal(config.get("test_plugin.version"), "1.2.0"); - assert.equal(lib.setPluginVersions(config, [pluginInfo]), false); + describe("static .pluginVersions", function() { + it("should write the version after migrations run", function() { + lib.Config.pluginVersions.set("test_plugin", "1.2.0"); + const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.1.0" }, "controller"); + assert.equal(config.get("test_plugin.version"), "1.2.0"); + assert.equal(config.dirty, true); + }); + it("should mark configs saved without a version as dirty", function() { + lib.Config.pluginVersions.set("test_plugin", "1.2.0"); + const config = lib.InstanceConfig.fromJSON({}, "controller"); + assert.equal(config.get("test_plugin.version"), "1.2.0"); + assert.equal(config.dirty, true); + }); + it("should not mark configs already at the version as dirty", function() { + lib.Config.pluginVersions.set("test_plugin", "1.2.0"); + const config = lib.InstanceConfig.fromJSON({ "test_plugin.version": "1.2.0" }, "controller"); + assert.equal(config.dirty, false); + }); + it("should write the version to new configs", function() { + lib.Config.pluginVersions.set("test_plugin", "1.2.0"); + const config = new lib.InstanceConfig("controller"); + assert.equal(config.get("test_plugin.version"), "1.2.0"); + }); + it("should leave the version empty when no versions are registered", function() { + const config = new lib.InstanceConfig("controller"); + assert.equal(config.get("test_plugin.version"), ""); + }); }); }); }); diff --git a/test/lib/plugin_loader.js b/test/lib/plugin_loader.js index c1b4c740..2f426567 100644 --- a/test/lib/plugin_loader.js +++ b/test/lib/plugin_loader.js @@ -128,11 +128,13 @@ describe("lib/plugin_loader", function() { }); afterEach(function() { lib.ControllerConfig.pluginMigrations.clear(); + lib.Config.pluginVersions.clear(); }); function info(entrypoint) { return { name: "test", + version: "1.0.0", packagePath: path.join(baseDir, "package.json"), migrationEntrypoint: entrypoint, }; @@ -143,6 +145,10 @@ describe("lib/plugin_loader", function() { assert.deepEqual([...lib.ControllerConfig.pluginMigrations.attached], ["test"]); assert.deepEqual(lib.ControllerConfig.pluginMigrations.reduce({}), { migratedBy: "test" }); }); + it("should register the version of every plugin", async function() { + await lib.loadPluginMigrations([info("migrations.js"), { name: "other", version: "2.0.0" }]); + assert.deepEqual([...lib.Config.pluginVersions], [["test", "1.0.0"], ["other", "2.0.0"]]); + }); it("should reject with a PluginError if the entrypoint throws", async function() { await assert.rejects( lib.loadPluginMigrations([info("throwing.js")]),