couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benoit Chesneau <bchesn...@gmail.com>
Subject Re: trouble with URL rewriting for key/startkey/endkey qs params
Date Tue, 22 Feb 2011 08:52:30 GMT
On Tue, Feb 22, 2011 at 6:15 AM, Adam Kocoloski <kocolosk@apache.org> 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

Mime
View raw message