From df2b2949302cb29c93463483c7909e95b4d91381 Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= Date: Sat, 4 Apr 2026 03:07:14 +0200 Subject: [PATCH] docs: update serena memories after full codebase audit - code_style_conventions: add constant unit suffixes, BaseError import constraint, utility usage rules, Vue UI conventions, Python conventions, testing patterns (AAA, __testable__, flushMicrotasks), enum merging, console.* rules, fix WeakMap description accuracy - project_overview: add auth subsystem, UI server transports, design patterns table, coverage thresholds, update tech stack versions - task_completion_checklist: add ruff standalone commands for Python --- .serena/memories/code_style_conventions.md | 62 ++++++++++++++++--- .serena/memories/project_overview.md | 49 ++++++++++++--- .serena/memories/task_completion_checklist.md | 10 +-- 3 files changed, 100 insertions(+), 21 deletions(-) diff --git a/.serena/memories/code_style_conventions.md b/.serena/memories/code_style_conventions.md index 0c187aa0..ec05d1c9 100644 --- a/.serena/memories/code_style_conventions.md +++ b/.serena/memories/code_style_conventions.md @@ -2,17 +2,22 @@ ## TypeScript Conventions -- **Naming**: camelCase for variables/functions/methods, PascalCase for classes/types/enums/interfaces, SCREAMING_SNAKE_CASE for test constants +- **Naming**: camelCase for variables/functions/methods, PascalCase for classes/types/enums/interfaces, SCREAMING_SNAKE_CASE for all constants (production and test) +- **Constant unit suffixes**: Time/size constants MUST include unit in name: `_MS`, `_SECONDS`, `_BYTES`. Counts/ratios/strings: no suffix. No inline `// Ms` comments — the name IS the documentation - **Async**: Prefer async/await over raw Promises; fire-and-forget wrapped in `void`; handle rejections with try/catch - **Error handling**: Typed errors (`BaseError`, `OCPPError`) with structured properties; avoid generic `Error`; use `instanceof` guards (not `as` casts) +- **BaseError import constraint**: `src/utils/` CANNOT import from `exception/index.js` (barrel) — causes circular dep via `OCPPError → OCPPConstants → utils/Constants`. `new Error()` is acceptable in utils/ and worker/ - **Null safety**: Avoid non-null assertions (`!`); use optional chaining (`?.`) and nullish coalescing (`??`) - **Type safety**: Prefer explicit types over `any`; use type guards and discriminated unions; no `as any`, `@ts-ignore`, `@ts-expect-error` -## Enums +## Enums & Type Merging - **`enum`** (not `const enum`) for all enumerations — 0 `const enum` in project -- **`as const` objects** for cross-version OCPP enum merges (e.g., `IncomingRequestCommand = { ...OCPP16..., ...OCPP20... } as const`) +- **`as const` objects** for cross-version OCPP enum merges (e.g., `ConnectorStatusEnum = { ...OCPP16ChargePointStatus, ...OCPP20ConnectorStatusEnumType } as const`) - Pattern: `enum` for individual definitions, `as const` for composition of multiple enums +- **Discriminated unions** for cross-version request/response types: `type BootNotificationRequest = OCPP16BootNotificationRequest | OCPP20BootNotificationRequest` +- **Type barrel chain**: `ocpp/1.6/*.ts` + `ocpp/2.0/*.ts` → unified `ocpp/*.ts` → `types/index.ts` +- **Configuration type composition**: Intersection types (`A & B & C`), not inheritance ## Imports @@ -37,7 +42,7 @@ - **Version handling**: OCPP 1.6 and 2.0.x in separate directories/namespaces - **Payload validation**: Against OCPP JSON schemas when `ocppStrictCompliance` is enabled - **Message format**: SRPC format: `[messageTypeId, messageId, action, payload]` -- **Per-station state**: WeakMap-based isolation per station (not singleton properties) +- **Per-station state**: Instance-based (each `ChargingStation` owns its own Maps/state). `WeakMap` used only in `OCPP20IncomingRequestService.stationsState` for handler-scoped state keyed by station reference ### Request Architecture @@ -45,6 +50,9 @@ - **`buildRequestPayload()`** enriches where needed (1.6: meterStart/idTag/timestamp; 2.0: CSR generation), passthrough otherwise - **Service utils** (`buildTransactionEvent`, `buildStatusNotificationRequest`, `buildMeterValue`, etc.) build complex payloads upstream before calling `requestHandler` - **Broadcast channel handlers** are simple passthroughs to `requestHandler` — no state management or flow duplication +- **Request/Response correlation**: UUID Map with resolve/reject/timeout per pending request +- **Payload validation**: AJV against OCPP JSON schemas when `ocppStrictCompliance` is enabled +- **Version dispatch**: `OCPPServiceOperations.ts` provides version-agnostic operations (startTransaction, stopTransaction, isIdTagAuthorized) that dispatch to version-specific handlers ## Logging @@ -52,6 +60,7 @@ - **Format**: `${chargingStation.logPrefix()} ${moduleName}.methodName: Message` - **Daily rotation** enabled by default; configurable max files/size - **Log config**: `src/utils/Configuration.ts` canonical defaults +- **`console.*` is acceptable ONLY** in: `start.ts` (entry point), `Configuration.ts`/`ConfigurationMigration.ts` (static init, logger not ready), `ErrorUtils.ts` (uncaught handlers, logger may be null), `WorkerUtils.ts` (standalone, no logger access). Everywhere else: use `logger` ## Configuration @@ -65,14 +74,47 @@ Full guide: `tests/TEST_STYLE_GUIDE.md`. Key points: -- **Assertions**: `node:assert/strict` — strict only; `assert.ok` only for boolean/existence -- **Naming**: `should [verb]` lowercase; files as `ModuleName.test.ts` -- **Structure**: Single top-level `await describe()` per file; AAA pattern -- **Isolation**: Fresh instances in `beforeEach`; `standardCleanup()` in `afterEach` (mandatory) -- **No real delays**: Use `withMockTimers()` for timer-dependent tests +- **Assertions**: `node:assert/strict` — strict only; `assert.ok` only for boolean/existence; never loose equality +- **Naming**: `should [verb]` lowercase; files as `ModuleName.test.ts`; OCPP tests: spec code prefix (e.g., `B11 - Reset`) +- **Structure**: Single top-level `await describe()` per file (CRITICAL for Windows CI `--test-force-exit`); AAA pattern with comments +- **File headers**: Mandatory `@file` + `@description` JSDoc +- **Isolation**: Fresh instances in `beforeEach`; `standardCleanup()` in `afterEach` (mandatory); `cleanupChargingStation()` for station instances +- **No real delays**: Use `withMockTimers(t, ['setTimeout'], async () => {...})` for timer tests +- **Mocking**: `t.mock.method()` for spying; `mock.fn()` for stubs; all restored by `standardCleanup()` +- **Async side-effects**: Use `flushMicrotasks()` for event emitter draining (not `await Promise.resolve()`) +- **Station factory**: `createMockChargingStation(options?)` returns `{ station, mocks }` with MockWebSocket, parentPortMessages, file system mocks +- **Auth factories**: `createMockAuthRequest()`, `createMockAuthorizationResult()`, `createMockAuthService()` in `tests/charging-station/ocpp/auth/helpers/MockFactories.ts` +- **Transaction setup**: `setupConnectorWithTransaction(station, connectorId, { transactionId, idTag? })` +- **Re-export hub**: `tests/charging-station/ChargingStationTestUtils.ts` aggregates all test utilities +- **`__testable__` pattern**: `ocpp/1.6/__testable__/` and `ocpp/2.0/__testable__/` directories expose internal classes (e.g., `OCPP20VariableManagerTestable`, `OCPP20RequestServiceTestable`) for unit testing private internals. Import from `__testable__/index.ts` barrel in tests only + +## Utility Usage Rules + +- **Emptiness checks**: Use `isEmpty()` / `isNotEmptyArray()` instead of `.length === 0` / `.size > 0` (except in worker/) +- **Number parsing**: Use `convertToInt()` / `convertToFloat()` instead of `Number.parseInt()` / `Number.parseFloat()`. Exception: when NaN fallback is needed (e.g., `getLimitFromSampledValueTemplateCustomValue` — keep `Number.parseFloat`) +- **Cloning**: Use `clone()` — never `JSON.parse(JSON.stringify())` +- **Random**: Use `secureRandom()` / `generateUUID()` — not direct `randomBytes()` / `randomUUID()` (except in worker/ which has its own copies) + +## Vue UI Conventions + +- **Route names**: Use `ROUTE_NAMES` constant object from `composables/Constants.ts` — never hardcode route strings +- **Placeholders**: Use `EMPTY_VALUE_PLACEHOLDER` constant for unknown/missing values — never hardcode `'Ø'` +- **localStorage keys**: Use `UI_SERVER_CONFIGURATION_INDEX_KEY` and `TOGGLE_BUTTON_KEY_PREFIX` constants +- **Separate package**: UI Web cannot import from the backend `src/` — shared logic is duplicated in `composables/Utils.ts` ## Common Pitfalls - **ESLint cache**: Clear `.eslintcache` if lint results seem stale after config changes - **Web UI is independent**: Always run its quality gates separately from `ui/web/` directory -- **OCPP server is Python**: Uses Poetry, not pnpm +- **OCPP server is Python**: Uses Poetry, not pnpm. Linter is ruff (not pylint/flake8). Type checker is mypy +- **Barrel circular deps**: `src/utils/` must NOT import from `src/exception/index.js` — import directly from `BaseError.js` if absolutely needed + +## Python Conventions (tests/ocpp-server/) + +- **Naming**: SCREAMING_SNAKE_CASE constants with unit suffixes (`_SECONDS`), snake_case functions, PascalCase classes +- **Constants**: Module-level, grouped by category (server config, auth, defaults) +- **ruff S105**: Constants with "TOKEN" in name trigger false positive — suppress with `# noqa: S105` +- **Async**: `async/await` throughout, `asyncio_mode = "auto"` in pytest +- **Testing**: pytest fixtures for charge point variants (normal, whitelist, blacklist, offline, rate_limit, command); parametrized tests for command paths +- **Server architecture**: `ChargePoint` class inherits `ocpp.v201.ChargePoint`, uses `_COMMAND_HANDLERS` ClassVar for dispatch, `Timer` class for delayed/periodic commands +- **No thin wrappers**: Inline `require_value=True/False` at call sites rather than creating one-liner wrapper functions diff --git a/.serena/memories/project_overview.md b/.serena/memories/project_overview.md index ce233c43..7427b02b 100644 --- a/.serena/memories/project_overview.md +++ b/.serena/memories/project_overview.md @@ -14,11 +14,18 @@ Node.js simulator for OCPP-J charging stations, part of SAP e-Mobility solution. ## Tech Stack -| Sub-project | Runtime | Language | Package Manager | Test Framework | Build Tool | -| ----------- | ---------------- | ---------------------- | --------------- | ----------------------- | ---------- | -| Simulator | Node.js >=22.0.0 | TypeScript 5.9 | pnpm >=10.9.0 | Node.js native `--test` | esbuild | -| Web UI | Node.js >=22.0.0 | TypeScript 5.9 + Vue 3 | pnpm >=10.9.0 | Vitest | Vite | -| OCPP Server | Python >=3.12 | Python | Poetry >=2.0 | pytest + pytest-asyncio | N/A | +| Sub-project | Runtime | Language | Package Manager | Test Framework | Build Tool | +| ----------- | ---------------- | ------------------------ | --------------- | ----------------------- | ---------- | +| Simulator | Node.js >=22.0.0 | TypeScript 6.0 | pnpm >=10.9.0 | Node.js native `--test` | esbuild | +| Web UI | Node.js >=22.0.0 | TypeScript 6.0 + Vue 3.5 | pnpm >=10.9.0 | Vitest | Vite 8 | +| OCPP Server | Python >=3.12 | Python | Poetry >=2.0 | pytest + pytest-asyncio | N/A | + +## Coverage Thresholds + +| Sub-project | Branches | Functions | Lines/Statements | +| ----------- | ------------- | --------- | ---------------- | +| Web UI | 89% | 83% | 91% | +| OCPP Server | 83% (overall) | — | — | ## Source Structure @@ -46,10 +53,38 @@ src/ - `charging-station/` and `ocpp/` are SEPARATE components with their own barrels - `ocpp/1.6/` and `ocpp/2.0/` are separate sub-components of ocpp -- `worker/` is self-contained and intentionally defines its own types (portable to other projects) +- `ocpp/auth/` is an independent subsystem with its own barrel, interfaces, and strategy pattern +- `worker/` is **fully standalone** — zero imports from other local modules. Has its own `sleep()`, `secureRandom()`, `mergeDeepRight()`. Uses `new Error()` (not `BaseError`). Portable to other projects - `types/` is pure type definitions, depends on nothing except `worker/` (type-only) - `utils/` depends on `types/` and `charging-station/` (type-only + 1 runtime: `getMessageTypeString`) -- `exception/` is a leaf module with no dependencies +- `exception/` — `BaseError` has no imports; `OCPPError` imports from `types/` and `ocpp/OCPPConstants`. The barrel creates a transitive dep chain NOT usable by `utils/` (circular) + +## Design Patterns + +| Pattern | Where | Detail | +| -------------------- | --------------------------------------------------------------- | --------------------------------------------------------------- | ---------- | +| Singleton | Bootstrap, Configuration, PerformanceStatistics, UIClient (Vue) | Lazy `getInstance()` | +| Strategy | Auth subsystem | Local/Remote/Certificate strategies with priority chain | +| Factory | WorkerFactory, StorageFactory, AuthComponentFactory | Create implementations from config | +| EventEmitter | ChargingStation, Bootstrap | State change events | +| SRPC | UI WebSocket | `[uuid, procedureName, payload]` request/response correlation | +| Barrel exports | All components | `index.ts` re-exports public API | +| Discriminated unions | OCPP types | `BootNotificationRequest = OCPP16... | OCPP20...` | +| `as const` merge | OCPP enums | `ConnectorStatusEnum = { ...OCPP16..., ...OCPP20... } as const` | + +## Auth Subsystem (`ocpp/auth/`) + +- **OCPPAuthServiceImpl**: Strategy priority chain (local → remote → certificate) +- **3 strategies**: LocalAuthStrategy (cache + local list), RemoteAuthStrategy (CSMS), CertificateAuthStrategy (X.509) +- **InMemoryAuthCache**: LRU with TTL, rate limiting, periodic cleanup +- **AuthComponentFactory**: Creates adapters, strategies, caches with DI +- **Version adapters**: OCPP16AuthAdapter, OCPP20AuthAdapter + +## UI Server (`ui-server/`) + +- **3 transports**: UIWebSocketServer (SRPC), UIMCPServer (Model Context Protocol for LLM agents), UIHttpServer (REST, deprecated) +- **AbstractUIServer**: Base with HTTP/HTTP2, auth (Basic/Protocol), rate limiting, debounced client notifications (500ms) +- **Protocol**: `[uuid, procedureName, payload]` requests, `[uuid, responsePayload]` responses, `[ServerNotification.REFRESH]` notifications ## Test Structure diff --git a/.serena/memories/task_completion_checklist.md b/.serena/memories/task_completion_checklist.md index e4b3db92..c2667bb4 100644 --- a/.serena/memories/task_completion_checklist.md +++ b/.serena/memories/task_completion_checklist.md @@ -27,12 +27,14 @@ pnpm test ```bash cd tests/ocpp-server -poetry run task format -poetry run task typecheck -poetry run task lint -poetry run task test +poetry run task format # ruff check --fix + ruff format +poetry run task typecheck # mypy +poetry run task lint # ruff check --diff + ruff format --check +poetry run task test # pytest -v ``` +Standalone ruff (without Poetry) also works: `ruff check .` and `ruff format --check .` + ### 4. Documentation - Update docs if public API changed -- 2.53.0