v2.0.0 (DO NOT MERGE)

[noprompt]

Version 2.0.0 is primarily focused on addressing issues #16 and #41 as well as cleaning up some of the API. As with any major release, this means breaking changes.

Solution to #16

Solving #16 required the removal of the global *routes* atom. Removing it had a dramatic impact on the code consequently

• creating a new record type Route ([pattern action]) which specifies default implementations for all of the routing protocols. It also implements IFn which defaults to render-route.
• adding a new make-route function as a convenience for creating routes. This is now what defroute uses reducing the complexity of the macro.
• removing functions such as locate-route and locate-route-value. Since routes are no longer stored in a container managed by Secretary, these can be implemented by the library consumer.

Solution to #41

Solving #41 required solving #16 in order to create customer dispatchers. ~~There is a new data type Dispatcher ([routes handler]) which is simply a container for housing routes and a handler function ([routes dispatch-val]). The Dispatcher type implements a unary IFn which takes a dispatch value and delegates to the handler.~~ This means

• removing dispatch!. We simply do not need this function any longer.
• creating a new function uri-dispatcher [routes handler] which returns ~~an instance of Dispatcher~~ a unary function of [uri] that does most of the work the original dispatch! function did with the exception handler is a ring style handler. Why is it called uri-dispatcher? Because that is what it's designed to do: dispatch actions based on URIs. There is nothing in the routing protocols that say patterns must apply to URIs. A input-dispatcher might be used to dispatch against the values of <input> element.
• removing *config*. This was a "fix it for now" artifact. The problem of prepending/removing prefixes and ensuring leading slashes shouldn't be a concern for Secretary. These are problems which are better addressed by the library consumer. For example, to ensure a URI has a leading / one can create a function which ~~creates a new instance of Dispatcher wrapping the Dispatcher return from~~ wraps uri-dispatcher. Prepending prefixes to Routes can be handled by overriding IFn with an implementation that does so.

(defroute r1 "/foo/:x" {{:keys [x]} :params :as req}
  [x req])

(defroute r2 "/foo/:y/bar" {{:keys [y]} :params :as req}
  [y req])

(defroute r3 #"/bar/(\d+)" {[id] :params :as req}
  [(js/parseInt id) req])

(defroute r4 #"/bar/([a-z]+)" {[alpha] :params :as req}
  [alpha req])

(let [d (uri-dispatcher
         [r1 r2 r3 r4]
         (fn [handler]
           (fn [req]
             (let [route-name (get {r1 :foo/x
                                    r2 :foo/y
                                    r3 :bar/digit
                                    r4 :bar/alpha}
                                   (:route req)
                                   :not-found)]
               (handler (assoc req :route-name route-name))))))]
  {:r1 (d "/foo/hello") 
   :r2 (d "/foo/goodbye/bar")
   :r3 (d "/bar/100")
   :r4 (d "/bar/fizzle")})

;; =>

{:r1 ["hello" {:uri "/foo/hello"
               :query-string nil
               :query-params {}
               :params {:x "hello"}
               :route-name :foo/x}]
 :r2 ["goodbye" {:uri "/foo/goodbye/bar"
                 :query-string nil
                 :query-params {}
                 :params {:y "goodbye"}
                 :route-name :foo/y}]
 :r3 [100 {:uri "/bar/100"
           :query-string nil
           :query-params {}
           :params ["100"]
           :route-name :bar/digit}]
 :r4 ["fizzle" {:uri "/bar/fizzle"
                :query-string nil
                :query-params {}
                :params ["fizzle"]
                :route-name :bar/alpha}]}

Clean up

In the spirit of Ring, query-parameter encoding/decoding and serialization has been moved from secretary.core to secretary.codec.

Won't fix?

I do not believe, although I'm open to conversation, we need to implement #18, defroutes, or other concepts from Compojure. My rational, again, being that these are library consumer decisions and are trivial to implement when needed.

Summary

This version focuses on trimming down the parts of the API we simply do not need and creating the right abstractions and functions such that solving client-side routing issues is easy and flexible. These changes will definitely break existing applications which use Secretary and upgrading will require extra on behalf of the consumer (although hopefully not much). Overall, I hope many of these changes will be welcome.

[jeluard]

This looks great!

I have one scenario in mind that currently requires some gymnastics. I'm wondering how it could be improved or if I'm completely overlooking it.

The scenario is the following: a page requires some extra dispatching for logged users, on top of regular dispatching applied to anonymous users. Because checking for logged users take some time the dispatching is done in 2 steps. Also this check is performed via JS and does not impact the URL.

Currently I'm handling this via atoms set before calling dispatch. Could the API helps with that? Especially I'm wondering if some extra arguments could be provided to the function returned by uri-dispatcher then propagated to routes. Would that make sense?

[noprompt]

@jeluard Hmm... I don't think there is anything special about the new API that would apply to that problem specifically. OTOH you could create middleware which would check if the user is logged in and

1. if they are, proceed normally
2. if they are not, update the value of :route in the request map to either
   1. be a different instance of Route (pseudo redirect)
   2. be a new instance of Route but with a different value for :action

