disneystreaming / smithy4s

https://disneystreaming.github.io/smithy4s/
Other
340 stars 68 forks source link

Document interaction with http4s Router and relative paths #152

Open keynmol opened 2 years ago

keynmol commented 2 years ago

Let's say you define your smithy ops as mounted at root:

@http(method: "POST", uri: "/{name}", code: 200)

If you then build the routes using val routes = SimpleRestJsonBuilder.routes(...).resource, you can't just shift the API to a custom prefix:

val shifted = Router("/api" -> routes)

The request to /api/bla will fail with 404.

This approach will work with manually defined routes, e.g.

val other = HttpRoutes.of[IO] {
  case r if r.pathInfo.segments.headOption.exists(_.decoded() == "hello") =>
    import org.http4s.dsl.io.*
    Ok(r.toString)
}

val shifted = Router("/api" -> (routes <+> other))

The request to /api/hello/bla will succeed.

I don't know exactly whether the recommendation should be writing absolute paths in the smithy files, or whether this should be considered undesired behaviour. Note that it will obviously affect the generated Swagger docs as well.

Baccata commented 2 years ago

I don't know exactly whether the recommendation should be writing absolute paths in the smithy files

That is a protocol-specific concern, and for the "simpleRestJson" protocol, the implicit rule is that the API spec should contain absolute paths. The rationale is as follows :

  1. It reinforces the source of truth aspect of the smithy specs
  2. It facilitates interop with other tooling (such as swagger/openapi)

Facilitating shifting would break that paradigm. That being said, the fact that it doesn't work is somewhat accidental, and I'd like to understand why. But I'm not sure I want to fix it, since the bug is, in essence, a happy accident 😅.

You are correct in the fact that it ought to be documented

kubukoz commented 1 year ago

https://smithy.io/2.0/spec/http-bindings.html?highlight=http#uri

The resolved absolute URI of an operation is formed by combining the URI of the operation with the endpoint of the service. (that is, the host and any base URL of where the service is deployed). For example, given a service endpoint of https://example.com/v1 and an operation pattern of /myresource, the resolved absolute URI of the operation is https://example.com/v1/myresource.

It's fine that @simpleRestJson doesn't support this, but I think SmithyHttp4sRouter could. For other protocols which might want to reuse that, it'd save some trouble and still be in line with the Smithy spec (I think).

As for why it doesn't work, I believe Router doesn't modify the Uri that's passed to the underlying services, but rather updates the Request.Keys.PathInfoCaret attribute, which affects r.pathInfo.

Smithy4s doesn't look at r.pathInfo, it looks at the entire r.uri.path.