[PATCH] Request#accept_media_types

[PATCH] Request#accept_media_types

[mynyml]

Patch is at: http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64eaf24d9e2b
-----
Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
types by "quality" (preference level) (following HTTP 1.1 specs).

The main advantage of being able to parse the Accept header is to
centralize the lookup for the request's "desired" type(s). Usually, a
client asks for a specific media type to be sent back by appending an
extension to the URL; not only does this require parsing by every
middleware/app that needs to know about this media type, but it also
ignores the Accept hierarchy, which can be usefull for cascading down
possible handlings of the resquest (think of a respond_to mechanism,
for instance).

I've encountered many use cases already:

o http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404fb5bd8d9f1eb/lib/rack/contrib/accept_format.rb
o http://github.com/mynyml/rack-abstract-format
o http://github.com/mynyml/rack-supported-media-types
o http://github.com/mynyml/rack-js4xhr
o http://github.com/mynyml/rack-respond_to

which led me to write the Accept header parsing code:

o http://github.com/mynyml/rack-accept-media-types

In my rack clone, the request_accept branch is a straight port of the
rack-accept-media-types gem, and the request_accept-compact branch
minimizes the code into two methods (instead of two classes). The
latter reflects the rest of the Request class much better so it's
probably a more appropriate patch.

Feedback/questions/suggestions ?

Thanks

Re: [PATCH] Request#accept_media_types

[Joshua Hull]

Just a couple of comments on this:

types = types.select {|type| media_type_quality(type).between?(0.1, 1) }

this should probably check that its greater than 0 and less than or
equal to 1 instead

types = types.select {|type| quality = media_type_quality(type);
quality > 0 && quality <= 1 }

so that values of 0.01 still get through.

