Nested Parameters Revisited

[Josh Peek]

I want to revisit our nested parameters discussion before Rack 1.0. I
am in strong favor of having it the default params parser in Rack
core.

"Everyone is doing it!" Nested parameters are a well known convention
and pretty much every framework built on top of Rack is adding support
for nested parameters.

Extensions are clunky. In order to be friendly with other middleware
using the current parser, you have to build the nested parser on top
of the simple one. You can not rely on reading the original input
since it may of been already read and non-rewindable. So your stuck
with something like: def params; parse_nested_params(super); end. The
code could be much simpler if it was merged together.

Speed. Having to build on top of the existing parser means you have to
reiterate over all the params twice. Its much faster to do it all in
one pass. The implementation I posted, which was adapted from Merbs,
checks key.include?('[') before it any complex parsing so it adds very
little overhead if the key does not contain any []. Plus rack could
memoize so it would only need to be done once.

I don't think adding this feature to Request#params is a total breaker
for anyone either. I highly doubt people are using the foo[bar]
convention and expecting params["foo[bar]"] to work like that. However
it would cause more confusion if it sometimes parsed nested params and
other times it didn't depending on the middleware. Middleware should
always count on Rack::Request#params to be consistent.

I don't like the idea of a middleware preprocessor either. It breaks
any lazy loading behavior and it could screw up other middleware below
it that expect the params to not be parsed with nested hashes. Making
it a configurable option is bound to cause problems since it would
affect other middleware using rack request and not expect the params
to be parsed simple/nested.

We *need* one consistent parameters parser in Rack. Not many confusing
options like parse_simply, parse_nested, parse_nested_with_bacon, ...

If we can all agree that we need this, we can move on to discuss the
output spec. Please hold off on these comments, we can have another
fun discussion all about it after this :P

I've posted some *real* code on lighthouse: http://rack.lighthouseapp.com/projects/22435/tickets/3

Please vote +/- if you think we ought to have a nested params parser
be the default parser for Rack::Request#params.

Other discussions:
http://groups.google.com/group/rack-devel/browse_thread/thread/1a9b8dc431bff499#
http://github.com/bmizerany/sinatra/commit/1fa9807f4b99e6b61e6111cf9e99b8255fb0d217#comments
http://github.com/rack/rack-contrib/commit/86a2ced04d56572f7fc6b5651b78247309c37e18#comments

[Mark Bates]

+1

-------------------------------------------------------------------------------------------------
Mark Bates
ma...@mackframework.com
http://www.mackframework.com
http://api.mackframework.com/
http://github.com/markbates/mack

[Ezra Zygmuntowicz]

On Jan 25, 2009, at 6:05 PM, Josh Peek wrote:

> Please vote +/- if you think we ought to have a nested params parser
> be the default parser for Rack::Request#params.

+1

Ezra Zygmuntowicz
e...@engineyard.com

[Nathan]

+1

[Nicolás Sanguinetti]

Ah, well, I +1'd over at LH, but +1 here too :)

On Mon, Jan 26, 2009 at 12:43 AM, Nathan <XGam...@gmail.com> wrote:
>
> +1
>
> On Jan 25, 6:05 pm, Josh Peek <j...@joshpeek.com> wrote:

[James Britt]

Josh Peek wrote:

> Please vote +/- if you think we ought to have a nested params parser
> be the default parser for Rack::Request#params.

-1

--
James Britt

www.happycamperstudios.com - Wicked Cool Coding
www.jamesbritt.com - Playing with Better Toys
www.ruby-doc.org - Ruby Help & Documentation
www.rubystuff.com - The Ruby Store for Ruby Stuff

[Pratik]

On Mon, Jan 26, 2009 at 2:05 AM, Josh Peek <jo...@joshpeek.com> wrote:

> "Everyone is doing it!" Nested parameters are a well known convention
> and pretty much every framework built on top of Rack is adding support
> for nested parameters.

+1

--
Cheers!
- Pratik
http://m.onkey.org

[Yehuda Katz]

