Skip to content
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

Before/after middlewares called on every route. #69

Closed
jcreager opened this issue Aug 3, 2020 · 4 comments
Closed

Before/after middlewares called on every route. #69

jcreager opened this issue Aug 3, 2020 · 4 comments

Comments

@jcreager
Copy link

jcreager commented Aug 3, 2020

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)
  1. joy server
  2. 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
Copy link
Collaborator

swlkr commented Aug 7, 2020

I pushed a fix for this here 01f6b8d 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
Copy link
Author

jcreager commented Aug 8, 2020

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

@jcreager
Copy link
Author

jcreager commented Aug 8, 2020

@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!

@jcreager jcreager closed this as completed Aug 8, 2020
@swlkr
Copy link
Collaborator

swlkr commented Aug 8, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants