From 995054d46bc8c4c5f02c78b1c4d0d6155d449b75 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 17 Apr 2023 16:25:07 -0400 Subject: [PATCH] chore: adds tests to validate queries --- src/errors/QueryError.ts | 16 ++ src/mongoose/buildQuery.ts | 11 +- test/collections-rest/config.ts | 48 +++- test/collections-rest/int.spec.ts | 382 ++++++++++++++++++++++++------ test/helpers/rest.ts | 14 +- 5 files changed, 383 insertions(+), 88 deletions(-) create mode 100644 src/errors/QueryError.ts diff --git a/src/errors/QueryError.ts b/src/errors/QueryError.ts new file mode 100644 index 0000000000..6d254267a3 --- /dev/null +++ b/src/errors/QueryError.ts @@ -0,0 +1,16 @@ +import httpStatus from 'http-status'; +import type { TFunction } from 'i18next'; +import APIError from './APIError'; + +class QueryError extends APIError { + constructor(results: { path: string }[], t?: TFunction) { + const message = t ? t('error:unspecific', { count: results.length }) : `The following path${results.length === 1 ? '' : 's'} cannot be queried:`; + super( + `${message} ${results.map((err) => err.path).join(', ')}`, + httpStatus.BAD_REQUEST, + results, + ); + } +} + +export default QueryError; diff --git a/src/mongoose/buildQuery.ts b/src/mongoose/buildQuery.ts index 86d51d67b4..531a26971d 100644 --- a/src/mongoose/buildQuery.ts +++ b/src/mongoose/buildQuery.ts @@ -12,6 +12,7 @@ import { CollectionPermission, FieldPermissions, GlobalPermission } from '../aut import flattenFields from '../utilities/flattenTopLevelFields'; import { getEntityPolicies } from '../utilities/getEntityPolicies'; import { SanitizedConfig } from '../config/types'; +import QueryError from '../errors/QueryError'; const validOperators = ['like', 'contains', 'in', 'all', 'not_in', 'greater_than_equal', 'greater_than', 'less_than_equal', 'less_than', 'not_equals', 'equals', 'exists', 'near']; @@ -46,8 +47,6 @@ type ParamParserArgs = { overrideAccess?: boolean } -type QueryError = { path: string } - export class ParamParser { collectionSlug?: string @@ -74,7 +73,7 @@ export class ParamParser { }; } - errors: QueryError[] + errors: { path: string }[] constructor({ req, @@ -566,7 +565,11 @@ const getBuildQueryPlugin = ({ overrideAccess, }); const result = await paramParser.parse(); - // TODO: throw errors here + + if (this.errors.length > 0) { + throw new QueryError(this.errors); + } + return result; } modifiedSchema.statics.buildQuery = buildQuery; diff --git a/test/collections-rest/config.ts b/test/collections-rest/config.ts index cb60c5382c..0ece14ea68 100644 --- a/test/collections-rest/config.ts +++ b/test/collections-rest/config.ts @@ -35,6 +35,29 @@ export const customIdNumberSlug = 'custom-id-number'; export const errorOnHookSlug = 'error-on-hooks'; export default buildConfig({ + endpoints: [ + { + path: '/send-test-email', + method: 'get', + handler: async (req, res) => { + await req.payload.sendEmail({ + from: 'dev@payloadcms.com', + to: devUser.email, + subject: 'Test Email', + html: 'This is a test email.', + // to recreate a failing email transport, add the following credentials + // to the `email` property of `payload.init()` in `../dev.ts` + // the app should fail to send the email, but the error should be handled without crashing the app + // transportOptions: { + // host: 'smtp.ethereal.email', + // port: 587, + // }, + }); + + res.status(200).send('Email sent'); + }, + }, + ], collections: [ { slug, @@ -78,6 +101,13 @@ export default buildConfig({ relationTo: [relationSlug, 'dummy'], hasMany: true, }, + { + name: 'restrictedField', + type: 'text', + access: { + read: () => false, + }, + }, ], }, { @@ -91,7 +121,23 @@ export default buildConfig({ ], }, collectionWithName(relationSlug), - collectionWithName('dummy'), + { + slug: 'dummy', + access: openAccess, + fields: [ + { + type: 'text', + name: 'title', + }, + { + type: 'text', + name: 'name', + access: { + read: () => false, + }, + }, + ], + }, { slug: customIdSlug, access: openAccess, diff --git a/test/collections-rest/int.spec.ts b/test/collections-rest/int.spec.ts index 119f7b3fe6..b9803520c9 100644 --- a/test/collections-rest/int.spec.ts +++ b/test/collections-rest/int.spec.ts @@ -2,7 +2,7 @@ import mongoose from 'mongoose'; import { randomBytes } from 'crypto'; import { initPayloadTest } from '../helpers/configHelpers'; import type { Relation } from './config'; -import config, { customIdNumberSlug, customIdSlug, slug, relationSlug, pointSlug, errorOnHookSlug } from './config'; +import config, { customIdNumberSlug, customIdSlug, errorOnHookSlug, pointSlug, relationSlug, slug } from './config'; import payload from '../../src'; import { RESTClient } from '../helpers/rest'; import type { ErrorOnHook, Post } from './payload-types'; @@ -49,10 +49,16 @@ describe('collections-rest', () => { }); it('should update existing', async () => { - const { id, description } = await createPost({ description: 'desc' }); + const { + id, + description, + } = await createPost({ description: 'desc' }); const updatedTitle = 'updated-title'; - const { status, doc: updated } = await client.update({ + const { + status, + doc: updated, + } = await client.update({ id, data: { title: updatedTitle }, }); @@ -62,98 +68,305 @@ describe('collections-rest', () => { expect(updated.description).toEqual(description); // Check was not modified }); - it('should bulk update', async () => { - await mapAsync([...Array(11)], async (_, i) => { - await createPost({ description: `desc ${i}` }); + describe('Bulk operations', () => { + it('should bulk update', async () => { + await mapAsync([...Array(11)], async (_, i) => { + await createPost({ description: `desc ${i}` }); + }); + + const description = 'updated'; + const { + status, + docs, + } = await client.updateMany({ + where: { title: { equals: 'title' } }, + data: { description }, + }); + + expect(status).toEqual(200); + expect(docs[0].title).toEqual('title'); // Check was not modified + expect(docs[0].description).toEqual(description); + expect(docs.pop().description).toEqual(description); }); - const description = 'updated'; - const { status, docs } = await client.updateMany({ - query: { title: { equals: 'title' } }, - data: { description }, + it('should not bulk update with a bad query', async () => { + await mapAsync([...Array(2)], async (_, i) => { + await createPost({ description: `desc ${i}` }); + }); + + const description = 'updated'; + + const { status, docs: noDocs, errors } = await client.updateMany({ + where: { missing: { equals: 'title' } }, + data: { description }, + }); + + expect(status).toEqual(400); + expect(noDocs).toBeUndefined(); + expect(errors).toHaveLength(1); + + const { docs } = await payload.find({ + collection: slug, + }); + + expect(docs[0].description).not.toEqual(description); + expect(docs.pop().description).not.toEqual(description); }); - expect(status).toEqual(200); - expect(docs[0].title).toEqual('title'); // Check was not modified - expect(docs[0].description).toEqual(description); - expect(docs.pop().description).toEqual(description); - }); + it('should not bulk update with a bad relationship query', async () => { + await mapAsync([...Array(2)], async (_, i) => { + await createPost({ description: `desc ${i}` }); + }); - it('should return formatted errors for bulk updates', async () => { - const text = 'bulk-update-test-errors'; - const errorDoc = await payload.create({ - collection: errorOnHookSlug, - data: { - text, - errorBeforeChange: true, - }, - }); - const successDoc = await payload.create({ - collection: errorOnHookSlug, - data: { - text, - errorBeforeChange: false, - }, + const description = 'updated'; + const { status: relationFieldStatus, docs: relationFieldDocs, errors: relationFieldErrors } = await client.updateMany({ + where: { 'relationField.missing': { equals: 'title' } }, + data: { description }, + }); + + console.log({ relationFieldStatus, relationFieldDocs, relationFieldErrors }); + + const { status: relationMultiRelationToStatus } = await client.updateMany({ + where: { 'relationMultiRelationTo.missing': { equals: 'title' } }, + data: { description }, + }); + + const { docs } = await payload.find({ + collection: slug, + }); + + expect(relationFieldStatus).toEqual(400); + expect(relationMultiRelationToStatus).toEqual(400); + expect(docs[0].description).not.toEqual(description); + expect(docs.pop().description).not.toEqual(description); }); - const update = 'update'; + it('should not bulk update with a read restricted field query', async () => { + const { id } = await payload.create({ + collection: slug, + data: { + restrictedField: 'restricted', + }, + }); - const result = await client.updateMany({ - slug: errorOnHookSlug, - query: { text: { equals: text } }, - data: { text: update }, + const description = 'description'; + const { status } = await client.updateMany({ + query: { restrictedField: { equals: 'restricted' } }, + data: { description }, + }); + + const doc = await payload.findByID({ + collection: slug, + id, + }); + + expect(status).toEqual(400); + expect(doc.description).toBeUndefined(); }); - expect(result.status).toEqual(400); - expect(result.docs).toHaveLength(1); - expect(result.docs[0].id).toEqual(successDoc.id); - expect(result.errors).toHaveLength(1); - expect(result.errors[0].message).toBeDefined(); - expect(result.errors[0].id).toEqual(errorDoc.id); - expect(result.docs[0].text).toEqual(update); - }); + it('should bulk update with a relationship field that exists in one collection and not another', async () => { + const relationOne = await payload.create({ + collection: 'dummy', + data: { + title: 'title', + }, + }); + const relationTwo = await payload.create({ + collection: 'relation', + data: { + name: 'name', + }, + }); - it('should bulk delete', async () => { - const count = 11; - await mapAsync([...Array(count)], async (_, i) => { - await createPost({ description: `desc ${i}` }); + const description = 'desc'; + const relationPost = await payload.create({ + collection: slug, + data: { + description, + relationMultiRelationTo: { + value: relationTwo.id, + relationTo: 'relation', + }, + }, + }); + + const relationToDummyPost = await payload.create({ + collection: slug, + data: { + description, + relationMultiRelationTo: { + value: relationOne.id, + relationTo: 'dummy', + }, + }, + }); + + const updatedDescription = 'updated'; + + const { status: relationMultiRelationToStatus, docs: updated } = await client.updateMany({ + where: { + 'relationMultiRelationTo.title': { + equals: relationOne.title, + }, + }, + data: { description: updatedDescription }, + }); + + const updatedDoc = await payload.findByID({ + collection: slug, + id: relationToDummyPost.id, + }); + + const otherDoc = await payload.findByID({ + collection: slug, + id: relationPost.id, + }); + + expect(relationMultiRelationToStatus).toEqual(200); + expect(updated).toHaveLength(1); + expect(updated[0].id).toEqual(relationToDummyPost.id); + expect(updatedDoc.description).toEqual(updatedDescription); + expect(otherDoc.description).toEqual(description); }); - const { status, docs } = await client.deleteMany({ - query: { title: { eq: 'title' } }, + it('should bulk update with a relationship field that exists in one collection and is restricted in another', async () => { + const name = 'name'; + const relationOne = await payload.create({ + collection: 'dummy', + data: { + title: 'title', + name, // read access: () => false + }, + }); + const relationTwo = await payload.create({ + collection: 'relation', + data: { + name, + }, + }); + + const description = 'desc'; + const relationPost = await payload.create({ + collection: slug, + data: { + description, + relationMultiRelationTo: { + value: relationTwo.id, + relationTo: 'relation', + }, + }, + }); + + const relationToDummyPost = await payload.create({ + collection: slug, + data: { + description, + relationMultiRelationTo: { + value: relationOne.id, + relationTo: 'dummy', + }, + }, + }); + + const updatedDescription = 'updated'; + + const { status } = await client.updateMany({ + where: { 'relationMultiRelationTo.name': { equals: name } }, + data: { description: updatedDescription }, + }); + + + const updatedDoc = await payload.findByID({ + collection: slug, + id: relationPost.id, + }); + + const otherDoc = await payload.findByID({ + collection: slug, + id: relationToDummyPost.id, + }); + + expect(status).toEqual(200); + expect(updatedDoc.description).toEqual(updatedDescription); + expect(otherDoc.description).toEqual(description); }); - expect(status).toEqual(200); - expect(docs[0].title).toEqual('title'); // Check was not modified - expect(docs).toHaveLength(count); - }); + it('should return formatted errors for bulk updates', async () => { + const text = 'bulk-update-test-errors'; + const errorDoc = await payload.create({ + collection: errorOnHookSlug, + data: { + text, + errorBeforeChange: true, + }, + }); + const successDoc = await payload.create({ + collection: errorOnHookSlug, + data: { + text, + errorBeforeChange: false, + }, + }); - it('should return formatted errors for bulk deletes', async () => { - await payload.create({ - collection: errorOnHookSlug, - data: { - text: 'test', - errorAfterDelete: true, - }, - }); - await payload.create({ - collection: errorOnHookSlug, - data: { - text: 'test', - errorAfterDelete: false, - }, + const update = 'update'; + + const result = await client.updateMany({ + slug: errorOnHookSlug, + where: { text: { equals: text } }, + data: { text: update }, + }); + + expect(result.status).toEqual(400); + expect(result.docs).toHaveLength(1); + expect(result.docs[0].id).toEqual(successDoc.id); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBeDefined(); + expect(result.errors[0].id).toEqual(errorDoc.id); + expect(result.docs[0].text).toEqual(update); }); - const result = await client.deleteMany({ - slug: errorOnHookSlug, - query: { text: { equals: 'test' } }, + it('should bulk delete', async () => { + const count = 11; + await mapAsync([...Array(count)], async (_, i) => { + await createPost({ description: `desc ${i}` }); + }); + + const { status, docs } = await client.deleteMany({ + where: { title: { eq: 'title' } }, + }); + + expect(status).toEqual(200); + expect(docs[0].title).toEqual('title'); // Check was not modified + expect(docs).toHaveLength(count); }); - expect(result.status).toEqual(400); - expect(result.docs).toHaveLength(1); - expect(result.errors).toHaveLength(1); - expect(result.errors[0].message).toBeDefined(); - expect(result.errors[0].id).toBeDefined(); + it('should return formatted errors for bulk deletes', async () => { + await payload.create({ + collection: errorOnHookSlug, + data: { + text: 'test', + errorAfterDelete: true, + }, + }); + await payload.create({ + collection: errorOnHookSlug, + data: { + text: 'test', + errorAfterDelete: false, + }, + }); + + const result = await client.deleteMany({ + slug: errorOnHookSlug, + where: { text: { equals: 'test' } }, + }); + + expect(result.status).toEqual(400); + expect(result.docs).toHaveLength(1); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBeDefined(); + expect(result.errors[0].id).toBeDefined(); + }); }); describe('Custom ID', () => { @@ -342,7 +555,24 @@ describe('collections-rest', () => { expect(result.totalDocs).toEqual(1); }); - it.todo('nested by property value'); + it('nested by property value', async () => { + const post1 = await createPost({ + relationMultiRelationTo: { relationTo: relationSlug, value: relation.id }, + }); + await createPost(); + + const { status, result } = await client.find({ + query: { + 'relationMultiRelationTo.value.name': { + equals: relation.name, + }, + }, + }); + + expect(status).toEqual(200); + expect(result.docs).toEqual([post1]); + expect(result.totalDocs).toEqual(1); + }); }); describe('relationTo multi hasMany', () => { diff --git a/test/helpers/rest.ts b/test/helpers/rest.ts index 36867a93b8..b8dfba3267 100644 --- a/test/helpers/rest.ts +++ b/test/helpers/rest.ts @@ -57,7 +57,7 @@ type UpdateManyArgs = { slug?: string; data: Partial; auth?: boolean; - query: any; + where: any; }; type DeleteArgs = { @@ -69,7 +69,7 @@ type DeleteArgs = { type DeleteManyArgs = { slug?: string; auth?: boolean; - query: any; + where: any; }; type FindGlobalArgs = { @@ -92,7 +92,7 @@ type DocResponse = { type DocsResponse = { status: number; docs: T[]; - errors?: { name: string, message: string, data: any, id: string | number}[] + errors?: { name: string, message: string, data: any, id: string | number }[] }; const headers = { @@ -205,9 +205,9 @@ export class RESTClient { } async updateMany(args: UpdateManyArgs): Promise> { - const { slug, data, query } = args; + const { slug, data, where } = args; const formattedQs = qs.stringify({ - ...(query ? { where: query } : {}), + ...(where ? { where } : {}), }, { addQueryPrefix: true, }); @@ -225,9 +225,9 @@ export class RESTClient { } async deleteMany(args: DeleteManyArgs): Promise> { - const { slug, query } = args; + const { slug, where } = args; const formattedQs = qs.stringify({ - ...(query ? { where: query } : {}), + ...(where ? { where } : {}), }, { addQueryPrefix: true, });