Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 53 additions & 14 deletions packages/orm/src/client/crud/dialects/base-dialect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
const fieldRef = this.fieldRef(
fieldDef.originModel ?? model,
key,
fieldDef.originModel ?? modelAlias,
this.delegateFieldAlias(model, modelAlias, fieldDef),
true,
computedArgs,
);
Expand Down Expand Up @@ -327,10 +327,10 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
const _order = negateOrderBy ? (order === 'asc' ? 'desc' : 'asc') : order;
const op = j === i ? (_order === 'asc' ? '>=' : '<=') : '=';
// Fields inherited from a delegate base live on the base table, which is
// joined with its model name as alias. See buildSelectModel/buildDelegateJoin.
// joined under `delegateJoinAlias`. See buildSelectModel/buildDelegateJoin.
const fieldDef = requireField(this.schema, model, field);
const outerAlias = fieldDef.originModel ?? modelAlias;
const subSelectAlias = fieldDef.originModel ?? subQueryAlias;
const outerAlias = this.delegateFieldAlias(model, modelAlias, fieldDef);
const subSelectAlias = this.delegateFieldAlias(model, subQueryAlias, fieldDef);
andFilters.push(
this.eb(
this.eb.ref(`${outerAlias}.${field}`),
Expand Down Expand Up @@ -430,7 +430,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
this.schema,
model,
// if field is from a base, use the base model to join
fieldDef.originModel ?? modelAlias,
this.delegateFieldAlias(model, modelAlias, fieldDef),
field,
joinAlias,
);
Expand Down Expand Up @@ -1170,7 +1170,13 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
const buildFieldRef = (model: string, field: string, modelAlias: string, computedArgs?: unknown) => {
const fieldDef = requireField(this.schema, model, field);
return fieldDef.originModel
? this.fieldRef(fieldDef.originModel, field, fieldDef.originModel, true, computedArgs)
? this.fieldRef(
fieldDef.originModel,
field,
this.delegateJoinAlias(model, modelAlias, fieldDef.originModel),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
true,
computedArgs,
)
: this.fieldRef(model, field, modelAlias, true, computedArgs);
};

Expand Down Expand Up @@ -1269,12 +1275,25 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {

// order by to-one relation
const joinAlias = tmpAlias(`${modelAlias}$ob$${index}`);
const joined = query.leftJoin(`${relationModel} as ${joinAlias}`, (join) => {
const joinPairs = buildJoinPairs(this.schema, model, modelAlias, field, joinAlias);
let joined = query.leftJoin(`${relationModel} as ${joinAlias}`, (join) => {
const joinPairs = buildJoinPairs(
this.schema,
model,
// if field is from a base, use the base model to join
this.delegateFieldAlias(model, modelAlias, fieldDef),
field,
joinAlias,
);
return join.on((eb) =>
this.and(...joinPairs.map(([left, right]) => eb(this.eb.ref(left), '=', this.eb.ref(right)))),
);
});
// join delegate bases so that the ordering can reference inherited fields
let joinBase = requireModel(this.schema, relationModel).baseModel;
while (joinBase) {
joined = this.buildDelegateJoin(relationModel, joinAlias, joinBase, joined);
joinBase = requireModel(this.schema, joinBase).baseModel;
}
return this.buildOrderBy(joined, relationModel, joinAlias, value, negated, take);
}

Expand Down Expand Up @@ -1461,6 +1480,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
const descendants = getDelegateDescendantModels(this.schema, model);
for (const subModel of descendants) {
result = this.buildDelegateJoin(model, modelAlias, subModel.name, result);
const subModelAlias = this.delegateJoinAlias(model, modelAlias, subModel.name);
result = result.select(() => {
const jsonObject: Record<string, Expression<any>> = {};
for (const fieldDef of getModelFields(this.schema, subModel.name, { computed: true })) {
Expand All @@ -1471,7 +1491,7 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
if (fieldDef.computed && fieldDef.params) {
continue;
}
jsonObject[fieldDef.name] = this.fieldRef(subModel.name, fieldDef.name, subModel.name);
jsonObject[fieldDef.name] = this.fieldRef(subModel.name, fieldDef.name, subModelAlias);
}
return this.buildJsonObject(jsonObject).as(`${DELEGATE_JOINED_FIELD_PREFIX}${subModel.name}`);
});
Expand Down Expand Up @@ -1532,10 +1552,9 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
): SelectQueryBuilder<any, any, any> {
const fieldDef = requireField(this.schema, model, field);

// if field is defined on a delegate base, the base model is joined with its
// model name from outer query, so we should use it directly as the alias
// if field is defined on a delegate base, it's selected from the joined base table
const fieldModel = fieldDef.originModel ?? model;
const alias = fieldDef.originModel ?? modelAlias;
const alias = this.delegateFieldAlias(model, modelAlias, fieldDef);

// `computedArgs` is set for a parameterized computed field selected via `select`/`include`
return query.select(() => this.fieldRef(fieldModel, field, alias, true, computedArgs).as(field));
Expand All @@ -1544,11 +1563,13 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
buildDelegateJoin(
thisModel: string,
thisModelAlias: string,
otherModelAlias: string,
otherModel: string,
query: SelectQueryBuilder<any, any, any>,
) {
const idFields = requireIdFields(this.schema, thisModel);
query = query.leftJoin(otherModelAlias, (qb) => {
const otherModelAlias = this.delegateJoinAlias(thisModel, thisModelAlias, otherModel);
const table = otherModelAlias === otherModel ? otherModel : `${otherModel} as ${otherModelAlias}`;
query = query.leftJoin(table, (qb) => {
for (const idField of idFields) {
qb = qb.onRef(`${thisModelAlias}.${idField}`, '=', `${otherModelAlias}.${idField}`);
}
Expand All @@ -1557,6 +1578,24 @@ export abstract class BaseCrudDialect<Schema extends SchemaDef> {
return query;
}

/**
* Gets the alias under which a delegate base or sub model is joined to a model selected as `modelAlias`.
* A model selected under its own name (a top-level query) has them joined under their own model names.
* A model selected under another alias (a relation subquery) has them joined under aliases derived from
* it, so they never shadow a table of an enclosing query in the same delegate hierarchy.
*/
delegateJoinAlias(model: string, modelAlias: string, joinedModel: string) {
return modelAlias === model ? joinedModel : tmpAlias(`${modelAlias}$${joinedModel}`);
}

/**
* Gets the alias of the table holding a field of a model selected as `modelAlias`: the joined base
* table for a field inherited from a delegate base, otherwise the model's own table.
*/
delegateFieldAlias(model: string, modelAlias: string, fieldDef: FieldDef) {
return fieldDef.originModel ? this.delegateJoinAlias(model, modelAlias, fieldDef.originModel) : modelAlias;
}

buildCountJson(model: string, eb: ExpressionBuilder<any, any>, parentAlias: string, payload: any) {
const modelDef = requireModel(this.schema, model);
const toManyRelations = Object.entries(modelDef.fields).filter(([, field]) => field.relation && field.array);
Expand Down
2 changes: 1 addition & 1 deletion packages/orm/src/client/crud/operations/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ export abstract class BaseOperationHandler<Schema extends SchemaDef> {
result,
fieldDef.originModel,
field,
fieldDef.originModel,
this.dialect.delegateJoinAlias(model, parentAlias, fieldDef.originModel),
payload,
);
} else {
Expand Down
7 changes: 5 additions & 2 deletions packages/plugins/soft-delete/src/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ class SoftDeleteHandler<Schema extends SchemaDef> extends OperationNodeTransform
// Resolve the single `@deletedAt` column to filter when reading a table. `@@@onceInModel` allows
// at most one marker per inheritance hierarchy, so it's either declared on the model itself or
// inherited from exactly one delegate base — where the column physically lives on the base's
// table (LEFT-joined under its own model name, see buildDelegateJoin), so the filter must key off
// table (LEFT-joined under `delegateJoinAlias`, see buildDelegateJoin), so the filter must key off
// that base rather than the queried table.
private getSoftDeleteTarget(info: TableInfo): { table: string; column: string } | undefined {
const own = this.getDeletedAtField(info.model);
Expand All @@ -218,7 +218,10 @@ class SoftDeleteHandler<Schema extends SchemaDef> extends OperationNodeTransform
while (base) {
const marker = this.getDeletedAtField(base);
if (marker) {
return { table: base, column: marker.name };
return {
table: this.dialect.delegateJoinAlias(info.model, info.alias ?? info.model, base),
column: marker.name,
};
}
base = QueryUtils.getModel(this.client.$schema, base)?.baseModel;
}
Expand Down
39 changes: 39 additions & 0 deletions packages/plugins/soft-delete/test/soft-delete.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,4 +431,43 @@ model Article extends Content {
const row = await raw.article.findUniqueOrThrow({ where: { id: a.id } });
expect(row.title).toBe('a');
});

it('hides soft-deleted delegate rows reached through a relation', async () => {
// In a relation subquery the delegate base is joined under an alias derived from the
// subquery's alias, so the read filter must reference the base through that alias.
const delegateSchema = `
model User {
id Int @id @default(autoincrement())
articles Article[]
}

model Content {
id Int @id @default(autoincrement())
contentType String
deletedAt DateTime? @deletedAt
@@delegate(contentType)
}

model Article extends Content {
title String
author User @relation(fields: [authorId], references: [id])
authorId Int
}
`;
const raw = await createSoftDeleteTestClient(delegateSchema, { usePrismaPush: true });
const db = raw.$use(new SoftDeletePlugin());

const user = await db.user.create({ data: {} });
const kept = await db.article.create({ data: { title: 'kept', authorId: user.id } });
const removed = await db.article.create({ data: { title: 'removed', authorId: user.id } });
await db.article.delete({ where: { id: removed.id } });

await expect(db.user.findUnique({ where: { id: user.id }, include: { articles: true } })).resolves.toEqual(
expect.objectContaining({ articles: [expect.objectContaining({ id: kept.id })] }),
);
await expect(
db.user.findUnique({ where: { id: user.id }, include: { _count: { select: { articles: true } } } }),
).resolves.toMatchObject({ _count: { articles: 1 } });
await expect(db.user.count({ where: { articles: { some: { title: 'removed' } } } })).resolves.toBe(0);
});
});
175 changes: 175 additions & 0 deletions tests/regression/test/issue-2870.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import { createPolicyTestClient, createTestClient } from '@zenstackhq/testtools';
import { describe, expect, it } from 'vitest';

// https://github.com/zenstackhq/zenstack/issues/2870
describe('Regression for issue #2870', () => {
const schema = `
model Item {
id String @id @default(cuid())
itemKind String
rank Int @default(0)
notesAsSource Note[] @relation("NoteSource")
pinnedTaskId String?
pinnedTask Task? @relation("Pinned", fields: [pinnedTaskId], references: [id])
tags Tag[] @relation("ItemTags")

@@delegate(itemKind)
}

model Task extends Item {
pinnedBy Item[] @relation("Pinned")
}

model Tag extends Item {
taggedItems Item[] @relation("ItemTags")
}

model Note extends Item {
sourceId String?
source Item? @relation("NoteSource", fields: [sourceId], references: [id])
comments Comment[]
}

model Comment {
id String @id @default(cuid())
noteId String?
note Note? @relation(fields: [noteId], references: [id])
}
`;

// explicit ids so that ordering by id differs from ordering by relation count
async function setup() {
const db = await createTestClient(schema, { usePrismaPush: true });
const task = await db.task.create({ data: { id: 'z-task' } });
const note = await db.note.create({ data: { id: 'a-note', sourceId: task.id, pinnedTaskId: task.id } });
const tag = await db.tag.create({ data: { id: 'm-tag', taggedItems: { connect: { id: note.id } } } });
return { db, task, note, tag };
}

const ids = (rows: { id: string }[]) => rows.map((r) => r.id).sort();

it('filters a base model by a to-many relation into its own hierarchy', async () => {
const { db, task, note, tag } = await setup();

expect(ids(await db.item.findMany({ where: { notesAsSource: { some: {} } } }))).toEqual([task.id]);
expect(ids(await db.item.findMany({ where: { notesAsSource: { none: {} } } }))).toEqual(
[note.id, tag.id].sort(),
);
expect(ids(await db.item.findMany({ where: { notesAsSource: { every: { id: { not: note.id } } } } }))).toEqual(
[note.id, tag.id].sort(),
);
await expect(db.item.count({ where: { notesAsSource: { some: {} } } })).resolves.toBe(1);
expect(ids(await db.item.findMany({ where: { tags: { some: {} } } }))).toEqual([note.id]);
});

it('filters by a to-one relation into the same hierarchy', async () => {
const { db, note } = await setup();

expect(ids(await db.item.findMany({ where: { pinnedTask: { is: {} } } }))).toEqual([note.id]);
expect(ids(await db.note.findMany({ where: { pinnedTask: { is: {} } } }))).toEqual([note.id]);
expect(ids(await db.note.findMany({ where: { source: { is: { itemKind: 'Task' } } } }))).toEqual([note.id]);
});

it('counts a base model relation into its own hierarchy', async () => {
const { db, task, note } = await setup();

await expect(
db.item.findUnique({ where: { id: task.id }, include: { _count: { select: { notesAsSource: true } } } }),
).resolves.toMatchObject({ _count: { notesAsSource: 1 } });
await expect(
db.item.findUnique({
where: { id: task.id },
select: { _count: { select: { notesAsSource: { where: { id: note.id } } } } },
}),
).resolves.toEqual({ _count: { notesAsSource: 1 } });
await expect(
db.item.findUnique({ where: { id: note.id }, include: { _count: { select: { tags: true } } } }),
).resolves.toMatchObject({ _count: { tags: 1 } });
await expect(
db.item.findFirst({ orderBy: [{ notesAsSource: { _count: 'desc' } }, { id: 'asc' }] }),
).resolves.toMatchObject({ id: task.id });
});

it('orders by an inherited field of a to-one relation', async () => {
const db = await createTestClient(schema, { usePrismaPush: true });
const highTask = await db.task.create({ data: { rank: 2 } });
const lowTask = await db.task.create({ data: { rank: 1 } });
// each note's own rank is the opposite of its pinned task's rank
const first = await db.note.create({ data: { rank: 2, pinnedTaskId: lowTask.id } });
const second = await db.note.create({ data: { rank: 1, pinnedTaskId: highTask.id } });

// outer model in the same hierarchy as the related model's base
await expect(db.note.findMany({ orderBy: { pinnedTask: { rank: 'asc' } } })).resolves.toEqual([
expect.objectContaining({ id: first.id }),
expect.objectContaining({ id: second.id }),
]);

// outer model outside the hierarchy
const c1 = await db.comment.create({ data: { noteId: second.id } });
const c2 = await db.comment.create({ data: { noteId: first.id } });
await expect(db.comment.findMany({ orderBy: { note: { rank: 'desc' } } })).resolves.toEqual([
expect.objectContaining({ id: c2.id }),
expect.objectContaining({ id: c1.id }),
]);
});

it('includes relations into the same hierarchy', async () => {
const { db, task, note, tag } = await setup();

const taskWithNotes = await db.item.findUnique({ where: { id: task.id }, include: { notesAsSource: true } });
expect(ids(taskWithNotes!.notesAsSource)).toEqual([note.id]);

await expect(db.note.findUnique({ where: { id: note.id }, include: { source: true } })).resolves.toMatchObject({
source: { id: task.id },
});
await expect(
db.item.findUnique({ where: { id: note.id }, include: { pinnedTask: true } }),
).resolves.toMatchObject({ pinnedTask: { id: task.id } });

const noteWithTags = await db.item.findUnique({ where: { id: note.id }, include: { tags: true } });
expect(ids(noteWithTags!.tags)).toEqual([tag.id]);
});

it('gives the same result through the sub model', async () => {
const { db, task } = await setup();

expect(ids(await db.task.findMany({ where: { notesAsSource: { some: {} } } }))).toEqual([task.id]);
expect(ids(await db.task.findMany({ where: { pinnedBy: { some: {} } } }))).toEqual([task.id]);
await expect(
db.task.findFirst({ include: { _count: { select: { notesAsSource: true } } } }),
).resolves.toMatchObject({ _count: { notesAsSource: 1 } });
});

it('includes relations into the same hierarchy with access policies', async () => {
const db = await createPolicyTestClient(`
model Item {
id String @id @default(cuid())
itemKind String
notesAsSource Note[] @relation("NoteSource")

@@delegate(itemKind)
@@allow('create', true)
@@allow('read', notesAsSource?[true] || itemKind == 'Note')
}

model Task extends Item {}

model Note extends Item {
sourceId String?
source Item? @relation("NoteSource", fields: [sourceId], references: [id])
}
`);
const raw = db.$unuseAll();
const task = await raw.task.create({ data: {} });
const hidden = await raw.task.create({ data: {} });
const note = await raw.note.create({ data: { sourceId: task.id } });

expect(ids(await db.item.findMany())).toEqual([task.id, note.id].sort());
await expect(db.task.findUnique({ where: { id: hidden.id } })).resolves.toBeNull();
await expect(db.note.findFirst({ include: { source: true } })).resolves.toMatchObject({
source: { id: task.id },
});
const taskWithNotes = await db.item.findUnique({ where: { id: task.id }, include: { notesAsSource: true } });
expect(ids(taskWithNotes!.notesAsSource)).toEqual([note.id]);
});
});
Loading