domkm / silk

Routing for Clojure & ClojureScript
222 stars 13 forks source link

default query parameter not being merged within [:domkm.silk/url :query] #7

Closed geraldodev closed 9 years ago

geraldodev commented 9 years ago

I'was kind of rewriting a buddy example https://github.com/niwibe/buddy/blob/master/examples/sessionexample/src/sessionexample/core.clj which uses another routing library and came to this:

The :next default key is present but not within [:domkm.silk/url :query]

(silk/arrive 
  (silk/routes [[:home-page [["login"] {"next" (silk/? :next {:next "/"}) }] ]]) 
  "/login?foo=bar")
dev=> (pp)
{:domkm.silk/url
 {:scheme nil,
  :user nil,
  :host nil,
  :port nil,
  :path ["login"],
  :query {"foo" "bar"},
  :fragment nil},
 :domkm.silk/routes #<Routes domkm.silk.Routes@5c383a2f>,
 :domkm.silk/pattern
 {:query
  {"next"
   #<silk$_QMARK_$reify__10692 domkm.silk$_QMARK_$reify__10692@6f7412f5>},
  :path ["login"]},
 :domkm.silk/name :home-page,
 :next "/"}

Two points: When you have just one query parameter and it's defined as default. 1) The default mechanism does not trigger unless you have a bogus query parameter as above

dev=> (silk/arrive 
        (silk/routes [[:home-page 
                       [["login"] {"next" (silk/? :next {:next "/"}) }] ]]) 
        "/login")
nil

That forces me to write two rules for this case.

2) Even when it triggers the default value does not appear in :query

leblowl commented 9 years ago

I am also running into this problem. Not sure if the best way, but I would also like to attach data to routes and default query params appears to be one way to accomplish this.

(silk/match routes {:path ["login"] :query {}})

also works for the case above, but you still have to specify :query.

domkm commented 9 years ago

@geraldodev Sorry for dropping the ball on this issue. I forgot about it. :frowning:

@leblowl Thanks for reminding me. :smiley:

Unfortunately, I don't have time to fix this right now. Sorry about that. Please be patient and stay tuned to this issue. I haven't forgotten about Silk. :wink:

Deraen commented 9 years ago

Hi,

I also bumped into this. I found two workarounds for my problem but I think this could be quite easily fixed.

(def routes
  (silk/routes [[:search [["users"] {"q" :q}]]
                [:search [["users"]]]]))

(def routes
  (silk/routes {:search [["users"] (silk/? {"q" (silk/? :q {:q "a"})} {:q "b"})]}))
; If no query params at all -> {:q "b"}, if has query params but no "q" -> {:q "a"}.

I took a quick look at the implementation and it looks to me that url function used by arrive already creates empty map for query params always, so the problem is probably that Pattern implementation of PersistentArrayMap is only calling match for keys which are present in the url.

Maybe the match implementation of map could call match-coll with something like (set (concat (keys this) (keys that)))? Then match would be called for query string keys in the url and in the pattern?

Edit: I took another look at decode-query and now I see that it's returning nil instead of empty map if the url doesn't have query string at all.