perf(spec-parser-tdp): list all params rather than just one (#11219)

Co-authored-by: turenlong <rentu@microsoft.com>
OfficeDev · Mar 29, 2024 · bad4111 · bad4111
1 parent 4cb5c08
commit bad4111
Show file tree

Hide file tree

Showing 5 changed files with 62 additions and 37 deletions.
diff --git a/packages/spec-parser/src/manifestUpdater.ts b/packages/spec-parser/src/manifestUpdater.ts
@@ -5,7 +5,14 @@
 import { OpenAPIV3 } from "openapi-types";
 import fs from "fs-extra";
 import path from "path";
-import { AuthInfo, ErrorType, ParseOptions, ProjectType, WarningResult } from "./interfaces";
+import {
+  AuthInfo,
+  ErrorType,
+  ParseOptions,
+  ProjectType,
+  WarningResult,
+  WarningType,
+} from "./interfaces";
 import { Utils } from "./utils";
 import { SpecParserError } from "./specParserError";
 import { ConstantString } from "./constants";
@@ -297,7 +304,25 @@ export class ManifestUpdater {
             if (options.allowMethods?.includes(method)) {
               const operationItem = (operations as any)[method];
               if (operationItem) {
-                const [command, warning] = Utils.parseApiInfo(operationItem, options);
+                const command = Utils.parseApiInfo(operationItem, options);
+
+                if (
+                  command.parameters &&
+                  command.parameters.length >= 1 &&
+                  command.parameters.some((param) => param.isRequired)
+                ) {
+                  command.parameters = command.parameters.filter((param) => param.isRequired);
+                } else if (command.parameters && command.parameters.length > 0) {
+                  command.parameters = [command.parameters[0]];
+                  warnings.push({
+                    type: WarningType.OperationOnlyContainsOptionalParam,
+                    content: Utils.format(
+                      ConstantString.OperationOnlyContainsOptionalParam,
+                      command.id
+                    ),
+                    data: command.id,
+                  });
+                }
 
                 if (adaptiveCardFolder) {
                   const adaptiveCardPath = path.join(adaptiveCardFolder, command.id + ".json");
@@ -306,10 +331,6 @@ export class ManifestUpdater {
                     : "";
                 }
 
-                if (warning) {
-                  warnings.push(warning);
-                }
-
                 commands.push(command);
               }
             }

diff --git a/packages/spec-parser/src/specParser.browser.ts b/packages/spec-parser/src/specParser.browser.ts
@@ -109,7 +109,7 @@ export class SpecParser {
           continue;
         }
 
-        const [command, warning] = Utils.parseApiInfo(pathObjectItem, this.options);
+        const command = Utils.parseApiInfo(pathObjectItem, this.options);
 
         const apiInfo: APIInfo = {
           method: method,
@@ -120,10 +120,6 @@ export class SpecParser {
           description: command.description!,
         };
 
-        if (warning) {
-          apiInfo.warning = warning;
-        }
-
         apiInfos.push(apiInfo);
       }
 

diff --git a/packages/spec-parser/src/utils.ts b/packages/spec-parser/src/utils.ts
@@ -639,7 +639,7 @@ export class Utils {
   static parseApiInfo(
     operationItem: OpenAPIV3.OperationObject,
     options: ParseOptions
-  ): [IMessagingExtensionCommand, WarningResult | undefined] {
+  ): IMessagingExtensionCommand {
     const requiredParams: IParameter[] = [];
     const optionalParams: IParameter[] = [];
     const paramObject = operationItem.parameters as OpenAPIV3.ParameterObject[];
@@ -689,13 +689,7 @@ export class Utils {
 
     const operationId = operationItem.operationId!;
 
-    const parameters = [];
-
-    if (requiredParams.length !== 0) {
-      parameters.push(...requiredParams);
-    } else {
-      parameters.push(optionalParams[0]);
-    }
+    const parameters = [...requiredParams, ...optionalParams];
 
     const command: IMessagingExtensionCommand = {
       context: ["compose"],
@@ -708,16 +702,7 @@ export class Utils {
         ConstantString.CommandDescriptionMaxLens
       ),
     };
-    let warning: WarningResult | undefined = undefined;
-
-    if (requiredParams.length === 0 && optionalParams.length > 1) {
-      warning = {
-        type: WarningType.OperationOnlyContainsOptionalParam,
-        content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, operationId),
-        data: operationId,
-      };
-    }
-    return [command, warning];
+    return command;
   }
 
   static listAPIs(spec: OpenAPIV3.Document, options: ParseOptions): APIMap {

diff --git a/packages/spec-parser/test/browser/specParser.browser.test.ts b/packages/spec-parser/test/browser/specParser.browser.test.ts
@@ -180,13 +180,13 @@ describe("SpecParser in Browser", () => {
               title: "UserId",
               description: "User Id",
             },
+            {
+              name: "name",
+              title: "Name",
+              description: "User Name",
+            },
           ],
           description: "Get user by user id, balabala",
-          warning: {
-            type: WarningType.OperationOnlyContainsOptionalParam,
-            content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, "getUserById"),
-            data: "getUserById",
-          },
         },
       ]);
     });

diff --git a/packages/spec-parser/test/manifestUpdater.test.ts b/packages/spec-parser/test/manifestUpdater.test.ts
@@ -1142,7 +1142,13 @@ describe("manifestUpdater", () => {
     );
 
     expect(result).to.deep.equal(expectedManifest);
-    expect(warnings).to.deep.equal([]);
+    expect(warnings).to.deep.equal([
+      {
+        type: WarningType.OperationOnlyContainsOptionalParam,
+        content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, "getPets"),
+        data: "getPets",
+      },
+    ]);
   });
 
   it("should contain auth property in manifest if pass the api key auth", async () => {
@@ -2243,7 +2249,13 @@ describe("generateCommands", () => {
         type: "query",
       },
     ]);
-    expect(warnings).to.deep.equal([]);
+    expect(warnings).to.deep.equal([
+      {
+        type: WarningType.OperationOnlyContainsOptionalParam,
+        content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, "createPet"),
+        data: "createPet",
+      },
+    ]);
   });
 
   it("should not show warning for each GET/POST operation in the spec if only contains 1 optional parameters", async () => {
@@ -2320,7 +2332,18 @@ describe("generateCommands", () => {
         type: "query",
       },
     ]);
-    expect(warnings).to.deep.equal([]);
+    expect(warnings).to.deep.equal([
+      {
+        type: WarningType.OperationOnlyContainsOptionalParam,
+        content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, "getPets"),
+        data: "getPets",
+      },
+      {
+        type: WarningType.OperationOnlyContainsOptionalParam,
+        content: Utils.format(ConstantString.OperationOnlyContainsOptionalParam, "createPet"),
+        data: "createPet",
+      },
+    ]);
   });
 
   it("should only generate commands for GET operation with required parameter", async () => {