fix: findFirst args validation issues #233
Conversation
WalkthroughUnifies single-record retrieval via a new findOne flag across find operations and validation. Updates validator API to accept options { unique, findOne } and enforce take = 1 for findOne. Adjusts tests to reflect validation of take and retrieval behavior. Introduces InputValidationError and minor call/style simplifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
Claude finished @ymc9's task —— View job PR Review:
|
There was a problem hiding this comment.
Pull Request Overview
This PR merges changes from the svetch/main branch that implement input validation for the findFirst operation to ensure it only accepts take: 1 parameter.
- Adds input validation to restrict
findFirstoperations totake: 1only - Updates validator to differentiate between
findOneandfindManyoperations - Refactors find operation handling to use a unified approach for single-result operations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/runtime/src/client/crud/validator.ts | Updates validation logic to restrict take parameter for findOne operations |
| packages/runtime/src/client/crud/operations/find.ts | Refactors find operations to use findOne flag and ensures take: 1 for single-result operations |
| packages/runtime/test/client-api/find.test.ts | Adds test cases to verify findFirst validation with InputValidationError |
| packages/runtime/test/client-api/computed-fields.test.ts | Fixes existing test to use valid take: 1 instead of invalid take: -1 |
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: 0
🧹 Nitpick comments (5)
packages/runtime/src/client/crud/validator.ts (2)
196-215: Enforcing take = 1 with findOne is correct; unify option typing and consider a clearer validation message.This correctly constrains
takewhenfindOneis true. Two small nits:
- Reuse the FindOptions alias here for consistency.
- Optional: add a custom refinement message so users don’t see a generic “expected 1” literal error.
Apply:
-private makeFindSchema(model: string, options: { unique: boolean; findOne: boolean }) { +private makeFindSchema(model: string, options: FindOptions) { @@ - if (options.findOne) { - fields['take'] = z.literal(1).optional(); - } else { - fields['take'] = this.makeTakeSchema().optional(); - } + fields['take'] = options.findOne + ? z.literal(1, { errorMap: () => ({ message: '"take" must be 1 when using findFirst/findOne' }) }).optional() + : this.makeTakeSchema().optional();Note: The custom message is optional; keep if you want a clearer DX.
43-47: Extract FindOptions alias and apply to both methods
Introduce atype FindOptions = Readonly<{ unique: boolean; findOne: boolean }>and use it invalidateFindArgsandmakeFindSchema. Verified no callers assume non-undefined returns.packages/runtime/test/client-api/find.test.ts (1)
57-60: Good coverage for the new take=1 rule on findFirst.Consider adding a negative-take case (e.g.,
take: -1) to ensure it also raisesInputValidationError.packages/runtime/src/client/crud/operations/find.ts (2)
13-19: Validation path update is correct; small naming nit.
parsedArgsisn’t actually the parsed Zod output (we return the original args). Consider renaming tovalidatedArgsorargsObjfor clarity.-let parsedArgs = validateArgs +let validatedArgs = validateArgs ? this.inputValidator.validateFindArgs(this.model, normalizedArgs, { unique: operation === 'findUnique', findOne, }) - : (normalizedArgs as FindArgs<Schema, GetModels<Schema>, true> | undefined); + : (normalizedArgs as FindArgs<Schema, GetModels<Schema>, true> | undefined);And update subsequent references.
20-24: Avoid mutating validated args; create a new object.Safer to not mutate the object that just passed validation.
-if (findOne) { - // ensure "limit 1" - parsedArgs = parsedArgs ?? {}; - parsedArgs.take = 1; -} +if (findOne) { + // ensure "limit 1" + validatedArgs = { ...(validatedArgs ?? {}), take: 1 } as FindArgs<Schema, GetModels<Schema>, true>; +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/runtime/src/client/crud/operations/find.ts(1 hunks)packages/runtime/src/client/crud/validator.ts(3 hunks)packages/runtime/test/client-api/computed-fields.test.ts(1 hunks)packages/runtime/test/client-api/find.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
{packages,samples,tests}/**
📄 CodeRabbit inference engine (CLAUDE.md)
Packages are located in
packages/,samples/, andtests/
Files:
packages/runtime/test/client-api/computed-fields.test.tspackages/runtime/src/client/crud/operations/find.tspackages/runtime/test/client-api/find.test.tspackages/runtime/src/client/crud/validator.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-test (20.x)
- GitHub Check: claude-review
🔇 Additional comments (3)
packages/runtime/test/client-api/computed-fields.test.ts (1)
76-79: LGTM: aligns with new findFirst semantics.Switching
takefrom -1 to 1 matches the enforcedfindOnebehavior.packages/runtime/test/client-api/find.test.ts (1)
3-3: Public error import updated correctly.Importing
InputValidationErrorreflects the new exported error type.packages/runtime/src/client/crud/operations/find.ts (1)
27-30: Result shaping is clear and correct.Single code path for read, then unwrap for
findOnekeeps behavior consistent.
Summary by CodeRabbit
New Features
Bug Fixes
Tests