Refactoring of sorting and filtering in API.

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,
-        [
+        isNodeSortField,
-            'hostname',
-            'nickname',
-            'email',
-            'token',
-            'mac',
-            'key',
-            'site',
-            'domain',
-            'coords',
-            'onlineState',
-            'monitoringState'
-        ],
         restParams
     );
     const pageNodes = Resources.getPageEntities(sortedNodes, restParams);

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(

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',
+        MailSortField.ID,
-        ['id', 'failures', 'sender', 'recipient', 'email', 'created_at', 'modified_at'],
+        isMailSortField,
         ['id', 'failures', 'sender', 'recipient', 'email']
     );
 

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}} = {
+const MONITORING_OFFLINE_MAILS_SCHEDULE: { [key: number]: { amount: number, unit: unitOfTime.DurationConstructor } } = {
-    1: { amount: 3, unit: 'hours' },
+    1: {amount: 3, unit: 'hours'},
-    2: { amount: 1, unit: 'days' },
+    2: {amount: 1, unit: 'days'},
-    3: { amount: 7, 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}> {
+function doRequest(url: string): Promise<{ response: request.Response, body: string }> {
-    return new Promise<{response: request.Response, body: string}>((resolve, reject) => {
+    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<RetrieveN
     }
 }
 
-export async function getAll(restParams: RestParams): Promise<{total: number, monitoringStates: any[]}> {
+export async function getAll(restParams: RestParams): Promise<{ total: number, monitoringStates: any[] }> {
-    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'
-    ];
     const filterFields = [
         'hostname',
         'mac',
@@ -566,8 +560,8 @@ export async function getAll(restParams: RestParams): Promise<{total: number, mo
 
     const filter = Resources.filterClause(
         restParams,
-        'id',
+        MonitoringSortField.ID,
-        sortFields,
+        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<void> {
 
     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<void> {
     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<void> {
 
     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<void> {
             '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);
     }

server/types/shared.ts

@@ -3,6 +3,9 @@ import {ArrayField, Field, RawJsonField} from "sparkson";
 // Types shared with the client.
 export type TypeGuard<T> = (arg: unknown) => arg is T;
 
+export type EnumValue<E> = E[keyof E];
+export type EnumTypeGuard<E> = TypeGuard<EnumValue<E>>;
+
 export function isObject(arg: unknown): arg is object {
     return arg !== null && typeof arg === "object";
 }
@@ -39,8 +42,8 @@ export function toIsArray<T>(isT: TypeGuard<T>): TypeGuard<T[]> {
     return (arg): arg is T[] => isArray(arg, isT);
 }
 
-export function toIsEnum<E>(enumDef: E): TypeGuard<E> {
+export function toIsEnum<E>(enumDef: E): EnumTypeGuard<E> {
-    return (arg): arg is E => Object.values(enumDef).includes(arg as [keyof E]);
+    return (arg): arg is EnumValue<E> => Object.values(enumDef).includes(arg as [keyof E]);
 }
 
 export function isOptional<T>(arg: unknown, isT: TypeGuard<T>): 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);

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;
+    _sortField?: GenericSortField;
-    _sortDir?: string;
+    _sortDir?: SortDirection;
 
     _page: number;
     _perPage: number;
@@ -20,9 +21,9 @@ export type RestParams = {
     filters?: FilterClause;
 };
 
-export type OrderByClause = {query: string, params: any[]};
+export type OrderByClause = { query: string, params: any[] };
-export type LimitOffsetClause = {query: string, params: any[]};
+export type LimitOffsetClause = { query: string, params: any[] };
-export type FilterClause = {query: string, params: any[]};
+export type FilterClause = { query: string, params: any[] };
 
 function respond(res: Response, httpCode: number, data: any, type: string): void {
     switch (type) {
@@ -38,18 +39,18 @@ function respond(res: Response, httpCode: number, data: any, type: string): void
     }
 }
 
-function orderByClause(
+function orderByClause<S>(
     restParams: RestParams,
-    defaultSortField: string,
+    defaultSortField: EnumValue<S>,
-    allowedSortFields: string[]
+    isSortField: EnumTypeGuard<S>,
 ): OrderByClause {
-    let sortField = _.includes(allowedSortFields, restParams._sortField) ? restParams._sortField : undefined;
+    let sortField: EnumValue<S> | 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} {
+function getConstrainedValues(data: { [key: string]: any }, constraints: Constraints): { [key: string]: any } {
-    const values: {[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<RestParams> {
-    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<E>(entities: ArrayLike<E>, allowedFilterFields: string[],
     });
 }
 
-export function sort<T>(entities: ArrayLike<T>, allowedSortFields: string[], restParams: RestParams): ArrayLike<T> {
+export function sort<T, S>(entities: ArrayLike<T>, isSortField: TypeGuard<S>, restParams: RestParams): ArrayLike<T> {
-    const sortField = _.includes(allowedSortFields, restParams._sortField) ? restParams._sortField : undefined;
+    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<Entity>, restParams: RestParams) {
+export function getPageEntities(entities: ArrayLike<Entity>, restParams: RestParams) {
     const page = restParams._page;
     const perPage = restParams._perPage;
 
@@ -231,16 +232,16 @@ export function getPageEntities (entities: ArrayLike<Entity>, restParams: RestPa
 
 export {filterCondition as whereCondition};
 
-export function filterClause (
+export function filterClause<S>(
     restParams: RestParams,
-    defaultSortField: string,
+    defaultSortField: EnumValue<S>,
-    allowedSortFields: string[],
+    isSortField: EnumTypeGuard<S>,
     filterFields: string[],
 ): FilterClause {
-    const orderBy = orderByClause(
+    const orderBy = orderByClause<S>(
         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');
 }