open #http_req{} for include

[benoitc]

Latest change in cowboy made #http_req{} opaque . Due to this change we can only access to its properties using the cowboy_req api which is inefficient when you want to access to more than one property at the same time.

Can you make it available again or at least propose a way to query them if you don't want to rely on an include file?

Here is a sample usage in mochicow:

benoitc/mochicow@914ae2b

[essen]

Please see #266.

Also careful there, a new value was added (fragment) and another will very likely be removed (urldecode). There might also be a resp_cookie value added later on.

[benoitc]

well I edietd it. This is all the point of this ticket. It would be easier if #http_req{} would be available in an include file. So nobody have to worry about possible changes in :)

[essen]

But then people start to use it when they shouldn't (not everyone uses dialyzer), and if it becomes a frame or a dict or whatever the file gets removed anyway. There's only one way to do it properly. :)

[essen]

By the way I was thinking of adding a single (most likely undocumented, at least for now) function that could work like:

[Host, Path, Headers] = cowboy_req:get([host, path, headers], Req).

[rambocoder]

Can the list of atoms in the get([], Req) be arbitrary. For example, what if I want to know if it is an http or https connection, or many other request parameters that do not change the state of the request. Maybe it can return a proper list, like [{host, Host}, {transport, Transport}]...

[essen]

It'd be the equivalent of doing:

#http_req{host=Host, path=Path, headers=Headers} = Req.

with the understanding that if a value is lazy evaluated it won't get evaluated by this call.

[benoitc]

Get is easy to fix I think. But how would you do that :

https://github.com/benoitc/mochicow/blob/master/src/mochicow_upgrade.erl#L86

? Maybe there is a cleaner way to do that

[essen]

I did it with a special function tied to my needs in cowboy_protocol but it can be generalized like this:

set([{buffer, Buffer}, {resp_state, RespState}, ...], Req).

This would allow doing low level access/changes on Req without needing to know the structure (you still need to understand the behaviors with regards to lazy evaluation particularly, but can't really do anything about that).

[rambocoder]

Can you explain what role lazy evaluation plays in cowboy_req?

[essen]

Everything that can be parsed later is parsed later. This allows us to save CPU and memory for things you don't necessarily need server-side. For example if your URL has a query string but the query string is only used by client-side Javascript, then Cowboy won't ever waste time parsing it. This is done for various things, I let you look at the file to see what's lazy and what's not today.

[rambocoder]

Is this an example of lazy evaluation? https://github.com/extend/cowboy/blob/master/src/cowboy_req.erl#L219
The peer address and port number of the remote host are not parsed until they are needed?

And cookies is also an example?
https://github.com/extend/cowboy/blob/master/src/cowboy_req.erl#L504

As well as query strings?
https://github.com/extend/cowboy/blob/master/src/cowboy_req.erl#L305

Is this a correct explanation of how this lazy evaluation works: by default, the records fields are undefined, so when you call API cowboy_req:qs_vals, erlang will pattern match to a function that has the field as undefined, or to a function that will just return the value. If it goes to the function matched to the undefined field, your code parses data and sets the field value, effectivelly caching it for future use, because next time I call the API function, erlang will match to a function that does not have a signature of qs_vals=undefined

This makes sence to me and is a pretty cool pattern. I just had to stare at the code some more and give your last comment some time to sink in.

[essen]

Yes, that's it. :)

[essen]

Guess it's better called "lazy parsing" than evaluation.

[essen]

There you go: c326a19.

I believe this should be more than enough for any purpose.

[essen]

Closing this now but feel free to reopen if I overlooked something. :)

[benoitc]

looks good for me. Would you be interrested by a patch ading mget/mset as well ? so we could do :

mget([headers, path, host, qs, buffer]) and get the result in a proplist. What do you think ?

[benoitc]

also thanks!

[essen]

I think that you can build the proplist easily with lists:zip. :)

[benoitc]

indeed :)

[essen]

I mean get already allows retrieving many values, it just returns them as a list of value instead of a proplist, so there isn't much to do past that point to have a proplist.

[benoitc]

true. I misread that line. Was thinking you were convertink keys to atom... nvm then