[migration tsp] A non-model response body type will introduce breakings during tsp migration

[kazrael2119]

1: This breaking is found in verify breakings in tsp conversion.
Breaking message:

- Operation EmailServices.listVerifiedExchangeOnlineDomains has a new signature

Details:
The service has an operation named listVerifiedExchangeOnlineDomains and its response is a string array:
Swagger:

"responses": {
    "200": {
      "description": "Success. The response describe a list of verified domains from Exchange Online.",
      "schema": {
        "$ref": "#/definitions/VerifiedExchangeOnlineDomainList"
      }
    },
    ...
}
...
"VerifiedExchangeOnlineDomainList": {
  "description": "List of FQDNs of verified domains in Exchange Online.",
  "type": "array",
  "items": {
    "type": "string"
  }
},

In HLC, we will separately package the response into a model called xxxResponse, and based on this code, and assign a body property to a non-model response:

export interface EmailServices {
     listVerifiedExchangeOnlineDomains(options?: EmailServicesListVerifiedExchangeOnlineDomainsOptionalParams):Promise<EmailServicesListVerifiedExchangeOnlineDomainsResponse>;
}

// @public
export type EmailServicesListVerifiedExchangeOnlineDomainsResponse = {
    body: string[];
};

However, in Modular, the string[] type will be used as the promise response type directly.
TypeSpec:

listVerifiedExchangeOnlineDomains is ArmProviderActionSync<
  Response = ArmResponse<VerifiedExchangeOnlineDomainList>,
  Scope = SubscriptionActionScope
>;

model VerifiedExchangeOnlineDomainList is string[];

Modular:

export interface EmailServicesOperations {
     listVerifiedExchangeOnlineDomains: (options?: EmailServicesListVerifiedExchangeOnlineDomainsOptionalParams) =>Promise<string[]>;
}

2: The same breakings are introduced for file/bytes type.
Related issue: #3635 (Modular fails to generate if the response type is File<"application/octet-stream">).
Original Swagger:

"operationId": "WebApps_GetWebSiteContainerLogs",
"responses": {
  "200": {
    "description": "Azure operation completed successfully.",
    "schema": {
      "type": "file"
    }
  },
}

HLC:

export type WebAppsGetWebSiteContainerLogsResponse = {
  blobBody?: Promise<Blob>;

  readableStreamBody?: NodeJS.ReadableStream;
};

  getWebSiteContainerLogs(
    resourceGroupName: string,
    name: string,
    options?: WebAppsGetWebSiteContainerLogsOptionalParams,
  ): Promise<WebAppsGetWebSiteContainerLogsResponse> {
    return this.client.sendOperationRequest(
      { resourceGroupName, name, options },
      getWebSiteContainerLogsOperationSpec,
    );
  }

blobBody and readableStreamBody are also generated from codegen, not source Swagger.

In modular, it will be

export async function _testDeserialize(result: PathUncheckedResponse): Promise<Uint8Array> {
   ...
}

Do we accept this break or need to make the behavior the same as HLC?
I create a pr to investigate this

Proposal

We decided not accepting this breaking from HLC and Modular so we decide to have a feature flag named wrap-non-model-return in Modular and by default enabling this flag for all services, when the flag is set as false, we will still generate the old modular ways.

For normal operation(non paging or LRO operations), when the return type is non-model type, HLC would genereate a wrapper interface XXXResponse not directly returning the model type, so to not introduce any breakings in Modular, we would generate the same interfaces in these cases.

The XXXResponse would be like OperationGroupNameMethodNameResponse.

Case 1: If the return type is bytes type &the mediaType is KnownMediaType.Binary

Here is the code to define the binary type. Practical examples that map to binary in the content-type classifier are:application/octet-stream, image/, audio/, video/, and unrecognized application/(non json/text ect), same mapping concept used by tooling.

Please note the handling for multipart with bytes will not change.

HLC code: https://github.com/Azure/autorest.typescript/blob/main/packages/autorest.typescript/src/generators/modelsGenerator.ts#L307-L326

TypeSpec

getWebSiteContainerLogs is SiteOps.ActionSync<
    Site,
    void,
    {
      @header("Content-Type")
      contentType: "application/octet-stream";

      @doc("Receipt body in COSE format")
      @bodyRoot
      body: bytes;
    } | NoContentResponse,
    OverrideErrorType = DefaultErrorResponse
  >;

current modular generated code which is wrap-non-model-return: false:

export interface PrivateDnsZoneSuffixOperations {
    getWebSiteContainerLogs: (options?: GetWebSiteContainerLogsOptionalParams) => Promise<Uint8Array>;
}

expected generated code which is wrap-non-model-return: true:

