From e293acb8b654f7bacc283956383033ec91fdb462 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Iv=C3=A1n=20Ovejero?= Date: Wed, 14 Apr 2021 18:00:43 +0200 Subject: [PATCH] :hammer: Refactor per feedback --- packages/cli/src/Interfaces.ts | 15 +-- packages/cli/src/Server.ts | 64 ++++++------- packages/cli/src/TagHelpers.ts | 167 ++++++++++++++++++--------------- 3 files changed, 134 insertions(+), 112 deletions(-) diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 2114341139..21a1a25bd0 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -74,20 +74,23 @@ export interface IWorkflowBase extends IWorkflowBaseWorkflow { id?: number | string; } -export interface ITagBase { - name: string; - createdAt: Date; - updatedAt: Date; +export interface IWorkflowRequest extends Request { + body: { + tags?: string[]; + }; } -export interface ITagDb extends ITagBase { +export interface ITagDb { id: string | number; + name: string; + createdAt?: Date; + updatedAt?: Date; } // Almost identical to editor-ui.Interfaces.ts export interface IWorkflowDb extends IWorkflowBase { id: number | string; - tags: Array<{ id: string | number; name: string }>; + tags: ITagDb[]; } export interface IWorkflowResponse extends IWorkflowBase { diff --git a/packages/cli/src/Server.ts b/packages/cli/src/Server.ts index 44cb3a54f0..492c99a080 100644 --- a/packages/cli/src/Server.ts +++ b/packages/cli/src/Server.ts @@ -107,7 +107,7 @@ import * as querystring from 'querystring'; import * as Queue from '../src/Queue'; import { OptionsWithUrl } from 'request-promise-native'; import { Registry } from 'prom-client'; -import { ITagBase, ITagDb, } from './Interfaces'; +import { ITagDb, IWorkflowRequest } from './Interfaces'; import * as TagHelpers from './TagHelpers'; @@ -485,7 +485,7 @@ class App { // Creates a new workflow - this.app.post(`/${this.restEndpoint}/workflows`, ResponseHelper.send(async (req: express.Request, res: express.Response): Promise => { + this.app.post(`/${this.restEndpoint}/workflows`, ResponseHelper.send(async (req: IWorkflowRequest, res: express.Response): Promise => { const newWorkflowData = req.body as IWorkflowBase; @@ -500,16 +500,21 @@ class App { // Save the workflow in DB const result = await Db.collections.Workflow!.save(newWorkflowData); - const { tags } = req.body as { tags: string | undefined }; + const { tags } = req.body; - if (tags) { - const tagIds = tags.split(','); - await TagHelpers.createRelations(result.id as string, tagIds); - const foundTags = await Db.collections.Tag!.find({ + const tagIds = tags; + const workflowId = result.id as string; + + if (tagIds) { + await TagHelpers.validateTags(tagIds); + await TagHelpers.createRelations(workflowId, tagIds); + + const found = await Db.collections.Tag!.find({ select: ['id', 'name'], where: { id: In(tagIds) }, }); - result.tags = foundTags.map(({ id, name }) => ({ id: id.toString(), name })); + + result.tags = TagHelpers.stringifyId(found); } // Convert to response format in which the id is a string @@ -561,7 +566,7 @@ class App { const results = await Db.collections.Workflow!.find(findQuery); results.forEach(workflow => { if (workflow.tags) { - workflow.tags = workflow.tags.map(({ id, name }) => ({ id: id.toString(), name })); + workflow.tags = TagHelpers.stringifyId(workflow.tags); } }); @@ -591,9 +596,8 @@ class App { // Updates an existing workflow - this.app.patch(`/${this.restEndpoint}/workflows/:id`, ResponseHelper.send(async (req: express.Request, res: express.Response): Promise => { - - const { tags } = req.body as { tags: string | undefined }; + this.app.patch(`/${this.restEndpoint}/workflows/:id`, ResponseHelper.send(async (req: IWorkflowRequest, res: express.Response): Promise => { + const { tags } = req.body; const newWorkflowData = _.omit(req.body, ['tags']) as IWorkflowBase; const id = req.params.id; @@ -663,23 +667,22 @@ class App { } } - if (tags) { - await TagHelpers.deleteAllTagsForWorkflow(id); + const workflowId = id; + const tagIds = tags; - const tagIds = tags.split(','); + if (tagIds) { + await TagHelpers.validateTags(tagIds); + await TagHelpers.validateNotRelated(workflowId, tagIds); - for (const tagId of tagIds) { - await TagHelpers.validateId(tagId); - } + await TagHelpers.removeRelations(workflowId); + await TagHelpers.createRelations(workflowId, tagIds); - await TagHelpers.createRelations(id, tagIds); - - const foundTags = await Db.collections.Tag!.find({ + const found = await Db.collections.Tag!.find({ select: ['id', 'name'], where: { id: In(tagIds) }, }); - responseData.tags = foundTags.map(({ id, name }) => ({ id: id.toString(), name })); + responseData.tags = TagHelpers.stringifyId(found); } // Convert to response format in which the id is a string @@ -767,13 +770,13 @@ class App { // Creates a tag this.app.post(`/${this.restEndpoint}/tags`, ResponseHelper.send(async (req: express.Request, res: express.Response): Promise<{ id: string, name: string }> => { - TagHelpers.validateRequestBody(req.body); - const { name } = req.body; + console.log('------------------------'); + console.log(typeof name); + console.log('------------------------'); await TagHelpers.validateName(name); - TagHelpers.validateLength(name); - const newTag: ITagBase = { + const newTag: Partial = { name, createdAt: this.getCurrentDate(), updatedAt: this.getCurrentDate(), @@ -787,21 +790,18 @@ class App { // Deletes a tag this.app.delete(`/${this.restEndpoint}/tags/:id`, ResponseHelper.send(async (req: express.Request, res: express.Response): Promise => { const { id } = req.params; - await TagHelpers.validateId(id); + await TagHelpers.exists(id); await Db.collections.Tag!.delete({ id }); return true; })); - // Updates an existing tag + // Updates a tag this.app.patch(`/${this.restEndpoint}/tags/:id`, ResponseHelper.send(async (req: express.Request, res: express.Response): Promise<{ id: string, name: string }> => { - TagHelpers.validateRequestBody(req.body); - const { name } = req.body; await TagHelpers.validateName(name); - TagHelpers.validateLength(name); const { id } = req.params; - await TagHelpers.validateId(id); + await TagHelpers.exists(id); const updatedTag: Partial = { name, diff --git a/packages/cli/src/TagHelpers.ts b/packages/cli/src/TagHelpers.ts index e872a60fe4..1af476dea9 100644 --- a/packages/cli/src/TagHelpers.ts +++ b/packages/cli/src/TagHelpers.ts @@ -1,97 +1,36 @@ import { - FindOneOptions, getConnection, - In, } from "typeorm"; import { Db, ITagDb, - IWorkflowDb, ResponseHelper, } from "."; - // ---------------------------------- -// validators +// utils // ---------------------------------- /** - * Validate whether a tag ID exists so that it can be used for a workflow create or tag update operation. + * Type guard for string array. */ -export async function validateId(id: string): Promise | never { - const findQuery = { where: { id } } as FindOneOptions; - const tag = await Db.collections.Tag!.findOne(findQuery); - - if (!tag) { - throw new ResponseHelper.ResponseError(`Tag with ID ${id} does not exist.`, undefined, 400); - } +function isStringArray(tags: unknown[]): tags is string[] { + return Array.isArray(tags) && !tags.some((value) => typeof value !== 'string'); } /** - * Validate whether a tag name has 1 to 24 characters. + * Stringify the ID in every `ITagDb` in an array. + * Side effect: Remove `createdAt` and `updatedAt` for a slimmer response. */ -export function validateLength(name: string): void | never { - if (name.length <= 0 || name.length > 24) { - throw new ResponseHelper.ResponseError('Tag name must be 1 to 24 characters long.', undefined, 400); - } +export function stringifyId(tags: ITagDb[]) { + return tags.map(({ id, name }) => ({ id: id.toString(), name })); } /** - * Validate whether a tag name exists so that it cannot be used for a tag create or tag update operation. + * Check if a workflow and a tag are related. */ -export async function validateName(name: string): Promise | never { - const findQuery = { where: { name } } as FindOneOptions; - const tag = await Db.collections.Tag!.findOne(findQuery); - - if (tag) { - throw new ResponseHelper.ResponseError('Tag name already exists.', undefined, 400); - } -} - -/** - * Validate that a tag and a workflow are not related so that a link can be created. - */ -export async function validateNoRelation(workflowId: number, tagId: number): Promise | never { - const areRelated = await checkRelated(workflowId, tagId); - - if (areRelated) { - throw new ResponseHelper.ResponseError(`Workflow ID ${workflowId} and tag ID ${tagId} are already related.`, undefined, 400); - } -} - -/** - * Validate that a tag and a workflow are related so that their link can be deleted. - */ -export async function validateRelation(workflowId: number, tagId: number): Promise | never { - const areRelated = await checkRelated(workflowId, tagId); - - if (!areRelated) { - throw new ResponseHelper.ResponseError(`Workflow ID ${workflowId} and tag ID ${tagId} are not related.`, undefined, 400); - } -} - -/** - * Validate whether the request body for a create/update operation has a `name` property. - */ -export function validateRequestBody({ name }: { name: string | undefined }): void | never { - if (!name) { - throw new ResponseHelper.ResponseError(`Property 'name' missing from request body.`, undefined, 400); - } -} - - -// ---------------------------------- -// queries -// ---------------------------------- - -/** - * Check if a workflow and a tag are related. Used only in validators. - */ -async function checkRelated( - workflowId: number, - tagId: number -): Promise { +async function checkRelated(workflowId: string, tagId: string): Promise { const result = await getConnection().createQueryBuilder() .select() .from('workflows_tags', 'workflows_tags') @@ -102,8 +41,88 @@ async function checkRelated( } /** - * Retrieve all existing tags, whether linked to a workflow or not, - * including how many workflows each tag is linked to. + * Check whether a tag ID exists in the `tag_entity` table. + * + * Used for creating a workflow or updating a tag. + */ +export async function exists(id: string): Promise | never { + const tag = await Db.collections.Tag!.findOne({ where: { id }}); + + if (!tag) { + throw new ResponseHelper.ResponseError(`Tag with ID ${id} does not exist.`, undefined, 400); + } +} + +// ---------------------------------- +// validators +// ---------------------------------- + +/** + * Validate whether every tag ID is a string array and exists in the `tag_entity` table. + * + * Used for creating a workflow or updating a tag. + */ +export async function validateTags(tags: unknown[]): Promise | never { + if (!isStringArray(tags)) { + throw new ResponseHelper.ResponseError(`The tags property is not an array of strings.`, undefined, 400); + } + + for (const tagId of tags) { + await exists(tagId); + } +} + +/** + * Validate whether a tag name + * - is present in the request body, + * - is a string, + * - is 1 to 24 characters long, and + * - does not exist already. + * + * Used for creating or updating a tag. + */ +export async function validateName(name: unknown): Promise | never { + if (name === undefined) { + throw new ResponseHelper.ResponseError(`Property 'name' missing from request body.`, undefined, 400); + } + + if (typeof name !== 'string') { + throw new ResponseHelper.ResponseError(`Property 'name' must be a string.`, undefined, 400); + } + + if (name.length <= 0 || name.length > 24) { + throw new ResponseHelper.ResponseError('Tag name must be 1 to 24 characters long.', undefined, 400); + } + + const tag = await Db.collections.Tag!.findOne({ where: { name } }); + + if (tag) { + throw new ResponseHelper.ResponseError('Tag name already exists.', undefined, 400); + } +} + +/** + * Validate that the provided tags are related to a workflow. + * + * Used for relating the tags and the workflow. + */ +export async function validateNotRelated(workflowId: string, tags: string[]): Promise | never { + tags.forEach(async tagId => { + const areRelated = await checkRelated(workflowId, tagId); + + if (areRelated) { + throw new ResponseHelper.ResponseError(`Workflow ID ${workflowId} and tag ID ${tagId} are already related.`, undefined, 400); + } + }); +} + +// ---------------------------------- +// queries +// ---------------------------------- + +/** + * Retrieve all existing tags, whether related to a workflow or not, + * including how many workflows each tag is related to. */ export async function getAllTagsWithUsageCount(): Promise