From: Jérôme Benoit Date: Fri, 3 Apr 2026 17:02:57 +0000 (+0200) Subject: refactor: dry improvements in web UI and OCPP mock server X-Git-Tag: ocpp-server@v4.3.0~12 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=599c58469621f8280a904cff8a665b75db08e7df;p=e-mobility-charging-stations-simulator.git refactor: dry improvements in web UI and OCPP mock server Web UI: - extract localStorage key constants (UI_SERVER_CONFIGURATION_INDEX_KEY, TOGGLE_BUTTON_KEY_PREFIX, SHARED_TOGGLE_BUTTON_KEY_PREFIX) - add deleteLocalStorageByKeyPattern() utility with safe collect-then- delete iteration - add getWebSocketStateName() utility replacing duplicated switch - remove redundant .trim() calls (already handled by v-model.trim) OCPP mock server: - extract FALLBACK_TRANSACTION_ID constant and shared helper method - replace 20-case match/case with _COMMAND_HANDLERS dict dispatch - unify _parse_set/get_variable_specs into shared _parse_variable_specs - add ClassVar annotation for proper mypy typing --- diff --git a/tests/ocpp-server/server.py b/tests/ocpp-server/server.py index 0e510ef9..0ebab661 100644 --- a/tests/ocpp-server/server.py +++ b/tests/ocpp-server/server.py @@ -11,6 +11,7 @@ from datetime import datetime, timedelta, timezone from enum import StrEnum from functools import partial from random import randint +from typing import ClassVar import ocpp.v201 import websockets @@ -59,6 +60,7 @@ DEFAULT_HOST = "127.0.0.1" DEFAULT_PORT = 9000 DEFAULT_HEARTBEAT_INTERVAL = 60 DEFAULT_TOTAL_COST = 10.0 +FALLBACK_TRANSACTION_ID = "test_transaction_123" MAX_REQUEST_ID = 2**31 - 1 SHUTDOWN_TIMEOUT = 30.0 SUBPROTOCOLS: list[websockets.Subprotocol] = [ @@ -447,11 +449,16 @@ class ChargePoint(ocpp.v201.ChargePoint): await self.call(request, suppress=False) logger.info("%s response received", Action.request_start_transaction) - async def _send_request_stop_transaction(self): + def _get_active_or_fallback_transaction_id(self) -> str: + """Return the first active transaction ID, or fall back to a test ID.""" transaction_id = next(iter(self._active_transactions), "") if not transaction_id: logger.warning("No active transaction found, using fallback ID") - transaction_id = "test_transaction_123" + transaction_id = FALLBACK_TRANSACTION_ID + return transaction_id + + async def _send_request_stop_transaction(self): + transaction_id = self._get_active_or_fallback_transaction_id() request = ocpp.v201.call.RequestStopTransaction(transaction_id=transaction_id) await self.call(request, suppress=False) logger.info("%s response received", Action.request_stop_transaction) @@ -554,10 +561,7 @@ class ChargePoint(ocpp.v201.ChargePoint): await self._call_and_log(request, Action.get_log, LogStatusEnumType.accepted) async def _send_get_transaction_status(self): - transaction_id = next(iter(self._active_transactions), "") - if not transaction_id: - logger.warning("No active transaction found, using fallback ID") - transaction_id = "test_transaction_123" + transaction_id = self._get_active_or_fallback_transaction_id() request = ocpp.v201.call.GetTransactionStatus( transaction_id=transaction_id, ) @@ -615,52 +619,37 @@ class ChargePoint(ocpp.v201.ChargePoint): # --- Command dispatch --- + _COMMAND_HANDLERS: ClassVar[dict[Action, str]] = { + Action.clear_cache: "_send_clear_cache", + Action.get_base_report: "_send_get_base_report", + Action.get_variables: "_send_get_variables", + Action.set_variables: "_send_set_variables", + Action.request_start_transaction: "_send_request_start_transaction", + Action.request_stop_transaction: "_send_request_stop_transaction", + Action.reset: "_send_reset", + Action.unlock_connector: "_send_unlock_connector", + Action.change_availability: "_send_change_availability", + Action.trigger_message: "_send_trigger_message", + Action.data_transfer: "_send_data_transfer", + Action.certificate_signed: "_send_certificate_signed", + Action.customer_information: "_send_customer_information", + Action.delete_certificate: "_send_delete_certificate", + Action.get_installed_certificate_ids: "_send_get_installed_certificate_ids", + Action.get_log: "_send_get_log", + Action.get_transaction_status: "_send_get_transaction_status", + Action.install_certificate: "_send_install_certificate", + Action.set_network_profile: "_send_set_network_profile", + Action.update_firmware: "_send_update_firmware", + } + async def _send_command(self, command_name: Action): logger.debug("Sending OCPP command %s", command_name) try: - match command_name: - case Action.clear_cache: - await self._send_clear_cache() - case Action.get_base_report: - await self._send_get_base_report() - case Action.get_variables: - await self._send_get_variables() - case Action.set_variables: - await self._send_set_variables() - case Action.request_start_transaction: - await self._send_request_start_transaction() - case Action.request_stop_transaction: - await self._send_request_stop_transaction() - case Action.reset: - await self._send_reset() - case Action.unlock_connector: - await self._send_unlock_connector() - case Action.change_availability: - await self._send_change_availability() - case Action.trigger_message: - await self._send_trigger_message() - case Action.data_transfer: - await self._send_data_transfer() - case Action.certificate_signed: - await self._send_certificate_signed() - case Action.customer_information: - await self._send_customer_information() - case Action.delete_certificate: - await self._send_delete_certificate() - case Action.get_installed_certificate_ids: - await self._send_get_installed_certificate_ids() - case Action.get_log: - await self._send_get_log() - case Action.get_transaction_status: - await self._send_get_transaction_status() - case Action.install_certificate: - await self._send_install_certificate() - case Action.set_network_profile: - await self._send_set_network_profile() - case Action.update_firmware: - await self._send_update_firmware() - case _: - logger.warning("Not supported command %s", command_name) + handler_name = self._COMMAND_HANDLERS.get(command_name) + if handler_name is not None: + await getattr(self, handler_name)() + else: + logger.warning("Not supported command %s", command_name) except TimeoutError: logger.error("Timeout waiting for %s response", command_name) except OCPPError as e: @@ -804,46 +793,42 @@ def _parse_commands(commands_str: str) -> list[tuple[Action, float]]: return result -def _parse_set_variable_specs(specs_str: str) -> list[dict]: +def _parse_variable_specs(specs_str: str, require_value: bool = False) -> list[dict]: result = [] for entry in specs_str.split(","): entry = entry.strip() if not entry: continue - if "=" not in entry or "." not in entry.split("=")[0]: - raise argparse.ArgumentTypeError( - f"Invalid variable spec '{entry}': expected 'Component.Variable=Value'" - ) - component_var, value = entry.split("=", 1) + if require_value: + if "=" not in entry or "." not in entry.split("=")[0]: + raise argparse.ArgumentTypeError( + f"Invalid variable spec '{entry}':" + " expected 'Component.Variable=Value'" + ) + component_var, value = entry.split("=", 1) + else: + if "." not in entry: + raise argparse.ArgumentTypeError( + f"Invalid variable spec '{entry}': expected 'Component.Variable'" + ) + component_var = entry component, variable = component_var.strip().split(".", 1) - result.append( - { - "component": {"name": component.strip()}, - "variable": {"name": variable.strip()}, - "attribute_value": value.strip(), - } - ) + spec: dict = { + "component": {"name": component.strip()}, + "variable": {"name": variable.strip()}, + } + if require_value: + spec["attribute_value"] = value.strip() + result.append(spec) return result +def _parse_set_variable_specs(specs_str: str) -> list[dict]: + return _parse_variable_specs(specs_str, require_value=True) + + def _parse_get_variable_specs(specs_str: str) -> list[dict]: - result = [] - for entry in specs_str.split(","): - entry = entry.strip() - if not entry: - continue - if "." not in entry: - raise argparse.ArgumentTypeError( - f"Invalid variable spec '{entry}': expected 'Component.Variable'" - ) - component, variable = entry.split(".", 1) - result.append( - { - "component": {"name": component.strip()}, - "variable": {"name": variable.strip()}, - } - ) - return result + return _parse_variable_specs(specs_str, require_value=False) async def main(): diff --git a/tests/ocpp-server/test_server.py b/tests/ocpp-server/test_server.py index ecf8c177..8ae4699f 100644 --- a/tests/ocpp-server/test_server.py +++ b/tests/ocpp-server/test_server.py @@ -45,6 +45,7 @@ from ocpp.v201.enums import ( from server import ( DEFAULT_HEARTBEAT_INTERVAL, DEFAULT_TOTAL_COST, + FALLBACK_TRANSACTION_ID, MAX_REQUEST_ID, AuthConfig, AuthMode, @@ -794,7 +795,7 @@ class TestTransactionTracking: ) await command_charge_point._send_request_stop_transaction() request = command_charge_point.call.call_args[0][0] - assert request.transaction_id == "test_transaction_123" + assert request.transaction_id == FALLBACK_TRANSACTION_ID async def test_empty_transaction_id_not_stored(self, charge_point): await charge_point.on_transaction_event( @@ -995,7 +996,7 @@ class TestOutgoingCommands: command_charge_point.call.assert_called_once() request = command_charge_point.call.call_args[0][0] assert isinstance(request, ocpp.v201.call.RequestStopTransaction) - assert request.transaction_id == "test_transaction_123" + assert request.transaction_id == FALLBACK_TRANSACTION_ID async def test_send_reset(self, command_charge_point): command_charge_point.call.return_value = ocpp.v201.call_result.Reset( @@ -1123,7 +1124,7 @@ class TestOutgoingCommands: command_charge_point.call.assert_called_once() request = command_charge_point.call.call_args[0][0] assert isinstance(request, ocpp.v201.call.GetTransactionStatus) - assert request.transaction_id == "test_transaction_123" + assert request.transaction_id == FALLBACK_TRANSACTION_ID async def test_send_install_certificate(self, command_charge_point): command_charge_point.call.return_value = ( diff --git a/ui/web/src/components/actions/StartTransaction.vue b/ui/web/src/components/actions/StartTransaction.vue index 9365d72d..be35e2d2 100644 --- a/ui/web/src/components/actions/StartTransaction.vue +++ b/ui/web/src/components/actions/StartTransaction.vue @@ -73,7 +73,7 @@ const toggleButtonId = computed( ) const handleStartTransaction = async (): Promise => { - const idTag = state.value.idTag.trim().length > 0 ? state.value.idTag.trim() : undefined + const idTag = state.value.idTag.length > 0 ? state.value.idTag : undefined if (!isOCPP20x.value && state.value.authorizeIdTag) { if (idTag == null) { diff --git a/ui/web/src/components/buttons/ToggleButton.vue b/ui/web/src/components/buttons/ToggleButton.vue index b5631975..ab40fd62 100644 --- a/ui/web/src/components/buttons/ToggleButton.vue +++ b/ui/web/src/components/buttons/ToggleButton.vue @@ -11,7 +11,12 @@ import { ref } from 'vue' import Button from '@/components/buttons/Button.vue' -import { getFromLocalStorage, setToLocalStorage } from '@/composables' +import { + getFromLocalStorage, + setToLocalStorage, + SHARED_TOGGLE_BUTTON_KEY_PREFIX, + TOGGLE_BUTTON_KEY_PREFIX, +} from '@/composables' const props = defineProps<{ id: string @@ -23,7 +28,7 @@ const props = defineProps<{ const $emit = defineEmits(['clicked']) -const id = props.shared === true ? `shared-toggle-button-${props.id}` : `toggle-button-${props.id}` +const id = props.shared === true ? `${SHARED_TOGGLE_BUTTON_KEY_PREFIX}${props.id}` : `${TOGGLE_BUTTON_KEY_PREFIX}${props.id}` const state = ref<{ status: boolean }>({ status: getFromLocalStorage(id, props.status ?? false), @@ -32,7 +37,7 @@ const state = ref<{ status: boolean }>({ const click = (): void => { if (props.shared === true) { for (const key in localStorage) { - if (key !== id && key.startsWith('shared-toggle-button-')) { + if (key !== id && key.startsWith(SHARED_TOGGLE_BUTTON_KEY_PREFIX)) { setToLocalStorage(key, false) state.value.status = getFromLocalStorage(key, false) } diff --git a/ui/web/src/components/charging-stations/CSData.vue b/ui/web/src/components/charging-stations/CSData.vue index cb51e7d5..d9430a5a 100644 --- a/ui/web/src/components/charging-stations/CSData.vue +++ b/ui/web/src/components/charging-stations/CSData.vue @@ -10,7 +10,7 @@ {{ getSupervisionUrl() }} - {{ getWSState() }} + {{ getWebSocketStateName(chargingStation.wsState) }} {{ chargingStation.bootNotificationResponse?.status ?? 'Ø' }} @@ -144,8 +144,8 @@ import StateButton from '@/components/buttons/StateButton.vue' import ToggleButton from '@/components/buttons/ToggleButton.vue' import CSConnector from '@/components/charging-stations/CSConnector.vue' import { - deleteFromLocalStorage, - getLocalStorage, + deleteLocalStorageByKeyPattern, + getWebSocketStateName, useExecuteAction, useUIClient, } from '@/composables' @@ -192,20 +192,6 @@ const getSupervisionUrl = (): string => { const supervisionUrl = new URL(props.chargingStation.supervisionUrl) return `${supervisionUrl.protocol}//${supervisionUrl.host.split('.').join('.\u200b')}` } -const getWSState = (): string => { - switch (props.chargingStation?.wsState) { - case WebSocket.CLOSED: - return 'Closed' - case WebSocket.CLOSING: - return 'Closing' - case WebSocket.CONNECTING: - return 'Connecting' - case WebSocket.OPEN: - return 'Open' - default: - return 'Ø' - } -} const $uiClient = useUIClient() @@ -245,11 +231,7 @@ const deleteChargingStation = (): void => { $uiClient .deleteChargingStation(props.chargingStation.stationInfo.hashId) .then(() => { - for (const key in getLocalStorage()) { - if (key.includes(props.chargingStation.stationInfo.hashId)) { - deleteFromLocalStorage(key) - } - } + deleteLocalStorageByKeyPattern(props.chargingStation.stationInfo.hashId) $emit('need-refresh') return $toast.success('Charging station successfully deleted') }) diff --git a/ui/web/src/composables/Constants.ts b/ui/web/src/composables/Constants.ts index 5079f3d8..152907cc 100644 --- a/ui/web/src/composables/Constants.ts +++ b/ui/web/src/composables/Constants.ts @@ -1,3 +1,6 @@ // Local UI project constants +export const SHARED_TOGGLE_BUTTON_KEY_PREFIX = 'shared-toggle-button-' +export const TOGGLE_BUTTON_KEY_PREFIX = 'toggle-button-' +export const UI_SERVER_CONFIGURATION_INDEX_KEY = 'uiServerConfigurationIndex' export const UI_WEBSOCKET_REQUEST_TIMEOUT_MS = 60_000 diff --git a/ui/web/src/composables/Utils.ts b/ui/web/src/composables/Utils.ts index fbc71980..99a4ee51 100644 --- a/ui/web/src/composables/Utils.ts +++ b/ui/web/src/composables/Utils.ts @@ -5,6 +5,7 @@ import { useToast } from 'vue-toast-notification' import type { ChargingStationData, ConfigurationData, UUIDv4 } from '@/types' +import { SHARED_TOGGLE_BUTTON_KEY_PREFIX, TOGGLE_BUTTON_KEY_PREFIX } from './Constants' import { UIClient } from './UIClient' export const configurationKey: InjectionKey> = Symbol('configuration') @@ -68,13 +69,51 @@ export const getLocalStorage = (): Storage => { return localStorage } +/** + * Deletes all localStorage entries whose key includes the given pattern. + * @param pattern - Substring to match against localStorage keys + */ +export const deleteLocalStorageByKeyPattern = (pattern: string): void => { + const keysToDelete: string[] = [] + for (const key in getLocalStorage()) { + if (key.includes(pattern)) { + keysToDelete.push(key) + } + } + for (const key of keysToDelete) { + deleteFromLocalStorage(key) + } +} + +/** + * Returns a human-readable name for a WebSocket ready state. + * @param state - The WebSocket readyState value + * @returns The state name or 'Ø' for unknown/undefined states + */ +export const getWebSocketStateName = (state: number | undefined): string => { + switch (state) { + case WebSocket.CLOSED: + return 'Closed' + case WebSocket.CLOSING: + return 'Closing' + case WebSocket.CONNECTING: + return 'Connecting' + case WebSocket.OPEN: + return 'Open' + default: + return 'Ø' + } +} + /** * Resets the state of a toggle button by removing its entry from localStorage. * @param id - The identifier of the toggle button * @param shared - Whether the toggle button is shared */ export const resetToggleButtonState = (id: string, shared = false): void => { - const key = shared ? `shared-toggle-button-${id}` : `toggle-button-${id}` + const key = shared + ? `${SHARED_TOGGLE_BUTTON_KEY_PREFIX}${id}` + : `${TOGGLE_BUTTON_KEY_PREFIX}${id}` deleteFromLocalStorage(key) } diff --git a/ui/web/src/composables/index.ts b/ui/web/src/composables/index.ts index d7cf7eff..a9543fa7 100644 --- a/ui/web/src/composables/index.ts +++ b/ui/web/src/composables/index.ts @@ -1,3 +1,8 @@ +export { + SHARED_TOGGLE_BUTTON_KEY_PREFIX, + TOGGLE_BUTTON_KEY_PREFIX, + UI_SERVER_CONFIGURATION_INDEX_KEY, +} from './Constants' export { UIClient } from './UIClient' export { chargingStationsKey, @@ -5,8 +10,10 @@ export { convertToBoolean, convertToInt, deleteFromLocalStorage, + deleteLocalStorageByKeyPattern, getFromLocalStorage, getLocalStorage, + getWebSocketStateName, randomUUID, resetToggleButtonState, setToLocalStorage, diff --git a/ui/web/src/views/ChargingStationsView.vue b/ui/web/src/views/ChargingStationsView.vue index 87623609..55c0419e 100644 --- a/ui/web/src/views/ChargingStationsView.vue +++ b/ui/web/src/views/ChargingStationsView.vue @@ -13,7 +13,7 @@ @change=" () => { if ( - getFromLocalStorage('uiServerConfigurationIndex', 0) !== state.uiServerIndex + getFromLocalStorage(UI_SERVER_CONFIGURATION_INDEX_KEY, 0) !== state.uiServerIndex ) { $uiClient.setConfiguration( ($configuration.uiServer as UIServerConfigurationSection[])[state.uiServerIndex] @@ -22,7 +22,7 @@ $uiClient.registerWSEventListener( 'open', () => { - setToLocalStorage('uiServerConfigurationIndex', state.uiServerIndex) + setToLocalStorage(UI_SERVER_CONFIGURATION_INDEX_KEY, state.uiServerIndex) clearToggleButtons() refresh() $route.name !== 'charging-stations' && @@ -34,12 +34,12 @@ 'error', () => { state.uiServerIndex = getFromLocalStorage( - 'uiServerConfigurationIndex', + UI_SERVER_CONFIGURATION_INDEX_KEY, 0 ) $uiClient.setConfiguration( ($configuration.uiServer as UIServerConfigurationSection[])[ - getFromLocalStorage('uiServerConfigurationIndex', 0) + getFromLocalStorage(UI_SERVER_CONFIGURATION_INDEX_KEY, 0) ] ) registerWSEventListeners() @@ -116,11 +116,11 @@ import ToggleButton from '@/components/buttons/ToggleButton.vue' import CSTable from '@/components/charging-stations/CSTable.vue' import Container from '@/components/Container.vue' import { - deleteFromLocalStorage, + deleteLocalStorageByKeyPattern, getFromLocalStorage, - getLocalStorage, randomUUID, setToLocalStorage, + UI_SERVER_CONFIGURATION_INDEX_KEY, useChargingStations, useConfiguration, useTemplates, @@ -149,7 +149,7 @@ const state = ref<{ gettingTemplates: false, renderAddChargingStations: randomUUID(), renderChargingStations: randomUUID(), - uiServerIndex: getFromLocalStorage('uiServerConfigurationIndex', 0), + uiServerIndex: getFromLocalStorage(UI_SERVER_CONFIGURATION_INDEX_KEY, 0), }) const refresh = (): void => { @@ -158,11 +158,7 @@ const refresh = (): void => { } const clearToggleButtons = (): void => { - for (const key in getLocalStorage()) { - if (key.includes('toggle-button')) { - deleteFromLocalStorage(key) - } - } + deleteLocalStorageByKeyPattern('toggle-button') } const $configuration = useConfiguration()