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(delegate): delegate model shouldn't inherit @@index from an indirect abstract base #1818

Merged
merged 1 commit into from
Oct 30, 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
27 changes: 23 additions & 4 deletions packages/schema/src/utils/ast-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,13 @@ import {
ModelImport,
ReferenceExpr,
} from '@zenstackhq/language/ast';
import { getModelFieldsWithBases, getRecursiveBases, isDelegateModel, isFromStdlib } from '@zenstackhq/sdk';
import {
getInheritanceChain,
getModelFieldsWithBases,
getRecursiveBases,
isDelegateModel,
isFromStdlib,
} from '@zenstackhq/sdk';
import {
AstNode,
copyAstNode,
Expand Down Expand Up @@ -61,7 +67,7 @@ export function mergeBaseModels(model: Model, linker: Linker) {
.concat(dataModel.fields);

dataModel.attributes = bases
.flatMap((base) => base.attributes.filter((attr) => filterBaseAttribute(base, attr)))
.flatMap((base) => base.attributes.filter((attr) => filterBaseAttribute(dataModel, base, attr)))
.map((attr) => cloneAst(attr, dataModel, buildReference))
.concat(dataModel.attributes);
}
Expand All @@ -85,7 +91,7 @@ export function mergeBaseModels(model: Model, linker: Linker) {
linkContentToContainer(model);
}

function filterBaseAttribute(base: DataModel, attr: DataModelAttribute) {
function filterBaseAttribute(forModel: DataModel, base: DataModel, attr: DataModelAttribute) {
if (attr.$inheritedFrom) {
// don't inherit from skip-level base
return false;
Expand All @@ -101,13 +107,26 @@ function filterBaseAttribute(base: DataModel, attr: DataModelAttribute) {
return false;
}

if (isDelegateModel(base) && uninheritableFromDelegateAttributes.includes(attr.decl.$refText)) {
if (
// checks if the inheritance is from a delegate model or through one, if so,
// the attribute shouldn't be inherited as the delegate already inherits it
isInheritedFromOrThroughDelegate(forModel, base) &&
uninheritableFromDelegateAttributes.includes(attr.decl.$refText)
) {
return false;
}

return true;
}

function isInheritedFromOrThroughDelegate(model: DataModel, base: DataModel) {
if (isDelegateModel(base)) {
return true;
}
const chain = getInheritanceChain(model, base);
return !!chain?.some(isDelegateModel);
}

// deep clone an AST, relink references, and set its container
function cloneAst<T extends InheritableNode>(
node: T,
Expand Down
21 changes: 21 additions & 0 deletions packages/sdk/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -569,3 +569,24 @@ export function getInheritedFromDelegate(field: DataModelField) {
const foundBase = bases.findLast((base) => base.fields.some((f) => f.name === field.name) && isDelegateModel(base));
return foundBase;
}

/**
* Gets the inheritance chain from "from" to "to", excluding them.
*/
export function getInheritanceChain(from: DataModel, to: DataModel): DataModel[] | undefined {
if (from === to) {
return [];
}

for (const base of from.superTypes) {
if (base.ref === to) {
return [];
}
const path = getInheritanceChain(base.ref!, to);
if (path) {
return [base.ref as DataModel, ...path];
}
}

return undefined;
}
48 changes: 48 additions & 0 deletions tests/regression/tests/issue-1786.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { loadSchema } from '@zenstackhq/testtools';

describe('issue 1786', () => {
it('regression', async () => {
await loadSchema(
`
model User {
id String @id @default(cuid())
email String @unique @email @length(6, 32)
password String @password @omit
contents Content[]

// everybody can signup
@@allow('create', true)

// full access by self
@@allow('all', auth() == this)
}

abstract model BaseContent {
published Boolean @default(false)

@@index([published])
}

model Content extends BaseContent {
id String @id @default(cuid())
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt
owner User @relation(fields: [ownerId], references: [id])
ownerId String
contentType String

@@delegate(contentType)
}

model Post extends Content {
title String
}

model Video extends Content {
name String
duration Int
}
`
);
});
});
Loading