diff --git a/packages/runtime/src/client/crud/operations/base.ts b/packages/runtime/src/client/crud/operations/base.ts index f2c5769a..926bb907 100644 --- a/packages/runtime/src/client/crud/operations/base.ts +++ b/packages/runtime/src/client/crud/operations/base.ts @@ -1325,6 +1325,11 @@ export abstract class BaseOperationHandler { return (returnData ? [] : { count: 0 }) as Result; } + const modelDef = this.requireModel(model); + if (modelDef.baseModel && limit !== undefined) { + throw new QueryError('Updating with a limit is not supported for polymorphic models'); + } + filterModel ??= model; let updateFields: any = {}; @@ -1335,7 +1340,6 @@ export abstract class BaseOperationHandler { updateFields[field] = this.processScalarFieldUpdateData(model, field, data); } - const modelDef = this.requireModel(model); let shouldFallbackToIdFilter = false; if (limit !== undefined && !this.dialect.supportsUpdateWithLimit) { @@ -1358,7 +1362,6 @@ export abstract class BaseOperationHandler { modelDef.baseModel, where, updateFields, - limit, filterModel, ); updateFields = baseResult.remainingFields; @@ -1412,7 +1415,6 @@ export abstract class BaseOperationHandler { model: string, where: any, updateFields: any, - limit: number | undefined, filterModel: GetModels, ) { const thisUpdateFields: any = {}; @@ -1433,7 +1435,7 @@ export abstract class BaseOperationHandler { model as GetModels, where, thisUpdateFields, - limit, + undefined, false, filterModel, ); @@ -1983,24 +1985,18 @@ export abstract class BaseOperationHandler { const fieldDef = this.requireField(fromRelation.model, fromRelation.field); invariant(fieldDef.relation?.opposite); - deleteResult = await this.delete( - kysely, - model, - { - AND: [ - { - [fieldDef.relation.opposite]: { - some: fromRelation.ids, - }, - }, - { - OR: deleteConditions, + deleteResult = await this.delete(kysely, model, { + AND: [ + { + [fieldDef.relation.opposite]: { + some: fromRelation.ids, }, - ], - }, - undefined, - false, - ); + }, + { + OR: deleteConditions, + }, + ], + }); } else { const { ownedByModel, keyPairs } = getRelationForeignKeyFieldPairs( this.schema, @@ -2018,36 +2014,24 @@ export abstract class BaseOperationHandler { const fieldDef = this.requireField(fromRelation.model, fromRelation.field); invariant(fieldDef.relation?.opposite); - deleteResult = await this.delete( - kysely, - model, - { - AND: [ - // filter for parent - Object.fromEntries(keyPairs.map(({ fk, pk }) => [pk, fromEntity[fk]])), - { - OR: deleteConditions, - }, - ], - }, - undefined, - false, - ); + deleteResult = await this.delete(kysely, model, { + AND: [ + // filter for parent + Object.fromEntries(keyPairs.map(({ fk, pk }) => [pk, fromEntity[fk]])), + { + OR: deleteConditions, + }, + ], + }); } else { - deleteResult = await this.delete( - kysely, - model, - { - AND: [ - Object.fromEntries(keyPairs.map(({ fk, pk }) => [fk, fromRelation.ids[pk]])), - { - OR: deleteConditions, - }, - ], - }, - undefined, - false, - ); + deleteResult = await this.delete(kysely, model, { + AND: [ + Object.fromEntries(keyPairs.map(({ fk, pk }) => [fk, fromRelation.ids[pk]])), + { + OR: deleteConditions, + }, + ], + }); } } @@ -2064,54 +2048,109 @@ export abstract class BaseOperationHandler { // #endregion - protected async delete< - ReturnData extends boolean, - Result = ReturnData extends true ? unknown[] : { count: number }, - >( + protected async delete( kysely: ToKysely, model: GetModels, where: any, - limit: number | undefined, - returnData: ReturnData, - ): Promise { + limit?: number, + filterModel?: GetModels, + ): Promise<{ count: number }> { + filterModel ??= model; + + const modelDef = this.requireModel(model); + + if (modelDef.baseModel) { + if (limit !== undefined) { + throw new QueryError('Deleting with a limit is not supported for polymorphic models'); + } + // just delete base and it'll cascade back to this model + return this.processBaseModelDelete(kysely, modelDef.baseModel, where, limit, filterModel); + } + let query = kysely.deleteFrom(model); + let needIdFilter = false; + + if (limit !== undefined && !this.dialect.supportsDeleteWithLimit) { + // if the dialect doesn't support delete with limit natively, we'll + // simulate it by filtering by id with a limit + needIdFilter = true; + } - if (limit === undefined) { + if (modelDef.isDelegate || modelDef.baseModel) { + // if the model is in a delegate hierarchy, we'll need to filter by + // id because the filter may involve fields in different models in + // the hierarchy + needIdFilter = true; + } + + if (!needIdFilter) { query = query.where((eb) => this.dialect.buildFilter(eb, model, model, where)); } else { - if (this.dialect.supportsDeleteWithLimit) { - query = query.where((eb) => this.dialect.buildFilter(eb, model, model, where)).limit(limit!); - } else { - query = query.where((eb) => - eb( - eb.refTuple( - // @ts-expect-error - ...this.buildIdFieldRefs(kysely, model), - ), - 'in', - kysely - .selectFrom(model) - .where((eb) => this.dialect.buildFilter(eb, model, model, where)) - .select(this.buildIdFieldRefs(kysely, model)) - .limit(limit!), + query = query.where((eb) => + eb( + eb.refTuple( + // @ts-expect-error + ...this.buildIdFieldRefs(kysely, model), ), - ); - } + 'in', + this.dialect + .buildSelectModel(eb, filterModel) + .where((eb) => this.dialect.buildFilter(eb, filterModel, filterModel, where)) + .select(this.buildIdFieldRefs(kysely, filterModel)) + .$if(limit !== undefined, (qb) => qb.limit(limit!)), + ), + ); } + // if the model being deleted has a relation to a model that extends a delegate model, and if that + // relation is set to trigger a cascade delete from this model, the deletion will not automatically + // clean up the base hierarchy of the relation side (because polymorphic model's cascade deletion + // works downward not upward). We need to take care of the base deletions manually here. + await this.processDelegateRelationDelete(kysely, modelDef, where, limit); + query = query.modifyEnd(this.makeContextComment({ model, operation: 'delete' })); + const result = await query.executeTakeFirstOrThrow(); + return { count: Number(result.numDeletedRows) }; + } - if (returnData) { - const result = await query.execute(); - return result as Result; - } else { - const result = (await query.executeTakeFirstOrThrow()) as DeleteResult; - return { - count: Number(result.numDeletedRows), - } as Result; + private async processDelegateRelationDelete( + kysely: ToKysely, + modelDef: ModelDef, + where: any, + limit: number | undefined, + ) { + for (const fieldDef of Object.values(modelDef.fields)) { + if (fieldDef.relation && fieldDef.relation.opposite) { + const oppositeModelDef = this.requireModel(fieldDef.type); + const oppositeRelation = this.requireField(fieldDef.type, fieldDef.relation.opposite); + if (oppositeModelDef.baseModel && oppositeRelation.relation?.onDelete === 'Cascade') { + if (limit !== undefined) { + throw new QueryError('Deleting with a limit is not supported for polymorphic models'); + } + // the deletion will propagate upward to the base model chain + await this.delete( + kysely, + fieldDef.type as GetModels, + { + [fieldDef.relation.opposite]: where, + }, + undefined, + ); + } + } } } + private async processBaseModelDelete( + kysely: ToKysely, + model: string, + where: any, + limit: number | undefined, + filterModel: GetModels, + ) { + return this.delete(kysely, model as GetModels, where, limit, filterModel); + } + protected makeIdSelect(model: string) { const modelDef = this.requireModel(model); return modelDef.idFields.reduce((acc, f) => { @@ -2154,9 +2193,7 @@ export abstract class BaseOperationHandler { } else { // otherwise, create a new transaction and execute the callback let txBuilder = this.kysely.transaction(); - if (isolationLevel) { - txBuilder = txBuilder.setIsolationLevel(isolationLevel); - } + txBuilder = txBuilder.setIsolationLevel(isolationLevel ?? 'repeatable read'); return txBuilder.execute(callback); } } diff --git a/packages/runtime/src/client/crud/operations/delete.ts b/packages/runtime/src/client/crud/operations/delete.ts index e20c48be..a33c2179 100644 --- a/packages/runtime/src/client/crud/operations/delete.ts +++ b/packages/runtime/src/client/crud/operations/delete.ts @@ -27,15 +27,22 @@ export class DeleteOperationHandler extends BaseOperat if (!existing) { throw new NotFoundError(this.model); } - const result = await this.delete(this.kysely, this.model, args.where, undefined, false); - if (result.count === 0) { - throw new NotFoundError(this.model); - } + + // TODO: avoid using transaction for simple delete + await this.safeTransaction(async (tx) => { + const result = await this.delete(tx, this.model, args.where, undefined); + if (result.count === 0) { + throw new NotFoundError(this.model); + } + }); + return existing; } async runDeleteMany(args: DeleteManyArgs> | undefined) { - const result = await this.delete(this.kysely, this.model, args?.where, args?.limit, false); - return result; + return await this.safeTransaction(async (tx) => { + const result = await this.delete(tx, this.model, args?.where, args?.limit); + return result; + }); } } diff --git a/packages/runtime/test/client-api/delegate.test.ts b/packages/runtime/test/client-api/delegate.test.ts index 6c8ece12..791cbf45 100644 --- a/packages/runtime/test/client-api/delegate.test.ts +++ b/packages/runtime/test/client-api/delegate.test.ts @@ -18,7 +18,7 @@ model User { model Comment { id Int @id @default(autoincrement()) content String - asset Asset? @relation(fields: [assetId], references: [id]) + asset Asset? @relation(fields: [assetId], references: [id], onDelete: Cascade) assetId Int? } @@ -27,7 +27,7 @@ model Asset { createdAt DateTime @default(now()) updatedAt DateTime @updatedAt viewCount Int @default(0) - owner User? @relation(fields: [ownerId], references: [id]) + owner User? @relation(fields: [ownerId], references: [id], onDelete: Cascade) ownerId Int? comments Comment[] assetType String @@ -45,13 +45,13 @@ model Video extends Asset { model RatedVideo extends Video { rating Int - user User? @relation(name: 'direct', fields: [userId], references: [id]) + user User? @relation(name: 'direct', fields: [userId], references: [id], onDelete: Cascade) userId Int? } model Image extends Asset { format String - gallery Gallery? @relation(fields: [galleryId], references: [id]) + gallery Gallery? @relation(fields: [galleryId], references: [id], onDelete: Cascade) galleryId Int? } @@ -214,6 +214,32 @@ model Gallery { ]), ); }); + + it('ensures create is atomic', async () => { + // create with a relation that fails + await expect( + client.ratedVideo.create({ + data: { + duration: 100, + url: 'abc', + rating: 5, + }, + }), + ).toResolveTruthy(); + await expect( + client.ratedVideo.create({ + data: { + duration: 200, + url: 'abc', + rating: 3, + }, + }), + ).rejects.toThrow('constraint'); + + await expect(client.ratedVideo.findMany()).toResolveWithLength(1); + await expect(client.video.findMany()).toResolveWithLength(1); + await expect(client.asset.findMany()).toResolveWithLength(1); + }); }); it('works with find', async () => { @@ -805,6 +831,15 @@ model Gallery { }), ]), ); + + // updateMany with limit unsupported + await expect( + client.ratedVideo.updateMany({ + where: { duration: { gt: 200 } }, + data: { viewCount: 200, duration: 300 }, + limit: 1, + }), + ).rejects.toThrow('Updating with a limit is not supported for polymorphic models'); }); it('works with updateManyAndReturn', async () => { @@ -900,5 +935,140 @@ model Gallery { }); }); }); + + describe('Delegate delete tests', () => { + it('works with delete', async () => { + // delete from sub model + await client.ratedVideo.create({ + data: { + id: 1, + duration: 100, + url: 'abc', + rating: 5, + }, + }); + await expect( + client.ratedVideo.delete({ + where: { url: 'abc' }, + }), + ).resolves.toMatchObject({ + id: 1, + duration: 100, + url: 'abc', + rating: 5, + }); + await expect(client.ratedVideo.findMany()).toResolveWithLength(0); + await expect(client.video.findMany()).toResolveWithLength(0); + await expect(client.asset.findMany()).toResolveWithLength(0); + + // delete from base model + await client.ratedVideo.create({ + data: { + id: 1, + duration: 100, + url: 'abc', + rating: 5, + }, + }); + await expect( + client.asset.delete({ + where: { id: 1 }, + }), + ).resolves.toMatchObject({ + id: 1, + duration: 100, + url: 'abc', + rating: 5, + }); + await expect(client.ratedVideo.findMany()).toResolveWithLength(0); + await expect(client.video.findMany()).toResolveWithLength(0); + await expect(client.asset.findMany()).toResolveWithLength(0); + + // nested delete + await client.user.create({ + data: { + id: 1, + email: 'abc', + }, + }); + await client.ratedVideo.create({ + data: { + id: 1, + duration: 100, + url: 'abc', + rating: 5, + owner: { connect: { id: 1 } }, + }, + }); + await expect( + client.user.update({ + where: { id: 1 }, + data: { + assets: { + delete: { id: 1 }, + }, + }, + include: { assets: true }, + }), + ).resolves.toMatchObject({ assets: [] }); + await expect(client.ratedVideo.findMany()).toResolveWithLength(0); + await expect(client.video.findMany()).toResolveWithLength(0); + await expect(client.asset.findMany()).toResolveWithLength(0); + + // delete user should cascade to ratedVideo and in turn delete its bases + await client.ratedVideo.create({ + data: { + id: 1, + duration: 100, + url: 'abc', + rating: 5, + user: { connect: { id: 1 } }, + }, + }); + await expect( + client.user.delete({ + where: { id: 1 }, + }), + ).toResolveTruthy(); + await expect(client.ratedVideo.findMany()).toResolveWithLength(0); + await expect(client.video.findMany()).toResolveWithLength(0); + await expect(client.asset.findMany()).toResolveWithLength(0); + }); + + it('works with deleteMany', async () => { + await client.ratedVideo.createMany({ + data: [ + { + id: 1, + viewCount: 1, + duration: 100, + url: 'abc', + rating: 5, + }, + { + id: 2, + viewCount: 2, + duration: 200, + url: 'def', + rating: 4, + }, + ], + }); + + await expect( + client.video.deleteMany({ + where: { duration: { gt: 150 }, viewCount: 1 }, + }), + ).resolves.toMatchObject({ count: 0 }); + await expect( + client.video.deleteMany({ + where: { duration: { gt: 150 }, viewCount: 2 }, + }), + ).resolves.toMatchObject({ count: 1 }); + await expect(client.ratedVideo.findMany()).toResolveWithLength(1); + await expect(client.video.findMany()).toResolveWithLength(1); + await expect(client.asset.findMany()).toResolveWithLength(1); + }); + }); }, );