From b9e465b94e6215dd58846ac0911862e52dc337f9 Mon Sep 17 00:00:00 2001 From: Timothee Guerin Date: Tue, 13 Aug 2024 10:06:15 -0700 Subject: [PATCH] Merge Hotfix back into main (#4167) --- .github/workflows/consistency.yml | 6 ++-- eng/common/pipelines/ci.yml | 1 + packages/http/CHANGELOG.md | 7 +++++ packages/http/package.json | 2 +- packages/http/src/route.ts | 25 +++++++++++------ packages/openapi3/CHANGELOG.md | 7 +++++ packages/openapi3/package.json | 2 +- packages/openapi3/src/openapi.ts | 8 ++++-- packages/openapi3/test/metadata.test.ts | 2 ++ packages/openapi3/test/parameters.test.ts | 27 ++++++++++++++---- packages/openapi3/test/shared-routes.test.ts | 5 ++++ packages/rest/CHANGELOG.md | 7 +++++ packages/rest/package.json | 2 +- packages/rest/src/rest.ts | 10 +++++-- packages/rest/test/routes.test.ts | 28 ++++++++++++++++++- .../@typespec/openapi3/openapi.yaml | 2 ++ .../nullable/@typespec/openapi3/openapi.yaml | 1 + .../optional/@typespec/openapi3/openapi.yaml | 1 + .../@typespec/openapi3/openapi.yaml | 1 + .../petstore/@typespec/openapi3/openapi.yaml | 2 ++ .../petstore/@typespec/openapi3/openapi.yaml | 1 + .../@typespec/openapi3/openapi.yaml | 1 - 22 files changed, 119 insertions(+), 29 deletions(-) diff --git a/.github/workflows/consistency.yml b/.github/workflows/consistency.yml index 13d142a90..05421262e 100644 --- a/.github/workflows/consistency.yml +++ b/.github/workflows/consistency.yml @@ -24,13 +24,13 @@ jobs: - uses: ./.github/actions/setup - - run: git pull --force --no-tags origin main:main - name: Get main ref + - run: git pull --force --no-tags origin ${{ github.event.pull_request.base.ref }}:${{ github.event.pull_request.base.ref }} + name: Get ${{ github.event.pull_request.base.ref }} ref for ${{ github.ref}}, evt ${{ github.event_name }} - run: pnpm install name: Install dependencies - - run: npx chronus verify + - run: npx chronus verify --since ${{ github.event.pull_request.base.ref }} name: Check changelog if: | !startsWith(github.head_ref, 'publish/') && diff --git a/eng/common/pipelines/ci.yml b/eng/common/pipelines/ci.yml index 6f0689ecb..ae0841d79 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/CHANGELOG.md b/packages/http/CHANGELOG.md index c967b032d..d082a0fe9 100644 --- a/packages/http/CHANGELOG.md +++ b/packages/http/CHANGELOG.md @@ -1,5 +1,12 @@ # Change Log - @typespec/http +## 0.59.1 + +### Bug Fixes + +- [#4155](https://github.com/microsoft/typespec/pull/4155) HotFix: Uri template not correctly built when using `@autoRoute` + + ## 0.59.0 ### Bug Fixes diff --git a/packages/http/package.json b/packages/http/package.json index 0f9172773..92c6e0b54 100644 --- a/packages/http/package.json +++ b/packages/http/package.json @@ -1,6 +1,6 @@ { "name": "@typespec/http", - "version": "0.59.0", + "version": "0.59.1", "author": "Microsoft Corporation", "description": "TypeSpec HTTP protocol binding", "homepage": "https://github.com/microsoft/typespec", diff --git a/packages/http/src/route.ts b/packages/http/src/route.ts index 5a7a6e9be..25751507f 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/openapi3/CHANGELOG.md b/packages/openapi3/CHANGELOG.md index 6847ca353..aa12169fa 100644 --- a/packages/openapi3/CHANGELOG.md +++ b/packages/openapi3/CHANGELOG.md @@ -1,5 +1,12 @@ # Change Log - @typespec/openapi3 +## 0.59.1 + +### Bug Fixes + +- [#4168](https://github.com/microsoft/typespec/pull/4168) Fix: query params are `explode: true` by default in OpenAPI 3.0 + + ## 0.59.0 ### Bug Fixes diff --git a/packages/openapi3/package.json b/packages/openapi3/package.json index cda7bfc04..ee49a9812 100644 --- a/packages/openapi3/package.json +++ b/packages/openapi3/package.json @@ -1,6 +1,6 @@ { "name": "@typespec/openapi3", - "version": "0.59.0", + "version": "0.59.1", "author": "Microsoft Corporation", "description": "TypeSpec library for emitting OpenAPI 3.0 from the TypeSpec REST protocol binding and converting OpenAPI3 to TypeSpec", "homepage": "https://typespec.io", diff --git a/packages/openapi3/src/openapi.ts b/packages/openapi3/src/openapi.ts index cfc879713..b19882084 100644 --- a/packages/openapi3/src/openapi.ts +++ b/packages/openapi3/src/openapi.ts @@ -1456,8 +1456,9 @@ function createOAPIEmitter( function getQueryParameterAttributes(parameter: HttpOperationParameter & { type: "query" }) { const attributes: { style?: string; explode?: boolean } = {}; - if (parameter.explode) { - attributes.explode = true; + if (parameter.explode !== true) { + // For query parameters(style: form) the default is explode: true https://spec.openapis.org/oas/v3.0.2#fixed-fields-9 + attributes.explode = false; } switch (parameter.format) { @@ -1465,9 +1466,10 @@ function createOAPIEmitter( return { style: "spaceDelimited", explode: false }; case "pipes": return { style: "pipeDelimited", explode: false }; - case undefined: case "csv": case "simple": + return { explode: false }; + case undefined: case "multi": case "form": return attributes; diff --git a/packages/openapi3/test/metadata.test.ts b/packages/openapi3/test/metadata.test.ts index cf4e6d363..5b5130398 100644 --- a/packages/openapi3/test/metadata.test.ts +++ b/packages/openapi3/test/metadata.test.ts @@ -661,6 +661,7 @@ describe("openapi3: metadata", () => { "Parameters.q": { name: "q", in: "query", + explode: false, required: true, schema: { type: "string" }, }, @@ -717,6 +718,7 @@ describe("openapi3: metadata", () => { name: "q", in: "query", required: true, + explode: false, schema: { type: "string" }, }, { diff --git a/packages/openapi3/test/parameters.test.ts b/packages/openapi3/test/parameters.test.ts index 41d51218c..9de14a76f 100644 --- a/packages/openapi3/test/parameters.test.ts +++ b/packages/openapi3/test/parameters.test.ts @@ -31,18 +31,32 @@ describe("query parameters", () => { strictEqual(param.name, "$select"); }); - describe("set explode: true", () => { + describe("doesn't set explode if explode: true (Openapi3.0 inverse default)", () => { it("with option", async () => { const param = await getQueryParam(`op test(@query(#{explode: true}) myParam: string): void;`); - expect(param).toMatchObject({ - explode: true, - }); + expect(param).not.toHaveProperty("explode"); }); it("with uri template", async () => { const param = await getQueryParam(`@route("{?myParam*}") op test(myParam: string): void;`); + expect(param).not.toHaveProperty("explode"); + }); + }); + + describe("set explode: false if explode is not set", () => { + it("with option", async () => { + const param = await getQueryParam( + `op test(@query(#{explode: false}) myParam: string): void;` + ); expect(param).toMatchObject({ - explode: true, + explode: false, + }); + }); + + it("with uri template", async () => { + const param = await getQueryParam(`@route("{?myParam}") op test(myParam: string): void;`); + expect(param).toMatchObject({ + explode: false, }); }); }); @@ -66,7 +80,6 @@ describe("query parameters", () => { in: "query", name: "$multi", required: true, - explode: true, schema: { type: "array", items: { @@ -77,6 +90,7 @@ describe("query parameters", () => { deepStrictEqual(params[1], { in: "query", name: "$csv", + explode: false, schema: { type: "array", items: { @@ -134,6 +148,7 @@ describe("query parameters", () => { deepStrictEqual(res.paths["/"].get.parameters[0], { in: "query", name: "id", + explode: false, required: true, schema: { type: "string", diff --git a/packages/openapi3/test/shared-routes.test.ts b/packages/openapi3/test/shared-routes.test.ts index ed3775035..6c2807bd6 100644 --- a/packages/openapi3/test/shared-routes.test.ts +++ b/packages/openapi3/test/shared-routes.test.ts @@ -70,6 +70,7 @@ describe("openapi3: shared routes", () => { { in: "query", name: "resourceGroup", + explode: false, required: false, schema: { type: "string", @@ -78,6 +79,7 @@ describe("openapi3: shared routes", () => { { in: "query", name: "foo", + explode: false, required: true, schema: { type: "string", @@ -86,6 +88,7 @@ describe("openapi3: shared routes", () => { { in: "query", name: "subscription", + explode: false, required: false, schema: { type: "string", @@ -130,6 +133,7 @@ describe("openapi3: shared routes", () => { { in: "query", name: "filter", + explode: false, required: true, schema: { type: "string", @@ -176,6 +180,7 @@ describe("openapi3: shared routes", () => { name: "filter", in: "query", required: false, + explode: false, schema: { type: "string", enum: ["resourceGroup"], diff --git a/packages/rest/CHANGELOG.md b/packages/rest/CHANGELOG.md index 3f1e55fde..d3e69cc02 100644 --- a/packages/rest/CHANGELOG.md +++ b/packages/rest/CHANGELOG.md @@ -1,5 +1,12 @@ # Change Log - @typespec/rest +## 0.59.1 + +### Bug Fixes + +- [#4155](https://github.com/microsoft/typespec/pull/4155) HotFix: Uri template not correctly built when using `@autoRoute` + + ## 0.59.0 ### Bump dependencies diff --git a/packages/rest/package.json b/packages/rest/package.json index 7336b0a07..6a807e816 100644 --- a/packages/rest/package.json +++ b/packages/rest/package.json @@ -1,6 +1,6 @@ { "name": "@typespec/rest", - "version": "0.59.0", + "version": "0.59.1", "author": "Microsoft Corporation", "description": "TypeSpec REST protocol binding", "homepage": "https://typespec.io", diff --git a/packages/rest/src/rest.ts b/packages/rest/src/rest.ts index d4992ecf4..4858b040f 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 08819f839..5a14a2a92 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); + }); + }); +}); diff --git a/packages/samples/test/output/grpc-library-example/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/grpc-library-example/@typespec/openapi3/openapi.yaml index 68eaa504d..8f81ff352 100644 --- a/packages/samples/test/output/grpc-library-example/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/grpc-library-example/@typespec/openapi3/openapi.yaml @@ -270,6 +270,7 @@ components: schema: type: integer format: int32 + explode: false ListRequestBase.page_token: name: page_token in: query @@ -281,6 +282,7 @@ components: returned from the previous call to `ListShelves` method. schema: type: string + explode: false MergeShelvesRequest.name: name: name in: path diff --git a/packages/samples/test/output/nullable/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/nullable/@typespec/openapi3/openapi.yaml index 534cf40e9..1c82d2ee0 100644 --- a/packages/samples/test/output/nullable/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/nullable/@typespec/openapi3/openapi.yaml @@ -14,6 +14,7 @@ paths: schema: type: string nullable: true + explode: false responses: '200': description: The request has succeeded. diff --git a/packages/samples/test/output/optional/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/optional/@typespec/openapi3/openapi.yaml index f6ea73e68..87823a863 100644 --- a/packages/samples/test/output/optional/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/optional/@typespec/openapi3/openapi.yaml @@ -14,6 +14,7 @@ paths: schema: type: string default: defaultQueryString + explode: false responses: '200': description: The request has succeeded. diff --git a/packages/samples/test/output/param-decorators/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/param-decorators/@typespec/openapi3/openapi.yaml index 2a8f87b07..486b24128 100644 --- a/packages/samples/test/output/param-decorators/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/param-decorators/@typespec/openapi3/openapi.yaml @@ -23,6 +23,7 @@ paths: format: int32 minimum: 0 maximum: 10 + explode: false responses: '200': description: The request has succeeded. diff --git a/packages/samples/test/output/petstore/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/petstore/@typespec/openapi3/openapi.yaml index 0e25cb32b..970810016 100644 --- a/packages/samples/test/output/petstore/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/petstore/@typespec/openapi3/openapi.yaml @@ -15,6 +15,7 @@ paths: required: false schema: type: string + explode: false responses: '200': description: The request has succeeded. @@ -103,6 +104,7 @@ paths: required: true schema: type: string + explode: false responses: '200': description: The request has succeeded. diff --git a/packages/samples/test/output/rest/petstore/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/rest/petstore/@typespec/openapi3/openapi.yaml index 322293978..26bbe10d6 100644 --- a/packages/samples/test/output/rest/petstore/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/rest/petstore/@typespec/openapi3/openapi.yaml @@ -459,6 +459,7 @@ paths: required: true schema: type: string + explode: false responses: '200': description: The request has succeeded. diff --git a/packages/samples/test/output/visibility/@typespec/openapi3/openapi.yaml b/packages/samples/test/output/visibility/@typespec/openapi3/openapi.yaml index 85d990e5d..9161085fe 100644 --- a/packages/samples/test/output/visibility/@typespec/openapi3/openapi.yaml +++ b/packages/samples/test/output/visibility/@typespec/openapi3/openapi.yaml @@ -86,7 +86,6 @@ paths: type: array items: type: string - explode: true responses: '200': description: The request has succeeded.