From e3a2eb5c9a658f429dc7e25e14d910d6d64b99f7 Mon Sep 17 00:00:00 2001 From: Dustin Townsend Date: Sun, 23 Nov 2025 20:34:15 -0500 Subject: [PATCH] fix: broken tests from middleware add --- .../src/middlewares/__tests__/builtin.test.ts | 2 +- .../middlewares/__tests__/middleware.test.ts | 2 +- .../mizzle-orm/src/query/collection-facade.ts | 214 ++++++++++-------- 3 files changed, 122 insertions(+), 96 deletions(-) diff --git a/packages/mizzle-orm/src/middlewares/__tests__/builtin.test.ts b/packages/mizzle-orm/src/middlewares/__tests__/builtin.test.ts index d789ca8..22c0cf1 100644 --- a/packages/mizzle-orm/src/middlewares/__tests__/builtin.test.ts +++ b/packages/mizzle-orm/src/middlewares/__tests__/builtin.test.ts @@ -636,7 +636,7 @@ describe('Built-in Middlewares', () => { dbName: 'test', schema, middlewares: [ - onOperations(['create', 'update'], async (ctx, next) => { + onOperations(['create', 'updateById'], async (ctx, next) => { executedOperations.push(ctx.operation); return next(); }), diff --git a/packages/mizzle-orm/src/middlewares/__tests__/middleware.test.ts b/packages/mizzle-orm/src/middlewares/__tests__/middleware.test.ts index ee1ac5d..addb3b9 100644 --- a/packages/mizzle-orm/src/middlewares/__tests__/middleware.test.ts +++ b/packages/mizzle-orm/src/middlewares/__tests__/middleware.test.ts @@ -445,7 +445,7 @@ describe('Middleware System', () => { // Delete await db().users.deleteById(user._id); - expect(operations).toEqual(['create', 'findOne', 'findById', 'findMany', 'count', 'updateById', 'delete']); + expect(operations).toEqual(['create', 'findOne', 'findById', 'findMany', 'count', 'updateById', 'deleteById']); await db.close(); }); diff --git a/packages/mizzle-orm/src/query/collection-facade.ts b/packages/mizzle-orm/src/query/collection-facade.ts index 63f4870..4a227f3 100644 --- a/packages/mizzle-orm/src/query/collection-facade.ts +++ b/packages/mizzle-orm/src/query/collection-facade.ts @@ -341,7 +341,13 @@ export class CollectionFacade< */ async updateById(id: string | ObjectId, data: TUpdate): Promise { const filter = this.buildIdFilter(id); - return this.updateOne(filter, data); + return this.executeWithMiddlewares( + 'updateById', + async () => { + return this.updateOneInternal(filter, data); + }, + { filter, data }, + ); } /** @@ -351,73 +357,80 @@ export class CollectionFacade< return this.executeWithMiddlewares( 'update', async () => { - const finalFilter = this.applyPolicies(filter); - - // Get old document for hooks and policies - const oldDoc = await this.collection.findOne(finalFilter, { - session: this.ctx.session, - }); - if (!oldDoc) { - return null; - } - - // Apply update timestamp - const updateData = this.applyUpdateTimestamps(data as any); + return this.updateOneInternal(filter, data); + }, + { filter, data, oldDoc: undefined }, // oldDoc will be fetched inside + ); + } - // Run before hooks - let finalUpdate = updateData; - if (this.collectionDef._meta.hooks.beforeUpdate) { - finalUpdate = await this.collectionDef._meta.hooks.beforeUpdate( - this.ctx, - oldDoc as any, - updateData, - ); - } + /** + * Internal update logic (shared by updateOne and updateById) + */ + private async updateOneInternal(filter: Filter, data: TUpdate): Promise { + const finalFilter = this.applyPolicies(filter); - // Check policies - if (this.collectionDef._meta.policies.canUpdate) { - const allowed = await this.collectionDef._meta.policies.canUpdate( - this.ctx, - oldDoc as any, - finalUpdate, - ); - if (!allowed) { - throw new Error('Update not allowed by policy'); - } - } + // Get old document for hooks and policies + const oldDoc = await this.collection.findOne(finalFilter, { + session: this.ctx.session, + }); + if (!oldDoc) { + return null; + } - // Validate references - await this.relationHelper.validateReferences(finalUpdate as any); + // Apply update timestamp + const updateData = this.applyUpdateTimestamps(data as any); - // Process forward embeds (fetch and embed referenced data) - finalUpdate = (await this.relationHelper.processForwardEmbeds(finalUpdate as any)) as any; - - // Update - const result = await this.collection.findOneAndUpdate( - finalFilter, - { $set: finalUpdate } as any, - { - returnDocument: 'after', - session: this.ctx.session, - }, - ); + // Run before hooks + let finalUpdate = updateData; + if (this.collectionDef._meta.hooks.beforeUpdate) { + finalUpdate = await this.collectionDef._meta.hooks.beforeUpdate( + this.ctx, + oldDoc as any, + updateData, + ); + } - if (!result) { - return null; - } + // Check policies + if (this.collectionDef._meta.policies.canUpdate) { + const allowed = await this.collectionDef._meta.policies.canUpdate( + this.ctx, + oldDoc as any, + finalUpdate, + ); + if (!allowed) { + throw new Error('Update not allowed by policy'); + } + } - // Run after hooks - if (this.collectionDef._meta.hooks.afterUpdate) { - await this.collectionDef._meta.hooks.afterUpdate(this.ctx, oldDoc as any, result as any); - } + // Validate references + await this.relationHelper.validateReferences(finalUpdate as any); - // Propagate reverse embeds if this collection is a source for any embeds - await this.propagateReverseEmbeds(result as TDoc, finalUpdate); + // Process forward embeds (fetch and embed referenced data) + finalUpdate = (await this.relationHelper.processForwardEmbeds(finalUpdate as any)) as any; - return result as TDoc; + // Update + const result = await this.collection.findOneAndUpdate( + finalFilter, + { $set: finalUpdate } as any, + { + returnDocument: 'after', + session: this.ctx.session, }, - { filter, data, oldDoc: undefined }, // oldDoc will be fetched inside ); + + if (!result) { + return null; + } + + // Run after hooks + if (this.collectionDef._meta.hooks.afterUpdate) { + await this.collectionDef._meta.hooks.afterUpdate(this.ctx, oldDoc as any, result as any); + } + + // Propagate reverse embeds if this collection is a source for any embeds + await this.propagateReverseEmbeds(result as TDoc, finalUpdate); + + return result as TDoc; } /** @@ -445,7 +458,13 @@ export class CollectionFacade< */ async deleteById(id: string | ObjectId): Promise { const filter = this.buildIdFilter(id); - return this.deleteOne(filter); + return this.executeWithMiddlewares( + 'deleteById', + async () => { + return this.deleteOneInternal(filter); + }, + { filter }, + ); } /** @@ -455,48 +474,55 @@ export class CollectionFacade< return this.executeWithMiddlewares( 'delete', async () => { - const finalFilter = this.applyPolicies(filter); + return this.deleteOneInternal(filter); + }, + { filter }, + ); + } - // Get document for hooks and policies - const doc = await this.collection.findOne(finalFilter, { - session: this.ctx.session, - }); - if (!doc) { - return false; - } + /** + * Internal delete logic (shared by deleteOne and deleteById) + */ + private async deleteOneInternal(filter: Filter): Promise { + const finalFilter = this.applyPolicies(filter); - // Run before hooks - if (this.collectionDef._meta.hooks.beforeDelete) { - await this.collectionDef._meta.hooks.beforeDelete(this.ctx, doc as any); - } + // Get document for hooks and policies + const doc = await this.collection.findOne(finalFilter, { + session: this.ctx.session, + }); + if (!doc) { + return false; + } - // Check policies - if (this.collectionDef._meta.policies.canDelete) { - const allowed = await this.collectionDef._meta.policies.canDelete(this.ctx, doc as any); - if (!allowed) { - throw new Error('Delete not allowed by policy'); - } - } + // Run before hooks + if (this.collectionDef._meta.hooks.beforeDelete) { + await this.collectionDef._meta.hooks.beforeDelete(this.ctx, doc as any); + } - // Delete - const result = await this.collection.deleteOne(finalFilter, { - session: this.ctx.session, - }); + // Check policies + if (this.collectionDef._meta.policies.canDelete) { + const allowed = await this.collectionDef._meta.policies.canDelete(this.ctx, doc as any); + if (!allowed) { + throw new Error('Delete not allowed by policy'); + } + } - // Run after hooks - if (result.deletedCount > 0 && this.collectionDef._meta.hooks.afterDelete) { - await this.collectionDef._meta.hooks.afterDelete(this.ctx, doc as any); - } + // Delete + const result = await this.collection.deleteOne(finalFilter, { + session: this.ctx.session, + }); - // Handle delete cascades - if (result.deletedCount > 0) { - await this.handleDeleteCascades(doc as TDoc); - } + // Run after hooks + if (result.deletedCount > 0 && this.collectionDef._meta.hooks.afterDelete) { + await this.collectionDef._meta.hooks.afterDelete(this.ctx, doc as any); + } - return result.deletedCount > 0; - }, - { filter }, - ); + // Handle delete cascades + if (result.deletedCount > 0) { + await this.handleDeleteCascades(doc as TDoc); + } + + return result.deletedCount > 0; } /**