diff --git a/packages/fx-core/src/common/spec-parser/manifestUpdater.ts b/packages/fx-core/src/common/spec-parser/manifestUpdater.ts index e35d5da6f4..895731ade8 100644 --- a/packages/fx-core/src/common/spec-parser/manifestUpdater.ts +++ b/packages/fx-core/src/common/spec-parser/manifestUpdater.ts @@ -67,7 +67,7 @@ export function generateParametersFromSchema( title: updateFirstLetter(name), description: schema.description ?? "", }; - if (isRequired) { + if (isRequired && schema.default === undefined) { requiredParams.push(parameter); } else { optionalParams.push(parameter); @@ -123,10 +123,14 @@ export async function generateCommands( title: updateFirstLetter(param.name), description: param.description ?? "", }; - if (param.required) { - requiredParams.push(parameter); - } else { - optionalParams.push(parameter); + + const schema = param.schema as OpenAPIV3.SchemaObject; + if (param.in !== "header" && param.in !== "cookie") { + if (param.required && schema?.default === undefined) { + requiredParams.push(parameter); + } else { + optionalParams.push(parameter); + } } }); } diff --git a/packages/fx-core/src/common/spec-parser/utils.ts b/packages/fx-core/src/common/spec-parser/utils.ts index 12c7bbc84f..914042d27a 100644 --- a/packages/fx-core/src/common/spec-parser/utils.ts +++ b/packages/fx-core/src/common/spec-parser/utils.ts @@ -43,28 +43,30 @@ export function checkParameters(paramObject: OpenAPIV3.ParameterObject[]): Check for (let i = 0; i < paramObject.length; i++) { const param = paramObject[i]; + const schema = param.schema as OpenAPIV3.SchemaObject; + const isRequiredWithoutDefault = param.required && schema.default === undefined; + if (param.in === "header" || param.in === "cookie") { - if (param.required) { + if (isRequiredWithoutDefault) { paramResult.isValid = false; } continue; } - const schema = param.schema as OpenAPIV3.SchemaObject; if ( schema.type !== "boolean" && schema.type !== "string" && schema.type !== "number" && schema.type !== "integer" ) { - if (param.required) { + if (isRequiredWithoutDefault) { paramResult.isValid = false; } continue; } if (param.in === "query" || param.in === "path") { - if (param.required) { + if (isRequiredWithoutDefault) { paramResult.requiredNum = paramResult.requiredNum + 1; } else { paramResult.optionalNum = paramResult.optionalNum + 1; @@ -89,13 +91,15 @@ export function checkPostBody( return paramResult; } + const isRequiredWithoutDefault = isRequired && schema.default === undefined; + if ( schema.type === "string" || schema.type === "integer" || schema.type === "boolean" || schema.type === "number" ) { - if (isRequired) { + if (isRequiredWithoutDefault) { paramResult.requiredNum = paramResult.requiredNum + 1; } else { paramResult.optionalNum = paramResult.optionalNum + 1; @@ -113,7 +117,7 @@ export function checkPostBody( paramResult.isValid = paramResult.isValid && result.isValid; } } else { - if (isRequired) { + if (isRequiredWithoutDefault) { paramResult.isValid = false; } } diff --git a/packages/fx-core/tests/common/spec-parser/manifestUpdater.test.ts b/packages/fx-core/tests/common/spec-parser/manifestUpdater.test.ts index ff58038b7e..4b8a2e305c 100644 --- a/packages/fx-core/tests/common/spec-parser/manifestUpdater.test.ts +++ b/packages/fx-core/tests/common/spec-parser/manifestUpdater.test.ts @@ -547,6 +547,56 @@ describe("generateCommands", () => { ]); }); + it("should treat POST operation required parameter with default value as optional parameter", async () => { + const spec: any = { + paths: { + "/pets": { + post: { + operationId: "createPet", + summary: "Create a pet", + requestBody: { + content: { + "application/json": { + schema: { + type: "object", + required: ["name"], + properties: { + name: { + type: "string", + description: "Name of the pet", + default: "value", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + sinon.stub(fs, "pathExists").resolves(true); + + const [result, warnings] = await generateCommands(spec, adaptiveCardFolder, manifestPath); + expect(result).to.deep.equal([ + { + apiResponseRenderingTemplateFile: "adaptiveCards/createPet.json", + context: ["compose"], + id: "createPet", + parameters: [ + { + description: "Name of the pet", + name: "name", + title: "Name", + }, + ], + title: "Create a pet", + type: "query", + }, + ]); + expect(warnings).to.deep.equal([]); + }); + it("should not show warning for each GET/POST operation in the spec if only contains 1 optional parameters", async () => { const spec: any = { paths: { @@ -648,6 +698,62 @@ describe("generateCommands", () => { expect(warnings).to.deep.equal([]); }); + it("should treat required parameter with default value as optional parameter", async () => { + const spec: any = { + paths: { + "/pets": { + get: { + operationId: "getPets", + summary: "Get all pets", + parameters: [ + { + name: "limit", + in: "query", + description: "Maximum number of pets to return", + required: false, + }, + { + name: "id", + in: "query", + description: "ID of the pet", + required: true, + schema: { + type: "string", + default: "value", + }, + }, + ], + }, + }, + }, + }; + sinon.stub(fs, "pathExists").resolves(true); + + const expectedCommands = [ + { + context: ["compose"], + type: "query", + title: "Get all pets", + id: "getPets", + parameters: [ + { name: "limit", title: "Limit", description: "Maximum number of pets to return" }, + ], + apiResponseRenderingTemplateFile: "adaptiveCards/getPets.json", + }, + ]; + + const [result, warnings] = await generateCommands(spec, adaptiveCardFolder, manifestPath); + + expect(result).to.deep.equal(expectedCommands); + expect(warnings).to.deep.equal([ + { + type: WarningType.OperationOnlyContainsOptionalParam, + content: format(ConstantString.OperationOnlyContainsOptionalParam, "getPets"), + data: "getPets", + }, + ]); + }); + it("should generate commands for POST operation with string schema", async () => { const spec: any = { paths: { diff --git a/packages/fx-core/tests/common/spec-parser/utils.test.ts b/packages/fx-core/tests/common/spec-parser/utils.test.ts index e28cd64cf6..a4ba482de5 100644 --- a/packages/fx-core/tests/common/spec-parser/utils.test.ts +++ b/packages/fx-core/tests/common/spec-parser/utils.test.ts @@ -375,6 +375,63 @@ describe("utils", () => { assert.strictEqual(result, false); }); + it("should return true if method is POST, but requestBody contains unsupported parameter and required but has default value", () => { + const method = "POST"; + const path = "/users"; + const spec = { + paths: { + "/users": { + post: { + parameters: [ + { + in: "query", + required: true, + schema: { type: "string" }, + }, + ], + requestBody: { + content: { + "application/json": { + schema: { + type: "object", + required: ["name"], + properties: { + name: { + type: "array", + default: ["item"], + items: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + responses: { + 200: { + content: { + "application/json": { + schema: { + type: "object", + properties: { + name: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const result = isSupportedApi(method, path, spec as any); + assert.strictEqual(result, true); + }); + it("should return true if method is POST, path is valid, parameter is supported and only one required param in postBody", () => { const method = "POST"; const path = "/users"; @@ -525,7 +582,7 @@ describe("utils", () => { assert.strictEqual(result, false); }); - it("should return false if parameter is not supported", () => { + it("should return false if parameter is not supported and required", () => { const method = "GET"; const path = "/users"; const spec = { @@ -535,6 +592,7 @@ describe("utils", () => { parameters: [ { in: "query", + required: true, schema: { type: "object" }, }, ], @@ -562,6 +620,44 @@ describe("utils", () => { assert.strictEqual(result, false); }); + it("should ignore unsupported schema type with default value", () => { + const method = "GET"; + const path = "/users"; + const spec = { + paths: { + "/users": { + get: { + parameters: [ + { + in: "query", + required: true, + schema: { type: "object", default: { name: "test" } }, + }, + ], + responses: { + 200: { + content: { + "application/json": { + schema: { + type: "object", + properties: { + name: { + type: "string", + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const result = isSupportedApi(method, path, spec as any); + assert.strictEqual(result, false); + }); + it("should return false if parameter is in header and required supported", () => { const method = "GET"; const path = "/users"; @@ -743,6 +839,43 @@ describe("utils", () => { assert.strictEqual(result.isValid, false); }); + it("should valid if parameter in header or cookie is required but have default value", () => { + const paramObject = [ + { in: "query", required: true, schema: { type: "string" } }, + { in: "path", required: false, schema: { type: "string" } }, + { in: "header", required: true, schema: { type: "string", default: "value" } }, + ]; + const result = checkParameters(paramObject as OpenAPIV3.ParameterObject[]); + assert.strictEqual(result.isValid, true); + // header param is ignored + assert.strictEqual(result.requiredNum, 1); + assert.strictEqual(result.optionalNum, 1); + }); + + it("should treat required param with default value as optional param", () => { + const paramObject = [ + { in: "query", required: true, schema: { type: "string", default: "value" } }, + { in: "path", required: false, schema: { type: "string" } }, + { in: "query", required: true, schema: { type: "string" } }, + ]; + const result = checkParameters(paramObject as OpenAPIV3.ParameterObject[]); + assert.strictEqual(result.isValid, true); + assert.strictEqual(result.requiredNum, 1); + assert.strictEqual(result.optionalNum, 2); + }); + + it("should ignore required query param with default value and array type", () => { + const paramObject = [ + { in: "query", required: true, schema: { type: "string" } }, + { in: "path", required: false, schema: { type: "string" } }, + { in: "query", required: true, schema: { type: "array", default: ["item"] } }, + ]; + const result = checkParameters(paramObject as OpenAPIV3.ParameterObject[]); + assert.strictEqual(result.isValid, true); + assert.strictEqual(result.requiredNum, 1); + assert.strictEqual(result.optionalNum, 1); + }); + it("should ignore in header or cookie if is not required", () => { const paramObject = [ { in: "query", required: true, schema: { type: "string" } }, @@ -793,6 +926,23 @@ describe("utils", () => { assert.strictEqual(result.optionalNum, 0); }); + it("should treat required schema with default value as optional param", () => { + const schema = { + type: "object", + required: ["name"], + properties: { + name: { + type: "string", + default: "value", + }, + }, + }; + const result = checkPostBody(schema as any); + assert.strictEqual(result.requiredNum, 0); + assert.strictEqual(result.optionalNum, 1); + assert.strictEqual(result.isValid, true); + }); + it("should return 1 if the schema has a required string property", () => { const schema = { type: "object", @@ -868,6 +1018,26 @@ describe("utils", () => { const result = checkPostBody(schema as any); assert.strictEqual(result.isValid, false); }); + + it("should return valid for an unsupported schema type but it is required with default value", () => { + const schema = { + type: "object", + required: ["name"], + properties: { + name: { + type: "array", + default: ["item"], + items: { + type: "string", + }, + }, + }, + }; + const result = checkPostBody(schema as any); + assert.strictEqual(result.isValid, true); + assert.strictEqual(result.requiredNum, 0); + assert.strictEqual(result.optionalNum, 0); + }); }); describe("checkServerUrl", () => {