fix: error on graphql multiple queries (#3985)

This commit is contained in:
Dan Ribbens
2023-11-08 12:38:25 -05:00
committed by GitHub
parent 611438177b
commit 57da3c99a7
31 changed files with 292 additions and 143 deletions

View File

@@ -4,6 +4,7 @@ export const commitTransaction: CommitTransaction = async function commitTransac
if (!this.sessions[id]?.inTransaction()) {
return
}
await this.sessions[id].commitTransaction()
await this.sessions[id].endSession()
delete this.sessions[id]

View File

@@ -3,10 +3,20 @@ import type { RollbackTransaction } from 'payload/database'
export const rollbackTransaction: RollbackTransaction = async function rollbackTransaction(
id = '',
) {
if (!this.sessions[id]?.inTransaction()) {
this.payload.logger.warn('rollbackTransaction called when no transaction exists')
// if multiple operations are using the same transaction, the first will flow through and delete the session.
// subsequent calls should be ignored.
if (!this.sessions[id]) {
return
}
// when session exists but is not inTransaction something unexpected is happening to the session
if (!this.sessions[id].inTransaction()) {
this.payload.logger.warn('rollbackTransaction called when no transaction exists')
delete this.sessions[id]
return
}
// the first call for rollback should be aborted and deleted causing any other operations with the same transaction to fail
await this.sessions[id].abortTransaction()
await this.sessions[id].endSession()
delete this.sessions[id]

View File

@@ -1,6 +1,7 @@
import type { CommitTransaction } from 'payload/database'
export const commitTransaction: CommitTransaction = async function commitTransaction(id) {
// if the session was deleted it has already been aborted
if (!this.sessions[id]) {
return
}

View File

@@ -3,12 +3,15 @@ import type { RollbackTransaction } from 'payload/database'
export const rollbackTransaction: RollbackTransaction = async function rollbackTransaction(
id = '',
) {
// if multiple operations are using the same transaction, the first will flow through and delete the session.
// subsequent calls should be ignored.
if (!this.sessions[id]) {
this.payload.logger.warn('rollbackTransaction called when no transaction exists')
return
}
// end the session promise in failure by calling reject
await this.sessions[id].reject()
// delete the session causing any other operations with the same transaction to fail
delete this.sessions[id]
}

View File

@@ -1,3 +1,4 @@
import type { PayloadRequest } from '../../../express/types'
import type { Payload } from '../../../payload'
import formatName from '../../../graphql/utilities/formatName'
@@ -18,7 +19,7 @@ const formatConfigNames = (results, configs) => {
function accessResolver(payload: Payload) {
async function resolver(_, args, context) {
const options = {
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const accessResults = await access(options)

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import forgotPassword from '../../operations/forgotPassword'
@@ -11,7 +12,7 @@ function forgotPasswordResolver(collection: Collection): any {
},
disableEmail: args.disableEmail,
expiration: args.expiration,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
await forgotPassword(options)

View File

@@ -1,10 +1,12 @@
import type { PayloadRequest } from '../../../express/types'
import init from '../../operations/init'
function initResolver(collection: string) {
async function resolver(_, args, context) {
const options = {
collection,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
return init(options)

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import login from '../../operations/login'
@@ -11,7 +12,7 @@ function loginResolver(collection: Collection) {
password: args.password,
},
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
res: context.res,
}

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import logout from '../../operations/logout'
@@ -6,7 +7,7 @@ function logoutResolver(collection: Collection): any {
async function resolver(_, args, context) {
const options = {
collection,
req: context.req,
req: { ...context.req } as PayloadRequest,
res: context.res,
}

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import me from '../../operations/me'
@@ -7,10 +8,11 @@ function meResolver(collection: Collection): any {
const options = {
collection,
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
return me(options)
}
return resolver
}

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import getExtractJWT from '../../getExtractJWT'
import refresh from '../../operations/refresh'
@@ -17,7 +18,7 @@ function refreshResolver(collection: Collection) {
const options = {
collection,
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
res: context.res,
token,
}

View File

@@ -1,5 +1,6 @@
/* eslint-disable no-param-reassign */
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import resetPassword from '../../operations/resetPassword'
@@ -13,7 +14,7 @@ function resetPasswordResolver(collection: Collection) {
collection,
data: args,
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
res: context.res,
}

View File

@@ -1,4 +1,5 @@
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import unlock from '../../operations/unlock'
@@ -7,12 +8,13 @@ function unlockResolver(collection: Collection) {
const options = {
collection,
data: { email: args.email },
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await unlock(options)
return result
}
return resolver
}

View File

@@ -1,5 +1,6 @@
/* eslint-disable no-param-reassign */
import type { Collection } from '../../../collections/config/types'
import type { PayloadRequest } from '../../../express/types'
import verifyEmail from '../../operations/verifyEmail'
@@ -11,7 +12,7 @@ function verifyEmailResolver(collection: Collection) {
const options = {
api: 'GraphQL',
collection,
req: context.req,
req: { ...context.req } as PayloadRequest,
res: context.res,
token: args.token,
}

View File

@@ -37,7 +37,7 @@ export default function createResolver<TSlug extends keyof GeneratedTypes['colle
data: args.data,
depth: 0,
draft: args.draft,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await create(options)

View File

@@ -30,7 +30,7 @@ export default function getDeleteResolver<TSlug extends keyof GeneratedTypes['co
id: args.id,
collection,
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await deleteByID(options)

View File

@@ -18,7 +18,7 @@ export function docAccessResolver(): Resolver {
async function resolver(_, args, context) {
return docAccess({
id: args.id,
req: context.req,
req: { ...context.req } as PayloadRequest,
})
}

View File

@@ -36,7 +36,7 @@ export default function findResolver(collection: Collection): Resolver {
draft: args.draft,
limit: args.limit,
page: args.page,
req: context.req,
req: { ...context.req } as PayloadRequest,
sort: args.sort,
where: args.where,
}

View File

@@ -31,7 +31,7 @@ export default function findVersionByIDResolver(collection: Collection): Resolve
collection,
depth: 0,
draft: args.draft,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await findVersionByID(options)

View File

@@ -35,7 +35,7 @@ export default function findVersionsResolver(collection: Collection): Resolver {
depth: 0,
limit: args.limit,
page: args.page,
req: context.req,
req: { ...context.req } as PayloadRequest,
sort: args.sort,
where: args.where,
}

View File

@@ -23,7 +23,7 @@ export default function restoreVersionResolver(collection: Collection): Resolver
id: args.id,
collection,
depth: 0,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await restoreVersion(options)

View File

@@ -36,7 +36,7 @@ export default function updateResolver<TSlug extends keyof GeneratedTypes['colle
data: args.data,
depth: 0,
draft: args.draft,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await updateByID<TSlug>(options)

View File

@@ -16,7 +16,7 @@ export function docAccessResolver(global: SanitizedGlobalConfig): Resolver {
async function resolver(_, context) {
return docAccess({
globalConfig: global,
req: context.req,
req: { ...context.req } as PayloadRequest,
})
}

View File

@@ -1,6 +1,6 @@
/* eslint-disable no-param-reassign */
import type { Document } from '../../../types'
import type { Document, PayloadRequest } from '../../../types'
import type { SanitizedGlobalConfig } from '../../config/types'
import findOne from '../../operations/findOne'
@@ -16,7 +16,7 @@ export default function findOneResolver(globalConfig: SanitizedGlobalConfig): Do
depth: 0,
draft: args.draft,
globalConfig,
req: context.req,
req: { ...context.req } as PayloadRequest,
slug,
}

View File

@@ -31,7 +31,7 @@ export default function findVersionByIDResolver(globalConfig: SanitizedGlobalCon
depth: 0,
draft: args.draft,
globalConfig,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await findVersionByID(options)

View File

@@ -29,7 +29,7 @@ export default function findVersionsResolver(globalConfig: SanitizedGlobalConfig
globalConfig,
limit: args.limit,
page: args.page,
req: context.req,
req: { ...context.req } as PayloadRequest,
sort: args.sort,
where: args.where,
}

View File

@@ -22,7 +22,7 @@ export default function restoreVersionResolver(globalConfig: SanitizedGlobalConf
id: args.id,
depth: 0,
globalConfig,
req: context.req,
req: { ...context.req } as PayloadRequest,
}
const result = await restoreVersion(options)

View File

@@ -35,7 +35,7 @@ export default function updateResolver<TSlug extends keyof GeneratedTypes['globa
depth: 0,
draft: args.draft,
globalConfig,
req: context.req,
req: { ...context.req } as PayloadRequest,
slug,
}

View File

@@ -12,14 +12,13 @@ export interface Relation {
const openAccess = {
create: () => true,
delete: () => true,
read: () => true,
update: () => true,
delete: () => true,
}
const collectionWithName = (collectionSlug: string): CollectionConfig => {
return {
slug: collectionSlug,
access: openAccess,
fields: [
{
@@ -27,6 +26,7 @@ const collectionWithName = (collectionSlug: string): CollectionConfig => {
type: 'text',
},
],
slug: collectionSlug,
}
}
@@ -35,47 +35,27 @@ export const relationSlug = 'relation'
export const pointSlug = 'point'
export const errorOnHookSlug = 'error-on-hooks'
export default buildConfigWithDefaults({
graphQL: {
schemaOutputFile: path.resolve(__dirname, 'schema.graphql'),
queries: (GraphQL) => {
return {
QueryWithInternalError: {
type: new GraphQL.GraphQLObjectType({
name: 'QueryWithInternalError',
fields: {
text: {
type: GraphQL.GraphQLString,
},
},
}),
resolve: () => {
// Throwing an internal error with potentially sensitive data
throw new Error('Lost connection to the Pentagon. Secret data: ******')
},
},
}
},
},
collections: [
{
slug: 'users',
auth: true,
access: openAccess,
auth: true,
fields: [],
slug: 'users',
},
{
slug: pointSlug,
access: openAccess,
fields: [
{
type: 'point',
name: 'point',
type: 'point',
},
],
slug: pointSlug,
},
{
slug,
access: openAccess,
fields: [
{
@@ -92,173 +72,173 @@ export default buildConfigWithDefaults({
},
{
name: 'min',
type: 'number',
min: 10,
type: 'number',
},
// Relationship
{
name: 'relationField',
type: 'relationship',
relationTo: relationSlug,
type: 'relationship',
},
{
name: 'relationToCustomID',
type: 'relationship',
relationTo: 'custom-ids',
type: 'relationship',
},
// Relation hasMany
{
name: 'relationHasManyField',
type: 'relationship',
relationTo: relationSlug,
hasMany: true,
relationTo: relationSlug,
type: 'relationship',
},
// Relation multiple relationTo
{
name: 'relationMultiRelationTo',
type: 'relationship',
relationTo: [relationSlug, 'dummy'],
type: 'relationship',
},
// Relation multiple relationTo hasMany
{
name: 'relationMultiRelationToHasMany',
type: 'relationship',
relationTo: [relationSlug, 'dummy'],
hasMany: true,
relationTo: [relationSlug, 'dummy'],
type: 'relationship',
},
{
name: 'A1',
type: 'group',
fields: [
{
type: 'text',
name: 'A2',
defaultValue: 'textInRowInGroup',
type: 'text',
},
],
type: 'group',
},
{
name: 'B1',
type: 'group',
fields: [
{
type: 'collapsible',
label: 'Collapsible',
fields: [
{
type: 'text',
name: 'B2',
defaultValue: 'textInRowInGroup',
type: 'text',
},
],
label: 'Collapsible',
type: 'collapsible',
},
],
type: 'group',
},
{
name: 'C1',
type: 'group',
fields: [
{
type: 'text',
name: 'C2Text',
type: 'text',
},
{
type: 'row',
fields: [
{
type: 'collapsible',
label: 'Collapsible2',
fields: [
{
name: 'C2',
type: 'group',
fields: [
{
type: 'row',
fields: [
{
type: 'collapsible',
label: 'Collapsible2',
fields: [
{
type: 'text',
name: 'C3',
type: 'text',
},
],
label: 'Collapsible2',
type: 'collapsible',
},
],
type: 'row',
},
],
type: 'group',
},
],
label: 'Collapsible2',
type: 'collapsible',
},
],
type: 'row',
},
],
type: 'group',
},
{
type: 'tabs',
tabs: [
{
label: 'Tab1',
name: 'D1',
fields: [
{
name: 'D2',
type: 'group',
fields: [
{
type: 'row',
fields: [
{
type: 'collapsible',
label: 'Collapsible2',
fields: [
{
type: 'tabs',
tabs: [
{
label: 'Tab1',
fields: [
{
name: 'D3',
type: 'group',
fields: [
{
type: 'row',
fields: [
{
type: 'collapsible',
label: 'Collapsible2',
fields: [
{
type: 'text',
name: 'D4',
type: 'text',
},
],
label: 'Collapsible2',
type: 'collapsible',
},
],
type: 'row',
},
],
type: 'group',
},
],
label: 'Tab1',
},
],
type: 'tabs',
},
],
label: 'Collapsible2',
type: 'collapsible',
},
],
type: 'row',
},
],
type: 'group',
},
],
label: 'Tab1',
},
],
type: 'tabs',
},
],
slug,
},
{
slug: 'custom-ids',
access: {
read: () => true,
},
@@ -272,47 +252,98 @@ export default buildConfigWithDefaults({
type: 'text',
},
],
slug: 'custom-ids',
},
collectionWithName(relationSlug),
collectionWithName('dummy'),
{
slug: 'payload-api-test-ones',
access: {
read: () => true,
},
...collectionWithName(errorOnHookSlug),
fields: [
{
name: 'payloadAPI',
name: 'title',
type: 'text',
hooks: {
afterRead: [({ req }) => req.payloadAPI],
},
},
{
name: 'errorBeforeChange',
type: 'checkbox',
},
],
hooks: {
afterDelete: [
({ doc }) => {
if (doc?.errorAfterDelete) {
throw new Error('Error After Delete Thrown')
}
},
],
beforeChange: [
({ originalDoc }) => {
if (originalDoc?.errorBeforeChange) {
throw new Error('Error Before Change Thrown')
}
},
],
},
},
{
slug: 'payload-api-test-twos',
access: {
read: () => true,
},
fields: [
{
name: 'payloadAPI',
type: 'text',
hooks: {
afterRead: [({ req }) => req.payloadAPI],
},
type: 'text',
},
],
slug: 'payload-api-test-ones',
},
{
access: {
read: () => true,
},
fields: [
{
name: 'payloadAPI',
hooks: {
afterRead: [({ req }) => req.payloadAPI],
},
type: 'text',
},
{
name: 'relation',
type: 'relationship',
relationTo: 'payload-api-test-ones',
type: 'relationship',
},
],
slug: 'payload-api-test-twos',
},
],
graphQL: {
queries: (GraphQL) => {
return {
QueryWithInternalError: {
resolve: () => {
// Throwing an internal error with potentially sensitive data
throw new Error('Lost connection to the Pentagon. Secret data: ******')
},
type: new GraphQL.GraphQLObjectType({
name: 'QueryWithInternalError',
fields: {
text: {
type: GraphQL.GraphQLString,
},
},
}),
},
}
},
schemaOutputFile: path.resolve(__dirname, 'schema.graphql'),
},
onInit: async (payload) => {
const user = await payload.create({
await payload.create({
collection: 'users',
data: {
email: devUser.email,
@@ -331,8 +362,8 @@ export default buildConfigWithDefaults({
await payload.create({
collection: slug,
data: {
title: 'has custom ID relation',
relationToCustomID: 1,
title: 'has custom ID relation',
},
})
@@ -353,23 +384,23 @@ export default buildConfigWithDefaults({
await payload.create({
collection: slug,
data: {
title: 'with-description',
description: 'description',
title: 'with-description',
},
})
await payload.create({
collection: slug,
data: {
title: 'numPost1',
number: 1,
title: 'numPost1',
},
})
await payload.create({
collection: slug,
data: {
title: 'numPost2',
number: 2,
title: 'numPost2',
},
})
@@ -390,15 +421,15 @@ export default buildConfigWithDefaults({
await payload.create({
collection: slug,
data: {
title: 'rel to hasMany',
relationHasManyField: rel1.id,
title: 'rel to hasMany',
},
})
await payload.create({
collection: slug,
data: {
title: 'rel to hasMany 2',
relationHasManyField: rel2.id,
title: 'rel to hasMany 2',
},
})
@@ -406,11 +437,11 @@ export default buildConfigWithDefaults({
await payload.create({
collection: slug,
data: {
title: 'rel to multi',
relationMultiRelationTo: {
relationTo: relationSlug,
value: rel2.id,
},
title: 'rel to multi',
},
})
@@ -418,7 +449,6 @@ export default buildConfigWithDefaults({
await payload.create({
collection: slug,
data: {
title: 'rel to multi hasMany',
relationMultiRelationToHasMany: [
{
relationTo: relationSlug,
@@ -429,6 +459,7 @@ export default buildConfigWithDefaults({
value: rel2.id,
},
],
title: 'rel to multi hasMany',
},
})

View File

@@ -5,7 +5,8 @@ import type { Post } from './payload-types'
import payload from '../../packages/payload/src'
import { mapAsync } from '../../packages/payload/src/utilities/mapAsync'
import { initPayloadTest } from '../helpers/configHelpers'
import configPromise, { pointSlug, slug } from './config'
import { idToString } from '../helpers/idToString'
import configPromise, { errorOnHookSlug, pointSlug, slug } from './config'
const title = 'title'
@@ -42,8 +43,7 @@ describe('collections-graphql', () => {
beforeEach(async () => {
existingDoc = await createPost()
existingDocGraphQLID =
payload.db.defaultIDType === 'number' ? existingDoc.id : `"${existingDoc.id}"`
existingDocGraphQLID = idToString(existingDoc.id, payload)
})
it('should create', async () => {
@@ -67,7 +67,7 @@ describe('collections-graphql', () => {
title
}
}`
const response = await client.request(query, { title })
const response = (await client.request(query, { title })) as any
const doc: Post = response.createPost
expect(doc).toMatchObject({ title })
@@ -102,6 +102,92 @@ describe('collections-graphql', () => {
expect(docs).toContainEqual(expect.objectContaining({ id: existingDoc.id }))
})
it('should read using multiple queries', async () => {
const query = `query {
postIDs: Posts {
docs {
id
}
}
posts: Posts {
docs {
id
title
}
}
}`
const response = await client.request(query)
const { postIDs, posts } = response
expect(postIDs.docs).toBeDefined()
expect(posts.docs).toBeDefined()
})
it('should commit or rollback multiple mutations independently', async () => {
const firstTitle = 'first title'
const secondTitle = 'second title'
const first = await payload.create({
collection: errorOnHookSlug,
data: {
errorBeforeChange: true,
title: firstTitle,
},
})
const second = await payload.create({
collection: errorOnHookSlug,
data: {
errorBeforeChange: true,
title: secondTitle,
},
})
const updated = 'updated title'
const query = `mutation {
createPost(data: {title: "${title}"}) {
id
title
}
updateFirst: updateErrorOnHook(id: ${idToString(
first.id,
payload,
)}, data: {title: "${updated}"}) {
title
}
updateSecond: updateErrorOnHook(id: ${idToString(
second.id,
payload,
)}, data: {title: "${updated}"}) {
id
title
}
}`
client.requestConfig.errorPolicy = 'all'
const response = await client.request(query)
client.requestConfig.errorPolicy = 'none'
const createdResult = await payload.findByID({
id: response.createPost.id,
collection: slug,
})
const updateFirstResult = await payload.findByID({
id: first.id,
collection: errorOnHookSlug,
})
const updateSecondResult = await payload.findByID({
id: second.id,
collection: errorOnHookSlug,
})
expect(response?.createPost.id).toBeDefined()
expect(response?.updateFirst).toBeNull()
expect(response?.updateSecond).toBeNull()
expect(createdResult).toMatchObject(response.createPost)
expect(updateFirstResult).toMatchObject(first)
expect(updateSecondResult).toStrictEqual(second)
})
it('should retain payload api', async () => {
const query = `
query {
@@ -685,11 +771,11 @@ describe('collections-graphql', () => {
// language=graphQL
const query = `query {
Posts(where: { title: { exists: true }}) {
docs {
badFieldName
Posts(where: { title: { exists: true }}) {
docs {
badFieldName
}
}
}
}`
await client.request(query).catch((err) => {
error = err
@@ -702,12 +788,12 @@ describe('collections-graphql', () => {
let error
// language=graphQL
const query = `mutation {
createPost(data: {min: 1}) {
id
min
createdAt
updatedAt
}
createPost(data: {min: 1}) {
id
min
createdAt
updatedAt
}
}`
await client.request(query).catch((err) => {
@@ -722,21 +808,21 @@ describe('collections-graphql', () => {
let error
// language=graphQL
const query = `mutation createTest {
test1:createUser(data: { email: "test@test.com", password: "test" }) {
email
}
test1:createUser(data: { email: "test@test.com", password: "test" }) {
email
}
test2:createUser(data: { email: "test2@test.com", password: "" }) {
email
}
test2:createUser(data: { email: "test2@test.com", password: "" }) {
email
}
test3:createUser(data: { email: "test@test.com", password: "test" }) {
email
}
test3:createUser(data: { email: "test@test.com", password: "test" }) {
email
}
test4:createUser(data: { email: "", password: "test" }) {
email
}
test4:createUser(data: { email: "", password: "test" }) {
email
}
}`
await client.request(query).catch((err) => {
@@ -775,9 +861,9 @@ describe('collections-graphql', () => {
let error
// language=graphQL
const query = `query {
QueryWithInternalError {
text
}
QueryWithInternalError {
text
}
}`
await client.request(query).catch((err) => {

View File

@@ -0,0 +1,4 @@
import type { Payload } from '../../packages/payload/src'
export const idToString = (id: number | string, payload: Payload): string =>
`${payload.db.defaultIDType === 'number' ? id : `"${id}"`}`