-
-
Notifications
You must be signed in to change notification settings - Fork 12
fix: stricter default schema validation and proper transformation to Prisma schema #390
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
Conversation
WalkthroughThe pull request implements PostgreSQL custom schema support by adding validation for defaultSchema fields in datasources, filtering the field from Prisma output, auto-generating @@Schema attributes when needed, and introducing helper utilities for schema field access. New end-to-end tests verify validation and SQL generation behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job Code Review CompleteReviewing PR #390: "fix: stricter default schema validation and proper transformation to Prisma schema" Todo List:
Overall Assessment: ✅ Strong ImplementationThis PR implements stricter validation for PostgreSQL schema configurations and ensures proper transformation to Prisma schema. The changes are well-structured and address important validation gaps. Key Improvements🔧 Enhanced Schema Validation (
|
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.
Pull Request Overview
This PR enhances PostgreSQL schema validation and ensures proper transformation from ZModel to Prisma schema by introducing stricter validation rules and automatic schema attribute generation.
- Adds validation to require
defaultSchemato be included in theschemasarray when both are specified - Filters out ZModel-specific
defaultSchemafield when generating Prisma datasource configuration - Automatically adds
@@schemaattributes to models whenschemasis defined but no explicit schema is specified
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/e2e/orm/client-api/pg-custom-schema.test.ts | Adds tests to verify validation of defaultSchema being included in schemas array and proper schema assignment to models without explicit @@schema attributes |
| packages/sdk/src/prisma/prisma-schema-generator.ts | Filters out defaultSchema from datasource fields, adds automatic @@schema attribute generation for models when schemas is defined, and refactors to use centralized getStringLiteral utility |
| packages/language/src/validators/datasource-validator.ts | Adds validation to ensure defaultSchema is a string literal and is included in the schemas array when both are specified |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/language/src/validators/datasource-validator.ts (1)
42-56: Validation logic looks good.The defaultSchema validation correctly enforces:
- Provider must be postgresql
- Value must be a string literal
Consider adding validation to reject empty string values for better error messages:
defaultSchemaValue = getStringLiteral(defaultSchemaField.value); if (!defaultSchemaValue) { accept('error', '"defaultSchema" must be a string literal', { node: defaultSchemaField.value, }); +} else if (defaultSchemaValue.trim() === '') { + accept('error', '"defaultSchema" cannot be an empty string', { + node: defaultSchemaField.value, + }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/language/src/validators/datasource-validator.ts(2 hunks)packages/sdk/src/prisma/prisma-schema-generator.ts(7 hunks)tests/e2e/orm/client-api/pg-custom-schema.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
tests/e2e/orm/client-api/pg-custom-schema.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(53-182)
packages/language/src/validators/datasource-validator.ts (1)
packages/language/src/utils.ts (1)
getStringLiteral(75-77)
packages/sdk/src/prisma/prisma-schema-generator.ts (3)
packages/sdk/src/prisma/prisma-builder.ts (2)
SimpleField(6-6)Model(115-164)packages/language/src/utils.ts (2)
getAllAttributes(546-570)getStringLiteral(75-77)packages/language/src/generated/ast.ts (3)
Model(559-563)Model(565-565)isDataSource(417-419)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (8)
packages/language/src/validators/datasource-validator.ts (1)
73-81: Excellent cross-field validation.The logic correctly ensures that when both
defaultSchemaandschemasare present, the default schema value is included in the schemas array. Reporting the error on theschemasFieldnode is appropriate since the fix requires updating the schemas array.tests/e2e/orm/client-api/pg-custom-schema.test.ts (2)
197-214: Comprehensive validation test.This test effectively verifies that the cross-field validation between
defaultSchemaandschemasworks correctly, ensuring the default schema is included in the schemas array.
216-260: Excellent integration test.This test validates the key functionality of the PR:
- Models with explicit
@@schemause that schema- Models without
@@schemause thedefaultSchemavalue- SQL generation correctly targets the appropriate schemas
The use of log inspection to verify SQL targeting is a robust approach.
packages/sdk/src/prisma/prisma-schema-generator.ts (5)
67-68: Clean abstraction for non-Prisma fields.Defining
NON_PRISMA_DATASOURCE_FIELDSas a constant provides a clear, maintainable way to identify fields that exist in ZModel but not in Prisma schema.
113-118: Correct filtering implementation.The datasource field filtering correctly excludes non-Prisma fields while preserving all standard Prisma datasource configuration.
216-228: Well-designed helper utilities.These helper methods provide clean abstractions for:
- Accessing datasource fields
- Checking for schemas configuration
- Determining the default PostgreSQL schema name with appropriate fallback
The use of the imported
getStringLiteralutility is the right choice.
185-187: Improved attribute handling.Computing
allAttributesonce and reusing it is more efficient and enables the subsequent @@Schema existence check.
193-204: Correct auto-generation logic.The conditional auto-generation of
@@schemaattributes is well-designed:
- Only activates when the datasource declares
schemas- Respects explicit
@@schemaattributes on models- Uses the appropriate default value from
defaultSchemaor falls back to 'public'This implementation aligns perfectly with the test case "allows specifying schema only on a few models".
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes