diff --git a/packages/runtime/src/enhancements/node/delegate.ts b/packages/runtime/src/enhancements/node/delegate.ts index 59bc79793..54c1abf96 100644 --- a/packages/runtime/src/enhancements/node/delegate.ts +++ b/packages/runtime/src/enhancements/node/delegate.ts @@ -902,7 +902,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { } else { // translate to plain `update` for nested write into base fields const findArgs = { - where: clone(args.where), + where: clone(args.where ?? {}), select: this.queryUtils.makeIdSelection(model), }; await this.injectUpdateHierarchy(db, model, findArgs); @@ -959,7 +959,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { this.injectWhereHierarchy(model, (args as any)?.where); this.doProcessUpdatePayload(model, (args as any)?.data); } else { - const where = this.queryUtils.buildReversedQuery(context, false, false); + const where = await this.queryUtils.buildReversedQuery(db, context, false, false); await this.queryUtils.transaction(db, async (tx) => { await this.doUpdateMany(tx, model, { ...args, where }, simpleUpdateMany); }); @@ -1022,7 +1022,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { }, delete: async (model, _args, context) => { - const where = this.queryUtils.buildReversedQuery(context, false, false); + const where = await this.queryUtils.buildReversedQuery(db, context, false, false); await this.queryUtils.transaction(db, async (tx) => { await this.doDelete(tx, model, { where }); }); @@ -1030,7 +1030,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { }, deleteMany: async (model, _args, context) => { - const where = this.queryUtils.buildReversedQuery(context, false, false); + const where = await this.queryUtils.buildReversedQuery(db, context, false, false); await this.queryUtils.transaction(db, async (tx) => { await this.doDeleteMany(tx, model, where); }); @@ -1095,7 +1095,7 @@ export class DelegateProxyHandler extends DefaultPrismaProxyHandler { private async doDeleteMany(db: CrudContract, model: string, where: any): Promise<{ count: number }> { // query existing entities with id const idSelection = this.queryUtils.makeIdSelection(model); - const findArgs = { where: clone(where), select: idSelection }; + const findArgs = { where: clone(where ?? {}), select: idSelection }; this.injectWhereHierarchy(model, findArgs.where); if (this.options.logPrismaQuery) { diff --git a/packages/runtime/src/enhancements/node/policy/handler.ts b/packages/runtime/src/enhancements/node/policy/handler.ts index 5c5fdd4ca..4285d3bd2 100644 --- a/packages/runtime/src/enhancements/node/policy/handler.ts +++ b/packages/runtime/src/enhancements/node/policy/handler.ts @@ -809,7 +809,7 @@ export class PolicyProxyHandler implements Pr const unsafe = isUnsafeMutate(model, args, this.modelMeta); // handles the connection to upstream entity - const reversedQuery = this.policyUtils.buildReversedQuery(context, true, unsafe); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context, true, unsafe); if ((!unsafe || context.field.isRelationOwner) && reversedQuery[context.field.backLink]) { // if mutation is safe, or current field owns the relation (so the other side has no fk), // and the reverse query contains the back link, then we can build a "connect" with it @@ -885,7 +885,7 @@ export class PolicyProxyHandler implements Pr if (args.skipDuplicates) { // get a reversed query to include fields inherited from upstream mutation, // it'll be merged with the create payload for unique constraint checking - const upstreamQuery = this.policyUtils.buildReversedQuery(context); + const upstreamQuery = await this.policyUtils.buildReversedQuery(db, context); if (await this.hasDuplicatedUniqueConstraint(model, item, upstreamQuery, db)) { if (this.shouldLogQuery) { this.logger.info(`[policy] \`createMany\` skipping duplicate ${formatObject(item)}`); @@ -910,7 +910,7 @@ export class PolicyProxyHandler implements Pr if (operation === 'disconnect') { // disconnect filter is not unique, need to build a reversed query to // locate the entity and use its id fields as unique filter - const reversedQuery = this.policyUtils.buildReversedQuery(context); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context); const found = await db[model].findUnique({ where: reversedQuery, select: this.policyUtils.makeIdSelection(model), @@ -936,7 +936,7 @@ export class PolicyProxyHandler implements Pr const visitor = new NestedWriteVisitor(this.modelMeta, { update: async (model, args, context) => { // build a unique query including upstream conditions - const uniqueFilter = this.policyUtils.buildReversedQuery(context); + const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context); // handle not-found const existing = await this.policyUtils.checkExistence(db, model, uniqueFilter, true); @@ -997,7 +997,7 @@ export class PolicyProxyHandler implements Pr if (preValueSelect) { select = { ...select, ...preValueSelect }; } - const reversedQuery = this.policyUtils.buildReversedQuery(context); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context); const currentSetQuery = { select, where: reversedQuery }; this.policyUtils.injectAuthGuardAsWhere(db, currentSetQuery, model, 'read'); @@ -1027,7 +1027,7 @@ export class PolicyProxyHandler implements Pr } else { // we have to process `updateMany` separately because the guard may contain // filters using relation fields which are not allowed in nested `updateMany` - const reversedQuery = this.policyUtils.buildReversedQuery(context); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context); const updateWhere = this.policyUtils.and(reversedQuery, updateGuard); if (this.shouldLogQuery) { this.logger.info( @@ -1066,7 +1066,7 @@ export class PolicyProxyHandler implements Pr upsert: async (model, args, context) => { // build a unique query including upstream conditions - const uniqueFilter = this.policyUtils.buildReversedQuery(context); + const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context); // branch based on if the update target exists const existing = await this.policyUtils.checkExistence(db, model, uniqueFilter); @@ -1090,7 +1090,7 @@ export class PolicyProxyHandler implements Pr // convert upsert to update const convertedUpdate = { - where: args.where, + where: args.where ?? {}, data: this.validateUpdateInputSchema(model, args.update), }; this.mergeToParent(context.parent, 'update', convertedUpdate); @@ -1143,7 +1143,7 @@ export class PolicyProxyHandler implements Pr set: async (model, args, context) => { // find the set of items to be replaced - const reversedQuery = this.policyUtils.buildReversedQuery(context); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context); const findCurrSetArgs = { select: this.policyUtils.makeIdSelection(model), where: reversedQuery, @@ -1162,7 +1162,7 @@ export class PolicyProxyHandler implements Pr delete: async (model, args, context) => { // build a unique query including upstream conditions - const uniqueFilter = this.policyUtils.buildReversedQuery(context); + const uniqueFilter = await this.policyUtils.buildReversedQuery(db, context); // handle not-found await this.policyUtils.checkExistence(db, model, uniqueFilter, true); @@ -1179,7 +1179,7 @@ export class PolicyProxyHandler implements Pr } else { // we have to process `deleteMany` separately because the guard may contain // filters using relation fields which are not allowed in nested `deleteMany` - const reversedQuery = this.policyUtils.buildReversedQuery(context); + const reversedQuery = await this.policyUtils.buildReversedQuery(db, context); const deleteWhere = this.policyUtils.and(reversedQuery, guard); if (this.shouldLogQuery) { this.logger.info(`[policy] \`deleteMany\` ${model}:\n${formatObject({ where: deleteWhere })}`); @@ -1579,12 +1579,15 @@ export class PolicyProxyHandler implements Pr if (this.shouldLogQuery) { this.logger.info( `[policy] \`findMany\` ${this.model}: ${formatObject({ - where: args.where, + where: args.where ?? {}, select: candidateSelect, })}` ); } - const candidates = await tx[this.model].findMany({ where: args.where, select: candidateSelect }); + const candidates = await tx[this.model].findMany({ + where: args.where ?? {}, + select: candidateSelect, + }); // build a ID filter based on id values filtered by the additional checker const { idFilter } = this.buildIdFilterWithEntityChecker(candidates, entityChecker.func); diff --git a/packages/runtime/src/enhancements/node/policy/policy-utils.ts b/packages/runtime/src/enhancements/node/policy/policy-utils.ts index ef4285d78..0e655a3e5 100644 --- a/packages/runtime/src/enhancements/node/policy/policy-utils.ts +++ b/packages/runtime/src/enhancements/node/policy/policy-utils.ts @@ -30,7 +30,6 @@ import { } from '../../../types'; import { getVersion } from '../../../version'; import type { InternalEnhancementOptions } from '../create-enhancement'; -import { Logger } from '../logger'; import { QueryUtils } from '../query-utils'; import type { DelegateConstraint, @@ -47,7 +46,6 @@ import { formatObject, prismaClientKnownRequestError } from '../utils'; * Access policy enforcement utilities */ export class PolicyUtil extends QueryUtils { - private readonly logger: Logger; private readonly modelMeta: ModelMeta; private readonly policy: PolicyDef; private readonly zodSchemas?: ZodSchemas; @@ -62,7 +60,6 @@ export class PolicyUtil extends QueryUtils { ) { super(db, options); - this.logger = new Logger(db); this.user = context?.user; ({ diff --git a/packages/runtime/src/enhancements/node/query-utils.ts b/packages/runtime/src/enhancements/node/query-utils.ts index 75e729b0f..58cd0098d 100644 --- a/packages/runtime/src/enhancements/node/query-utils.ts +++ b/packages/runtime/src/enhancements/node/query-utils.ts @@ -10,11 +10,17 @@ import { } from '../../cross'; import type { CrudContract, DbClientContract } from '../../types'; import { getVersion } from '../../version'; +import { formatObject } from '../edge'; import { InternalEnhancementOptions } from './create-enhancement'; +import { Logger } from './logger'; import { prismaClientUnknownRequestError, prismaClientValidationError } from './utils'; export class QueryUtils { - constructor(private readonly prisma: DbClientContract, protected readonly options: InternalEnhancementOptions) {} + protected readonly logger: Logger; + + constructor(private readonly prisma: DbClientContract, protected readonly options: InternalEnhancementOptions) { + this.logger = new Logger(prisma); + } getIdFields(model: string) { return getIdFields(this.options.modelMeta, model, true); @@ -60,7 +66,12 @@ export class QueryUtils { /** * Builds a reversed query for the given nested path. */ - buildReversedQuery(context: NestedWriteVisitorContext, forMutationPayload = false, unsafeOperation = false) { + async buildReversedQuery( + db: CrudContract, + context: NestedWriteVisitorContext, + forMutationPayload = false, + uncheckedOperation = false + ) { let result, currQuery: any; let currField: FieldInfo | undefined; @@ -102,8 +113,8 @@ export class QueryUtils { const shouldPreserveRelationCondition = // doing a mutation forMutationPayload && - // and it's a safe mutate - !unsafeOperation && + // and it's not an unchecked mutate + !uncheckedOperation && // and the current segment is the direct parent (the last one is the mutate itself), // the relation condition should be preserved and will be converted to a "connect" later i === context.nestingPath.length - 2; @@ -111,8 +122,26 @@ export class QueryUtils { if (fkMapping && !shouldPreserveRelationCondition) { // turn relation condition into foreign key condition, e.g.: // { user: { id: 1 } } => { userId: 1 } + + let parentPk = visitWhere; + if (Object.keys(fkMapping).some((k) => !(k in parentPk) || parentPk[k] === undefined)) { + // it can happen that the parent condition actually doesn't contain all id fields + // （when the parent condition is not a primary key but unique constraints) + // and in such case we need to load it to get the pks + + if (this.options.logPrismaQuery && this.logger.enabled('info')) { + this.logger.info( + `[reverseLookup] \`findUniqueOrThrow\` ${model}: ${formatObject(where)}` + ); + } + parentPk = await db[model].findUniqueOrThrow({ + where, + select: this.makeIdSelection(model), + }); + } + for (const [r, fk] of Object.entries(fkMapping)) { - currQuery[fk] = visitWhere[r]; + currQuery[fk] = parentPk[r]; } if (i > 0) { diff --git a/packages/server/src/api/rest/index.ts b/packages/server/src/api/rest/index.ts index 16de93637..c50e5aa5b 100644 --- a/packages/server/src/api/rest/index.ts +++ b/packages/server/src/api/rest/index.ts @@ -678,7 +678,7 @@ class RequestHandler extends APIHandlerBase { args.take = limit; const [entities, count] = await Promise.all([ prisma[type].findMany(args), - prisma[type].count({ where: args.where }), + prisma[type].count({ where: args.where ?? {} }), ]); const total = count as number; diff --git a/packages/testtools/src/schema.ts b/packages/testtools/src/schema.ts index 4a7b575bd..51814a3e7 100644 --- a/packages/testtools/src/schema.ts +++ b/packages/testtools/src/schema.ts @@ -96,7 +96,11 @@ datasource db { generator js { provider = 'prisma-client-js' - ${options.previewFeatures ? `previewFeatures = ${JSON.stringify(options.previewFeatures)}` : ''} + ${ + options.previewFeatures + ? `previewFeatures = ${JSON.stringify(options.previewFeatures)}` + : 'previewFeatures = ["strictUndefinedChecks"]' + } } plugin enhancer { diff --git a/tests/integration/tests/enhancements/with-policy/unique-as-id.test.ts b/tests/integration/tests/enhancements/with-policy/unique-as-id.test.ts index a7ec74fa5..3f4de2fd1 100644 --- a/tests/integration/tests/enhancements/with-policy/unique-as-id.test.ts +++ b/tests/integration/tests/enhancements/with-policy/unique-as-id.test.ts @@ -171,4 +171,127 @@ describe('With Policy: unique as id', () => { }) ).toResolveTruthy(); }); + + it('unique fields with to-many nested update', async () => { + const { enhance } = await loadSchema( + ` + model A { + id Int @id @default(autoincrement()) + x Int + y Int + value Int + bs B[] + @@unique([x, y]) + + @@allow('read,create', true) + @@allow('update,delete', value > 0) + } + + model B { + id Int @id @default(autoincrement()) + value Int + a A @relation(fields: [aId], references: [id]) + aId Int + + @@allow('all', value > 0) + } + `, + { logPrismaQuery: true } + ); + + const db = enhance(); + + await db.a.create({ + data: { x: 1, y: 1, value: 1, bs: { create: [{ id: 1, value: 1 }] } }, + }); + + await db.a.create({ + data: { x: 2, y: 2, value: 2, bs: { create: [{ id: 2, value: 2 }] } }, + }); + + await db.a.update({ + where: { x_y: { x: 1, y: 1 } }, + data: { bs: { updateMany: { data: { value: 3 } } } }, + }); + + // check b#1 is updated + await expect(db.b.findUnique({ where: { id: 1 } })).resolves.toMatchObject({ value: 3 }); + + // check b#2 is not affected + await expect(db.b.findUnique({ where: { id: 2 } })).resolves.toMatchObject({ value: 2 }); + + await db.a.update({ + where: { x_y: { x: 1, y: 1 } }, + data: { bs: { deleteMany: {} } }, + }); + + // check b#1 is deleted + await expect(db.b.findUnique({ where: { id: 1 } })).resolves.toBeNull(); + + // check b#2 is not affected + await expect(db.b.findUnique({ where: { id: 2 } })).resolves.toMatchObject({ value: 2 }); + }); + + it('unique fields with to-one nested update', async () => { + const { enhance } = await loadSchema( + ` + model A { + id Int @id @default(autoincrement()) + x Int + y Int + value Int + b B? + @@unique([x, y]) + + @@allow('read,create', true) + @@allow('update,delete', value > 0) + } + + model B { + id Int @id @default(autoincrement()) + value Int + a A @relation(fields: [aId], references: [id]) + aId Int @unique + + @@allow('all', value > 0) + } + ` + ); + + const db = enhance(); + + await db.a.create({ + data: { x: 1, y: 1, value: 1, b: { create: { id: 1, value: 1 } } }, + }); + + await db.a.create({ + data: { x: 2, y: 2, value: 2, b: { create: { id: 2, value: 2 } } }, + }); + + await db.a.update({ + where: { x_y: { x: 1, y: 1 } }, + data: { b: { update: { data: { value: 3 } } } }, + }); + + // check b#1 is updated + await expect(db.b.findUnique({ where: { id: 1 } })).resolves.toMatchObject({ value: 3 }); + + // check b#2 is not affected + await expect(db.b.findUnique({ where: { id: 2 } })).resolves.toMatchObject({ value: 2 }); + + await db.a.update({ + where: { x_y: { x: 1, y: 1 } }, + data: { b: { delete: true } }, + }); + + // check b#1 is deleted + await expect(db.b.findUnique({ where: { id: 1 } })).resolves.toBeNull(); + await expect(db.a.findUnique({ where: { x_y: { x: 1, y: 1 } }, include: { b: true } })).resolves.toMatchObject({ + b: null, + }); + + // check b#2 is not affected + await expect(db.b.findUnique({ where: { id: 2 } })).resolves.toMatchObject({ value: 2 }); + await expect(db.a.findUnique({ where: { x_y: { x: 2, y: 2 } }, include: { b: true } })).resolves.toBeTruthy(); + }); }); diff --git a/tests/regression/tests/issue-1964.test.ts b/tests/regression/tests/issue-1964.test.ts new file mode 100644 index 000000000..e90aae32e --- /dev/null +++ b/tests/regression/tests/issue-1964.test.ts @@ -0,0 +1,128 @@ +import { loadSchema } from '@zenstackhq/testtools'; + +describe('issue 1964', () => { + it('regression1', async () => { + const { enhance } = await loadSchema( + ` +model User { + id Int @id + orgId String +} + +model Author { + id Int @id @default(autoincrement()) + orgId String + name String + posts Post[] + + @@unique([orgId, name]) + @@allow('all', auth().orgId == orgId) +} + +model Post { + id Int @id @default(autoincrement()) + orgId String + title String + author Author @relation(fields: [authorId], references: [id]) + authorId Int + + @@allow('all', auth().orgId == orgId) +} + `, + { + previewFeatures: ['strictUndefinedChecks'], + } + ); + + const db = enhance({ id: 1, orgId: 'org' }); + + const newauthor = await db.author.create({ + data: { + name: `Foo ${Date.now()}`, + orgId: 'org', + posts: { + createMany: { data: [{ title: 'Hello', orgId: 'org' }] }, + }, + }, + include: { posts: true }, + }); + + await expect( + db.author.update({ + where: { orgId_name: { orgId: 'org', name: newauthor.name } }, + data: { + name: `Bar ${Date.now()}`, + posts: { deleteMany: { id: { equals: newauthor.posts[0].id } } }, + }, + }) + ).toResolveTruthy(); + }); + + it('regression2', async () => { + const { enhance } = await loadSchema( + ` +model User { + id Int @id @default(autoincrement()) + slug String @unique + profile Profile? + @@allow('all', true) +} + +model Profile { + id Int @id @default(autoincrement()) + slug String @unique + name String + addresses Address[] + userId Int? @unique + user User? @relation(fields: [userId], references: [id]) + @@allow('all', true) +} + +model Address { + id Int @id @default(autoincrement()) + profileId Int @unique + profile Profile @relation(fields: [profileId], references: [id]) + city String + @@allow('all', true) +} + `, + { + previewFeatures: ['strictUndefinedChecks'], + } + ); + + const db = enhance({ id: 1, orgId: 'org' }); + + await db.user.create({ + data: { + slug: `user1`, + profile: { + create: { + name: `My Profile`, + slug: 'profile1', + addresses: { + create: { id: 1, city: 'City' }, + }, + }, + }, + }, + }); + + await expect( + db.user.update({ + where: { slug: 'user1' }, + data: { + profile: { + update: { + addresses: { + deleteMany: { id: { equals: 1 } }, + }, + }, + }, + }, + }) + ).toResolveTruthy(); + + await expect(db.address.count()).resolves.toEqual(0); + }); +}); diff --git a/tests/regression/tests/issue-765.test.ts b/tests/regression/tests/issue-765.test.ts index f29ef1a5a..13f88bf19 100644 --- a/tests/regression/tests/issue-765.test.ts +++ b/tests/regression/tests/issue-765.test.ts @@ -21,7 +21,10 @@ describe('Regression: issue 765', () => { @@allow('all', true) } - ` + `, + { + previewFeatures: [], + } ); const db = enhance();