Azure / azure-functions-openapi-extension

This extension provides an Azure Functions app with Open API capability for better discoverability to consuming parties
https://www.nuget.org/packages/Microsoft.Azure.WebJobs.Extensions.OpenApi/
MIT License
374 stars 195 forks source link

Support "catch all" http trigger route #70

Open shibayan opened 3 years ago

shibayan commented 3 years ago

Writing a route like {*path} and {**path} to get all subsequent path parameters did not correctly output the OpenAPI definition.

Microsoft.Azure.WebJobs.Extensions.OpenApi: 0.5.1-preview

Repro code

[FunctionName("CatchAll")]
[OpenApiOperation(operationId: "CatchAll", tags: new[] { "name" })]
[OpenApiParameter("path", In = ParameterLocation.Path, Type = typeof(string))]
[OpenApiResponseWithoutBody(HttpStatusCode.OK)]
public static ActionResult CatchAll([HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "catchall/{*path}")]
                                                 HttpRequest req,
                                                 string path)
{
    return new OkObjectResult(path);
}

Actual behavior

image

justinyoo commented 3 years ago

@shibayan Thanks for the issue! The actual function endpoint itself works fine, if you send a request directly to the endpoint like:

curl http://localhost:7071/api/catchall/hello/world

However, like you mentioned, I can confirm that it doesn't work on the UI as expected. I'm not sure it's the issue on my end or the UI's end.

Have you got the similar experience with Swagger UI on other application like ASP.NET Core Web API with Swashbuckle?

shibayan commented 3 years ago

@justinyoo

I tried the same route with the ASP.NET Core Web API with Swashbuckle, and it worked. Looking at the generated openapi definition, it seems that "*" should not be included in the definition.

[Route("api/[controller]")]
[ApiController]
public class ValuesController : ControllerBase
{
    [HttpGet("{*path}")]
    public ActionResult Get(string path)
    {
        return Ok(path);
    }
}
{
  "openapi": "3.0.1",
  "info": {
    "title": "WebApplication6",
    "version": "v1"
  },
  "paths": {
    "/api/Values/{path}": {
      "get": {
        "tags": [
          "Values"
        ],
        "parameters": [
          {
            "name": "path",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
              "nullable": true
            }
          }
        ],
        "responses": {
          "200": {
            "description": "Success"
          }
        }
      }
    }
  },
  "components": { }
}
justinyoo commented 3 years ago

Thanks for confirming! Let me have a look.

justinyoo commented 3 years ago

@shibayan I looked at the issue. However, if the wildcard character is omitted, it actually changes the semantics of the endpoint. In other words, the following path template implies the {*path} can be hello, hello/world or hello/world/lorem/ipsum.

/catchall/{*path}

But the following path template implies the {path} can only be hello or world, not hello/world

/catchall/{path}

I can change the path template from /catchall/{*path} to /catchall/{path} so that the OpenAPI doc shows the latter but, like I said, it changes semantics. Even though I applied the change to Swagger UI, it doesn't work properly either. It should be the UI's issue, not ours.

image

image

image

image

shibayan commented 3 years ago

@justinyoo Thanks for the reply. I don't think the URL encoding of parameters is a Swagger UI issue, so I don't think it needs to be addressed, but I think the "catch all" parameter not being included in the OpenAPI definition is another issue.

justinyoo commented 3 years ago

@shibayan Yeah, it's not something I can do on my end. If you Google it, there are so many discussions around - from the spec wise to the implementation perspective.

I'm not too sure, we can get it through the proxies.json file. Let me try.