Fix non-resource action operation behavior (#5052)

As titled. --------- Co-authored-by: Pan Shao <[email protected]>
Azure · Jan 9, 2025 · 6eb062e · 6eb062e
1 parent 1b159a6
commit 6eb062e
Show file tree

Hide file tree

Showing 20 changed files with 196 additions and 130 deletions.
diff --git a/common/changes/@autorest/openapi-to-typespec/operationFix_2025-01-08-08-34.json b/common/changes/@autorest/openapi-to-typespec/operationFix_2025-01-08-08-34.json
@@ -0,0 +1,10 @@
+{
+  "changes": [
+    {
+      "packageName": "@autorest/openapi-to-typespec",
+      "comment": "Fix action operation for non-resource operation",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@autorest/openapi-to-typespec"
+}
diff --git a/packages/extensions/openapi-to-typespec/src/generate/generate-arm-resource.ts b/packages/extensions/openapi-to-typespec/src/generate/generate-arm-resource.ts
@@ -15,6 +15,7 @@ import { getTSPOperationGroupName } from "../transforms/transform-arm-resources"
 import { generateAugmentedDecorators, generateDecorators } from "../utils/decorators";
 import { generateDocs } from "../utils/docs";
 import { getLogger } from "../utils/logger";
+import { generateLroHeaders } from "../utils/lro";
 import { getModelPropertiesDeclarations } from "../utils/model-generation";
 import { generateSuppressions } from "../utils/suppressions";
 import { generateOperation, generateParameters } from "./generate-operations";
@@ -175,15 +176,6 @@ function generateArmRequest(request: TypespecParameter | TypespecVoidType | Type
   return request.name;
 }
 
-function generateLroHeaders(lroHeaders: TspLroHeaders): string {
-  if (lroHeaders === "Azure-AsyncOperation") {
-    return "ArmAsyncOperationHeader & Azure.Core.Foundations.RetryAfterHeader";
-  } else if (lroHeaders === "Location") {
-    return "ArmLroLocationHeader & Azure.Core.Foundations.RetryAfterHeader";
-  }
-  throw new Error(`Unknown LRO header: ${lroHeaders}`);
-}
-
 function generateArmResponse(responses: TypespecTemplateModel[] | TypespecVoidType): string {
   if (!Array.isArray(responses)) {
     return "void";

diff --git a/packages/extensions/openapi-to-typespec/src/generate/generate-operations.ts b/packages/extensions/openapi-to-typespec/src/generate/generate-operations.ts
@@ -7,6 +7,8 @@ import {
 import { getOptions } from "../options";
 import { generateDecorators } from "../utils/decorators";
 import { generateDocs, generateSummary } from "../utils/docs";
+import { generateLroHeaders } from "../utils/lro";
+import { generateSuppressions } from "../utils/suppressions";
 import { generateParameter } from "./generate-parameter";
 
 export function generateOperation(operation: TypespecOperation, operationGroup?: TypespecOperationGroup) {
@@ -78,14 +80,22 @@ export function generateProviderAction(operation: TspArmProviderActionOperation)
   summary && statements.push(summary);
   statements.push(doc);
   const templateParameters = [];
-  const responses = [...new Set(operation.responses)];
-  // Workaround for array response, refactor later.
-  const response =
-    responses.length === 1 && responses[0].endsWith("[]") ? `{@bodyRoot _: ${responses[0]}}` : responses.join("|");
-  if (response !== "void") {
-    templateParameters.push(`Response = ${response}`);
+
+  if (operation.request) {
+    templateParameters.push(`Request = ${operation.request.type}`);
+  }
+
+  if (operation.response) {
+    if (operation.response.endsWith("[]")) {
+      templateParameters.push(`Response = {@bodyRoot _: ${operation.response}}`);
+    } else {
+      templateParameters.push(`Response = ${operation.response}`);
+    }
+  }
+
+  if (operation.scope) {
+    templateParameters.push(`Scope = ${operation.scope}`);
   }
-  templateParameters.push(`Scope = ${operation.scope}`);
 
   if (operation.parameters.length > 0) {
     const params: string[] = [];
@@ -104,14 +114,19 @@ export function generateProviderAction(operation: TspArmProviderActionOperation)
       templateParameters.push(`Parameters = {${params}}`);
     }
   }
-  if (operation.request) {
-    templateParameters.push(`Request = ${operation.request.type}`);
-  }
 
+  if (operation.lroHeaders) {
+    templateParameters.push(`LroHeaders = ${generateLroHeaders(operation.lroHeaders)}`);
+  }
+  if (operation.suppressions) {
+    statements.push(generateSuppressions(operation.suppressions).join("\n"));
+  }
   if (operation.decorators) {
     statements.push(generateDecorators(operation.decorators));
   }
-  statements.push(`@${operation.verb}`);
+  if (operation.verb) {
+    statements.push(`@${operation.verb}`);
+  }
   if (operation.action) {
     statements.push(`@action("${operation.action}")`);
   }
@@ -172,11 +187,8 @@ export function generateOperationGroup(operationGroup: TypespecOperationGroup) {
   const { name, operations } = operationGroup;
 
   statements.push(`${doc}`);
+  operationGroup.suppressions && statements.push(generateSuppressions(operationGroup.suppressions).join("\n"));
   const hasInterface = Boolean(name);
-  const hasProvider = operations.find((o) => (o as TspArmProviderActionOperation).kind !== undefined) !== undefined;
-  if (hasProvider && hasInterface) {
-    statements.push(`@armResourceOperations`);
-  }
   hasInterface && statements.push(`interface ${name} {`);
 
   for (const operation of operations) {

diff --git a/packages/extensions/openapi-to-typespec/src/interfaces.ts b/packages/extensions/openapi-to-typespec/src/interfaces.ts
@@ -33,7 +33,7 @@ export interface WithDecorators {
   clientDecorators?: TypespecDecorator[];
 }
 
-export interface TypespecOperationGroup extends WithDoc {
+export interface TypespecOperationGroup extends WithDoc, WithSuppressDirectives {
   name: string;
   operations: (TypespecOperation | TspArmProviderActionOperation)[];
 }
@@ -315,16 +315,17 @@ export type TspArmOperationType =
   | "ArmResourceListAtScope";
 
 // TO-DO: consolidate with other templates
-export interface TspArmProviderActionOperation extends WithDoc, WithSummary {
+export interface TspArmProviderActionOperation extends WithDoc, WithSummary, WithSuppressDirectives {
   kind: "ArmProviderActionAsync" | "ArmProviderActionSync";
   name: string;
   action?: string;
-  responses?: string[];
-  verb: string;
+  response?: string;
+  verb?: string;
   scope?: "TenantActionScope" | "SubscriptionActionScope";
   parameters: TypespecParameter[];
   request?: TypespecParameter;
   decorators?: TypespecDecorator[];
+  lroHeaders?: TspLroHeaders;
 }
 
 export interface TspArmResource extends TypespecObject {

diff --git a/packages/extensions/openapi-to-typespec/src/transforms/transform-operations.ts b/packages/extensions/openapi-to-typespec/src/transforms/transform-operations.ts
@@ -24,6 +24,7 @@ import {
   TypespecParameterLocation,
   Extension,
   TspArmProviderActionOperation,
+  TypespecDecorator,
 } from "../interfaces";
 import { transformDataType } from "../model";
 import { getOptions } from "../options";
@@ -45,10 +46,20 @@ export function transformOperationGroup(
     acc = [...acc, ...transformOperation(op, codeModel, name)];
     return acc;
   }, []);
+  const { isArm, isFullCompatible } = getOptions();
+  const suppressions =
+    isArm && isFullCompatible
+      ? [
+          getSuppresssionWithCode(
+            "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator",
+          ),
+        ]
+      : undefined;
   return {
     name,
     doc,
     operations: ops,
+    suppressions,
   };
 }
 
@@ -149,7 +160,7 @@ export function transformRequest(
   }
 
   const resource = operation.language.default.resource;
-  let decorators = undefined;
+  let decorators: TypespecDecorator[] | undefined = undefined;
   if (
     groupName &&
     operation.operationId &&
@@ -169,15 +180,35 @@ export function transformRequest(
       const action = getActionForPrviderTemplate(route);
       if (action !== undefined) {
         const isLongRunning = operation.extensions?.["x-ms-long-running-operation"] ?? false;
+        decorators ??= [];
+        decorators.push({
+          name: "autoRoute",
+          module: "@typespec/rest",
+          namespace: "TypeSpec.Rest",
+        });
+
+        const verb = transformVerb(requests?.[0].protocol);
+
+        // For Async, there should be a 202 response without body and might be a 200 response with body
+        // For Sync, there might be a 200 response with body
+        const response = transformedResponses.find((r) => r[0] === "200")?.[1] ?? undefined;
+        const finalStateVia =
+          operation.extensions?.["x-ms-long-running-operation-options"]?.["final-state-via"] ?? "location";
+        const lroHeaders =
+          isLongRunning && finalStateVia === "azure-async-operation" ? "Azure-AsyncOperation" : undefined;
+        const suppressions =
+          isFullCompatible && lroHeaders
+            ? [getSuppresssionWithCode("@azure-tools/typespec-azure-resource-manager/lro-location-header")]
+            : undefined;
         return {
           kind: isLongRunning ? "ArmProviderActionAsync" : "ArmProviderActionSync",
           doc,
           summary,
           name,
-          verb: transformVerb(requests?.[0].protocol),
+          verb: verb === "post" ? undefined : verb,
           action: action === name ? undefined : action,
-          scope: route.startsWith("/providers/") ? "TenantActionScope" : "SubscriptionActionScope",
-          responses: transformedResponses.map((r) => r[1]),
+          scope: route.startsWith("/providers/") ? undefined : "SubscriptionActionScope",
+          response,
           parameters: parameters
             .filter((p) => p.location !== "body")
             .map((p) => {
@@ -193,6 +224,8 @@ export function transformRequest(
             }),
           request: parameters.find((p) => p.location === "body"),
           decorators,
+          lroHeaders,
+          suppressions,
         };
       }
     }

diff --git a/packages/extensions/openapi-to-typespec/src/utils/lro.ts b/packages/extensions/openapi-to-typespec/src/utils/lro.ts
@@ -1,4 +1,5 @@
 import { CodeModel, isObjectSchema, Operation } from "@autorest/codemodel";
+import { TspLroHeaders } from "../interfaces";
 import { isResponseSchema } from "./schemas";
 
 export function markLRO(codeModel: CodeModel) {
@@ -35,3 +36,12 @@ export function hasLROExtension(operation: Operation) {
     operation.extensions?.["x-ms-long-running-operation-options"]
   );
 }
+
+export function generateLroHeaders(lroHeaders: TspLroHeaders): string {
+  if (lroHeaders === "Azure-AsyncOperation") {
+    return "ArmAsyncOperationHeader & Azure.Core.Foundations.RetryAfterHeader";
+  } else if (lroHeaders === "Location") {
+    return "ArmLroLocationHeader & Azure.Core.Foundations.RetryAfterHeader";
+  }
+  throw new Error(`Unknown LRO header: ${lroHeaders}`);
+}
diff --git a/packages/extensions/openapi-to-typespec/src/utils/type-mapping.ts b/packages/extensions/openapi-to-typespec/src/utils/type-mapping.ts
@@ -14,6 +14,7 @@ export function getFullyQualifiedName(type: string, namespace: string | undefine
     case "SubscriptionBaseParameters":
     case "ExtensionBaseParameters":
     case "LocationBaseParameters":
+    case "DefaultBaseParameters":
       return `${namespace ?? "Azure.ResourceManager.Foundations"}.${type}`;
     default:
       return type;

diff --git a/packages/extensions/openapi-to-typespec/test/arm-agrifood/swagger-output/swagger.json b/packages/extensions/openapi-to-typespec/test/arm-agrifood/swagger-output/swagger.json
@@ -66,9 +66,6 @@
     },
     {
       "name": "DataManagerForAgricultureSolutions"
-    },
-    {
-      "name": "CheckNameAvailabilityOperations"
     }
   ],
   "paths": {
@@ -348,9 +345,6 @@
     "/subscriptions/{subscriptionId}/providers/Microsoft.AgFoodPlatform/checkNameAvailability": {
       "post": {
         "operationId": "CheckNameAvailability_CheckNameAvailability",
-        "tags": [
-          "CheckNameAvailabilityOperations"
-        ],
         "description": "Checks the name availability of the resource with requested resource name.",
         "parameters": [
           {

diff --git a/packages/extensions/openapi-to-typespec/test/arm-agrifood/tsp-output/routes.tsp b/packages/extensions/openapi-to-typespec/test/arm-agrifood/tsp-output/routes.tsp
@@ -13,22 +13,23 @@ using TypeSpec.OpenAPI;
 
 namespace Microsoft.AgFoodPlatform;
 
-@armResourceOperations
+#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "For backward compatibility"
 interface CheckNameAvailabilityOperations {
   /**
    * Checks the name availability of the resource with requested resource name.
    */
   #suppress "@azure-tools/typespec-azure-core/no-openapi" "non-standard operations"
   @operationId("CheckNameAvailability_CheckNameAvailability")
-  @post
+  @autoRoute
   checkNameAvailability is ArmProviderActionSync<
+    Request = CheckNameAvailabilityRequest,
     Response = CheckNameAvailabilityResponse,
     Scope = SubscriptionActionScope,
-    Parameters = {},
-    Request = CheckNameAvailabilityRequest
+    Parameters = {}
   >;
 }
 
+#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "For backward compatibility"
 interface OperationResultsOperations {
   /**
    * Get operationResults for a Data Manager For Agriculture resource.

diff --git a/...ages/extensions/openapi-to-typespec/test/arm-alertsmanagement/swagger-output/swagger.json b/...ages/extensions/openapi-to-typespec/test/arm-alertsmanagement/swagger-output/swagger.json
@@ -47,18 +47,12 @@
     },
     {
       "name": "SmartGroups"
-    },
-    {
-      "name": "AlertsOperations"
     }
   ],
   "paths": {
     "/providers/Azure.ResourceManager.AlertsManagement/alertsMetaData": {
       "get": {
         "operationId": "Alerts_MetaData",
-        "tags": [
-          "AlertsOperations"
-        ],
         "description": "List alerts meta data information based on value of identifier parameter.",
         "parameters": [
           {
@@ -701,9 +695,6 @@
     "/subscriptions/{subscriptionId}/providers/Azure.ResourceManager.AlertsManagement/alertsSummary": {
       "get": {
         "operationId": "Alerts_GetSummary",
-        "tags": [
-          "AlertsOperations"
-        ],
         "description": "Get a summarized count of your alerts grouped by various parameters (e.g. grouping by 'Severity' returns the count of alerts for each severity).",
         "parameters": [
           {

diff --git a/packages/extensions/openapi-to-typespec/test/arm-alertsmanagement/tsp-output/routes.tsp b/packages/extensions/openapi-to-typespec/test/arm-alertsmanagement/tsp-output/routes.tsp
@@ -13,18 +13,18 @@ using TypeSpec.OpenAPI;
 
 namespace Azure.ResourceManager.AlertsManagement;
 
-@armResourceOperations
+#suppress "@azure-tools/typespec-azure-resource-manager/arm-resource-interface-requires-decorator" "For backward compatibility"
 interface AlertsOperations {
   /**
    * List alerts meta data information based on value of identifier parameter.
    */
   #suppress "@azure-tools/typespec-azure-core/no-openapi" "non-standard operations"
   @operationId("Alerts_MetaData")
+  @autoRoute
   @get
   @action("alertsMetaData")
   metaData is ArmProviderActionSync<
     Response = AlertsMetaData,
-    Scope = TenantActionScope,
     Parameters = {
       /**
        * Identification of the information to be retrieved by API call.
@@ -38,6 +38,7 @@ interface AlertsOperations {
    */
   #suppress "@azure-tools/typespec-azure-core/no-openapi" "non-standard operations"
   @operationId("Alerts_GetSummary")
+  @autoRoute
   @get
   @action("alertsSummary")
   getSummary is ArmProviderActionSync<

diff --git a/...ages/extensions/openapi-to-typespec/test/arm-analysisservices/swagger-output/swagger.json b/...ages/extensions/openapi-to-typespec/test/arm-analysisservices/swagger-output/swagger.json
@@ -42,18 +42,12 @@
   "tags": [
     {
       "name": "AnalysisServicesServersOperationGroup"
-    },
-    {
-      "name": "ServersOperations"
     }
   ],
   "paths": {
     "/subscriptions/{subscriptionId}/providers/Azure.ResourceManager.Analysis/locations/{location}/checkNameAvailability": {
       "post": {
         "operationId": "Servers_CheckNameAvailability",
-        "tags": [
-          "ServersOperations"
-        ],
         "description": "Check the name availability in the target location.",
         "parameters": [
           {
@@ -130,9 +124,6 @@
     "/subscriptions/{subscriptionId}/providers/Azure.ResourceManager.Analysis/skus": {
       "get": {
         "operationId": "Servers_ListSkusForNew",
-        "tags": [
-          "ServersOperations"
-        ],
         "description": "Lists eligible SKUs for Analysis Services resource provider.",
         "parameters": [
           {