diff --git a/CHANGELOG.md b/CHANGELOG.md index 01d89b31..1be4d2ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ #### Fixes +* [#438](https://github.com/ruby-grape/grape-swagger/pull/438): Route version was missing in :options passed to PathString, so Endpoint.path_and_definitions_objects wasn't returning a versioned path when required - [@texpert](https://github.com/texpert). + * Your contribution here. ### 0.20.4 (May 16, 2016) diff --git a/lib/grape-swagger/doc_methods/path_string.rb b/lib/grape-swagger/doc_methods/path_string.rb index 36fd4481..b1fccd55 100644 --- a/lib/grape-swagger/doc_methods/path_string.rb +++ b/lib/grape-swagger/doc_methods/path_string.rb @@ -2,7 +2,8 @@ module GrapeSwagger module DocMethods class PathString class << self - def build(path, options = {}) + def build(route, options = {}) + path = route.path # always removing format path.sub!(/\(\.\w+?\)$/, '') path.sub!('(.:format)', '') @@ -13,8 +14,8 @@ def build(path, options = {}) # set item from path, this could be used for the definitions object item = path.gsub(%r{/{(.+?)}}, '').split('/').last.singularize.underscore.camelize || 'Item' - if options[:version] && options[:add_version] - path.sub!('{version}', options[:version].to_s) + if route.version && options[:add_version] + path.sub!('{version}', route.version.to_s) else path.sub!('/{version}', '') end diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index b50ac0eb..521dc67c 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -86,7 +86,7 @@ def path_item(routes, options) routes.each do |route| next if hidden?(route) - @item, path = GrapeSwagger::DocMethods::PathString.build(route.path, options) + @item, path = GrapeSwagger::DocMethods::PathString.build(route, options) @entity = route.entity || route.options[:success] verb, method_object = method_object(route, options, path) diff --git a/lib/grape-swagger/grape/route.rb b/lib/grape-swagger/grape/route.rb index f9dc6309..e84b6c9a 100644 --- a/lib/grape-swagger/grape/route.rb +++ b/lib/grape-swagger/grape/route.rb @@ -1,7 +1,8 @@ # backwards compatibility for Grape < 0.16.0 module Grape class Route - [:path, :prefix, :entity, :description, :settings, :params, :headers, :http_codes].each do |m| + [:path, :prefix, :entity, :description, :settings, :params, :headers, :http_codes, :version] + .each do |m| define_method m do send "route_#{m}" end diff --git a/spec/lib/path_string_spec.rb b/spec/lib/path_string_spec.rb index b6f2d474..40c6592d 100644 --- a/spec/lib/path_string_spec.rb +++ b/spec/lib/path_string_spec.rb @@ -8,29 +8,83 @@ describe 'operation_id_object' do describe 'version' do - describe 'defaults: not given, false' do + describe 'defaults: given, true' do + let(:options) { { add_version: true } } + let(:route) { Struct.new(:version, :path).new('v1') } + + specify 'The returned path includes version' do + route.path = '/{version}/thing(.json)' + expect(subject.build(route, options)).to eql ['Thing', '/v1/thing'] + route.path = '/{version}/thing/foo(.json)' + expect(subject.build(route, options)).to eql ['Foo', '/v1/thing/foo'] + route.path = '/{version}/thing(.:format)' + expect(subject.build(route, options)).to eql ['Thing', '/v1/thing'] + route.path = '/{version}/thing/foo(.:format)' + expect(subject.build(route, options)).to eql ['Foo', '/v1/thing/foo'] + route.path = '/{version}/thing/:id' + expect(subject.build(route, options)).to eql ['Thing', '/v1/thing/{id}'] + route.path = '/{version}/thing/foo/:id' + expect(subject.build(route, options)).to eql ['Foo', '/v1/thing/foo/{id}'] + end + end + + describe 'defaults: not given, both false' do let(:options) { { add_version: false } } + let(:route) { Struct.new(:version, :path).new } - specify do - expect(subject.build('/thing(.json)', options)).to eql ['Thing', '/thing'] - expect(subject.build('/thing/foo(.json)', options)).to eql ['Foo', '/thing/foo'] - expect(subject.build('/thing(.:format)', options)).to eql ['Thing', '/thing'] - expect(subject.build('/thing/foo(.:format)', options)).to eql ['Foo', '/thing/foo'] - expect(subject.build('/thing/:id', options)).to eql ['Thing', '/thing/{id}'] - expect(subject.build('/thing/foo/:id', options)).to eql ['Foo', '/thing/foo/{id}'] + specify 'The returned path does not include version' do + route.path = '/{version}/thing(.json)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.json)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing(.:format)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.:format)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing/:id' + expect(subject.build(route, options)).to eql ['Thing', '/thing/{id}'] + route.path = '/{version}/thing/foo/:id' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo/{id}'] end end - describe 'defaults: given, true' do - let(:options) { { version: 'v1', add_version: true } } - - specify do - expect(subject.build('/{version}/thing(.json)', options)).to eql ['Thing', '/v1/thing'] - expect(subject.build('/{version}/thing/foo(.json)', options)).to eql ['Foo', '/v1/thing/foo'] - expect(subject.build('/{version}/thing(.:format)', options)).to eql ['Thing', '/v1/thing'] - expect(subject.build('/{version}/thing/foo(.:format)', options)).to eql ['Foo', '/v1/thing/foo'] - expect(subject.build('/{version}/thing/:id', options)).to eql ['Thing', '/v1/thing/{id}'] - expect(subject.build('/{version}/thing/foo/:id', options)).to eql ['Foo', '/v1/thing/foo/{id}'] + describe 'defaults: add_version false' do + let(:options) { { add_version: false } } + let(:route) { Struct.new(:version, :path).new('v1') } + + specify 'The returned path does not include version' do + route.path = '/{version}/thing(.json)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.json)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing(.:format)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.:format)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing/:id' + expect(subject.build(route, options)).to eql ['Thing', '/thing/{id}'] + route.path = '/{version}/thing/foo/:id' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo/{id}'] + end + end + + describe 'defaults: root_version nil' do + let(:options) { { add_version: true } } + let(:route) { Struct.new(:version, :path).new } + + specify 'The returned path does not include version' do + route.path = '/{version}/thing(.json)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.json)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing(.:format)' + expect(subject.build(route, options)).to eql ['Thing', '/thing'] + route.path = '/{version}/thing/foo(.:format)' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo'] + route.path = '/{version}/thing/:id' + expect(subject.build(route, options)).to eql ['Thing', '/thing/{id}'] + route.path = '/{version}/thing/foo/:id' + expect(subject.build(route, options)).to eql ['Foo', '/thing/foo/{id}'] end end end diff --git a/spec/swagger_v2/endpoint_versioned_path_spec.rb b/spec/swagger_v2/endpoint_versioned_path_spec.rb new file mode 100644 index 00000000..e8a3f41a --- /dev/null +++ b/spec/swagger_v2/endpoint_versioned_path_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe 'Grape::Endpoint#path_and_definitions' do + before do + module API + module V1 + class Item < Grape::API + version 'v1', using: :path + + resource :item do + get '/' + end + end + end + + class Root < Grape::API + mount API::V1::Item + add_swagger_documentation add_version: true + end + end + + @options = { add_version: true } + @target_routes = API::Root.combined_namespace_routes + end + + it 'is returning a versioned path' do + expect(API::V1::Item.endpoints[0] + .path_and_definition_objects(@target_routes, @options)[0].keys[0]).to eql '/v1/item' + end +end