Skip to content

perf: improve polymorphism code generation speed #1073

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 4, 2024
Merged

perf: improve polymorphism code generation speed #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
64a52dc
perf: improve polymorphism code generation speed
ymc9 Mar 4, 2024
03957f4
fix tests
ymc9 Mar 4, 2024
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
4 changes: 2 additions & 2 deletions packages/schema/src/plugins/enhancer/delegate/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { PrismaSchemaGenerator } from '../../prisma/schema-generator';
import path from 'path';

export async function generate(model: Model, options: PluginOptions, project: Project, outDir: string) {
const prismaGenerator = new PrismaSchemaGenerator();
await prismaGenerator.generate(model, {
const prismaGenerator = new PrismaSchemaGenerator(model);
await prismaGenerator.generate({
provider: '@internal',
schemaPath: options.schemaPath,
output: path.join(outDir, 'delegate.prisma'),
Comment on lines 5 to 12
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [12-13]

The use of path.join with options.schemaPath and outDir could potentially lead to path traversal vulnerabilities if user input is not properly sanitized or validated. Ensure that any user-supplied paths are rigorously checked to prevent unauthorized file system access.

- output: path.join(outDir, 'delegate.prisma'),
- overrideClientGenerationPath: path.join(outDir, '.delegate'),
+ output: sanitizePath(path.join(outDir, 'delegate.prisma')),
+ overrideClientGenerationPath: sanitizePath(path.join(outDir, '.delegate')),

Consider implementing a sanitizePath function that checks for and mitigates any path traversal patterns.

Expand Down
370 changes: 238 additions & 132 deletions packages/schema/src/plugins/enhancer/enhance/index.ts
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/schema/src/plugins/prisma/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ export const name = 'Prisma';
export const description = 'Generating Prisma schema';

const run: PluginFunction = async (model, options, _dmmf, _globalOptions) => {
return new PrismaSchemaGenerator().generate(model, options);
return new PrismaSchemaGenerator(model).generate(options);
};

export default run;
45 changes: 35 additions & 10 deletions packages/schema/src/plugins/prisma/schema-generator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
InvocationExpr,
isArrayExpr,
isDataModel,
isDataSource,
isInvocationExpr,
isLiteralExpr,
isNullExpr,
Comment on lines 17 to 23
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [707-707]

Potential security issue detected: user input is used in path.resolve within the getDefaultPrismaOutputFile function, which could lead to a path traversal vulnerability. Ensure to sanitize or validate user input before using it in path resolution functions.

- return path.resolve(path.dirname(pkgJsonPath), pkgJson.zenstack.prisma);
+ // Ensure pkgJson.zenstack.prisma is sanitized or validated before use
+ return path.resolve(path.dirname(pkgJsonPath), sanitizedInput);

Expand Down Expand Up @@ -79,6 +80,7 @@ import {

const MODEL_PASSTHROUGH_ATTR = '@@prisma.passthrough';
const FIELD_PASSTHROUGH_ATTR = '@prisma.passthrough';
const PROVIDERS_SUPPORTING_NAMED_CONSTRAINTS = ['postgresql', 'mysql', 'cockroachdb'];

/**
* Generates Prisma schema file
Expand All @@ -95,7 +97,9 @@ export class PrismaSchemaGenerator {

private mode: 'logical' | 'physical' = 'physical';

async generate(model: Model, options: PluginOptions) {
constructor(private readonly zmodel: Model) {}

async generate(options: PluginOptions) {
const warnings: string[] = [];
if (options.mode) {
this.mode = options.mode as 'logical' | 'physical';
Expand All @@ -110,7 +114,7 @@ export class PrismaSchemaGenerator {

const prisma = new PrismaModel();

for (const decl of model.declarations) {
for (const decl of this.zmodel.declarations) {
switch (decl.$type) {
case DataSource:
this.generateDataSource(prisma, decl as DataSource);
Expand Down Expand Up @@ -151,7 +155,7 @@ export class PrismaSchemaGenerator {
const generateClient = options.generateClient !== false;

if (generateClient) {
let generateCmd = `prisma generate --schema "${outFile}"`;
let generateCmd = `prisma generate --schema "${outFile}"${this.mode === 'logical' ? ' --no-engine' : ''}`;
if (typeof options.generateArgs === 'string') {
generateCmd += ` ${options.generateArgs}`;
}
Expand Down Expand Up @@ -452,17 +456,23 @@ export class PrismaSchemaGenerator {
new AttributeArgValue('FieldReference', new PrismaFieldReference(idField.name))
)
);
relationField.attributes.push(
new PrismaFieldAttribute('@relation', [
new PrismaAttributeArg('fields', args),
new PrismaAttributeArg('references', args),

const addedRel = new PrismaFieldAttribute('@relation', [
new PrismaAttributeArg('fields', args),
new PrismaAttributeArg('references', args),
]);

if (this.supportNamedConstraints) {
addedRel.args.push(
// generate a `map` argument for foreign key constraint disambiguation
new PrismaAttributeArg(
'map',
new PrismaAttributeArgValue('String', `${relationField.name}_fk`)
),
])
);
)
);
}

relationField.attributes.push(addedRel);
} else {
relationField.attributes.push(this.makeFieldAttribute(relAttr as DataModelFieldAttribute));
}
Expand All @@ -471,6 +481,21 @@ export class PrismaSchemaGenerator {
});
}

private get supportNamedConstraints() {
const ds = this.zmodel.declarations.find(isDataSource);
if (!ds) {
return false;
}

const provider = ds.fields.find((f) => f.name === 'provider');
if (!provider) {
return false;
}

const value = getStringLiteral(provider.value);
return value && PROVIDERS_SUPPORTING_NAMED_CONSTRAINTS.includes(value);
}

private isPrismaAttribute(attr: DataModelAttribute | DataModelFieldAttribute) {
if (!attr.decl.ref) {
return false;
Expand Down
26 changes: 13 additions & 13 deletions packages/schema/tests/generator/prisma-generator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('Prisma generator test', () => {
}
`);

await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -128,7 +128,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -162,7 +162,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -194,7 +194,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -230,7 +230,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -270,7 +270,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -321,7 +321,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -357,7 +357,7 @@ describe('Prisma generator test', () => {
}
`);
const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand All @@ -380,7 +380,7 @@ describe('Prisma generator test', () => {
const model = await loadDocument(path.join(__dirname, './zmodel/schema.zmodel'));

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -430,7 +430,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -461,7 +461,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down Expand Up @@ -496,7 +496,7 @@ describe('Prisma generator test', () => {
`);

const { name } = tmp.fileSync({ postfix: '.prisma' });
await new PrismaSchemaGenerator().generate(model, {
await new PrismaSchemaGenerator(model).generate({
name: 'Prisma',
provider: '@core/prisma',
schemaPath: 'schema.zmodel',
Expand Down
4 changes: 2 additions & 2 deletions ...cements/with-delegate/regressions.test.ts → ...ncements/with-delegate/issue-1058.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { loadSchema } from '@zenstackhq/testtools';

describe('Regression tests', () => {
it('FK Constraint Ambiguity', async () => {
describe('Regression for issue 1058', () => {
it('test', async () => {
const schema = `
model User {
id String @id @default(cuid())
Expand Down
Loading