[v6] improve request multi-mediatype support

[chradek]

This PR is an attempt to improve on the multi media-type support on operation requests.

This fixes an issue where the contentType contains a single value and is a ConstantSchema type.

This also gets rid of the type union for optional parameters that accepted an optional contentType. The contentType needs to be specified before specifying other parameters for intellisense to be helpful, so I made it a required parameter if an operation supports multiple media types.

Since contentType is now a required parameter, this PR generated an operation overload for each media type. Here's an example generated from the media_types.json swagger file:

 /**
   * Analyze body, that could be different media types.
   * @param contentType Upload file type
   * @param options The options parameters.
   */
  analyzeBody(
    contentType: ContentType,
    options?: MediaTypesClientAnalyzeBody$binaryOptionalParams
  ): Promise<MediaTypesClientAnalyzeBodyResponse>;
  /**
   * Analyze body, that could be different media types.
   * @param contentType Body Parameter content-type
   * @param options The options parameters.
   */
  analyzeBody(
    contentType: "application/json",
    options?: MediaTypesClientAnalyzeBody$jsonOptionalParams
  ): Promise<MediaTypesClientAnalyzeBodyResponse>;
  /**
   * Analyze body, that could be different media types.
   * @param contentType Upload file type
   * @param options The options parameters.
   */
  analyzeBody(
    contentType: ContentType | "application/json",
    options?:
      | MediaTypesClientAnalyzeBody$binaryOptionalParams
      | MediaTypesClientAnalyzeBody$jsonOptionalParams
  ): Promise<MediaTypesClientAnalyzeBodyResponse> {

I'd love to get some feedback on this approach. The one issue I see with this approach is that if an operation supports a single media type, then adds support for another one in the future, the contentType parameter gets added as a required parameter which would be a breaking change. I don't know if this is a real issue though since I don't know if adding additional media types is actually allowed once a service has released.

If the above is a concern I might be able to work around this by checking if the modeled contentType parameter is the json media type and remove the contentType.

/cc @bterlson @jeremymeng

[joheredi]

Looks good 😄

[daviwil]

Looks great! Just some small nits here and there.

[daviwil]

Nice workaround for the modelerfour issue, I'm planning to fix this so that it's no longer necessary 😅

[chradek]

You'll only make it required if there are multiple media types though right? :)

[daviwil]

Actually, I think the issue I'm thinking of is not related to Content-Type parameters, but body parameters when there are multiple requests. Those body parameters are not carrying forward required when they should be.

[chradek]

Ah right, Brian mentioned that. That will make the overload signatures look much better because the optional parameter will be the same type in all overloads (the body is currently the only thing in the optional type which is why we need multiple versions of it today).

[daviwil]

What does Boolean() do? Coerce to boolean somehow?

[chradek]

Yes! But I don't need it and will remove.

[bterlson]

Yep, functionally equivalent to !! but more clear IMO. Both call the ToBoolean abstract operation.

[daviwil]

Oh, one thing I forgot: Why is the second overload needed? Is application/json not covered by the ContentType enum?

[chradek]

Correct. ContentType contains the content-types that are binary media types. e.g. application/pdf.
This actually works well for us because one overload accepts the binary content-types (and the request body would accept coreHttp.HttpRequestBody), and the other overload accepts just application/json (so the body parameter is a structured object).