On Tue, Feb 22, 2011 at 6:15 AM, Adam Kocoloski <[email protected]> wrote: > Hi, a user pointed out that a rewrite rule which adds quotes around a > variable causes the variable substution not to happen, e.g. > > { > "query": { "key": "\":arg\"" } > } > > I had to stare a long time at couch_httpd_rewrite to see why that was the > case. I found the replace_var/3 function to be awesomely gnarly: > > https://github.com/apache/couchdb/blob/trunk/src/couchdb/couch_httpd_rewrite.erl#L256 > > There are so many bad things going on in this function: > > 1) I'm pretty sure the final clause of the case statement can never be > reached. Calls to replace_var/3 are guarded by is_list/1 and is_binary/1 > guards on the Value, so we're always going to hit one of the previous clauses. > > 2) Putting that aside for a moment, what was the point of the inner case > statement? Are we writing code that distinguishes between > lists:flatten(?JSON_ENCODE(Value)) and iolist_to_binary(?JSON_ENCODE(Value)) > ? I hope not. > > 3) I found the is_list(Value) clause to be almost a copy of the outer clause, > but not quite. It looks to me like that clause is being used to handle the > situation like "query":{"key": [":a",":b"]}, but I believe there are better > ways to do that.
> > 4) Sure enough, the rewriter requires an exact match on <<":", Var/binary>> > in order to do the substitution, and it does not JSON encode the result, even > if the query key is something that requires a JSON-encoded value. In effect, > I see no way to use the rewriter to set the value of key to a string. Not sure I follow, the key is supposed to be a binary here. > > I started to refactor this function a bit and ended up with > http://friendpaste.com/ZzyuR2gM0JVdDmgGhjXbI. It still doesn't address the > root problem of the JSON-encoding requirement for key/startkey/endkey/etc., > but it's a start. I've never had occasion to look closely at the rewriter > before, so someone else should check to make sure I'm not way out in left > field here. > > Adam Hi, thanks to go back on this topic. I've already send a mail 1 month or 2 about that. There are different concerns to handl in the rewriter. - some peope are passing a list values via the query parameters. Values can be integer here, in this case last case is use or in array the 2 jspon - some values can be wildcards detected from the rules - values can just be binaries starting by a ":" - values can be lists or array - values can be string in the case a value is coming from url rewriting., values can be lists in an array. The guard: _ when is_list(Value) -> Value1 = lists:foldr(fun(V, Acc) -> V1 = case V of <<":", VName/binary>> -> case get_var(VName, Bindings, V) of V2 when is_list(V2) -> iolist_to_binary(V2); V2 -> V2 end; <<"*">> -> get_var(V, Bindings, V); _ -> V end, [V1|Acc] end, [], Value), to_json(Value1); was trying to handle the last 2 cases. I didn't found the right way for now to detect strings against lists. It seems your diff handle only 2 cases, the ":" pattern matching I'm not happy with that and raised the issue long time ago and recently I've openened a thread for that on the ml without too much reactions: http://couchdb.markmail.org/thread/47iahuy4po55am4c The current designer don't handle and can't handle all the cases wanted by a user. Main problem is to manage different values coming. And for other they want a more complex rewriting rule. At the end I've rewritten the rewritter to achieve that: https://github.com/benoitc/couchapp-ng/ The code of the new rewriter is also a little more simple. It also remove some oddities introcuced since I wrote the code like this useless atom transformation (to_binding) that I don't quite understand, etc. It also introduce some new features like directly allowing setup of proxies. This code online isn't the last version I will use in upondata.com . I'm not claiming this code should replace the current one. We could start by changing the way the replace_var is working, but we surely will have hard time to do that. The first thing to do would be to find a way to detect strings vs lists. The version of couchapp_ng handle that by fixing patterns associated to a key. - benoit
