joy-framework / joy

A full stack web framework written in janet
https://joy.swlkr.com
MIT License
532 stars 29 forks source link

Before/after middlewares called on every route. #69

Closed jcreager closed 4 years ago

jcreager commented 4 years ago

I like the idea of the new before/after middlewares in 0.9. However, it looks like they are called every route.

Version
joy version
0.9.0
Steps to reproduce
  1. Create a new joy project.
  2. Edit main.janet
  3. Add a before middleware.
    (before "/foo/*" :run-before)
    (defn run-before [req]
    (pp req)
    req)
  4. joy server
  5. curl localhost:9001
Expected Result

The middleware for /foo/* is not invoked.

Actual Result

The middleware for /foo/* is invoked on /.

More info

I believe the source of the issue is the wildcard-params function.

(defn- wildcard-params [patt uri]
  (let [p (->> (string/split "*" patt)
               (interpose :param)
               (filter |(not (empty? $)))
               (slash-suffix)
               (freeze))

        route-peg ~{:param (<- (some (+ :w (set "%$-_.+!*'(),"))))
                    :slash-param (<- (some (+ :w (set "%$-_.+!*'(),/"))))
                    :main (* ,;p)}]

    (or (peg/match route-peg uri)
        @[])))

If peg/match is not truthy, this function returns @[]. However, @[] is truthy.

janet:1:> (if @[] true)
true

If this function is modified to return the result of peg/match before/after middlewares appear to work as expected. For example:

(defn- wildcard-params [patt uri]
  (let [p (->> (string/split "*" patt)
               (interpose :param)
               (filter |(not (empty? $)))
               (slash-suffix)
               (freeze))

        route-peg ~{:param (<- (some (+ :w (set "%$-_.+!*'(),"))))
                    :slash-param (<- (some (+ :w (set "%$-_.+!*'(),/"))))
                    :main (* ,;p)}]

    (peg/match route-peg uri)))

I think the intent was to keep the returned values consistent in this function. Since result of wildcard-params is not consumed in with-before-middleware or with-after-middleware it might be enough to have a small empty check called in the when-let binding of those functions.

This probably isn't as elegant as what you are looking for but it could be something like this.

(defn not-empty? [arr] (> (length arr) 0))

And used like:

janet:10:> (when-let [foo (not-empty? @[])] (if foo true))
nil

I'm still a little new to Janet so there is a good chance this is covered in the standard library and I'm not aware of it.

If I can provide any additional info let me know. Also if I'm way of base about the intended behavior of the before/after middlewares let me know. Thanks!

swlkr commented 4 years ago

I pushed a fix for this here https://github.com/joy-framework/joy/commit/01f6b8d8ad3cfefc5c61890562d173b579b16870 let me know if it helped out.

I also had the requirement to show the last visited url after signing in, this is what I came up with:

(after "*" :return-to)
(defn return-to [request response]
  (if (and (not= (request :uri) "/sign-in")
           (get? request))
    (let [session (merge (or (request :session) @{}) (or (response :session) @{}))
          session (merge session {:return-to (request :uri)})]
      (merge response {:session session}))
    response))
jcreager commented 4 years ago

Awesome, thanks @swlkr. I'll check out that hash. Also, thanks for the info on the last visited url middleware.

jcreager commented 4 years ago

@swlkr 01f6b8d appears to have fixed this issue.

One thing to note is that I needed to upgrade to Janet 1.11.3 from 1.10.0. Otherwise I got this error:

compile error: unknown symbol any? on line 186, column 19 while compiling /usr/local/Cellar/janet/1.10.0/lib/janet/joy/router.janet

any? was added in 1.11.0.

I'm going to close this issue. Thanks for this fix!

swlkr commented 4 years ago

Ah yes, we’re at the bleeding edge here in joy land haha

I’m going to mention which version of janet joy is compatible with on the readme