merge dev to main (v2.17.0) #2191
Conversation
Co-authored-by: Yiming <yiming@whimslab.io> Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
Co-authored-by: Tobias Brugger <tobias.brugger@bfh.ch> Co-authored-by: ymc9 <104139426+ymc9@users.noreply.github.com>
📝 WalkthroughWalkthroughThis update introduces customizable model name mapping for REST API URLs and OpenAPI generation, allowing external route segments to differ from internal model names. It adds new metadata attributes ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant RESTHandler
participant ModelMap
participant InternalLogic
Client->>RESTHandler: Request to /api/myUser/123
RESTHandler->>ModelMap: Map "myUser" to internal "user"
ModelMap-->>RESTHandler: "user"
RESTHandler->>InternalLogic: Process request for "user" id=123
InternalLogic-->>RESTHandler: Result
RESTHandler->>Client: Respond with data (using "myUser" in URLs)
sequenceDiagram
participant Schema
participant ModelMetaGen
Schema->>ModelMetaGen: Contains @meta and @@meta attributes
ModelMetaGen->>ModelMetaGen: Parse attributes with exprToValue
ModelMetaGen-->>Schema: Model metadata with meta fields
Possibly related PRs
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://app.smartcontext.ai/default/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (1)
packages/testtools/src/schema.ts (1)
251-381: Consider decomposing this large function for better maintainability.While the extraction is a good first step, this function handles many responsibilities. Consider:
- Extract file writing logic (lines 285-320) into a separate function like
writeSchemaFiles.- Add error handling for workspace root validation (lines 262-264) - currently throws a generic error.
- The
FILE_SPLITTERapproach seems like a workaround. Consider using a more structured format (JSON/YAML) for multi-file schemas.Would you like me to help refactor this into smaller, more focused functions?
🧹 Nitpick comments (12)
packages/schema/src/utils/is-container.ts (1)
1-23: LGTM! Well-implemented container detection with efficient caching.The implementation properly combines Docker and Podman detection with appropriate error handling and caching. The TODO comment about using
??=is reasonable - consider evaluating if the project can target Node.js 16+ to enable this cleaner syntax.tests/integration/tests/misc/meta-annotation.test.ts (3)
3-4: Consider more descriptive test naming.The test suite name mentions
@@metabut the test also covers@metaattributes. Consider renaming to be more inclusive.-describe('`@@meta` attribute tests', () => { - it('generates generated into model-meta', async () => { +describe('Meta attribute tests', () => { + it('correctly parses @meta and @@meta attributes into model metadata', async () => {
17-18: Consider improving type safety.The use of
anytype weakens type safety. Consider defining a proper interface for attribute objects.+interface AttributeObject { + name: string; + args: Array<{ name: string; value: any }>; +} + - const userModelMeta = model.attributes.filter((attr: any) => attr.name === '@@meta'); + const userModelMeta = model.attributes.filter((attr: AttributeObject) => attr.name === '@@meta');
1-59: Consider adding edge case and error handling tests.While the current test covers the happy path well, consider adding additional test cases for:
- Invalid meta attribute syntax
- Empty meta attributes
- Meta attributes with no arguments
- Very deeply nested objects
- Special characters in meta keys/values
This would help ensure robustness of the meta annotation parsing.
tests/regression/tests/issue-2175.test.ts (2)
11-11: Remove trailing whitespace.There are trailing spaces after the closing brace.
- } + }
26-49: Verify error handling in test code.The
main.tscontent executes async operations without error handling. While this is test code, proper error handling would make debugging easier if tests fail.Add try-catch blocks to handle potential errors:
async function main() { - const newUser = await dbExtended.user.signUp("a@b.com"); - console.log(newUser); + try { + const newUser = await dbExtended.user.signUp("a@b.com"); + console.log(newUser); + } catch (error) { + console.error('Test failed:', error); + process.exit(1); + } }Also applies to: 90-113
tests/integration/tests/misc/prisma-client-generator.test.ts (1)
75-76: Consider extracting conditional imports to helper functions.The conditional imports with string concatenation make the code harder to read.
Consider creating helper functions:
function generateModelImport(config: typeof config, modelName: string): string { return config.isNewGenerator ? `import type { ${modelName}Model } from '${config.modelsLoadPath}';` : ''; } function generateModelUsage(config: typeof config, modelName: string): string { return config.isNewGenerator ? `const ${modelName.toLowerCase()}1: ${modelName}Model = { id: 1, name: 'Alice', role: 'USER' };\nconsole.log(${modelName.toLowerCase()}1);` : ''; }Also applies to: 81-82, 168-169, 174-175
packages/server/src/api/rest/index.ts (2)
275-277: Consider caching URL pattern matches for performance.The
mapModelNameandmatchUrlPatternmethods are called on every request. For high-traffic APIs, this could impact performance.Consider implementing a simple cache:
private urlPatternCache = new Map<string, Match | null>(); private matchUrlPattern(path: string, routeType: UrlPatterns): Match | undefined { const cacheKey = `${path}:${routeType}`; if (this.urlPatternCache.has(cacheKey)) { const cached = this.urlPatternCache.get(cacheKey); return cached ?? undefined; } // ... existing matching logic ... this.urlPatternCache.set(cacheKey, match ?? null); return match; }Also applies to: 290-301
290-294: Consider providing a more helpful error message.The error message could be more user-friendly by suggesting the correct mapped name.
-throw new InvalidValueError( - `use the mapped model name: ${this.modelNameMapping[match.type]} and not ${match.type}` -); +throw new InvalidValueError( + `Invalid URL: Use "${this.modelNameMapping[match.type]}" instead of "${match.type}" in the URL path` +);packages/testtools/src/schema.ts (1)
173-249: Well-structured refactoring that improves separation of concerns.The extraction of compilation logic into
createProjectAndCompilemakes the code more modular and easier to test. Consider these enhancements:
- Extract Prisma client instantiation (lines 183-197) into a separate helper function for better testability.
- Add try-catch blocks around module loading operations (lines 182, 217-228) to provide more informative error messages if modules fail to load.
packages/schema/src/plugins/enhancer/enhance/index.ts (2)
117-172: Well-structured implementation for the new Prisma client generator.The method correctly handles the new generator's modular structure with separate export files. Good practice to generate both node and edge versions.
Consider extracting the export file creation logic (lines 132-159) into a helper method for better readability:
+private createExportFiles(resultPrismaBaseImport: string) { + // models.ts + const modelsTsContent = [ + `export * from '${resultPrismaBaseImport}/models';`, + `export * from './json-types';`, + ].join('\n'); + // ... rest of the export file creation logic +}
569-631: Improved file transformation approach with better performance.The approach of collecting statements before creating new files is cleaner than in-place editing.
Consider adding error handling for the rename operations:
for (const d of this.model.declarations.filter(isDataModel)) { const fixedFileName = `${prismaClientDir}/models/${d.name}-fixed.ts`; const fileName = `${prismaClientDir}/models/${d.name}.ts`; - - fs.renameSync(fixedFileName, fileName); + + try { + fs.renameSync(fixedFileName, fileName); + } catch (error) { + // Clean up any remaining fixed files + if (fs.existsSync(fixedFileName)) { + fs.unlinkSync(fixedFileName); + } + throw error; + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (20)
package.jsonis excluded by!**/*.jsonpackages/ide/jetbrains/package.jsonis excluded by!**/*.jsonpackages/language/package.jsonis excluded by!**/*.jsonpackages/misc/redwood/package.jsonis excluded by!**/*.jsonpackages/plugins/openapi/package.jsonis excluded by!**/*.jsonpackages/plugins/swr/package.jsonis excluded by!**/*.jsonpackages/plugins/tanstack-query/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v10/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/nuxt-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/plugins/trpc/tests/projects/t3-trpc-v11/package.jsonis excluded by!**/*.jsonpackages/runtime/package.jsonis excluded by!**/*.jsonpackages/schema/package.jsonis excluded by!**/*.jsonpackages/sdk/package.jsonis excluded by!**/*.jsonpackages/server/package.jsonis excluded by!**/*.jsonpackages/testtools/package.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yamltests/integration/test-run/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/nextjs/test-project/package.jsonis excluded by!**/*.jsontests/integration/tests/frameworks/trpc/test-project/package.jsonis excluded by!**/*.json
📒 Files selected for processing (18)
packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/plugins/openapi/src/rest-generator.ts(5 hunks)packages/schema/src/plugins/enhancer/enhance/index.ts(12 hunks)packages/schema/src/res/stdlib.zmodel(2 hunks)packages/schema/src/telemetry.ts(3 hunks)packages/schema/src/utils/is-ci.ts(1 hunks)packages/schema/src/utils/is-container.ts(1 hunks)packages/sdk/src/model-meta-generator.ts(4 hunks)packages/server/src/api/rest/index.ts(18 hunks)packages/server/tests/api/rest.test.ts(1 hunks)packages/testtools/src/schema.ts(3 hunks)script/test-scaffold.ts(1 hunks)tests/integration/tests/cli/generate.test.ts(1 hunks)tests/integration/tests/cli/plugins.test.ts(2 hunks)tests/integration/tests/enhancements/json/typing.test.ts(1 hunks)tests/integration/tests/misc/meta-annotation.test.ts(1 hunks)tests/integration/tests/misc/prisma-client-generator.test.ts(5 hunks)tests/regression/tests/issue-2175.test.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
packages/plugins/openapi/src/rest-generator.ts (1)
Learnt from: ymc9
PR: zenstackhq/zenstack#2187
File: packages/server/src/api/rest/index.ts:287-289
Timestamp: 2025-07-16T03:09:04.699Z
Learning: In REST API URL pattern matching with model name mapping, only the `type` parameter (representing model names) should be mapped using `modelNameMapping`. The `relationship` parameter represents field names on models and should not be mapped, as field names are distinct from model names in the API structure.
packages/server/src/api/rest/index.ts (1)
Learnt from: ymc9
PR: zenstackhq/zenstack#2187
File: packages/server/src/api/rest/index.ts:287-289
Timestamp: 2025-07-16T03:09:04.699Z
Learning: In REST API URL pattern matching with model name mapping, only the `type` parameter (representing model names) should be mapped using `modelNameMapping`. The `relationship` parameter represents field names on models and should not be mapped, as field names are distinct from model names in the API structure.
packages/server/tests/api/rest.test.ts (1)
<retrieved_learning>
Learnt from: ymc9
PR: #2187
File: packages/server/src/api/rest/index.ts:287-289
Timestamp: 2025-07-16T03:09:04.699Z
Learning: In REST API URL pattern matching with model name mapping, only the type parameter (representing model names) should be mapped using modelNameMapping. The relationship parameter represents field names on models and should not be mapped, as field names are distinct from model names in the API structure.
</retrieved_learning>
🧬 Code Graph Analysis (7)
script/test-scaffold.ts (1)
packages/testtools/src/schema.ts (1)
run(45-67)
tests/integration/tests/cli/generate.test.ts (1)
packages/testtools/src/schema.ts (1)
installPackage(69-71)
packages/schema/src/telemetry.ts (2)
packages/schema/src/utils/is-container.ts (1)
isInContainer(16-23)packages/schema/src/utils/is-ci.ts (1)
isInCi(2-8)
tests/integration/tests/enhancements/json/typing.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(173-249)
tests/regression/tests/issue-2175.test.ts (1)
packages/testtools/src/schema.ts (1)
loadSchema(173-249)
tests/integration/tests/misc/prisma-client-generator.test.ts (1)
packages/testtools/src/schema.ts (2)
createProjectAndCompile(251-381)loadSchema(173-249)
packages/testtools/src/schema.ts (3)
packages/runtime/src/enhancements/node/types.ts (1)
PolicyDef(106-115)packages/runtime/src/constants.ts (1)
DEFAULT_RUNTIME_LOAD_PATH(4-4)packages/runtime/src/types.ts (2)
AuthUser(50-50)EnhancementOptions(108-148)
⏰ 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). (6)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (23)
script/test-scaffold.ts (1)
22-22: LGTM! Prisma version update is consistent.The dependency version bump from 6.10.x to 6.11.x aligns with similar updates across other test files, ensuring consistent Prisma package versions in test environments.
tests/integration/tests/cli/plugins.test.ts (1)
78-78: LGTM! Consistent Prisma version updates in test dependencies.The version updates from 6.10.x to 6.11.x for both
@prisma/clientandprismapackages maintain consistency with other test files and ensure the integration tests use aligned dependency versions.Also applies to: 88-88
tests/integration/tests/cli/generate.test.ts (1)
48-48: LGTM! Good practice to pin zod to major version 3.Explicitly specifying
zod@3ensures test reproducibility and prevents potential breaking changes from newer major versions. This aligns with the broader dependency management improvements in the test suite.packages/schema/src/utils/is-ci.ts (1)
1-8: LGTM! Robust CI environment detection.The implementation correctly handles common CI environment variables and properly treats string values '0' and 'false' as falsy. The logic covers standard CI platforms and uses efficient short-circuit evaluation.
packages/schema/src/telemetry.ts (3)
14-15: LGTM: New utility imports are correctly added.The imports for container and CI detection utilities are properly structured and align with the enhanced telemetry functionality.
49-50: LGTM: Environment detection properties are well-implemented.The properties are appropriately declared as private readonly and correctly initialized using the utility functions. This ensures immutable environment state detection.
115-116: LGTM: Telemetry payload correctly extended with environment context.The new container and CI detection properties are appropriately added to the telemetry payload, maintaining consistency with the existing environment detection pattern.
packages/schema/src/res/stdlib.zmodel (2)
508-509: LGTM: JSON type modifier expansion enhances flexibility.Expanding the target fields to include
TypeDefFieldallows@db.Jsonand@db.JsonBattributes to be used more broadly, which aligns with the enhanced JSON field typing support demonstrated in the integration tests.
753-761: LGTM: New metadata attributes provide valuable extensibility.The
@@metaand@metaattributes are well-designed for attaching arbitrary metadata at model/type-def and field levels respectively. The parameter types (String name, Any value) provide appropriate flexibility for metadata storage.tests/integration/tests/enhancements/json/typing.test.ts (1)
389-420: LGTM: Comprehensive test for enhanced JSON attribute support.This test effectively validates the expanded
@db.Jsonand@db.JsonBattribute support forTypeDefFieldtargets. The PostgreSQL-specific test is appropriate since it supports both JSON types, and the schema compilation focus ensures the attribute expansion works correctly.packages/sdk/src/model-meta-generator.ts (3)
6-6: LGTM: New imports support enhanced expression parsing.The added imports for
Expression,ObjectExpr,isObjectExpr, andts-patternappropriately support the refactored expression-to-value conversion logic.Also applies to: 13-13, 17-17, 26-26
369-372: LGTM: Attribute argument processing is well-refactored.The simplification to use
exprToValuemakes the code more maintainable and consistent. The centralized expression parsing approach is cleaner than the previous inline type checking.
599-620: LGTM: Helper functions provide robust expression-to-value conversion.The
exprToValueandexprToObjectfunctions are well-implemented using pattern matching. The recursive handling of arrays and objects, along with the fallback number parsing (parseInt → parseFloat), creates a robust and extensible expression evaluation system.packages/ide/jetbrains/build.gradle.kts (1)
12-12: LGTM: Version bump for v2.17.0 release.The version update from "2.16.1" to "2.17.0" aligns with the PR objectives for the v2.17.0 release merge.
tests/integration/tests/misc/meta-annotation.test.ts (3)
5-15: Well-structured test schema with comprehensive coverage.The test schema effectively covers multiple scenarios:
- Multiple
@metaattributes on a single field- Different value types (string, boolean, number)
- Model-level
@@metaattributes with both positional and named argument syntax- Complex nested object values
This provides good coverage for the meta annotation functionality.
19-36: Excellent assertion structure for model-level meta attributes.The test properly validates:
- Multiple
@@metaattributes are collected- Named argument syntax (
name: 'doc', value: '...')- Positional argument syntax (
'info', { ... })- Complex nested object preservation
The use of
expect.arrayContainingallows for flexible ordering while ensuring all expected attributes are present.
38-57: Comprehensive field-level meta attribute validation.The test correctly verifies that multiple
@metaattributes on the same field are properly collected and that different value types (boolean, number) are preserved accurately.packages/server/tests/api/rest.test.ts (1)
3017-3113: Excellent test coverage for model name mapping feature!This new test suite comprehensively validates the model name mapping functionality. The implementation correctly tests:
- Rejection of original model names when mapping is configured (returns 400)
- Acceptance of mapped model names for API operations
- URL generation using mapped names in response links
- Normal operation of unmapped models like "post"
- Relationship handling between mapped and unmapped models
The test design aligns perfectly with the retrieved learning that only the
typeparameter should be mapped while relationship parameters use internal model names. The assertions properly verify both functional behavior and URL structure consistency.tests/integration/tests/misc/prisma-client-generator.test.ts (1)
279-284: Test should verify auth() default behavior.The test creates a post but doesn't verify that the
authorIdis correctly set fromauth().id.Add assertions to verify the auth default:
const user = await prisma.user.create({ data: { id: 1 } }); const db = enhance({ id: user.id }); await expect(db.post.create({ data: { id: 1, title: 'Hello World' } })).resolves.toMatchObject({ + id: 1, + title: 'Hello World', authorId: user.id, });Likely an incorrect or invalid review comment.
packages/server/src/api/rest/index.ts (1)
586-587: LGTM! Consistent application of model name mapping.The model name mapping is consistently applied across all URL constructions for resources, relationships, and pagination links.
Also applies to: 638-639, 737-738, 1067-1068, 1218-1219, 1271-1272, 1288-1289
packages/schema/src/plugins/enhancer/enhance/index.ts (3)
107-115: Clean separation of generator logic by version.Excellent use of the delegation pattern to handle different Prisma client generator versions. This makes the code more maintainable and easier to extend for future generator versions.
314-328: Correct handling of runtime type imports for different generators.The conditional import logic properly addresses the structural differences between the new and old Prisma client generators.
1089-1091: Good improvement to handle trailing commas.The regex update to remove leading commas on empty lines helps clean up the generated code when properties are removed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/plugins/openapi/tests/openapi-restful.test.ts (1)
400-431: Well-implemented test for model name mapping feature.The test properly validates the core functionality:
- Verifies mapped model names appear in generated paths (
/api/myUser)- Confirms original model names are excluded (
/api/userreturns falsy)- Ensures unmapped models still work correctly (
/api/post)Consider enhancing the test coverage with additional assertions:
const api = await OpenAPIParser.validate(output); expect(api.paths?.['/api/myUser']).toBeTruthy(); expect(api.paths?.['/api/user']).toBeFalsy(); expect(api.paths?.['/api/post']).toBeTruthy(); + + // Verify operation IDs also use mapped names + expect(api.paths?.['/api/myUser']?.['get']?.operationId).toContain('myUser'); + expect(api.paths?.['/api/myUser/{id}']?.['get']?.operationId).toContain('myUser'); + + // Verify relationship paths use mapped names + expect(api.paths?.['/api/myUser/{id}/posts']).toBeTruthy(); + expect(api.paths?.['/api/post/{id}/relationships/author']).toBeTruthy();This would ensure the mapping is consistently applied across all generated OpenAPI elements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/plugins/openapi/tests/openapi-restful.test.ts(2 hunks)packages/server/tests/api/rest.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/server/tests/api/rest.test.ts
⏰ 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). (6)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: OSSAR-Scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/plugins/openapi/tests/openapi-restful.test.ts (1)
132-132: LGTM! Test name improvement enhances clarity.The rename from 'options' to 'common options' makes the test purpose more descriptive and helps distinguish it from other option-related tests.
No description provided.