Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Frontier vendoring #116

Merged
merged 2 commits into from

3 participants

@jwkoelewijn

class MyAPI < Grape::API
vendor 'v1', :using => :header, :vendor => 'twitter', :format => :json

get '/' do
'hello world'
end
end

and let it still return the '/' even when the accept-header
"application/vnd.some_other_vendor-v1+json" was given. This commit will
let it return a 404, and sets the X-Cascade pass as well.

I've added this because although the version is right, part of the
Accept header (the vendor part) is not correct.

@jwkoelewijn jwkoelewijn Added consideration of vendors when using header based version selection
class MyAPI < Grape::API
vendor 'v1', :using => :header, :vendor => 'twitter', :format => :json

get '/' do
'hello world'
end
end

and let it still return the '/' even when the accept-header
"application/vnd.some_other_vendor-v1+json" was given. This commit will
let it return a 404, and sets the X-Cascade pass as well.

I've added this because although the version is right, part of the
Accept header (the vendor part) is not correct.
cba257e
lib/grape/middleware/versioner/header.rb
@@ -35,7 +35,10 @@ def before
env['api.subtype'] = subtype
subtype.scan(/vnd\.(.+)?-(.+)?\+(.*)?/) do |vendor, version, format|
- if options[:versions] && !options[:versions].include?(version)
+ is_vendored = options[:version_options] && options[:version_options][:vendor]
+ correctly_vendored = is_vendored ? options[:version_options][:vendor] == vendor : true
@dblock Owner
dblock added a note

This may be a pedantic/style comment or maybe I don't understand what this is doing :)

"Correctly" jumped as a validation thing, which is not what we'redoing here. So the header is vendored, but then the header vendor matches (rather than correct), right? I would rename correctly_vendored to something like is_vendored_match to make the intent more specific.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jwkoelewijn

@dblock You were right, is_vendored_match is a semantically more correct name :)

@jch jch merged commit 5a3bc1e into ruby-grape:frontier
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 5, 2012
  1. @jwkoelewijn

    Added consideration of vendors when using header based version selection

    jwkoelewijn authored
    class MyAPI < Grape::API
    vendor 'v1', :using => :header, :vendor => 'twitter', :format => :json
    
    get '/' do
    'hello world'
    end
    end
    
    and let it still return the '/' even when the accept-header
    "application/vnd.some_other_vendor-v1+json" was given. This commit will
    let it return a 404, and sets the X-Cascade pass as well.
    
    I've added this because although the version is right, part of the
    Accept header (the vendor part) is not correct.
  2. @jwkoelewijn
This page is out of date. Refresh to see the latest.
View
5 lib/grape/middleware/versioner/header.rb
@@ -35,7 +35,10 @@ def before
env['api.subtype'] = subtype
subtype.scan(/vnd\.(.+)?-(.+)?\+(.*)?/) do |vendor, version, format|
- if options[:versions] && !options[:versions].include?(version)
+ is_vendored = options[:version_options] && options[:version_options][:vendor]
+ is_vendored_match = is_vendored ? options[:version_options][:vendor] == vendor : true
+
+ if (options[:versions] && !options[:versions].include?(version)) || !is_vendored_match
throw :error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found"
end
View
20 spec/grape/middleware/versioner/header_spec.rb
@@ -80,6 +80,26 @@
end
end
+ context 'vendors' do
+ before do
+ @options = {
+ :version => ['v1'],
+ :version_options => {:using => :header, :vendor => 'vendor'}
+ }
+ end
+
+ it 'should match with correct vendor' do
+ status = subject.call('HTTP_ACCEPT' => accept).first
+ status.should == 200
+ end
+
+ it 'should not match with an incorrect vendor' do
+ expect {
+ env = subject.call('HTTP_ACCEPT' => 'application/vnd.othervendor-v1+json').last
+ }.to throw_symbol(:error, :status => 404, :headers => {'X-Cascade' => 'pass'}, :message => "404 API Version Not Found")
+ end
+ end
+
context 'no matched version' do
before do
@options = {
Something went wrong with that request. Please try again.