Skip to content
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

[core-client-rest] Fix serialization of non-string path parameters #31352

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions sdk/core/core-client-rest/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### Bugs Fixed

- Allow `number` path parameters. PR [#31352](https://github.com/Azure/azure-sdk-for-js/pull/31352/files)

### Other Changes

## 2.3.0 (2024-10-03)
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-client-rest/review/core-client.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,15 @@ export interface OperationRequestOptions {

// @public
export type PathParameters<TRoute extends string> = TRoute extends `${infer _Head}/{${infer _Param}}${infer Tail}` ? [
pathParameter: string | PathParameterWithOptions,
pathParameter: string | number | PathParameterWithOptions,
...pathParameters: PathParameters<Tail>
] : [
];

// @public
export interface PathParameterWithOptions {
allowReserved?: boolean;
value: string;
value: string | number;
}

// @public
Expand Down
7 changes: 5 additions & 2 deletions sdk/core/core-client-rest/src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,10 @@ export type PathParameters<
// additional parameters we can call RouteParameters recursively on the Tail to match the remaining parts,
// in case the Tail has more parameters, it will return a tuple with the parameters found in tail.
// We spread the second path params to end up with a single dimension tuple at the end.
[pathParameter: string | PathParameterWithOptions, ...pathParameters: PathParameters<Tail>]
[
pathParameter: string | number | PathParameterWithOptions,
...pathParameters: PathParameters<Tail>,
]
: // When the path doesn't match the template, it means that we have no path parameters so we return
// an empty tuple.
[];
Expand Down Expand Up @@ -438,7 +441,7 @@ export interface PathParameterWithOptions {
/**
* The value of the parameter.
*/
value: string;
value: string | number;

/**
* Whether to allow for reserved characters in the value. If set to true, special characters such as '/' in the parameter's value will not be URL encoded.
Expand Down
11 changes: 5 additions & 6 deletions sdk/core/core-client-rest/src/urlHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function isQueryParameterWithOptions(x: unknown): x is QueryParameterWithOptions
export function buildRequestUrl(
endpoint: string,
routePath: string,
pathParameters: (string | PathParameterWithOptions)[],
pathParameters: (string | number | PathParameterWithOptions)[],
options: RequestParameters = {},
): string {
if (routePath.startsWith("https://") || routePath.startsWith("http://")) {
Expand Down Expand Up @@ -187,19 +187,18 @@ export function buildBaseUrl(endpoint: string, options: RequestParameters): stri

function buildRoutePath(
routePath: string,
pathParameters: (string | PathParameterWithOptions)[],
pathParameters: (string | number | PathParameterWithOptions)[],
options: RequestParameters = {},
): string {
for (const pathParam of pathParameters) {
const allowReserved =
typeof pathParam === "string" ? false : (pathParam?.allowReserved ?? false);
let value = typeof pathParam === "string" ? pathParam : pathParam?.value;
const allowReserved = typeof pathParam === "object" && (pathParam.allowReserved ?? false);
let value = typeof pathParam === "object" ? pathParam.value : pathParam;

if (!options.skipUrlEncoding && !allowReserved) {
value = encodeURIComponent(value);
}

routePath = routePath.replace(/\{\w+\}/, value);
routePath = routePath.replace(/\{\w+\}/, String(value));
}
return routePath;
}
Expand Down
6 changes: 6 additions & 0 deletions sdk/core/core-client-rest/test/urlHelpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,12 @@ describe("urlHelpers", () => {
assert.equal(result, `https://example.org/foo/one`);
});

it("should append number path parameters", () => {
timovv marked this conversation as resolved.
Show resolved Hide resolved
const result = buildRequestUrl(mockBaseUrl, "/foo/{id}", [12345]);

assert.equal(result, "https://example.org/foo/12345");
});

it("should append path, fill path parameters and append query parameters", () => {
const result = buildRequestUrl(mockBaseUrl, "/foo/{id}", ["one"], {
queryParameters: { foo: "1", bar: "two" },
Expand Down