As well, the default value of all types is 1.0, but we probably want
to do something similar to what apache does, and assign */* a value of
0.01 and type/* a value of 0.02.

Cheers
Joshua

On Fri, Jun 12, 2009 at 5:48 PM, mynyml<martin.aumont@gmail.com> wrote:
>
> Patch is at: http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64eaf24d9e2b
> -----
> Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
> types by "quality" (preference level) (following HTTP 1.1 specs).
>
> The main advantage of being able to parse the Accept header is to
> centralize the lookup for the request's "desired" type(s). Usually, a
> client asks for a specific media type to be sent back by appending an
> extension to the URL; not only does this require parsing by every
> middleware/app that needs to know about this media type, but it also
> ignores the Accept hierarchy, which can be usefull for cascading down
> possible handlings of the resquest (think of a respond_to mechanism,
> for instance).
>
> I've encountered many use cases already:
>
> o http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404fb5bd8d9f1eb/lib/rack/contrib/accept_format.rb
> o http://github.com/mynyml/rack-abstract-format
> o http://github.com/mynyml/rack-supported-media-types
> o http://github.com/mynyml/rack-js4xhr
> o http://github.com/mynyml/rack-respond_to
>
> which led me to write the Accept header parsing code:
>
> o http://github.com/mynyml/rack-accept-media-types
>
> In my rack clone, the request_accept branch is a straight port of the
> rack-accept-media-types gem, and the request_accept-compact branch
> minimizes the code into two methods (instead of two classes). The
> latter reflects the rest of the Request class much better so it's
> probably a more appropriate patch.
>
> Feedback/questions/suggestions ?
>
> Thanks

Re: [PATCH] Request#accept_media_types

[Yehuda Katz]

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

Take a look at what we do in Merb. We've optimized our Accept header parsing
quite a bit (for performance; it can be shockingly slow), and handle quality
factors (both absolute and relative).

-- Yehuda

On Sun, Jun 14, 2009 at 9:50 AM, Joshua Hull <joshbuddy@gmail.com> wrote:

>
> Just a couple of comments on this:
>
> types = types.select {|type| media_type_quality(type).between?(0.1, 1) }
>
> this should probably check that its greater than 0 and less than or
> equal to 1 instead
>
> types = types.select {|type| quality = media_type_quality(type);
> quality > 0 && quality <= 1 }
>
> so that values of 0.01 still get through.
>
> As well, the default value of all types is 1.0, but we probably want
> to do something similar to what apache does, and assign */* a value of
> 0.01 and type/* a value of 0.02.
>
> Cheers
> Joshua
>
> On Fri, Jun 12, 2009 at 5:48 PM, mynyml<martin.aumont@gmail.com> wrote:
> >
> > Patch is at:
> http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64eaf24d9e2b
> > -----
> > Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
> > types by "quality" (preference level) (following HTTP 1.1 specs).
> >
> > The main advantage of being able to parse the Accept header is to
> > centralize the lookup for the request's "desired" type(s). Usually, a
> > client asks for a specific media type to be sent back by appending an
> > extension to the URL; not only does this require parsing by every
> > middleware/app that needs to know about this media type, but it also
> > ignores the Accept hierarchy, which can be usefull for cascading down
> > possible handlings of the resquest (think of a respond_to mechanism,
> > for instance).
> >
> > I've encountered many use cases already:
> >
> > o
> http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404fb5bd8d9f1eb/lib/rack/contrib/accept_format.rb
> > o http://github.com/mynyml/rack-abstract-format
> > o http://github.com/mynyml/rack-supported-media-types
> > o http://github.com/mynyml/rack-js4xhr
> > o http://github.com/mynyml/rack-respond_to
> >
> > which led me to write the Accept header parsing code:
> >
> > o http://github.com/mynyml/rack-accept-media-types
> >
> > In my rack clone, the request_accept branch is a straight port of the
> > rack-accept-media-types gem, and the request_accept-compact branch
> > minimizes the code into two methods (instead of two classes). The
> > latter reflects the rest of the Request class much better so it's
> > probably a more appropriate patch.
> >
> > Feedback/questions/suggestions ?
> >
> > Thanks
>



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

[-- Attachment #2: Type: text/html, Size: 4034 bytes --]

Re: [PATCH] Request#accept_media_types

[Ryan Tomayko]

I haven't reviewed the patch yet but I'd love to have a set of simple
utility classes/methods/mixins for parsing basic HTTP header value
types. Accept, Accept-Language, Accept-Encoding, Cache-Control, etc.
all follow a few simple syntax grammars. We could then use the
utilities from Rack::Request, or directly from framework/app code if
more control is needed.

Ryan

On Sun, Jun 14, 2009 at 8:39 AM, Yehuda Katz<wycats@gmail.com> wrote:
> Take a look at what we do in Merb. We've optimized our Accept header parsing
> quite a bit (for performance; it can be shockingly slow), and handle quality
> factors (both absolute and relative).
>
> -- Yehuda
>
> On Sun, Jun 14, 2009 at 9:50 AM, Joshua Hull <joshbuddy@gmail.com> wrote:
>>
>> Just a couple of comments on this:
>>
>> types = types.select {|type| media_type_quality(type).between?(0.1, 1) }
>>
>> this should probably check that its greater than 0 and less than or
>> equal to 1 instead
>>
>> types = types.select {|type| quality = media_type_quality(type);
>> quality > 0 && quality <= 1 }
>>
>> so that values of 0.01 still get through.
>>
>> As well, the default value of all types is 1.0, but we probably want
>> to do something similar to what apache does, and assign */* a value of
>> 0.01 and type/* a value of 0.02.
>>
>> Cheers
>> Joshua
>>
>> On Fri, Jun 12, 2009 at 5:48 PM, mynyml<martin.aumont@gmail.com> wrote:
>> >
>> > Patch is at:
>> > http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64eaf24d9e2b
>> > -----
>> > Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
>> > types by "quality" (preference level) (following HTTP 1.1 specs).
>> >
>> > The main advantage of being able to parse the Accept header is to
>> > centralize the lookup for the request's "desired" type(s). Usually, a
>> > client asks for a specific media type to be sent back by appending an
>> > extension to the URL; not only does this require parsing by every
>> > middleware/app that needs to know about this media type, but it also
>> > ignores the Accept hierarchy, which can be usefull for cascading down
>> > possible handlings of the resquest (think of a respond_to mechanism,
>> > for instance).
>> >
>> > I've encountered many use cases already:
>> >
>> > o
>> > http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404fb5bd8d9f1eb/lib/rack/contrib/accept_format.rb
>> > o http://github.com/mynyml/rack-abstract-format
>> > o http://github.com/mynyml/rack-supported-media-types
>> > o http://github.com/mynyml/rack-js4xhr
>> > o http://github.com/mynyml/rack-respond_to
>> >
>> > which led me to write the Accept header parsing code:
>> >
>> > o http://github.com/mynyml/rack-accept-media-types
>> >
>> > In my rack clone, the request_accept branch is a straight port of the
>> > rack-accept-media-types gem, and the request_accept-compact branch
>> > minimizes the code into two methods (instead of two classes). The
>> > latter reflects the rest of the Request class much better so it's
>> > probably a more appropriate patch.
>> >
>> > Feedback/questions/suggestions ?
>> >
>> > Thanks
>
>
>
> --
> Yehuda Katz
> Developer | Engine Yard
> (ph) 718.877.1325
>

Re: Request#accept_media_types

[mynyml]

@Joshua: Nice catch about the quality values. The specs do specify
they can contain up to three decimal digits. I updated the code to
reflect that.

Now about Apache's behaviour of assigning 0.01 to */* and 0.02 to type/
*: I didn't know about this so I did some reading. Seems that Apache
does that as a fix for browsers that don't send any quality values at
all, as it assumes they don't really know what they're doing.

  http://httpd.apache.org/docs/1.3/content-negotiation.html#better