export type WebAppsGetWebSiteContainerLogsResponse = {
  /**
   * BROWSER ONLY
   *
   * The response body as a browser Blob.
   * Always `undefined` in node.js.
   */
  blobBody?: Promise<Blob>;
  /**
   * NODEJS ONLY
   *
   * The response body as a node.js Readable stream.
   * Always `undefined` in the browser.
   */
  readableStreamBody?: NodeJS.ReadableStream;
};

  getWebSiteContainerLogs(
    resourceGroupName: string,
    name: string,
    options?: WebAppsGetWebSiteContainerLogsOptionalParams,
  ): Promise<WebAppsGetWebSiteContainerLogsResponse> {
    return this.client.sendOperationRequest(
      { resourceGroupName, name, options },
      getWebSiteContainerLogsOperationSpec,
    );
  }

And the Deserialize function would be like:

export async function _getWebSiteContainerLogsDeserialize(
  result: PathUncheckedResponse
): Promise<WebAppsGetWebSiteContainerLogsResponse> {
  const expectedStatuses = ["200"];
  if (!expectedStatuses.includes(result.status)) {
    throw createRestError(result);
  }

  return {
     blobBody: toBlob((result as StreamableMethod).asBrowserStream()),
     readableStreamBody: (result as StreamableMethod).asNodeStream()
  };
}

where the toBlob is the static helper

async function toBlob(
  stream: ReadableStream<Uint8Array> | undefined,
  mimeType?: string,
): Promise<Blob> | undefined {
  if (!stream) return undefined;
  return mimeType
    ? new Response(stream, { headers: { "Content-Type": mimeType } }).blob()
    : new Response(stream).blob();
}

Case 2: If the return type is string/array/any/enum/.. which is any non-model and non-record type

TypeSpec

@autoRoute
  @action("getPrivateDnsZoneSuffix")
  get is ArmProviderActionSync<Response = {
    @body
    privateDnsZoneSuffix: PrivateDnsZoneSuffix;
  }>;

scalar PrivateDnsZoneSuffix extends string;

current modular generated code which is wrap-non-model-return:flase:

export interface PrivateDnsZoneSuffixOperations {
    get: (options?: PrivateDnsZoneSuffixGetOptionalParams) => Promise<string>;
}

expected generated code which is wrap-non-model-return: true:

export type PrivateDnsZoneSuffixGetResponse = {
    body: string;
};

export interface PrivateDnsZoneSuffix {
    get(options?: PrivateDnsZoneSuffixGetOptionalParams): Promise<PrivateDnsZoneSuffixGetResponse>;

And the Deserialize function would be like:

export async function _getDeserialize(
  result: PathUncheckedResponse
): Promise<PrivateDnsZoneSuffixGetResponse > {
  const expectedStatuses = ["200"];
  if (!expectedStatuses.includes(result.status)) {
    throw createRestError(result);
  }

  return {
     body: result.body
  };
}

[qiaozha]

Please review the breaking change with @joheredi and @maorleger

[joheredi]

When the response body is a model, does this problem exist?

[JialinHuang803]

When the response body is a model, does this problem exist?

@joheredi No. For example, for the client.virtualMachines.get() operation which returns a VirtualMachine model:

In HLC, the generated response type equals to the response body (I think this is the main difference):

export type VirtualMachinesGetResponse = VirtualMachine;

get(
    resourceGroupName: string,
    vmName: string,
    options?: VirtualMachinesGetOptionalParams,
  ): Promise<VirtualMachinesGetResponse> {
    return this.client.sendOperationRequest(
      { resourceGroupName, vmName, options },
      getOperationSpec,
    );
  }

In Modular, there isn't xxxResponse for each operation, and the VirtualMachine model is returned. That means, both HLC and Modular return the model in the Promise directly.

/** Retrieves information about the model view or the instance view of a virtual machine. */
export async function get(
  context: Client,
  resourceGroupName: string,
  vmName: string,
  options: VirtualMachinesGetOptionalParams = { requestOptions: {} },
): Promise<VirtualMachine> {
  const result = await _getSend(context, resourceGroupName, vmName, options);
  return _getDeserialize(result);
}

For non-model responses, HLC doesn't let the xxxResponse equal to the response body (e.g. xxxResponse = string[]). Instead, it inserts the response body to the body property, while in Modular the response body string[] is returned directly.

export type xxxResponse = {
    body: string[];
};

If the response body is a file type, there are two generated properties (blobBody and readableStreamBody) in the xxxResponse in HLC:

export type xxxResponse = {
  blobBody?: Promise<Blob>;

  readableStreamBody?: NodeJS.ReadableStream;
};

We should implement the same behavior in Modular if we don't want to introduce breaking.

[v-jiaodi]

AppService exits same breaking change.