From 41a6f7799805eec8fd09f75fe944784edcd1d8b4 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Sun, 1 Oct 2023 15:02:56 -0400 Subject: [PATCH] fix: Types on overridden findByPk methods (#5908) --- server/commands/documentMover.ts | 9 +++-- server/models/Collection.ts | 20 +++++++++-- server/models/Document.ts | 35 +++++++++++++++---- .../queues/processors/RevisionsProcessor.ts | 3 +- server/routes/api/documents/documents.ts | 2 +- server/utils/jwt.ts | 6 ++-- 6 files changed, 59 insertions(+), 16 deletions(-) diff --git a/server/commands/documentMover.ts b/server/commands/documentMover.ts index a689fee0d..c1608ec80 100644 --- a/server/commands/documentMover.ts +++ b/server/commands/documentMover.ts @@ -1,5 +1,6 @@ import invariant from "invariant"; import { Transaction } from "sequelize"; +import { ValidationError } from "@server/errors"; import { traceFunction } from "@server/logging/tracing"; import { User, Document, Collection, Pin, Event } from "@server/models"; import pinDestroyer from "./pinDestroyer"; @@ -50,7 +51,10 @@ async function documentMover({ } if (document.template) { - invariant(collectionId, "collectionId should exist"); + if (!document.collectionId) { + throw ValidationError("Templates must be in a collection"); + } + document.collectionId = collectionId; document.parentDocumentId = null; document.lastModifiedById = user.id; @@ -142,7 +146,8 @@ async function documentMover({ }).findByPk(collectionId, { transaction, }); - invariant(newCollection, "collection should exist"); + invariant(newCollection, "Collection not found"); + result.collections.push(newCollection); await Document.update( diff --git a/server/models/Collection.ts b/server/models/Collection.ts index f6ad8cb21..796ebafcb 100644 --- a/server/models/Collection.ts +++ b/server/models/Collection.ts @@ -1,9 +1,16 @@ +/* eslint-disable lines-between-class-members */ import find from "lodash/find"; import findIndex from "lodash/findIndex"; import remove from "lodash/remove"; import uniq from "lodash/uniq"; import randomstring from "randomstring"; -import { Identifier, Transaction, Op, FindOptions } from "sequelize"; +import { + Identifier, + Transaction, + Op, + FindOptions, + NonNullFindOptions, +} from "sequelize"; import { Sequelize, Table, @@ -362,8 +369,17 @@ class Collection extends ParanoidModel { * Overrides the standard findByPk behavior to allow also querying by urlId * * @param id uuid or urlId - * @returns collection instance + * @param options FindOptions + * @returns A promise resolving to a collection instance or null */ + static async findByPk( + id: Identifier, + options?: NonNullFindOptions + ): Promise; + static async findByPk( + id: Identifier, + options?: FindOptions + ): Promise; static async findByPk( id: Identifier, options: FindOptions = {} diff --git a/server/models/Document.ts b/server/models/Document.ts index f0fec704c..4912a884f 100644 --- a/server/models/Document.ts +++ b/server/models/Document.ts @@ -1,8 +1,9 @@ +/* eslint-disable lines-between-class-members */ import compact from "lodash/compact"; import isNil from "lodash/isNil"; import uniq from "lodash/uniq"; import randomstring from "randomstring"; -import type { SaveOptions } from "sequelize"; +import type { Identifier, NonNullFindOptions, SaveOptions } from "sequelize"; import { Sequelize, Transaction, @@ -52,6 +53,11 @@ import Length from "./validators/Length"; export const DOCUMENT_VERSION = 2; +type AdditionalFindOptions = { + userId?: string; + includeState?: boolean; +}; + @DefaultScope(() => ({ attributes: { exclude: ["state"], @@ -429,13 +435,30 @@ class Document extends ParanoidModel { return this.scope(["defaultScope", collectionScope, viewScope]); } + /** + * Overrides the standard findByPk behavior to allow also querying by urlId + * + * @param id uuid or urlId + * @param options FindOptions + * @returns A promise resolving to a collection instance or null + */ static async findByPk( - id: string, - options: FindOptions & { - userId?: string; - includeState?: boolean; - } = {} + id: Identifier, + options?: NonNullFindOptions & AdditionalFindOptions + ): Promise; + static async findByPk( + id: Identifier, + options?: FindOptions & AdditionalFindOptions + ): Promise; + static async findByPk( + id: Identifier, + options: (NonNullFindOptions | FindOptions) & + AdditionalFindOptions = {} ): Promise { + if (typeof id !== "string") { + return null; + } + // allow default preloading of collection membership if `userId` is passed in find options // almost every endpoint needs the collection membership to determine policy permissions. const scope = this.scope([ diff --git a/server/queues/processors/RevisionsProcessor.ts b/server/queues/processors/RevisionsProcessor.ts index 692241b1f..5caa039b4 100644 --- a/server/queues/processors/RevisionsProcessor.ts +++ b/server/queues/processors/RevisionsProcessor.ts @@ -1,4 +1,3 @@ -import invariant from "invariant"; import revisionCreator from "@server/commands/revisionCreator"; import { Revision, Document, User } from "@server/models"; import { DocumentEvent, RevisionEvent, Event } from "@server/types"; @@ -22,8 +21,8 @@ export default class RevisionsProcessor extends BaseProcessor { const document = await Document.findByPk(event.documentId, { paranoid: false, + rejectOnEmpty: true, }); - invariant(document, "Document should exist"); const previous = await Revision.findLatest(document.id); // we don't create revisions if identical to previous revision, this can diff --git a/server/routes/api/documents/documents.ts b/server/routes/api/documents/documents.ts index b391fe50e..0cd391d29 100644 --- a/server/routes/api/documents/documents.ts +++ b/server/routes/api/documents/documents.ts @@ -928,8 +928,8 @@ router.post( // reload to get all of the data needed to present (user, collection etc) const reloaded = await Document.findByPk(document.id, { userId: user.id, + rejectOnEmpty: true, }); - invariant(reloaded, "document not found"); ctx.body = { data: await presentDocument(reloaded), diff --git a/server/utils/jwt.ts b/server/utils/jwt.ts index 64f201a4e..3caeda769 100644 --- a/server/utils/jwt.ts +++ b/server/utils/jwt.ts @@ -1,5 +1,4 @@ import { subMinutes } from "date-fns"; -import invariant from "invariant"; import JWT from "jsonwebtoken"; import { Team, User } from "@server/models"; import { AuthenticationError } from "../errors"; @@ -85,8 +84,9 @@ export async function getUserForEmailSigninToken(token: string): Promise { } } - const user = await User.scope("withTeam").findByPk(payload.id); - invariant(user, "User not found"); + const user = await User.scope("withTeam").findByPk(payload.id, { + rejectOnEmpty: true, + }); try { JWT.verify(token, user.jwtSecret);