tsp-openapi3 - fixes query decorator arg generation (#4873)

Fixes #4298 Added tests for all the OpenAPI permutations of query params explode/style. This also updates tsp generation to be compatible with the change introduced in `@typespec/http` v0.59.1 where `@query` without params sets `explode: false` (previously it was not set - which assumed `explode: true`.) --------- Co-authored-by: Christopher Radek <[email protected]>
microsoft · Oct 31, 2024 · 012ec81 · 012ec81
1 parent b8462a0
commit 012ec81
Show file tree

Hide file tree

Showing 12 changed files with 382 additions and 47 deletions.
diff --git a/.chronus/changes/tsp-openapi3-fix-query-params-2024-9-25-14-20-22.md b/.chronus/changes/tsp-openapi3-fix-query-params-2024-9-25-14-20-22.md
@@ -0,0 +1,7 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/openapi3"
+---
+
+Updates tsp-openapi3 query decorator generation to use the value arguments.
diff --git a/packages/openapi3/src/cli/actions/convert/generators/generate-decorators.ts b/packages/openapi3/src/cli/actions/convert/generators/generate-decorators.ts
@@ -1,8 +1,10 @@
-import { TypeSpecDecorator } from "../interfaces.js";
+import { TSValue, TypeSpecDecorator } from "../interfaces.js";
 
 function generateDecorator({ name, args }: TypeSpecDecorator): string {
   const hasArgs = args.length;
-  const stringifiedArguments = hasArgs ? `(${args.map((a) => JSON.stringify(a)).join(", ")})` : "";
+  const stringifiedArguments = hasArgs
+    ? `(${args.map((a) => (isTSValue(a) ? a.value : JSON.stringify(a))).join(", ")})`
+    : "";
 
   return `@${name}${stringifiedArguments}`;
 }
@@ -11,3 +13,9 @@ export function generateDecorators(decorators: TypeSpecDecorator[]): string[] {
   const uniqueDecorators = new Set<string>(decorators.map(generateDecorator));
   return Array.from(uniqueDecorators);
 }
+
+function isTSValue(value: unknown): value is TSValue {
+  return Boolean(
+    value && typeof value === "object" && "__kind" in value && value["__kind"] === "value",
+  );
+}
diff --git a/packages/openapi3/src/cli/actions/convert/interfaces.ts b/packages/openapi3/src/cli/actions/convert/interfaces.ts
@@ -34,7 +34,16 @@ export interface TypeSpecServiceInfo {
 
 export interface TypeSpecDecorator {
   name: string;
-  args: (object | number | string)[];
+  args: (object | number | string | TSValue)[];
+}
+
+export interface TSValue {
+  __kind: "value";
+  /**
+   * String representation of the value.
+   * This will be directly substituted into the generated code.
+   */
+  value: string;
 }
 
 export interface TypeSpecAugmentation extends TypeSpecDecorator {

diff --git a/packages/openapi3/src/cli/actions/convert/utils/decorators.ts b/packages/openapi3/src/cli/actions/convert/utils/decorators.ts
@@ -1,6 +1,6 @@
 import { ExtensionKey } from "@typespec/openapi";
 import { Extensions, OpenAPI3Parameter, OpenAPI3Schema, Refable } from "../../../../types.js";
-import { TypeSpecDecorator } from "../interfaces.js";
+import { TSValue, TypeSpecDecorator } from "../interfaces.js";
 
 const validLocations = ["header", "query", "path"];
 const extensionDecoratorName = "extension";
@@ -45,35 +45,79 @@ function getLocationDecorator(parameter: OpenAPI3Parameter): TypeSpecDecorator |
   };
 
   let format: string | undefined;
+  let decoratorArgs: TypeSpecDecorator["args"][0] | undefined;
   switch (parameter.in) {
     case "header":
       format = getHeaderFormat(parameter.style);
       break;
     case "query":
-      format = getQueryFormat(parameter.explode, parameter.style);
+      decoratorArgs = getQueryArgs(parameter);
       break;
   }
 
   if (format) {
     decorator.args.push({ format });
   }
+  if (decoratorArgs) {
+    decorator.args.push(decoratorArgs);
+  }
 
   return decorator;
 }
 
-function getQueryFormat(explode?: boolean, style?: string): string | undefined {
-  if (explode) {
-    return "form";
-  } else if (style === "form") {
-    return "simple";
-  } else if (style === "spaceDelimited") {
-    return "ssv";
-  } else if (style === "pipeDelimited") {
-    return "pipes";
+function getQueryArgs(parameter: OpenAPI3Parameter): TSValue | undefined {
+  const queryOptions = getNormalizedQueryOptions(parameter);
+  if (Object.keys(queryOptions).length) {
+    return {
+      __kind: "value",
+      value: `#{${Object.entries(queryOptions)
+        .map(([key, value]) => {
+          return `${key}: ${JSON.stringify(value)}`;
+        })
+        .join(", ")}}`,
+    };
   }
+
   return;
 }
 
+type QueryOptions = { explode?: boolean; format?: string };
+
+function getNormalizedQueryOptions({
+  explode,
+  style = "",
+}: {
+  explode?: boolean;
+  style?: string;
+}): QueryOptions {
+  const queryOptions: QueryOptions = {};
+  // In OpenAPI 3, default style is 'form', and explode is true when 'form' is the style
+  if (typeof explode !== "boolean") {
+    if (style === "form" || !style) {
+      explode = true;
+    } else {
+      explode = false;
+    }
+  }
+
+  // Format only emits additional data if set to one of the following:
+  // ssv (spaceDelimited), pipes (pipeDelimited)
+  if (style === "spaceDelimited") {
+    queryOptions.format = "ssv";
+  } else if (style === "pipeDelimited") {
+    queryOptions.format = "pipes";
+  }
+
+  // In TypeSpec, default explode is "false"
+  if (!explode && queryOptions.format) {
+    queryOptions.explode = false;
+  } else if (explode) {
+    queryOptions.explode = true;
+  }
+
+  return queryOptions;
+}
+
 function getHeaderFormat(style?: string): string | undefined {
   return style === "simple" ? "simple" : undefined;
 }

diff --git a/packages/openapi3/test/tsp-openapi3/output/escaped-identifiers/main.tsp b/packages/openapi3/test/tsp-openapi3/output/escaped-identifiers/main.tsp
@@ -27,7 +27,7 @@ model `Escaped-Model` {
 model `get-thingDefaultResponse` {}
 
 @route("/{escaped-property}") @get op `get-thing`(
-  @query `weird@param`?: `Foo-Bar`,
+  @query(#{ explode: true }) `weird@param`?: `Foo-Bar`,
   ...Parameters.`Escaped-Model`.`escaped-property`,
   @bodyRoot body: `Escaped-Model`,
 ): `get-thingDefaultResponse`;

diff --git a/packages/openapi3/test/tsp-openapi3/output/param-decorators/main.tsp b/packages/openapi3/test/tsp-openapi3/output/param-decorators/main.tsp
@@ -42,7 +42,7 @@ model Operations_putThing200ApplicationJsonResponse {
 
   @minValue(0)
   @maxValue(10)
-  @query
+  @query(#{ explode: true })
   count: int32,
 ): Operations_getThing200ApplicationJsonResponse;
 

diff --git a/packages/openapi3/test/tsp-openapi3/output/petstore-sample/main.tsp b/packages/openapi3/test/tsp-openapi3/output/petstore-sample/main.tsp
@@ -89,7 +89,7 @@ op listPets(
   /**
    * How many items to return at one time (max 100)
    */
-  @query limit?: int32,
+  @query(#{ explode: true }) limit?: int32,
 ): listPets200ApplicationJsonResponse | listPetsDefaultApplicationJsonResponse;
 
 @tag("pets")

diff --git a/packages/openapi3/test/tsp-openapi3/output/petstore-swagger/main.tsp b/packages/openapi3/test/tsp-openapi3/output/petstore-swagger/main.tsp
@@ -539,10 +539,7 @@ op findPetsByStatus(
   /**
    * Status values that need to be considered for filter
    */
-  @query({
-    format: "form",
-  })
-  status?: "available" | "pending" | "sold" = "available",
+  @query(#{ explode: true }) status?: "available" | "pending" | "sold" = "available",
 ): findPetsByStatus200ApplicationXmlResponse | findPetsByStatus200ApplicationJsonResponse | findPetsByStatus400Response;
 
 /**
@@ -556,10 +553,7 @@ op findPetsByTags(
   /**
    * Tags to filter by
    */
-  @query({
-    format: "form",
-  })
-  tags?: string[],
+  @query(#{ explode: true }) tags?: string[],
 ): findPetsByTags200ApplicationXmlResponse | findPetsByTags200ApplicationJsonResponse | findPetsByTags400Response;
 
 @tag("pet")
@@ -606,12 +600,12 @@ op updatePetWithForm(
   /**
    * Name of pet that needs to be updated
    */
-  @query name?: string,
+  @query(#{ explode: true }) name?: string,
 
   /**
    * Status of pet that needs to be updated
    */
-  @query status?: string,
+  @query(#{ explode: true }) status?: string,
 ): updatePetWithForm405Response;
 
 @tag("pet")
@@ -627,7 +621,7 @@ op uploadFile(
   /**
    * Additional Metadata
    */
-  @query additionalMetadata?: string,
+  @query(#{ explode: true }) additionalMetadata?: string,
 
   @header contentType: "application/octet-stream",
   @bodyRoot body: bytes,
@@ -722,12 +716,12 @@ op loginUser(
   /**
    * The user name for login
    */
-  @query username?: string,
+  @query(#{ explode: true }) username?: string,
 
   /**
    * The password for login in clear text
    */
-  @query password?: string,
+  @query(#{ explode: true }) password?: string,
 ): loginUser200ApplicationXmlResponse | loginUser200ApplicationJsonResponse | loginUser400Response;
 
 @tag("user")