Skip to content

Commit

Permalink
Fix: OpenAPI3 params are explode: true by default (#4168)
Browse files Browse the repository at this point in the history
fix #4165
  • Loading branch information
timotheeguerin authored Aug 13, 2024
1 parent 95e05ba commit 7797b72
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
# Change versionKind to one of: internal, fix, dependencies, feature, deprecation, breaking
changeKind: fix
packages:
- "@typespec/openapi3"
---

Fix: query params are `explode: true` by default in OpenAPI 3.0
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
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

0 comments on commit 7797b72

Please sign in to comment.