New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nesting path combinators #570

Open
slogsdon opened this Issue Jan 22, 2017 · 5 comments

Comments

Projects
None yet
6 participants
@slogsdon
Copy link

slogsdon commented Jan 22, 2017

As a newcomer to Suave, I've been working to move past copying and pasting examples I come across, and I've started building an application from scratch. During this process, I attempted to do something like this:

let app =
  path "/" >=> OK "index"
  path "/api"
    >=> choose [
          path "" >=> NO_CONTENT
          path "/users" >=> OK """{"api": "users"}"""
        ]

expecting a request to /api would return a 204 No Content response and a request to /api/users would return the JSON-encoded string. To my surprise, I received a 404 Not Found for both. After doing some digging, I discovered that path tests if the given path matches the request path, so with path "/users" above, it compares a request to /api/users with the given /users, fails, and returns None.

I wasn't able to find anything in the existing code base to handle this, so I added the following as a helper module in my application to achieve the expected behavior:

let optionally pred value =
  if pred then Some value else None

let getCurrentRoot ctx =
  match ctx.userState.TryFind("rootPath") with
  | None -> ""
  | Some p -> string p

let rootPath (part : string) (ctx : HttpContext) =
  let root = getCurrentRoot ctx
  { ctx with userState = ctx.userState.Add("rootPath", root + part) }
  |> Some
  |> async.Return

let subPath (part : string) (ctx : HttpContext) =
  let fullPath = (getCurrentRoot ctx) + part
  ctx
  |> optionally (fullPath = ctx.request.path)
  |> async.Return

allowing the following to work as expected:

let app =
  path "/" >=> OK "index"
  rootPath "/api"
    >=> choose [
          // handles requests to `/api`
          subPath "" >=> NO_CONTENT
          // handles requests to `/api/users`
          subPath "/users" >=> OK """{"api": "users"}"""
        ]

I took to Twitter to see if there was an official way of doing this, and the official Twitter handle suggested I open a PR to start a discussion (see: https://twitter.com/SuaveIO/status/823219104995741696). I'm a a big fan of using issues for these types of discussions, but if something like the above is desired for Suave, I'd be happy to submit a PR for any changes.

Does something like this exist today? If not, would it be useful to include this functionality as a part of Suave?

@caindy

This comment has been minimized.

Copy link

caindy commented Feb 12, 2017

Not a Suave expert, but if I understand correctly, you simply want something like:

let apiPath = ((+) "/api") >> path

So you can do

choose [ apiPath "" >=> NO_CONTENT; apiPath "/users" >=> OK "...json..." ]

Later maybe you'd extend your apiPath to check the mime-type or something.

@slogsdon

This comment has been minimized.

Copy link

slogsdon commented Feb 13, 2017

@caindy Thanks for the suggestion, but there would still be some duplication within a developer's application that could stand to be removed. A developer need to create and manage these helper functions, so a benefit here would be reducing the amount of code the developer's application would need to hold itself.

@ademar ademar added the feature label Feb 28, 2017

@danieljsummers

This comment has been minimized.

Copy link
Contributor

danieljsummers commented Apr 24, 2017

Are you looking for something like Express.js's router/module paradigm? (briefly described here)

Something like...

let apiRoutes =
  router [
    routePath "/" >=> NO_CONTENT
    routePath "/users" >=> OK """{"api": "users"}"""
    ]

let app =
  choose [
    path "/" >=> OK "index"
    path "/api" >=> apiRoutes
    ]
@czifro

This comment has been minimized.

Copy link

czifro commented Jul 28, 2017

I too found this to be a minor let down. However, I (am no way a Suave expert) can see why path does not do what you ( @slogsdon ) would want/expect. It isn't really idiomatic. It would make more sense to have a separate function do what you are talking about. @danieljsummers snippet actually looks really nice. I think that path (pun!) seems more idiomatic.

@haf

This comment has been minimized.

Copy link
Contributor

haf commented Aug 7, 2017

+1 to PRs where we can discuss this over code.

@ademar ademar added the you-take-it label Oct 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment