Skip to content
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

fix(zmodel): clean up logic for detecting what attributes should be inherited from base models #1249

Merged
merged 1 commit into from
Apr 13, 2024
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
33 changes: 25 additions & 8 deletions packages/schema/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
BinaryExpr,
DataModel,
DataModelAttribute,
DataModelField,
Expression,
InheritableNode,
Expand Down Expand Up @@ -63,14 +64,7 @@ export function mergeBaseModel(model: Model, linker: Linker) {
.concat(dataModel.fields);

dataModel.attributes = bases
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
.flatMap((base) => base.attributes)
// don't inherit skip-level attributes
.filter((attr) => !attr.$inheritedFrom)
// don't inherit `@@delegate` attribute
.filter((attr) => attr.decl.$refText !== '@@delegate')
// don't inherit `@@map` attribute
.filter((attr) => attr.decl.$refText !== '@@map')
.flatMap((base) => base.attributes.filter((attr) => filterBaseAttribute(base, attr)))
.map((attr) => cloneAst(attr, dataModel, buildReference))
.concat(dataModel.attributes);

Expand All @@ -85,6 +79,29 @@ export function mergeBaseModel(model: Model, linker: Linker) {
model.declarations = model.declarations.filter((x) => !(isDataModel(x) && x.isAbstract));
}

function filterBaseAttribute(base: DataModel, attr: DataModelAttribute) {
if (attr.$inheritedFrom) {
// don't inherit from skip-level base
return false;
}

// uninheritable attributes for all inheritance
const uninheritableAttributes = ['@@delegate', '@@map'];

// uninheritable attributes for delegate inheritance (they reference fields from the base)
const uninheritableFromDelegateAttributes = ['@@unique', '@@index', '@@fulltext'];

if (uninheritableAttributes.includes(attr.decl.$refText)) {
return false;
}

if (isDelegateModel(base) && uninheritableFromDelegateAttributes.includes(attr.decl.$refText)) {
return false;
}

return true;
}

// deep clone an AST, relink references, and set its container
function cloneAst<T extends InheritableNode>(
node: T,
Expand Down
20 changes: 20 additions & 0 deletions packages/schema/tests/schema/abstract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,24 @@ describe('Abstract Schema Tests', () => {

`);
});

it('multiple id fields from base', async () => {
await loadModel(`
abstract model Base {
id1 String
id2 String
value String

@@id([id1, id2])
}

model Item1 extends Base {
x String
}

model Item2 extends Base {
y String
}
`);
});
});
56 changes: 34 additions & 22 deletions packages/sdk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,39 +176,51 @@ export function isDataModelFieldReference(node: AstNode): node is ReferenceExpr
}

/**
* Gets `@@id` fields declared at the data model level
* Gets `@@id` fields declared at the data model level (including search in base models)
*/
export function getModelIdFields(model: DataModel) {
const idAttr = model.attributes.find((attr) => attr.decl.$refText === '@@id');
if (!idAttr) {
return [];
}
const fieldsArg = idAttr.args.find((a) => a.$resolvedParam?.name === 'fields');
if (!fieldsArg || !isArrayExpr(fieldsArg.value)) {
return [];
const modelsToCheck = model.$baseMerged ? [model] : [model, ...getRecursiveBases(model)];

for (const modelToCheck of modelsToCheck) {
const idAttr = modelToCheck.attributes.find((attr) => attr.decl.$refText === '@@id');
if (!idAttr) {
continue;
}
const fieldsArg = idAttr.args.find((a) => a.$resolvedParam?.name === 'fields');
if (!fieldsArg || !isArrayExpr(fieldsArg.value)) {
continue;
}

return fieldsArg.value.items
.filter((item): item is ReferenceExpr => isReferenceExpr(item))
.map((item) => resolved(item.target) as DataModelField);
}

return fieldsArg.value.items
.filter((item): item is ReferenceExpr => isReferenceExpr(item))
.map((item) => resolved(item.target) as DataModelField);
return [];
}

/**
* Gets `@@unique` fields declared at the data model level
* Gets `@@unique` fields declared at the data model level (including search in base models)
*/
export function getModelUniqueFields(model: DataModel) {
const uniqueAttr = model.attributes.find((attr) => attr.decl.$refText === '@@unique');
if (!uniqueAttr) {
return [];
}
const fieldsArg = uniqueAttr.args.find((a) => a.$resolvedParam?.name === 'fields');
if (!fieldsArg || !isArrayExpr(fieldsArg.value)) {
return [];
const modelsToCheck = model.$baseMerged ? [model] : [model, ...getRecursiveBases(model)];

for (const modelToCheck of modelsToCheck) {
const uniqueAttr = modelToCheck.attributes.find((attr) => attr.decl.$refText === '@@unique');
if (!uniqueAttr) {
continue;
}
const fieldsArg = uniqueAttr.args.find((a) => a.$resolvedParam?.name === 'fields');
if (!fieldsArg || !isArrayExpr(fieldsArg.value)) {
continue;
}

return fieldsArg.value.items
.filter((item): item is ReferenceExpr => isReferenceExpr(item))
.map((item) => resolved(item.target) as DataModelField);
}

return fieldsArg.value.items
.filter((item): item is ReferenceExpr => isReferenceExpr(item))
.map((item) => resolved(item.target) as DataModelField);
return [];
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import { loadSchema } from '@zenstackhq/testtools';

describe('Regression for issue 1243', () => {
it('uninheritable fields', async () => {
const schema = `
model Base {
id String @id @default(cuid())
type String
foo String

@@delegate(type)
@@index([foo])
@@map('base')
@@unique([foo])
}

model Item1 extends Base {
x String
}

model Item2 extends Base {
y String
}
`;

await loadSchema(schema, {
enhancements: ['delegate'],
});
});

it('multiple id fields', async () => {
const schema = `
model Base {
id1 String
id2 String
type String

@@delegate(type)
@@id([id1, id2])
}

model Item1 extends Base {
x String
}

model Item2 extends Base {
y String
}
`;

await loadSchema(schema, {
enhancements: ['delegate'],
});
});
});
Loading