fix(atg): invalidate configurationValidationResult on ChargingStationEvents.updated...

Subscribe to ChargingStationEvents.updated in AutomaticTransactionGenerator's
constructor so in-flight configuration mutations that reach any charging
station mutation emit cannot leave a stale memoized validation decision
behind. stop() still clears the cache as a redundant safety net.

deleteInstance unsubscribes the handler before removing the entry so the
retired instance is not retained via the charging station's listener array
across a getInstance/deleteInstance churn cycle.

Fixes #1965.

diff --git a/src/charging-station/AutomaticTransactionGenerator.ts b/src/charging-station/AutomaticTransactionGenerator.ts
index fff6474801a2ca7df57733f42ee774abca8ae2e8..b4f9db12ff43e70d538f99060665b1e9561ad75b 100644 (file)
--- a/src/charging-station/AutomaticTransactionGenerator.ts
+++ b/src/charging-station/AutomaticTransactionGenerator.ts
@@ -62,6 +62,13 @@ export class AutomaticTransactionGenerator {
     this.connectorAbortControllers = new Map<number, AbortController>()
     this.configurationValidationResult = undefined
     this.initializeConnectorsStatus()
+    // Event-driven cache invalidation via ChargingStationEvents.updated —
+    // ATG must track any in-flight config change (e.g. the templateFileWatcher
+    // reload path in ChargingStation.start()) between successive
+    // validateConfiguration retries (issue #1965). The clear is O(1); the
+    // next call re-parses four numeric bounds. stop() still clears the
+    // cache as a safety net.
+    this.chargingStation.on(ChargingStationEvents.updated, this.onUpdated)
   }
 
   public static deleteInstance (chargingStation: ChargingStation): boolean {
@@ -69,6 +76,13 @@ export class AutomaticTransactionGenerator {
     if (hashId == null) {
       return false
     }
+    const instance = AutomaticTransactionGenerator.instances.get(hashId)
+    if (instance != null) {
+      // Symmetric cleanup: constructor subscribed, deleteInstance unsubscribes
+      // so the retired instance is not retained by the charging station's
+      // listener array across a getInstance/deleteInstance churn cycle.
+      instance.chargingStation.off(ChargingStationEvents.updated, instance.onUpdated)
+    }
     return AutomaticTransactionGenerator.instances.delete(hashId)
   }
 
@@ -415,6 +429,10 @@ export class AutomaticTransactionGenerator {
     )
   }
 
