HotFix: Uri template not correctly built when using @autoRoute (#4155)

fix #4153
microsoft · Aug 13, 2024 · 9e1937c · 9e1937c
1 parent dda9db6
commit 9e1937c
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 13 deletions.
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
@@ -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
@@ -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
@@ -15,6 +15,7 @@ import {
   HttpOperation,
   HttpOperationParameter,
   HttpOperationParameters,
+  HttpOperationPathParameter,
   PathParameterOptions,
   RouteOptions,
   RoutePath,
@@ -223,24 +224,30 @@ const styleToOperator: Record<PathParameterOptions["style"], string> = {
   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
@@ -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
@@ -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);
+    });
+  });
+});