diff --git a/.chronus/changes/fix-uri-template-autoroute-2024-7-12-23-14-1.md b/.chronus/changes/fix-uri-template-autoroute-2024-7-12-23-14-1.md new file mode 100644 index 0000000000..87ccf3e165 --- /dev/null +++ b/.chronus/changes/fix-uri-template-autoroute-2024-7-12-23-14-1.md @@ -0,0 +1,9 @@ +--- +# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking +changeKind: fix +packages: + - "@typespec/http" + - "@typespec/rest" +--- + +HotFix: Uri template not correctly built when using `@autoRoute` diff --git a/eng/common/pipelines/ci.yml b/eng/common/pipelines/ci.yml index 55939b3b64..2b71688f6a 100644 --- a/eng/common/pipelines/ci.yml +++ b/eng/common/pipelines/ci.yml @@ -7,6 +7,7 @@ pr: branches: include: - main + - release/* extends: template: /eng/common/pipelines/templates/1es-redirect.yml diff --git a/packages/http/src/route.ts b/packages/http/src/route.ts index 5a7a6e9be2..25751507fc 100644 --- a/packages/http/src/route.ts +++ b/packages/http/src/route.ts @@ -15,6 +15,7 @@ import { HttpOperation, HttpOperationParameter, HttpOperationParameters, + HttpOperationPathParameter, PathParameterOptions, RouteOptions, RoutePath, @@ -223,24 +224,30 @@ const styleToOperator: Record = { fragment: "#", }; -function addOperationTemplateToUriTemplate(uriTemplate: string, params: HttpOperationParameter[]) { - const pathParams = params - .filter((x) => x.type === "path") - .map((param) => { - const operator = param.allowReserved ? "+" : styleToOperator[param.style]; - return `{${operator}${param.name}${param.explode ? "*" : ""}}`; - }); +export function getUriTemplatePathParam(param: HttpOperationPathParameter) { + const operator = param.allowReserved ? "+" : styleToOperator[param.style]; + return `{${operator}${param.name}${param.explode ? "*" : ""}}`; +} + +export function addQueryParamsToUriTemplate(uriTemplate: string, params: HttpOperationParameter[]) { const queryParams = params.filter((x) => x.type === "query"); - const pathPart = joinPathSegments([uriTemplate, ...pathParams]); return ( - pathPart + + uriTemplate + (queryParams.length > 0 ? `{?${queryParams.map((x) => escapeUriTemplateParamName(x.name)).join(",")}}` : "") ); } +function addOperationTemplateToUriTemplate(uriTemplate: string, params: HttpOperationParameter[]) { + const pathParams = params.filter((x) => x.type === "path").map(getUriTemplatePathParam); + const queryParams = params.filter((x) => x.type === "query"); + + const pathPart = joinPathSegments([uriTemplate, ...pathParams]); + return addQueryParamsToUriTemplate(pathPart, queryParams); +} + function escapeUriTemplateParamName(name: string) { return name.replaceAll(":", "%3A"); } diff --git a/packages/rest/src/rest.ts b/packages/rest/src/rest.ts index d4992ecf4f..4858b040f8 100644 --- a/packages/rest/src/rest.ts +++ b/packages/rest/src/rest.ts @@ -12,11 +12,13 @@ import { Type, } from "@typespec/compiler"; import { + addQueryParamsToUriTemplate, DefaultRouteProducer, getOperationParameters, getOperationVerb, getRoutePath, getRouteProducer, + getUriTemplatePathParam, HttpOperation, HttpOperationParameter, HttpOperationParameters, @@ -119,7 +121,7 @@ function autoRouteProducer( ); for (const httpParam of parameters.parameters) { - const { type, param, name } = httpParam; + const { type, param } = httpParam; if (type === "path") { addSegmentFragment(program, param, segments); @@ -137,7 +139,7 @@ function autoRouteProducer( segments.push(`/${param.type.value}`); continue; // Skip adding to the parameter list } else { - segments.push(`/{${name}}`); + segments.push(`/${getUriTemplatePathParam(httpParam)}`); } } } @@ -155,8 +157,10 @@ function autoRouteProducer( // Add the operation's action segment if present addActionFragment(program, operation, segments); + const pathPart = joinPathSegments(segments); + return diagnostics.wrap({ - uriTemplate: joinPathSegments(segments), + uriTemplate: addQueryParamsToUriTemplate(pathPart, filteredParameters), parameters: { ...parameters, parameters: filteredParameters, diff --git a/packages/rest/test/routes.test.ts b/packages/rest/test/routes.test.ts index 08819f8392..5a14a2a92d 100644 --- a/packages/rest/test/routes.test.ts +++ b/packages/rest/test/routes.test.ts @@ -2,7 +2,7 @@ import { ModelProperty, Operation } from "@typespec/compiler"; import { expectDiagnostics } from "@typespec/compiler/testing"; import { isSharedRoute } from "@typespec/http"; import { deepStrictEqual, strictEqual } from "assert"; -import { describe, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { compileOperations, createRestTestRunner, @@ -521,3 +521,29 @@ describe("rest: routes", () => { ]); }); }); + +describe("uri template", () => { + async function getOp(code: string) { + const ops = await getOperations(code); + return ops[0]; + } + + describe("build uriTemplate from parameter", () => { + it.each([ + ["@path one: string", "/foo/{one}"], + ["@path(#{allowReserved: true}) one: string", "/foo/{+one}"], + ["@path(#{explode: true}) one: string", "/foo/{one*}"], + [`@path(#{style: "matrix"}) one: string`, "/foo/{;one}"], + [`@path(#{style: "label"}) one: string`, "/foo/{.one}"], + [`@path(#{style: "fragment"}) one: string`, "/foo/{#one}"], + [`@path(#{style: "path"}) one: string`, "/foo/{/one}"], + ["@path(#{allowReserved: true, explode: true}) one: string", "/foo/{+one*}"], + ["@query one: string", "/foo{?one}"], + // cspell:ignore Atwo + [`@query("one:two") one: string`, "/foo{?one%3Atwo}"], + ])("%s -> %s", async (param, expectedUri) => { + const op = await getOp(`@route("/foo") interface Test {@autoRoute op foo(${param}): void;}`); + expect(op.uriTemplate).toEqual(expectedUri); + }); + }); +});