diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2aef71158..bf672eb8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -26,23 +26,22 @@ jobs: strategy: matrix: entry: - - { ruby: '3.1', grape: '1.8.0' } - - { ruby: '3.2', grape: '1.8.0' } - - { ruby: '3.3', grape: '1.8.0' } - - { ruby: '3.4', grape: '1.8.0' } - - { ruby: '3.1', grape: '2.0.0' } - - { ruby: '3.2', grape: '2.0.0' } - - { ruby: '3.3', grape: '2.0.0' } - - { ruby: '3.4', grape: '2.0.0' } - - { ruby: '3.1', grape: '2.1.3' } - - { ruby: '3.2', grape: '2.1.3' } - - { ruby: '3.3', grape: '2.1.3' } - - { ruby: '3.4', grape: '2.1.3' } - - { ruby: '3.1', grape: '2.2.0' } - - { ruby: '3.2', grape: '2.2.0' } - - { ruby: '3.3', grape: '2.2.0' } - - { ruby: '3.4', grape: '2.2.0' } - - { ruby: 'head', grape: '2.2.0' } + - { ruby: '3.1', grape: '2.4.0' } + - { ruby: '3.2', grape: '2.4.0' } + - { ruby: '3.3', grape: '2.4.0' } + - { ruby: '3.4', grape: '2.4.0' } + - { ruby: '3.1', grape: '3.0.1' } + - { ruby: '3.2', grape: '3.0.1' } + - { ruby: '3.3', grape: '3.0.1' } + - { ruby: '3.4', grape: '3.0.1' } + - { ruby: '3.1', grape: '3.1.1' } + - { ruby: '3.2', grape: '3.1.1' } + - { ruby: '3.3', grape: '3.1.1' } + - { ruby: '3.4', grape: '3.1.1' } + - { ruby: '3.2', grape: '3.2.1' } + - { ruby: '3.3', grape: '3.2.1' } + - { ruby: '3.4', grape: '3.2.1' } + - { ruby: 'head', grape: '3.2.1' } - { ruby: '3.2', grape: 'HEAD' } - { ruby: '3.3', grape: 'HEAD' } - { ruby: '3.4', grape: 'HEAD' } diff --git a/.rubocop.yml b/.rubocop.yml index 9930e5608..139b6988a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -6,7 +6,7 @@ AllCops: - example/**/* UseCache: true NewCops: enable - TargetRubyVersion: 3.3 + TargetRubyVersion: 3.4 SuggestExtensions: false # Layout stuff diff --git a/CHANGELOG.md b/CHANGELOG.md index 428ddb86e..2fb739c2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ #### Features +* [#976](https://github.com/ruby-grape/grape-swagger/pull/976): Ruby 3.4 and refactor swagger documentation modules - [@moskvin](https://github.com/moskvin). * Your contribution here. #### Fixes diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 9e929b25f..75c452a31 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -11,6 +11,8 @@ require 'grape-swagger/doc_methods' require 'grape-swagger/model_parsers' require 'grape-swagger/request_param_parser_registry' +require 'grape-swagger/swagger_routing' +require 'grape-swagger/swagger_documentation_adder' require 'grape-swagger/token_owner_resolver' module GrapeSwagger @@ -44,166 +46,4 @@ def request_param_parsers }.freeze end -module SwaggerRouting - private - - def combine_routes(app, doc_klass) - app.routes.each_with_object({}) do |route, combined_routes| - route_path = route.path - route_match = route_path.split(/^.*?#{route.prefix}/).last - next unless route_match - - # want to match emojis … ;) - # route_match = route_match - # .match('\/([\p{Alnum}p{Emoji}\-\_]*?)[\.\/\(]') || route_match.match('\/([\p{Alpha}\p{Emoji}\-\_]*)$') - route_match = route_match.match('\/([\p{Alnum}\-\_]*?)[\.\/\(]') || route_match.match('\/([\p{Alpha}\-\_]*)$') - next unless route_match - - resource = route_match.captures.first - resource = '/' if resource.empty? - combined_routes[resource] ||= [] - next if doc_klass.hide_documentation_path && route.path.match(/#{doc_klass.mount_path}($|\/|\(\.)/) - - combined_routes[resource] << route - end - end - - def determine_namespaced_routes(name, parent_route, routes) - return routes.values.flatten if parent_route.nil? - - parent_route.select do |route| - route_path_start_with?(route, name) || route_namespace_equals?(route, name) - end - end - - def combine_namespace_routes(namespaces, routes) - combined_namespace_routes = {} - # iterate over each single namespace - namespaces.each_key do |name, _| - # get the parent route for the namespace - parent_route_name = extract_parent_route(name) - parent_route = routes[parent_route_name] - # fetch all routes that are within the current namespace - namespace_routes = determine_namespaced_routes(name, parent_route, routes) - - # default case when not explicitly specified or nested == true - standalone_namespaces = namespaces.reject do |_, ns| - !ns.options.key?(:swagger) || - !ns.options[:swagger].key?(:nested) || - ns.options[:swagger][:nested] != false - end - - parent_standalone_namespaces = standalone_namespaces.select { |ns_name, _| name.start_with?(ns_name) } - # add only to the main route - # if the namespace is not within any other namespace appearing as standalone resource - # rubocop:disable Style/Next - if parent_standalone_namespaces.empty? - # default option, append namespace methods to parent route - combined_namespace_routes[parent_route_name] ||= [] - combined_namespace_routes[parent_route_name].push(*namespace_routes) - end - # rubocop:enable Style/Next - end - - combined_namespace_routes - end - - def extract_parent_route(name) - route_name = name.match(%r{^/?([^/]*).*$})[1] - return route_name unless route_name.include? ':' - - matches = name.match(/\/\p{Alpha}+/) - matches.nil? ? route_name : matches[0].delete('/') - end - - def route_namespace_equals?(route, name) - patterns = Enumerator.new do |yielder| - yielder << "/#{name}" - yielder << "/:version/#{name}" - end - - patterns.any? { |p| route.namespace == p } - end - - def route_path_start_with?(route, name) - patterns = Enumerator.new do |yielder| - if route.prefix - yielder << "/#{route.prefix}/#{name}" - yielder << "/#{route.prefix}/:version/#{name}" - else - yielder << "/#{name}" - yielder << "/:version/#{name}" - end - end - - patterns.any? { |p| route.path.start_with?(p) } - end -end - -module SwaggerDocumentationAdder - attr_accessor :combined_namespaces, :combined_routes, :combined_namespace_routes - - include SwaggerRouting - - def add_swagger_documentation(options = {}) - documentation_class = create_documentation_class - - version_for(options) - options = { target_class: self }.merge(options) - @target_class = options[:target_class] - auth_wrapper = options[:endpoint_auth_wrapper] || Class.new - - use auth_wrapper if auth_wrapper.method_defined?(:before) && !middleware.flatten.include?(auth_wrapper) - - documentation_class.setup(options) - mount(documentation_class) - - combined_routes = combine_routes(@target_class, documentation_class) - combined_namespaces = combine_namespaces(@target_class) - combined_namespace_routes = combine_namespace_routes(combined_namespaces, combined_routes) - exclusive_route_keys = combined_routes.keys - combined_namespaces.keys - @target_class.combined_namespace_routes = combined_namespace_routes.merge( - combined_routes.slice(*exclusive_route_keys) - ) - @target_class.combined_routes = combined_routes - @target_class.combined_namespaces = combined_namespaces - - documentation_class - end - - private - - def version_for(options) - options[:version] = version if version - end - - def combine_namespaces(app) - combined_namespaces = {} - endpoints = app.endpoints.clone - - while endpoints.any? - endpoint = endpoints.shift - - endpoints.push(*endpoint.options[:app].endpoints) if endpoint.options[:app] - namespace_stackable = endpoint.inheritable_setting.namespace_stackable - ns = (namespace_stackable[:namespace] || []).last - next unless ns - - # use the full namespace here (not the latest level only) - # and strip leading slash - mount_path = (namespace_stackable[:mount_path] || []).join('/') - full_namespace = (mount_path + endpoint.namespace).sub(/\/{2,}/, '/').sub(/^\//, '') - combined_namespaces[full_namespace] = ns - end - - combined_namespaces - end - - def create_documentation_class - Class.new(GrapeInstance) do - extend GrapeSwagger::DocMethods - end - end -end - GrapeInstance.extend(SwaggerDocumentationAdder) diff --git a/lib/grape-swagger/request_param_parsers/route.rb b/lib/grape-swagger/request_param_parsers/route.rb index d3d27d20c..734bd2bbb 100644 --- a/lib/grape-swagger/request_param_parsers/route.rb +++ b/lib/grape-swagger/request_param_parsers/route.rb @@ -57,7 +57,8 @@ def fulfill_params(path_params) next if param.is_a?(String) && accum.key?(key) defined_options = definition.is_a?(Hash) ? definition : {} - value = (path_params[param] || {}).merge(defined_options) + path_options = path_params[param] || path_params[param.to_s] || path_params[param.to_sym] || {} + value = path_options.merge(defined_options) accum[key] = value.empty? ? DEFAULT_PARAM_TYPE : value end end diff --git a/lib/grape-swagger/swagger_documentation_adder.rb b/lib/grape-swagger/swagger_documentation_adder.rb new file mode 100644 index 000000000..da97c7ebc --- /dev/null +++ b/lib/grape-swagger/swagger_documentation_adder.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module SwaggerDocumentationAdder + attr_accessor :combined_namespaces, :combined_routes, :combined_namespace_routes + + include SwaggerRouting + + def add_swagger_documentation(options = {}) + documentation_class = create_documentation_class + + version_for(options) + options = { target_class: self }.merge(options) + @target_class = options[:target_class] + auth_wrapper = options[:endpoint_auth_wrapper] || Class.new + + use auth_wrapper if auth_wrapper.method_defined?(:before) && !middleware.flatten.include?(auth_wrapper) + + documentation_class.setup(options) + mount(documentation_class) + + combined_routes = combine_routes(@target_class, documentation_class) + combined_namespaces = combine_namespaces(@target_class) + combined_namespace_routes = combine_namespace_routes(combined_namespaces, combined_routes) + exclusive_route_keys = combined_routes.keys - combined_namespaces.keys + @target_class.combined_namespace_routes = combined_namespace_routes.merge( + combined_routes.slice(*exclusive_route_keys) + ) + @target_class.combined_routes = combined_routes + @target_class.combined_namespaces = combined_namespaces + + documentation_class + end + + private + + def version_for(options) + options[:version] = version if version + end + + def combine_namespaces(app) + combined_namespaces = {} + endpoints = app.endpoints.clone + + while endpoints.any? + endpoint = endpoints.shift + + endpoints.push(*endpoint.options[:app].endpoints) if endpoint.options[:app] + namespace_stackable = endpoint.inheritable_setting.namespace_stackable + ns = (namespace_stackable[:namespace] || []).last + next unless ns + + # use the full namespace here (not the latest level only) + # and strip leading slash + mount_path = (namespace_stackable[:mount_path] || []).join('/') + full_namespace = (mount_path + endpoint.namespace).sub(/\/{2,}/, '/').sub(/^\//, '') + combined_namespaces[full_namespace] = ns + end + + combined_namespaces + end + + def create_documentation_class + Class.new(GrapeInstance) do + extend GrapeSwagger::DocMethods + end + end +end diff --git a/lib/grape-swagger/swagger_routing.rb b/lib/grape-swagger/swagger_routing.rb new file mode 100644 index 000000000..4c4353a98 --- /dev/null +++ b/lib/grape-swagger/swagger_routing.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +module SwaggerRouting + private + + def combine_routes(app, doc_klass) + app.routes.each_with_object({}) do |route, combined_routes| + route_path = route.path + route_match = route_path.split(/^.*?#{route.prefix}/).last + next unless route_match + + # want to match emojis ... ;) + # route_match = route_match + # .match('\/([\p{Alnum}p{Emoji}\-\_]*?)[\.\/\(]') || route_match.match('\/([\p{Alpha}\p{Emoji}\-\_]*)$') + route_match = route_match.match('\/([\p{Alnum}\-\_]*?)[\.\/\(]') || route_match.match('\/([\p{Alpha}\-\_]*)$') + next unless route_match + + resource = route_match.captures.first + resource = '/' if resource.empty? + combined_routes[resource] ||= [] + next if doc_klass.hide_documentation_path && route.path.match(/#{doc_klass.mount_path}($|\/|\(\.)/) + + combined_routes[resource] << route + end + end + + def determine_namespaced_routes(name, parent_route, routes) + return routes.values.flatten if parent_route.nil? + + parent_route.select do |route| + route_path_start_with?(route, name) || route_namespace_equals?(route, name) + end + end + + def combine_namespace_routes(namespaces, routes) + combined_namespace_routes = {} + # iterate over each single namespace + namespaces.each_key do |name, _| + # get the parent route for the namespace + parent_route_name = extract_parent_route(name) + parent_route = routes[parent_route_name] + # fetch all routes that are within the current namespace + namespace_routes = determine_namespaced_routes(name, parent_route, routes) + + # default case when not explicitly specified or nested == true + standalone_namespaces = namespaces.reject do |_, ns| + !ns.options.key?(:swagger) || + !ns.options[:swagger].key?(:nested) || + ns.options[:swagger][:nested] != false + end + + parent_standalone_namespaces = standalone_namespaces.select { |ns_name, _| name.start_with?(ns_name) } + # add only to the main route + # if the namespace is not within any other namespace appearing as standalone resource + # rubocop:disable Style/Next + if parent_standalone_namespaces.empty? + # default option, append namespace methods to parent route + combined_namespace_routes[parent_route_name] ||= [] + combined_namespace_routes[parent_route_name].push(*namespace_routes) + end + # rubocop:enable Style/Next + end + + combined_namespace_routes + end + + def extract_parent_route(name) + route_name = name.match(%r{^/?([^/]*).*$})[1] + return route_name unless route_name.include? ':' + + matches = name.match(/\/\p{Alpha}+/) + matches.nil? ? route_name : matches[0].delete('/') + end + + def route_namespace_equals?(route, name) + patterns = Enumerator.new do |yielder| + yielder << "/#{name}" + yielder << "/:version/#{name}" + end + + patterns.any? { |p| route.namespace == p } + end + + def route_path_start_with?(route, name) + patterns = Enumerator.new do |yielder| + if route.prefix + yielder << "/#{route.prefix}/#{name}" + yielder << "/#{route.prefix}/:version/#{name}" + else + yielder << "/#{name}" + yielder << "/:version/#{name}" + end + end + + patterns.any? { |p| route.path.start_with?(p) } + end +end diff --git a/spec/issues/537_enum_values_spec.rb b/spec/issues/537_enum_values_spec.rb index 6e1bf168f..2ebe85b64 100644 --- a/spec/issues/537_enum_values_spec.rb +++ b/spec/issues/537_enum_values_spec.rb @@ -6,14 +6,14 @@ let(:app) do Class.new(Grape::API) do namespace :issue_537 do - class Spec < Grape::Entity + class Issue537Spec < Grape::Entity expose :enum_property, documentation: { values: %i[foo bar] } expose :enum_property_default, documentation: { values: %w[a b c], default: 'c' } expose :own_format, documentation: { format: 'log' } end desc 'create account', - success: Spec + success: Issue537Spec get do { message: 'hi' } end @@ -28,13 +28,13 @@ class Spec < Grape::Entity JSON.parse(last_response.body) end - let(:property) { subject['definitions']['Spec']['properties']['enum_property'] } + let(:property) { subject['definitions']['Issue537Spec']['properties']['enum_property'] } specify do expect(property).to include 'enum' expect(property['enum']).to eql %w[foo bar] end - let(:property_default) { subject['definitions']['Spec']['properties']['enum_property_default'] } + let(:property_default) { subject['definitions']['Issue537Spec']['properties']['enum_property_default'] } specify do expect(property_default).to include 'enum' expect(property_default['enum']).to eql %w[a b c] @@ -42,7 +42,7 @@ class Spec < Grape::Entity expect(property_default['default']).to eql 'c' end - let(:own_format) { subject['definitions']['Spec']['properties']['own_format'] } + let(:own_format) { subject['definitions']['Issue537Spec']['properties']['own_format'] } specify do expect(own_format).to include 'format' expect(own_format['format']).to eql 'log' diff --git a/spec/issues/579_align_put_post_parameters_spec.rb b/spec/issues/579_align_put_post_parameters_spec.rb index 466f06565..cb555d409 100644 --- a/spec/issues/579_align_put_post_parameters_spec.rb +++ b/spec/issues/579_align_put_post_parameters_spec.rb @@ -12,7 +12,7 @@ class BodySpec < Grape::Entity expose :content, documentation: { type: String, in: 'body' } end - class Spec < Grape::Entity + class Issue579Spec < Grape::Entity expose :guid, documentation: { type: String, format: 'guid' } expose :name, documentation: { type: String } expose :content, documentation: { type: String } @@ -32,8 +32,8 @@ class Spec < Grape::Entity namespace :form_parameter do desc 'update spec', consumes: ['application/x-www-form-urlencoded'], - success: Spec, - params: Spec.documentation + success: Issue579Spec, + params: Issue579Spec.documentation put ':guid' do # your code goes here end @@ -57,8 +57,8 @@ class Spec < Grape::Entity namespace :form_parameter do desc 'update spec', consumes: ['application/x-www-form-urlencoded'], - success: Spec, - params: Spec.documentation + success: Issue579Spec, + params: Issue579Spec.documentation params do requires :guid end @@ -83,8 +83,8 @@ class Spec < Grape::Entity namespace :form_parameter do desc 'update spec', consumes: ['application/x-www-form-urlencoded'], - success: Spec, - params: Spec.documentation + success: Issue579Spec, + params: Issue579Spec.documentation put do # your code goes here end diff --git a/spec/lib/parse_params_spec.rb b/spec/lib/parse_params_spec.rb index efb1de84a..51e82f71c 100644 --- a/spec/lib/parse_params_spec.rb +++ b/spec/lib/parse_params_spec.rb @@ -79,4 +79,19 @@ end end end + + describe '#parse_additional_properties' do + let(:definitions) { {} } + + it 'warns and reads deprecated additionalProperties option' do + settings = { additionalProperties: true } + allow(GrapeSwagger::Errors::SwaggerSpecDeprecated).to receive(:tell!) + + parsed = subject.send(:parse_additional_properties, definitions, settings) + + expect(GrapeSwagger::Errors::SwaggerSpecDeprecated) + .to have_received(:tell!).with(:additionalProperties) + expect(parsed).to eql([true, true]) + end + end end diff --git a/spec/lib/request_param_parsers/route_spec.rb b/spec/lib/request_param_parsers/route_spec.rb new file mode 100644 index 000000000..8954f334b --- /dev/null +++ b/spec/lib/request_param_parsers/route_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +describe GrapeSwagger::RequestParamParsers::Route do + let(:route) { instance_double('route', app: nil) } + let(:parser) { described_class.new(route, nil, nil, nil) } + + describe '#parse' do + subject(:parse_request_params) { described_class.parse(route, nil, nil, nil) } + + context 'when inherited namespace stackable values contain path params across levels' do + let(:root_stackable) { Grape::Util::StackableValues.new } + let(:nested_stackable) { Grape::Util::StackableValues.new(root_stackable) } + let(:inheritable_setting) { instance_double('inheritable_setting', namespace_stackable: nested_stackable) } + let(:app) { instance_double('app', inheritable_setting: inheritable_setting) } + + before do + root_stackable[:namespace] = instance_double('namespace', space: ':account_id', options: { required: true, type: 'Integer' }) + nested_stackable[:namespace] = instance_double('namespace', space: ':id', options: { required: true, type: 'String' }) + + allow(route).to receive(:app).and_return(app) + allow(route).to receive(:params).and_return( + 'account_id' => {}, + 'id' => {} + ) + end + + it 'merges path params from the full inherited stackable chain' do + expect(parse_request_params).to eq( + account_id: { required: true, type: 'Integer' }, + id: { required: true, type: 'String' } + ) + end + end + + context 'when route params include both path-derived and explicitly defined keys' do + before do + allow(route).to receive(:params).and_return( + 'id' => {}, + id: { required: true, type: 'String' }, + name: { required: false, type: 'String' } + ) + end + + it 'keeps explicitly defined params over inferred string path params' do + expect(parse_request_params).to eq( + id: { required: true, type: 'String' }, + name: { required: false, type: 'String' } + ) + end + end + end + + describe '#fulfill_params' do + subject(:fulfilled_params) { parser.send(:fulfill_params, path_params) } + + context 'when route.params uses symbol keys and path params use string keys' do + let(:path_params) { { 'id' => { required: true, type: 'Integer' } } } + + before do + allow(route).to receive(:params).and_return( + id: {} + ) + end + + it 'looks up path options via param.to_s' do + expect(fulfilled_params).to eq( + id: { required: true, type: 'Integer' } + ) + end + end + + context 'when route.params uses string keys and path params use symbol keys' do + let(:path_params) { { id: { required: true, type: 'Integer' } } } + + before do + allow(route).to receive(:params).and_return( + 'id' => {} + ) + end + + it 'looks up path options via param.to_sym' do + expect(fulfilled_params).to eq( + id: { required: true, type: 'Integer' } + ) + end + end + end +end diff --git a/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb b/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb index cf394f99e..32eaae098 100644 --- a/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb +++ b/spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb @@ -59,7 +59,7 @@ class BodyParamTypeApi < Grape::API namespace :with_entity_param do desc 'put in body with entity parameter' params do - optional :data, type: ::Entities::NestedModule::ApiResponse, documentation: { desc: 'request data' } + optional :data, type: String, documentation: { type: ::Entities::NestedModule::ApiResponse, desc: 'request data' } end post do diff --git a/spec/swagger_v2/params_example_spec.rb b/spec/swagger_v2/params_example_spec.rb index fbddf7cca..e58dc6dab 100644 --- a/spec/swagger_v2/params_example_spec.rb +++ b/spec/swagger_v2/params_example_spec.rb @@ -11,7 +11,7 @@ def app params :common_params do requires :id, type: Integer, documentation: { example: 123 } optional :name, type: String, documentation: { example: 'Person' } - optional :obj, type: 'Object', documentation: { example: { 'foo' => 'bar' } } + optional :obj, type: Hash, documentation: { type: 'Object', example: { 'foo' => 'bar' } } end end