From 22244a2c3a56740be484dd56fd2dbb5d93caa45e Mon Sep 17 00:00:00 2001 From: dblock Date: Mon, 4 Apr 2016 12:45:19 -0400 Subject: [PATCH] Require Grape 0.16.1 and fix route_xxx deprecations. --- .travis.yml | 9 +------ Gemfile | 2 +- grape-swagger.gemspec | 2 +- lib/grape-swagger.rb | 8 +++--- lib/grape-swagger/doc_methods.rb | 45 +++++++++++++++++++------------- spec/api_description_spec.rb | 18 ++++++------- spec/i18n_spec.rb | 14 +++++----- 7 files changed, 50 insertions(+), 48 deletions(-) diff --git a/.travis.yml b/.travis.yml index 6fa81417..8b6262a1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,14 +10,7 @@ rvm: - rbx-2 env: - - GRAPE_VERSION=0.8.0 - - GRAPE_VERSION=0.9.0 - - GRAPE_VERSION=0.10.0 - - GRAPE_VERSION=0.10.1 - - GRAPE_VERSION=0.11.0 - - GRAPE_VERSION=0.12.0 - - GRAPE_VERSION=0.13.0 - - GRAPE_VERSION=0.14.0 + - GRAPE_VERSION=0.16.1 - GRAPE_VERSION=HEAD matrix: diff --git a/Gemfile b/Gemfile index 666d9c3b..1470a3f3 100644 --- a/Gemfile +++ b/Gemfile @@ -2,7 +2,7 @@ source 'http://rubygems.org' gemspec -case version = ENV['GRAPE_VERSION'] || '~> 0.9.0' +case version = ENV['GRAPE_VERSION'] || '~> 0.16.1' when 'HEAD' gem 'grape', github: 'ruby-grape/grape' else diff --git a/grape-swagger.gemspec b/grape-swagger.gemspec index 3ff1b1bf..96e0aa34 100644 --- a/grape-swagger.gemspec +++ b/grape-swagger.gemspec @@ -11,7 +11,7 @@ Gem::Specification.new do |s| s.summary = 'A simple way to add auto generated documentation to your Grape API that can be displayed with Swagger.' s.license = 'MIT' - s.add_runtime_dependency 'grape', '>= 0.8.0' + s.add_runtime_dependency 'grape', '>= 0.16.1' s.add_runtime_dependency 'grape-entity', '< 0.5.0' s.add_development_dependency 'rake' diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 07606cb9..c39aaebe 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -23,15 +23,15 @@ def add_swagger_documentation(options = {}) @target_class.combined_routes = {} @target_class.routes.each do |route| - route_path = route.route_path - route_match = route_path.split(/^.*?#{route.route_prefix.to_s}/).last + route_path = route.path + route_match = route_path.split(/^.*?#{route.prefix.to_s}/).last next unless route_match route_match = route_match.match('\/([\w|-]*?)[\.\/\(]') || route_match.match('\/([\w|-]*)$') next unless route_match resource = route_match.captures.first next if resource.empty? @target_class.combined_routes[resource] ||= [] - next if documentation_class.hide_documentation_path && route.route_path.match(/#{documentation_class.mount_path}($|\/|\(\.)/) + next if documentation_class.hide_documentation_path && route.path.match(/#{documentation_class.mount_path}($|\/|\(\.)/) @target_class.combined_routes[resource] << route end @@ -77,7 +77,7 @@ def combine_namespace_routes(namespaces) parent_route = @target_class.combined_routes[parent_route_name] # fetch all routes that are within the current namespace namespace_routes = parent_route.collect do |route| - route if (route.route_path.start_with?(route.route_prefix ? "/#{route.route_prefix}/#{name}" : "/#{name}") || route.route_path.start_with?((route.route_prefix ? "/#{route.route_prefix}/:version/#{name}" : "/:version/#{name}"))) && + route if (route.path.start_with?(route.prefix ? "/#{route.prefix}/#{name}" : "/#{name}") || route.path.start_with?((route.prefix ? "/#{route.prefix}/:version/#{name}" : "/:version/#{name}"))) && (route.instance_variable_get(:@options)[:namespace] == "/#{name}" || route.instance_variable_get(:@options)[:namespace] == "/:version/#{name}") end.compact diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index a584bd8e..628ae727 100644 --- a/lib/grape-swagger/doc_methods.rb +++ b/lib/grape-swagger/doc_methods.rb @@ -436,7 +436,9 @@ def setup(options) end namespace_routes_array = namespace_routes.keys.map do |local_route| - next if namespace_routes[local_route].map(&:route_hidden).all? { |value| value.respond_to?(:call) ? value.call : value } + next if namespace_routes[local_route].map { |route| + route.settings[:description] && route.settings[:description][:hidden] + }.all? { |value| value.respond_to?(:call) ? value.call : value } url_format = '.{format}' unless @@hide_format url_locale = "?locale=#{params[:locale]}" unless params[:locale].blank? @@ -488,11 +490,12 @@ def setup(options) error!('Not Found', 404) unless routes visible_ops = routes.reject do |route| - route.route_hidden.respond_to?(:call) ? route.route_hidden.call : route.route_hidden + hidden = route.route_hidden + hidden && hidden.respond_to?(:call) ? hidden.call : hidden end ops = visible_ops.group_by do |route| - @@documentation_class.parse_path(route.route_path, api_version) + @@documentation_class.parse_path(route.path, api_version) end error!('Not Found', 404) unless ops.any? @@ -501,51 +504,57 @@ def setup(options) ops.each do |path, op_routes| operations = op_routes.map do |route| + route_settings_description = route.settings[:description] || {} endpoint = target_class.endpoint_mapping[route.to_s.sub('(.:format)', '')] endpoint_path = endpoint.options[:path] unless endpoint.nil? - i18n_key = [route.route_namespace, endpoint_path, route.route_method.downcase].flatten.join('/') + i18n_key = [route.namespace, endpoint_path, route.request_method.downcase].flatten.join('/') i18n_key = i18n_key.split('/').reject(&:empty?).join('.') summary = @@documentation_class.translate( - route.route_description, i18n_scope, + route.description, i18n_scope, [:"#{i18n_key}.desc", :"#{i18n_key}.description"] ) notes = @@documentation_class.translate( - route.route_detail || route.route_notes, i18n_scope, + route_settings_description[:detail] || route_settings_description[:notes], i18n_scope, [:"#{i18n_key}.detail", :"#{i18n_key}.notes"] ) notes = @@documentation_class.as_markdown(notes) - http_codes = @@documentation_class.parse_http_codes(route.route_http_codes, models) + http_codes = @@documentation_class.parse_http_codes(route.http_codes, models) - models.merge(Array(route.route_entity)) if route.route_entity.present? + models.merge(Array(route.entity)) if route.entity.present? + + route_settings_description = route.settings[:description] || {} operation = { notes: notes.to_s, summary: summary, - nickname: route.route_nickname || (route.route_method + route.route_path.gsub(/[\/:\(\)\.]/, '-')), - method: route.route_method, - parameters: @@documentation_class.parse_header_params(route.route_headers, scope: i18n_scope, key: i18n_key) + - @@documentation_class.parse_params(route.route_params, route.route_path, route.route_method, + nickname: route_settings_description[:nickname] || (route.request_method + route.path.gsub(/[\/:\(\)\.]/, '-')), + method: route.request_method, + parameters: @@documentation_class.parse_header_params(route.headers, scope: i18n_scope, key: i18n_key) + + @@documentation_class.parse_params(route.route_params, route.path, route.request_method, scope: i18n_scope, key: i18n_key), - type: route.route_is_array ? 'array' : 'void' + type: route_settings_description[:is_array] ? 'array' : 'void' } - operation[:authorizations] = route.route_authorizations unless route.route_authorizations.nil? || route.route_authorizations.empty? + + authorizations = route_settings_description[:authorizations] + operation[:authorizations] = authorizations if authorizations && authorizations.any? + if operation[:parameters].any? { |param| param[:type] == 'File' } operation.merge!(consumes: ['multipart/form-data']) end operation.merge!(responseMessages: http_codes) unless http_codes.empty? - if route.route_entity - type = @@documentation_class.parse_entity_name(Array(route.route_entity).first) - if route.route_is_array + if route.entity + type = @@documentation_class.parse_entity_name(Array(route.entity).first) + if route_settings_description[:is_array] operation.merge!(items: { '$ref' => type }) else operation.merge!(type: type) end end - operation[:nickname] = route.route_nickname if route.route_nickname + operation[:nickname] = route_settings_description[:nickname] if route_settings_description.key?(:nickname) operation end.compact apis << { diff --git a/spec/api_description_spec.rb b/spec/api_description_spec.rb index d5fcb63b..b5850edf 100644 --- a/spec/api_description_spec.rb +++ b/spec/api_description_spec.rb @@ -11,10 +11,10 @@ it 'describes the API with defaults' do routes = subject.endpoints.first.routes expect(routes.count).to eq 2 - expect(routes.first.route_description).to eq 'Swagger compatible API description' - expect(routes.first.route_params).to eq('locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) - expect(routes.last.route_description).to eq 'Swagger compatible API description for specific API' - expect(routes.last.route_params).to eq('name' => { desc: 'Resource name of mounted API', type: 'String', required: true }, + expect(routes.first.description).to eq 'Swagger compatible API description' + expect(routes.first.params).to eq('locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) + expect(routes.last.description).to eq 'Swagger compatible API description for specific API' + expect(routes.last.params).to eq('name' => { desc: 'Resource name of mounted API', type: 'String', required: true }, 'locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) end end @@ -31,11 +31,11 @@ it 'describes the API with defaults' do routes = subject.endpoints.first.routes expect(routes.count).to eq 2 - expect(routes.first.route_description).to eq 'First' - expect(routes.first.route_params).to eq(x: 1, 'locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) - expect(routes.first.route_xx).to eq(11) - expect(routes.last.route_description).to eq 'Second' - expect(routes.last.route_params).to eq('name' => { desc: 'Resource name of mounted API', type: 'String', required: true }, y: 42, + expect(routes.first.description).to eq 'First' + expect(routes.first.params).to eq(x: 1, 'locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) + expect(routes.first.settings[:description][:xx]).to eq(11) + expect(routes.last.description).to eq 'Second' + expect(routes.last.params).to eq('name' => { desc: 'Resource name of mounted API', type: 'String', required: true }, y: 42, 'locale' => { desc: 'Locale of API documentation', type: 'Symbol', required: false }) expect(routes.last.route_yy).to eq(4242) end diff --git a/spec/i18n_spec.rb b/spec/i18n_spec.rb index ca29763b..90b40d6e 100644 --- a/spec/i18n_spec.rb +++ b/spec/i18n_spec.rb @@ -164,8 +164,8 @@ def api_translations summary: 'Finds user by id', notes: '' ) expect(result[:apis][api_index][:operations][0][:parameters]).to eq [ - { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false }, - { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false } + { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false }, + { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false } ] api_index += 1 @@ -173,8 +173,8 @@ def api_translations summary: "Changes a user's email", notes: '' ) expect(result[:apis][api_index][:operations][0][:parameters]).to eq [ - { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false }, { paramType: 'form', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false }, + { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false }, { paramType: 'form', name: 'email', description: 'A new email', type: 'string', required: true, allowMultiple: false } ] @@ -183,8 +183,8 @@ def api_translations summary: "Gets the strength estimation of a user's password", notes: 'The estimation is done by a well-known algorithm when he changed his password' ) expect(result[:apis][api_index][:operations][0][:parameters]).to eq [ - { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false }, - { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false } + { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false }, + { paramType: 'path', name: 'id', description: 'User id', type: 'string', required: true, allowMultiple: false } ] end @@ -207,8 +207,8 @@ def api_translations summary: 'Gets specific resource API document' ) expect(result[:apis][api_index][:operations][0][:parameters]).to eq [ - { paramType: 'path', name: 'name', description: 'Resource name', type: 'string', required: true, allowMultiple: false }, - { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false } + { paramType: 'query', name: 'locale', description: "Used to change locale of endpoint's responding message", type: 'string', required: false, allowMultiple: false }, + { paramType: 'path', name: 'name', description: 'Resource name', type: 'string', required: true, allowMultiple: false } ] end end