Skip to content

Commit 9699d5b

Browse files
authored
fix(delegate,policy): several issues due to interaction between delegate and policy (#299)
1 parent 8d9f296 commit 9699d5b

File tree

8 files changed

+372
-210
lines changed

8 files changed

+372
-210
lines changed

packages/plugins/policy/src/expression-transformer.ts

Lines changed: 60 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -575,20 +575,34 @@ export class ExpressionTransformer<Schema extends SchemaDef> {
575575
}
576576

577577
const fromModel = context.model;
578+
const relationFieldDef = QueryUtils.requireField(this.schema, fromModel, field);
578579
const { keyPairs, ownedByModel } = QueryUtils.getRelationForeignKeyFieldPairs(this.schema, fromModel, field);
579580

580581
let condition: OperationNode;
581582
if (ownedByModel) {
582583
// `fromModel` owns the fk
584+
583585
condition = conjunction(
584586
this.dialect,
585-
keyPairs.map(({ fk, pk }) =>
586-
BinaryOperationNode.create(
587-
ReferenceNode.create(ColumnNode.create(fk), TableNode.create(context.alias ?? fromModel)),
587+
keyPairs.map(({ fk, pk }) => {
588+
let fkRef: OperationNode = ReferenceNode.create(
589+
ColumnNode.create(fk),
590+
TableNode.create(context.alias ?? fromModel),
591+
);
592+
if (relationFieldDef.originModel && relationFieldDef.originModel !== fromModel) {
593+
fkRef = this.buildDelegateBaseFieldSelect(
594+
fromModel,
595+
context.alias ?? fromModel,
596+
fk,
597+
relationFieldDef.originModel,
598+
);
599+
}
600+
return BinaryOperationNode.create(
601+
fkRef,
588602
OperatorNode.create('='),
589603
ReferenceNode.create(ColumnNode.create(pk), TableNode.create(relationModel)),
590-
),
591-
),
604+
);
605+
}),
592606
);
593607
} else {
594608
// `relationModel` owns the fk
@@ -633,8 +647,47 @@ export class ExpressionTransformer<Schema extends SchemaDef> {
633647
return relationQuery.toOperationNode();
634648
}
635649

636-
private createColumnRef(column: string, context: ExpressionTransformerContext<Schema>): ReferenceNode {
637-
return ReferenceNode.create(ColumnNode.create(column), TableNode.create(context.alias ?? context.model));
650+
private createColumnRef(column: string, context: ExpressionTransformerContext<Schema>) {
651+
// if field comes from a delegate base model, we need to use the join alias
652+
// of that base model
653+
654+
const tableName = context.alias ?? context.model;
655+
656+
// "create" policies evaluate table from "VALUES" node so no join from delegate bases are
657+
// created and thus we should directly use the model table name
658+
if (context.operation === 'create') {
659+
return ReferenceNode.create(ColumnNode.create(column), TableNode.create(tableName));
660+
}
661+
662+
const fieldDef = QueryUtils.requireField(this.schema, context.model, column);
663+
if (!fieldDef.originModel || fieldDef.originModel === context.model) {
664+
return ReferenceNode.create(ColumnNode.create(column), TableNode.create(tableName));
665+
}
666+
667+
return this.buildDelegateBaseFieldSelect(context.model, tableName, column, fieldDef.originModel);
668+
}
669+
670+
private buildDelegateBaseFieldSelect(model: string, modelAlias: string, field: string, baseModel: string) {
671+
const idFields = QueryUtils.requireIdFields(this.client.$schema, model);
672+
return {
673+
kind: 'SelectQueryNode',
674+
from: FromNode.create([TableNode.create(baseModel)]),
675+
selections: [
676+
SelectionNode.create(ReferenceNode.create(ColumnNode.create(field), TableNode.create(baseModel))),
677+
],
678+
where: WhereNode.create(
679+
conjunction(
680+
this.dialect,
681+
idFields.map((idField) =>
682+
BinaryOperationNode.create(
683+
ReferenceNode.create(ColumnNode.create(idField), TableNode.create(baseModel)),
684+
OperatorNode.create('='),
685+
ReferenceNode.create(ColumnNode.create(idField), TableNode.create(modelAlias)),
686+
),
687+
),
688+
),
689+
),
690+
} satisfies SelectQueryNode;
638691
}
639692

640693
private isAuthCall(value: unknown): value is CallExpression {

packages/plugins/policy/src/functions.ts

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,18 +36,60 @@ export const check: ZModelFunction<any> = (
3636
invariant(!fieldDef.array, `Field "${fieldName}" is a to-many relation, which is not supported by "check"`);
3737
const relationModel = fieldDef.type;
3838

39-
const op = arg2Node ? (arg2Node.value as CRUD) : operation;
39+
// build the join condition between the current model and the related model
40+
const joinConditions: Expression<any>[] = [];
41+
const fkInfo = QueryUtils.getRelationForeignKeyFieldPairs(client.$schema, model, fieldName);
42+
const idFields = QueryUtils.requireIdFields(client.$schema, model);
4043

41-
const policyHandler = new PolicyHandler(client);
44+
// helper to build a base model select for delegate models
45+
const buildBaseSelect = (baseModel: string, field: string): Expression<any> => {
46+
return eb
47+
.selectFrom(baseModel)
48+
.select(field)
49+
.where(
50+
eb.and(
51+
idFields.map((idField) =>
52+
eb(eb.ref(`${fieldDef.originModel}.${idField}`), '=', eb.ref(`${modelAlias}.${idField}`)),
53+
),
54+
),
55+
);
56+
};
57+
58+
if (fkInfo.ownedByModel) {
59+
// model owns the relation
60+
joinConditions.push(
61+
...fkInfo.keyPairs.map(({ fk, pk }) => {
62+
let fkRef: Expression<any>;
63+
if (fieldDef.originModel && fieldDef.originModel !== model) {
64+
// relation is actually defined in a delegate base model, select from there
65+
fkRef = buildBaseSelect(fieldDef.originModel, fk);
66+
} else {
67+
fkRef = eb.ref(`${modelAlias}.${fk}`);
68+
}
69+
return eb(fkRef, '=', eb.ref(`${relationModel}.${pk}`));
70+
}),
71+
);
72+
} else {
73+
// related model owns the relation
74+
joinConditions.push(
75+
...fkInfo.keyPairs.map(({ fk, pk }) => {
76+
let pkRef: Expression<any>;
77+
if (fieldDef.originModel && fieldDef.originModel !== model) {
78+
// relation is actually defined in a delegate base model, select from there
79+
pkRef = buildBaseSelect(fieldDef.originModel, pk);
80+
} else {
81+
pkRef = eb.ref(`${modelAlias}.${pk}`);
82+
}
83+
return eb(pkRef, '=', eb.ref(`${relationModel}.${fk}`));
84+
}),
85+
);
86+
}
4287

43-
// join with parent model
44-
const joinPairs = QueryUtils.buildJoinPairs(client.$schema, model, modelAlias, fieldName, relationModel);
45-
const joinCondition =
46-
joinPairs.length === 1
47-
? eb(eb.ref(joinPairs[0]![0]), '=', eb.ref(joinPairs[0]![1]))
48-
: eb.and(joinPairs.map(([left, right]) => eb(eb.ref(left), '=', eb.ref(right))));
88+
const joinCondition = joinConditions.length === 1 ? joinConditions[0]! : eb.and(joinConditions);
4989

5090
// policy condition of the related model
91+
const policyHandler = new PolicyHandler(client);
92+
const op = arg2Node ? (arg2Node.value as CRUD) : operation;
5193
const policyCondition = policyHandler.buildPolicyFilter(relationModel, undefined, op);
5294

5395
// build the final nested select that evaluates the policy condition

packages/plugins/policy/src/policy-handler.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,10 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
297297
// #region overrides
298298

299299
protected override transformSelectQuery(node: SelectQueryNode) {
300+
if (!node.from) {
301+
return super.transformSelectQuery(node);
302+
}
303+
300304
let whereNode = this.transformNode(node.where);
301305

302306
// get combined policy filter for all froms, and merge into where clause
@@ -327,6 +331,7 @@ export class PolicyHandler<Schema extends SchemaDef> extends OperationNodeTransf
327331

328332
// build a nested query with policy filter applied
329333
const filter = this.buildPolicyFilter(table.model, table.alias, 'read');
334+
330335
const nestedSelect: SelectQueryNode = {
331336
kind: 'SelectQueryNode',
332337
from: FromNode.create([node.table]),

packages/testtools/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
"description": "ZenStack Test Tools",
55
"type": "module",
66
"scripts": {
7-
"build": "tsc --noEmit && tsup-node && copyfiles -f ./src/types.d.ts ./dist",
7+
"build": "tsc --noEmit && tsup-node",
88
"watch": "tsup-node --watch",
99
"lint": "eslint src --ext ts",
1010
"pack": "pnpm pack"
@@ -53,7 +53,6 @@
5353
"@types/pg": "^8.11.11",
5454
"@zenstackhq/eslint-config": "workspace:*",
5555
"@zenstackhq/typescript-config": "workspace:*",
56-
"copyfiles": "^2.4.1",
5756
"typescript": "catalog:"
5857
}
5958
}

packages/testtools/tsup.config.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import fs from 'fs';
12
import { defineConfig } from 'tsup';
23

34
export default defineConfig({
@@ -7,7 +8,9 @@ export default defineConfig({
78
outDir: 'dist',
89
splitting: false,
910
sourcemap: true,
10-
clean: true,
1111
dts: true,
1212
format: ['cjs', 'esm'],
13+
async onSuccess() {
14+
fs.cpSync('src/types.d.ts', 'dist/types.d.ts', { force: true });
15+
},
1316
});

0 commit comments

Comments
 (0)