Merge pull request #1170 from suan/better_vendor_resource_handling

allow dashes, periods, and other RFC6838-compliant characters in header vendor

Author: dblock

CHANGELOG.md

@@ -5,6 +5,7 @@
 
 * Your contribution here.
 
+* [#1170](https://github.com/ruby-grape/grape/pull/1170): Allow dashes and periods in header vendor - [@suan](https://github.com/suan).
 * [#1167](https://github.com/ruby-grape/grape/pull/1167): Convenience wrapper `type: File` for validating multipart file parameters - [@dslh](https://github.com/dslh).
 * [#1167](https://github.com/ruby-grape/grape/pull/1167): Refactor and extend coercion and type validation system - [@dslh](https://github.com/dslh).
 * [#1163](https://github.com/ruby-grape/grape/pull/1163): First-class `JSON` parameter type - [@dslh](https://github.com/dslh).

README.md

@@ -332,6 +332,17 @@ Using this versioning strategy, clients should pass the desired version in the U
 version 'v1', using: :header, vendor: 'twitter'
 ```
 
+Currently, Grape only supports versioned media types in the following format:
+
+```
+vnd.vendor-and-or-resource-v1234+format
+```
+
+Basically all tokens between the final `-` and the `+` will be interpreted as the version.
+Grape also only supports alphanumerics, periods, and dashes in the vendor/resource/version parts
+of the media type, even though [the appropriate RFC](http://tools.ietf.org/html/rfc6838#section-4.2)
+technically allows far more characters.
+
 Using this versioning strategy, clients should pass the desired version in the HTTP `Accept` head.
 
     curl -H Accept:application/vnd.twitter-v1+json http://localhost:9292/statuses/public_timeline

lib/grape/middleware/versioner/header.rb

@@ -8,13 +8,13 @@ module Versioner
       # application/vnd.:vendor-:version+:format
       #
       # Example: For request header
-      #    Accept: application/vnd.mycompany-v1+json
+      #    Accept: application/vnd.mycompany.a-cool-resource-v1+json
       #
       # The following rack env variables are set:
       #
       #    env['api.type']    => 'application'
-      #    env['api.subtype'] => 'vnd.mycompany-v1+json'
-      #    env['api.vendor]   => 'mycompany'
+      #    env['api.subtype'] => 'vnd.mycompany.a-cool-resource-v1+json'
+      #    env['api.vendor]   => 'mycompany.a-cool-resource'
       #    env['api.version]  => 'v1'
       #    env['api.format]   => 'json'
       #
@@ -23,7 +23,10 @@ module Versioner
       # route.
       class Header < Base
         VENDOR_VERSION_HEADER_REGEX =
-          /\Avnd\.([a-z0-9*.]+)(?:-([a-z0-9*\-.]+))?(?:\+([a-z0-9*\-.+]+))?\z/
+          /\Avnd\.([a-z0-9.\-_!#\$&\^]+?)(?:-([a-z0-9*.]+))?(?:\+([a-z0-9*\-.]+))?\z/
+
+        HAS_VENDOR_REGEX = /\Avnd\.[a-z0-9.\-_!#\$&\^]+/
+        HAS_VERSION_REGEX = /\Avnd\.([a-z0-9.\-_!#\$&\^]+?)(?:-([a-z0-9*.]+))+/
 
         def before
           strict_header_checks if strict?
@@ -122,7 +125,7 @@ def headers_contain_wrong_vendor?
 
         def headers_contain_wrong_version?
           header.values.all? do |header_value|
-            version?(header_value)
+            version?(header_value) && !versions.include?(request_version(header_value))
           end
         end
 
@@ -169,19 +172,24 @@ def error_headers
         # @return [Boolean] whether the content type sets a vendor
         def vendor?(media_type)
           _, subtype = Rack::Accept::Header.parse_media_type(media_type)
-          subtype[/\Avnd\.[a-z0-9*.]+/]
+          subtype[HAS_VENDOR_REGEX]
         end
 
         def request_vendor(media_type)
           _, subtype = Rack::Accept::Header.parse_media_type(media_type)
           subtype.match(VENDOR_VERSION_HEADER_REGEX)[1]
         end
 
+        def request_version(media_type)
+          _, subtype = Rack::Accept::Header.parse_media_type(media_type)
+          subtype.match(VENDOR_VERSION_HEADER_REGEX)[2]
+        end
+
         # @param [String] media_type a content type
         # @return [Boolean] whether the content type sets an API version
         def version?(media_type)
           _, subtype = Rack::Accept::Header.parse_media_type(media_type)
-          subtype[/\Avnd\.[a-z0-9*.]+-[a-z0-9*\-.]+/]
+          subtype[HAS_VERSION_REGEX]
         end
       end
     end

spec/grape/middleware/versioner/header_spec.rb

@@ -252,7 +252,7 @@
     end
   end
 
-  context 'when there are multiple versions specified with rescue_from :all' do
+  context 'when there are multiple versions with complex vendor specified with rescue_from :all' do
     subject {
       Class.new(Grape::API) do
         rescue_from :all
@@ -261,7 +261,11 @@
 
     let(:v1_app) {
       Class.new(Grape::API) do
-        version 'v1', using: :header, vendor: 'test'
+        version 'v1', using: :header, vendor: 'test.a-cool-resource'
+        content_type :v1_test, 'application/vnd.test.a-cool-resource-v1+json'
+        formatter :v1_test, ->(object, _) { object }
+        format :v1_test
+
         resources :users do
           get :hello do
             'one'
@@ -272,7 +276,11 @@
 
     let(:v2_app) {
       Class.new(Grape::API) do
-        version 'v2', using: :header, vendor: 'test'
+        version 'v2', using: :header, vendor: 'test.a-cool-resource'
+        content_type :v2_test, 'application/vnd.test.a-cool-resource-v2+json'
+        formatter :v2_test, ->(object, _) { object }
+        format :v2_test
+
         resources :users do
           get :hello do
             'two'
@@ -289,13 +297,13 @@ def app
 
     context 'with header versioned endpoints and a rescue_all block defined' do
       it 'responds correctly to a v1 request' do
-        versioned_get '/users/hello', 'v1', using: :header, vendor: 'test'
+        versioned_get '/users/hello', 'v1', using: :header, vendor: 'test.a-cool-resource'
         expect(last_response.body).to eq('one')
         expect(last_response.body).not_to include('API vendor or version not found')
       end
 
       it 'responds correctly to a v2 request' do
-        versioned_get '/users/hello', 'v2', using: :header, vendor: 'test'
+        versioned_get '/users/hello', 'v2', using: :header, vendor: 'test.a-cool-resource'
         expect(last_response.body).to eq('two')
         expect(last_response.body).not_to include('API vendor or version not found')
       end