From 975d5288a3e9811a90e61d624ce10dbae5ae029c Mon Sep 17 00:00:00 2001 From: baldo Date: Thu, 23 Jun 2022 14:26:15 +0200 Subject: [PATCH] Refactoring of sorting and filtering in API. --- server/resources/nodeResource.ts | 16 +----- server/resources/taskResource.ts | 3 +- server/services/mailService.ts | 6 +-- server/services/monitoringService.ts | 72 ++++++++++++--------------- server/types/shared.ts | 73 +++++++++++++++++++++++++++- server/utils/resources.ts | 61 +++++++++++------------ 6 files changed, 140 insertions(+), 91 deletions(-) diff --git a/server/resources/nodeResource.ts b/server/resources/nodeResource.ts index d8ac067..a658303 100644 --- a/server/resources/nodeResource.ts +++ b/server/resources/nodeResource.ts @@ -10,7 +10,7 @@ import {forConstraint, forConstraints} from "../validation/validator"; import * as Resources from "../utils/resources"; import {Entity} from "../utils/resources"; import {Request, Response} from "express"; -import {EnhancedNode, Node} from "../types"; +import {EnhancedNode, isNodeSortField, Node} from "../types"; const nodeFields = ['hostname', 'key', 'email', 'nickname', 'mac', 'coords', 'monitoring']; @@ -130,19 +130,7 @@ async function doGetAll(req: Request): Promise<{ total: number; pageNodes: any } const sortedNodes = Resources.sort( filteredNodes, - [ - 'hostname', - 'nickname', - 'email', - 'token', - 'mac', - 'key', - 'site', - 'domain', - 'coords', - 'onlineState', - 'monitoringState' - ], + isNodeSortField, restParams ); const pageNodes = Resources.getPageEntities(sortedNodes, restParams); diff --git a/server/resources/taskResource.ts b/server/resources/taskResource.ts index 4668129..49d5c75 100644 --- a/server/resources/taskResource.ts +++ b/server/resources/taskResource.ts @@ -8,6 +8,7 @@ import {getTasks, Task, TaskState} from "../jobs/scheduler"; import {normalizeString} from "../utils/strings"; import {forConstraint} from "../validation/validator"; import {Request, Response} from "express"; +import {isTaskSortField} from "../types"; const isValidId = forConstraint(CONSTRAINTS.id, false); @@ -81,7 +82,7 @@ async function doGetAll(req: Request): Promise<{total: number, pageTasks: Entity const tasks = Resources.sort( _.values(getTasks()), - ['id', 'name', 'schedule', 'state', 'runningSince', 'lastRunStarted'], + isTaskSortField, restParams ); const filteredTasks = Resources.filter( diff --git a/server/services/mailService.ts b/server/services/mailService.ts index abda8bb..166a083 100644 --- a/server/services/mailService.ts +++ b/server/services/mailService.ts @@ -9,7 +9,7 @@ import Logger from "../logger"; import * as MailTemplateService from "./mailTemplateService"; import * as Resources from "../utils/resources"; import {RestParams} from "../utils/resources"; -import {Mail, MailData, MailId, MailType} from "../types"; +import {isMailSortField, Mail, MailData, MailId, MailSortField, MailType} from "../types"; const MAIL_QUEUE_DB_BATCH_SIZE = 50; @@ -128,8 +128,8 @@ export async function getPendingMails (restParams: RestParams): Promise<{mails: const filter = Resources.filterClause( restParams, - 'id', - ['id', 'failures', 'sender', 'recipient', 'email', 'created_at', 'modified_at'], + MailSortField.ID, + isMailSortField, ['id', 'failures', 'sender', 'recipient', 'email'] ); diff --git a/server/services/monitoringService.ts b/server/services/monitoringService.ts index c3f3184..ae288fd 100644 --- a/server/services/monitoringService.ts +++ b/server/services/monitoringService.ts @@ -16,7 +16,17 @@ import {normalizeMac} from "../utils/strings"; import {monitoringDisableUrl} from "../utils/urlBuilder"; import CONSTRAINTS from "../validation/constraints"; import {forConstraint} from "../validation/validator"; -import {MAC, MailType, Node, NodeId, OnlineState, NodeStateData, UnixTimestampSeconds} from "../types"; +import { + isMonitoringSortField, + MAC, + MailType, + MonitoringSortField, + Node, + NodeId, + NodeStateData, + OnlineState, + UnixTimestampSeconds +} from "../types"; const MONITORING_STATE_MACS_CHUNK_SIZE = 100; const NEVER_ONLINE_NODES_DELETION_CHUNK_SIZE = 20; @@ -24,12 +34,12 @@ const MONITORING_MAILS_DB_BATCH_SIZE = 50; /** * Defines the intervals emails are sent if a node is offline */ -const MONITORING_OFFLINE_MAILS_SCHEDULE: {[key: number]: {amount: number, unit: unitOfTime.DurationConstructor}} = { - 1: { amount: 3, unit: 'hours' }, - 2: { amount: 1, unit: 'days' }, - 3: { amount: 7, unit: 'days' } +const MONITORING_OFFLINE_MAILS_SCHEDULE: { [key: number]: { amount: number, unit: unitOfTime.DurationConstructor } } = { + 1: {amount: 3, unit: 'hours'}, + 2: {amount: 1, unit: 'days'}, + 3: {amount: 7, unit: 'days'} }; -const DELETE_OFFLINE_NODES_AFTER_DURATION: {amount: number, unit: unitOfTime.DurationConstructor} = { +const DELETE_OFFLINE_NODES_AFTER_DURATION: { amount: number, unit: unitOfTime.DurationConstructor } = { amount: 100, unit: 'days' }; @@ -220,7 +230,7 @@ export function parseNode(importTimestamp: Moment, nodeData: any): ParsedNode { } // TODO: Use sparkson for JSON parsing. -export function parseNodesJson (body: string): NodesParsingResult { +export function parseNodesJson(body: string): NodesParsingResult { Logger.tag('monitoring', 'information-retrieval').debug('Parsing nodes.json...'); const json = JSON.parse(body); @@ -255,8 +265,7 @@ export function parseNodesJson (body: string): NodesParsingResult { const parsedNode = parseNode(result.importTimestamp, nodeData); Logger.tag('monitoring', 'parsing-nodes-json').debug(`Parsing node successful: ${parsedNode.mac}`); result.nodes.push(parsedNode); - } - catch (error) { + } catch (error) { result.failedNodesCount += 1; Logger.tag('monitoring', 'parsing-nodes-json').error("Could not parse node.", error, nodeData); } @@ -414,8 +423,8 @@ async function sendOfflineMails(startTime: Moment, mailNumber: number): Promise< ); } -function doRequest(url: string): Promise<{response: request.Response, body: string}> { - return new Promise<{response: request.Response, body: string}>((resolve, reject) => { +function doRequest(url: string): Promise<{ response: request.Response, body: string }> { + return new Promise<{ response: request.Response, body: string }>((resolve, reject) => { request(url, function (err, response, body) { if (err) { return reject(err); @@ -531,22 +540,7 @@ async function retrieveNodeInformationForUrls(urls: string[]): Promise { - const sortFields = [ - 'id', - 'hostname', - 'mac', - 'site', - 'domain', - 'monitoring_state', - 'state', - 'last_seen', - 'import_timestamp', - 'last_status_mail_type', - 'last_status_mail_sent', - 'created_at', - 'modified_at' - ]; +export async function getAll(restParams: RestParams): Promise<{ total: number, monitoringStates: any[] }> { const filterFields = [ 'hostname', 'mac', @@ -566,8 +560,8 @@ export async function getAll(restParams: RestParams): Promise<{total: number, mo const filter = Resources.filterClause( restParams, - 'id', - sortFields, + MonitoringSortField.ID, + isMonitoringSortField, filterFields ); @@ -579,12 +573,12 @@ export async function getAll(restParams: RestParams): Promise<{total: number, mo return {monitoringStates, total}; } -export async function getByMacs(macs: MAC[]): Promise<{[key: string]: NodeStateData}> { +export async function getByMacs(macs: MAC[]): Promise<{ [key: string]: NodeStateData }> { if (_.isEmpty(macs)) { return {}; } - const nodeStateByMac: {[key: string]: NodeStateData} = {}; + const nodeStateByMac: { [key: string]: NodeStateData } = {}; for (const subMacs of _.chunk(macs, MONITORING_STATE_MACS_CHUNK_SIZE)) { const inCondition = DatabaseUtil.inCondition('mac', subMacs); @@ -648,8 +642,7 @@ export async function sendMonitoringMails(): Promise { try { await sendOnlineAgainMails(startTime); - } - catch (error) { + } catch (error) { // only logging an continuing with next type Logger .tag('monitoring', 'mail-sending') @@ -659,8 +652,7 @@ export async function sendMonitoringMails(): Promise { for (let mailNumber = 1; mailNumber <= 3; mailNumber++) { try { await sendOfflineMails(startTime, mailNumber); - } - catch (error) { + } catch (error) { // only logging an continuing with next type Logger .tag('monitoring', 'mail-sending') @@ -731,7 +723,7 @@ async function deleteNeverOnlineNodesBefore(deleteBefore: UnixTimestampSeconds): ',' ); - const rows: {mac: MAC}[] = await db.all( + const rows: { mac: MAC }[] = await db.all( `SELECT * FROM node_state WHERE mac IN (${placeholders})`, macs ); @@ -746,7 +738,7 @@ async function deleteNeverOnlineNodesBefore(deleteBefore: UnixTimestampSeconds): ' nodes found in monitoring database. Those should be skipped.' ); - const seenMacs: MAC[] = _.map(rows, (row: {mac: MAC}) => row.mac as MAC); + const seenMacs: MAC[] = _.map(rows, (row: { mac: MAC }) => row.mac as MAC); const neverSeenMacs = _.difference(macs, seenMacs); Logger @@ -786,8 +778,7 @@ async function deleteNodeByMac(mac: MAC): Promise { try { node = await NodeService.getNodeDataByMac(mac); - } - catch (error) { + } catch (error) { // Only log error. We try to delete the nodes state anyways. Logger.tag('nodes', 'delete-offline').error('Could not find node to delete: ' + mac, error); } @@ -801,8 +792,7 @@ async function deleteNodeByMac(mac: MAC): Promise { 'DELETE FROM node_state WHERE mac = ? AND state = ?', [mac, 'OFFLINE'], ); - } - catch (error) { + } catch (error) { // Only log error and continue with next node. Logger.tag('nodes', 'delete-offline').error('Could not delete node state: ' + mac, error); } diff --git a/server/types/shared.ts b/server/types/shared.ts index b1676bb..5903603 100644 --- a/server/types/shared.ts +++ b/server/types/shared.ts @@ -3,6 +3,9 @@ import {ArrayField, Field, RawJsonField} from "sparkson"; // Types shared with the client. export type TypeGuard = (arg: unknown) => arg is T; +export type EnumValue = E[keyof E]; +export type EnumTypeGuard = TypeGuard>; + export function isObject(arg: unknown): arg is object { return arg !== null && typeof arg === "object"; } @@ -39,8 +42,8 @@ export function toIsArray(isT: TypeGuard): TypeGuard { return (arg): arg is T[] => isArray(arg, isT); } -export function toIsEnum(enumDef: E): TypeGuard { - return (arg): arg is E => Object.values(enumDef).includes(arg as [keyof E]); +export function toIsEnum(enumDef: E): EnumTypeGuard { + return (arg): arg is EnumValue => Object.values(enumDef).includes(arg as [keyof E]); } export function isOptional(arg: unknown, isT: TypeGuard): arg is (T | undefined) { @@ -333,6 +336,22 @@ export function isEnhancedNode(arg: unknown): arg is EnhancedNode { ); } +export enum NodeSortField { + HOSTNAME = 'hostname', + NICKNAME = 'nickname', + EMAIL = 'email', + TOKEN = 'token', + MAC = 'mac', + KEY = 'key', + SITE = 'site', + DOMAIN = 'domain', + COORDS = 'coords', + ONLINE_STATE = 'onlineState', + MONITORING_STATE = 'monitoringState', +} + +export const isNodeSortField = toIsEnum(NodeSortField); + export interface NodesFilter { hasKey?: boolean; hasCoords?: boolean; @@ -365,3 +384,53 @@ export function isNodesFilter(arg: unknown): arg is NodesFilter { isOptional(filter.onlineState, isOnlineState) ); } + +export enum MonitoringSortField { + ID = 'id', + HOSTNAME = 'hostname', + MAC = 'mac', + SITE = 'site', + DOMAIN = 'domain', + MONITORING_STATE = 'monitoring_state', + STATE = 'state', + LAST_SEEN = 'last_seen', + IMPORT_TIMESTAMP = 'import_timestamp', + LAST_STATUS_MAIL_TYPE = 'last_status_mail_type', + LAST_STATUS_MAIL_SENT = 'last_status_mail_sent', + CREATED_AT = 'created_at', + MODIFIED_AT = 'modified_at', +} + +export const isMonitoringSortField = toIsEnum(MonitoringSortField); + +export enum TaskSortField { + ID = 'id', + NAME = 'name', + SCHEDULE = 'schedule', + STATE = 'state', + RUNNING_SINCE = 'runningSince', + LAST_RUN_STARTED = 'lastRunStarted', +} + +export const isTaskSortField = toIsEnum(TaskSortField); + +export enum MailSortField { + ID = 'id', + FAILURES = 'failures', + SENDER = 'sender', + RECIPIENT = 'recipient', + EMAIL = 'email', + CREATED_AT = 'created_at', + MODIFIED_AT = 'modified_at', +} + +export const isMailSortField = toIsEnum(MailSortField); + +export type GenericSortField = string; + +export enum SortDirection { + ASCENDING = "ASC", + DESCENDING = "DESC", +} + +export const isSortDirection = toIsEnum(SortDirection); diff --git a/server/utils/resources.ts b/server/utils/resources.ts index bc92fe4..38d0904 100644 --- a/server/utils/resources.ts +++ b/server/utils/resources.ts @@ -5,14 +5,15 @@ import ErrorTypes from "../utils/errorTypes"; import Logger from "../logger"; import {Constraints, forConstraints, isConstraints} from "../validation/validator"; import {Request, Response} from "express"; +import {EnumTypeGuard, EnumValue, type GenericSortField, SortDirection, TypeGuard} from "../types"; -export type Entity = {[key: string]: any}; +export type Entity = { [key: string]: any }; export type RestParams = { q?: string; - _sortField?: string; - _sortDir?: string; + _sortField?: GenericSortField; + _sortDir?: SortDirection; _page: number; _perPage: number; @@ -20,36 +21,36 @@ export type RestParams = { filters?: FilterClause; }; -export type OrderByClause = {query: string, params: any[]}; -export type LimitOffsetClause = {query: string, params: any[]}; -export type FilterClause = {query: string, params: any[]}; +export type OrderByClause = { query: string, params: any[] }; +export type LimitOffsetClause = { query: string, params: any[] }; +export type FilterClause = { query: string, params: any[] }; function respond(res: Response, httpCode: number, data: any, type: string): void { switch (type) { case 'html': res.writeHead(httpCode, {'Content-Type': 'text/html'}); res.end(data); - break; + break; default: res.writeHead(httpCode, {'Content-Type': 'application/json'}); res.end(JSON.stringify(data)); - break; + break; } } -function orderByClause( +function orderByClause( restParams: RestParams, - defaultSortField: string, - allowedSortFields: string[] + defaultSortField: EnumValue, + isSortField: EnumTypeGuard, ): OrderByClause { - let sortField = _.includes(allowedSortFields, restParams._sortField) ? restParams._sortField : undefined; + let sortField: EnumValue | undefined = isSortField(restParams._sortField) ? restParams._sortField : undefined; if (!sortField) { sortField = defaultSortField; } return { - query: 'ORDER BY ' + sortField + ' ' + (restParams._sortDir === 'ASC' ? 'ASC' : 'DESC'), + query: 'ORDER BY LOWER(' + sortField + ') ' + (restParams._sortDir === SortDirection.ASCENDING ? 'ASC' : 'DESC'), params: [] }; } @@ -97,8 +98,8 @@ function filterCondition(restParams: RestParams, filterFields: string[]): Filter }; } -function getConstrainedValues(data: {[key: string]: any}, constraints: Constraints): {[key: string]: any} { - const values: {[key: string]: any} = {}; +function getConstrainedValues(data: { [key: string]: any }, constraints: Constraints): { [key: string]: any } { + const values: { [key: string]: any } = {}; _.each(_.keys(constraints), (key: string): void => { const value = data[key]; values[key] = @@ -109,7 +110,7 @@ function getConstrainedValues(data: {[key: string]: any}, constraints: Constrain return values; } -export function getData (req: Request): any { +export function getData(req: Request): any { return _.extend({}, req.body, req.params, req.query); } @@ -118,7 +119,7 @@ export async function getValidRestParams( subtype: string | null, req: Request, ): Promise { - const restConstraints = CONSTRAINTS.rest as {[key: string]: any}; + const restConstraints = CONSTRAINTS.rest as { [key: string]: any }; let constraints: Constraints; if (!(type in restConstraints) || !isConstraints(restConstraints[type])) { Logger.tag('validation', 'rest').error('Unknown REST resource type: {}', type); @@ -129,7 +130,7 @@ export async function getValidRestParams( let filterConstraints: Constraints = {}; if (subtype) { const subtypeFilters = subtype + 'Filters'; - const constraintsObj = CONSTRAINTS as {[key: string]: any}; + const constraintsObj = CONSTRAINTS as { [key: string]: any }; if (!(subtypeFilters in constraintsObj) || !isConstraints(constraintsObj[subtypeFilters])) { Logger.tag('validation', 'rest').error('Unknown REST resource subtype: {}', subtype); throw {data: 'Internal error.', type: ErrorTypes.internalError}; @@ -208,21 +209,21 @@ export function filter(entities: ArrayLike, allowedFilterFields: string[], }); } -export function sort(entities: ArrayLike, allowedSortFields: string[], restParams: RestParams): ArrayLike { - const sortField = _.includes(allowedSortFields, restParams._sortField) ? restParams._sortField : undefined; +export function sort(entities: ArrayLike, isSortField: TypeGuard, restParams: RestParams): ArrayLike { + const sortField: S | undefined = isSortField(restParams._sortField) ? restParams._sortField : undefined; if (!sortField) { return entities; } const sorted: T[] = _.sortBy(entities, [sortField]); - if (restParams._sortDir === 'ASC') { + if (restParams._sortDir === SortDirection.ASCENDING) { return sorted; } else { return _.reverse(sorted); } } -export function getPageEntities (entities: ArrayLike, restParams: RestParams) { +export function getPageEntities(entities: ArrayLike, restParams: RestParams) { const page = restParams._page; const perPage = restParams._perPage; @@ -231,16 +232,16 @@ export function getPageEntities (entities: ArrayLike, restParams: RestPa export {filterCondition as whereCondition}; -export function filterClause ( +export function filterClause( restParams: RestParams, - defaultSortField: string, - allowedSortFields: string[], + defaultSortField: EnumValue, + isSortField: EnumTypeGuard, filterFields: string[], ): FilterClause { - const orderBy = orderByClause( + const orderBy = orderByClause( restParams, defaultSortField, - allowedSortFields + isSortField, ); const limitOffset = limitOffsetClause(restParams); @@ -255,14 +256,14 @@ export function filterClause ( }; } -export function success (res: Response, data: any) { +export function success(res: Response, data: any) { respond(res, 200, data, 'json'); } -export function successHtml (res: Response, html: string) { +export function successHtml(res: Response, html: string) { respond(res, 200, html, 'html'); } -export function error (res: Response, err: {data: any, type: {code: number}}) { +export function error(res: Response, err: { data: any, type: { code: number } }) { respond(res, err.type.code, err.data, 'json'); }