+  private readonly onUpdated = (): void => {
+    this.configurationValidationResult = undefined
+  }
+
   private setStartConnectorStatus (
     connectorId: number,
     stopAbsoluteDuration = this.chargingStation.getAutomaticTransactionGeneratorConfiguration()

diff --git a/tests/charging-station/AutomaticTransactionGenerator.test.ts b/tests/charging-station/AutomaticTransactionGenerator.test.ts
index 68516e7a5c11a699b60a0e17690879cb4b9b1558..a15a17ebe3f32ce948808a60be331443b85359c0 100644 (file)
--- a/tests/charging-station/AutomaticTransactionGenerator.test.ts
+++ b/tests/charging-station/AutomaticTransactionGenerator.test.ts
@@ -15,13 +15,14 @@
  */
 
 import assert from 'node:assert/strict'
+import { EventEmitter } from 'node:events'
 import { afterEach, describe, it } from 'node:test'
 
 import type { ChargingStation } from '../../src/charging-station/index.js'
 
 import { AutomaticTransactionGenerator } from '../../src/charging-station/index.js'
 import { BaseError } from '../../src/exception/index.js'
-import { type StartTransactionResult } from '../../src/types/index.js'
+import { ChargingStationEvents, type StartTransactionResult } from '../../src/types/index.js'
 import { Constants } from '../../src/utils/index.js'
 import { flushMicrotasks, standardCleanup } from '../helpers/TestLifecycleHelpers.js'
 import { createMockChargingStation } from './helpers/StationHelpers.js'
@@ -128,6 +129,68 @@ function resetATGInstances (): void {
   atgClass.instances.clear()
 }
 
+/**
+ * Overrides the mock station's ATG configuration for a single test with the
+ * given min/max bounds so validateConfiguration can be steered to pass or fail.
+ * @param station - The mock station to reconfigure
+ * @param bounds - Partial ATG bounds; unspecified fields keep valid defaults
+ */
+function setATGBounds (
+  station: ChargingStation,
+  bounds: Partial<{
+    maxDelayBetweenTwoTransactions: number
+    maxDuration: number
+    minDelayBetweenTwoTransactions: number
+    minDuration: number
+  }>
+): void {
+  const stationExt = station as unknown as {
+    getAutomaticTransactionGeneratorConfiguration: () => Record<string, unknown>
+  }
+  stationExt.getAutomaticTransactionGeneratorConfiguration = () => ({
+    ...Constants.DEFAULT_ATG_CONFIGURATION,
+    enable: true,
+    idTagDistribution: 'random',
+    requireAuthorize: false,
+    stopAfterHours: 1,
+    ...bounds,
+  })
+}
+
+/**
+ * Wires a real Node.js EventEmitter onto the mock station's on/off/emit/
+ * emitChargingStationEvent/listenerCount methods so tests can exercise the
+ * ATG constructor's ChargingStationEvents.updated subscription (issue #1965).
+ * The shared mock defaults these to no-ops; overriding here keeps the blast
+ * radius local to this test file.
+ *
+ * **Must be called before `getDefinedATG`** (and therefore before any
+ * `AutomaticTransactionGenerator.getInstance` call for this station) so that
+ * the ATG constructor's `station.on(...)` call binds to the real emitter.
+ * Calling this after ATG construction silently leaves the listener on the
+ * no-op stub and the cache-invalidation assertions will fail.
+ * @param station - The mock station to wire
+ * @returns The underlying EventEmitter for test assertions
+ */
+function wireEventEmitter (station: ChargingStation): EventEmitter {
+  const emitter = new EventEmitter()
+  const stationExt = station as unknown as {
+    emit: EventEmitter['emit']
+    emitChargingStationEvent: (event: string, ...args: unknown[]) => void
+    listenerCount: EventEmitter['listenerCount']
+    off: EventEmitter['off']
+    on: EventEmitter['on']
+  }
+  stationExt.on = emitter.on.bind(emitter)
+  stationExt.off = emitter.off.bind(emitter)
+  stationExt.emit = emitter.emit.bind(emitter)
+  stationExt.listenerCount = emitter.listenerCount.bind(emitter)
+  stationExt.emitChargingStationEvent = (event, ...args): void => {
+    emitter.emit(event, ...args)
+  }
+  return emitter
+}
+
 await describe('AutomaticTransactionGenerator', async () => {
   afterEach(() => {
     standardCleanup()
@@ -267,4 +330,121 @@ await describe('AutomaticTransactionGenerator', async () => {
       assert.strictEqual(connectorStatus.rejectedStartTransactionRequests, 1)
     })
   })
+
+  await describe('configurationValidationResult cache invalidation (issue #1965)', async () => {
+    /**
+     * @param atg - ATG instance
+     * @returns validation result via the private method
+     */
+    function validateConfiguration (atg: AutomaticTransactionGenerator): boolean {
+      return (atg as unknown as { validateConfiguration: () => boolean }).validateConfiguration()
+    }
+
+    /**
+     * @param atg - ATG instance
+     * @returns the current memoized cache value
+     */
+    function readCache (atg: AutomaticTransactionGenerator): boolean | undefined {
+      return (atg as unknown as { configurationValidationResult: boolean | undefined })
+        .configurationValidationResult
+    }
+
+    await it('should invalidate the cache on ChargingStationEvents.updated', () => {
+      const station = createStationForATG()
+      const emitter = wireEventEmitter(station)
+      const atg = getDefinedATG(station)
+      setATGBounds(station, {
+        maxDelayBetweenTwoTransactions: 5,
+        minDelayBetweenTwoTransactions: 100,
+      })
+
+      assert.strictEqual(validateConfiguration(atg), false)
+      assert.strictEqual(readCache(atg), false)
+
+      setATGBounds(station, {})
+      emitter.emit(ChargingStationEvents.updated)
+
+      assert.strictEqual(readCache(atg), undefined)
+      assert.strictEqual(validateConfiguration(atg), true)
+      assert.strictEqual(readCache(atg), true)
+    })
+
+    await it('should recover startConnector after ChargingStationEvents.updated invalidates a cached-false decision', () => {
+      const station = createStationForATG()
+      const emitter = wireEventEmitter(station)
+      const atg = getDefinedATG(station)
+      mockInternalStartConnector(atg)
+      setATGBounds(station, {
+        maxDelayBetweenTwoTransactions: 5,
+        minDelayBetweenTwoTransactions: 100,
+      })
+
+      atg.startConnector(1)
+      assert.strictEqual(readCache(atg), false)
+
+      setATGBounds(station, {})
+      emitter.emit(ChargingStationEvents.updated)
+
+      atg.startConnector(1)
+      assert.strictEqual(readCache(atg), true)
+    })
+
+    await it('should still clear the cache on stop() (redundant with event-driven invalidation)', () => {
+      const station = createStationForATG()
+      wireEventEmitter(station)
+      const atg = getDefinedATG(station)
+      mockInternalStartConnector(atg)
+      setATGBounds(station, {})
+      atg.start()
+
+      assert.strictEqual(readCache(atg), true)
+
+      atg.stop()
+
+      assert.strictEqual(readCache(atg), undefined)
+    })
+
+    await it('should be idempotent under sequential updated events', () => {
+      const station = createStationForATG()
+      const emitter = wireEventEmitter(station)
+      const atg = getDefinedATG(station)
+      setATGBounds(station, {})
+
+      assert.strictEqual(validateConfiguration(atg), true)
+
+      assert.doesNotThrow(() => {
+        emitter.emit(ChargingStationEvents.updated)
+        emitter.emit(ChargingStationEvents.updated)
+      })
+
+      assert.strictEqual(readCache(atg), undefined)
+      assert.strictEqual(validateConfiguration(atg), true)
+    })
+
+    await it('should not leak listeners across deleteInstance/getInstance cycles', () => {
+      const station = createStationForATG()
+      const emitter = wireEventEmitter(station)
+
+      const atg1 = getDefinedATG(station)
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 1)
+
+      AutomaticTransactionGenerator.deleteInstance(station)
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 0)
+
+      const atg2 = getDefinedATG(station)
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 1)
+      assert.notStrictEqual(atg1, atg2)
+
+      for (let i = 0; i < 5; i++) {
+        AutomaticTransactionGenerator.deleteInstance(station)
+        getDefinedATG(station)
+      }
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 1)
+
+      AutomaticTransactionGenerator.deleteInstance(station)
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 0)
+      assert.strictEqual(AutomaticTransactionGenerator.deleteInstance(station), false)
+      assert.strictEqual(emitter.listenerCount(ChargingStationEvents.updated), 0)
+    })
+  })
 })

diff --git a/tests/charging-station/helpers/StationHelpers.factory.ts b/tests/charging-station/helpers/StationHelpers.factory.ts
index 434b74b24a5b2ea4655f0cc9d7bf5b224c3ee55b..a7aee78d72a1d000310dbf63101d97fd294a5f8b 100644 (file)
--- a/tests/charging-station/helpers/StationHelpers.factory.ts
+++ b/tests/charging-station/helpers/StationHelpers.factory.ts
@@ -479,6 +479,8 @@ export function createMockChargingStation (
       ...ocppRequestService,
     },
 
+    off: () => station,
+
     on: () => station,
 
     once: () => station,