Copilot review mirror for orval-labs/orval#3717 (issue #3710) — do not merge#9
Open
the-ult wants to merge 9 commits into
Open
Copilot review mirror for orval-labs/orval#3717 (issue #3710) — do not merge#9the-ult wants to merge 9 commits into
the-ult wants to merge 9 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Orval’s Angular httpResource generator to support a request-descriptor “extension” (extra headers, HttpContext, and a request transform hook) and adds regression tests/snapshots to pin behavior, including multi-content-type Accept precedence.
Changes:
- Emit
OrvalHttpResourceRequestExtension+applyOrvalRequestExtensionhelpers and thread them through generatedhttpResourcefactories (including multi-content-type branches). - Fix required
Signal<>parameter optionality forQUERY_PARAM/BODY/HEADERprops by avoiding thedefault: falsesentinel being treated as a real default. - Add/expand generator tests, sample app runtime tests, docs, and new snapshots/specs/configs to cover the feature and regressions.
Reviewed changes
Copilot reviewed 53 out of 53 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/specifications/angular-http-resource-request-extension.yaml | New OpenAPI fixture to reproduce orval-labs#3710 (required header + multi content-types). |
| tests/configs/angular.config.ts | Adds Angular generation configs for new httpResource header/extension coverage. |
| tests/api-generation.spec.ts | Adds substring-based test asserting the new request-extension surface is emitted/applied. |
| tests/snapshots/angular/url-encode-parameters-http-resource/endpoints.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/issue-3624/petstore.client.service.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/http-resource-zod/endpoints.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/http-resource-zod-disabled/endpoints.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/http-resource-tags/pets.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/http-resource-tags/health.ts | Snapshot updates for request-extension helpers and application in generated factories. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/model/itemEvents.ts | New snapshot model outputs for the multi-content extension fixture. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/model/itemEvent.ts | New snapshot model outputs for the multi-content extension fixture. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/model/index.ts | New snapshot model index for the multi-content extension fixture. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/model/getItemEventsHeaders.ts | New snapshot for generated in: header type. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/model/error.ts | New snapshot model output for error schema. |
| tests/snapshots/angular/http-resource-request-extension-multi-content/endpoints.ts | New snapshot endpoints showing Accept re-application after extension. |
| tests/snapshots/angular/http-resource-headers/model/petWithTag.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/pets.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/petCountry.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/petCallingCode.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/pet.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/listPetsXExample.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/listPetsSort.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/listPetsParams.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/listPetsHeaders.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/labradoodleBreed.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/labradoodle.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/index.ts | New snapshot model index for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/error.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/dogType.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/dog.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/dachshundBreed.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/dachshund.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/createPetsXExample.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/createPetsSort.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/createPetsParams.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/createPetsHeaders.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/createPetsBody.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/catType.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/model/cat.ts | New snapshot model outputs for httpResource headers mode. |
| tests/snapshots/angular/http-resource-headers/endpoints.ts | New snapshot endpoints exercising output.headers: true + extension helpers. |
| tests/snapshots/angular/http-resource-both-tags-split/pets/pets.resource.ts | Snapshot updates for request-extension helpers in split resource output. |
| tests/snapshots/angular/http-resource-both-tags-split/health/health.resource.ts | Snapshot updates for request-extension helpers in split resource output. |
| samples/angular-app/src/app/http-resource.page.spec.ts | Adds sample-app integration tests for Accept precedence + reactive header recomputation. |
| samples/angular-app/src/app/http-resource-request-extension.spec.ts | Adds runtime unit tests for applyOrvalRequestExtension behavior/merging. |
| samples/angular-app/src/api/http-resource/pets/pets.service.ts | Generated sample API updated to include extension helpers and new factory wiring. |
| samples/angular-app/src/api/http-resource-zod/pets/pets.service.ts | Generated sample Zod API updated to include extension helpers and new factory wiring. |
| samples/angular-app/src/api/http-both/pets/pets.resource.ts | Generated “both” sample updated to include extension helpers and new factory wiring. |
| samples/angular-app/snapshots/api/http-resource/pets/pets.service.ts | Snapshot update for generated sample httpResource output. |
| samples/angular-app/snapshots/api/http-resource-zod/pets/pets.service.ts | Snapshot update for generated sample httpResource+Zod output. |
| samples/angular-app/snapshots/api/http-both/pets/pets.resource.ts | Snapshot update for generated sample both-mode output. |
| packages/angular/src/http-resource.ts | Generator implementation updates: extension helpers, multi-content factory body, required-signal fix, import handling. |
| packages/angular/src/http-resource.test.ts | Generator tests expanded for orval-labs#3710 behavior, required-signal regression, and import routing. |
| docs/content/docs/guides/angular.mdx | Documentation for new request-extension surface and precedence. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d object keys (orval-labs#3692) * fix(core,zod): escape spec-controlled strings in generated template literals Unescaped spec values (servers[].url, path segments, schema defaults) were baked into generated template literals, allowing code injection via backtick or ${ in an attacker-controlled OpenAPI spec. Uses jsesc with quotes: 'backtick' to escape at three boundaries: - getRoute(): raw OpenAPI path before param processing - getFullRoute(): resolved server URL after variable substitution - formatDefaultValue(): schema default values in zod generation Addresses: GHSA-88f2-fpv8-89q2, GHSA-w727-8j6c-2rj4, GHSA-3575-w9fc-c2j6, GHSA-2h9g-j24r-h63g, GHSA-8j6p-r8jg-mxqh, GHSA-p4cg-3328-rvfg * fix(zod): escape object keys via JSON.stringify in zod.object generation Unescaped schema property and parameter names were emitted as double-quoted keys in zod.object({...}), allowing computed property key injection via " in the name. Replaces "${key}" with JSON.stringify(key) at all 5 render sites. Addresses: GHSA-6437-gxhq-pqv8, GHSA-653q-5476-x79g, GHSA-6mr6-jvcr-2f25 * fix(core): escape single-quoted object keys in getKey via jsStringLiteralEscape Unescaped schema property names were wrapped in single quotes by getKey(), allowing computed property key injection via ' in the name in MSW mock output. Wraps the key body with jsStringLiteralEscape before quoting. Addresses: GHSA-2w86-xfrc-g85r * fix(query,zod): escape raw route prefix and prove backtick-in-key safety mutation-generator.ts passed a raw spec path prefix to getFullRoute, bypassing getRoute's escaping. Wrap with getRoute to close the gap. Add test proving JSON.stringify-wrapped object keys are safe against backtick injection (backtick is harmless inside double-quoted strings). * fix: escape object-default keys and add ${} path test - Escape object-default keys with JSON.stringify in zod schema defaults (same fix as zod.object keys) - Add test verifying ${globalThis.X} in path segments is not re-interpreted as interpolation - Update existing test expectations for quoted object keys * fix(core): prevent ${} re-interpretation in getRoutePath and escape single quotes in getRouteAsArray getRoutePath: after jsesc escapes ${ to \${, the remaining {evil} was mistaken for an OpenAPI path param and re-converted to ${evil}. Add early-return when { is preceded by $. getRouteAsArray: segments wrapped in single-quoted strings without escaping '. A spec path containing ' would break out. Now escapes single quotes at both wrap sites. * fix(core): handle ${} in getRoutePath and escaped tags in getRouteAsArray getRoutePath: skip past ${...} block and continue processing remaining suffix so later {param} segments are still converted. getRouteAsArray: add (?<!\) to split/match regexes so jsesc-escaped ${...} is treated as literal text, preventing standalone backslash from breaking single-quote wrapping. * fix(core): escape per-segment in getRoute to preserve params after ${...}
…#3685) (orval-labs#3696) The axios generator used a module-level Map (returnTypesToWrite) to collect *Result type declarations during implementation generation, then read them during footer generation. In tags-split mode, all implementations are generated first (populating the map), then footers are generated per-tag. When two operations from different tags shared the same operationName (via override), the second overwrote the first's entry, causing the wrong *Result type to be emitted in the first tag's footer. Fix: eliminate the module-level map entirely. The return type generator is now returned from generateAxiosImplementation as part of GeneratorClient.returnType, stored on GeneratorOperation.types.result, and passed to the footer via the new optional operations param on ClientFooterBuilder/GeneratorClientFooter. The footer reads from operations[].types.result instead of the side-effect map, ensuring each tag's footer only emits types for its own operations.
…orval-labs#3700) (orval-labs#3701) An optional OpenAPI request body maps to an optional `Signal` parameter on the generated `httpResource` factory. Previously the factory accessed it unconditionally (`body: bodySignal?.()`), so omitting the signal still produced a request object with an undefined body and eagerly fired the resource instead of leaving it idle. Emit an idle guard at the top of the request factory (`if (!bodySignal) return undefined;`) for optional direct request bodies, matching Angular's `undefined`-request contract. Past the guard the signal is known to be defined, so it is invoked directly (`body: bodySignal()`). The guard is only emitted on the single-content path, where the request is built lazily inside the factory. The multi-content path assembles the request eagerly at the function-body level (where returning `undefined` would violate the `HttpResourceRef` return type) and keeps the runtime-safe `?.()` form. Co-authored-by: The Ult <the-ult@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…content-type resources (orval-labs#3715) * fix(angular): read signals inside the httpResource factory for multi-content-type resources The multi-content-type httpResource template built the request descriptor (path/query/version/body signal reads, normalizeRequest, Accept-header merge) eagerly, before httpResource() was called, and the factory only spread the pre-resolved object. Angular's httpResource tracks only signal reads that happen during factory execution, so these resources fetched once at creation and never refetched when their input signals changed. The request construction now lives in a buildRequest closure passed as the factory, matching the single-content-type output; accept-based branch selection stays eager. Also guards withSignal's default-detection against core's `default: false` boolean sentinel for query/body/header props, which wrongly rendered spec-required Signal parameters as optional (and produced invalid TS1016 overloads when combined with a trailing required accept parameter). Required query-param signals are now correctly non-optional — a compile-time tightening for consumers whose specs mark query params required. Fixes orval-labs#3713 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(angular): dispatch blob accept values to httpResource.blob in multi-content resources The multi-content accept dispatch covered json/text/arrayBuffer but never selected the blob factory, even though blob overloads are generated and blobType participates in the fallback chain — accept values like image/png fell through to the default branch and created the wrong resource kind. Emit a conditional blob branch (accept startsWith image/ or includes blob), mirroring the arrayBuffer branch and getHttpResourceFactory priority. Addresses Copilot review on orval-labs#3715. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: The Ult <the-ult@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
… in arrayItems factories (orval-labs#3709) * fix(mock): use exported element aliases for top-level array responses in arrayItems factories (orval-labs#3706) getArrayItemFactoryNames fabricated `${pascal(operationId)}${pascal(propertyName)}Item` for top-level array responses (no parentName), producing type names the schema generator never emits — unexported/undeclared identifiers that fail tsc. Now the mock generator derives the real emitted alias instead: for inline top-level arrays, reuse the alias before the trailing `[]` (bailing on non-identifier bases like unions or `readonly`); for `$ref`'d array schemas, reuse `<RefName><itemSuffix>` (bailing when the items are a multi-ref `allOf` with no direct properties, since core emits no alias there). All previously-working branches (itemsRef, parentName) are byte-identical. Fixes orval-labs#3706 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * style(mock): apply formatter to array-item-factory tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(mock): derive element aliases for nullable top-level array responses Copilot review: nullable top-level arrays reach the no-parentName branch with a ' | null'-suffixed definition ('CatalogItems | null'), which fell into the bare-ref-name path and fabricated phantom names. The suffix is now stripped before branch selection, and the bare-name branch gained an identifier guard that bails to the always-correct inline body for any non-identifier shape. Fixture gains a nullable top-level array operation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: The Ult <the-ult@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tpResource helpers Generated httpResource helpers only accepted HttpResourceOptions, while the generated HttpClient service methods already accept headers/context via HttpClientOptions. The trailing options bag now also accepts an OrvalHttpResourceRequestExtension: - headers: extra request headers (static or reactive function form), merged over generated ones via an emitted mergeOrvalResourceHeaders helper - context: Angular HttpContext for interceptor policy (static or function) - request: escape-hatch callback over the final HttpResourceRequest All extension reads happen inside the httpResource reactive factory, so signal-driven headers/context retrigger requests. The generated Accept header is always re-applied last in multi-content-type helpers. OpenAPI in:header parameters were already supported via output.headers; this adds test coverage and documentation for that path. Also fixes two latent generator bugs surfaced by the new coverage: required body/query/header props rendered as optional Signals (core's `default: false` sentinel), and the both-mode extra-file writer misrouting external imports through the schema-import bucket. Fixes orval-labs#3710 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…er calls The extension rides on the trailing options parameter; multi-content helpers and output.headers operations generate accept/header arguments before it. Show both call shapes with the real petstore-sample signature. Addresses Copilot review on orval-labs#3717. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ster Folds request-extension application into the reactive buildRequest closure (from orval-labs#3715's signal-inside-factory fix) for multi-content-type resources, and updates the sample/snapshot fixtures accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…quest merge Rebasing folded the request-extension application into orval-labs#3715's shared buildRequest closure, so the multi-content-type template no longer emits a separate acceptHeaders local — the Accept-header merge is inlined in buildRequest's return statement instead. Update the assertion to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
the-ult
force-pushed
the
feat/3710-angular-httpresource-request-descriptors
branch
from
July 12, 2026 14:21
9c8316f to
9980522
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Mirror of orval-labs#3717 for requesting a Copilot review. Do not merge.