merge dev to main (v2.11.0) #1943
Conversation
|
Warning Rate limit exceeded@ymc9 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 23 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request introduces several enhancements and improvements across the ZenStack project, primarily focusing on adding new features like encryption support, improving policy and inheritance validation, and updating various utility functions. The changes span multiple packages, including runtime, schema, SDK, and server components. A version update to 2.11.0 is also included, reflecting the significance of the introduced features. Changes
Sequence DiagramsequenceDiagram
participant Client
participant PrismaClient
participant EncryptionHandler
participant Database
Client->>PrismaClient: Create/Read/Update Record
PrismaClient->>EncryptionHandler: Preprocess Payload
alt Write Operation
EncryptionHandler->>EncryptionHandler: Encrypt Sensitive Fields
end
PrismaClient->>Database: Execute Operation
alt Read Operation
Database->>PrismaClient: Return Encrypted Data
PrismaClient->>EncryptionHandler: Decrypt Sensitive Fields
end
EncryptionHandler->>PrismaClient: Return Decrypted Data
PrismaClient->>Client: Return Result
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
packages/runtime/src/enhancements/edge/encryption.ts (1)
Line range hint
162-162: Replacebtoaandatobwith environment-compatible methodsThe functions
btoaandatobare not universally available in all JavaScript environments, such as Node.js. Since this code may run in environments where these functions are undefined, consider replacing them withBuffermethods for broader compatibility.Apply these changes to ensure compatibility:
In the
encryptmethod:-return `${btoa(JSON.stringify(meta))}.${btoa(String.fromCharCode(...cipherBytes))}`; +const metaBase64 = Buffer.from(JSON.stringify(meta), 'utf-8').toString('base64'); +const cipherBase64 = Buffer.from(cipherBytes).toString('base64'); +return `${metaBase64}.${cipherBase64}`;In the
decryptmethod:-metaObj = JSON.parse(atob(metaText)); +metaObj = JSON.parse(Buffer.from(metaText, 'base64').toString('utf-8'));-const bytes = Uint8Array.from(atob(cipherText), (c) => c.charCodeAt(0)); +const bytes = new Uint8Array(Buffer.from(cipherText, 'base64'));Also applies to: 177-177, 192-192
🧹 Nitpick comments (21)
packages/schema/src/language-server/validator/datamodel-validator.ts (2)
415-433: LGTM! Consider adding null safety checks.The circular inheritance detection is well-implemented using a breadth-first traversal. However, consider adding null safety checks:
- const current = todo.shift()!; + const current = todo.shift(); + if (!current) break;- const todo: DataModel[] = dm.superTypes.map((superType) => superType.ref!); + const todo: DataModel[] = dm.superTypes + .map((superType) => superType.ref) + .filter((ref): ref is DataModel => ref !== undefined);
415-415: Add JSDoc documentation for the new method.Please add JSDoc documentation to describe the method's purpose, parameters, and behavior.
Example:
/** * Validates that there are no circular inheritance relationships in the data model. * @param dm The data model to validate * @param accept The validation acceptor for reporting errors */ private validateInheritance(dm: DataModel, accept: ValidationAcceptor) {packages/schema/src/utils/version-utils.ts (1)
Line range hint
2-12: Consider making package.json path resolution more robust.The current implementation uses relative paths which could break if the file structure changes. Consider:
- Using
require.resolveorimport.meta.resolvefor more reliable path resolution- Adding error logging when version lookup fails to help with debugging
Example implementation:
export function getVersion(): string | undefined { + const logger = console; try { - return require('../package.json').version; + return require(require.resolve('../package.json')).version; } catch { try { // dev environment - return require('../../package.json').version; + return require(require.resolve('../../package.json')).version; } catch (err) { + logger.debug('Failed to load version from package.json:', err); return undefined; } } }packages/server/src/api/rest/index.ts (1)
1106-1107: Architectural change in DELETE response structureThe change from 204 (No Content) to 200 (OK) with
{ meta: {} }makes the API responses more consistent across all operations. While both approaches are RESTful, returning 200 with a meta object allows for future extensibility if metadata about the deletion needs to be communicated.packages/runtime/src/enhancements/edge/encryption.ts (1)
Line range hint
103-104: Ensure consistent error handling usingqueryUtils.unknownErrorIn several methods, errors are thrown using
throw new Error('...'). For consistency and better error tracing, consider usingthis.queryUtils.unknownError('...')to wrap error messages uniformly throughout the codebase.Apply these changes for consistent error handling:
-throw new Error('Unexpected custom encryption settings'); +throw this.queryUtils.unknownError('Unexpected custom encryption settings');Also applies to: 113-114, 123-124
packages/runtime/src/enhancements/node/encryption.ts (1)
103-104: UsequeryUtils.unknownErrorfor consistent error handlingTo maintain consistency in error handling and improve error tracking, replace
throw new Error('...')withthrow this.queryUtils.unknownError('...')in the respective methods.Make the following changes:
-throw new Error('Unexpected custom encryption settings'); +throw this.queryUtils.unknownError('Unexpected custom encryption settings');Also applies to: 113-114, 123-124
packages/sdk/src/utils.ts (1)
553-556: Consider cloning theseenset to maintain function purityCurrently, the
seenset is mutated directly withseen.add(dataModel). To ensure that the function remains pure and avoids side effects, consider creating a newSetwhen making recursive calls. This approach prevents potential issues in concurrent or asynchronous environments.Apply this change to clone the
seenset:- seen.add(dataModel); - result.push(...getRecursiveBases(baseDecl, includeDelegate, seen)); + const newSeen = new Set(seen); + newSeen.add(dataModel); + result.push(...getRecursiveBases(baseDecl, includeDelegate, newSeen));packages/sdk/src/typescript-expression-transformer.ts (3)
329-340: Validatecasingargument in_currentModelfunctionIn the
_currentModelfunction, thecasingargument retrieved fromargs[0]may not be a validCasingvalue. Adding validation ensures robustness against invalid input.Implement validation for the
casingargument:if (args[0]) { const argValue = getLiteral<string>(args[0]) as Casing; const validCasings: Casing[] = ['original', 'upper', 'lower', 'capitalize', 'uncapitalize']; if (validCasings.includes(argValue)) { casing = argValue; } else { throw new TypeScriptExpressionTransformerError(`Invalid casing value: ${argValue}`); } }
342-359: Ensure consistent handling ofcasingin_currentOperationfunctionSimilar to
_currentModel, the_currentOperationfunction should validate thecasingargument to prevent invalid casing values from causing errors.Apply validation to the
casingargument:if (args[0]) { const argValue = getLiteral<string>(args[0]) as Casing; const validCasings: Casing[] = ['original', 'upper', 'lower', 'capitalize', 'uncapitalize']; if (validCasings.includes(argValue)) { casing = argValue; } else { throw new TypeScriptExpressionTransformerError(`Invalid casing value: ${argValue}`); } }
355-358: Avoid variable reassignment for clarityIn
_currentOperation, reassigningcontextOperationfrom'postUpdate'to'update'could lead to confusion. Consider using a separate variable to enhance code readability.Refactor to use a new variable:
let operation = this.options.operationContext; if (operation === 'postUpdate') { operation = 'update'; } return this.toStringWithCaseChange(operation, casing);packages/runtime/src/enhancements/node/delegate.ts (1)
248-269: Avoid mutating arguments directly to prevent side effectsDirectly modifying
argsanddatacan lead to unintended side effects. Consider cloning these objects before making changes to ensure that the original arguments remain unmodified.Use a deep clone when necessary:
const argsClone = clone(args); // Make modifications to argsClone instead of args🧰 Tools
🪛 Biome (1.9.4)
[error] 264-264: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/schema/tests/schema/validation/cyclic-inheritance.test.ts (1)
19-38: Consider adding edge cases for delegate inheritance.While the current test covers the basic scenarios well, consider adding test cases for:
- Models with multiple delegate fields
- Inheritance chains longer than two levels
tests/regression/tests/issue-1467.test.ts (2)
3-4: Add documentation for the regression test.Please add a comment explaining what issue #1467 was about and what specific scenario this test is validating. This helps maintain context for future maintenance.
38-44: Consider adding negative test cases.The test only covers the happy path. Consider adding test cases for:
- Attempting to create containers with non-existent drinks
- Deleting a drink and verifying cascade behavior
tests/regression/tests/issue-1930.test.ts (1)
3-80: Test validates content visibility rules effectively.The test case properly verifies that:
- Content creation and reading works for active entities
- Content is not readable when the parent entity is deleted
Consider adding these edge cases:
- Updating content of a deleted entity
- Bulk operations (createMany/updateMany)
- Cascade deletion behavior
tests/integration/tests/enhancements/with-policy/currentOperation.test.ts (1)
25-153: Consider using test.each for more maintainable tests.While the test coverage is excellent, the test cases are quite repetitive. Consider using Jest's
test.eachto make the tests more maintainable:const cases = [ ['all rule', 'all', 'create'], ['upper case', 'create', 'CREATE', 'upper'], ['lower case', 'create', 'create', 'lower'], // ... more cases ]; test.each(cases)('%s', async (name, operation, expected, casing) => { const { enhance } = await loadSchema(` model User { id Int @id @@allow('read', true) @@allow('${operation}', currentOperation(${casing ? `'${casing}'` : ''}) == '${expected}') } // ... rest of the schema `); // ... test assertions });packages/runtime/src/types.ts (1)
176-210: Consider adding key size validation.While the encryption types are well-defined, consider adding runtime validation for encryption key sizes to prevent weak keys from being used.
For example, you could add a utility function:
function validateEncryptionKey(key: Uint8Array): void { if (key.length < 32) { // 256 bits minimum throw new Error('Encryption key must be at least 32 bytes long'); } }packages/schema/src/language-server/validator/function-invocation-validator.ts (1)
90-91: Consider tracking the TODO as a GitHub issue.The TODO comment about expressing function validation rules declaratively in ZModel should be tracked for future implementation.
Would you like me to create a GitHub issue to track this enhancement?
packages/schema/src/res/stdlib.zmodel (1)
579-584: Consider enhancing the encryption documentation.While the
@encryptedattribute is well-implemented, the documentation could be enhanced with:
- Details about the encryption algorithm used
- Key management approach
- Any limitations or considerations when using encryption
tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts (1)
381-439: Well-structured test case for polymorphic relation counting!The test comprehensively validates counting functionality across the inheritance hierarchy. Consider adding edge cases:
- Empty relations (no B/C records)
- Large number of related records
- Concurrent creation/counting scenarios
packages/schema/src/cli/actions/generate.ts (1)
40-41: Improved version checking robustness.The changes enhance type safety and correctness. Consider adding:
- Logging of specific version conflicts for easier debugging
- Error handling for invalid version formats
- if (packages.length > 0) { - const versions = new Set<string>(packages.map((p) => p.version).filter((v): v is string => !!v)); + if (packages.length > 0) { + const versions = new Set<string>( + packages + .map((p) => p.version) + .filter((v): v is string => { + if (!v) return false; + // Add version format validation + return /^\d+\.\d+\.\d+(-\w+)?$/.test(v); + }) + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
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/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!**/*.json
📒 Files selected for processing (47)
packages/ide/jetbrains/build.gradle.kts(1 hunks)packages/plugins/openapi/src/rest-generator.ts(1 hunks)packages/plugins/openapi/tests/openapi-restful.test.ts(3 hunks)packages/runtime/src/constants.ts(1 hunks)packages/runtime/src/cross/nested-write-visitor.ts(2 hunks)packages/runtime/src/enhancements/edge/encryption.ts(1 hunks)packages/runtime/src/enhancements/node/create-enhancement.ts(4 hunks)packages/runtime/src/enhancements/node/default-auth.ts(2 hunks)packages/runtime/src/enhancements/node/delegate.ts(3 hunks)packages/runtime/src/enhancements/node/encryption.ts(1 hunks)packages/runtime/src/enhancements/node/password.ts(2 hunks)packages/runtime/src/types.ts(4 hunks)packages/schema/src/cli/actions/generate.ts(1 hunks)packages/schema/src/cli/actions/info.ts(1 hunks)packages/schema/src/cli/actions/init.ts(1 hunks)packages/schema/src/cli/cli-util.ts(3 hunks)packages/schema/src/cli/index.ts(1 hunks)packages/schema/src/language-server/validator/attribute-application-validator.ts(4 hunks)packages/schema/src/language-server/validator/datamodel-validator.ts(2 hunks)packages/schema/src/language-server/validator/function-invocation-validator.ts(1 hunks)packages/schema/src/plugins/enhancer/enhance/index.ts(3 hunks)packages/schema/src/plugins/enhancer/policy/expression-writer.ts(1 hunks)packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts(1 hunks)packages/schema/src/plugins/prisma/schema-generator.ts(2 hunks)packages/schema/src/res/stdlib.zmodel(2 hunks)packages/schema/src/utils/ast-utils.ts(4 hunks)packages/schema/src/utils/version-utils.ts(1 hunks)packages/schema/tests/schema/validation/cyclic-inheritance.test.ts(1 hunks)packages/sdk/src/code-gen.ts(1 hunks)packages/sdk/src/typescript-expression-transformer.ts(10 hunks)packages/sdk/src/utils.ts(1 hunks)packages/server/src/api/rest/index.ts(1 hunks)packages/server/tests/adapter/express.test.ts(1 hunks)packages/server/tests/adapter/fastify.test.ts(1 hunks)packages/server/tests/adapter/hono.test.ts(1 hunks)packages/server/tests/adapter/next.test.ts(1 hunks)packages/server/tests/adapter/sveltekit.test.ts(1 hunks)packages/server/tests/api/rest.test.ts(2 hunks)tests/integration/tests/enhancements/with-delegate/enhanced-client.test.ts(1 hunks)tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts(1 hunks)tests/integration/tests/enhancements/with-encrypted/with-encrypted.test.ts(1 hunks)tests/integration/tests/enhancements/with-password/with-password.test.ts(2 hunks)tests/integration/tests/enhancements/with-policy/currentModel.test.ts(1 hunks)tests/integration/tests/enhancements/with-policy/currentOperation.test.ts(1 hunks)tests/regression/tests/issue-1467.test.ts(1 hunks)tests/regression/tests/issue-1647.test.ts(1 hunks)tests/regression/tests/issue-1930.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/ide/jetbrains/build.gradle.kts
👮 Files not reviewed due to content moderation or server errors (3)
- packages/schema/src/utils/ast-utils.ts
- packages/schema/src/plugins/enhancer/enhance/index.ts
- packages/schema/src/plugins/prisma/schema-generator.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/runtime/src/enhancements/node/delegate.ts
[error] 264-264: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: build-test (20.x)
- GitHub Check: build-test (20.x)
- GitHub Check: dependency-review
- GitHub Check: build-test (20.x)
🔇 Additional comments (64)
packages/schema/src/language-server/validator/datamodel-validator.ts (1)
36-39: LGTM! Well-placed inheritance validation.The addition of inheritance validation is placed appropriately after other validations and is correctly guarded by a superTypes length check.
packages/schema/src/utils/version-utils.ts (1)
2-2: LGTM! Good addition of return type annotation.The explicit return type
string | undefinedimproves type safety and makes the function's contract clearer.packages/server/tests/adapter/sveltekit.test.ts (1)
167-167: Consider documenting the change in DELETE response behavior.The change from 204 to 200 status code indicates that DELETE operations now return a response body instead of no content. While both are valid REST practices, this change should be documented in the API documentation to ensure consumers are aware of the new behavior.
Let's verify if this change is documented:
packages/server/tests/adapter/hono.test.ts (1)
170-170: Ensure consistent DELETE response behavior across all adapters.The status code change from 204 to 200 matches the behavior in other adapters, maintaining consistency across the framework.
packages/server/tests/adapter/express.test.ts (1)
193-193: Verify DELETE response format in REST API documentation.The change to return 200 status code suggests DELETE operations now include a response body. This should be consistently documented in the REST API specification.
Let's verify the REST API documentation:
✅ Verification successful
DELETE response format follows JSON:API specification
The implementation returns HTTP 200 for DELETE operations, which is compliant with the JSON:API specification. The behavior is consistently implemented and well-documented in the test suite.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for REST API documentation fd "rest|api" -e md -e mdx . | xargs rg -l "DELETE.*response.*body"Length of output: 69
Script:
#!/bin/bash # Search for API documentation in various formats echo "=== Searching for API documentation files ===" fd -e md -e mdx -e yaml -e json -e yml . | rg -l -i "api|swagger|openapi" echo -e "\n=== Checking specific paths for API docs ===" for dir in docs api-docs swagger openapi; do if [ -d "$dir" ]; then echo "Found $dir directory" ls -R "$dir" 2>/dev/null fi done echo -e "\n=== Looking for DELETE endpoints in test files ===" fd "test|spec" -e ts -e js | xargs rg "delete.*response" -A 2 -B 2Length of output: 729
Script:
#!/bin/bash # Check the specific test file and context echo "=== Examining express.test.ts ===" cat packages/server/tests/adapter/express.test.ts echo -e "\n=== Looking for DELETE patterns in tests ===" rg "delete.*200|200.*delete" -i packages/server/tests -A 3 -B 3 echo -e "\n=== Checking for other response status patterns ===" ast-grep --pattern 'expect($_).toBe(200)' packages/server/tests/adapter/express.test.tsLength of output: 9308
packages/server/tests/adapter/fastify.test.ts (1)
236-236: Confirm consistent DELETE response structure across all adapters.The status code change aligns with other adapters, maintaining a unified API behavior. Consider documenting the response body structure that warranted this change from 204 (No Content) to 200 (OK).
Let's verify the response structure consistency:
✅ Verification successful
Status code 200 is correct and consistent across all adapters
The change from 204 to 200 is verified to be correct, as all adapters (Fastify, Hono, and SvelteKit) consistently return 200 for DELETE operations. This is appropriate since the endpoints return meaningful response data (deletion counts for bulk operations and confirmation data for single deletes).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for response structure definitions rg -A 5 "interface.*Response" --type tsLength of output: 41
Script:
#!/bin/bash # Search for DELETE-related test cases and status code assertions rg -B 2 -A 2 "DELETE.*|statusCode.*200|statusCode.*204" --type ts packages/server/tests/adapter/ # Search for DELETE handler implementations ast-grep --pattern 'delete($$$) { $$$ }'Length of output: 6390
packages/server/tests/adapter/next.test.ts (1)
310-310: Test updated to match new DELETE response structureThe test has been correctly updated to expect a 200 status code for DELETE operations, maintaining consistency with the architectural change.
packages/server/tests/api/rest.test.ts (1)
2343-2344: Comprehensive test coverage for new DELETE response structureThe tests have been thoroughly updated to verify both the status code (200) and response body (
{ meta: {} }). The coverage includes both regular and compound ID scenarios, ensuring the new behavior is consistently implemented.Also applies to: 2358-2359
packages/runtime/src/enhancements/edge/encryption.ts (1)
1-1:⚠️ Potential issueRemove unintended line at the beginning of the file
The first line contains an unexpected reference
../node/encryption.ts, which seems to be a leftover or misplacement. This may cause syntax errors or unintended behavior.Apply this diff to remove the unintended line:
-../node/encryption.tsLikely invalid or redundant comment.
tests/integration/tests/enhancements/with-encrypted/with-encrypted.test.ts (1)
5-478: Excellent test coverage for encryption featuresThe test suite thoroughly covers various scenarios, including simple encryption, handling of nullish values, nested encrypted fields, multiple decryption keys, custom encryption implementations, and error conditions. This comprehensive coverage ensures the encryption enhancement is robust and reliable.
packages/sdk/src/utils.ts (2)
547-551: Prevent infinite recursion ingetRecursiveBasesby tracking processed modelsThe addition of the
seenparameter togetRecursiveBasesis a prudent way to prevent infinite recursion when traversing inheritance hierarchies that may contain cycles. By maintaining aSetof already processedDataModelinstances, the function avoids revisiting the same models.
558-563:⚠️ Potential issueEnsure
superTypesis defined before iterationTo prevent potential runtime errors, check if
dataModel.superTypesis defined before attempting to iterate over it. This is important in cases wheresuperTypesmight beundefinedornull.Modify the iteration to handle undefined
superTypes:- dataModel.superTypes.forEach((superType) => { + (dataModel.superTypes ?? []).forEach((superType) => {Likely invalid or redundant comment.
packages/sdk/src/typescript-expression-transformer.ts (4)
44-45: IntroduceCasingtype to support string casing transformationsAdding the
Casingtype provides a clear definition of allowable casing options. This enhancement facilitates flexible string transformations in the expression transformer.
156-156: Ensure correctthiscontext in function handlersBy invoking
handler.value.call(this, expr, args, normalizeUndefined);, you correctly bind thethiscontext within function handlers. This ensures that instance properties and methods are accessible within the handlers.
174-174: Update function signatures to include the_invocationparameterIncluding the
_invocationparameter in function signatures allows access to the invocation context, which can be useful for more advanced transformations and contextual processing within the functions.
315-326: Handle invalidcasingvalues intoStringWithCaseChangeThe
toStringWithCaseChangemethod does not currently handle cases where an invalidcasingvalue is provided. This could lead to unexpected behavior or runtime errors.Consider adding a default case or validation to handle unexpected
casingvalues:.return( - .exhaustive(); + .otherwise(() => { throw new TypeScriptExpressionTransformerError(`Invalid casing value: ${casing}`); }) ).exhaustive();Alternatively, ensure that the
casingargument is validated before being used:const validCasings: Casing[] = ['original', 'upper', 'lower', 'capitalize', 'uncapitalize']; if (!validCasings.includes(casing)) { throw new TypeScriptExpressionTransformerError(`Invalid casing value: ${casing}`); }packages/runtime/src/enhancements/node/delegate.ts (4)
183-271: RefactorinjectSelectIncludeHierarchyto enhance clarity and maintainabilityThe introduction of selectors simplifies the handling of different cases for injecting
selectandincludeclauses. This refactoring improves the readability and maintainability of the code by clearly separating concerns and handling both regular fields and_countfields appropriately.🧰 Tools
🪛 Biome (1.9.4)
[error] 264-264: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
264-264: Avoid using thedeleteoperator to maintain object shapeUsing the
deleteoperator can negatively impact performance due to changes in the object's hidden class. Instead, assignundefinedor create a new object without the property.[performance_issue]
Modify the code to assign
undefined:- delete args[kind]['_count']; + args[kind]['_count'] = undefined;Or create a new object:
args[kind] = { ...args[kind], _count: undefined };🧰 Tools
🪛 Biome (1.9.4)
[error] 264-264: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Line range hint
330-373: EnhanceinjectBaseFieldSelectwithforCountparameter for_countpayloadsBy adding the
forCountparameter,injectBaseFieldSelectcan now differentiate between regular selections and_countselections. This enhancement increases the method's flexibility and accommodates the specific structure required for_countpayloads.
358-370: Ensure correct structure when injecting_countfieldsThe updated logic within
injectBaseFieldSelectappropriately handles the nested structure needed for_countpayloads. This ensures that_countfields are correctly included in the base model hierarchy without disrupting the overall selection structure.packages/schema/tests/schema/validation/cyclic-inheritance.test.ts (1)
4-17: LGTM! Well-structured test for abstract inheritance.The test effectively validates both direct (A->B->A) and indirect (C->B->A->B) circular inheritance scenarios.
tests/regression/tests/issue-1467.test.ts (1)
45-49: LGTM! Good use of Prisma's aggregation.The query effectively uses
_countaggregation to verify the relationship count.packages/schema/src/cli/actions/info.ts (2)
19-21: LGTM! Good defensive programming.The null check prevents adding undefined versions to the set, which could cause issues in version comparison logic.
Line range hint
19-50: Verify version comparison behavior with undefined versions.The null check prevents adding undefined versions, but we should verify the behavior when no valid versions are found.
✅ Verification successful
Version comparison behavior is properly handled
The code safely handles all version scenarios:
- Undefined versions are filtered before adding to the Set
- Version comparison only runs when valid versions exist
- When no versions are found, the function exits after displaying the package list
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are any other version-related error handling cases in the codebase rg -A 3 "versions\.size === 0" . rg -A 3 "versions\.size < 1" .Length of output: 67
Script:
#!/bin/bash # Get the complete info.ts file content cat packages/schema/src/cli/actions/info.ts # Look for version-related functions ast-grep --pattern 'function $_($$_) { $$$ version $$$ }' # Search for version-related error handling rg -A 3 "version(\s+|\.|\))" packages/schema/src/cli/actions/info.tsLength of output: 2505
packages/runtime/src/constants.ts (1)
71-81: Consider adding newer Prisma write actions.The list looks comprehensive but consider adding other write actions like:
deletedeleteMany
This ensures future-proofing for all possible write operations.Verify usage across the codebase.
Let's verify where this constant is being used to ensure consistent adoption.
tests/regression/tests/issue-1647.test.ts (2)
4-35: Test validates schema inheritance behavior effectively.The test case properly verifies that child models inherit the parent's schema by default, with clear assertions and schema setup.
36-68: Test effectively validates schema override behavior.The test case properly verifies that child models can override the parent's schema, with appropriate assertions checking both schemas.
packages/runtime/src/enhancements/node/password.ts (2)
4-4: Improved code maintainability with centralized action definitions.Good refactoring to use
ACTIONS_WITH_WRITE_PAYLOADconstant instead of inline array. This ensures consistency across handlers.Also applies to: 42-42
42-45: Verify password hashing behavior for all write actions.The change in action checking logic could affect password hashing behavior. Please verify that password hashing works correctly for all actions in
ACTIONS_WITH_WRITE_PAYLOAD.✅ Verification successful
Password hashing behavior verified for all write operations
The password hashing implementation correctly processes all write operations (
create,createMany,createManyAndReturn,update,updateMany) through the NestedWriteVisitor, using bcryptjs with proper salt configuration in both Edge and non-Edge runtimes.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if any password-related operations are handled in other files rg -l "password" | xargs rg "ACTIONS_WITH_WRITE_PAYLOAD" # Check for any password-related tests fd -e test.ts | xargs rg "@password|hashFunc|bcrypt"Length of output: 2407
Script:
#!/bin/bash # Check ACTIONS_WITH_WRITE_PAYLOAD definition rg -A 5 "ACTIONS_WITH_WRITE_PAYLOAD = \[" packages/runtime/src/constants.ts # Check password preprocessing implementation ast-grep --pattern 'preprocessWritePayload($$$) { $$$ }' # Check test cases for different write actions rg -B 2 -A 2 "@password.*@omit" tests/integration/tests/enhancements/with-password/with-password.test.tsLength of output: 601
Script:
#!/bin/bash # Find password preprocessing implementation rg -B 5 -A 15 "preprocessWritePayload" packages/runtime/src/enhancements/node/password.ts # Check test cases for different write operations rg -B 2 -A 10 "describe.*password" tests/integration/tests/enhancements/with-password/with-password.test.tsLength of output: 1455
Script:
#!/bin/bash # Find hashFunc implementation rg -B 2 -A 5 "hashFunc" packages/runtime/src/enhancements/node/password.ts # Check test cases for create and update operations rg -B 2 -A 10 "test\(.*create.*password|test\(.*update.*password" tests/integration/tests/enhancements/with-password/with-password.test.ts # Check NestedWriteVisitor implementation ast-grep --pattern 'class NestedWriteVisitor { $$$ }'Length of output: 25422
packages/schema/src/cli/actions/init.ts (1)
66-66: Documentation link updated to point to main docs.The change directs users to the main documentation instead of a specific guide, which is more appropriate for general usage instructions.
tests/integration/tests/enhancements/with-password/with-password.test.ts (3)
Line range hint
17-41: LGTM! Good test coverage for basic password operations.The test cases effectively validate password hashing for create and update operations.
42-50: LGTM! Good security validation for bulk operations.The test effectively ensures that:
- Passwords are not stored in plain text
- Bulk creation properly hashes passwords
- Password comparison works as expected
52-61: LGTM! Comprehensive test coverage for new bulk operation.The test thoroughly validates the new
createManyAndReturnoperation, ensuring proper password handling and security.tests/integration/tests/enhancements/with-policy/currentOperation.test.ts (1)
1-23: LGTM! Well-structured test for operation-specific policies.The test effectively validates that
currentOperation()correctly identifies and enforces operation-specific access rules.packages/runtime/src/types.ts (2)
137-141: LGTM! Well-documented encryption options.The encryption option is properly typed and documented.
154-154: LGTM! Clear enhancement kind addition.The 'encryption' enhancement kind is properly added to the type definition.
packages/runtime/src/enhancements/node/create-enhancement.ts (3)
13-13: LGTM! Proper integration of encryption enhancement.The encryption enhancement is correctly imported and added to the list of all enhancements.
Also applies to: 24-24
104-104: LGTM! Consistent attribute detection.The detection of @Encrypted fields follows the established pattern used for other attributes.
125-140: LGTM! Proper enhancement ordering and validation.The encryption enhancement is correctly applied after password handling but before policy enforcement. The error handling for missing encryption options is appropriate.
Let's verify that no other enhancements depend on field length:
✅ Verification successful
Ordering requirement in comment is verified correct
The comment accurately states that password and encryption enhancements must be applied before policy validation. The encryption process modifies field content length through base64 encoding, and policy validation should operate on these final transformed values. No policy validations were found that would be incorrectly affected by this ordering.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for length-dependent validations in other enhancements rg -l "length|size" --type ts packages/runtime/src/enhancements/Length of output: 644
Script:
#!/bin/bash # Check for length-related validations in policy files ast-grep --pattern 'length' packages/runtime/src/enhancements/node/policy/ # Also check encryption.ts since it's directly related to our context cat packages/runtime/src/enhancements/node/encryption.tsLength of output: 10755
Script:
#!/bin/bash # Check how @length attribute is handled in policy files ast-grep --pattern '@length' packages/runtime/src/enhancements/node/policy/ # Also check for length-related validation logic rg -A 5 "validateLength|lengthCheck" packages/runtime/src/enhancements/node/policy/Length of output: 161
Script:
#!/bin/bash # Broader search for length-related validation patterns rg -B2 -A2 "\.length|minLength|maxLength" packages/runtime/src/enhancements/node/policy/ # Check for field validation patterns ast-grep --pattern 'validate($$$)' packages/runtime/src/enhancements/node/policy/Length of output: 19037
tests/integration/tests/enhancements/with-policy/currentModel.test.ts (4)
4-24: LGTM! Well-structured basic test case.The test effectively validates the core functionality of
currentModel()in policy rules, with clear assertions for both positive and negative cases.
26-46: LGTM! Good coverage of upper case handling.The test properly validates case-sensitive model name comparisons using the 'upper' casing option.
114-134: LGTM! Comprehensive inheritance testing.The test effectively validates that
currentModel()works correctly with abstract model inheritance, which is crucial for policy inheritance scenarios.
161-171: LGTM! Good error case coverage.The test properly validates that
currentModel()cannot be used in default values, with clear error messages.packages/runtime/src/enhancements/node/default-auth.ts (1)
Line range hint
54-58: LGTM! Good refactoring to use shared constant.The change improves maintainability by using
ACTIONS_WITH_WRITE_PAYLOADinstead of a hardcoded array, making the code more DRY and consistent across the codebase.packages/schema/src/language-server/validator/function-invocation-validator.ts (1)
92-100: LGTM! Good validation for casing arguments.The validation logic properly ensures that
currentModelandcurrentOperationfunctions receive valid casing arguments, with clear error messages.packages/plugins/openapi/tests/openapi-restful.test.ts (2)
87-87: LGTM! Good refactoring to use dynamic specVersion.The change improves test maintainability by consistently using the specVersion parameter in buildOptions.
Also applies to: 327-327
343-363: LGTM! Good test coverage for integer IDs.The test properly validates that integer ID fields are correctly represented in the OpenAPI specification.
packages/runtime/src/cross/nested-write-visitor.ts (2)
7-7: LGTM! Import statement added for new utility functions.The addition of
enumerateandgetModelFieldsimports aligns with the refactoring invisitSubPayload.
313-339: Improved iteration and field handling logic.The refactoring enhances readability and maintainability by:
- Using
enumeratefor consistent iteration- Leveraging
getModelFieldsfor field access- Maintaining clear separation between field types
packages/schema/src/language-server/validator/attribute-application-validator.ts (4)
28-28: LGTM! Import statement updated to include streamAllContents.The addition of
streamAllContentsis necessary for the new encrypted fields validation.
141-143: Enhanced model-level policy validation with encrypted fields check.Good addition of encrypted fields validation to prevent their use in policy rules.
172-174: Enhanced field-level policy validation with encrypted fields check.Consistent application of encrypted fields validation across both model and field-level policies.
215-221: Well-implemented validation for encrypted fields.The method effectively:
- Traverses all nodes in the attribute application
- Identifies encrypted fields in policy rules
- Reports appropriate validation errors
packages/schema/src/cli/cli-util.ts (3)
230-236: Improved type safety and error handling.Good improvements:
- Added explicit return type annotation
- Changed error handling to return empty array instead of undefined
248-248: Enhanced type safety for version handling.Explicit casting of version to string type improves type safety.
289-289: Strengthened version comparison logic.Added null check for both versions before comparison, preventing potential runtime errors.
tests/integration/tests/enhancements/with-delegate/policy-interaction.test.ts (2)
575-614: Well-structured test for optional relation policy enforcement.The test effectively:
- Sets up a clear model structure
- Tests policy enforcement on private posts
- Verifies partial field visibility based on policies
616-653: Comprehensive test for required relation policy enforcement.The test thoroughly:
- Validates policy enforcement with required relations
- Confirms proper field masking based on privacy settings
- Maintains consistency with the optional relation test case
packages/schema/src/res/stdlib.zmodel (2)
174-183: Well-designed function with clear documentation!The
currentModelfunction is well-implemented with:
- Clear documentation of its purpose and parameter
- Flexible casing options for the returned model name
- Proper context restriction to access policies
185-195: Well-documented function with clear expansion behavior!The
currentOperationfunction is well-implemented with:
- Clear documentation of its purpose and parameter
- Explicit explanation of how "all" operation is expanded
- Proper context restriction to access policies
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (1)
Line range hint
457-485: Well-structured policy guard optimization!The changes improve policy guard generation by:
- Optimizing cases where constant functions can be used
- Properly handling the relationship between update and postUpdate policies
- Clear logic flow with good comments explaining the intent
packages/schema/src/plugins/enhancer/policy/expression-writer.ts (1)
842-854: Clean refactoring of relation check logic!The changes improve the code by:
- Using writeFieldCondition for better modularity and reuse
- Clear handling of postUpdate policies for relations
- Good comments explaining why postUpdate policies are not delegated
packages/plugins/openapi/src/rest-generator.ts (1)
909-916: Verify JSON:API compatibility with non-string IDs.While the changes allow for more accurate type representation, there's a potential compatibility issue:
- JSON:API specification requires ID fields to be strings
- Current implementation returns the original data type from ZModel schema
Consider either:
- Converting IDs to strings in the REST API handler
- Documenting this deviation from the JSON:API specification
packages/sdk/src/code-gen.ts (1)
50-54: Consider a more targeted approach to type checking disablement.While adding
// @ts-nocheckglobally solves immediate type checking issues, it might hide potential problems. Consider:
- Only disabling specific type checks using more granular directives
- Adding documentation explaining why type checking is disabled
- Implementing a mechanism to selectively enable/disable type checking based on configuration
Let's verify if there are specific type errors that need to be addressed:
✅ Verification successful
Global
@ts-nocheckis appropriate for generated codeThe usage of
// @ts-nocheckis acceptable here since it's being added to generated files, not source code. Type checking generated code is less critical as it's not meant to be manually maintained, and fixing all type issues in generated code would be impractical and time-consuming.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for TypeScript errors in generated files # Note: This helps identify which specific type checks need to be disabled # Find all TypeScript files fd -e ts -E "*.d.ts" --exec echo "// Checking {}"Length of output: 36021
No description provided.