🔨 Refactor per feedback

This commit is contained in:
Iván Ovejero 2021-04-14 18:00:43 +02:00
parent 18794942f2
commit e293acb8b6
3 changed files with 134 additions and 112 deletions

View file

@ -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 {

View file

@ -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<IWorkflowResponse> => {
this.app.post(`/${this.restEndpoint}/workflows`, ResponseHelper.send(async (req: IWorkflowRequest, res: express.Response): Promise<IWorkflowResponse> => {
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<IWorkflowResponse> => {
const { tags } = req.body as { tags: string | undefined };
this.app.patch(`/${this.restEndpoint}/workflows/:id`, ResponseHelper.send(async (req: IWorkflowRequest, res: express.Response): Promise<IWorkflowResponse> => {
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<ITagDb> = {
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<boolean> => {
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<ITagDb> = {
name,

View file

@ -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<void> | 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<void> | 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<void> | 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<void> | 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<boolean> {
async function checkRelated(workflowId: string, tagId: string): Promise<boolean> {
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<void> | 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<void> | 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<void> | 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<void> | 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<Array<{
id: number;
@ -156,7 +175,7 @@ export async function createRelations(workflowId: string, tagIds: string[]) {
/**
* Remove all tags for a workflow during a tag update operation.
*/
export async function deleteAllTagsForWorkflow(workflowId: string) {
export async function removeRelations(workflowId: string) {
await getConnection().createQueryBuilder()
.delete()
.from('workflows_tags')