From: Jérôme Benoit Date: Mon, 30 Mar 2026 22:59:34 +0000 (+0200) Subject: fix(auth): remove unnecessary async from sync interface methods X-Git-Tag: ocpp-server@v4.1.0~15 X-Git-Url: https://git.piment-noir.org/?a=commitdiff_plain;h=b6db2cab118c7669ccfeedda71509f09825ecab4;p=e-mobility-charging-stations-simulator.git fix(auth): remove unnecessary async from sync interface methods isRemoteAvailable() was declared as boolean | Promise in the OCPPAuthAdapter interface while both implementations return boolean synchronously. This forced unnecessary await, Promise.resolve() wrappers, and async propagation through the call chain. Fix the interface to boolean and cascade through: - RemoteAuthStrategy: getStats(), testConnectivity(), checkRemoteAvailability() become synchronous - AuthStrategy.getStats(): JsonObject | Promise becomes JsonObject - OCPPAuthService.getStats(): Promise becomes AuthStats - OCPPAuthServiceImpl.getStats() becomes synchronous Update mocks and tests to match sync signatures. --- diff --git a/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts b/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts index 35536482..d7782dfc 100644 --- a/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts +++ b/src/charging-station/ocpp/auth/interfaces/OCPPAuthService.ts @@ -165,7 +165,7 @@ export interface AuthStrategy { /** * Get strategy-specific statistics */ - getStats(): JsonObject | Promise + getStats(): JsonObject /** * Initialize the strategy with configuration @@ -370,7 +370,7 @@ export interface OCPPAuthAdapter { /** * Check if remote authorization is available */ - isRemoteAvailable(): boolean | Promise + isRemoteAvailable(): boolean /** * The OCPP version this adapter handles @@ -410,7 +410,7 @@ export interface OCPPAuthService { /** * Get authentication statistics */ - getStats(): Promise + getStats(): AuthStats /** * Invalidate cached authorization for an identifier diff --git a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts index 26672e0e..655e6c89 100644 --- a/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts +++ b/src/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.ts @@ -350,7 +350,7 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { * Get authentication statistics * @returns Authentication statistics including cache and rate limiting metrics */ - public async getStats (): Promise { + public getStats (): AuthStats { const avgResponseTime = this.metrics.totalRequests > 0 ? this.metrics.totalResponseTime / this.metrics.totalRequests @@ -373,7 +373,7 @@ export class OCPPAuthServiceImpl implements OCPPAuthService { | { blockedRequests: number; rateLimitedIdentifiers: number; totalChecks: number } const remoteStrategy = this.strategies.get('remote') if (remoteStrategy?.getStats) { - const strategyStatistics = await remoteStrategy.getStats() + const strategyStatistics = remoteStrategy.getStats() if ('cache' in strategyStatistics) { const cacheStatistics = strategyStatistics.cache as { rateLimit?: { diff --git a/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts b/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts index 7a073503..acf185a6 100644 --- a/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts +++ b/src/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.ts @@ -99,7 +99,7 @@ export class RemoteAuthStrategy implements AuthStrategy { } // Check if remote service is available - const isAvailable = await this.checkRemoteAvailability(adapter, config) + const isAvailable = this.checkRemoteAvailability(adapter, config) if (!isAvailable) { logger.debug(`${moduleName}: Remote service unavailable`) return undefined @@ -223,13 +223,13 @@ export class RemoteAuthStrategy implements AuthStrategy { * Get strategy statistics * @returns Strategy statistics including success rates, response times, and error counts */ - public async getStats (): Promise { + public getStats (): JsonObject { const cacheStatistics = this.authCache ? this.authCache.getStats() : null let adapterAvailable = false if (this.adapter) { try { - adapterAvailable = await this.adapter.isRemoteAvailable() + adapterAvailable = this.adapter.isRemoteAvailable() } catch { adapterAvailable = false } @@ -335,13 +335,13 @@ export class RemoteAuthStrategy implements AuthStrategy { * Test connectivity to remote authorization service * @returns True if the OCPP adapter can reach its remote service */ - public async testConnectivity (): Promise { + public testConnectivity (): boolean { if (!this.isInitialized || this.adapter == null) { return false } try { - return await this.adapter.isRemoteAvailable() + return this.adapter.isRemoteAvailable() } catch { return false } @@ -391,20 +391,12 @@ export class RemoteAuthStrategy implements AuthStrategy { /** * Check if remote authorization service is available * @param adapter - OCPP adapter to check for remote service availability - * @param config - Authentication configuration with timeout settings - * @returns True if the remote service responds within timeout + * @param _config - Authentication configuration (unused) + * @returns True if the remote service responds */ - private async checkRemoteAvailability ( - adapter: OCPPAuthAdapter, - config: AuthConfiguration - ): Promise { + private checkRemoteAvailability (adapter: OCPPAuthAdapter, _config: AuthConfiguration): boolean { try { - const timeout = (config.authorizationTimeout * 1000) / 2 - return await promiseWithTimeout( - Promise.resolve(adapter.isRemoteAvailable()), - timeout, - new AuthenticationError('Availability check timeout', AuthErrorCode.TIMEOUT) - ) + return adapter.isRemoteAvailable() } catch (error) { const errorMessage = getErrorMessage(error) logger.debug(`${moduleName}: Remote availability check failed: ${errorMessage}`) diff --git a/tests/charging-station/ocpp/auth/helpers/MockFactories.ts b/tests/charging-station/ocpp/auth/helpers/MockFactories.ts index cfc863a2..b77b5d92 100644 --- a/tests/charging-station/ocpp/auth/helpers/MockFactories.ts +++ b/tests/charging-station/ocpp/auth/helpers/MockFactories.ts @@ -121,19 +121,16 @@ export const createMockAuthService = (overrides?: Partial): OCP /* empty */ }, getConfiguration: () => ({}) as AuthConfiguration, - getStats: () => - new Promise>(resolve => { - resolve({ - avgResponseTime: 0, - cacheHitRate: 0, - failedAuth: 0, - lastUpdatedDate: new Date(), - localUsageRate: 1, - remoteSuccessRate: 0, - successfulAuth: 0, - totalRequests: 0, - }) - }), + getStats: () => ({ + avgResponseTime: 0, + cacheHitRate: 0, + failedAuth: 0, + lastUpdatedDate: new Date(), + localUsageRate: 1, + remoteSuccessRate: 0, + successfulAuth: 0, + totalRequests: 0, + }), invalidateCache: () => { /* empty */ }, diff --git a/tests/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.test.ts b/tests/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.test.ts index 919ce6cd..655b146e 100644 --- a/tests/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.test.ts +++ b/tests/charging-station/ocpp/auth/services/OCPPAuthServiceImpl.test.ts @@ -182,9 +182,9 @@ await describe('OCPPAuthServiceImpl', async () => { mockStation = createMockAuthServiceTestStation('getStats') }) - await it('should return authentication statistics', async () => { + await it('should return authentication statistics', () => { const authService = new OCPPAuthServiceImpl(mockStation) - const stats = await authService.getStats() + const stats = authService.getStats() assert.notStrictEqual(stats, undefined) assert.notStrictEqual(stats.totalRequests, undefined) diff --git a/tests/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.test.ts b/tests/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.test.ts index 0d34d37f..f6fb2d6a 100644 --- a/tests/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.test.ts +++ b/tests/charging-station/ocpp/auth/strategies/RemoteAuthStrategy.test.ts @@ -416,30 +416,30 @@ await describe('RemoteAuthStrategy', async () => { }) await describe('testConnectivity', async () => { - await it('should test connectivity successfully', async () => { + await it('should test connectivity successfully', () => { strategy.initialize(createTestAuthConfig()) - const result = await strategy.testConnectivity() + const result = strategy.testConnectivity() assert.strictEqual(result, true) }) - await it('should return false when not initialized', async () => { + await it('should return false when not initialized', () => { const newStrategy = new RemoteAuthStrategy() - const result = await newStrategy.testConnectivity() + const result = newStrategy.testConnectivity() assert.strictEqual(result, false) }) - await it('should return false when adapter unavailable', async () => { + await it('should return false when adapter unavailable', () => { mockOCPP16Adapter.isRemoteAvailable = () => false strategy.initialize(createTestAuthConfig()) - const result = await strategy.testConnectivity() + const result = strategy.testConnectivity() assert.strictEqual(result, false) }) }) await describe('getStats', async () => { - await it('should return strategy statistics', async () => { - const stats = await strategy.getStats() + await it('should return strategy statistics', () => { + const stats = strategy.getStats() assert.strictEqual(stats.hasAdapter, true) assert.strictEqual(stats.failedRemoteAuth, 0) assert.strictEqual(stats.hasAuthCache, true) @@ -448,9 +448,9 @@ await describe('RemoteAuthStrategy', async () => { assert.strictEqual(stats.totalRequests, 0) }) - await it('should include adapter statistics', async () => { + await it('should include adapter statistics', () => { strategy.initialize(createTestAuthConfig()) - const stats = await strategy.getStats() + const stats = strategy.getStats() assert.strictEqual(typeof stats.adapterAvailable, 'boolean') }) @@ -463,7 +463,7 @@ await describe('RemoteAuthStrategy', async () => { }) await strategy.authenticate(successRequest, createTestAuthConfig()) - const statsAfterSuccess = await strategy.getStats() + const statsAfterSuccess = strategy.getStats() assert.strictEqual(statsAfterSuccess.totalRequests, 1) assert.strictEqual(statsAfterSuccess.successfulRemoteAuth, 1) assert.strictEqual(statsAfterSuccess.failedRemoteAuth, 0) @@ -477,7 +477,7 @@ await describe('RemoteAuthStrategy', async () => { }) await strategy.authenticate(failRequest, createTestAuthConfig()) - const statsAfterFailure = await strategy.getStats() + const statsAfterFailure = strategy.getStats() assert.strictEqual(statsAfterFailure.totalRequests, 2) assert.strictEqual(statsAfterFailure.successfulRemoteAuth, 1) assert.strictEqual(statsAfterFailure.failedRemoteAuth, 1) @@ -485,9 +485,9 @@ await describe('RemoteAuthStrategy', async () => { }) await describe('cleanup', async () => { - await it('should reset strategy state', async () => { + await it('should reset strategy state', () => { strategy.cleanup() - const stats = await strategy.getStats() + const stats = strategy.getStats() assert.strictEqual(stats.isInitialized, false) assert.strictEqual(stats.totalRequests, 0) }) diff --git a/tests/helpers/OCPPAuthIntegrationTest.ts b/tests/helpers/OCPPAuthIntegrationTest.ts index 499d7337..ef0e5f2d 100644 --- a/tests/helpers/OCPPAuthIntegrationTest.ts +++ b/tests/helpers/OCPPAuthIntegrationTest.ts @@ -251,7 +251,7 @@ export class OCPPAuthIntegrationTest { throw new Error('Invalid connectivity test result') } - const stats = await this.authService.getStats() + const stats = this.authService.getStats() if (typeof stats.totalRequests !== 'number') { throw new Error('Invalid statistics object') }