Open kwladyka opened 5 years ago
Thank you for the issue report, but I'm afraid I don't understand your example. You're writing edn to the request body, but you're reporting a bug in form-encoded parameters, for middleware designed for use with form-encoded parameters.
This middleware affect body request by removing namespaces. I think it shouldn't. Otherwise developers have to make 2 separate app: first for JSON / EDN, second for POST forms or add middleware separate to each response function.
So if developer want to have responses for FORM inputs and JSON with namespaces in the same app:
1) has to figure out wrap-nested-params
is the source of the issue of disappearing namespaces
2) has to remove wrap-nested-params
from def app
3) has to add wrap-nested-params
separate in each function for handler to response for form request
For now I have plan to use only JSON / GraphQL and maybe EDN only, so this bug is not the issue for me at that moment. But I found it so I reported. Maybe somebody else will have the same issue and will find here solution. Figuring out this wrapper removes namespaces for requests take a while.
For me solution is to just remove this middleware, because I have no plan to use it. It was there because I copy it from another app as default scratch to begin.
This middleware affect body request by removing namespaces.
It's a little more specific than that. wrap-nested-params
uses name
to read the keys of the :params
map, so your issue is occuring because you have namespaced keywords in your :params
map when wrap-nested-params
is called.
One workaround is just to rearrange your middleware so that wrap-nested-params
and wrap-params
are placed more at the top of your middleware chain.
However, wrap-nested-params
should ideally ignore non-string keys, as they're not relevant for what it does. I'd accept a PR to fix that.
However,
wrap-nested-params
should ideally ignore non-string keys, as they're not relevant for what it does. I'd accept a PR to fix that.
I'll take a quick look at this
@weavejester When you say ignore non-string keys, you mean not operate on them and pass them along, yes?
@weavejester When you say ignore non-string keys, you mean not operate on them and pass them along, yes?
{:foo/bar 1}
pass as it is. By ignore he means not modify it.
@kwladyka Thanks for the confirmation
Spent a few hours looking at this (I'm new to Clojure, everything takes a while). I won't be doing any more with it. I'm including below my assessment should it prove useful for whoever picks it up.
By my fallible understanding, parse-nested-keys
(called by nested-params-request
, in turn called by wrap-nested-params
) was the most sensible place to make the change. Specifically would recommend adding a string check in the rematches
block of the let that destructures to [_ k ks]
, such that name
is still called if param-name
is a string and otherwise returned unmodified.
Aside from the unlikely possibility of somebody relying on the current stringifying or namespace-truncating behavior--and maybe the need to benchmark the old and new implementation if there are any doubts about the probably trivial-in-most-to-all-reasonable-contexts performance difference--it seems like just write a few failing tests and drop a string check in parse-nested-keys
.
(defn parse-nested-keys
"Parse a parameter name into a list of keys using a 'C'-like index
notation.
For example:
\"foo[bar][][baz]\"
=> [\"foo\" \"bar\" \"\" \"baz\"]"
[param-name]
(println param-name)
(let [[_ k ks] (re-matches #"(?s)(.*?)((?:\[.*?\])*)" (name param-name)) ; <- Add bare return here if param-name not string
keys (if ks (map second (re-seq #"\[(.*?)\]" ks)))]
(cons k keys)))
params
valueWhat is wrong:
When
wrap-nested-params
is uncomment it removes all namespaces in 1-deep, but no deeper.{:action "sign-up", :email "foo@example.com", :password "qwaszx", :foo #:bar{:baz "mee"}}
^ removed:user/
, but not:bar/
.When it is commented I get values as I expected:
{:action "sign-up", :user/email "foo@example.com", :user/password "qwaszx", :foo #:bar{:baz "mee"}}
How it should be It shouldn't remove namespaces.
{:action "sign-up", :user/email "foo@example.com", :user/password "qwaszx", :foo #:bar{:baz "mee"}}