diff --git a/packages/db-mongodb/src/connect.ts b/packages/db-mongodb/src/connect.ts index d52ba649a84..7e7b86fa64b 100644 --- a/packages/db-mongodb/src/connect.ts +++ b/packages/db-mongodb/src/connect.ts @@ -29,7 +29,15 @@ export const connect: Connect = async function connect( ...this.connectOptions, } - if (hotReload) { + const shouldDropDatabase = !hotReload && process.env.PAYLOAD_DROP_DATABASE === 'true' + + // Mongoose starts building indexes as soon as the connection opens, and does not + // wait for those builds to finish. They then run at the same time as the drop below + // and as the first writes from `onInit`, which makes MongoDB reject the write with + // `Cannot create collection - database is in the process of being dropped` or + // `Unable to write to collection due to catalog changes`. Build the indexes after + // the drop instead, and wait for them. + if (hotReload || shouldDropDatabase) { connectionOptions.autoIndex = false } @@ -81,19 +89,21 @@ export const connect: Connect = async function connect( this.beginTransaction = defaultBeginTransaction() } - if (!hotReload) { - if (process.env.PAYLOAD_DROP_DATABASE === 'true') { - this.payload.logger.info('---- DROPPING DATABASE ----') - await this.connection.dropDatabase() + if (shouldDropDatabase) { + this.payload.logger.info('---- DROPPING DATABASE ----') + await this.connection.dropDatabase() - this.payload.logger.info('---- DROPPED DATABASE ----') - } + this.payload.logger.info('---- DROPPED DATABASE ----') } - if (this.ensureIndexes) { + // Version models were previously left to Mongoose's background builds, so their + // indexes were still missing when `connect` resolved. `globals` is excluded because + // every global shares that one collection through a discriminator, and their + // auto-named indexes collide with each other. + if (this.ensureIndexes || shouldDropDatabase) { await Promise.all( - this.payload.config.collections.map(async (coll) => { - await this.collections[coll.slug]?.ensureIndexes() + [...Object.values(this.collections), ...Object.values(this.versions)].map(async (model) => { + await model.ensureIndexes() }), ) } diff --git a/test/database/int.spec.ts b/test/database/int.spec.ts index b5819beb20f..21ee0f00fe9 100644 --- a/test/database/int.spec.ts +++ b/test/database/int.spec.ts @@ -1806,6 +1806,42 @@ describe('database', () => { }) }) + describe('index creation on init', { db: 'mongo' }, () => { + it('should build every declared index on version collections before init resolves', async () => { + const db = payload.db as MongooseAdapter + + const missingIndexes: string[] = [] + + for (const [slug, model] of Object.entries(db.versions)) { + const declaredKeys = model.schema.indexes().map(([fields]) => Object.keys(fields).join('_')) + + if (declaredKeys.length === 0) { + continue + } + + const namespaces = await db.connection + .db!.listCollections({ name: model.collection.name }) + .toArray() + + if (namespaces.length === 0) { + missingIndexes.push(`${slug}: collection does not exist`) + continue + } + + const builtIndexes = await db.connection.db!.collection(model.collection.name).indexes() + const builtKeys = builtIndexes.map((index) => Object.keys(index.key).join('_')) + + for (const declaredKey of declaredKeys) { + if (!builtKeys.includes(declaredKey)) { + missingIndexes.push(`${slug}: ${declaredKey}`) + } + } + } + + expect(missingIndexes).toEqual([]) + }) + }) + describe('migrations', () => { let ranFreshTest = false diff --git a/test/database/postgres-vector.int.spec.ts b/test/database/postgres-vector.int.spec.ts index 6480eb4a945..b2722aa421c 100644 --- a/test/database/postgres-vector.int.spec.ts +++ b/test/database/postgres-vector.int.spec.ts @@ -112,7 +112,9 @@ describePostgres('postgres vector custom column', () => { const similarity = sql`1 - (${cosineDistance(payload.db.tables.posts.embedding, catEmbedding)})` - const db = payload.db.drizzle as PostgresDB + // `payload.db.drizzle` is replica-routed when readReplicas are configured, so the + // rows written above may not have replicated yet. Query the primary directly. + const db = (payload.db.primaryDrizzle ?? payload.db.drizzle) as PostgresDB const res = await db .select() @@ -234,7 +236,9 @@ describePostgres('postgres vector custom column', () => { const distance = sql`(${l2Distance(payload.db.tables.posts.embedding, catEmbedding)})` - const db = payload.db.drizzle as PostgresDB + // `payload.db.drizzle` is replica-routed when readReplicas are configured, so the + // rows written above may not have replicated yet. Query the primary directly. + const db = (payload.db.primaryDrizzle ?? payload.db.drizzle) as PostgresDB const res = await db .select() @@ -346,7 +350,9 @@ describePostgres('postgres vector custom column', () => { const similarity = sql`1 - (${jaccardDistance(payload.db.tables.posts.embedding, catEmbedding)})` - const db = payload.db.drizzle as PostgresDB + // `payload.db.drizzle` is replica-routed when readReplicas are configured, so the + // rows written above may not have replicated yet. Query the primary directly. + const db = (payload.db.primaryDrizzle ?? payload.db.drizzle) as PostgresDB const res = await db .select() diff --git a/test/plugin-ecommerce/int.spec.ts b/test/plugin-ecommerce/int.spec.ts index b1ca613b878..704f6178646 100644 --- a/test/plugin-ecommerce/int.spec.ts +++ b/test/plugin-ecommerce/int.spec.ts @@ -32,6 +32,11 @@ async function createGuestCartWithItems( const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + // The cart secret is the only thing authorising the guest calls below. When it is + // missing, every later request fails access control and reports "cart not found", + // which reads as an unrelated failure many tests further down. + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + // Add an item using the add-item endpoint await client .POST(`/carts/${cartId}/add-item`, { @@ -313,6 +318,14 @@ describe('ecommerce', () => { limit: 1, }) variantId = variants.docs[0]?.id as string + + // `?.` leaves these undefined when the seed is incomplete, which the hook happily + // accepts and every dependent test then fails on for unrelated-looking reasons. + if (!productId || !variantId) { + throw new Error( + `ecommerce seed is incomplete: productId=${productId}, variantId=${variantId}`, + ) + } }) describe('add-item endpoint', () => { @@ -331,6 +344,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + // Add an item using the endpoint const addItemResponse = await restClient .POST(`/carts/${cartId}/add-item`, { @@ -371,6 +386,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + const addItemResponse = await restClient .POST(`/carts/${cartId}/add-item`, { auth: false, @@ -409,6 +426,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + // Add item first time await restClient .POST(`/carts/${cartId}/add-item`, { @@ -470,6 +489,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + const addItemResponse = await restClient .POST(`/carts/${cartId}/add-item`, { auth: false, @@ -611,6 +632,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + // Add item with quantity 5 await restClient .POST(`/carts/${cartId}/add-item`, { @@ -695,6 +718,8 @@ describe('ecommerce', () => { const cartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + // Add multiple items await restClient .POST(`/carts/${cartId}/add-item`, { @@ -774,6 +799,14 @@ describe('ecommerce', () => { limit: 1, }) variantId = variants.docs[0]?.id as string + + // `?.` leaves these undefined when the seed is incomplete, which the hook happily + // accepts and every dependent test then fails on for unrelated-looking reasons. + if (!productId || !variantId) { + throw new Error( + `ecommerce seed is incomplete: productId=${productId}, variantId=${variantId}`, + ) + } }) it('should merge guest cart into user cart', async () => { @@ -857,6 +890,8 @@ describe('ecommerce', () => { const guestCartId = createResponse.doc.id const cartSecret = createResponse.doc.secret + expect(cartSecret, 'guest cart was created without a secret').toBeTruthy() + await restClient .POST(`/carts/${guestCartId}/add-item`, { auth: false, @@ -1061,6 +1096,12 @@ describe('ecommerce', () => { limit: 1, }) productId = products.docs[0]?.id as string + + // `?.` leaves this undefined when the seed is incomplete, which the hook happily + // accepts and every dependent test then fails on for unrelated-looking reasons. + if (!productId) { + throw new Error(`ecommerce seed is incomplete: productId=${productId}`) + } }) it('should allow authenticated users to access their cart without secret', async () => { @@ -1182,6 +1223,12 @@ describe('ecommerce', () => { limit: 1, }) productId = products.docs[0]?.id as string + + // `?.` leaves this undefined when the seed is incomplete, which the hook happily + // accepts and every dependent test then fails on for unrelated-looking reasons. + if (!productId) { + throw new Error(`ecommerce seed is incomplete: productId=${productId}`) + } }) it('should allow transferring guest cart to user by updating customer field', async () => { diff --git a/test/plugin-ecommerce/seed/index.ts b/test/plugin-ecommerce/seed/index.ts index 56c7a9bc2e2..686abedafb4 100644 --- a/test/plugin-ecommerce/seed/index.ts +++ b/test/plugin-ecommerce/seed/index.ts @@ -12,6 +12,34 @@ const colorVariantOptions = [ { label: 'White', value: 'white' }, ] +/** + * MongoDB cannot create the same collection namespace from two concurrent + * transactions. On the first write to `variantOptions` the namespace and its + * versions namespace do not exist yet, so creating the options in parallel + * makes them race and fail with a TransientTransactionError. + */ +const createVariantOptions = async ( + payload: Payload, + variantTypeID: string, + options: { label: string; value: string }[], +) => { + const created = [] + + for (const option of options) { + created.push( + await payload.create({ + collection: 'variantOptions', + data: { + ...option, + variantType: variantTypeID, + }, + }), + ) + } + + return created +} + export const seed = async (payload: Payload): Promise => { payload.logger.info('Seeding data for ecommerce...') const req = {} as PayloadRequest @@ -34,16 +62,10 @@ export const seed = async (payload: Payload): Promise => { }, }) - const [small, medium, large, xlarge] = await Promise.all( - sizeVariantOptions.map((option) => { - return payload.create({ - collection: 'variantOptions', - data: { - ...option, - variantType: sizeVariantType.id, - }, - }) - }), + const [small, medium, large, xlarge] = await createVariantOptions( + payload, + sizeVariantType.id, + sizeVariantOptions, ) const colorVariantType = await payload.create({ @@ -54,16 +76,10 @@ export const seed = async (payload: Payload): Promise => { }, }) - const [black, white] = await Promise.all( - colorVariantOptions.map((option) => { - return payload.create({ - collection: 'variantOptions', - data: { - ...option, - variantType: colorVariantType.id, - }, - }) - }), + const [black, white] = await createVariantOptions( + payload, + colorVariantType.id, + colorVariantOptions, ) const hoodieProduct = await payload.create({ @@ -138,7 +154,9 @@ export const seed = async (payload: Payload): Promise => { return true } catch (err) { - console.error(err) - return false + // Swallowing this leaves the collections empty and surfaces thirteen tests later + // as unrelated cart failures, so fail the suite where the problem actually is. + payload.logger.error({ err, msg: 'Seeding data for ecommerce failed' }) + throw err } }