diff --git a/Gemfile b/Gemfile index 5a1116308..3a4049676 100644 --- a/Gemfile +++ b/Gemfile @@ -19,6 +19,7 @@ group :development, :test do gem 'grape-entity' gem 'pry', platforms: [:mri] gem 'pry-byebug', platforms: [:mri] + gem 'super_diff', require: false grape_version = ENV.fetch('GRAPE_VERSION', '2.2.0') if grape_version == 'HEAD' || Gem::Version.new(grape_version) >= Gem::Version.new('2.0.0') diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 7a36ea0c9..8eb429348 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -10,22 +10,27 @@ require 'grape-swagger/doc_methods' require 'grape-swagger/model_parsers' +require 'grape-swagger/request_param_parser_registry' module GrapeSwagger class << self def model_parsers @model_parsers ||= GrapeSwagger::ModelParsers.new end + + def request_param_parsers + @request_param_parsers ||= GrapeSwagger::RequestParamParserRegistry.new + end end autoload :Rake, 'grape-swagger/rake/oapi_tasks' # Copied from https://github.com/ruby-grape/grape/blob/v2.2.0/lib/grape/formatter.rb FORMATTER_DEFAULTS = { + xml: Grape::Formatter::Xml, + serializable_hash: Grape::Formatter::SerializableHash, json: Grape::Formatter::Json, jsonapi: Grape::Formatter::Json, - serializable_hash: Grape::Formatter::SerializableHash, - txt: Grape::Formatter::Txt, - xml: Grape::Formatter::Xml + txt: Grape::Formatter::Txt }.freeze # Copied from https://github.com/ruby-grape/grape/blob/v2.2.0/lib/grape/content_types.rb diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index efb850e34..9e22499d7 100644 --- a/lib/grape-swagger/doc_methods.rb +++ b/lib/grape-swagger/doc_methods.rb @@ -11,7 +11,6 @@ require 'grape-swagger/doc_methods/tag_name_description' require 'grape-swagger/doc_methods/parse_params' require 'grape-swagger/doc_methods/move_params' -require 'grape-swagger/doc_methods/headers' require 'grape-swagger/doc_methods/build_model_definition' require 'grape-swagger/doc_methods/version' diff --git a/lib/grape-swagger/doc_methods/format_data.rb b/lib/grape-swagger/doc_methods/format_data.rb index dee52b41b..4a6ca9447 100644 --- a/lib/grape-swagger/doc_methods/format_data.rb +++ b/lib/grape-swagger/doc_methods/format_data.rb @@ -7,7 +7,7 @@ class << self def to_format(parameters) parameters.reject { |parameter| parameter[:in] == 'body' }.each do |b| related_parameters = parameters.select do |p| - p[:name] != b[:name] && p[:name].to_s.start_with?("#{b[:name].to_s.gsub(/\[\]\z/, '')}[") + p[:name] != b[:name] && p[:name].start_with?("#{b[:name].to_s.gsub(/\[\]\z/, '')}[") end parameters.reject! { |p| p[:name] == b[:name] } if move_down(b, related_parameters) end diff --git a/lib/grape-swagger/doc_methods/headers.rb b/lib/grape-swagger/doc_methods/headers.rb deleted file mode 100644 index e84ec4565..000000000 --- a/lib/grape-swagger/doc_methods/headers.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -module GrapeSwagger - module DocMethods - class Headers - class << self - def parse(route) - route.headers.to_a.map do |route_header| - route_header.tap do |header| - hash = header[1] - description = hash.delete('description') - hash[:documentation] = { desc: description, in: 'header' } - hash[:type] = hash['type'].titleize if hash['type'] - end - end - end - end - end - end -end diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index 7329b46c8..c8417ce5a 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -163,7 +163,7 @@ def object_type end def prepare_nested_names(property, params) - params.each { |x| x[:name] = x[:name].sub(property, '').sub('[', '').sub(']', '') } + params.each { |x| x[:name] = x[:name].sub(property.to_s, '').sub('[', '').sub(']', '') } end def unify!(params) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 1c4274f00..54dd81915 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -2,7 +2,9 @@ require 'active_support' require 'active_support/core_ext/string/inflections' -require 'grape-swagger/endpoint/params_parser' +require_relative 'request_param_parsers/headers' +require_relative 'request_param_parsers/route' +require_relative 'request_param_parsers/body' module Grape class Endpoint # rubocop:disable Metrics/ClassLength @@ -12,9 +14,7 @@ def content_types_for(target_class) if content_types.empty? formats = [target_class.format, target_class.default_format].compact.uniq formats = GrapeSwagger::FORMATTER_DEFAULTS.keys if formats.empty? - content_types = GrapeSwagger::CONTENT_TYPE_DEFAULTS.select do |content_type, _mime_type| # rubocop:disable Style/HashSlice - formats.include? content_type - end.values + content_types = formats.filter_map { |f| GrapeSwagger::CONTENT_TYPE_DEFAULTS[f] } end content_types.uniq @@ -383,45 +383,15 @@ def build_file_response(memo) end def build_request_params(route, settings) - required = merge_params(route) - required = GrapeSwagger::DocMethods::Headers.parse(route) + required unless route.headers.nil? - - default_type(required) - - request_params = GrapeSwagger::Endpoint::ParamsParser.parse_request_params(required, settings, self) - - request_params.empty? ? required : request_params - end - - def merge_params(route) - path_params = get_path_params(route.app&.inheritable_setting&.namespace_stackable) - param_keys = route.params.keys - - # Merge path params options into route params - route_params = route.params - route_params.each_key do |key| - path = path_params[key] || {} - params = route_params[key] - params = {} unless params.is_a? Hash - route_params[key] = path.merge(params) - end - - route_params.delete_if { |key| key.is_a?(String) && param_keys.include?(key.to_sym) }.to_a - end - - # Iterates over namespaces recursively - # to build a hash of path params with options, including type - def get_path_params(stackable_values) - params = {} - return param unless stackable_values - return params unless stackable_values.is_a? Grape::Util::StackableValues - - stackable_values&.new_values&.dig(:namespace)&.each do |namespace| # rubocop:disable Style/SafeNavigationChainLength - space = namespace.space.to_s.gsub(':', '') - params[space] = namespace.options || {} + GrapeSwagger.request_param_parsers.each_with_object({}) do |parser_klass, accum| + params = parser_klass.parse( + route, + accum, + settings, + self + ) + accum.merge!(params.stringify_keys) end - inherited_params = get_path_params(stackable_values.inherited_values) - inherited_params.merge(params) end def default_type(params) diff --git a/lib/grape-swagger/request_param_parser_registry.rb b/lib/grape-swagger/request_param_parser_registry.rb new file mode 100644 index 000000000..17d292018 --- /dev/null +++ b/lib/grape-swagger/request_param_parser_registry.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require_relative 'request_param_parsers/headers' +require_relative 'request_param_parsers/route' +require_relative 'request_param_parsers/body' + +module GrapeSwagger + class RequestParamParserRegistry + DEFAULT_PARSERS = [ + GrapeSwagger::RequestParamParsers::Headers, + GrapeSwagger::RequestParamParsers::Route, + GrapeSwagger::RequestParamParsers::Body + ].freeze + + include Enumerable + + def initialize + @parsers = DEFAULT_PARSERS.dup + end + + def register(klass) + remove_parser(klass) + @parsers << klass + end + + def insert_before(before_klass, klass) + remove_parser(klass) + insert_at = @parsers.index(before_klass) || @parsers.size + @parsers.insert(insert_at, klass) + end + + def insert_after(after_klass, klass) + remove_parser(klass) + insert_at = @parsers.index(after_klass) + @parsers.insert(insert_at ? insert_at + 1 : @parsers.size, klass) + end + + def each(&) + @parsers.each(&) + end + + private + + def remove_parser(klass) + @parsers.reject! { |k| k == klass } + end + end +end diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/request_param_parsers/body.rb similarity index 70% rename from lib/grape-swagger/endpoint/params_parser.rb rename to lib/grape-swagger/request_param_parsers/body.rb index 1d0381516..b8814396d 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/request_param_parsers/body.rb @@ -1,27 +1,26 @@ # frozen_string_literal: true module GrapeSwagger - module Endpoint - class ParamsParser - attr_reader :params, :settings, :endpoint + module RequestParamParsers + class Body + attr_reader :route, :params, :settings, :endpoint - def self.parse_request_params(params, settings, endpoint) - new(params, settings, endpoint).parse_request_params + def self.parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse end - def initialize(params, settings, endpoint) + def initialize(_route, params, settings, endpoint) @params = params @settings = settings @endpoint = endpoint end - def parse_request_params + def parse public_params.each_with_object({}) do |(name, options), memo| name = name.to_s - param_type = options[:type] - param_type = param_type.to_s unless param_type.nil? + param_type = options[:type]&.to_s - if param_type_is_array?(param_type) + if array_param?(param_type) options[:is_array] = true name += '[]' if array_use_braces? end @@ -36,8 +35,8 @@ def array_use_braces? @array_use_braces ||= settings[:array_use_braces] && !includes_body_param? end - def param_type_is_array?(param_type) - return false unless param_type + def array_param?(param_type) + return false if param_type.nil? return true if param_type == 'Array' param_types = param_type.match(/\[(.*)\]$/) @@ -48,11 +47,10 @@ def param_type_is_array?(param_type) end def public_params - params.select { |param| public_parameter?(param) } + params.select { |_key, param| public_parameter?(param) } end - def public_parameter?(param) - param_options = param.last + def public_parameter?(param_options) return true unless param_options.key?(:documentation) && !param_options[:required] param_hidden = param_options[:documentation].fetch(:hidden, false) diff --git a/lib/grape-swagger/request_param_parsers/headers.rb b/lib/grape-swagger/request_param_parsers/headers.rb new file mode 100644 index 000000000..6115a4782 --- /dev/null +++ b/lib/grape-swagger/request_param_parsers/headers.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module GrapeSwagger + module RequestParamParsers + class Headers + attr_reader :route + + def self.parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse + end + + def initialize(route, _params, _settings, _endpoint) + @route = route + end + + def parse + return {} unless route.headers + + route.headers.each_with_object({}) do |(name, definition), accum| + # Extract the description from any key type (string or symbol) + description = definition[:description] || definition['description'] + doc = { desc: description, in: 'header' } + + header_attrs = definition.symbolize_keys.except(:description, 'description') + header_attrs[:type] = definition[:type].titleize if definition[:type] + header_attrs[:documentation] = doc + + accum[name] = header_attrs + end + end + end + end +end diff --git a/lib/grape-swagger/request_param_parsers/route.rb b/lib/grape-swagger/request_param_parsers/route.rb new file mode 100644 index 000000000..d3d27d20c --- /dev/null +++ b/lib/grape-swagger/request_param_parsers/route.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +module GrapeSwagger + module RequestParamParsers + class Route + DEFAULT_PARAM_TYPE = { required: true, type: 'Integer' }.freeze + + attr_reader :route + + def self.parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse + end + + def initialize(route, _params, _settings, _endpoint) + @route = route + end + + def parse + stackable_values = route.app&.inheritable_setting&.namespace_stackable + + path_params = build_path_params(stackable_values) + + fulfill_params(path_params) + end + + private + + def build_path_params(stackable_values) + params = {} + + while stackable_values.is_a?(Grape::Util::StackableValues) + params.merge!(fetch_inherited_params(stackable_values)) + stackable_values = stackable_values.inherited_values + end + + params + end + + def fetch_inherited_params(stackable_values) + return {} unless stackable_values.new_values + + namespaces = stackable_values.new_values[:namespace] || [] + + namespaces.each_with_object({}) do |namespace, params| + space = namespace.space.to_s.gsub(':', '') + params[space] = namespace.options || {} + end + end + + def fulfill_params(path_params) + # Merge path params options into route params + route.params.each_with_object({}) do |(param, definition), accum| + # The route.params hash includes both parametrized params (with a string as a key) + # and well-defined params from body/query (with a symbol as a key). + # We avoid overriding well-defined params with parametrized ones. + key = param.is_a?(String) ? param.to_sym : param + next if param.is_a?(String) && accum.key?(key) + + defined_options = definition.is_a?(Hash) ? definition : {} + value = (path_params[param] || {}).merge(defined_options) + accum[key] = value.empty? ? DEFAULT_PARAM_TYPE : value + end + end + end + end +end diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index a577285fd..ba6724488 100644 --- a/spec/lib/endpoint_spec.rb +++ b/spec/lib/endpoint_spec.rb @@ -47,9 +47,9 @@ end describe 'parse_request_params' do - let(:subject) { GrapeSwagger::Endpoint::ParamsParser } + let(:subject) { GrapeSwagger::RequestParamParsers::Body } before do - subject.send(:parse_request_params, params, {}, nil) + subject.parse(nil, params, {}, nil) end context 'when params do not contain an array' do diff --git a/spec/lib/endpoint/params_parser_spec.rb b/spec/lib/request_param_parsers/body_spec.rb similarity index 84% rename from spec/lib/endpoint/params_parser_spec.rb rename to spec/lib/request_param_parsers/body_spec.rb index 347d88189..79add0a8c 100644 --- a/spec/lib/endpoint/params_parser_spec.rb +++ b/spec/lib/request_param_parsers/body_spec.rb @@ -2,15 +2,15 @@ require 'spec_helper' -describe GrapeSwagger::Endpoint::ParamsParser do +describe GrapeSwagger::RequestParamParsers::Body do let(:settings) { {} } let(:params) { [] } let(:endpoint) { nil } - let(:parser) { described_class.new(params, settings, endpoint) } + let(:parser) { described_class.new(nil, params, settings, endpoint) } describe '#parse_request_params' do - subject(:parse_request_params) { parser.parse_request_params } + subject(:parse_request_params) { parser.parse } context 'when param is of array type' do let(:params) { [['param_1', { type: 'Array[String]' }]] } @@ -39,7 +39,7 @@ let(:settings) { { array_use_braces: true } } it 'does not add braces to the param key' do - expect(parser.parse_request_params.keys.first).to eq 'param_1' + expect(parser.parse.keys.first).to eq 'param_1' end end end @@ -109,16 +109,16 @@ end end - describe '#param_type_is_array?' do + describe '#array_param?' do it 'returns true if the value passed represents an array' do - expect(parser.send(:param_type_is_array?, 'Array')).to be_truthy - expect(parser.send(:param_type_is_array?, '[String]')).to be_truthy - expect(parser.send(:param_type_is_array?, 'Array[Integer]')).to be_truthy + expect(parser.send(:array_param?, 'Array')).to be_truthy + expect(parser.send(:array_param?, '[String]')).to be_truthy + expect(parser.send(:array_param?, 'Array[Integer]')).to be_truthy end it 'returns false if the value passed does not represent an array' do - expect(parser.send(:param_type_is_array?, 'String')).to be_falsey - expect(parser.send(:param_type_is_array?, '[String, Integer]')).to be_falsey + expect(parser.send(:array_param?, 'String')).to be_falsey + expect(parser.send(:array_param?, '[String, Integer]')).to be_falsey end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e236c8ca5..2a27c8e1f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,7 @@ require 'rack' require 'rack/test' +require 'super_diff/rspec' if ENV.key?('SUPER_DIFF') RSpec.configure do |config| require 'rspec/expectations'