com-lihaoyi / cask

Cask: a Scala HTTP micro-framework. Cask makes it easy to set up a website, backend server, or REST API using Scala
https://com-lihaoyi.github.io/cask/
Other
525 stars 55 forks source link

Strange overlap report after adding another route #96

Closed OndrejSpanel closed 2 months ago

OndrejSpanel commented 8 months ago

Following code prints error when run:

object MinimalApplication extends cask.MainRoutes {

  @cask.get("/root/x")
  def hello() = {
    "Hello World!"
  }

  @cask.get("/:path")
  def doThing(path: String) = {
    path.reverse
  }

  initialize()
}

The error is:

Routes overlap with wildcards: get /root/x, get /:path

When I replace the "/root/x" route with "/", the error disappears (this is how default MinimalApplication looks like).

lihaoyi commented 8 months ago

This appears to be a limitation in the Mill routing logic; it doesn't realize the /:path and /root/x cannot clash. Should be fixable

OndrejSpanel commented 8 months ago

The framework looks very nice to me as an idea (typical Li Haoyi Easy, Boring and Fast), but my first experience is there are some rough egdes which make work unpleasant up to the level of nearly impossible. I hope you will not be annoyed by me raising issues I may have - I will try to stay factual and brief.

lihaoyi commented 8 months ago

Please do continue to report issues. We need these reports in order to improve the framework, so they are very much appreciated

OndrejSpanel commented 8 months ago

This one is currently a blocker for me. I am unable to implement OAuth for my app, as I am unable to handle /auth?code=xxxx loopback, as /auth overlaps with @get("/", subpath = true). I use this "serve all" at the moment to serve everything, as this is the only way to handle overlapping paths I have found to work, but this "serve all" is not called once there are any query parameters.

Btw, routing directly is Scala based on remainingPathSegments does not look bad at all:

      request.remainingPathSegments match {
        case Seq("auth") =>
          authRoute(request)
        case Seq("favicon.ico") =>
           resourceRoute("html/favicon.ico").withContentType("image/x-icon")
        case "classes" +: prefix +: path if whiteListClasses.contains(prefix) =>
          resourceRoute((prefix +: path).mkString("/")).withAutoContentType(path.last)
        case Seq(path) if path.endsWith(".js") =>
          jsRoute(path).withContentType("text/javascript")
      }
OndrejSpanel commented 8 months ago

No longer a blocker, this is my "catch them all" endpoint at the moment:

  @get("/", subpath = true)
  def root(code: Option[String] = None, state: Option[String] = None, request: Request): Response[Response.Data]

It is kind of strange to declare all possible query parameters this way, as I am using them directly from the request eventually, but it works and allows me to proceed.

jodersky commented 2 months ago

134 might have also fixed this. Is this still an issue?

OndrejSpanel commented 2 months ago

My app uses a single "catch them all" wildcard at the moment and I am working on different things now. The intensions seems reasonable and well thought and it should fix this.