Skip to content

Commit

Permalink
Merge Hotfix back into main (#4167)
Browse files Browse the repository at this point in the history
  • Loading branch information
timotheeguerin authored Aug 13, 2024
1 parent 6567ec2 commit b9e465b
Show file tree
Hide file tree
Showing 22 changed files with 118 additions and 28 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/consistency.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/') &&
Expand Down
1 change: 1 addition & 0 deletions eng/common/pipelines/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pr:
branches:
include:
- main
- release/*

extends:
template: /eng/common/pipelines/templates/1es-redirect.yml
Expand Down
7 changes: 7 additions & 0 deletions packages/http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/http/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
25 changes: 16 additions & 9 deletions packages/http/src/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
HttpOperation,
HttpOperationParameter,
HttpOperationParameters,
HttpOperationPathParameter,
PathParameterOptions,
RouteOptions,
RoutePath,
Expand Down Expand Up @@ -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");
}
Expand Down
7 changes: 7 additions & 0 deletions packages/openapi3/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/openapi3/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
8 changes: 5 additions & 3 deletions packages/openapi3/src/openapi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1456,18 +1456,20 @@ 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) {
case "ssv":
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;
Expand Down
2 changes: 2 additions & 0 deletions packages/openapi3/test/metadata.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -661,6 +661,7 @@ describe("openapi3: metadata", () => {
"Parameters.q": {
name: "q",
in: "query",
explode: false,
required: true,
schema: { type: "string" },
},
Expand Down Expand Up @@ -717,6 +718,7 @@ describe("openapi3: metadata", () => {
name: "q",
in: "query",
required: true,
explode: false,
schema: { type: "string" },
},
{
Expand Down
25 changes: 20 additions & 5 deletions packages/openapi3/test/parameters.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).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;`);
const param = await getQueryParam(`@route("{?myParam}") op test(myParam: string): void;`);
expect(param).toMatchObject({
explode: true,
explode: false,
});
});
});
Expand All @@ -66,7 +80,6 @@ describe("query parameters", () => {
in: "query",
name: "$multi",
required: true,
explode: true,
schema: {
type: "array",
items: {
Expand All @@ -77,6 +90,7 @@ describe("query parameters", () => {
deepStrictEqual(params[1], {
in: "query",
name: "$csv",
explode: false,
schema: {
type: "array",
items: {
Expand Down Expand Up @@ -134,6 +148,7 @@ describe("query parameters", () => {
deepStrictEqual(res.paths["/"].get.parameters[0], {
in: "query",
name: "id",
explode: false,
required: true,
schema: {
type: "string",
Expand Down
5 changes: 5 additions & 0 deletions packages/openapi3/test/shared-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ describe("openapi3: shared routes", () => {
{
in: "query",
name: "resourceGroup",
explode: false,
required: false,
schema: {
type: "string",
Expand All @@ -78,6 +79,7 @@ describe("openapi3: shared routes", () => {
{
in: "query",
name: "foo",
explode: false,
required: true,
schema: {
type: "string",
Expand All @@ -86,6 +88,7 @@ describe("openapi3: shared routes", () => {
{
in: "query",
name: "subscription",
explode: false,
required: false,
schema: {
type: "string",
Expand Down Expand Up @@ -130,6 +133,7 @@ describe("openapi3: shared routes", () => {
{
in: "query",
name: "filter",
explode: false,
required: true,
schema: {
type: "string",
Expand Down Expand Up @@ -176,6 +180,7 @@ describe("openapi3: shared routes", () => {
name: "filter",
in: "query",
required: false,
explode: false,
schema: {
type: "string",
enum: ["resourceGroup"],
Expand Down
7 changes: 7 additions & 0 deletions packages/rest/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion packages/rest/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
10 changes: 7 additions & 3 deletions packages/rest/src/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,13 @@ import {
Type,
} from "@typespec/compiler";
import {
addQueryParamsToUriTemplate,
DefaultRouteProducer,
getOperationParameters,
getOperationVerb,
getRoutePath,
getRouteProducer,
getUriTemplatePathParam,
HttpOperation,
HttpOperationParameter,
HttpOperationParameters,
Expand Down Expand Up @@ -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);

Expand All @@ -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)}`);
}
}
}
Expand All @@ -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,
Expand Down
28 changes: 27 additions & 1 deletion packages/rest/test/routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ components:
schema:
type: integer
format: int32
explode: false
ListRequestBase.page_token:
name: page_token
in: query
Expand All @@ -281,6 +282,7 @@ components:
returned from the previous call to `ListShelves` method.
schema:
type: string
explode: false
MergeShelvesRequest.name:
name: name
in: path
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ paths:
schema:
type: string
nullable: true
explode: false
responses:
'200':
description: The request has succeeded.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ paths:
schema:
type: string
default: defaultQueryString
explode: false
responses:
'200':
description: The request has succeeded.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ paths:
format: int32
minimum: 0
maximum: 10
explode: false
responses:
'200':
description: The request has succeeded.
Expand Down
Loading

0 comments on commit b9e465b

Please sign in to comment.