refactor(ocpp20): rewrite H9/H10/H11 + C10/C12 audit markers as descriptive comments...

Nine inline comments in OCPP20IncomingRequestService.ts (7) and its
sibling test file OCPP20IncomingRequestService-UpdateFirmware.test.ts
(2) carried internal audit-round finding numbers (H9, H10, H11, C10,
C12) as if they were code identifiers.

Production-file markers (7) originate from commit 3a6e89f634
'fix(ocpp20): remediate all OCPP 2.0.1 audit findings (#1726)' which
predates PR #1935. Test-file markers (2 H11) originate from commit
b50f9e5f 'refactor(tests): separate handler/listener tests and remove
setTimeout hacks' — same audit-round family, later commit.

The audit markers had the shape //<letter+><digits>[:-)] which is
distinct from the legitimate OCPP spec references elsewhere in the
repo (B11 - Reset, F06.FR.06, A02.FR.06, E05.FR.09, L01.FR.30,
C10.FR.04, C12.FR.05, ...) that all follow <UC>.FR.<NN>.

Each rewrite replaces the marker with a WHY-rationale or BDD-style
expectation describing the current-state behavior. See the PR body
table for the full before/after list.

Verification (POSIX character classes; portable across all git-grep -E
builds — the \s shorthand is a PCRE extension not supported by POSIX
ERE and silently returns no match on macOS git):

  git grep -inE '^[[:space:]]*//[[:space:]]*[A-Z]+[0-9]+[]:)-]' -- \
    src/ tests/

returns empty.

Comment-only change. No runtime behavior change.
Test invariant fail: 0, skipped: 6 holds.

Closes issue #1936 item (l).

diff --git a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts
index 83a9433d339c0fa8ca33ec99399779eea2114dbe..de66867a64607a9e282b47febfcad0ee986dfe71 100644 (file)
--- a/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts
+++ b/src/charging-station/ocpp/2.0/OCPP20IncomingRequestService.ts
@@ -3037,7 +3037,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
       `${chargingStation.logPrefix()} ${moduleName}.handleRequestUpdateFirmware: Received UpdateFirmware request with requestId ${requestId.toString()} for location '${firmware.location}'`
     )
 
-    // C10: Validate signing certificate PEM format if present
+    // Reject malformed signing certificates up front so the firmware update path handles only parsable PEMs
     if (isNotEmptyString(firmware.signingCertificate)) {
       if (
         !hasCertificateManager(chargingStation) ||
@@ -3066,7 +3066,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
       )
     }
 
-    // H10: Cancel any in-progress firmware update
+    // Cancel any in-progress firmware update; respond with AcceptedCanceled so the new request starts fresh
     const stationState = this.getStationState(chargingStation)
     if (stationState.activeFirmwareUpdateAbortController != null) {
       const previousRequestId = stationState.activeFirmwareUpdateRequestId
@@ -3673,7 +3673,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
   ): Promise<void> {
     const { installDateTime, location, retrieveDateTime, signature } = firmware
 
-    // H10: Set up abort controller for cancellation support
+    // Store the abort controller so a subsequent UpdateFirmware can cancel this in-progress update
     const abortController = new AbortController()
     const stationState = this.getStationState(chargingStation)
     stationState.activeFirmwareUpdateAbortController = abortController
@@ -3681,7 +3681,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
 
     const checkAborted = (): boolean => abortController.signal.aborted
 
-    // C12: If retrieveDateTime is in the future, send DownloadScheduled and wait
+    // Delay the download until retrieveDateTime; inform the CSMS via DownloadScheduled first
     const now = Date.now()
     const retrieveTime = convertToDate(retrieveDateTime)?.getTime() ?? now
     if (retrieveTime > now) {
@@ -3703,7 +3703,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
     await sleep(OCPP20Constants.FIRMWARE_STATUS_DELAY_MS)
     if (checkAborted()) return
 
-    // H9: If firmware location is empty or malformed, send DownloadFailed and stop
+    // Empty or malformed firmware location: simulate the L01.FR.30 download retries, then emit DownloadFailed and stop
     if (isEmpty(location) || !this.isValidFirmwareLocation(location)) {
       // L01.FR.30: Simulate download retries before reporting DownloadFailed
       const maxRetries = retries ?? 0
@@ -3778,7 +3778,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
       )
     }
 
-    // C12: If installDateTime is in the future, send InstallScheduled and wait
+    // Delay the install until installDateTime; inform the CSMS via InstallScheduled first
     if (installDateTime != null) {
       const currentTime = Date.now()
       const installTime = convertToDate(installDateTime)?.getTime() ?? currentTime
@@ -3845,7 +3845,7 @@ export class OCPP20IncomingRequestService extends OCPPIncomingRequestService {
       requestId
     )
 
-    // H11: Send SecurityEventNotification for successful firmware update
+    // Send SecurityEventNotification after a successful firmware update
     this.sendSecurityEventNotification(
       chargingStation,
       'FirmwareUpdated',

diff --git a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts
index 218676696b992d746cc9583863285f150a15b5e2..76858b408723504c309768cf45bd4f13b54bd256 100644 (file)
--- a/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts
+++ b/tests/charging-station/ocpp/2.0/OCPP20IncomingRequestService-UpdateFirmware.test.ts
@@ -405,7 +405,7 @@ await describe('L01/L02 - UpdateFirmware', async () => {
           await flushMicrotasks()
           assert.strictEqual(sentRequests[3].payload.status, OCPP20FirmwareStatusEnumType.Installed)
 
-          // H11: SecurityEventNotification for FirmwareUpdated
+          // Expect SecurityEventNotification of type FirmwareUpdated on successful install
           assert.strictEqual(sentRequests.length, 5)
           assert.strictEqual(
             sentRequests[4].command,
@@ -723,7 +723,7 @@ await describe('L01/L02 - UpdateFirmware', async () => {
           await flushMicrotasks()
           assert.strictEqual(sentRequests[4].payload.status, OCPP20FirmwareStatusEnumType.Installed)
 
-          // H11: SecurityEventNotification after Installed
+          // Expect SecurityEventNotification to be emitted after the Installed status
           assert.strictEqual(sentRequests.length, 6)
           assert.strictEqual(
             sentRequests[5].command,