Remove validation logic for duplicate route paths in `rest` library

[daviwil]

When you have two operations in a Cadl file which result in the same route path, the rest library raises a duplicate-operation diagnostic (Playground):

import "@cadl-lang/rest";

op fooOne(): { age: int32 };
op fooOnePrime(): { name: string; };

@johanste correctly pointed out to me that in the case of a route path which might have multiple request/response pairs (an overloaded operation), this diagnostic is undesired. It seems we should depend on the emitters to decide whether they support duplicate route paths and raise their own diagnostics in that case.

It is possible for us to update the openapi3 and autorest emitters to support this, so we should consider removing the restriction and adding emitter support for operation overloads like this.

[timotheeguerin]

is this true? I thought that overload was going to be explicit, so with the overload PR this diagnostic will need to be updated to take overload into account but will still need to be present.

[johanste]

Yes. Overloads is but one instance where things may be routed to the same endpoint. We really should let emitters decide if they can or cannot make sense out of the routes as defined.

[timotheeguerin]

The problem with having validation in emitters is that if multiple emitters will have the same requirement and are enabled at the same time you end up with duplicate diagnostic which is bad experience.

[markcowl]

@timotheeguerin @johanste I believe that the pattern we were talkin about is:

import "@cadl-lang/rest";

op foo(): {age: int32} | {name: string};
@overloads(foo)
op fooOne(): { age: int32 };
@overloads(foo)
op fooOnePrime(): { name: string; };

Is there a pattern where we don't want to have the union representation we would emit in OpenAPI?

[johanste]

Is there a pattern where we don't want to have the union representation we would emit in OpenAPI?

I'm assuming you are not talking about the non-deterministic:ness (from the client's perspective) in the example above that would make this somewhat problematic (i.e. there is no way for the service to distinguish between the two calls and thus there is not enough information in the signature to determine what the actual return value type would/should be.

In the general case, OpenApi 3.x would emit the union representation. Autorest-Swagger would use x-ms-paths. Client code emitters would do something language dependent. Python or TS would likely return a union. C# most likely would not do the overloads and use the distinct names.

[markcowl]

@johanste Unless I am missing something, the non-determinism is inherent in the initial example. If inputs are not different for these output types, then there is no way for the client to determine what the output would be.

The current POR is to use the overload example above, condition the diagnostic on the overload decorator being present, and have emitters determine whether or not to use the overloads. AFAIK this covers the proposed scenario, and has a strong requirement for using unions to ensure an accurate api description. Is there some scenario we want to support that is not covered by this?

[timotheeguerin]

Design solution

Be explicit when to bypass this error @route takes a new parameter shared, defaulting to false that would bypass verb+route validation.

@route("/uploadImage", { shared: true })
op uploadImageBytes(@body body: bytes, @header contentType: "image/png"): void

@route("/uploadImage", { shared: true })
op uploadImageJson(@body body: {imageBase64: bytes}, @header contentType: "application/json"): void

// error: not shared route
@route("/uploadImage")
op uploadImageJson(@body body: {imageBase64: bytes}, @header contentType: "application/json"): void

Resulting python

def upload_image_bytes(body: stream):
  request("/uploadImage", body)
def upload_image_json(body: object):
  request("/uploadImage", body)