From 7afe0a5e3605e2a017a2c58d75aa9814433c22e9 Mon Sep 17 00:00:00 2001 From: djey47 Date: Wed, 27 Dec 2023 21:43:46 +0100 Subject: [PATCH] refactor(api/powerOn): use common validator for deviceId --- api/src/services/power/powerOn.test.ts | 31 ++++++++++++++++++++++---- api/src/services/power/powerOn.ts | 27 ++++++++++++---------- 2 files changed, 42 insertions(+), 16 deletions(-) diff --git a/api/src/services/power/powerOn.test.ts b/api/src/services/power/powerOn.test.ts index e34c4ee..47845eb 100644 --- a/api/src/services/power/powerOn.test.ts +++ b/api/src/services/power/powerOn.test.ts @@ -1,9 +1,12 @@ +import { FastifyReply } from 'fastify/types/reply'; import globalMocks from '../../../config/jest/globalMocks'; import resetMocks from '../../../config/jest/resetMocks'; import { replyWithInternalError, replyWithItemNotFound, replyWithJson } from '../../common/api'; import { AppContext } from '../../common/context'; -import { getMockedFastifyReply } from '../../helpers/testing/mockObjects'; +import { getDefaultDeviceConfig, getMockedFastifyReply } from '../../helpers/testing/mockObjects'; import { PowerStatus } from '../../models/common'; +import { DeviceConfig } from '../../models/configuration'; +import { validateDeviceIdentifier } from '../common/validators'; import { powerOnForDevice, powerOnForDevicesScheduled } from './powerOn'; import type { WakeOptions } from 'wake_on_lan'; @@ -13,18 +16,22 @@ const { wakeonlanMock } = globalMocks; jest.mock('../../common/api'); jest.mock('../../common/logger', () => ({ coreLogger: { + error: jest.fn(), info: jest.fn(), }, })); +jest.mock('../common/validators'); const replyWithJsonMock = replyWithJson as jest.Mock; const replyWithInternalErrorMock = replyWithInternalError as jest.Mock; const replyWithItemNotFoundMock = replyWithItemNotFound as jest.Mock; +const validateDeviceIdentifierMock = validateDeviceIdentifier as jest.Mock; beforeEach(() => { wakeonlanMock.wake.mockImplementation((_a, _o, cb) => { cb(); - }); + }); + validateDeviceIdentifierMock.mockReturnValue(getDefaultDeviceConfig()); }); afterEach(() => { @@ -32,6 +39,7 @@ afterEach(() => { replyWithJsonMock.mockReset(); replyWithInternalErrorMock.mockReset(); replyWithItemNotFoundMock.mockReset(); + validateDeviceIdentifierMock.mockReset(); }); describe('powerOn service', () => { @@ -86,12 +94,16 @@ describe('powerOn service', () => { }); it('should return 404 when device is not found', async () => { - // given-when + // given + validateDeviceIdentifierMock.mockReturnValue(undefined); + + // when await powerOnForDevice('foo', defaultReply); // then expect(wakeonlanMock.wake).not.toHaveBeenCalled(); - expect(replyWithItemNotFoundMock).toHaveBeenCalledWith(defaultReply, 'deviceId', 'foo'); + expect(replyWithJsonMock).not.toHaveBeenCalled(); + expect(replyWithItemNotFoundMock).not.toHaveBeenCalled(); }); it('should not attempt to wake device nor update diags context and return 204 when already powered ON', async () => { @@ -123,5 +135,16 @@ describe('powerOn service', () => { expect(lastStartAttempt.reason).toBe('scheduled'); expect(wakeonlanMock.wake).toHaveBeenCalledTimes(1); }); + + it('should handle invalid device identifier', async () => { + // given + validateDeviceIdentifierMock.mockReturnValue(undefined); + + // when + await powerOnForDevicesScheduled(['0']); + + // then + expect(wakeonlanMock.wake).not.toHaveBeenCalled(); + }); }); }); diff --git a/api/src/services/power/powerOn.ts b/api/src/services/power/powerOn.ts index ec6bfe7..1216f1c 100644 --- a/api/src/services/power/powerOn.ts +++ b/api/src/services/power/powerOn.ts @@ -1,15 +1,14 @@ import wol from 'wake_on_lan'; -import { getDeviceConfig } from '../../common/configuration'; -import { replyWithInternalError, replyWithItemNotFound, replyWithJson } from '../../common/api'; +import { replyWithInternalError, replyWithJson } from '../../common/api'; import { coreLogger } from '../../common/logger'; import { AppContext } from '../../common/context'; import { ERROR_DEVICE_NOT_FOUND, ERROR_NOOP } from '../common/errors'; +import { validateDeviceIdentifier } from '../common/validators'; import type { FastifyBaseLogger } from 'fastify'; import type { FastifyReply } from 'fastify/types/reply'; import type { WakeOptions } from 'wake_on_lan'; import type { DeviceConfig } from '../../models/configuration'; -import { ApiItem } from '../../models/api'; import { PowerStatus } from '../../models/common'; import { LastPowerAttemptReason } from '../../processors/diag/models/diag'; @@ -20,17 +19,20 @@ export async function powerOnForDevice(deviceId: string, reply: FastifyReply) { const { log: logger } = reply; try { - await powerOnForDeviceBase(deviceId, logger, LastPowerAttemptReason.API); + await powerOnForDeviceBase(deviceId, logger, LastPowerAttemptReason.API, reply); replyWithJson(reply); } catch (error) { if (error === ERROR_DEVICE_NOT_FOUND) { - replyWithItemNotFound(reply, ApiItem.DEVICE_ID, deviceId); - } else if (error === ERROR_NOOP) { + return; + } + + if (error === ERROR_NOOP) { replyWithJson(reply); } else { // WOL error replyWithInternalError(reply, `Unable to perform wake on LAN: ${error}`); } + return; } } @@ -45,20 +47,21 @@ export async function powerOnForDevicesScheduled(deviceIds: string[]) { try { await Promise.all(powerOnPromises); } catch (error) { - coreLogger.error('(powerOnForDevicesScheduled) error when attempting scheduled power ON operation'); + coreLogger.error('(powerOnForDevicesScheduled) error when attempting scheduled power ON operation: %s'); } } -async function powerOnForDeviceBase(deviceId: string, logger: FastifyBaseLogger, reason: LastPowerAttemptReason) { - const deviceConfig = getDeviceConfig(deviceId); +async function powerOnForDeviceBase(deviceId: string, logger: FastifyBaseLogger, reason: LastPowerAttemptReason, reply?: FastifyReply) { + const deviceConfig = validateDeviceIdentifier(deviceId, reply); if (!deviceConfig) { + logger.error('(powerOn::powerOnForDeviceBase) Device not found: %s', deviceId); throw ERROR_DEVICE_NOT_FOUND; } // No need to power on device if its power status is ON already const deviceDiagContext = AppContext.get().diagnostics[deviceId]; if (deviceDiagContext?.power.state === PowerStatus.ON) { - logger.info(`(powerOn::powerOnForDeviceBase) NOOP serverId:${deviceId}`); + logger.info('(powerOn::powerOnForDeviceBase) NOOP deviceId: %s', deviceId); throw ERROR_NOOP; } @@ -71,9 +74,9 @@ async function powerOnForDeviceBase(deviceId: string, logger: FastifyBaseLogger, try { await awakeDevice(deviceConfig); - logger.info(`(powerOn::powerOnForDeviceBase) OK serverId:${deviceId}`); + logger.info('(powerOn::powerOnForDeviceBase) OK deviceId: %s', deviceId); } catch (wolError) { - logger.error(`(powerOn::powerOnForDeviceBase) KO serverId:${deviceId}-${wolError}`); + logger.error('(powerOn::powerOnForDeviceBase) KO deviceId: %s - %s', deviceId, wolError); throw wolError; } }