+1

--
Yehuda Katz
Developer | Engine Yard
(ph) 718.877.1325

[Dan Kubb (dkubb)]

> Please vote +/- if you think we ought to have a nested params parser
> be the default parser for Rack::Request#params.

+1

[Magnus Holm]

Dunno how much my vote counts, but whatever:

+1

//Magnus

[Ezra Zygmuntowicz]

On Jan 25, 2009, at 8:23 PM, James Britt wrote:

> -1
>
> --
> James Britt

I gotta ask what you think the downside of this is?

Thanks-
-Ezra

[Michael Fellinger]

On Mon, Jan 26, 2009 at 11:05 AM, Josh Peek <jo...@joshpeek.com> wrote:
> I've posted some *real* code on lighthouse: http://rack.lighthouseapp.com/projects/22435/tickets/3
>
> Please vote +/- if you think we ought to have a nested params parser
> be the default parser for Rack::Request#params.

I think we've tried a lot of different solutions which all have downsides.
The downside of this approach is added complexity of the request body
parsing which is quite complex already.
But having consistent params over all rack applications and the speed
increase with lower complexity on the rack application side are well
worth it.
We just need to make sure that we have an implementation that is fully
specified and has tests (preferably ported from merb/rails).

+1

^ manveru

[Matt Todd]

+1

--
Matt Todd
Highgroove Studios
www.highgroove.com
cell: 404-314-2612
blog: maraby.org

Scout - Web Monitoring and Reporting Software
www.scoutapp.com

[Scytrin dai Kinthra]

~1

--
stadik.net

[Adrian Madrid]

+1

Adrian Esteban Madrid
Lead Developer, Prefab Markets
http://www.prefabmarkets.com

[Christian Neukirchen]

Josh Peek <jo...@joshpeek.com> writes:

> I've posted some *real* code on lighthouse: http://rack.lighthouseapp.com/projects/22435/tickets/3

+1

We may want to keep the current method as #simple_parse or something.

--
Christian Neukirchen <chneuk...@gmail.com> http://chneukirchen.org

[Joshua Peek]

On Mon, Jan 26, 2009 at 2:20 PM, Christian Neukirchen
<chneuk...@gmail.com> wrote:
> +1

Hurray!

I'll make another post regarding the specs and API later today.

> We may want to keep the current method as #simple_parse or something.

Sounds like a fair compromise. But why? :)

Do you think its a performance issue, or that people are expecting
params["x[y]"] to still work?

--
Joshua Peek

[Josh Peek]

Lets work out the API details

http://groups.google.com/group/rack-devel/browse_thread/thread/c6583ccb5779408c

[Christian Neukirchen]

Joshua Peek <jo...@joshpeek.com> writes:

> Do you think its a performance issue, or that people are expecting
> params["x[y]"] to still work?

I could imagine this to be useful in certain cases (e.g. when the
field names are user-input or something.) It won't hurt to have.

[James Britt]

I'd prefer the core of Rack itself to not bundle in presumptions about
the data begin sent, what it means, and how it should be re-presented,
however popular those presumptions are right now.

What I've liked about Rack was that it did very little, but did just
enough to build on top of, in whatever way I prefer.

[Matt Todd]

My argument for nested params would be very similar to yours against, ironically. :)

> What I've liked about Rack was that it did very little, but did just enough to build on top of, in whatever way I prefer.

I think the problem is that almost every framework (though  not all) have rewritten the nested params parsing code which is an antipattern for what Rack represents as a stabilizing standard for web development for Ruby.

Matt

[James Britt]

Matt Todd wrote:
> My argument for nested params would be very similar to yours against,
> ironically. :)
>> What I've liked about Rack was that it did very little, but did just
> enough to build on top of, in whatever way I prefer.
>
> I think the problem is that almost every framework (though not all) have
> rewritten the nested params parsing code which is an antipattern for what
> Rack represents as a stabilizing standard for web development for Ruby.

I get that.