Turns out though there is another part of the HTTP spec (that I had
previously missed), which is that more specific types override less
specific ones. This automatically takes care of apache's behaviour. I
updated the code to include this too.

@Ryan: The code was becoming too repetitive so I split the media type
handling into it's own class. I put it in Rack::Mime::MimeType. I
guess that could provide a starting point. It could eventually
subclass something more generic.

@Yehuda: TBH I don't think I know enough performance optimization
tricks to add significant improvements, but I added the basics by
caching the parsed output.


Latest patch is @
  http://github.com/mynyml/rack/commit/2f0a743fe3a55b07cf0fc75ba06cb3180ddef7be

Questions, suggestions, patches?
Anyone wants to tackle performance? =)


On Jun 15, 3:58 pm, Ryan Tomayko <r...@tomayko.com> wrote:
> I haven't reviewed the patch yet but I'd love to have a set of simple
> utility classes/methods/mixins for parsing basic HTTP header value
> types. Accept, Accept-Language, Accept-Encoding, Cache-Control, etc.
> all follow a few simple syntax grammars. We could then use the
> utilities from Rack::Request, or directly from framework/app code if
> more control is needed.
>
> Ryan
>
> On Sun, Jun 14, 2009 at 8:39 AM, Yehuda Katz<wyc...@gmail.com> wrote:
> > Take a look at what we do in Merb. We've optimized our Accept header parsing
> > quite a bit (for performance; it can be shockingly slow), and handle quality
> > factors (both absolute and relative).
>
> > -- Yehuda
>
> > On Sun, Jun 14, 2009 at 9:50 AM, Joshua Hull <joshbu...@gmail.com> wrote:
>
> >> Just a couple of comments on this:
>
> >> types = types.select {|type| media_type_quality(type).between?(0.1, 1) }
>
> >> this should probably check that its greater than 0 and less than or
> >> equal to 1 instead
>
> >> types = types.select {|type| quality = media_type_quality(type);
> >> quality > 0 && quality <= 1 }
>
> >> so that values of 0.01 still get through.
>
> >> As well, the default value of all types is 1.0, but we probably want
> >> to do something similar to what apache does, and assign */* a value of
> >> 0.01 and type/* a value of 0.02.
>
> >> Cheers
> >> Joshua
>
> >> On Fri, Jun 12, 2009 at 5:48 PM, mynyml<martin.aum...@gmail.com> wrote:
>
> >> > Patch is at:
> >> >http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64ea...
> >> > -----
> >> > Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
> >> > types by "quality" (preference level) (following HTTP 1.1 specs).
>
> >> > The main advantage of being able to parse the Accept header is to
> >> > centralize the lookup for the request's "desired" type(s). Usually, a
> >> > client asks for a specific media type to be sent back by appending an
> >> > extension to the URL; not only does this require parsing by every
> >> > middleware/app that needs to know about this media type, but it also
> >> > ignores the Accept hierarchy, which can be usefull for cascading down
> >> > possible handlings of the resquest (think of a respond_to mechanism,
> >> > for instance).
>
> >> > I've encountered many use cases already:
>
> >> > o
> >> >http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404...
> >> > ohttp://github.com/mynyml/rack-abstract-format
> >> > ohttp://github.com/mynyml/rack-supported-media-types
> >> > ohttp://github.com/mynyml/rack-js4xhr
> >> > ohttp://github.com/mynyml/rack-respond_to
>
> >> > which led me to write the Accept header parsing code:
>
> >> > ohttp://github.com/mynyml/rack-accept-media-types
>
> >> > In my rack clone, the request_accept branch is a straight port of the
> >> > rack-accept-media-types gem, and the request_accept-compact branch
> >> > minimizes the code into two methods (instead of two classes). The
> >> > latter reflects the rest of the Request class much better so it's
> >> > probably a more appropriate patch.
>
> >> > Feedback/questions/suggestions ?
>
> >> > Thanks
>
> > --
> > Yehuda Katz
> > Developer | Engine Yard
> > (ph) 718.877.1325
>
>

Re: Request#accept_media_types

[mynyml]

Did anyone get a chance to take a look at the updated patch? Any more
suggestions?

Thanks


On Jun 15, 5:03 pm, mynyml <martin.aum...@gmail.com> wrote:
> @Joshua: Nice catch about the quality values. The specs do specify
> they can contain up to three decimal digits. I updated the code to
> reflect that.
>
> Now about Apache's behaviour of assigning 0.01 to */* and 0.02 to type/
> *: I didn't know about this so I did some reading. Seems that Apache
> does that as a fix for browsers that don't send any quality values at
> all, as it assumes they don't really know what they're doing.
>
>  http://httpd.apache.org/docs/1.3/content-negotiation.html#better
>
> Turns out though there is another part of the HTTP spec (that I had
> previously missed), which is that more specific types override less
> specific ones. This automatically takes care of apache's behaviour. I
> updated the code to include this too.
>
> @Ryan: The code was becoming too repetitive so I split the media type
> handling into it's own class. I put it in Rack::Mime::MimeType. I
> guess that could provide a starting point. It could eventually
> subclass something more generic.
>
> @Yehuda: TBH I don't think I know enough performance optimization
> tricks to add significant improvements, but I added the basics by
> caching the parsed output.
>
> Latest patch is @
>  http://github.com/mynyml/rack/commit/2f0a743fe3a55b07cf0fc75ba06cb318...
>
> Questions, suggestions, patches?
> Anyone wants to tackle performance? =)
>
> On Jun 15, 3:58 pm, Ryan Tomayko <r...@tomayko.com> wrote:
>
> > I haven't reviewed the patch yet but I'd love to have a set of simple
> > utility classes/methods/mixins for parsing basic HTTP header value
> > types. Accept, Accept-Language, Accept-Encoding, Cache-Control, etc.
> > all follow a few simple syntax grammars. We could then use the
> > utilities from Rack::Request, or directly from framework/app code if
> > more control is needed.
>
> > Ryan
>
> > On Sun, Jun 14, 2009 at 8:39 AM, Yehuda Katz<wyc...@gmail.com> wrote:
> > > Take a look at what we do in Merb. We've optimized our Accept header parsing
> > > quite a bit (for performance; it can be shockingly slow), and handle quality
> > > factors (both absolute and relative).
>
> > > -- Yehuda
>
> > > On Sun, Jun 14, 2009 at 9:50 AM, Joshua Hull <joshbu...@gmail.com> wrote:
>
> > >> Just a couple of comments on this:
>
> > >> types = types.select {|type| media_type_quality(type).between?(0.1, 1) }
>
> > >> this should probably check that its greater than 0 and less than or
> > >> equal to 1 instead
>
> > >> types = types.select {|type| quality = media_type_quality(type);
> > >> quality > 0 && quality <= 1 }
>
> > >> so that values of 0.01 still get through.
>
> > >> As well, the default value of all types is 1.0, but we probably want
> > >> to do something similar to what apache does, and assign */* a value of
> > >> 0.01 and type/* a value of 0.02.
>
> > >> Cheers
> > >> Joshua
>
> > >> On Fri, Jun 12, 2009 at 5:48 PM, mynyml<martin.aum...@gmail.com> wrote:
>
> > >> > Patch is at:
> > >> >http://github.com/mynyml/rack/commit/c9953dbc3f834fdcbcd1ebc8b71b64ea...
> > >> > -----
> > >> > Patch to add parsing of the HTTP_ACCEPT header's media types. Orders
> > >> > types by "quality" (preference level) (following HTTP 1.1 specs).
>
> > >> > The main advantage of being able to parse the Accept header is to
> > >> > centralize the lookup for the request's "desired" type(s). Usually, a
> > >> > client asks for a specific media type to be sent back by appending an
> > >> > extension to the URL; not only does this require parsing by every
> > >> > middleware/app that needs to know about this media type, but it also
> > >> > ignores the Accept hierarchy, which can be usefull for cascading down
> > >> > possible handlings of the resquest (think of a respond_to mechanism,
> > >> > for instance).
>
> > >> > I've encountered many use cases already:
>
> > >> > o
> > >> >http://github.com/rack/rack-contrib/blob/3f42d3afe7323d322567d77cd404...
> > >> > ohttp://github.com/mynyml/rack-abstract-format
> > >> > ohttp://github.com/mynyml/rack-supported-media-types
> > >> > ohttp://github.com/mynyml/rack-js4xhr
> > >> > ohttp://github.com/mynyml/rack-respond_to
>
> > >> > which led me to write the Accept header parsing code:
>
> > >> > ohttp://github.com/mynyml/rack-accept-media-types
>
> > >> > In my rack clone, the request_accept branch is a straight port of the
> > >> > rack-accept-media-types gem, and the request_accept-compact branch
> > >> > minimizes the code into two methods (instead of two classes). The
> > >> > latter reflects the rest of the Request class much better so it's
> > >> > probably a more appropriate patch.
>
> > >> > Feedback/questions/suggestions ?
>
> > >> > Thanks
>
> > > --
> > > Yehuda Katz
> > > Developer | Engine Yard
> > > (ph) 718.877.1325
>
>