Fix connector status race condition in UI server cache during startup (#1548)

* Initial plan

* Fix connector status race condition by setting status before sending request

Set connector status optimistically before sending the StatusNotification
request to ensure the updated event (emitted in the request handler's
finally block) contains the current status. Revert status on error.

This prevents the UI cache from being populated with stale connector
status during startup when multiple status notifications are sent
concurrently.

Fixes race condition where:
1. StatusNotification sent
2. Response received
3. 'updated' event emitted with OLD status (in finally block)
4. Connector status set to NEW status
5. UI cache now has outdated data

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* refactor: avoid non-null assertions in sendAndSetConnectorStatus

Replace non-null assertions (!) with null checks and early return to comply
with updated coding guidelines. Add error logging when connector status is null.

Changes:
- Check connectorStatus for null before accessing properties
- Store connectorStatus reference to avoid repeated calls
- Add error log and early return for non-existing connector
- Remove all eslint-disable comments for no-non-null-assertion

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* revert: remove incorrect fix to sendAndSetConnectorStatus

The previous fix addressed OCPP server communication, but the issue is about
UI server cache updates via worker messages. Reverting to investigate the
correct location for the fix.

The race condition is in how worker messages are sent to the main thread,
not in the OCPP StatusNotification timing.

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* fix: prevent race condition in UI cache during connector initialization

Add skipUpdatedEvent parameter to RequestParams to suppress automatic
updated events during connector status initialization loop. Emit single
updated event after all connectors are initialized.

This fixes the race condition where multiple partial ChargingStationData
messages were sent to the main thread during startup, potentially causing
the UI server cache to contain stale or incomplete connector status data.

Changes:
- Add skipUpdatedEvent to RequestParams interface
- Update OCPPRequestService to check skipUpdatedEvent before emitting
- Add params parameter to sendAndSetConnectorStatus
- Use skipUpdatedEvent in startMessageSequence for all connector inits
- Emit single updated event after initialization loop completes

The fix ensures UI cache receives complete, consistent charging station
data after all connectors are properly initialized, eliminating the race
condition caused by out-of-order or partial state updates.

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* revert: remove incorrect fix that broke status notification lifecycle

Previous fix suppressed updated events during initialization, which prevented
the correct chronological sending of status notifications to OCPP server.

The actual issue is in how the UI server cache handles out-of-order worker
messages, not in when updated events are emitted.

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* fix: prevent UI cache corruption from out-of-order worker messages

Add timestamp to ChargingStationData and update workerEventUpdated to only
update cache if incoming message is newer than cached data.

This prevents the UI server cache from being overwritten with stale data
when worker messages arrive out of chronological order during startup.

Changes:
- Add timestamp field to ChargingStationData interface
- Set timestamp in buildChargingStationDataPayload using Date.now()
- Update Bootstrap.workerEventUpdated to compare timestamps
- Only update cache if incoming data is newer or cache is empty

The fix ensures the UI cache always contains the most recent charging
station state, even when multiple updated messages are sent during
initialization and arrive out of order.

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* refactor: encapsulate chargingStations Map with getter/setter methods

Make chargingStations Map private in AbstractUIServer and add public
getter/setter methods to encapsulate access. Move timestamp comparison
logic into the setter method for centralized cache update control.

Changes:
- Make chargingStations Map private in AbstractUIServer
- Add getChargingStationData() getter for single entry
- Add listChargingStationData() to return all entries as array
- Add setChargingStationData() with timestamp comparison logic
- Add deleteChargingStationData() for deletion
- Add hasChargingStationData() for existence check
- Add getChargingStationsCount() for size
- Update Bootstrap event handlers to use setter methods
- Update AbstractUIService to use getter methods

This encapsulation ensures all cache updates go through the timestamp
comparison logic, preventing corruption from out-of-order messages.

Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
* [autofix.ci] apply automated fixes

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: jerome-benoit <6537332+jerome-benoit@users.noreply.github.com>
Co-authored-by: Jérôme Benoit <jerome.benoit@piment-noir.org>
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>

diff --git a/src/charging-station/Bootstrap.ts b/src/charging-station/Bootstrap.ts
index c60f200bdf0d0646b9f1e5ea6179d14bf520476b..52abb18845f0a72714ac60089c2847d9a8187e3b 100644 (file)
--- a/src/charging-station/Bootstrap.ts
+++ b/src/charging-station/Bootstrap.ts
@@ -543,7 +543,7 @@ export class Bootstrap extends EventEmitter {
   }
 
   private readonly workerEventAdded = (data: ChargingStationData): void => {
-    this.uiServer.chargingStations.set(data.stationInfo.hashId, data)
+    this.uiServer.setChargingStationData(data.stationInfo.hashId, data)
     logger.info(
       `${this.logPrefix()} ${moduleName}.workerEventAdded: Charging station ${
         // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
@@ -553,7 +553,7 @@ export class Bootstrap extends EventEmitter {
   }
 
   private readonly workerEventDeleted = (data: ChargingStationData): void => {
-    this.uiServer.chargingStations.delete(data.stationInfo.hashId)
+    this.uiServer.deleteChargingStationData(data.stationInfo.hashId)
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     const templateStatistics = this.templateStatistics.get(data.stationInfo.templateName)!
     --templateStatistics.added
@@ -582,7 +582,7 @@ export class Bootstrap extends EventEmitter {
   }
 
   private readonly workerEventStarted = (data: ChargingStationData): void => {
-    this.uiServer.chargingStations.set(data.stationInfo.hashId, data)
+    this.uiServer.setChargingStationData(data.stationInfo.hashId, data)
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     ++this.templateStatistics.get(data.stationInfo.templateName)!.started
     logger.info(
@@ -594,7 +594,7 @@ export class Bootstrap extends EventEmitter {
   }
 
   private readonly workerEventStopped = (data: ChargingStationData): void => {
-    this.uiServer.chargingStations.set(data.stationInfo.hashId, data)
+    this.uiServer.setChargingStationData(data.stationInfo.hashId, data)
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     --this.templateStatistics.get(data.stationInfo.templateName)!.started
     logger.info(
@@ -606,6 +606,6 @@ export class Bootstrap extends EventEmitter {
   }
 
   private readonly workerEventUpdated = (data: ChargingStationData): void => {
-    this.uiServer.chargingStations.set(data.stationInfo.hashId, data)
+    this.uiServer.setChargingStationData(data.stationInfo.hashId, data)
   }
 }

diff --git a/src/charging-station/ui-server/AbstractUIServer.ts b/src/charging-station/ui-server/AbstractUIServer.ts
index 74bb3ad07724f04b0b70c6d33535b2e7c3255111..7f0ff345ec2367b7930740a1c9936d0ef2f32a10 100644 (file)
--- a/src/charging-station/ui-server/AbstractUIServer.ts
+++ b/src/charging-station/ui-server/AbstractUIServer.ts
@@ -26,10 +26,9 @@ import { getUsernameAndPasswordFromAuthorizationToken } from './UIServerUtils.js
 const moduleName = 'AbstractUIServer'
 
 export abstract class AbstractUIServer {
-  public readonly chargingStations: Map<string, ChargingStationData>
   public readonly chargingStationTemplates: Set<string>
-
   protected readonly httpServer: Http2Server | Server
+
   protected readonly responseHandlers: Map<
     `${string}-${string}-${string}-${string}-${string}`,
     ServerResponse | WebSocket
@@ -37,6 +36,8 @@ export abstract class AbstractUIServer {
 
   protected readonly uiServices: Map<ProtocolVersion, AbstractUIService>
 
+  private readonly chargingStations: Map<string, ChargingStationData>
+
   public constructor (protected readonly uiServerConfiguration: UIServerConfiguration) {
     this.chargingStations = new Map<string, ChargingStationData>()
     this.chargingStationTemplates = new Set<string>()
@@ -80,10 +81,30 @@ export abstract class AbstractUIServer {
     this.chargingStationTemplates.clear()
   }
 
+  public deleteChargingStationData (hashId: string): boolean {
+    return this.chargingStations.delete(hashId)
+  }
+
+  public getChargingStationData (hashId: string): ChargingStationData | undefined {
+    return this.chargingStations.get(hashId)
+  }
+
+  public getChargingStationsCount (): number {
+    return this.chargingStations.size
+  }
+
+  public hasChargingStationData (hashId: string): boolean {
+    return this.chargingStations.has(hashId)
+  }
+
   public hasResponseHandler (uuid: `${string}-${string}-${string}-${string}-${string}`): boolean {
     return this.responseHandlers.has(uuid)
   }
 
+  public listChargingStationData (): ChargingStationData[] {
+    return [...this.chargingStations.values()]
+  }
+
   public abstract logPrefix (moduleName?: string, methodName?: string, prefixSuffix?: string): string
 
   public async sendInternalRequest (request: ProtocolRequest): Promise<ProtocolResponse> {
@@ -98,6 +119,13 @@ export abstract class AbstractUIServer {
 
   public abstract sendResponse (response: ProtocolResponse): void
 
+  public setChargingStationData (hashId: string, data: ChargingStationData): void {
+    const cachedData = this.chargingStations.get(hashId)
+    if (cachedData == null || data.timestamp >= cachedData.timestamp) {
+      this.chargingStations.set(hashId, data)
+    }
+  }
+
   public abstract start (): void
 
   public stop (): void {

diff --git a/src/charging-station/ui-server/ui-services/AbstractUIService.ts b/src/charging-station/ui-server/ui-services/AbstractUIService.ts
index 8fe6bf7048dfd1c07ca73c7f0ab02ac8adb18257..789e7456615476c3eeca2469908ef41ebd986c9b 100644 (file)
--- a/src/charging-station/ui-server/ui-services/AbstractUIService.ts
+++ b/src/charging-station/ui-server/ui-services/AbstractUIService.ts
@@ -282,7 +282,7 @@ export abstract class AbstractUIService {
 
   private handleListChargingStations (): ResponsePayload {
     return {
-      chargingStations: [...this.uiServer.chargingStations.values()] as JsonType[],
+      chargingStations: this.uiServer.listChargingStationData() as JsonType[],
       status: ResponseStatus.SUCCESS,
     } satisfies ResponsePayload
   }
@@ -370,7 +370,7 @@ export abstract class AbstractUIService {
     if (isNotEmptyArray(payload.hashIds)) {
       payload.hashIds = payload.hashIds
         .map(hashId => {
-          if (this.uiServer.chargingStations.has(hashId)) {
+          if (this.uiServer.hasChargingStationData(hashId)) {
             return hashId
           }
           logger.warn(
@@ -387,7 +387,7 @@ export abstract class AbstractUIService {
     }
     const expectedNumberOfResponses = Array.isArray(payload.hashIds)
       ? payload.hashIds.length
-      : this.uiServer.chargingStations.size
+      : this.uiServer.getChargingStationsCount()
     if (expectedNumberOfResponses === 0) {
       throw new BaseError(
         'hashIds array in the request payload does not contain any valid charging station hashId'

diff --git a/src/types/ChargingStationWorker.ts b/src/types/ChargingStationWorker.ts
index 05e3a85f86b9c9ad04637cfd446e283c83f900e4..3bea940f647052e42698bc5220d26e32122360e2 100644 (file)
--- a/src/types/ChargingStationWorker.ts
+++ b/src/types/ChargingStationWorker.ts
@@ -25,6 +25,7 @@ export interface ChargingStationData extends WorkerData {
   started: boolean
   stationInfo: ChargingStationInfo
   supervisionUrl: string
+  timestamp: number
   wsState?:
     | typeof WebSocket.CLOSED
     | typeof WebSocket.CLOSING

diff --git a/src/utils/MessageChannelUtils.ts b/src/utils/MessageChannelUtils.ts
index f1ca57c1d50f0d7a482d589d2471021606f94085..69abece32a2f0a4e509968c6fa7029b1c4e01350 100644 (file)
--- a/src/utils/MessageChannelUtils.ts
+++ b/src/utils/MessageChannelUtils.ts
@@ -94,6 +94,7 @@ const buildChargingStationDataPayload = (chargingStation: ChargingStation): Char
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     stationInfo: chargingStation.stationInfo!,
     supervisionUrl: chargingStation.wsConnectionUrl.href,
+    timestamp: Date.now(),
     wsState: chargingStation.wsConnection?.readyState,
     ...(chargingStation.automaticTransactionGenerator != null && {
       automaticTransactionGenerator: