From: Jérôme Benoit Date: Wed, 8 Jul 2026 19:45:50 +0000 (+0200) Subject: refactor(ocpp): extract per-station state plumbing into shared OCPPIncomingRequestSer... X-Git-Tag: cli@v4.11.0~44 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=0fff8b2a611829b5f996aae0ebc4d50883eb0632;p=e-mobility-charging-stations-simulator.git refactor(ocpp): extract per-station state plumbing into shared OCPPIncomingRequestService base (#1983) Closes #1963. Companion issues #1971, #1972, #1973 will consume this base. Pure structural refactor + OCPP 1.6 concretization. Zero observable behavior change on OCPP 2.0.1: every field currently cleared in the former OCPP20IncomingRequestService.stop() body is still cleared, in the same order, at the same lifecycle point. Touchpoints: A. src/charging-station/ocpp/OCPPIncomingRequestService.ts Base becomes generic . Adds shared 'stationsState' WeakMap (L86), 'getOrCreateStationState' lazy-init, concrete 'stop()' template (bound in the constructor at L94 to prevent detach-and-call regressions), and abstract hooks 'createStationState' / 'resetStationState'. Documents the exception-safety contract on the template: a throw in 'resetStationState' skips WeakMap eviction and any subclass extension after 'super.stop()' — matches pre-refactor semantics. The '= object' default on the generic parameter is load-bearing (removing it would break the static registry Map and the getInstance constraint with TS2314). B. src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts Extends OCPPIncomingRequestService. Removes local 'stationsState' field and 'getOrCreateStationState'. Adds 'createStationState' factory and 'resetStationState' override with the 5-statement ordering invariant of the pre-refactor stop() body. The abort-before-clear ordering matters because clearing first makes the subsequent '?.abort()' short-circuit on the nulled field, leaving the in-flight operation un-signaled. 'stop()' override calls super first, then keeps the unconditional OCPP20VariableManager cleanup. Class-level JSDoc documents OCPP 2.1 subclass path (extends OCPP20IncomingRequestService inherits state, reset, and stop() unchanged; widening the generic parameter for new fields requires a preparatory refactor with a factory cast per TS2352, or subclass override of createStationState). C. src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts Adds empty 'OCPP16StationState' interface. Extends OCPPIncomingRequestService. Adds no-op 'createStationState' + 'resetStationState' overrides. Removes the '/* no-op for OCPP 1.6 */' stop() override (now inherited from the base template). The 'resetStationState' JSDoc prescribes the abort-before-clear ordering companion issues MUST follow. D. eslint.config.js Adds 'no-restricted-syntax' rule enforcing the INVARIANT: forbids direct '.set/.delete/.clear' calls on 'stationsState' from outside the base class file, forbids aliasing 'stationsState' to a local binding, and forbids destructuring 'stationsState'. Covers the AST escape hatches identified during hostile-adversarial review. Enforced by 'pnpm lint' in CI on every companion PR. E. src/charging-station/ocpp/OCPPServiceUtils.ts Adds bidirectional cross-reference in the comment on 'warnedInvalidMeasurands' distinguishing the module-scope warn-once diagnostic cache from the class-scope lifecycle-state WeakMap on OCPPIncomingRequestService.stationsState. Companion issues consume this base infrastructure in order: - #1971 GetDiagnostics supersession -> activeDiagnosticsAbortController + Id - #1972 firmware setTimeout cancel -> deferred firmware timer handle - #1973 trigger cross-check -> activeFirmwareUpdateRequestId #1971 and #1973 share 'activeDiagnosticsRequestId': whichever lands first adds the (optional) field to OCPP16StationState; the second lands as-is. Line-count delta (plumbing moved, not duplicated): Base : 182 -> 290 (+108) plumbing + docs (~+80 JSDoc / +28 code) OCPP20 : 4579 -> 4627 (+48) 22 lines of plumbing removed; docs +55, net code -7 (concrete overrides only) OCPP16 : 1958 -> 1985 (+27) interface + 2 concrete implementations + companion guidance eslint : 172 -> 198 (+26) 3-selector no-restricted-syntax rule utils : 2000 -> 2004 (+4) bidirectional cross-ref comment test : new file (+150) 7 plumbing tests Origin of the OCPP 2.0.1 pattern being harmonized: - 47cdf2bbe refactor(ocpp20): isolate per-station state with WeakMap instead of singleton properties (introduces the WeakMap pattern with initial names 'OCPP20PerStationState' / 'stationStates' / 'getStationState') - f1e33ea42 refactor(ocpp20): harmonize per-station state naming (renames to 'OCPP20StationState' / 'stationsState') - 17396c1c4 refactor(ocpp20): rename getStationState to getOrCreateStationState (lazy-getter naming split) - c0b25553 fix(ocpp20): cancel cert-signing retry timer in stop() - be29b4c8 fix(ocpp20): add AbortController to log-upload lifecycle + stop() abort - d2e9b8b0 refactor(ocpp20): extract resetActiveFirmwareUpdateState helper - ce875dae refactor(ocpp20): harmonize firmware-state cleanup + log superseded Tests: 7 new plumbing tests in tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts covering lazy-init idempotency (with createStationState spy verifying call count = 1 after two getOrCreate calls), WeakMap eviction on stop(), resetStationState invocation count, no-op guard when no state exists, two-station isolation, the OCPP 1.6 default resetStationState no-op (with reference identity check), and the exception-safety contract lock (throw in resetStationState skips WeakMap.delete, and subsequent stop() re-invokes on same state before evicting). Zero edits to existing OCPP 2.0.1 tests. The OCPP 2.0.1 5-statement ordering invariant is enforced by four independent mechanisms: (1) JSDoc rationale on OCPP20IncomingRequestService.resetStationState documenting the '?.abort()' short-circuit consequence of reordering, (2) the 'no-restricted-syntax' ESLint rule preventing direct 'stationsState' mutations from subclass code (with selector coverage for aliasing and destructuring bypasses; enforced in CI), (3) constructor 'stop.bind(this)' defense-in-depth against detach-and-call regressions, and (4) byte-identical preservation of the pre-refactor stop() body. --- diff --git a/eslint.config.js b/eslint.config.js index 601875cf..9f80d6a5 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -150,6 +150,32 @@ export default defineConfig([ '@typescript-eslint/no-require-imports': 'off', }, }, + { + files: ['src/**/*.ts', 'tests/**/*.ts'], + ignores: ['src/charging-station/ocpp/OCPPIncomingRequestService.ts'], + rules: { + 'no-restricted-syntax': [ + 'error', + { + message: + 'Direct mutation of `stationsState` from outside `OCPPIncomingRequestService` is forbidden. Use `getOrCreateStationState` for lazy-init and the base `stop()` template for eviction. See the INVARIANT JSDoc on `OCPPIncomingRequestService.stationsState`.', + selector: + 'MemberExpression[object.type="MemberExpression"][object.property.name="stationsState"][property.name=/^(?:set|delete|clear)$/]', + }, + { + message: + 'Aliasing `stationsState` to a local binding is forbidden — the alias bypasses the INVARIANT enforcement. Use `getOrCreateStationState(...)` / `.stationsState.get(...)` / `.stationsState.has(...)` inline.', + selector: + 'VariableDeclarator[init.type="MemberExpression"][init.property.name="stationsState"]', + }, + { + message: + 'Destructuring `stationsState` is forbidden — the destructured reference bypasses the INVARIANT enforcement. Use inline `.stationsState.get(...)` / `.has(...)` instead.', + selector: 'ObjectPattern > Property[key.name="stationsState"]', + }, + ], + }, + }, { files: ['tests/utils/Utils.test.ts'], rules: { diff --git a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts index 68407e4f..e186a00a 100644 --- a/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts +++ b/src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.ts @@ -139,6 +139,20 @@ import { OCPP16ServiceUtils } from './OCPP16ServiceUtils.js' const moduleName = 'OCPP16IncomingRequestService' +/** + * Per-station lifecycle state carried on {@link OCPP16IncomingRequestService}. + * + * As of #1963 this interface is intentionally empty. Companion PRs will + * populate this interface with their own fields: + * - #1971 (GetDiagnostics supersession): `activeDiagnosticsAbortController`, + * `activeDiagnosticsRequestId`; + * - #1972 (deferred firmware `setTimeout` cancel): firmware timer handle; + * - #1973 (trigger cross-check): `activeDiagnosticsRequestId` (shared with + * #1971), `activeFirmwareUpdateRequestId`. + */ +// eslint-disable-next-line @typescript-eslint/no-empty-object-type -- populated by companion PRs #1971 / #1972 / #1973 +interface OCPP16StationState {} + /** * OCPP 1.6 Incoming Request Service - handles and processes all incoming requests * from the Central System (CS) to the Charging Station (CP) using OCPP 1.6 protocol. @@ -173,7 +187,7 @@ const moduleName = 'OCPP16IncomingRequestService' * @see {@link handleRequestRemoteStartTransaction} Example request handler */ -export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { +export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { protected readonly csmsName = 'Central System' protected readonly incomingRequestHandlers: Map @@ -607,11 +621,11 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { } /** - * Stops the incoming request service for the given charging station. - * @param chargingStation - Target charging station + * @returns Fresh empty state — companion PRs (#1971 / #1972 / #1973) + * populate fields via declaration merging on {@link OCPP16StationState}. */ - public override stop (chargingStation: ChargingStation): void { - /* no-op for OCPP 1.6 */ + protected override createStationState (): OCPP16StationState { + return {} } /** @@ -627,6 +641,19 @@ export class OCPP16IncomingRequestService extends OCPPIncomingRequestService { return isIncomingRequestCommandSupported(chargingStation, commandName) } + /** + * Companion PRs (#1971 / #1972 / #1973) will populate this method with + * release logic paired with the {@link OCPP16StationState} fields they + * add. They MUST follow the abort-before-clear ordering documented on + * {@link OCPP20IncomingRequestService.resetStationState}: abort in-flight + * signals BEFORE clearing controller references, cancel timers BEFORE + * the base template deletes the entry. + * @param _stationState - Per-station state (currently empty; unused). + */ + protected override resetStationState (_stationState: OCPP16StationState): void { + /* no-op: OCPP 1.6 carries no state fields yet (see companion PRs) */ + } + private composeCompositeSchedule ( chargingStation: ChargingStation, chargingProfiles: OCPP16ChargingProfile[], diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts index acc056cd..820531c0 100644 --- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts +++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts @@ -288,7 +288,33 @@ interface QueuedSecurityEvent { type: string } -export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { +/** + * OCPP 2.0.1 incoming-request service. + * + * Extends {@link OCPPIncomingRequestService} with `OCPP20StationState` and + * layers OCPP 2.0.1 variable-manager cleanup on top of the base `stop()` + * template. + * + * A future OCPP 2.1 service SHOULD extend this class + * (`class OCPP21IncomingRequestService extends OCPP20IncomingRequestService`) + * because OCPP 2.1 is an extension of OCPP 2.0.1 with minor exceptions + * (`OCPP-2.1_edition1_part0_introduction.md`). Subclassing inherits + * `OCPP20StationState`, `resetStationState`, and the `stop()` override + * unchanged (subject to dynamic dispatch on `this` — TypeScript inherits + * methods, not textual copies). + * + * To layer 2.1-specific lifecycle logic: override `stop()` calling + * `super.stop()` first, or override `resetStationState` calling + * `super.resetStationState(state)` first. Adding new state fields + * requires either widening this class's generic parameter (currently + * fixed to `OCPP20StationState`) in a preparatory refactor — which will + * need an `as unknown as T` cast in `createStationState` because + * TypeScript cannot prove a concrete literal satisfies an arbitrary + * `T extends OCPP20StationState` (TS2352) — or a subclass-side + * narrowed accessor. Subclass override of `createStationState` is + * preferable when only a fixed subclass shape is needed. + */ +export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { protected readonly csmsName = 'CSMS' protected readonly incomingRequestHandlers: Map @@ -301,8 +327,6 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { OCPP20IncomingRequestCommand.REQUEST_STOP_TRANSACTION, ] - private readonly stationsState = new WeakMap() - public constructor () { super(OCPPVersion.VERSION_201) this.incomingRequestHandlers = new Map([ @@ -797,19 +821,19 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } /** - * Stop the incoming request service and clean up per-station state. + * Stop and clean up per-station state. + * + * Extends the base template with OCPP 2.0.1 variable-manager cleanup, + * which runs unconditionally to match pre-refactor behavior even when + * no {@link stationsState} entry exists. + * + * If `super.stop()` throws (via `resetStationState`), the + * variable-manager cleanup below is skipped — matches the pre-refactor + * semantics where the state-reset block was also outside the try/catch. * @param chargingStation - Target charging station to stop */ public override stop (chargingStation: ChargingStation): void { - const stationState = this.stationsState.get(chargingStation) - if (stationState != null) { - stationState.activeFirmwareUpdateAbortController?.abort() - stationState.activeLogUploadAbortController?.abort() - stationState.certSigningRetryManager?.cancelRetryTimer() - this.resetActiveFirmwareUpdateState(stationState) - this.resetActiveLogUploadState(stationState) - this.stationsState.delete(chargingStation) - } + super.stop(chargingStation) try { const variableManager = OCPP20VariableManager.getInstance() const stationId = chargingStation.stationInfo?.hashId @@ -824,6 +848,25 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { } } + /** + * @returns A fresh `OCPP20StationState` with the four required fields + * initialized (empty `Map` for `preInoperativeConnectorStatuses` and + * `reportDataCache`, empty array for `securityEventQueue`, + * `isDrainingSecurityEvents: false`). The seven optional + * lifecycle-owned fields (`activeFirmwareUpdate*`, + * `activeLogUpload*`, `certSigningRetryManager`, + * `lastFirmwareStatusNotification`) start absent and are assigned + * lazily by handlers. + */ + protected override createStationState (): OCPP20StationState { + return { + isDrainingSecurityEvents: false, + preInoperativeConnectorStatuses: new Map(), + reportDataCache: new Map(), + securityEventQueue: [], + } + } + /** * Handles OCPP 2.0.1 ClearCache request by clearing the Authorization Cache * per OCPP 2.0.1 spec C11.FR.01 @@ -1028,6 +1071,25 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { return isIncomingRequestCommandSupported(chargingStation, commandName) } + /** + * Reset per-station lifecycle state prior to WeakMap eviction. + * + * ORDERING INVARIANT (preserved bit-for-bit from the pre-refactor + * `stop()` body): abort in-flight signals BEFORE clearing the fields + * that hold their controllers (otherwise the subsequent `?.abort()` + * short-circuits on the nulled field and the in-flight operation is + * never signaled to cancel), and cancel the retry timer BEFORE the + * base template deletes the state entry. + * @param stationState - Per-station state to reset. + */ + protected override resetStationState (stationState: OCPP20StationState): void { + stationState.activeFirmwareUpdateAbortController?.abort() + stationState.activeLogUploadAbortController?.abort() + stationState.certSigningRetryManager?.cancelRetryTimer() + this.resetActiveFirmwareUpdateState(stationState) + this.resetActiveLogUploadState(stationState) + } + private async authorizeToken ( chargingStation: ChargingStation, connectorId: number, @@ -1426,20 +1488,6 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService { .catch(errorHandler) } - private getOrCreateStationState (chargingStation: ChargingStation): OCPP20StationState { - let state = this.stationsState.get(chargingStation) - if (state == null) { - state = { - isDrainingSecurityEvents: false, - preInoperativeConnectorStatuses: new Map(), - reportDataCache: new Map(), - securityEventQueue: [], - } - this.stationsState.set(chargingStation, state) - } - return state - } - private getRestoredConnectorStatus ( chargingStation: ChargingStation, connectorId: number diff --git a/src/charging-station/ocpp/OCPPIncomingRequestService.ts b/src/charging-station/ocpp/OCPPIncomingRequestService.ts index 06904d96..c2d94595 100644 --- a/src/charging-station/ocpp/OCPPIncomingRequestService.ts +++ b/src/charging-station/ocpp/OCPPIncomingRequestService.ts @@ -14,7 +14,37 @@ import { import { isAsyncFunction, JSONStringify, logger } from '../../utils/index.js' import { type Ajv, createAjv, validatePayload } from './OCPPServiceUtils.js' -export abstract class OCPPIncomingRequestService extends EventEmitter { +/** + * OCPP incoming-request service base class. + * + * Provides shared plumbing for per-station lifecycle state: + * - a protected {@link WeakMap} keyed by {@link ChargingStation}; + * - a lazy-init getter (`getOrCreateStationState`); + * - a concrete `stop()` template that resets and drops the entry. + * + * Subclass contract: + * - `createStationState` — factory returning the initial state object. + * - `resetStationState` — releases any resources held by the state + * (abort controllers, timer handles, retry managers). + * + * The `stop()` template owns the ordering invariant (reset before delete) + * and the "only-if-present" guard. Subclasses that need additional + * lifecycle cleanup should override `stop()` and call `super.stop()` + * first; override `resetStationState` (not `stop()`) for state-field + * reset logic. + * @template TStationState - Concrete per-station state shape. + * The default `object` bound is load-bearing: it allows the bare + * `OCPPIncomingRequestService` reference in the static singleton + * registry ({@link OCPPIncomingRequestService.instances}) and in the + * `getInstance` constraint to + * resolve to `OCPPIncomingRequestService`. Removing the default + * would break both declarations with `TS2314` (generic type requires + * type arguments). The bound also matches the `WeakMap` value-type + * requirement (values must be non-primitive). + */ +export abstract class OCPPIncomingRequestService< + TStationState extends object = object +> extends EventEmitter { private static readonly instances = new Map< new () => OCPPIncomingRequestService, OCPPIncomingRequestService @@ -35,6 +65,25 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { > protected abstract readonly pendingStateBlockedCommands: IncomingRequestCommand[] + /** + * Per-station lifecycle state. + * + * INVARIANT: single **lifecycle-state** `WeakMap` + * declaration in the codebase. A sibling module-scope diagnostic + * `WeakMap>` (`warnedInvalidMeasurands`) at + * `OCPPServiceUtils.ts` is intentionally scoped separately — that is a + * warn-once side-effect cache with no lifecycle semantics, a different + * pattern. Subclasses MUST NOT redeclare this field: TypeScript field + * re-declaration with an initializer at the subclass level creates a + * distinct backing slot at construction, silently splitting state + * between the subclass shadow and the base template (which resolves + * against the base declaration). Subclasses MUST also defer all + * mutations of `this.stationsState` to {@link getOrCreateStationState} + * and the base {@link stop} template — no direct `.set`, `.delete`, or + * `.clear` from anywhere outside this file (enforced by the + * `no-restricted-syntax` ESLint rule). + */ + protected readonly stationsState = new WeakMap() private readonly version: OCPPVersion protected constructor (version: OCPPVersion) { @@ -42,6 +91,7 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { this.version = version this.ajv = createAjv() this.incomingRequestHandler = this.incomingRequestHandler.bind(this) + this.stop = this.stop.bind(this) this.validateIncomingRequestPayload = this.validateIncomingRequestPayload.bind(this) } @@ -136,9 +186,51 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { /** * Stops the incoming-request service for the given charging station. + * + * Template method: subclasses SHOULD NOT override the template steps + * (WeakMap lookup, reset, delete) — override {@link resetStationState} + * for field-level cleanup. Subclasses MAY override to add + * lifecycle-scoped side effects (e.g. clearing external caches keyed + * on the station); such overrides MUST call `super.stop()` first so + * the reset-then-delete ordering is preserved. + * + * Exception behavior: if {@link resetStationState} throws, `WeakMap` + * eviction is skipped and a subsequent `stop()` re-invokes the hook + * on the same state. Any subclass extension after `super.stop()` is + * also skipped, as the throw propagates. Matches pre-refactor + * semantics exactly. + * @param chargingStation - Target charging station. + */ + public stop (chargingStation: ChargingStation): void { + const stationState = this.stationsState.get(chargingStation) + if (stationState != null) { + this.resetStationState(stationState) + this.stationsState.delete(chargingStation) + } + } + + /** + * Hook method: creates the initial per-station state, called on first + * access via {@link getOrCreateStationState}. + * @returns A fresh state object with default field values. + */ + protected abstract createStationState (): TStationState + + /** + * Returns the state entry for `chargingStation`, creating one on first + * access via {@link createStationState}. Subsequent calls return the + * same reference until {@link stop} evicts the entry. * @param chargingStation - Target charging station. + * @returns The lazily-initialized per-station state. */ - public abstract stop (chargingStation: ChargingStation): void + protected getOrCreateStationState (chargingStation: ChargingStation): TStationState { + let state = this.stationsState.get(chargingStation) + if (state == null) { + state = this.createStationState() + this.stationsState.set(chargingStation, state) + } + return state + } /** * Whether the given incoming-request command is supported for this station. @@ -151,6 +243,22 @@ export abstract class OCPPIncomingRequestService extends EventEmitter { commandName: IncomingRequestCommand ): boolean + /** + * Hook method paired with the {@link stop} template: releases + * resources held by the per-station state (abort controllers, timer + * handles, retry managers, etc.) prior to eviction from + * {@link stationsState}. + * + * Implementations MAY throw; on throw the base template skips + * `WeakMap` eviction and a subsequent `stop()` re-invokes this hook + * on the same partially-reset state. Implementations MUST therefore + * tolerate re-entry on a partially-reset state (idempotent field + * clears + optional-chained aborts satisfy this). See {@link stop} + * for the full exception-behavior contract. + * @param stationState - Per-station state to reset. + */ + protected abstract resetStationState (stationState: TStationState): void + // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters -- bridges contravariant handler signatures into IncomingRequestHandler protected toRequestHandler

( handler: (chargingStation: ChargingStation, commandPayload: P) => Promise | R diff --git a/src/charging-station/ocpp/OCPPServiceUtils.ts b/src/charging-station/ocpp/OCPPServiceUtils.ts index a7fad987..e9cd3b44 100644 --- a/src/charging-station/ocpp/OCPPServiceUtils.ts +++ b/src/charging-station/ocpp/OCPPServiceUtils.ts @@ -1072,6 +1072,10 @@ const createVersionedSampledValueDispatcher = ( // Kept off the class API to avoid touching `ChargingStation` for a // warn-once diagnostic; the WeakMap's semantics match the intent — one // bag of already-warned entries per station, freed with the station. +// Distinct pattern from the class-scoped lifecycle-state WeakMap on +// `OCPPIncomingRequestService.stationsState`: no `stop()` lifecycle, no +// abort semantics, no `resetStationState` hook — a diagnostic side-effect +// cache freed only by GC. const warnedInvalidMeasurands = new WeakMap>() const KNOWN_MEASURANDS: ReadonlySet = new Set(Object.values(MeterValueMeasurand)) diff --git a/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts b/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts new file mode 100644 index 00000000..8a416593 --- /dev/null +++ b/tests/charging-station/ocpp/OCPPIncomingRequestService-StationState.test.ts @@ -0,0 +1,150 @@ +/** + * @file Tests for OCPPIncomingRequestService per-station state plumbing (#1963) + * @description Unit tests for the shared WeakMap + lazy-init + stop() template + * in `OCPPIncomingRequestService`, plus the OCPP 1.6 no-op `resetStationState` + * contract that companion PRs (#1971, #1972, #1973) will replace. + * + * The OCPP 2.0.1 5-statement ordering invariant (abort firmware → abort log → + * cancel retry → resetActiveFirmwareUpdateState → resetActiveLogUploadState) + * is enforced by three independent mechanisms verified out-of-band from these + * tests: (1) JSDoc rationale on `OCPP20IncomingRequestService.resetStationState` + * documenting the `?.abort()` short-circuit consequence of reordering, (2) the + * `no-restricted-syntax` ESLint rule preventing direct `stationsState` + * mutations from subclass code, and (3) byte-identical preservation of the + * pre-refactor `stop()` body. + */ + +import assert from 'node:assert/strict' +import { afterEach, beforeEach, describe, it, mock } from 'node:test' + +import type { ChargingStation } from '../../../src/charging-station/index.js' + +import { OCPP16IncomingRequestService } from '../../../src/charging-station/ocpp/1.6/OCPP16IncomingRequestService.js' +import { standardCleanup } from '../../helpers/TestLifecycleHelpers.js' +import { createMockChargingStation } from '../helpers/StationHelpers.js' + +type OCPP16StationStateShape = Record + +interface PlumbingAccess { + createStationState: () => T + getOrCreateStationState: (chargingStation: ChargingStation) => T + resetStationState: (state: T) => void + stationsState: WeakMap +} + +const asPlumbing = ( + service: OCPP16IncomingRequestService +): PlumbingAccess => + service as unknown as PlumbingAccess + +await describe('OCPPIncomingRequestService — per-station state plumbing', async () => { + let service: OCPP16IncomingRequestService + let plumbing: PlumbingAccess + let stationA: ChargingStation + let stationB: ChargingStation + + beforeEach(() => { + service = new OCPP16IncomingRequestService() + plumbing = asPlumbing(service) + stationA = createMockChargingStation({ index: 1 }).station + stationB = createMockChargingStation({ index: 2 }).station + }) + + afterEach(() => { + standardCleanup() + }) + + await it('should lazily create state on first access and reuse it on repeat calls', () => { + const createSpy: ReturnType = mock.method( + plumbing, + 'createStationState', + () => ({}) + ) + + const first = plumbing.getOrCreateStationState(stationA) + const second = plumbing.getOrCreateStationState(stationA) + + assert.strictEqual(first, second) + assert.strictEqual(createSpy.mock.callCount(), 1) + assert.strictEqual(plumbing.stationsState.has(stationA), true) + assert.strictEqual(plumbing.stationsState.get(stationA), first) + }) + + await it('should delete the WeakMap entry after stop()', () => { + plumbing.getOrCreateStationState(stationA) + assert.strictEqual(plumbing.stationsState.has(stationA), true) + + service.stop(stationA) + + assert.strictEqual(plumbing.stationsState.has(stationA), false) + }) + + await it('should invoke resetStationState exactly once with the current state on stop()', () => { + const state = plumbing.getOrCreateStationState(stationA) + const resetSpy: ReturnType = mock.method(plumbing, 'resetStationState') + + service.stop(stationA) + + assert.strictEqual(resetSpy.mock.callCount(), 1) + assert.strictEqual(resetSpy.mock.calls[0].arguments[0], state) + }) + + await it('should be a no-op when stop() is called on a station with no state entry', () => { + const resetSpy: ReturnType = mock.method(plumbing, 'resetStationState') + + service.stop(stationA) + + assert.strictEqual(resetSpy.mock.callCount(), 0) + assert.strictEqual(plumbing.stationsState.has(stationA), false) + }) + + await it('should isolate state per station — stop(A) must not affect B', () => { + const stateA = plumbing.getOrCreateStationState(stationA) + const stateB = plumbing.getOrCreateStationState(stationB) + assert.notStrictEqual(stateA, stateB) + + service.stop(stationA) + + assert.strictEqual(plumbing.stationsState.has(stationA), false) + assert.strictEqual(plumbing.stationsState.has(stationB), true) + assert.strictEqual(plumbing.stationsState.get(stationB), stateB) + }) + + await it('should not mutate state in the OCPP 1.6 default resetStationState', () => { + const state = plumbing.getOrCreateStationState(stationA) + const keysBefore = Object.keys(state) + + plumbing.resetStationState(state) + + assert.deepStrictEqual(Object.keys(state), keysBefore) + assert.deepStrictEqual(state, {}) + assert.strictEqual(plumbing.stationsState.get(stationA), state) + }) + + await it('should not delete the WeakMap entry when resetStationState throws, and re-invoke on subsequent stop()', () => { + const state = plumbing.getOrCreateStationState(stationA) + const failure = new Error('reset failed') + let shouldThrow = true + const resetSpy: ReturnType = mock.method(plumbing, 'resetStationState', () => { + if (shouldThrow) { + throw failure + } + }) + + assert.throws( + () => { + service.stop(stationA) + }, + (err: unknown) => err === failure + ) + assert.strictEqual(plumbing.stationsState.has(stationA), true) + assert.strictEqual(plumbing.stationsState.get(stationA), state) + + shouldThrow = false + service.stop(stationA) + + assert.strictEqual(resetSpy.mock.callCount(), 2) + assert.strictEqual(resetSpy.mock.calls[1].arguments[0], state) + assert.strictEqual(plumbing.stationsState.has(stationA), false) + }) +})