mjackson / rack-accept

HTTP Accept* for Ruby/Rack
http://mjackson.github.com/rack-accept
47 stars 16 forks source link

accept-extension parameter support #12

Open kamui opened 12 years ago

kamui commented 12 years ago

The http spec includes accept header extensions. So other than the q value, you can actually add arbitrary parameters. We want to use accept-extensions for version api numbers. It looks like this:

Accept: application/vnd.example-com.foo+json;version=1.0;q=0.1

I wanted to use rack-accept since it handles ordering by q values and filtering out the best type based on an array of supported media types, but rack-accept assumes that the q value is the only parameter, and that it'll be right after the semicolon. If I use the header above, the result I get is this:

accept.qvalues
=> {"application/vnd.example.com.foo+json;version=1.0"=>0.1}

rack-accept thinks that the version is part of the media type. Also, since everything after the q value is thrown out, there's no way to get the other parameters, such as version without manually parsing out the raw Accept header.

Looking at how rack-accept works, you assume only the media type and the q value and you store them as a hash, but it would be cool if the media type returns were an array of hashes that looks like this:

{
  "application/vnd.example.com.foo+json" : {
    "q" : 0.1,
    "version" : "1.0",
    ... other params ...
  }
}

Resources: http://www.informit.com/articles/article.aspx?p=1566460 http://publish.luisrei.com/articles/rest.html http://blog.steveklabnik.com/posts/2011-07-03-nobody-understands-rest-or-http http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html

mjackson commented 12 years ago

Hey @kamui - please see the comment in the code about accept-extension. :) I'd love to include support for it but just haven't gotten around to it. The main problem with the code as it is now is that the Header class assumes that all subclasses only have qvalues, and doesn't account for extensions. Would be interested to hear any ideas you might have on how to refactor the code to accommodate extension values.

kamui commented 12 years ago

Haha.. that code comment is hilarious.

As far as solutions, it looks like only media types support accept-extensions, so you could probably keep the same api. I think for the MediaType you can probably add an instance variable @extensions that is basically what I have above. It'll just be a hash where the key is the type, and the value is a hash of extension params, including q value.

Then in the initialize method in MediaType, parse out the header for extensions and media types, and store them in the @extensions instance variable, removing the super(header) call. You could also define a qvalues method that by default loads @qvalues, and if it's not defined, builds a qvalue hash from the @extensions hash.

How does that sound?

mjackson commented 12 years ago

Yeah, could definitely work. Although I think I'd like to keep the call to super(header) and just pass it a version of the header string that only has the qvalue stuff in it. I'll see if I can get around to adding something like this in the near future. In the mean time, if you need it quickly I'd be happy to merge in any pull request you'd like to contribute (if you have the time, of course)!

kamui commented 12 years ago

I've just started implementing it, but I'm not sure about the call to super(header). The new initializer already creates an @extensions (not sure on this variable name yet) instance variable and it'd be so much easier to iterate on that to create a @qvalues instance variable rather than call super and have it do parsing all over again.

One of the snags I ran into is what to do with the match method. In the API, you match with the parameters in a string. I'm wondering whether or not it makes sense to match based on any param at all. I'm trying to think of a use case this solves. For example, a version param will just trigger route matching for me. So I don't need to match params for content negotiation. What do you think?

Also, what should the API look like to access the extensions hash?

kamui commented 12 years ago

When you get a chance, take a look at my branch. matches currently ignores the param list, I'm not sure it makes sense to match on it, and I'm also not sure if it should filter on params or if it should prefer media types that match params.

@extensions returns a hash of hashes, The key for the first hash is the media type.

#params takes a media type, returns the params hash for the first matched media type using #matches.

I suppose I would use it like this:

# 'application/json;q=1;version=1.0'
accept = env['rack-accept.request'].media_type
media_type = accept.best_of('application/json', 'application/xml', 'text/html')
accept_params = accept.params(media_type)
puts accept_params['version'] # => '1.0'
puts accept_params['q] # => '1'
dzrw commented 11 years ago

What's the ETA on this?

kamui commented 10 years ago

It's been 2 years, so I decided to cleanup/update some of the foundation (gemspec, minitest, guard, bundler, require instead of autoload) and release it as another gem with accept-extension support added. https://github.com/kamui/rack-accept_headers