My concern is that there may be a set of param values or request headers
that are perfectly legit according to RFC 2616, but cause grief
because they don't adhere to some currently preferred interpretation
that's been baked in.

[Sam Roberts]

On Mon, Jan 26, 2009 at 9:15 PM, James Britt <james...@gmail.com> wrote:
> Matt Todd wrote:
>> I think the problem is that almost every framework (though not all) have
>> rewritten the nested params parsing code which is an antipattern for what
>> Rack represents as a stabilizing standard for web development for Ruby.
>
> I get that.
>
> My concern is that there may be a set of param values or request headers
> that are perfectly legit according to RFC 2616, but cause grief because
> they don't adhere to some currently preferred interpretation that's been
> baked in.

I'm assuming this layering of extra meaning into the query part would
be an optional rack middleware?

Sam

[Matt Todd]

My concern is that there may be a set of param values or request headers  that are perfectly legit according to RFC 2616, but cause grief because they don't adhere to some currently preferred interpretation that's been baked in.

I share the concern. I'm afraid though that too much concern could get in the way of going with sensible behavior.

I hate to say something like "most frameworks do it so it should be the standard", I'm not in favor of neglecting or ignoring the needs and desires of any framework or developer.

I think that we can sensibly default to parsing nested params and either through a config value (like Rack.config[:parse_nested_params]) use the simpler, flat method, or with an include that overrides the method definition. There could be other options as well.

At this point, I think Josh Peek's approach of actually enumerating what the spec should look like will be more fruitful.

Thoughts?

Matt

 

[Ryan Tomayko]

On 1/26/09 10:46 PM, Matt Todd wrote:
> I share the concern. I'm afraid though that too much concern could get
> in the way of going with sensible behavior.

<snip>

> At this point, I think Josh Peek's approach of actually enumerating what
> the spec should look like will be more fruitful.

+1

It seems a general consensus has been reached that a good implementation
is worth included in core. Let's see the proposed specs/implementations
so we have something fruitful to throw stones at :)

Thanks
--
Ryan http://tomayko.com/about

[Christian Neukirchen]

James Britt <james...@gmail.com> writes:

> What I've liked about Rack was that it did very little, but did just
> enough to build on top of, in whatever way I prefer.

Absolutely nothing stops you from parsing env["QUERY_STRING"] as you prefer.

[James Britt]

Christian Neukirchen wrote:
> James Britt <james...@gmail.com> writes:
>
>> What I've liked about Rack was that it did very little, but did just
>> enough to build on top of, in whatever way I prefer.
>
> Absolutely nothing stops you from parsing env["QUERY_STRING"] as you prefer.

Quite true, but it's an empty argument since the same can be said for
people who want to add a layer of semantics that is not part of any Web RFC.

I understand that 99% of people using Rack will prefer to think of form
POST data and query strings as nested hashes and arrays, not as
strings. But it changes what Rack is. It's no longer a minimal common
Web API.

Whether that is good or bad is another matter. So far, most people
think it a good thing.

[James Tucker]

Make it a plugin / configurable?

[Christian Neukirchen]

James Britt <james...@gmail.com> writes:

> I understand that 99% of people using Rack will prefer to think of
> form POST data and query strings as nested hashes and arrays, not as
> strings. But it changes what Rack is. It's no longer a minimal
> common Web API.

Rack is the SPEC.

Rack includes utilities, e.g. Rack::Request, which make the spec
actually usuable. If you don't like them, you can just write your own
utilities, in conformance with the spec, and everything will work
together.

[James Britt]

Christian Neukirchen wrote:
> James Britt <james...@gmail.com> writes:
>
>> I understand that 99% of people using Rack will prefer to think of
>> form POST data and query strings as nested hashes and arrays, not as
>> strings. But it changes what Rack is. It's no longer a minimal
>> common Web API.
>
> Rack is the SPEC.
>
> Rack includes utilities, e.g. Rack::Request, which make the spec
> actually usuable. If you don't like them, you can just write your own
> utilities, in conformance with the spec, and everything will work
> together.
>

Got it.