fix(mock): apply override.mock.properties inside array items - #3471
Conversation
📝 WalkthroughWalkthroughNormalize resolveMockOverride path matching by stripping ChangesMock override array recursion fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression fixture and changes mock-override path matching so override.mock.properties applies consistently to properties inside array items (Issue #2465).
Changes:
- Added a new OpenAPI spec + orval test config entry for issue-2465 mock generation.
- Updated
resolveMockOverrideto normalize array path markers (.[]) when matching non-regex override keys. - Added unit tests for the resolver behavior and updated/added snapshots for generated outputs.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/specifications/issue-2465.yaml | New regression OpenAPI spec reproducing override behavior in array items. |
| tests/configs/mock.config.ts | Adds test config entry to generate mocks/models for issue-2465 with property overrides. |
| packages/mock/src/faker/resolvers/value.ts | Normalizes .[] path markers to make overrides apply within arrays. |
| packages/mock/src/faker/resolvers/value.test.ts | Adds resolver unit tests covering array marker normalization. |
| tests/snapshots/mock/issue-2465/* | New snapshots for generated models/endpoints for the regression fixture. |
| tests/snapshots/mock/petstore/endpoints.ts | Snapshot changed in an existing fixture (potentially impacted by new matching semantics). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function resolveMockOverride( | ||
| properties: Record<string, unknown> | undefined = {}, | ||
| item: OpenApiSchemaObject & { name: string; path?: string }, | ||
| ) { | ||
| const path = item.path ?? `#.${item.name}`; | ||
| // Strip `.[]` array-items markers so a bare property-name override applies | ||
| // wherever the property literally appears, including inside arrays (#2465). | ||
| // Regex keys still match against the original (un-stripped) path so users | ||
| // can opt into array-scoped targeting explicitly if ever needed. | ||
| const pathWithoutArrayMarkers = path.replaceAll('.[]', ''); | ||
| const property = Object.entries(properties).find(([key]) => { | ||
| if (isRegex(key)) { | ||
| const regex = new RegExp(key.slice(1, -1)); | ||
| if (regex.test(item.name) || regex.test(path)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| if (`#.${key}` === path) { | ||
| if (`#.${key.replaceAll('.[]', '')}` === pathWithoutArrayMarkers) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
The .[\] literal-key syntax isn't documented in mock.md or the config types, and no existing test or example uses it for array scoping. (The samples/vue-query/vue-query-basic '[].id' case is using the marker because that's where the path historically ended up under the broken matcher — the intent was "this operation's array items' id", which is what the normalized form expresses too.)
The documented array-scoped targeting is via regex (/firstName/ matches item.name regardless of array marker in path) — preserved untouched here. So the trade-off the PR makes is: bare keys are property-name-by-depth-modulo-arrays (the natural reading the issue reporter expected), and explicit array-scoping uses regex. Keeping the exact-equality check first would re-introduce the "users.[].firstName vs users.firstName gives different results" cliff that the defensive-symmetry comment in the source intentionally smooths over. Out of scope to revisit here.
| properties: | ||
| id: | ||
| type: string |
There was a problem hiding this comment.
Intentional — this fixture mirrors the issue body verbatim (#2465) so the regression test reproduces the exact scenario the reporter pasted, including the OAS-arguably-weird required placement in the second allOf item.
The override-matching bug being fixed here is orthogonal to allOf required-vs-optional semantics — cleaning up the spec wouldn't strengthen the regression (the override resolution path doesn't read required), and it would diverge from the reporter's example. The point of tests/__snapshots__/mock/issue-2465/ is to lock in that the user's faker overrides land correctly in both the single-object and array-of-objects responses, which the snapshot demonstrates regardless of the required-vs-optional shape. Leaving as-is.
| ]), | ||
| '@id': faker.string.alpha({ length: { min: 10, max: 30 } }), | ||
| id: faker.number.int({ min: 0, max: 100 }), | ||
| name: faker.string.alpha({ length: { min: 10, max: 30 } }), | ||
| name: 'jon', | ||
| tag: faker.string.alpha({ length: { min: 10, max: 30 } }), |
There was a problem hiding this comment.
Intentional and load-bearing — this is the user-visible proof of the fix on existing fixtures. The petstore mock config sets override.mock.properties = { name: 'jon' } (tests/configs/mock.config.ts:13); before this PR that override was applied to single-Pet endpoints (createPets, showPetById) but silently dropped for listPets (array of Pet). The flipped line is exactly that gap closing — same root cause as the issue reporter's case, just on the existing committed fixture. The PR description's "Before / after" block walks through this same diff for context.
For the focused-unit-test ask — already covered: packages/mock/src/faker/resolvers/value.test.ts has a dedicated case "matches a bare-key override when the property lives inside an array (#2465)" with path #.[].firstName + bare key firstName, which is the exact shape of the petstore flip (path #.[].name + bare key name).
…abs#2465) Bare property-name keys in `override.mock.properties` were silently dropped for properties living inside array-of-object responses (e.g. `firstName` in `array of User`), even though the same key matched on a single-object response. Root cause: in `packages/mock/src/faker/resolvers/value.ts` `resolveMockOverride`, the bare-key path is compared as exact equality `#.${key} === path`. The mock walker appends `.[]` to the path when descending into array items, so a property `firstName` inside an array ends up at `#.[].firstName`, which never equals `#.firstName` — the user's faker function was discarded and a default per-type faker was emitted instead. Fix: strip `.[]` array-items markers from both sides of the comparison before the bare-key equality check. Regex keys still see the original path so users can keep targeting array-scoped overrides via regex. Scope: non-array nested objects (e.g. `PetWithTag.pet.name`) are intentionally out of scope here and tracked as follow-up orval-labs#3470. User-visible proof: one line in `tests/__snapshots__/mock/petstore/endpoints.ts` flips — `listPets` items' `name` now resolves to `'jon'` (matching the petstore mock config's `properties: { name: 'jon' }`) instead of `faker.string.alpha(...)`. Tests: - `packages/mock/src/faker/resolvers/value.test.ts` — 7 unit cases (top-level / array-of-objects / nested array / `.[]` in key / non-array-nested-stays-unmatched / regex / no-match). - `tests/specifications/issue-2465.yaml` + new `issue2465` entry in `tests/configs/mock.config.ts` — regression spec mirroring the issue's exact OpenAPI shape (baseUser + retrievedUser (allOf) + retrievedUsers (array)).
Initial implementation in the previous commit normalized `.[]` substrings
only, which broke previously-working override keys with a leading `[]`
(no preceding dot) — e.g. `properties: { '[].id': ... }` at the
operation level, as used in `samples/vue-query/vue-query-basic`.
Switch to segment-based stripping: split on `.`, drop `[]` segments,
rejoin. Now `[].id`, `foo.[].id`, `foo.[].bar.[].id`, etc. all normalize
equivalently to the corresponding `.[]`-free form.
Regression test added in `resolveMockOverride` unit suite covering the
leading-bracket case explicitly.
Sample snapshot updates surface the orval-labs#2465 fix correctly applying
operation-level `id` overrides for array-of-object responses in
`angular-app`, `angular-query`, and `basic` (previously the override was
silently dropped, so the snapshots stored the default
`faker.number.int(...)`). The `basic` sample's `orval.config.ts` also
wraps its `id` override in a thunk so the generated value is stable
across regenerations (matches the pattern already used by `angular-app`,
`angular-query`, and `vue-query-basic`).
b49f6a8 to
3d8649b
Compare
Closes #2465
Follow-up: #3470 (non-array nested object case, intentionally out of scope)
Summary
Bare property-name keys in
override.mock.properties(e.g.{ firstName: () => faker.person.firstName() }) were silently dropped for properties living inside array-of-object responses, even though the same key correctly applied to single-object responses. This PR makes the array-items boundary transparent for bare-key override matching.Root cause
resolveMockOverrideinpackages/mock/src/faker/resolvers/value.tscompares a bare-key override via exact equality\#.${key}` === path. The mock walker appends.[]to the path when descending into array items (scalar.ts:294). SofirstNameinside an array's items ends up at path#.[].firstName, which never equals#.firstName— the user's faker function is discarded and the default per-type faker (faker.string.alpha(...)`) is emitted instead.What changed
packages/mock/src/faker/resolvers/value.ts: inresolveMockOverride, drop[]segments (split on., filter, rejoin) from both the schema path and the override key before the bare-key equality check. Segment-based stripping handles both[].id(leading) andfoo.[].id(embedded) marker forms equivalently. Regex keys still match against the original (un-normalized) path so users can keep targeting array-scoped overrides via regex.packages/mock/src/faker/resolvers/value.test.ts(new, 8 cases): top-level / inside-array / nested-array /.[]in key / leading[].in key (matchessamples/vue-query/vue-query-basicstyle) / non-array-nested intentionally stays unmatched / regex still works / no-match returns undefined.tests/specifications/issue-2465.yaml+ a newissue2465entry intests/configs/mock.config.tsmirroring the issue's exact OpenAPI shape (baseUser+retrievedUser(allOf) +retrievedUsers(array of retrievedUser)). The committed snapshot undertests/__snapshots__/mock/issue-2465/shows both single-object and array-of-objects responses correctly emitting the user-supplied faker calls.idoperation-level overrides insamples/angular-app,samples/angular-query, andsamples/basicwere previously dropped and are now applied; snapshots and generatedsrc/mirror the corrected behavior.tests/__snapshots__/mock/petstore/endpoints.tsflips one line (listPetsitems'nameresolves to'jon'instead offaker.string.alpha(...)).samples/basic/orval.config.tswraps itsidoverride in a thunk (id: () => faker.number.int(...)) so the generated value is stable across regenerations. Before the fix the override was being dropped so this latent eager-evaluation bug was invisible; surfacing the override surfaced the instability too. Matches the pattern already used by the other samples.Before / after (from the petstore snapshot, with
properties: { name: 'jon' })export const getListPetsResponseMock = (): Pets => Array.from( { length: faker.number.int({ min: 3, max: 5 }) }, (_, i) => i + 1, ).map(() => ({ ...faker.helpers.arrayElement([ { ...getListPetsResponseDogMock() }, { ...getListPetsResponseCatMock() }, ]), '@id': faker.string.alpha({ length: { min: 10, max: 30 } }), id: faker.number.int({ min: 0, max: 100 }), - name: faker.string.alpha({ length: { min: 10, max: 30 } }), + name: 'jon', tag: faker.string.alpha({ length: { min: 10, max: 30 } }),Single-object endpoints (
createPets,showPetById) were already producingname: 'jon'and remain unchanged.Out of scope
Non-array nested object properties (e.g.
PetWithTag.pet.name) still aren't matched by bare keys —#.pet.namenever equaled#.name, and this PR's[]segment stripping doesn't change that. The committed petstore snapshot'sgetShowPetWithOwnerResponseMock.pet.namestays asfaker.string.alpha(...), confirming the fix is narrowly scoped to the array case. That nested-object case has the same root design (bare-key = top-level path match) and is filed as #3470 — extending bare-key transparency to non-array nested objects has a wider blast radius (users currently relying on top-level-only matching may not want bare keys to apply to nested objects) and deserves a separate review.Test plan
bun run build— passbun run typecheck— passbun run lint— passbun --filter @orval/mock run test— 158/158 (incl. the newresolveMockOverrideregression suite)bun --filter orval-tests run build(typecheck-generated+verify:mock-generated) — passbun run test:snapshots(workspace, 76 tasks including all samples) — pass