Skip to content

Commit

Permalink
refactor(api/powerOn): use common validator for deviceId
Browse files Browse the repository at this point in the history
  • Loading branch information
djey47 committed Dec 27, 2023
1 parent a031098 commit 618f939
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 13 deletions.
31 changes: 27 additions & 4 deletions api/src/services/power/powerOn.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -13,25 +16,30 @@ 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<DeviceConfig | undefined, [string, FastifyReply?]>;

beforeEach(() => {
wakeonlanMock.wake.mockImplementation((_a, _o, cb) => {
cb();
});
});
validateDeviceIdentifierMock.mockReturnValue(getDefaultDeviceConfig());
});

afterEach(() => {
resetMocks();
replyWithJsonMock.mockReset();
replyWithInternalErrorMock.mockReset();
replyWithItemNotFoundMock.mockReset();
validateDeviceIdentifierMock.mockReset();
});

describe('powerOn service', () => {
Expand Down Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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();
});
});
});
23 changes: 14 additions & 9 deletions api/src/services/power/powerOn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { DeviceConfig } from '../../models/configuration';
import { ApiItem } from '../../models/api';

Check warning on line 12 in api/src/services/power/powerOn.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

'ApiItem' is defined but never used. Allowed unused vars must match /_/u

Check warning on line 12 in api/src/services/power/powerOn.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

'ApiItem' is defined but never used. Allowed unused vars must match /_/u

Check warning on line 12 in api/src/services/power/powerOn.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

'ApiItem' is defined but never used. Allowed unused vars must match /_/u

Check warning on line 12 in api/src/services/power/powerOn.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

'ApiItem' is defined but never used. Allowed unused vars must match /_/u
import { PowerStatus } from '../../models/common';
import { LastPowerAttemptReason } from '../../processors/diag/models/diag';
import { validateDeviceIdentifier } from '../common/validators';

/**
* Power->ON service implementation: for a specified device
Expand All @@ -20,17 +21,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;
}
}
Expand All @@ -45,20 +49,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;
}

Expand All @@ -71,9 +76,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;
}
}
Expand Down

0 comments on commit 618f939

Please sign in to comment.