feat: implement field validation #290
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a runtime attribute-based validation engine and integrates it into CRUD validators, updates numeric validation signatures, refactors compound-unique filter flattening, makes Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Claude finished @ymc9's task —— View job PR Review: Field Validation ImplementationI've completed my review of this field validation implementation. This is a comprehensive addition that adds Zod-based input validation to the ZenStack runtime. Here's my detailed feedback: Todo List:
✅ StrengthsRobust Architecture
Excellent Field Support
Integration Quality
|
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive field validation functionality, introducing both built-in validators for primitive types and custom validation rules for models.
Key changes:
- Added built-in validation attributes (@Length, @email, @url, etc.) for string and number fields
- Implemented custom model-level validation using @@Validate attribute with expression evaluation
- Enhanced test framework with new validation assertion methods
Reviewed Changes
Copilot reviewed 9 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/orm/validation/toplevel.test.ts | Tests for field-level validation attributes on string and number types |
| tests/e2e/orm/validation/nested.test.ts | Tests for validation in nested create/update operations |
| tests/e2e/orm/validation/custom-validation.test.ts | Tests for custom validation rules using @@Validate attribute |
| tests/e2e/orm/client-api/compound-id.test.ts | Fixed import order |
| packages/testtools/src/vitest-ext.ts | Added validation test helpers and error message checking |
| packages/runtime/src/client/query-utils.ts | Enhanced compound unique filter flattening logic |
| packages/runtime/src/client/crud/validator/utils.ts | Core validation utility functions for attributes and expression evaluation |
| packages/runtime/src/client/crud/validator/index.ts | Integrated validation into the input validator system |
| packages/runtime/src/client/crud/operations/base.ts | Simplified exists check implementation |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
packages/runtime/src/client/query-utils.ts (1)
313-313: Consider validating that value is a plain object before Object.assign.
Object.assign(flattenedResult, value)assumesvalueis a plain object. If the input filter is malformed andvalueisnull,undefined, or a primitive,Object.assignwill silently produce unexpected results (e.g., spreading string characters as numbered keys). While the function's contract expects proper input, a type guard adds robustness.Apply this diff to add a validation check:
if (compoundUniques.some(({ name }) => name === key)) { // flatten the compound field + if (typeof value !== 'object' || value === null || Array.isArray(value)) { + throw new QueryError(`Invalid compound unique filter value for "${key}"`); + } Object.assign(flattenedResult, value);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
packages/runtime/src/client/crud/operations/base.ts(1 hunks)packages/runtime/src/client/crud/validator/index.ts(8 hunks)packages/runtime/src/client/crud/validator/utils.ts(1 hunks)packages/runtime/src/client/query-utils.ts(1 hunks)packages/testtools/src/types.d.ts(1 hunks)packages/testtools/src/vitest-ext.ts(4 hunks)tests/e2e/orm/client-api/compound-id.test.ts(1 hunks)tests/e2e/orm/validation/custom-validation.test.ts(1 hunks)tests/e2e/orm/validation/nested.test.ts(1 hunks)tests/e2e/orm/validation/toplevel.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
packages/runtime/src/client/query-utils.tspackages/runtime/src/client/crud/operations/base.tstests/e2e/orm/client-api/compound-id.test.tspackages/testtools/src/types.d.tstests/e2e/orm/validation/nested.test.tstests/e2e/orm/validation/toplevel.test.tspackages/runtime/src/client/crud/validator/index.tspackages/testtools/src/vitest-ext.tstests/e2e/orm/validation/custom-validation.test.tspackages/runtime/src/client/crud/validator/utils.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/orm/client-api/compound-id.test.tstests/e2e/orm/validation/nested.test.tstests/e2e/orm/validation/toplevel.test.tstests/e2e/orm/validation/custom-validation.test.ts
🧬 Code graph analysis (6)
tests/e2e/orm/validation/nested.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
tests/e2e/orm/validation/toplevel.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
packages/runtime/src/client/crud/validator/index.ts (4)
packages/runtime/src/client/errors.ts (1)
InputValidationError(9-13)packages/runtime/src/utils/zod-utils.ts (1)
formatError(8-14)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/runtime/src/client/crud/validator/utils.ts (3)
addStringValidation(23-84)addNumberValidation(86-117)addCustomValidation(119-139)
packages/testtools/src/vitest-ext.ts (1)
packages/runtime/src/client/errors.ts (1)
InputValidationError(9-13)
tests/e2e/orm/validation/custom-validation.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
packages/runtime/src/client/crud/validator/utils.ts (5)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/sdk/src/schema/expression.ts (5)
FieldExpression(22-25)UnaryExpression(33-37)BinaryExpression(39-44)MemberExpression(27-31)CallExpression(46-50)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)packages/runtime/src/client/errors.ts (1)
QueryError(18-22)
🪛 ast-grep (0.39.5)
packages/runtime/src/client/crud/validator/utils.ts
[warning] 60-60: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 282-282: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: claude-review
🔇 Additional comments (3)
tests/e2e/orm/client-api/compound-id.test.ts (1)
2-2: Import order change looks good.Reads cleanly and keeps vitest imports grouped.
packages/runtime/src/client/crud/operations/base.ts (1)
134-137: LGTM! Good refactor to eliminate duplication.Delegating to
readUniquerather than manually constructing a query reduces code duplication and improves maintainability. The behavior remains equivalent—both approaches return the entity's id fields (or null/undefined when not found)—and all call sites already check for truthy/falsy values, so the change is safe.packages/runtime/src/client/query-utils.ts (1)
307-337: LGTM! The refactored merge logic correctly handles edge cases.The separation into
flattenedResultandrestFilterclarifies intent, and the four-way branching properly handles:
- Empty flattening (return original)
- Complete flattening (return flattened)
- Key overlap (AND clause prevents conflicting conditions)
- Safe merge (no overlap)
The AND clause generation for overlapping keys (lines 328-332) is particularly important—it ensures that impossible conditions like
{ id1: 1, id1: 3 }are represented as an explicit AND that returns no results, rather than silently overwriting values.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
tests/e2e/orm/validation/custom-validation.test.ts (1)
94-95: Good fix: explicit assertion added.
expect(thrown).toBe(true);ensures the test fails if no error is thrown.packages/runtime/src/client/crud/validator/index.ts (1)
254-270: Primitive validators applied consistently.String/Int/Float/BigInt/Decimal now flow through attribute validators. Nice catch on Float.
🧹 Nitpick comments (3)
packages/runtime/src/client/crud/validator/index.ts (1)
1141-1157: Update operators for Decimal/BigInt accept only numbers.
set/increment/decrement/multiply/divideare typed asz.number(), which loses precision for Decimal and can’t represent large BigInt safely.Consider widening per field type (sketch):
- set: this.nullableIf(z.number().optional(), !!fieldDef.optional), - increment: z.number().optional(), + set: this.nullableIf( + match(fieldDef.type) + .with('BigInt', () => z.bigint().or(z.number().int())) + .with('Decimal', () => z.union([z.instanceof(Decimal), z.string(), z.number()])) + .otherwise(() => z.number()) + .optional(), + !!fieldDef.optional, + ), + increment: match(fieldDef.type) + .with('BigInt', () => z.bigint().or(z.number().int())) + .with('Decimal', () => z.union([z.instanceof(Decimal), z.string(), z.number()])) + .otherwise(() => z.number()) + .optional(), decrement: z.number().optional(), multiply: z.number().optional(), divide: z.number().optional(),packages/runtime/src/client/crud/validator/utils.ts (2)
121-141: BigInt thresholds parsed from number can be unsafe.Use string-or-number for the argument and parse via
BigInt(); numbers beyond 2^53-1 will be lossy.- const val = getArgValue<number>(attr.args?.[0]?.value); - if (val === undefined) { + const raw = attr.args?.[0]?.value; + const val = ExpressionUtils.isLiteral(raw) ? raw.value : undefined; + if (val === undefined || (typeof val !== 'number' && typeof val !== 'string')) { continue; } - const bigIntVal = BigInt(val); + const bigIntVal = BigInt(val as any);
61-64: Guard against ReDoS from untrusted regex patterns.
new RegExp(pattern)on user-provided patterns can be abused. Consider validating patterns (e.g., safe-regex) or limiting length/flags.Also applies to: 368-371
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
packages/language/res/stdlib.zmodel(1 hunks)packages/runtime/src/client/crud/dialects/postgresql.ts(1 hunks)packages/runtime/src/client/crud/dialects/sqlite.ts(1 hunks)packages/runtime/src/client/crud/validator/index.ts(8 hunks)packages/runtime/src/client/crud/validator/utils.ts(1 hunks)packages/runtime/src/client/query-builder.ts(1 hunks)packages/runtime/src/utils/type-utils.ts(1 hunks)packages/sdk/src/schema/schema.ts(1 hunks)tests/e2e/orm/client-api/type-coverage.test.ts(1 hunks)tests/e2e/orm/validation/custom-validation.test.ts(1 hunks)tests/e2e/orm/validation/nested.test.ts(1 hunks)tests/e2e/orm/validation/toplevel.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/orm/validation/toplevel.test.ts
- tests/e2e/orm/validation/nested.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
packages/runtime/src/client/crud/dialects/sqlite.tstests/e2e/orm/client-api/type-coverage.test.tspackages/language/res/stdlib.zmodelpackages/runtime/src/client/crud/dialects/postgresql.tstests/e2e/orm/validation/custom-validation.test.tspackages/runtime/src/client/crud/validator/index.tspackages/runtime/src/client/crud/validator/utils.tspackages/sdk/src/schema/schema.tspackages/runtime/src/client/query-builder.tspackages/runtime/src/utils/type-utils.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/orm/client-api/type-coverage.test.tstests/e2e/orm/validation/custom-validation.test.ts
**/schema.ts
📄 CodeRabbit inference engine (CLAUDE.md)
The generated TypeScript schema should be named
schema.ts
Files:
packages/sdk/src/schema/schema.ts
🧬 Code graph analysis (3)
tests/e2e/orm/validation/custom-validation.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
packages/runtime/src/client/crud/validator/index.ts (4)
packages/runtime/src/client/errors.ts (1)
InputValidationError(9-13)packages/runtime/src/utils/zod-utils.ts (1)
formatError(8-14)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/runtime/src/client/crud/validator/utils.ts (5)
addStringValidation(24-86)addNumberValidation(88-114)addBigIntValidation(116-143)addDecimalValidation(145-203)addCustomValidation(205-226)
packages/runtime/src/client/crud/validator/utils.ts (5)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/sdk/src/schema/expression.ts (5)
FieldExpression(22-25)UnaryExpression(33-37)BinaryExpression(39-44)MemberExpression(27-31)CallExpression(46-50)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)packages/runtime/src/client/errors.ts (1)
QueryError(18-22)
🪛 ast-grep (0.39.5)
packages/runtime/src/client/crud/validator/utils.ts
[warning] 62-62: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 369-369: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (1)
packages/runtime/src/utils/type-utils.ts (1)
1-1: No change needed for Decimal import
decimal.js v10.x supports both named and default imports, soimport type { Decimal } from 'decimal.js'is valid.Likely an incorrect or invalid review comment.
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (6)
packages/runtime/src/client/crud/validator/utils.ts (6)
99-111: ts-pattern match never executes: add.exhaustive()or.otherwise().Same issue as lines 31-83. None of the numeric validation handlers run without a terminal call.
Apply this fix:
match(attr.name) .with('@gt', () => { result = result.gt(val); }) .with('@gte', () => { result = result.gte(val); }) .with('@lt', () => { result = result.lt(val); }) - .with('@lte', () => { + .with('@lte', () => { result = result.lte(val); - }); + }) + .otherwise(() => undefined);
128-140: ts-pattern match never executes: add.exhaustive()or.otherwise().Same issue as lines 31-83. None of the BigInt validation handlers run without a terminal call.
Apply this fix:
match(attr.name) .with('@gt', () => { result = result.gt(bigIntVal); }) .with('@gte', () => { result = result.gte(bigIntVal); }) .with('@lt', () => { result = result.lt(bigIntVal); }) - .with('@lte', () => { + .with('@lte', () => { result = result.lte(bigIntVal); - }); + }) + .otherwise(() => undefined);
186-198: ts-pattern match never executes: add.exhaustive()or.otherwise().Same issue as lines 31-83. None of the Decimal validation handlers run without a terminal call.
Apply this fix:
match(attr.name) .with('@gt', () => { result = refine(result, 'gt', val); }) .with('@gte', () => { result = refine(result, 'gte', val); }) .with('@lt', () => { result = refine(result, 'lt', val); }) - .with('@lte', () => { + .with('@lte', () => { result = refine(result, 'lte', val); - }); + }) + .otherwise(() => undefined);
363-371: RegExp from user input: ReDoS risk.Same ReDoS vulnerability as line 62. Pattern comes from user input without validation.
Consider validating pattern safety:
.with('regex', (f) => { if (fieldArg === undefined || fieldArg === null) { return false; } invariant(typeof fieldArg === 'string', `"${f}" first argument must be a string`); const pattern = getArgValue<string>(expr.args?.[1])!; invariant(pattern !== undefined, `"${f}" requires a pattern argument`); // TODO: validate pattern safety before constructing RegExp return new RegExp(pattern).test(fieldArg); })Based on static analysis hints: [CWE-1333].
169-177: Decimal comparison loses precision: use Decimal arithmetic.Converting Decimal to number via
.toNumber()discards precision for large values and exponents. Despite past review feedback, this has not been fixed.Apply this fix to preserve precision:
- function refine(schema: z.ZodSchema, op: 'gt' | 'gte' | 'lt' | 'lte', value: number) { + function refine(schema: z.ZodSchema, op: 'gt' | 'gte' | 'lt' | 'lte', value: number | string) { + const threshold = new Decimal(value); return schema.superRefine((v, ctx) => { - const base = z.number(); - const { error } = base[op](value).safeParse((v as Decimal).toNumber()); - error?.errors.forEach((e) => { - ctx.addIssue(e); - }); + const dec = v as Decimal; + const cmp = dec.comparedTo(threshold); + const ok = + (op === 'gt' && cmp > 0) || + (op === 'gte' && cmp >= 0) || + (op === 'lt' && cmp < 0) || + (op === 'lte' && cmp <= 0); + if (!ok) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `Expected ${op} ${threshold.toString()}`, + }); + } }); }
31-83: ts-pattern match never executes: add.exhaustive()or.otherwise().The match chain is built but never finalized, so none of the validation handlers run. Despite a past review claiming this was fixed in c52b1df, the code still lacks a terminal call.
Apply this fix:
match(attr.name) .with('@length', () => { const min = getArgValue<number>(attr.args?.[0]?.value); if (min !== undefined) { result = result.min(min); } const max = getArgValue<number>(attr.args?.[1]?.value); if (max !== undefined) { result = result.max(max); } }) .with('@startsWith', () => { const value = getArgValue<string>(attr.args?.[0]?.value); if (value !== undefined) { result = result.startsWith(value); } }) .with('@endsWith', () => { const value = getArgValue<string>(attr.args?.[0]?.value); if (value !== undefined) { result = result.endsWith(value); } }) .with('@contains', () => { const value = getArgValue<string>(attr.args?.[0]?.value); if (value !== undefined) { result = result.includes(value); } }) .with('@regex', () => { const pattern = getArgValue<string>(attr.args?.[0]?.value); if (pattern !== undefined) { result = result.regex(new RegExp(pattern)); } }) .with('@email', () => { result = result.email(); }) .with('@datetime', () => { result = result.datetime(); }) .with('@url', () => { result = result.url(); }) .with('@trim', () => { result = result.trim(); }) .with('@lower', () => { result = result.toLowerCase(); }) - .with('@upper', () => { + .with('@upper', () => { result = result.toUpperCase(); - }); + }) + .otherwise(() => undefined); }
🧹 Nitpick comments (1)
tests/e2e/orm/validation/toplevel.test.ts (1)
117-142: Use BigInt literals consistently.Lines 135 and 138 correctly use BigInt literals (
1n,5n), but line 141 uses regular numbers for BigInt fields. While runtime coercion may handle this, using BigInt literals consistently improves test clarity and explicitly matches the field types.Apply this diff for consistency:
// satisfies all - await expect(db.foo.create({ data: { int1: 3, int2: 4 } })).toResolveTruthy(); + await expect(db.foo.create({ data: { int1: 3n, int2: 4n } })).toResolveTruthy();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/language/res/stdlib.zmodel(1 hunks)packages/runtime/src/client/contract.ts(1 hunks)packages/runtime/src/client/crud/validator/index.ts(8 hunks)packages/runtime/src/client/crud/validator/utils.ts(1 hunks)tests/e2e/orm/client-api/type-coverage.test.ts(1 hunks)tests/e2e/orm/validation/toplevel.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/language/res/stdlib.zmodel
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
tests/e2e/orm/validation/toplevel.test.tspackages/runtime/src/client/crud/validator/utils.tspackages/runtime/src/client/contract.tspackages/runtime/src/client/crud/validator/index.tstests/e2e/orm/client-api/type-coverage.test.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/orm/validation/toplevel.test.tstests/e2e/orm/client-api/type-coverage.test.ts
🧬 Code graph analysis (3)
tests/e2e/orm/validation/toplevel.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
packages/runtime/src/client/crud/validator/utils.ts (5)
packages/runtime/src/schema/expression.ts (1)
ExpressionUtils(16-120)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/sdk/src/schema/expression.ts (5)
FieldExpression(22-25)UnaryExpression(33-37)BinaryExpression(39-44)MemberExpression(27-31)CallExpression(46-50)packages/common-helpers/src/tiny-invariant.ts (1)
invariant(4-14)packages/runtime/src/client/errors.ts (1)
QueryError(18-22)
packages/runtime/src/client/crud/validator/index.ts (4)
packages/runtime/src/client/errors.ts (1)
InputValidationError(9-13)packages/runtime/src/utils/zod-utils.ts (1)
formatError(8-14)packages/sdk/src/schema/schema.ts (1)
AttributeApplication(39-42)packages/runtime/src/client/crud/validator/utils.ts (5)
addStringValidation(24-86)addNumberValidation(88-114)addBigIntValidation(116-143)addDecimalValidation(145-203)addCustomValidation(205-226)
🪛 ast-grep (0.39.5)
packages/runtime/src/client/crud/validator/utils.ts
[warning] 62-62: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 369-369: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(pattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (20.x, postgresql)
- GitHub Check: build-test (20.x, sqlite)
🔇 Additional comments (11)
tests/e2e/orm/client-api/type-coverage.test.ts (1)
1-1: LGTM!The import reordering improves code organization by grouping project-internal imports first.
tests/e2e/orm/validation/toplevel.test.ts (4)
1-4: LGTM!Imports are appropriate for validation testing, and the file location complies with the coding guidelines for e2e tests.
6-88: Excellent string validation coverage!The test comprehensively exercises all string validators across multiple CRUD actions with proper boundary testing and transformation verification. The distinction between updateMany's count response and other operations' object responses is correctly handled.
90-115: LGTM!Numeric validator boundary conditions are correctly tested with proper violation and acceptance cases.
144-172: LGTM!Decimal validation correctly tests numeric comparisons with diverse input formats (numbers, strings, Decimal objects) and properly validates both boundary violations and invalid decimal strings.
packages/runtime/src/client/contract.ts (1)
1-1: LGTM: Correct import style for decimal.js.The change from a named type import to a default type import is correct. Based on learnings, decimal.js exports Decimal as a default export, not a named export.
packages/runtime/src/client/crud/validator/utils.ts (1)
244-256: Good use of exhaustive ts-pattern matching.The expression evaluator correctly uses
.exhaustive()to ensure all cases are handled and the match chain executes.packages/runtime/src/client/crud/validator/index.ts (4)
2-2: LGTM: Correct default import for Decimal.The default import style is correct for decimal.js, consistent with the fix in contract.ts.
249-275: Good integration of attribute-based validation.The primitive schema builder now correctly accepts attributes and pipes them through validation helpers (addStringValidation, addNumberValidation, addBigIntValidation, addDecimalValidation). This addresses the past review concern that Float fields weren't validated.
202-209: Improved error messaging.The validation error now includes the model name, making debugging easier. The switch to destructured
{ error, data }from safeParse also improves clarity.
885-885: Good integration of field-level and model-level validation.Field attributes are correctly passed through
makePrimitiveSchema, and model-level custom validations are properly wrapped withaddCustomValidationfor both checked and unchecked schema variants.Also applies to: 914-926
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/orm/validation/toplevel.test.ts (1)
23-87: Consider extracting action test helper to reduce duplication.The action loop pattern (lines 23-87) is repeated across test cases and creates a somewhat complex nested structure. While functional, extracting this into a reusable test helper function would improve maintainability and readability, especially if similar patterns appear in other validation test files.
Example refactor:
// Helper function to test across all actions async function testActions(db: any, testCases: Array<{ data: any; shouldPass: boolean; errorMsgs?: string[] }>) { for (const action of ['create', 'update', 'upsert', 'updateMany']) { const _t = action === 'create' ? (data: any) => db.foo.create({ data }) : action === 'update' ? (data: any) => db.foo.update({ where: { id: 100 }, data }) : action === 'upsert' ? (data: any) => db.foo.upsert({ where: { id: 100 }, create: { id: 101, ...data }, update: data }) : (data: any) => db.foo.updateMany({ where: { id: 100 }, data }); for (const { data, shouldPass, errorMsgs } of testCases) { if (shouldPass) { await expect(_t(data)).toResolveTruthy(); } else { await expect(_t(data)).toBeRejectedByValidation(errorMsgs); } } } } // Usage: await testActions(db, [ { data: { str1: 'a' }, shouldPass: false }, // violates @length min { data: { str1: 'abcde' }, shouldPass: false }, // violates @length max { data: { str1: 'ambb' }, shouldPass: true }, // satisfies all // ... ]);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/e2e/orm/validation/toplevel.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place packages only under
packages/,samples/, ortests/
Files:
tests/e2e/orm/validation/toplevel.test.ts
tests/e2e/**
📄 CodeRabbit inference engine (CLAUDE.md)
End-to-end tests must live under
tests/e2e/
Files:
tests/e2e/orm/validation/toplevel.test.ts
🧬 Code graph analysis (1)
tests/e2e/orm/validation/toplevel.test.ts (1)
packages/testtools/src/client.ts (1)
createTestClient(51-167)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (20.x, sqlite)
- GitHub Check: build-test (20.x, postgresql)
🔇 Additional comments (1)
tests/e2e/orm/validation/toplevel.test.ts (1)
1-209: Comprehensive validation test coverage looks good!The test file provides thorough coverage of top-level field validators across multiple data types (String, Int, BigInt, Decimal) and CRUD operations. The test structure is well-organized, and the use of custom matchers (
toBeRejectedByValidation,toResolveTruthy) enhances readability. The relation field rejection tests correctly verify that validation expressions cannot access relation fields.
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
Tests
Chores