-
-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: implement relation check() function in ZModel #1556
Conversation
WalkthroughWalkthroughThe changes introduce enhancements to multiple files related to attribute validation, function invocation, policy generation, and expression writing. Key upgrades include new functions and methods for handling checks related to read, create, update, and delete operations, with additional test cases covering these scenarios. The update emphasizes reusing policies across related models, aligning with the feature request in issue #276. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Validator as attribute-application-validator.ts
participant FunctionValidator as function-invocation-validator.ts
participant Generator as policy-guard-generator.ts
participant Writer as expression-writer.ts
participant Tester as relation-check.test.ts
User->>Validator: Attribute validation request
Validator->>FunctionValidator: Request function validation
FunctionValidator->>Generator: Generate entity checker & policy guard
Generator->>Writer: Write relation checks and expressions
Writer->>Tester: Generate and run tests
Tester-->>User: Test results
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Outside diff range and nitpick comments (2)
packages/sdk/src/typescript-expression-transformer.ts (1)
279-310
: Consider adding comments to improve readability.The
_check
function is correctly implemented, but adding comments would improve readability.packages/schema/src/plugins/enhancer/policy/utils.ts (1)
Line range hint
123-258
:
Consider adding comments to improve readability.The
generateSelectForRules
function is correctly implemented, but adding comments would improve readability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
packages/schema/package.json
is excluded by!**/*.json
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
,!**/*.yaml
Files selected for processing (12)
- packages/schema/src/language-server/validator/attribute-application-validator.ts (1 hunks)
- packages/schema/src/language-server/validator/function-invocation-validator.ts (4 hunks)
- packages/schema/src/plugins/enhancer/policy/expression-writer.ts (10 hunks)
- packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (13 hunks)
- packages/schema/src/plugins/enhancer/policy/utils.ts (10 hunks)
- packages/schema/src/res/stdlib.zmodel (1 hunks)
- packages/schema/src/utils/ast-utils.ts (1 hunks)
- packages/schema/tests/generator/expression-writer.test.ts (1 hunks)
- packages/sdk/src/index.ts (1 hunks)
- packages/sdk/src/names.ts (1 hunks)
- packages/sdk/src/typescript-expression-transformer.ts (4 hunks)
- tests/integration/tests/enhancements/with-policy/relation-check.test.ts (1 hunks)
Additional comments not posted (39)
packages/sdk/src/index.ts (1)
4-4
: Export statement is correct.The export statement for
names.ts
is correctly added. Ensure thatnames.ts
exists and is relevant to the SDK.packages/sdk/src/names.ts (2)
1-13
: FunctiongetQueryGuardFunctionName
looks good.The function correctly constructs the name of a query guard function based on the provided parameters.
15-25
: FunctiongetEntityCheckerFunctionName
looks good.The function correctly constructs the name of an entity checker function based on the provided parameters.
packages/schema/src/language-server/validator/function-invocation-validator.ts (1)
194-264
: Method_checkCheck
looks good but consider adding more validation.The method correctly validates the
check
function invocation and handles cyclic dependencies. However, consider adding more detailed validation for the arguments and edge cases.packages/schema/src/utils/ast-utils.ts (1)
154-156
: FunctionisCheckInvocation
looks good.The function correctly identifies if a node represents a
check
function invocation from the standard library.packages/schema/src/language-server/validator/attribute-application-validator.ts (5)
24-24
: LGTM!The attributeCheckers registry is correctly implemented.
Line range hint
26-32
:
LGTM!The check decorator function is correctly implemented.
Line range hint
38-81
:
LGTM!The validate function is well-implemented and handles various validation scenarios effectively.
Line range hint
83-90
:
LGTM!The
_checkModelLevelPolicy
function is correctly implemented.
Line range hint
92-113
:
LGTM!The
_checkFieldLevelPolicy
function is correctly implemented.packages/sdk/src/typescript-expression-transformer.ts (2)
Line range hint
490-502
:
LGTM!The
collectionPredicate
function is correctly implemented.
25-25
: LGTM!The
TypeScriptExpressionTransformer
class is correctly implemented.Also applies to: 40-40
packages/schema/src/plugins/enhancer/policy/utils.ts (3)
264-291
: LGTM!The
generateConstantQueryGuardFunction
function is correctly implemented.
Line range hint
292-421
:
LGTM!The
generateQueryGuardFunction
function is correctly implemented.
Line range hint
459-491
:
LGTM!The
generateEntityCheckerFunction
function is correctly implemented.tests/integration/tests/enhancements/with-policy/relation-check.test.ts (13)
3-46
: LGTM!The
should work for read
test case is correctly implemented.
48-87
: LGTM!The
should work for simple create
test case is correctly implemented.
89-148
: LGTM!The
should work for nested create
test case is correctly implemented.
150-206
: LGTM!The
should work for update
test case is correctly implemented.
208-264
: LGTM!The
should work for delete
test case is correctly implemented.
266-324
: LGTM!The
should work for field-level
test case is correctly implemented.
326-383
: LGTM!The
should work for field-level with override
test case is correctly implemented.
385-441
: LGTM!The
should work for cross-model field comparison
test case is correctly implemented.
443-503
: LGTM!The
should work for implicit specific operations
test case is correctly implemented.
505-563
: LGTM!The
should work for implicit all operations
test case is correctly implemented.
565-637
: LGTM!The
should report error for invalid args
test case is correctly implemented.
639-671
: LGTM!The
should report error for cyclic relation check
test case is correctly implemented.
673-702
: LGTM!The
should report error for cyclic relation check indirect
test case is correctly implemented.packages/schema/src/plugins/enhancer/policy/policy-guard-generator.ts (7)
248-248
: LGTM!The
operationContext
is correctly set to'create'
.
325-325
: LGTM!The
operationContext
is correctly set to'postUpdate'
.
365-369
: LGTM!The
operationContext
parameter is correctly passed to thewriteEntityChecker
method.
Line range hint
372-413
: LGTM!The logic to determine if an entity checker should be used is correct.
Line range hint
415-436
: LGTM!The
operationContext
parameter is correctly passed to thegenerateEntityCheckerFunction
andgenerateSelectForRules
methods.
452-477
: LGTM!The
operationContext
is correctly used in thegenerateConstantQueryGuardFunction
method calls.
Line range hint
587-617
: LGTM!The
operationContext
is correctly used in thewriteEntityChecker
method calls.packages/schema/src/plugins/enhancer/policy/expression-writer.ts (3)
289-292
: LGTM!The
operationContext
is correctly passed to theExpressionWriter
instantiation.
450-450
: LGTM!The logic to handle
isCheckInvocation
andoperationContext
is correct.
Line range hint
748-821
: LGTM!The logic to handle
isCheckInvocation
and thewriteRelationCheck
method is correct.packages/schema/tests/generator/expression-writer.test.ts (1)
1372-1372
: LGTM! Ensure correct initialization ofExpressionWriter
.The addition of the
operationContext
option aligns with the new functionality introduced in the PR.However, verify that the
ExpressionWriter
is correctly initialized with theoperationContext
option in other parts of the codebase.
Fixes #276