Since Route is a record type you could just (assoc-in req [:route :action] action-when-not-logged-in). It would still be called with all of the same parameters, etc. You could even wrap the original :action in another function as a sort of proxy, etc. It's really up to you at the point because once you have the Route it's just data and function manipulation at that point.

Does that help?

[noprompt]

@jeluard This might be a better illustration

(defn wrap-add-login-status
  [handler]
  (fn [req]
    (handler (assoc req :logged-in? (logged-in?)))))

(defn wrap-authorized
  [handler]
  (fn [req]
    (if (:logged-in? req)
      (handler (assoc-in req [:params :secret-data] 42))
      ;; Alter the action.
      (handler (assoc-in req [:route :action] unauthorized-action)))))

FWIW, you could build a completely custom dispatching function. Really there's not a lot to it. You create some Routes and you build a function that does something with them. Like I said above, routing doesn't necessarily have to apply to URIs. It's simply calling a function f with matches ms extracted from a pattern p whenever p matches x.

[jeluard]

Sure this looks great! I still have to wrap my head around it but definitively looks like the necessary flexibility is there. Thanks!

[noprompt]

@jeluard If it helps at all have a look at very simple dispatcher here in the tests.

[stephanos]

Just wondering, what is the status of this? I'm looking at silk and secretary for my new project and would love to use secretary2 for the comparison.

[noprompt]

@stephanos I just need to write the documentation for it and patch one more thing. Other than that it's pretty much ready to go. We're using this branch in production right now with no problems. Look at the README on this branch to get the current version.

The biggest difference between Silk and Secretary is that Silk supports Clojure. This can be nice if your project has both a front-end and a back-end. Since most of the projects we work on at work do not do this I've never had the motivation to add Clojure support to Secretary (although it really wouldn't require that much more effort).

A lot of the problems that were brought up regarding Secretary on the mailing list now have largely been corrected on this branch.

[noprompt]

@stephanos I should clarify that there are other design differences between Silk and Secretary but in my opinion there isn't a lot of contrast between the two. If you find that you like Silk more than Secretary, I'd like to hear your thoughts.

[dgellow]

Hello @noprompt. What is the status of this pull request?

[noprompt]

@dgellow It's a bit stale at the moment. :disappointed:

[dgellow]

And what is the current state of the pull request? What is working, what still need to be done?

Ultimately on what parts can we work to help you?

In december 2014 you wrote:

I just need to write the documentation for it and patch one more thing. Other than that it's pretty much ready to go. We're using this branch in production right now with no problems. Look at the README on this branch to get the current version.

Is it still the case (just missing some documentation and a patch)?

[noprompt]

@dgellow Sort of. There have been some commits to master since this branch was created, some of it may need to be applied to this branch if it's relevant. Also I think there might be some work around defroute but I can't remember off-hand what the issues were with it. Other things to update would probably include the project.clj to use whatever the latest versions of Clojure and ClojureScript are.

If you're serious please send me a pull request for this branch. After review, I'd be happy to release a new version.

[zonotope]

@noprompt, I submitted this pull request yesterday that resolves all the conflicts with master and updates all of secretary's dependencies.

Do you remember what the remaining work around defroute was? I'd be happy to try and take it on.

[dgellow]

@zonotope good job! :+1:

[noprompt]

@zonotope This is awesome, thanks for stepping up. You know, I don't necessarily think that defroute has to be available right away. I'd be fine with cutting a v2.0.0-alpha release or something like that. I find defroute difficult to resolve in the context of the new design. The check for :ring-route? is particularly gross.

tl;dr I'd be happy to take it out for now for the sake of getting an alpha out.

[zonotope]

@noprompt I don't think we need a v2.0.0-alpha release if everyting isn't fully baked yet. We can just use v2.0.0.1-2b0752 or whatever in dependent projects until we figure everything out.

I opened another branch and made some tweaks to the macro definitions

First, I made some changes to route-action-form I couldn't quite get rid of the :ring-route? check, but I think I made the code a little clearer.

Also, I added a route macro to make defining anonymous routes a little easier. That macro reused most of the logic from defroute, so I redefined defroute in terms of it.

What do you think of those changes?

[noprompt]

@zonotope These additions are great. I especially like what you've done with route. Often I find that piece is nested inside a defthing form and it's nice to just have the thing on it's own e.g. for use in let.

I think I can get on board discarding the alpha and using the version format with the identifier (-2b0752) you've proposed. Perhaps we could drop one of the .0 so it's just a normal version with the identifier. I'm not sure we need three points.

How does the documentation look? It's been a while and I don't remember how far along I got syncing that up. If that looks good I think all that's necessary from me is to pull this down and try it out.

Also, if you're cool with it, I think this earns you contributor status once it's merged. @gf3 what do you think?

[noprompt]

@zonotope Sorry I dropped the ball on this. I guess I can take a look at the documentation and get this merged soon.

[borkdude]

@zonotope Your PR has been merged. @noprompt If you want, we can grant you access so you can finish this branch at one point (EDIT: I see you already have access).