feat: allow comparing fields from different models in mutation policies #1476
Conversation
- Generate TS checker functions to evaluate rules in JS runtime - Make sure fields needed in the checker are selected when reading entities Only supporting mutation rules (create, update, post-update, delete) because: 1. Evaluating read in JS runtime may result in reading lots of rows and then discard 2. Don't know how to support aggregation without reading all rows
WalkthroughWalkthroughThe recent changes introduce support for comparing fields from different models in policy rules. This enhancement allows for more complex policy expressions, enabling granular control over data access and operations. Key modifications include updates to policy handlers, utilities, constraint solvers, and type definitions, along with additional test cases to ensure the new functionality works correctly. Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant PolicyHandler
participant PolicyUtils
participant ConstraintSolver
participant Database
User->>PolicyHandler: Request operation (e.g., create, update)
PolicyHandler->>PolicyUtils: Check policies with additional checkers
PolicyUtils->>ConstraintSolver: Evaluate constraints
ConstraintSolver-->>PolicyUtils: Return evaluation result
PolicyUtils-->>PolicyHandler: Return policy check result
PolicyHandler-->>Database: Perform operation if allowed
Database-->>PolicyHandler: Return operation result
PolicyHandler-->>User: Return final result
Assessment against linked issues
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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 10
Outside diff range and nitpick comments (9)
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (2)
Line range hint
149-149: Avoid using template literals where simple strings suffice, as it can lead to unnecessary complexity and performance overhead.Also applies to: 163-163, 286-286, 300-300, 326-326, 390-390, 392-392, 399-399, 438-438, 440-440
Line range hint
241-247: Use template literals for string concatenation to improve readability and maintainability.- const expr = denies.length > 0 ? '!(' + denies.map(deny => transformer.transform(deny)).join(' || ') + ')' : undefined; + const expr = denies.length > 0 ? `!(${denies.map(deny => transformer.transform(deny)).join(' || ')})` : undefined;Also applies to: 257-257, 261-261
tests/integration/tests/enhancements/with-policy/field-level-policy.test.ts (5)
Line range hint
2-2: Use thenode:protocol for importing Node.js built-in modules.- import path from 'path'; + import path from 'node:path';
Line range hint
41-41: Define explicit types for variables to avoid implicit 'any' type issues.// Example of adding explicit types let r: Model | undefined;
Line range hint
236-236: Define explicit types for variables to avoid implicit 'any' type issues.// Example of adding explicit types let db: EnhancedDatabase;
Line range hint
337-337: Define explicit types for variables to avoid implicit 'any' type issues.// Example of adding explicit types let r: Model | undefined;
Line range hint
424-424: Define explicit types for variables to avoid implicit 'any' type issues.// Example of adding explicit types let r: Model | undefined;packages/runtime/src/enhancements/policy/handler.ts (2)
Line range hint
216-225: Theelseclause can be omitted as previous branches break early, simplifying the control flow.if (condition) { // code return; } - else { - // other code - } + // other code
Line range hint
269-274: Prefer usingfor...ofinstead offorEachfor better performance and readability.- enumerate(args.data).forEach((item) => { + for (const item of enumerate(args.data)) { // code - }); + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- packages/runtime/src/enhancements/policy/handler.ts (11 hunks)
- packages/runtime/src/enhancements/policy/policy-utils.ts (8 hunks)
- packages/runtime/src/enhancements/types.ts (3 hunks)
- packages/schema/src/language-server/validator/expression-validator.ts (4 hunks)
- packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (5 hunks)
- packages/schema/src/plugins/enhancer/policy/utils.ts (13 hunks)
- packages/sdk/src/typescript-expression-transformer.ts (3 hunks)
- tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-policy/field-level-policy.test.ts (2 hunks)
Additional context used
Biome
packages/runtime/src/enhancements/types.ts
[error] 19-19: Unexpected any. Specify a different type.
[error] 81-81: Unexpected any. Specify a different type.
[error] 86-86: Unexpected any. Specify a different type.
[error] 91-91: Unexpected any. Specify a different type.
[error] 1-2: All these imports are only used as types.
packages/schema/src/language-server/validator/expression-validator.ts
[error] 171-171: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 174-174: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 231-231: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 1-18: Some named imports are only used as types.
[error] 24-25: Some named imports are only used as types.
[error] 26-27: All these imports are only used as types.
packages/sdk/src/typescript-expression-transformer.ts
[error] 28-30: This constructor is unnecessary.
[error] 46-51: This function expression can be turned into an arrow function.
[error] 113-113: Do not use template literals if interpolation and special-character handling are not needed.
[error] 118-118: Do not use template literals if interpolation and special-character handling are not needed.
[error] 123-130: This else clause can be omitted because previous branches break early.
[error] 127-129: This else clause can be omitted because previous branches break early.
[error] 135-135: Do not use template literals if interpolation and special-character handling are not needed.
[error] 165-165: Do not use template literals if interpolation and special-character handling are not needed.
[error] 282-284: This else clause can be omitted because previous branches break early.
[error] 297-299: This else clause can be omitted because previous branches break early.
[error] 306-306: Do not use template literals if interpolation and special-character handling are not needed.
[error] 311-322: This else clause can be omitted because previous branches break early.
[error] 317-321: This else clause can be omitted because previous branches break early.
[error] 336-338: This else clause can be omitted because previous branches break early.
[error] 422-422: Template literals are preferred over string concatenation.
[error] 427-429: This else clause can be omitted because previous branches break early.
[error] 442-446: This else clause can be omitted because previous branches break early.
[error] 444-446: This else clause can be omitted because previous branches break early.
[error] 1-22: Some named imports are only used as types.
packages/schema/src/plugins/enhancer/policy/utils.ts
[error] 61-63: This else clause can be omitted because previous branches break early.
[error] 109-113: This else clause can be omitted because previous branches break early.
[error] 125-125: Unexpected any. Specify a different type.
[error] 200-202: This else clause can be omitted because previous branches break early.
[error] 203-205: This else clause can be omitted because previous branches break early.
[error] 206-225: This else clause can be omitted because previous branches break early.
[error] 213-215: This else clause can be omitted because previous branches break early.
[error] 216-225: This else clause can be omitted because previous branches break early.
[error] 219-225: This else clause can be omitted because previous branches break early.
[error] 230-230: Prefer for...of instead of forEach.
[error] 276-278: Prefer for...of instead of forEach.
[error] 279-281: Prefer for...of instead of forEach.
[error] 285-287: This else clause can be omitted because previous branches break early.
[error] 360-360: Template literals are preferred over string concatenation.
[error] 400-403: Prefer for...of instead of forEach.
[error] 405-408: Prefer for...of instead of forEach.
[error] 414-414: Template literals are preferred over string concatenation.
[error] 455-455: Do not use template literals if interpolation and special-character handling are not needed.
[error] 19-34: Some named imports are only used as types.
[error] 35-36: All these imports are only used as types.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
[error] 30-30: A Node.js builtin module should be imported with the node: protocol.
[error] 149-149: Do not use template literals if interpolation and special-character handling are not needed.
[error] 163-163: Do not use template literals if interpolation and special-character handling are not needed.
[error] 177-177: Forbidden non-null assertion.
[error] 241-247: Template literals are preferred over string concatenation.
[error] 257-257: Template literals are preferred over string concatenation.
[error] 261-261: Template literals are preferred over string concatenation.
[error] 286-286: Do not use template literals if interpolation and special-character handling are not needed.
[error] 300-300: Do not use template literals if interpolation and special-character handling are not needed.
[error] 326-326: Do not use template literals if interpolation and special-character handling are not needed.
[error] 368-368: Forbidden non-null assertion.
[error] 390-390: Do not use template literals if interpolation and special-character handling are not needed.
[error] 392-392: Do not use template literals if interpolation and special-character handling are not needed.
[error] 399-399: Do not use template literals if interpolation and special-character handling are not needed.
[error] 411-411: Forbidden non-null assertion.
[error] 438-438: Do not use template literals if interpolation and special-character handling are not needed.
[error] 440-440: Do not use template literals if interpolation and special-character handling are not needed.
[error] 446-446: Forbidden non-null assertion.
[error] 513-513: Forbidden non-null assertion.
[error] 527-527: Forbidden non-null assertion.
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
[error] 205-205: This let declares a variable that is only assigned once.
[error] 552-552: This let declares a variable that is only assigned once.
tests/integration/tests/enhancements/with-policy/field-level-policy.test.ts
[error] 2-2: A Node.js builtin module should be imported with the node: protocol.
[error] 41-41: This variable implicitly has the any type.
[error] 236-236: This variable implicitly has the any type.
[error] 337-337: This variable implicitly has the any type.
[error] 424-424: This variable implicitly has the any type.
packages/runtime/src/enhancements/policy/policy-utils.ts
[error] 26-26: Unexpected any. Specify a different type.
[error] 57-61: This else clause can be omitted because previous branches break early.
[error] 59-61: This else clause can be omitted because previous branches break early.
[error] 71-75: This else clause can be omitted because previous branches break early.
[error] 73-75: This else clause can be omitted because previous branches break early.
[error] 84-88: This else clause can be omitted because previous branches break early.
[error] 86-88: This else clause can be omitted because previous branches break early.
[error] 97-102: This else clause can be omitted because previous branches break early.
[error] 108-110: This else clause can be omitted because previous branches break early.
[error] 134-134: Unexpected any. Specify a different type.
[error] 135-135: Unexpected any. Specify a different type.
[error] 149-149: Unexpected any. Specify a different type.
[error] 154-154: The computed expression can be simplified without the use of a string literal.
[error] 166-166: Unexpected any. Specify a different type.
[error] 171-171: The computed expression can be simplified without the use of a string literal.
[error] 183-183: Unexpected any. Specify a different type.
[error] 191-191: Prefer for...of instead of forEach.
[error] 192-192: The computed expression can be simplified without the use of a string literal.
[error] 257-257: Unexpected any. Specify a different type.
[error] 355-355: Unexpected any. Specify a different type.
packages/runtime/src/enhancements/policy/handler.ts
[error] 36-36: Unexpected any. Specify a different type.
[error] 37-37: Unexpected any. Specify a different type.
[error] 56-56: Unexpected any. Specify a different type.
[error] 82-82: Unexpected any. Specify a different type.
[error] 97-97: Unexpected any. Specify a different type.
[error] 114-114: Unexpected any. Specify a different type.
[error] 118-118: Unexpected any. Specify a different type.
[error] 124-124: Unexpected any. Specify a different type.
[error] 129-129: Unexpected any. Specify a different type.
[error] 129-129: Unexpected any. Specify a different type.
[error] 139-139: Unexpected any. Specify a different type.
[error] 139-139: Unexpected any. Specify a different type.
[error] 163-163: Unexpected any. Specify a different type.
[error] 207-207: Unexpected any. Specify a different type.
[error] 216-225: This else clause can be omitted because previous branches break early.
[error] 230-232: This else clause can be omitted because previous branches break early.
[error] 237-237: Unexpected any. Specify a different type.
[error] 249-249: Unexpected any. Specify a different type.
[error] 269-274: Prefer for...of instead of forEach.
[error] 339-339: Unexpected any. Specify a different type.
Additional comments not posted (15)
packages/runtime/src/enhancements/types.ts (2)
148-153: The addition ofadditionalCheckerandadditionalCheckerSelectorfields toModelCrudCommonaligns with the PR's goal to enhance policy checks. Ensure that these fields are utilized effectively in policy evaluation logic.
190-190: The modification inModelPostUpdateDefto excludepermissionCheckerand includepreUpdateSelectoris a significant change. Verify that this aligns with the intended use cases and that the existing functionality is not adversely affected.Verification successful
The search results indicate that
preUpdateSelectoris used in the following files:
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.tspackages/runtime/src/enhancements/types.tspackages/runtime/src/enhancements/policy/policy-utils.tsLet's review these occurrences to ensure proper integration in post-update logic.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts:writer.writeLine(`preUpdateSelector: ${JSON.stringify(preValueSelect)},`);This line suggests that
preUpdateSelectoris being written to some output, likely as part of a configuration or policy generation process.
packages/runtime/src/enhancements/types.ts:preUpdateSelector?: object;This line defines
preUpdateSelectoras an optional property in a type, which aligns with the modification inModelPostUpdateDef.
packages/runtime/src/enhancements/policy/policy-utils.ts:return def.modelLevel.postUpdate.preUpdateSelector;This line indicates that
preUpdateSelectoris being accessed within the post-update logic, which is crucial for its intended use.Based on these observations, the integration of
preUpdateSelectorappears to be consistent with its intended use in post-update scenarios.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.tspackages/runtime/src/enhancements/types.tspackages/runtime/src/enhancements/policy/policy-utils.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `preUpdateSelector` in post-update scenarios. # Test: Search for the usage of `preUpdateSelector`. Expect: Proper integration in post-update logic. rg --type typescript $'preUpdateSelector'Length of output: 75
Script:
#!/bin/bash # Description: Verify the usage of `preUpdateSelector` in post-update scenarios. # Test: Search for the usage of `preUpdateSelector`. Expect: Proper integration in post-update logic. rg --type ts 'preUpdateSelector'Length of output: 381
packages/schema/src/language-server/validator/expression-validator.ts (3)
4-5: The imports ofDataModelAttributeandDataModelFieldAttributeare crucial for enabling cross-model field comparisons. Ensure these are utilized correctly in the validation logic.
20-24: The addition of utility functions likeisDataModelFieldReferenceandisEnumFieldReferenceenhances the validator's ability to handle more complex expressions. This is a positive change that supports the PR's objectives.
181-197: The logic to restrict cross-model comparisons in "read" rules is a critical addition. Ensure that this restriction is documented clearly to avoid confusion among developers.packages/sdk/src/typescript-expression-transformer.ts (2)
37-37: The addition offutureRefContextto theOptionstype is a necessary update to support future references in expressions. This aligns well with the PR's focus on enhancing expression capabilities.
120-122: The handling offuture()expressions inmemberAccessmethod is crucial for post-update conditions. Ensure that this logic is robust and handles edge cases effectively.packages/schema/src/plugins/enhancer/policy/utils.ts (2)
14-14: The import ofisDataModelFieldReferenceis essential for identifying data model field references in policy expressions. This supports the PR's goal of enhancing policy expression capabilities.
379-430: ThegenerateTypeScriptCheckerFunctionis a significant addition that enhances the generation of TypeScript checker functions. This function should be integrated carefully to ensure it does not introduce regressions.packages/runtime/src/enhancements/policy/policy-utils.ts (3)
729-729: The implementation ofgetAdditionalCheckerandgetAdditionalCheckerSelectormethods enhances the flexibility of policy checks by allowing additional conditions based on the model and operation. This aligns with the PR's objective to support comparing fields from different models in mutation policies. However, ensure that these methods are properly integrated and tested in the context of the entire system.Also applies to: 730-730, 736-736, 737-737, 743-748, 804-812
Line range hint
971-998: The methodinjectReadCheckSelecteffectively handles the injection of read checks and additional checkers into query arguments. This is crucial for ensuring that only authorized data is fetched, especially when dealing with complex policy rules involving multiple models.
771-783: The logging within theadditionalCheckerblock provides valuable debug information but consider guarding these log statements with a check to ensure logging is enabled to avoid unnecessary performance overhead.- if (this.logger.enabled('info')) { - this.logger.info(`[policy] running additional checker on ${model} for ${operation}`); - } + // Ensure logging is enabled before logging + if (this.logger.enabled('info')) { + this.logger.info(`[policy] running additional checker on ${model} for ${operation}`); + }Likely invalid or redundant comment.
packages/runtime/src/enhancements/policy/handler.ts (3)
27-27: The import ofAdditionalCheckerFuncandCheckerConstraintshould specify types to avoid usingany.- import type { AdditionalCheckerFunc, CheckerConstraint } from '../types'; + import type { AdditionalCheckerFunc, CheckerConstraint, SpecificType } from '../types';Likely invalid or redundant comment.
1185-1256: The methodupdateManyuses complex logic for handling updates with additional checks. Ensure that the transaction handling and post-write checks are correctly implemented and tested.Verification successful
The search results indicate that the
transactionmethod is used multiple times within thepackages/runtime/src/enhancements/policy/handler.tsfile. This suggests that transaction handling is indeed implemented. However, to ensure the correctness of the transaction handling and post-write checks specifically within theupdateManymethod, we need to verify the context of these usages.Let's refine our search to focus on the
updateManymethod and its transaction handling.
The search results confirm that the
updateManymethod includes transaction handling and post-write checks. The method usesthis.queryUtils.transactionto manage transactions and includes logic for collecting and running post-write checks.Key points from the code:
- The method checks for conditions to proceed without a transaction.
- If a transaction is needed, it collects pre-update values and merges selections required for additional checkers.
- It logs the queries and performs the update within a transaction.
- Post-write checks are collected and run after the update.
This confirms that the transaction handling and post-write checks are correctly implemented in the
updateManymethod.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the transaction handling in `updateMany` method. # Test: Search for transaction usage in `updateMany`. Expect: Proper transaction management. rg --type typescript $'transaction' packages/runtime/src/enhancements/policy/handler.tsLength of output: 121
Script:
#!/bin/bash # Description: Verify the transaction handling in `updateMany` method. # Test: Search for transaction usage in `updateMany`. Expect: Proper transaction management. rg --type ts 'transaction' packages/runtime/src/enhancements/policy/handler.tsLength of output: 931
Script:
#!/bin/bash # Description: Verify the transaction handling in `updateMany` method. # Test: Search for the `updateMany` method and its transaction usage. Expect: Proper transaction management. rg --type ts 'updateMany' packages/runtime/src/enhancements/policy/handler.ts -A 20Length of output: 6282
1693-1704: The methodbuildIdFilterWithAdditionalCheckerfilters candidates based on an additional checker. Ensure that the filtering logic is correct and does not introduce any security or performance issues.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
Outdated
Show resolved
Hide resolved
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
Show resolved
Hide resolved
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
Show resolved
Hide resolved
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
Show resolved
Hide resolved
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
Show resolved
Hide resolved
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (4)
packages/schema/src/language-server/validator/expression-validator.ts (2)
Line range hint
162-200: Updated logic to handle cross-model field comparisons in binary expressions.This block introduces complex logic for handling cross-model comparisons. Consider refactoring to improve readability and maintainability. Perhaps breaking down the validation into smaller, more focused methods could help.
- if (...) { - ... - } else if (...) { - ... - } + private handleCrossModelComparison(expr: BinaryExpr, accept: ValidationAcceptor) { + ... + }
Line range hint
171-171: Replace!=with!==for strict equality checks.Using
!==ensures that both the type and value are checked, which is more reliable and should be used unless comparing againstnull.- if (expr.left.$resolvedType?.array != expr.right.$resolvedType?.array) { + if (expr.left.$resolvedType?.array !== expr.right.$resolvedType?.array) {Also applies to: 174-174, 231-231
packages/schema/tests/schema/validation/datamodel-validation.test.ts (1)
Line range hint
249-249: Avoid using template literals where not necessary.Template literals are unnecessary here and could be replaced with regular strings to improve performance and readability.
- `${prelude}` + preludeAlso applies to: 262-262, 273-273, 284-284, 294-294, 305-305, 315-315, 326-326, 339-339, 353-353, 563-563, 782-782
packages/schema/tests/schema/validation/attribute-validation.test.ts (1)
Line range hint
31-31: Consider replacing template literals with regular strings.The static analysis tool has flagged the use of template literals where they are not necessary. Since there's no dynamic content being interpolated, using regular strings would be more appropriate and potentially improve readability and performance.
- `${prelude}` + preludeApply this change to all similar instances in the file.
Also applies to: 40-40, 58-58, 67-67, 418-418, 1135-1135
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- packages/schema/src/language-server/validator/expression-validator.ts (4 hunks)
- packages/schema/tests/schema/validation/attribute-validation.test.ts (1 hunks)
- packages/schema/tests/schema/validation/datamodel-validation.test.ts (1 hunks)
Additional context used
Biome
packages/schema/src/language-server/validator/expression-validator.ts
[error] 171-171: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 174-174: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 231-231: Use !== instead of !=.
!= is only allowed when comparing againstnullpackages/schema/tests/schema/validation/datamodel-validation.test.ts
[error] 249-249: Do not use template literals if interpolation and special-character handling are not needed.
[error] 262-262: Do not use template literals if interpolation and special-character handling are not needed.
[error] 273-273: Do not use template literals if interpolation and special-character handling are not needed.
[error] 284-284: Do not use template literals if interpolation and special-character handling are not needed.
[error] 294-294: Do not use template literals if interpolation and special-character handling are not needed.
[error] 305-305: Do not use template literals if interpolation and special-character handling are not needed.
[error] 315-315: Do not use template literals if interpolation and special-character handling are not needed.
[error] 326-326: Do not use template literals if interpolation and special-character handling are not needed.
[error] 339-339: Do not use template literals if interpolation and special-character handling are not needed.
[error] 353-353: Do not use template literals if interpolation and special-character handling are not needed.
[error] 563-563: Do not use template literals if interpolation and special-character handling are not needed.
[error] 782-782: Do not use template literals if interpolation and special-character handling are not needed.
packages/schema/tests/schema/validation/attribute-validation.test.ts
[error] 31-31: Do not use template literals if interpolation and special-character handling are not needed.
[error] 40-40: Do not use template literals if interpolation and special-character handling are not needed.
[error] 58-58: Do not use template literals if interpolation and special-character handling are not needed.
[error] 67-67: Do not use template literals if interpolation and special-character handling are not needed.
[error] 418-418: Do not use template literals if interpolation and special-character handling are not needed.
[error] 1135-1135: Do not use template literals if interpolation and special-character handling are not needed.
Additional comments not posted (5)
packages/schema/src/language-server/validator/expression-validator.ts (3)
4-5: Imports forDataModelAttributeandDataModelFieldAttributeadded.These imports are necessary for the new validation logic that handles cross-model comparisons, aligning with the PR's objectives to support field comparisons across different models.
11-11: Addition ofisDataModelFieldAttributeto imports.This import is essential for the new checks introduced in the binary expression validation logic, specifically for handling policy attributes.
20-24: Addition of multiple utility imports from@zenstackhq/sdk.These utilities are crucial for the enhanced validation logic, especially for handling new types of expressions and references that are part of the policy rule enhancements.
packages/schema/tests/schema/validation/datamodel-validation.test.ts (1)
91-91: Change in test assertion method fromtoMatchObjecttotoBeTruthy.This change likely reflects a shift in testing strategy to ensure that the conditions are truthy, which might be more appropriate for the tests modified.
packages/schema/tests/schema/validation/attribute-validation.test.ts (1)
702-703: Update the test case to reflect support for cross-model comparisons in "create" rules.This change aligns with the PR's objective to enable field comparisons across different models, specifically for mutation policies. It's good to see that the test case has been updated to reflect this new capability.
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (10)
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (2)
Line range hint
30-30: Consider using thenode:protocol for importing Node.js built-in modules to ensure compatibility and clarity.- import path from 'path'; + import path from 'node:path';
Line range hint
241-247: Prefer using template literals over string concatenation for better readability and maintainability.- let expr = denies.length > 0 ? '!(' + denies.map((deny) => { return transformer.transform(deny); }).join(' || ') + ')' : undefined; + let expr = denies.length > 0 ? `!(${denies.map(deny => transformer.transform(deny)).join(' || ')})` : undefined; - writer.write('return ' + expr); + writer.write(`return ${expr}`); - const denyStmt = denies.length > 0 ? '!(' + denies.map((deny) => { return transformer.transform(deny); }).join(' || ') + ')' : undefined; + const denyStmt = denies.length > 0 ? `!(${denies.map(deny => transformer.transform(deny)).join(' || ')})` : undefined;Also applies to: 625-631, 636-642
packages/runtime/src/enhancements/policy/policy-utils.ts (5)
Line range hint
34-34: Specify a more precise type instead ofanyforprismaModule.- private readonly prismaModule: any; + private readonly prismaModule: PrismaClient;
Line range hint
65-69: Remove unnecessary else clauses to simplify control flow.- } else { - return this.reduce({ OR: filtered }); - } + return this.reduce({ OR: filtered });Also applies to: 67-69, 79-83, 81-83, 92-96, 94-96, 105-110, 116-118, 358-360
Line range hint
142-142: Specify a more precise type instead ofanyfor various properties and parameters to enhance type safety.- private readonly user?: AuthUser; + private readonly user?: AuthUser | null;Also applies to: 143-143, 157-157, 174-174, 191-191, 265-265
Line range hint
162-162: Simplify computed expressions and use modern JavaScript syntax for better readability and performance.- 'AND' in condition && Array.isArray(condition.AND) && condition.AND.length === 0 + condition.AND?.length === 0Also applies to: 179-179, 200-200
Line range hint
199-199: Usefor...ofinstead offorEachfor better performance and readability.- Object.entries(fields).forEach(([k, v]) => { + for (const [k, v] of Object.entries(fields)) {packages/runtime/src/enhancements/policy/handler.ts (3)
Line range hint
56-56: Refactor to replaceanywith more specific types to improve type safety across various method parameters and return types.- findUnique(args: any) { + findUnique(args: FindUniqueArgs) { - findUniqueOrThrow(args: any) { + findUniqueOrThrow(args: FindUniqueArgs) { - findFirst(args?: any) { + findFirst(args?: FindFirstArgs) { - findFirstOrThrow(args: any) { + findFirstOrThrow(args: FindFirstArgs) { - findMany(args?: any) { + findMany(args?: FindManyArgs) { - create(args: any) { + create(args: CreateArgs) { - createMany(args: { data: any; skipDuplicates?: boolean }) { + createMany(args: { data: CreateData[]; skipDuplicates?: boolean }) { - update(args: any) { + update(args: UpdateArgs) { - updateMany(args: any) { + updateMany(args: UpdateManyArgs) { - upsert(args: any) { + upsert(args: UpsertArgs) { - delete(args: any) { + delete(args: DeleteArgs) { - deleteMany(args: any) { + deleteMany(args: DeleteManyArgs) { - aggregate(args: any) { + aggregate(args: AggregateArgs) { - groupBy(args: any) { + groupBy(args: GroupByArgs) { - count(args: any) { + count(args: CountArgs) { - subscribe(args: any) { + subscribe(args: SubscribeArgs) {Also applies to: 82-82, 97-97, 114-114, 118-118, 124-124, 129-129, 139-139, 163-163, 207-207, 237-237, 249-249, 339-339
Line range hint
216-225: Refactor to eliminate unnecessary else clauses and improve code readability.- if (!args) { - throw prismaClientValidationError(this.prisma, this.prismaModule, 'query argument is required'); - } else if (!args.where) { - throw prismaClientValidationError( - this.prisma, this.prismaModule, 'where field is required in query argument' - ); - } else if (!args.data) { - throw prismaClientValidationError( - this.prisma, this.prismaModule, 'data field is required in query argument' - ); - } + if (!args) throw prismaClientValidationError(this.prisma, this.prismaModule, 'query argument is required'); + if (!args.where) throw prismaClientValidationError(this.prisma, this.prismaModule, 'where field is required in query argument'); + if (!args.data) throw prismaClientValidationError(this.prisma, this.prismaModule, 'data field is required in query argument');Also applies to: 230-232
Line range hint
269-274: Prefer usingfor...ofinstead offorEachfor better performance and error handling.- enumerate(args.data).forEach((item) => { + for (const item of enumerate(args.data)) { const validationResult = this.validateCreateInputSchema(this.model, item); if (validationResult !== item) { this.policyUtils.replace(item, validationResult); } - }); + }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- packages/runtime/src/enhancements/policy/constraint-solver.ts (4 hunks)
- packages/runtime/src/enhancements/policy/handler.ts (12 hunks)
- packages/runtime/src/enhancements/policy/policy-utils.ts (20 hunks)
- packages/runtime/src/enhancements/types.ts (8 hunks)
- packages/runtime/src/types.ts (1 hunks)
- packages/schema/src/language-server/validator/expression-validator.ts (5 hunks)
- packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (8 hunks)
- packages/schema/src/plugins/enhancer/policy/utils.ts (14 hunks)
- packages/schema/tests/schema/validation/attribute-validation.test.ts (1 hunks)
- tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts (1 hunks)
Additional context used
Biome
packages/runtime/src/types.ts
[error] 3-3: Unexpected any. Specify a different type.
[error] 3-3: Unexpected any. Specify a different type.
[error] 9-9: Unexpected any. Specify a different type.
[error] 10-10: Unexpected any. Specify a different type.
[error] 11-11: Unexpected any. Specify a different type.
[error] 12-12: Unexpected any. Specify a different type.
[error] 13-13: Unexpected any. Specify a different type.
[error] 14-14: Unexpected any. Specify a different type.
[error] 16-16: Unexpected any. Specify a different type.
[error] 18-18: Unexpected any. Specify a different type.
[error] 19-19: Unexpected any. Specify a different type.
[error] 21-21: Unexpected any. Specify a different type.
[error] 22-22: Unexpected any. Specify a different type.
[error] 23-23: Unexpected any. Specify a different type.
[error] 24-24: Unexpected any. Specify a different type.
[error] 26-26: Unexpected any. Specify a different type.
[error] 59-59: Unexpected any. Specify a different type.
packages/runtime/src/enhancements/types.ts
[error] 19-19: Unexpected any. Specify a different type.
[error] 30-30: Unexpected any. Specify a different type.
[error] 1-2: All these imports are only used as types.
packages/runtime/src/enhancements/policy/constraint-solver.ts
[error] 173-175: This else clause can be omitted because previous branches break early.
[error] 202-205: This else clause can be omitted because previous branches break early.
packages/schema/src/language-server/validator/expression-validator.ts
[error] 169-169: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 172-172: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 227-227: Use !== instead of !=.
!= is only allowed when comparing againstnull
[error] 1-16: Some named imports are only used as types.
[error] 22-23: Some named imports are only used as types.
[error] 24-25: All these imports are only used as types.
packages/schema/src/plugins/enhancer/policy/utils.ts
[error] 107-111: This else clause can be omitted because previous branches break early.
[error] 119-119: Unexpected any. Specify a different type.
[error] 194-196: This else clause can be omitted because previous branches break early.
[error] 197-199: This else clause can be omitted because previous branches break early.
[error] 200-219: This else clause can be omitted because previous branches break early.
[error] 207-209: This else clause can be omitted because previous branches break early.
[error] 210-219: This else clause can be omitted because previous branches break early.
[error] 213-219: This else clause can be omitted because previous branches break early.
[error] 224-224: Prefer for...of instead of forEach.
[error] 270-272: Prefer for...of instead of forEach.
[error] 273-275: Prefer for...of instead of forEach.
[error] 279-281: This else clause can be omitted because previous branches break early.
[error] 354-354: Template literals are preferred over string concatenation.
[error] 394-397: Prefer for...of instead of forEach.
[error] 399-402: Prefer for...of instead of forEach.
[error] 408-408: Template literals are preferred over string concatenation.
[error] 449-449: Do not use template literals if interpolation and special-character handling are not needed.
[error] 19-34: Some named imports are only used as types.
[error] 35-36: All these imports are only used as types.
[error] 124-124: Reassigning a function parameter is confusing.
packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts
[error] 30-30: A Node.js builtin module should be imported with the node: protocol.
[error] 149-149: Do not use template literals if interpolation and special-character handling are not needed.
[error] 163-163: Do not use template literals if interpolation and special-character handling are not needed.
[error] 177-177: Forbidden non-null assertion.
[error] 241-247: Template literals are preferred over string concatenation.
[error] 257-257: Template literals are preferred over string concatenation.
[error] 261-261: Template literals are preferred over string concatenation.
[error] 286-286: Do not use template literals if interpolation and special-character handling are not needed.
[error] 300-300: Do not use template literals if interpolation and special-character handling are not needed.
[error] 326-326: Do not use template literals if interpolation and special-character handling are not needed.
[error] 395-395: Forbidden non-null assertion.
[error] 412-412: Do not use template literals if interpolation and special-character handling are not needed.
[error] 414-414: Do not use template literals if interpolation and special-character handling are not needed.
[error] 421-421: Do not use template literals if interpolation and special-character handling are not needed.
[error] 433-433: Forbidden non-null assertion.
[error] 460-460: Do not use template literals if interpolation and special-character handling are not needed.
[error] 462-462: Do not use template literals if interpolation and special-character handling are not needed.
[error] 468-468: Forbidden non-null assertion.
[error] 625-631: Template literals are preferred over string concatenation.
[error] 636-642: Template literals are preferred over string concatenation.
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
[error] 552-552: This let declares a variable that is only assigned once.
packages/schema/tests/schema/validation/attribute-validation.test.ts
[error] 31-31: Do not use template literals if interpolation and special-character handling are not needed.
[error] 40-40: Do not use template literals if interpolation and special-character handling are not needed.
[error] 58-58: Do not use template literals if interpolation and special-character handling are not needed.
[error] 67-67: Do not use template literals if interpolation and special-character handling are not needed.
[error] 418-418: Do not use template literals if interpolation and special-character handling are not needed.
[error] 1135-1135: Do not use template literals if interpolation and special-character handling are not needed.
packages/runtime/src/enhancements/policy/policy-utils.ts
[error] 34-34: Unexpected any. Specify a different type.
[error] 65-69: This else clause can be omitted because previous branches break early.
[error] 67-69: This else clause can be omitted because previous branches break early.
[error] 79-83: This else clause can be omitted because previous branches break early.
[error] 81-83: This else clause can be omitted because previous branches break early.
[error] 92-96: This else clause can be omitted because previous branches break early.
[error] 94-96: This else clause can be omitted because previous branches break early.
[error] 105-110: This else clause can be omitted because previous branches break early.
[error] 116-118: This else clause can be omitted because previous branches break early.
[error] 142-142: Unexpected any. Specify a different type.
[error] 143-143: Unexpected any. Specify a different type.
[error] 157-157: Unexpected any. Specify a different type.
[error] 162-162: The computed expression can be simplified without the use of a string literal.
[error] 174-174: Unexpected any. Specify a different type.
[error] 179-179: The computed expression can be simplified without the use of a string literal.
[error] 191-191: Unexpected any. Specify a different type.
[error] 199-199: Prefer for...of instead of forEach.
[error] 200-200: The computed expression can be simplified without the use of a string literal.
[error] 265-265: Unexpected any. Specify a different type.
[error] 358-360: This else clause can be omitted because previous branches break early.
packages/runtime/src/enhancements/policy/handler.ts
[error] 36-36: Unexpected any. Specify a different type.
[error] 37-37: Unexpected any. Specify a different type.
[error] 56-56: Unexpected any. Specify a different type.
[error] 82-82: Unexpected any. Specify a different type.
[error] 97-97: Unexpected any. Specify a different type.
[error] 114-114: Unexpected any. Specify a different type.
[error] 118-118: Unexpected any. Specify a different type.
[error] 124-124: Unexpected any. Specify a different type.
[error] 129-129: Unexpected any. Specify a different type.
[error] 129-129: Unexpected any. Specify a different type.
[error] 139-139: Unexpected any. Specify a different type.
[error] 139-139: Unexpected any. Specify a different type.
[error] 163-163: Unexpected any. Specify a different type.
[error] 207-207: Unexpected any. Specify a different type.
[error] 216-225: This else clause can be omitted because previous branches break early.
[error] 230-232: This else clause can be omitted because previous branches break early.
[error] 237-237: Unexpected any. Specify a different type.
[error] 249-249: Unexpected any. Specify a different type.
[error] 269-274: Prefer for...of instead of forEach.
[error] 339-339: Unexpected any. Specify a different type.
Additional comments not posted (11)
packages/runtime/src/types.ts (1)
65-65: The addition ofPermissionCheckerContextaligns with the PR's goal to enhance policy checks by providing a context for permission checkers.packages/runtime/src/enhancements/types.ts (2)
30-30: The addition ofEntityCheckerFuncis crucial for defining functions that check entity data for permissions, supporting the PR's objectives to enhance policy flexibility.
35-35: ThePermissionCheckerFunctype is well-defined and supports the new permission checking mechanisms introduced in this PR.packages/runtime/src/enhancements/policy/constraint-solver.ts (1)
25-25: ThecheckSatfunction correctly resets the solver's state before each check, ensuring that previous computations do not affect the current one.packages/schema/src/language-server/validator/expression-validator.ts (1)
17-17: Thevalidatefunction is well-structured and correctly handles different types of expressions, aligning with the PR's goal to improve validation logic.packages/schema/src/plugins/enhancer/policy/utils.ts (1)
48-48: ThegetPolicyExpressionsfunction correctly filters attributes based on the operation kind and whether they are overrides, supporting dynamic policy rule generation.tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts (4)
57-57: Skip adding support for cross-model field comparison in read rules as it's already planned.
178-178: Skip adding support for cross-model field comparison in read rules as it's already planned.
401-401: Skip adding support for cross-model field comparison in read rules as it's already planned.
522-522: Skip adding support for cross-model field comparison in read rules as it's already planned.packages/runtime/src/enhancements/policy/handler.ts (1)
27-27: Consider replacinganywith more specific types to enhance type safety.- import { PolicyCrudKind, PolicyOperationKind, type CrudContract, type DbClientContract } from '../../types'; + import { PolicyCrudKind, PolicyOperationKind, type CrudContract, type DbClientContract, type SpecificType } from '../../types';Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (3)
packages/schema/src/plugins/enhancer/policy/utils.ts (3)
48-49: The default parameterfilteringetPolicyExpressionsfunction enhances flexibility but ensure it's well-documented to avoid confusion.Consider adding detailed comments explaining the use of each filter option.
163-163: Ensure that ignoring future references is correctly documented to avoid misuse.Consider adding a comment explaining why
future()references are ignored in certain contexts.
375-426: ThegenerateEntityCheckerFunctionis a significant addition. Ensure it aligns with the overall architecture and is optimized for performance.Review the use of this function in the system to ensure it does not introduce performance bottlenecks, especially in complex policy evaluations.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/schema/src/plugins/enhancer/policy/utils.ts (15 hunks)
- tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts (1 hunks)
Additional context used
Biome
packages/schema/src/plugins/enhancer/policy/utils.ts
[error] 107-111: This else clause can be omitted because previous branches break early.
[error] 119-119: Unexpected any. Specify a different type.
[error] 194-196: This else clause can be omitted because previous branches break early.
[error] 197-199: This else clause can be omitted because previous branches break early.
[error] 200-221: This else clause can be omitted because previous branches break early.
[error] 207-211: This else clause can be omitted because previous branches break early.
[error] 212-221: This else clause can be omitted because previous branches break early.
[error] 215-221: This else clause can be omitted because previous branches break early.
[error] 226-226: Prefer for...of instead of forEach.
[error] 272-274: Prefer for...of instead of forEach.
[error] 275-277: Prefer for...of instead of forEach.
[error] 281-283: This else clause can be omitted because previous branches break early.
[error] 356-356: Template literals are preferred over string concatenation.
[error] 396-399: Prefer for...of instead of forEach.
[error] 401-404: Prefer for...of instead of forEach.
[error] 410-410: Template literals are preferred over string concatenation.
[error] 451-451: Do not use template literals if interpolation and special-character handling are not needed.
[error] 19-34: Some named imports are only used as types.
[error] 35-36: All these imports are only used as types.
[error] 124-124: Reassigning a function parameter is confusing.
tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts
[error] 552-552: This let declares a variable that is only assigned once.
Additional comments not posted (7)
packages/schema/src/plugins/enhancer/policy/utils.ts (3)
58-61: The logic to determineisOverrideis clear and concise.
143-146: The handling ofisFutureExprin thevisitfunction is efficient.
472-483: ThehasCrossModelComparisonfunction is critical for the new feature. Verify its integration with other components.tests/integration/tests/enhancements/with-policy/cross-model-field-comparison.test.ts (4)
57-57: Skip adding support for cross-model field comparison in read rules as per previous discussion.
178-178: Skip adding support for cross-model field comparison in read rules as per previous discussion.
401-401: Skip adding support for cross-model field comparison in read rules as per previous discussion.
522-522: Skip adding support for cross-model field comparison in read rules as per previous discussion.
| data: { profiles: { update: { where: { id: 1 }, data: { age: 20 } } } }, | ||
| }) | ||
| ).toResolveTruthy(); | ||
| let r = await prisma.user.findUnique({ where: { id: 1 }, include: { profiles: true } }); |
There was a problem hiding this comment.
Refactor to use const for variables that are assigned only once.
- let r = await prisma.user.findUnique({ where: { id: 1 }, include: { profiles: true } });
+ const r = await prisma.user.findUnique({ where: { id: 1 }, include: { profiles: true } });Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| let r = await prisma.user.findUnique({ where: { id: 1 }, include: { profiles: true } }); | |
| const r = await prisma.user.findUnique({ where: { id: 1 }, include: { profiles: true } }); |
Tools
Biome
[error] 552-552: This let declares a variable that is only assigned once.
| @@ -11,6 +11,7 @@ import { | |||
| getIdFields, | |||
| getLiteral, | |||
| isAuthInvocation, | |||
| isDataModelFieldReference, | |||
There was a problem hiding this comment.
Consider adding unit tests for isDataModelFieldReference.
Would you like me to help by generating unit tests or opening a GitHub issue to ensure this function is thoroughly tested?
| if (filter === 'onlyCrossModelComparison') { | ||
| result = result.filter((expr) => hasCrossModelComparison(expr)); | ||
| } else if (filter === 'withoutCrossModelComparison') { | ||
| result = result.filter((expr) => !hasCrossModelComparison(expr)); | ||
| } |
There was a problem hiding this comment.
Optimize the filtering logic in getPolicyExpressions by combining conditions to reduce redundancy.
- if (filter === 'onlyCrossModelComparison') {
- result = result.filter((expr) => hasCrossModelComparison(expr));
- } else if (filter === 'withoutCrossModelComparison') {
- result = result.filter((expr) => !hasCrossModelComparison(expr));
- }
+ result = result.filter(expr => filter === 'all' || (filter === 'onlyCrossModelComparison' && hasCrossModelComparison(expr)) || (filter === 'withoutCrossModelComparison' && !hasCrossModelComparison(expr)));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| if (filter === 'onlyCrossModelComparison') { | |
| result = result.filter((expr) => hasCrossModelComparison(expr)); | |
| } else if (filter === 'withoutCrossModelComparison') { | |
| result = result.filter((expr) => !hasCrossModelComparison(expr)); | |
| } | |
| result = result.filter(expr => filter === 'all' || (filter === 'onlyCrossModelComparison' && hasCrossModelComparison(expr)) || (filter === 'withoutCrossModelComparison' && !hasCrossModelComparison(expr))); |
| // recurse into RHS | ||
| const rhs = collectReferencePaths(expr.right); | ||
| if (path) { | ||
| // recurse into RHS | ||
| const rhs = collectReferencePaths(expr.right); | ||
| // combine path of LHS and RHS | ||
| return rhs.map((r) => [...path, ...r]); | ||
| } else { | ||
| return []; | ||
| // LHS is not rooted from the current model, | ||
| // only keep RHS items that contains '$this' | ||
| return rhs.filter((r) => r.includes('$this')); |
There was a problem hiding this comment.
Refactor the logic in collectReferencePaths to simplify the handling of right-hand side expressions.
- if (path) {
- // combine path of LHS and RHS
- return rhs.map((r) => [...path, ...r]);
- } else {
- // LHS is not rooted from the current model,
- // only keep RHS items that contains '$this'
- return rhs.filter((r) => r.includes('$this'));
- }
+ return path ? rhs.map(r => [...path, ...r]) : rhs.filter(r => r.includes('$this'));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // recurse into RHS | |
| const rhs = collectReferencePaths(expr.right); | |
| if (path) { | |
| // recurse into RHS | |
| const rhs = collectReferencePaths(expr.right); | |
| // combine path of LHS and RHS | |
| return rhs.map((r) => [...path, ...r]); | |
| } else { | |
| return []; | |
| // LHS is not rooted from the current model, | |
| // only keep RHS items that contains '$this' | |
| return rhs.filter((r) => r.includes('$this')); | |
| // recurse into RHS | |
| const rhs = collectReferencePaths(expr.right); | |
| // combine path of LHS and RHS or filter RHS items that contain '$this' | |
| return path ? rhs.map(r => [...path, ...r]) : rhs.filter(r => r.includes('$this')); |
| @@ -19,6 +20,7 @@ | |||
| import { | |||
| Enum, | |||
| Model, | |||
| isBinaryExpr, | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The search results indicate that there are no unit tests specifically covering the isBinaryExpr function. This function is crucial for evaluating expressions, and it is important to ensure it is thoroughly tested.
- Please add comprehensive unit tests for the
isBinaryExprfunction to ensure its correctness and reliability.
Analysis chain
The function isBinaryExpr is crucial for evaluating expressions. Ensure it is covered by comprehensive unit tests.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the unit tests for `isBinaryExpr`.
# Test: Search for unit tests covering `isBinaryExpr`. Expect: Comprehensive tests.
rg --type ts 'isBinaryExpr'
Length of output: 760
Script:
#!/bin/bash
# Description: Verify the unit tests for `isBinaryExpr`.
# Test: Search for unit tests covering `isBinaryExpr`. Expect: Comprehensive tests.
rg --type ts 'isBinaryExpr' --files-with-matches | xargs -I {} rg --type ts 'describe|test|it' {}
Length of output: 20229
| export function generateSelectForRules(rules: Expression[], forAuthContext = false, ignoreFutureReference = true) { | ||
| const result: any = {}; | ||
| const addPath = (path: string[]) => { | ||
| const thisIndex = path.lastIndexOf('$this'); | ||
| if (thisIndex >= 0) { | ||
| // drop everything before $this | ||
| path = path.slice(thisIndex + 1); | ||
| } |
There was a problem hiding this comment.
Tip
Codebase Verification
The search across the entire repository did not yield any test files directly covering the generateSelectForRules function. This indicates that the function might not be explicitly tested.
Please ensure that the generateSelectForRules function is covered by unit tests to handle edge cases.
- Add unit tests for
generateSelectForRulesin the appropriate test files.
Analysis chain
The function generateSelectForRules uses a complex logic to build select objects. Ensure it's covered by unit tests to handle edge cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the unit tests for `generateSelectForRules`.
# Test: Search for unit tests covering `generateSelectForRules`. Expect: Comprehensive tests.
rg --type ts 'generateSelectForRules'
Length of output: 762
Script:
#!/bin/bash
# Description: Verify the unit tests for `generateSelectForRules` by checking related test files.
# Find test files related to `policy-guard-generator.ts`
test_files=$(fd --type f --extension ts --glob '*policy-guard-generator*.test.ts')
# Search for `generateSelectForRules` in the identified test files
for test_file in $test_files; do
echo "Checking $test_file"
rg 'generateSelectForRules' "$test_file"
done
Length of output: 85
Script:
#!/bin/bash
# Description: Verify the unit tests for `generateSelectForRules` by checking test files in the relevant directory.
# Find all test files in the `packages/schema/src/plugins/enhancer/policy` directory
test_files=$(fd --type f --extension ts --glob '*.test.ts' packages/schema/src/plugins/enhancer/policy)
# Search for `generateSelectForRules` in the identified test files
for test_file in $test_files; do
echo "Checking $test_file"
rg 'generateSelectForRules' "$test_file"
done
Length of output: 106
Script:
#!/bin/bash
# Description: Verify the unit tests for `generateSelectForRules` by checking test files in the entire repository.
# Find all test files in the repository
test_files=$(fd --type f --extension ts --glob '*.test.ts')
# Search for `generateSelectForRules` in the identified test files
for test_file in $test_files; do
echo "Checking $test_file"
rg 'generateSelectForRules' "$test_file"
done
Length of output: 42207
Tools
Biome
[error] 119-119: Unexpected any. Specify a different type.
[error] 124-124: Reassigning a function parameter is confusing.
Only supporting mutation rules (create, update, post-update, delete) because:
Fixes #1463 #786