From aafbbb5d523ba074604a8a676c110a572a957455 Mon Sep 17 00:00:00 2001 From: trivernis Date: Mon, 20 Jan 2020 23:36:50 +0100 Subject: [PATCH] Cleanup code and http 404 for not found - Add 404 status code return for all not found errors --- src/graphql/BlacklistedResult.ts | 6 +++- src/graphql/MutationResolver.ts | 4 +-- src/graphql/QueryResolver.ts | 35 +++++++++-------------- src/graphql/SearchResult.ts | 5 ++-- src/graphql/Token.ts | 3 +- src/lib/dataAccess.ts | 25 +++++++++++----- src/lib/errors/ActivityNotFoundError.ts | 3 ++ src/lib/errors/BlacklistedError.ts | 3 ++ src/lib/errors/ChatNotFoundError.ts | 3 ++ src/lib/errors/GroupNotFoundError.ts | 3 ++ src/lib/errors/NoActionSpecifiedError.ts | 4 +++ src/lib/errors/NotAGroupAdminError.ts | 4 +-- src/lib/errors/NotTheGroupCreatorError.ts | 1 - src/lib/errors/PostNotFoundError.ts | 3 ++ src/lib/errors/RequestNotFoundError.ts | 3 ++ src/lib/errors/UserNotFoundError.ts | 3 ++ src/lib/errors/graphqlErrors.ts | 18 ------------ src/lib/markdown.ts | 2 +- src/lib/models/BlacklistedPhrase.ts | 12 +------- src/lib/models/Event.ts | 2 +- src/lib/models/Group.ts | 2 +- src/lib/models/Post.ts | 1 - src/routes/UploadRoute.ts | 1 - 23 files changed, 74 insertions(+), 72 deletions(-) diff --git a/src/graphql/BlacklistedResult.ts b/src/graphql/BlacklistedResult.ts index 4139424..8153bd1 100644 --- a/src/graphql/BlacklistedResult.ts +++ b/src/graphql/BlacklistedResult.ts @@ -1,6 +1,10 @@ +/** + * A result of a query to check if a phrase contains blacklisted phrases + */ export class BlacklistedResult { constructor( public blacklisted: boolean, public phrases: string[], - ) {} + ) { + } } diff --git a/src/graphql/MutationResolver.ts b/src/graphql/MutationResolver.ts index f70de4d..236f067 100644 --- a/src/graphql/MutationResolver.ts +++ b/src/graphql/MutationResolver.ts @@ -425,7 +425,7 @@ export class MutationResolver extends BaseResolver { * @param languageCode * @param request */ - public async addToBlacklist({phrase, languageCode}: {phrase: string, languageCode?: string}, request: any): + public async addToBlacklist({phrase, languageCode}: { phrase: string, languageCode?: string }, request: any): Promise { this.ensureLoggedIn(request); const user = await User.findByPk(request.session.userId); @@ -448,7 +448,7 @@ export class MutationResolver extends BaseResolver { * @param languageCode * @param request */ - public async removeFromBlacklist({phrase, languageCode}: {phrase: string, languageCode: string}, request: any): + public async removeFromBlacklist({phrase, languageCode}: { phrase: string, languageCode: string }, request: any): Promise { this.ensureLoggedIn(request); const user = await User.findByPk(request.session.userId); diff --git a/src/graphql/QueryResolver.ts b/src/graphql/QueryResolver.ts index 04d7cd8..4b0cce6 100644 --- a/src/graphql/QueryResolver.ts +++ b/src/graphql/QueryResolver.ts @@ -6,16 +6,7 @@ import {PostNotFoundGqlError} from "../lib/errors/graphqlErrors"; import {GroupNotFoundError} from "../lib/errors/GroupNotFoundError"; import {RequestNotFoundError} from "../lib/errors/RequestNotFoundError"; import {UserNotFoundError} from "../lib/errors/UserNotFoundError"; -import { - Activity, - BlacklistedPhrase, - ChatRoom, - Event, - Group, - Post, - Request, - User, -} from "../lib/models"; +import {Activity, BlacklistedPhrase, ChatRoom, Event, Group, Post, Request, User} from "../lib/models"; import {BlacklistedResult} from "./BlacklistedResult"; import {MutationResolver} from "./MutationResolver"; import {SearchResult} from "./SearchResult"; @@ -31,7 +22,7 @@ export class QueryResolver extends MutationResolver { * @param userId * @param handle */ - public async getUser({userId, handle}: {userId?: number, handle?: string}): Promise { + public async getUser({userId, handle}: { userId?: number, handle?: string }): Promise { let user: User; if (userId) { user = await User.findByPk(userId); @@ -52,7 +43,7 @@ export class QueryResolver extends MutationResolver { * @param args * @param request */ - public async getSelf(args: null, request: any): Promise { + public async getSelf(args: null, request: any): Promise { this.ensureLoggedIn(request); return User.findByPk(request.session.userId); } @@ -61,7 +52,7 @@ export class QueryResolver extends MutationResolver { * Returns a post for a given post id. * @param postId */ - public async getPost({postId}: {postId: number}): Promise { + public async getPost({postId}: { postId: number }): Promise { const post = await Post.findByPk(postId); if (post) { return post; @@ -74,7 +65,7 @@ export class QueryResolver extends MutationResolver { * Returns a chat for a given chat id * @param chatId */ - public async getChat({chatId}: {chatId: number}): Promise { + public async getChat({chatId}: { chatId: number }): Promise { const chat = await ChatRoom.findByPk(chatId); if (chat) { return chat; @@ -87,7 +78,7 @@ export class QueryResolver extends MutationResolver { * Returns a group for a given group id. * @param groupId */ - public async getGroup({groupId}: {groupId: number}): Promise { + public async getGroup({groupId}: { groupId: number }): Promise { const group = await Group.findByPk(groupId); if (group) { return group; @@ -100,7 +91,7 @@ export class QueryResolver extends MutationResolver { * Returns the request for a given id. * @param requestId */ - public async getRequest({requestId}: {requestId: number}): Promise { + public async getRequest({requestId}: { requestId: number }): Promise { const request = await Request.findByPk(requestId); if (request) { return request; @@ -115,7 +106,7 @@ export class QueryResolver extends MutationResolver { * @param first * @param offset */ - public async search({query, first, offset}: {query: number, first: number, offset: number}): Promise { + public async search({query, first, offset}: { query: number, first: number, offset: number }): Promise { const limit = first; const users = await User.findAll({ limit, @@ -151,7 +142,7 @@ export class QueryResolver extends MutationResolver { * @param offset * @param sort */ - public async getPosts({first, offset, sort}: {first: number, offset: number, sort: dataaccess.SortType}): + public async getPosts({first, offset, sort}: { first: number, offset: number, sort: dataaccess.SortType }): Promise { return await dataaccess.getPosts(first, offset, sort); } @@ -160,7 +151,7 @@ export class QueryResolver extends MutationResolver { * Returns all activities */ public async getActivities(): Promise { - return Activity.findAll(); + return Activity.findAll(); } /** @@ -168,7 +159,7 @@ export class QueryResolver extends MutationResolver { * @param email * @param passwordHash */ - public async getToken({email, passwordHash}: {email: string, passwordHash: string}): Promise { + public async getToken({email, passwordHash}: { email: string, passwordHash: string }): Promise { const user = await dataaccess.getUserByLogin(email, passwordHash); return new Token(await user.token(), Number(user.authExpire).toString()); } @@ -177,7 +168,7 @@ export class QueryResolver extends MutationResolver { * Returns if a input phrase contains blacklisted phrases and which one * @param phrase */ - public async blacklisted({phrase}: {phrase: string}): Promise { + public async blacklisted({phrase}: { phrase: string }): Promise { const phrases = await dataaccess.checkBlacklisted(phrase); return new BlacklistedResult(phrases.length > 0, phrases .map((p) => p.phrase)); @@ -188,7 +179,7 @@ export class QueryResolver extends MutationResolver { * @param first * @param offset */ - public async getBlacklistedPhrases({first, offset}: {first: number, offset: number}): Promise { + public async getBlacklistedPhrases({first, offset}: { first: number, offset: number }): Promise { return (await BlacklistedPhrase.findAll({limit: first, offset})) .map((p) => p.phrase); } diff --git a/src/graphql/SearchResult.ts b/src/graphql/SearchResult.ts index 739f023..bee76ef 100644 --- a/src/graphql/SearchResult.ts +++ b/src/graphql/SearchResult.ts @@ -1,4 +1,4 @@ -import {Group, Post, User, Event} from "../lib/models"; +import {Event, Group, Post, User} from "../lib/models"; /** * A class to wrap search results returned by the search resolver @@ -9,5 +9,6 @@ export class SearchResult { public groups: Group[], public posts: Post[], public events: Event[], - ) {} + ) { + } } diff --git a/src/graphql/Token.ts b/src/graphql/Token.ts index f56d369..af91a94 100644 --- a/src/graphql/Token.ts +++ b/src/graphql/Token.ts @@ -5,5 +5,6 @@ export class Token { constructor( public value: string, public expires: string, - ) {} + ) { + } } diff --git a/src/lib/dataAccess.ts b/src/lib/dataAccess.ts index 8fc11c9..3b1ad3b 100644 --- a/src/lib/dataAccess.ts +++ b/src/lib/dataAccess.ts @@ -1,5 +1,4 @@ import * as crypto from "crypto"; -import {GraphQLError} from "graphql"; import * as sqz from "sequelize"; import {Sequelize} from "sequelize-typescript"; import {ActivityNotFoundError} from "./errors/ActivityNotFoundError"; @@ -166,11 +165,20 @@ namespace dataaccess { } else { // more performant way to get the votes with plain sql return await sequelize.query( - `SELECT * FROM ( - SELECT *, - (SELECT count(*) FROM post_votes WHERE vote_type = 'UPVOTE' AND post_id = posts.id) AS upvotes , - (SELECT count(*) FROM post_votes WHERE vote_type = 'DOWNVOTE' AND post_id = posts.id) AS downvotes - FROM posts) AS a ORDER BY (a.upvotes - a.downvotes) DESC, a.upvotes DESC, a.id LIMIT ? OFFSET ?`, + `SELECT * + FROM ( + SELECT *, + (SELECT count(*) + FROM post_votes + WHERE vote_type = 'UPVOTE' AND post_id = posts.id) AS upvotes, + (SELECT count(*) + FROM post_votes + WHERE vote_type = 'DOWNVOTE' AND post_id = posts.id) AS downvotes + FROM posts) AS a + ORDER BY (a.upvotes - a.downvotes) DESC, a.upvotes DESC, a.id + LIMIT ? + OFFSET + ?`, {replacements: [first, offset], mapToModel: true, model: models.Post}) as models.Post[]; } } @@ -359,7 +367,10 @@ namespace dataaccess { export async function checkBlacklisted(phrase: string, language: string = "en"): Promise { return sequelize.query(` - SELECT * FROM blacklisted_phrases WHERE ? ~* phrase AND language = ?`, + SELECT * + FROM blacklisted_phrases + WHERE ? ~* phrase + AND language = ?`, {replacements: [phrase, language], mapToModel: true, model: BlacklistedPhrase}); } diff --git a/src/lib/errors/ActivityNotFoundError.ts b/src/lib/errors/ActivityNotFoundError.ts index fb0f5c9..2b51fe8 100644 --- a/src/lib/errors/ActivityNotFoundError.ts +++ b/src/lib/errors/ActivityNotFoundError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when an activity was not found. */ export class ActivityNotFoundError extends BaseError { + + public readonly statusCode = httpStatus.NOT_FOUND; + constructor(id: number) { super(`The activity with the id ${id} could not be found.`); } diff --git a/src/lib/errors/BlacklistedError.ts b/src/lib/errors/BlacklistedError.ts index a4a76c2..a82cf5c 100644 --- a/src/lib/errors/BlacklistedError.ts +++ b/src/lib/errors/BlacklistedError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * Represents an error that is thrown when a blacklisted phrase is used. */ export class BlacklistedError extends BaseError { + + public readonly statusCode = httpStatus.NOT_ACCEPTABLE; + constructor(public phrases: string[], field: string = "input") { super(`The ${field} contains the blacklisted words: ${phrases.join(", ")}`); } diff --git a/src/lib/errors/ChatNotFoundError.ts b/src/lib/errors/ChatNotFoundError.ts index 17042dc..7af3d38 100644 --- a/src/lib/errors/ChatNotFoundError.ts +++ b/src/lib/errors/ChatNotFoundError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when the chatroom doesn't exist */ export class ChatNotFoundError extends BaseError { + + public readonly statusCode = httpStatus.NOT_FOUND; + constructor(chatId: number) { super(`Chat with id ${chatId} not found.`); } diff --git a/src/lib/errors/GroupNotFoundError.ts b/src/lib/errors/GroupNotFoundError.ts index f63e119..2e618bc 100644 --- a/src/lib/errors/GroupNotFoundError.ts +++ b/src/lib/errors/GroupNotFoundError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when a group was not found for a specified id */ export class GroupNotFoundError extends BaseError { + + public readonly statusCode = httpStatus.NOT_FOUND; + constructor(groupId: number) { super(`Group ${groupId} not found!`); } diff --git a/src/lib/errors/NoActionSpecifiedError.ts b/src/lib/errors/NoActionSpecifiedError.ts index f18fed4..797ffde 100644 --- a/src/lib/errors/NoActionSpecifiedError.ts +++ b/src/lib/errors/NoActionSpecifiedError.ts @@ -4,6 +4,10 @@ import {BaseError} from "./BaseError"; * An error that is thrown when no action was specified on a group membership change */ export class NoActionSpecifiedError extends BaseError { + + public readonly statusCode = httpStatus.NO_CONTENT; + + // @ts-ignore constructor(actions?: any) { if (actions) { super(`No action of '${Object.keys(actions).join(", ")}'`); diff --git a/src/lib/errors/NotAGroupAdminError.ts b/src/lib/errors/NotAGroupAdminError.ts index 2538c37..f125853 100644 --- a/src/lib/errors/NotAGroupAdminError.ts +++ b/src/lib/errors/NotAGroupAdminError.ts @@ -1,11 +1,11 @@ -import * as status from "http-status"; +import * as httpStatus from "http-status"; import {BaseError} from "./BaseError"; /** * An error that is thrown when a non-admin tries to perform an admin action */ export class NotAGroupAdminError extends BaseError { - public readonly statusCode = status.FORBIDDEN; + public readonly statusCode = httpStatus.FORBIDDEN; constructor(groupId: number) { super(`You are not an admin of '${groupId}'`); diff --git a/src/lib/errors/NotTheGroupCreatorError.ts b/src/lib/errors/NotTheGroupCreatorError.ts index 7b1e748..479881f 100644 --- a/src/lib/errors/NotTheGroupCreatorError.ts +++ b/src/lib/errors/NotTheGroupCreatorError.ts @@ -10,5 +10,4 @@ export class NotTheGroupCreatorError extends BaseError { constructor(groupId: number) { super(`You are not the creator of '${groupId}'`); } - } diff --git a/src/lib/errors/PostNotFoundError.ts b/src/lib/errors/PostNotFoundError.ts index 76fcf77..54b20b4 100644 --- a/src/lib/errors/PostNotFoundError.ts +++ b/src/lib/errors/PostNotFoundError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when a post was not found */ export class PostNotFoundError extends BaseError { + + public readonly statusCode = httpStatus.NOT_FOUND; + constructor(postId: number) { super(`Post '${postId}' not found!`); } diff --git a/src/lib/errors/RequestNotFoundError.ts b/src/lib/errors/RequestNotFoundError.ts index 46fa064..fca6895 100644 --- a/src/lib/errors/RequestNotFoundError.ts +++ b/src/lib/errors/RequestNotFoundError.ts @@ -5,6 +5,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when a request for a sender, receiver and type was not found */ export class RequestNotFoundError extends BaseError { + public readonly statusCode = httpStatus.NOT_FOUND; + + // @ts-ignore constructor(sender: number, receiver?: number, type?: dataaccess.RequestType) { if (!receiver) { super(`Request with id '${sender} not found.'`); diff --git a/src/lib/errors/UserNotFoundError.ts b/src/lib/errors/UserNotFoundError.ts index 642dc74..23038d7 100644 --- a/src/lib/errors/UserNotFoundError.ts +++ b/src/lib/errors/UserNotFoundError.ts @@ -4,6 +4,9 @@ import {BaseError} from "./BaseError"; * An error that is thrown when a specified user was not found */ export class UserNotFoundError extends BaseError { + + public readonly statusCode = httpStatus.NOT_FOUND; + constructor(username: (string | number)) { super(`User ${username} not found!`); } diff --git a/src/lib/errors/graphqlErrors.ts b/src/lib/errors/graphqlErrors.ts index 26c08aa..f40aa5c 100644 --- a/src/lib/errors/graphqlErrors.ts +++ b/src/lib/errors/graphqlErrors.ts @@ -17,21 +17,3 @@ export class PostNotFoundGqlError extends GraphQLError { super(`Post '${postId}' not found!`); } } - -/** - * An error for the frontend that is thrown when a group was not found - */ -export class GroupNotFoundGqlError extends GraphQLError { - constructor(groupId: number) { - super(`Group '${groupId}' not found!`); - } -} - -/** - * An error for the frontend that is thrown when a nonadmin tries to perform an admin operation. - */ -export class NotAnAdminGqlError extends GraphQLError { - constructor() { - super("You are not an admin."); - } -} diff --git a/src/lib/markdown.ts b/src/lib/markdown.ts index e4d2044..adb7793 100644 --- a/src/lib/markdown.ts +++ b/src/lib/markdown.ts @@ -1,6 +1,6 @@ import * as MarkdownIt from "markdown-it/lib"; -const { html5Media } = require("markdown-it-html5-media"); +const {html5Media} = require("markdown-it-html5-media"); const mdEmoji = require("markdown-it-emoji"); namespace markdown { diff --git a/src/lib/models/BlacklistedPhrase.ts b/src/lib/models/BlacklistedPhrase.ts index 5591155..fcd8bff 100644 --- a/src/lib/models/BlacklistedPhrase.ts +++ b/src/lib/models/BlacklistedPhrase.ts @@ -1,15 +1,5 @@ import * as sqz from "sequelize"; -import { - BelongsTo, - BelongsToMany, - Column, - ForeignKey, - HasMany, - Model, - NotNull, - Table, - Unique, -} from "sequelize-typescript"; +import {Column, Model, NotNull, Table, Unique} from "sequelize-typescript"; /** * Represents a blacklisted phrase diff --git a/src/lib/models/Event.ts b/src/lib/models/Event.ts index c0119ed..d30e8ad 100644 --- a/src/lib/models/Event.ts +++ b/src/lib/models/Event.ts @@ -81,7 +81,7 @@ export class Event extends Model { * @param userId * @param request */ - public async deletable({userId}: {userId: number}, request: any): Promise { + public async deletable({userId}: { userId: number }, request: any): Promise { userId = userId ?? request.session.userId; if (userId) { const group = await this.$get("rGroup") as Group; diff --git a/src/lib/models/Group.ts b/src/lib/models/Group.ts index b2b5bf9..4184e34 100644 --- a/src/lib/models/Group.ts +++ b/src/lib/models/Group.ts @@ -150,7 +150,7 @@ export class Group extends Model { * @param userId * @param request */ - public async deletable({userId}: {userId?: number}, request: any): Promise { + public async deletable({userId}: { userId?: number }, request: any): Promise { userId = userId ?? request.session.userId; if (userId) { return this.creatorId === userId; diff --git a/src/lib/models/Post.ts b/src/lib/models/Post.ts index b96b88b..fad5013 100644 --- a/src/lib/models/Post.ts +++ b/src/lib/models/Post.ts @@ -1,6 +1,5 @@ import * as sqz from "sequelize"; import {BelongsTo, BelongsToMany, Column, CreatedAt, ForeignKey, Model, NotNull, Table} from "sequelize-typescript"; -import globals from "../globals"; import markdown from "../markdown"; import {Activity} from "./Activity"; import {PostVote, VoteType} from "./PostVote"; diff --git a/src/routes/UploadRoute.ts b/src/routes/UploadRoute.ts index 18784e1..6f8b10a 100644 --- a/src/routes/UploadRoute.ts +++ b/src/routes/UploadRoute.ts @@ -5,7 +5,6 @@ import {Router} from "express"; import * as fileUpload from "express-fileupload"; import {UploadedFile} from "express-fileupload"; import * as fsx from "fs-extra"; -import {IncomingMessage} from "http"; import * as status from "http-status"; import * as path from "path"; import * as sharp from "sharp";