Repository navigation
feat(policy): field-level update policies on M2M relation fields #2858
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -166,6 +166,52 @@ export function isRelationshipField(field: DataField) { | |
| return isDataModel(field.type.reference?.ref); | ||
| } | ||
|
|
||
| /** | ||
| * Returns the name of the relation the given field belongs to, as declared in its `@relation` | ||
| * attribute, or `undefined` if the field has no `@relation` attribute or no explicit name. | ||
| */ | ||
| function getRelationName(field: DataField): string | undefined { | ||
| const relAttr = field.attributes.find((attr) => attr.decl.ref?.name === '@relation'); | ||
| if (!relAttr) { | ||
| return undefined; | ||
| } | ||
| for (const arg of relAttr.args) { | ||
| if (!arg.name || arg.name === 'name') { | ||
| if (isStringLiteral(arg.value)) { | ||
| return arg.value.value; | ||
| } | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| /** | ||
| * Returns if the given field is a many-to-many relation field, i.e. a relation field that is an | ||
| * array and whose opposite relation field on the referenced model (belonging to the same relation) | ||
| * is also an array referencing back to the containing model. | ||
| */ | ||
| export function isManyToManyField(field: DataField) { | ||
| if (!isRelationshipField(field) || !field.type.array) { | ||
| return false; | ||
| } | ||
|
|
||
| const oppositeModel = field.type.reference!.ref as DataModel; | ||
| const containingModel = field.$container as DataModel; | ||
| const relationName = getRelationName(field); | ||
|
|
||
| return getAllFields(oppositeModel).some((f) => { | ||
| if (f === field || !f.type.array || f.type.reference?.ref?.name !== containingModel.name) { | ||
| return false; | ||
| } | ||
| // if the field declares an explicit relation name, the opposite field must belong to the | ||
| // same relation; otherwise any array field referencing back is the opposite | ||
| if (relationName !== undefined) { | ||
| return getRelationName(f) === relationName; | ||
| } | ||
| return true; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this condition is too loose here and it'll allow a false negative for the following case (field model Foo {
id Int @id
bars Bar[] @allow('update', true)
bar2 Bar @relation("other", fields: [bar2Id], references: [id])
bar2Id Int
}
model Bar {
id Int @id
foo Foo @relation(fields: [fooId], references: [id])
fooId Int
foos Foo[] @relation("other")
}
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * Returns if the given field is a computed field. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -38,6 +38,7 @@ import { | |
| isComputedField, | ||
| isDataFieldReference, | ||
| isDelegateModel, | ||
| isManyToManyField, | ||
| isNativeTypeMappingAttribute, | ||
| isRelationshipField, | ||
| mapBuiltinTypeToExpressionType, | ||
|
|
@@ -348,18 +349,26 @@ export default class AttributeApplicationValidator implements AstValidator<Attri | |
| }); | ||
| return; | ||
| } | ||
| this.validatePolicyKinds(kind, ['read', 'update', 'all'], attr, accept); | ||
| const kinds = this.validatePolicyKinds(kind, ['read', 'update', 'all'], attr, accept); | ||
|
|
||
| const expr = attr.args[1]?.value; | ||
| if (expr && AstUtils.streamAst(expr).some((node) => isBeforeInvocation(node))) { | ||
| accept('error', `"before()" is not allowed in field-level policies`, { node: expr }); | ||
| } | ||
|
|
||
| // relation fields are not allowed | ||
| // relation fields are not allowed, except for many-to-many fields which only support 'update' | ||
| const field = attr.$container as DataField; | ||
|
|
||
| if (isRelationshipField(field)) { | ||
| accept('error', `Field-level policies are not allowed for relation fields.`, { node: attr }); | ||
| if (isManyToManyField(field)) { | ||
| if (kinds.some((k) => k !== 'update')) { | ||
| accept('error', `Only 'update' policies are allowed on many-to-many relation fields`, { | ||
| node: attr, | ||
| }); | ||
| } | ||
| } else { | ||
| accept('error', `Field-level policies are not allowed for relation fields.`, { node: attr }); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shall we update this error to "Field-level policies are only allowed for implicit many-to-many relation fields"? |
||
| } | ||
| } | ||
|
|
||
| if (isComputedField(field)) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "ts-schema-generator.ts" file has a member
getRelationNamedoing the same thing but more robust. Shall we move that implementation here and avoid the duplicate?