diff --git a/src/lib/model/ControllerStorage.spec.ts b/src/lib/model/ControllerStorage.spec.ts new file mode 100644 index 000000000..5cecbd659 --- /dev/null +++ b/src/lib/model/ControllerStorage.spec.ts @@ -0,0 +1,78 @@ +import { Accessory } from "../Accessory"; +import { ControllerIdentifier, ControllerServiceMap, SerializableController, StateChangeDelegate } from "../controller"; +import * as uuid from "../util/uuid"; +import { ControllerStorage } from "./ControllerStorage"; + +interface State { + first?: string; + second?: string; +} + +// restores its state in two steps and reports a state change in between, like a controller whose +// characteristic updates during deserialize trigger its own change listeners +class SteppedController implements SerializableController { + state: State = {}; + private stateChangeDelegate?: StateChangeDelegate; + + controllerId(): ControllerIdentifier { + return "stepped"; + } + + constructServices(): ControllerServiceMap { + return {}; + } + + initWithServices(): void | ControllerServiceMap { + return undefined; + } + + configureServices(): void { + // nothing to configure + } + + handleControllerRemoved(): void { + // nothing to clean up + } + + serialize(): State | undefined { + return { ...this.state }; + } + + deserialize(serialized: State): void { + this.state.first = serialized.first; + this.stateChangeDelegate?.(); + this.state.second = serialized.second; + } + + setupStateChangeDelegate(delegate?: StateChangeDelegate): void { + this.stateChangeDelegate = delegate; + } + + handleFactoryReset(): void { + this.state = {}; + } +} + +describe("ControllerStorage", () => { + test("state changes reported while restoring do not overwrite the stored data", () => { + const accessory = new Accessory("Test", uuid.generate("controller-storage")); + const storage = new ControllerStorage(accessory); + const controller = new SteppedController(); + storage.trackController(controller); + + // @ts-expect-error: private access + storage.init([{ type: "stepped", controllerData: { data: { first: "a", second: "b" } } }]); + + expect(controller.state).toEqual({ first: "a", second: "b" }); + // @ts-expect-error: private access + expect(storage.controllerData.stepped.data).toEqual({ first: "a", second: "b" }); + + controller.state.second = "c"; + // @ts-expect-error: private access + storage.handleStateChange(controller); + // @ts-expect-error: private access + expect(storage.controllerData.stepped.data).toEqual({ first: "a", second: "c" }); + // @ts-expect-error: private access + clearTimeout(storage.queuedSaveTimeout); + }); +}); diff --git a/src/lib/model/ControllerStorage.ts b/src/lib/model/ControllerStorage.ts index 86c315291..cf6d5b820 100644 --- a/src/lib/model/ControllerStorage.ts +++ b/src/lib/model/ControllerStorage.ts @@ -47,6 +47,11 @@ export class ControllerStorage { private controllerData: Record = {}; private restoredAccessories?: Record; // indexed by accessory UUID + // deserializing a controller updates its characteristics, which can report state changes (its own, or + // those of another controller listening on a shared service) before the restore is complete. + // Those must not overwrite the stored data with a partially restored state. + private restoring = false; + private parent?: ControllerStorage; private linkedAccessories?: ControllerStorage[]; @@ -133,6 +138,9 @@ export class ControllerStorage { } private handleStateChange(controller: SerializableController) { + if (this.restoring) { + return; + } const id = controller.controllerId(); const serialized = controller.serialize(); @@ -174,11 +182,15 @@ export class ControllerStorage { this.accessoryUUID, controller.controllerId(), !!controllerData); if (controllerData) { + const wasRestoring = this.restoring; + this.restoring = true; try { controller.deserialize(controllerData.data); } catch (error) { console.warn(`Could not initialize controller of type '${controller.controllerId()}' from data stored on disk. Resetting to default: ${error.stack}`); controller.handleFactoryReset(); + } finally { + this.restoring = wasRestoring; } controllerData.purgeOnNextLoad = undefined; } @@ -202,10 +214,15 @@ export class ControllerStorage { } const restoredControllers: ControllerIdentifier[] = []; - this.trackedControllers.forEach(controller => { - this.restoreController(controller); - restoredControllers.push(controller.controllerId()); - }); + this.restoring = true; + try { + this.trackedControllers.forEach(controller => { + this.restoreController(controller); + restoredControllers.push(controller.controllerId()); + }); + } finally { + this.restoring = false; + } this.trackedControllers.splice(0, this.trackedControllers.length); // clear tracking list let purgedData = false;