From f86a88f4ef251bc0170cd23903471a659ca97d2d Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Mon, 10 Feb 2025 09:23:36 +0100 Subject: [PATCH 01/11] wip --- Gemfile | 1 + .../doc_methods/build_model_definition.rb | 22 ++++++++++++++- spec/spec_helper.rb | 2 ++ .../custom_parsed_type_parser.rb | 28 +++++++++++++++++++ .../api_swagger_v2_param_type_body_spec.rb | 26 ++++++++++++++++- 5 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 spec/support/model_parsers/custom_parsed_type_parser.rb diff --git a/Gemfile b/Gemfile index 5a111630..7ca79e45 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 'dry-schema' 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/doc_methods/build_model_definition.rb b/lib/grape-swagger/doc_methods/build_model_definition.rb index 444b17f8..8cddbaa2 100644 --- a/lib/grape-swagger/doc_methods/build_model_definition.rb +++ b/lib/grape-swagger/doc_methods/build_model_definition.rb @@ -4,6 +4,10 @@ module GrapeSwagger module DocMethods class BuildModelDefinition class << self + OBJECT_ATTRIBUTE_KEYS = [ + :$ref, :type, + ].freeze + def build(_model, properties, required, other_def_properties = {}) definition = { type: 'object', properties: properties }.merge(other_def_properties) @@ -13,6 +17,10 @@ def build(_model, properties, required, other_def_properties = {}) end def parse_params_from_model(parsed_response, model, model_name) + # If the parsed response looks like a complete object (e.g., containing `$ref` or `type`), + # it uses the provided response as-is. + return parsed_response if complete_object?(parsed_response) + if parsed_response.is_a?(Hash) && parsed_response.keys.first == :allOf refs_or_models = parsed_response[:allOf] parsed = parse_refs_and_models(refs_or_models, model) @@ -54,7 +62,7 @@ def parse_properties(properties) def parse_refs_and_models(refs_or_models, model) refs_or_models.map do |ref_or_models| - if ref_or_models.is_a?(Hash) && ref_or_models.keys.first == '$ref' + if complete_object?(ref_or_models) ref_or_models else properties, required = ref_or_models @@ -62,6 +70,18 @@ def parse_refs_and_models(refs_or_models, model) end end end + + private + + # Checks if the parsed response is already a complete object. + # + # @param parsed_response [Hash] The parsed response to check. + # @return [Boolean] True if the response has object attributes. + def complete_object?(parsed_response) + return false unless parsed_response.is_a?(Hash) + + parsed_response.keys.intersect?(OBJECT_ATTRIBUTE_KEYS) + end end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e236c8ca..5df6bf25 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,6 +11,8 @@ Dir[File.join(Dir.getwd, 'spec/support/*.rb')].each { |f| require f } require "grape-swagger/#{MODEL_PARSER}" if MODEL_PARSER != 'mock' require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_parser.rb") +require_relative './support/model_parsers/custom_parsed_type_parser' + require 'grape-entity' require 'grape-swagger-entity' diff --git a/spec/support/model_parsers/custom_parsed_type_parser.rb b/spec/support/model_parsers/custom_parsed_type_parser.rb new file mode 100644 index 00000000..5c8dae52 --- /dev/null +++ b/spec/support/model_parsers/custom_parsed_type_parser.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +CustomParsedType = Class.new + +class CustomParsedTypeParser + attr_reader :model, :endpoint + + def initialize(model, endpoint) + @model = model + @endpoint = endpoint + end + + def call + { + type: "object", + properties: { + custom: { + type: 'boolean', + description: "it's a custom type", + default: true, + } + }, + required: [], + } + end +end + +GrapeSwagger.model_parsers.register(CustomParsedTypeParser, CustomParsedType) 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 cf394f99..06d7d5a5 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 @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'spec_helper' +require 'dry-schema' describe 'setting of param type, such as `query`, `path`, `formData`, `body`, `header`' do include_context "#{MODEL_PARSER} swagger example" @@ -67,6 +68,18 @@ class BodyParamTypeApi < Grape::API end end + namespace :with_dry_schema_params do + contract do + required(:orders).array(:hash) do + required(:name).filled(:string) + optional(:volume).maybe(:integer, lt?: 9) + end + end + post do + { 'declared_params' => declared(params) } + end + end + add_swagger_documentation end end @@ -78,7 +91,7 @@ def app describe 'no entity given' do subject do - get '/swagger_doc/wo_entities' + get '/swagger_doc/' JSON.parse(last_response.body) end @@ -170,6 +183,17 @@ def app end end + describe 'with exceed type parser result' do + subject do + get '/swagger_doc/with_dry_schema_params' + JSON.parse(last_response.body) + end + + specify do + expect(subject['paths']['/with_dry_schema_params']).to eq({}) + end + end + describe 'complex entity given' do let(:request_parameters_definition) do [ From 22ce1ad5529143986b34084f1a7f65932a2acb28 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 11:06:49 +0100 Subject: [PATCH 02/11] Form params parser. First step --- lib/grape-swagger/doc_methods.rb | 1 - lib/grape-swagger/doc_methods/move_params.rb | 4 +- lib/grape-swagger/endpoint.rb | 62 ++++++-------- lib/grape-swagger/endpoint/contract_parser.rb | 38 +++++++++ .../endpoint/header_params_parser.rb | 32 ++++++++ .../headers.rb => endpoint/headers_parser.rb} | 4 +- lib/grape-swagger/endpoint/params_parser.rb | 20 +++-- .../endpoint/path_params_parser.rb | 81 +++++++++++++++++++ spec/lib/endpoint_spec.rb | 4 +- spec/spec_helper.rb | 1 + .../api_swagger_v2_param_type_body_spec.rb | 1 + 11 files changed, 198 insertions(+), 50 deletions(-) create mode 100644 lib/grape-swagger/endpoint/contract_parser.rb create mode 100644 lib/grape-swagger/endpoint/header_params_parser.rb rename lib/grape-swagger/{doc_methods/headers.rb => endpoint/headers_parser.rb} (92%) create mode 100644 lib/grape-swagger/endpoint/path_params_parser.rb diff --git a/lib/grape-swagger/doc_methods.rb b/lib/grape-swagger/doc_methods.rb index efb850e3..9e22499d 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/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index 7329b46c..ca0cf56a 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -163,7 +163,9 @@ 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].to_s.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 1c4274f0..380619b7 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -3,9 +3,19 @@ require 'active_support' require 'active_support/core_ext/string/inflections' require 'grape-swagger/endpoint/params_parser' +require 'grape-swagger/endpoint/path_params_parser' +require 'grape-swagger/endpoint/contract_parser' +require 'grape-swagger/endpoint/header_params_parser' module Grape class Endpoint # rubocop:disable Metrics/ClassLength + REQUEST_PARAM_PARSERS = [ + GrapeSwagger::Endpoint::PathParamsParser, + GrapeSwagger::Endpoint::HeaderParamsParser, + GrapeSwagger::Endpoint::ContractParser, + GrapeSwagger::Endpoint::ParamsParser, + ].freeze + def content_types_for(target_class) content_types = (target_class.content_types || {}).values @@ -383,45 +393,25 @@ 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) + # default_type(required) + # request_params.empty? ? required : request_params + + result = REQUEST_PARAM_PARSERS.each_with_object({}) do |parser_klass, accum| + params = parser_klass.parse( + route, + accum, + settings, + self + ) + puts "KLASS: #{parser_klass}" + puts "ACCUM: #{accum.inspect}" + puts "PARMS: #{params.inspect}" + accum.merge!(params.symbolize_keys) end - route_params.delete_if { |key| key.is_a?(String) && param_keys.include?(key.to_sym) }.to_a - end + puts "TOTAL PARMS: #{result.inspect}" - # 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 || {} - end - inherited_params = get_path_params(stackable_values.inherited_values) - inherited_params.merge(params) + result end def default_type(params) diff --git a/lib/grape-swagger/endpoint/contract_parser.rb b/lib/grape-swagger/endpoint/contract_parser.rb new file mode 100644 index 00000000..f4f31c3d --- /dev/null +++ b/lib/grape-swagger/endpoint/contract_parser.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module GrapeSwagger + module Endpoint + class ContractParser + attr_reader :route, :params, :settings, :endpoint + + class << self + def parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse + end + + alias parse_request_params parse + end + + def initialize(route, params, settings, endpoint) + @route = route + @params = params + @settings = settings + @endpoint = endpoint + end + + def parse + # return {} unless contract_defined? + + {} + end + + private + + def contract_defined? + endpoint_settings = @endpoint&.route&.app&.inheritable_setting&.namespace_stackable + binding.pry + return false unless endpoint_settings + end + end + end +end diff --git a/lib/grape-swagger/endpoint/header_params_parser.rb b/lib/grape-swagger/endpoint/header_params_parser.rb new file mode 100644 index 00000000..fde0be15 --- /dev/null +++ b/lib/grape-swagger/endpoint/header_params_parser.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module GrapeSwagger + module Endpoint + class HeaderParamsParser + 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| + params = { + documentation: { desc: definition[:description], in: 'header' }, + } + params[:type] = definition[:type].titleize if definition[:type] + + accum[name] = definition + .except(:description) + .merge(params) + end + end + end + end +end diff --git a/lib/grape-swagger/doc_methods/headers.rb b/lib/grape-swagger/endpoint/headers_parser.rb similarity index 92% rename from lib/grape-swagger/doc_methods/headers.rb rename to lib/grape-swagger/endpoint/headers_parser.rb index e84ec456..c7d05bf0 100644 --- a/lib/grape-swagger/doc_methods/headers.rb +++ b/lib/grape-swagger/endpoint/headers_parser.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true module GrapeSwagger - module DocMethods - class Headers + module Endpoint + class HeadersParser class << self def parse(route) route.headers.to_a.map do |route_header| diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index 1d038151..eae40b61 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -3,19 +3,23 @@ module GrapeSwagger module Endpoint class ParamsParser - attr_reader :params, :settings, :endpoint + attr_reader :route, :params, :settings, :endpoint - def self.parse_request_params(params, settings, endpoint) - new(params, settings, endpoint).parse_request_params + class << self + def parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse + end + + alias parse_request_params 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] @@ -29,6 +33,7 @@ def parse_request_params memo[name] = options end end + alias parse_request_params parse private @@ -48,11 +53,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/endpoint/path_params_parser.rb b/lib/grape-swagger/endpoint/path_params_parser.rb new file mode 100644 index 00000000..8337e968 --- /dev/null +++ b/lib/grape-swagger/endpoint/path_params_parser.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +module GrapeSwagger + module Endpoint + class PathParamsParser + 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 + + get_params = get_path_params(stackable_values) + path_params = build_path_params(stackable_values) + + fulfill_params(path_params) + end + + private + + def build_path_params(stackable_values) + params = {} + + loop do + break params unless stackable_values + break params unless stackable_values.is_a? Grape::Util::StackableValues + + params.merge!(fetch_inherited_params(stackable_values)) + stackable_values = stackable_values.inherited_values + end + end + + def fetch_inherited_params(stackable_values) + return {} unless stackable_values.new_values + + namespaces = stackable_values.new_values.dig(: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) + param_keys = route.params.keys + # Merge path params options into route params + route.params.each_with_object({}) do |(param, definition), accum| + value = (path_params[param] || {}).merge( + definition.is_a?(Hash) ? definition : {}, + ) + + accum[param.to_sym] = value.empty? ? DEFAULT_PARAM_TYPE : value + end + 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 || {} + end + inherited_params = get_path_params(stackable_values.inherited_values) + inherited_params.merge(params) + end + end + end +end diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index a577285f..d1b89b51 100644 --- a/spec/lib/endpoint_spec.rb +++ b/spec/lib/endpoint_spec.rb @@ -4,7 +4,7 @@ describe Grape::Endpoint do subject do - described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get) + described_class.new(nil, Grape::Util::InheritableSetting.new, path: '/', method: :get) end describe '.content_types_for' do @@ -49,7 +49,7 @@ describe 'parse_request_params' do let(:subject) { GrapeSwagger::Endpoint::ParamsParser } before do - subject.send(:parse_request_params, params, {}, nil) + subject.send(:parse, params, {}, nil) end context 'when params do not contain an array' do diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 5df6bf25..9904e932 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -19,6 +19,7 @@ Bundler.setup :default, :test +require 'pry' require 'rack' require 'rack/test' 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 06d7d5a5..5f61cab0 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 @@ -190,6 +190,7 @@ def app end specify do + puts JSON.pretty_generate(subject) expect(subject['paths']['/with_dry_schema_params']).to eq({}) end end From bf52d5c6cf3409e1f7f9f4d3bb9d1d492f504a5c Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 11:57:13 +0100 Subject: [PATCH 03/11] Cleanup mess a bit --- lib/grape-swagger/doc_methods/format_data.rb | 2 +- lib/grape-swagger/endpoint.rb | 14 ++------------ lib/grape-swagger/endpoint/params_parser.rb | 8 ++------ spec/lib/endpoint/params_parser_spec.rb | 2 +- spec/lib/endpoint_spec.rb | 4 ++-- 5 files changed, 8 insertions(+), 22 deletions(-) diff --git a/lib/grape-swagger/doc_methods/format_data.rb b/lib/grape-swagger/doc_methods/format_data.rb index dee52b41..4a6ca944 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/endpoint.rb b/lib/grape-swagger/endpoint.rb index 380619b7..53cb6187 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -393,25 +393,15 @@ def build_file_response(memo) end def build_request_params(route, settings) - # default_type(required) - # request_params.empty? ? required : request_params - - result = REQUEST_PARAM_PARSERS.each_with_object({}) do |parser_klass, accum| + REQUEST_PARAM_PARSERS.each_with_object({}) do |parser_klass, accum| params = parser_klass.parse( route, accum, settings, self ) - puts "KLASS: #{parser_klass}" - puts "ACCUM: #{accum.inspect}" - puts "PARMS: #{params.inspect}" - accum.merge!(params.symbolize_keys) + accum.merge!(params.stringify_keys) end - - puts "TOTAL PARMS: #{result.inspect}" - - result end def default_type(params) diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index eae40b61..92923004 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -5,12 +5,8 @@ module Endpoint class ParamsParser attr_reader :route, :params, :settings, :endpoint - class << self - def parse(route, params, settings, endpoint) - new(route, params, settings, endpoint).parse - end - - alias parse_request_params parse + def self.parse(route, params, settings, endpoint) + new(route, params, settings, endpoint).parse end def initialize(_route, params, settings, endpoint) diff --git a/spec/lib/endpoint/params_parser_spec.rb b/spec/lib/endpoint/params_parser_spec.rb index 347d8818..42a80007 100644 --- a/spec/lib/endpoint/params_parser_spec.rb +++ b/spec/lib/endpoint/params_parser_spec.rb @@ -7,7 +7,7 @@ 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 } diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index d1b89b51..4230fca7 100644 --- a/spec/lib/endpoint_spec.rb +++ b/spec/lib/endpoint_spec.rb @@ -4,7 +4,7 @@ describe Grape::Endpoint do subject do - described_class.new(nil, Grape::Util::InheritableSetting.new, path: '/', method: :get) + described_class.new(Grape::Util::InheritableSetting.new, path: '/', method: :get) end describe '.content_types_for' do @@ -49,7 +49,7 @@ describe 'parse_request_params' do let(:subject) { GrapeSwagger::Endpoint::ParamsParser } before do - subject.send(:parse, params, {}, nil) + subject.send(:parse, nil, params, {}, nil) end context 'when params do not contain an array' do From 567ff4cbff5e25555d1f5771fd29cf2d69a13403 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 12:21:32 +0100 Subject: [PATCH 04/11] Fix specs --- lib/grape-swagger/endpoint.rb | 2 +- .../endpoint/header_params_parser.rb | 8 ++++++-- lib/grape-swagger/endpoint/headers_parser.rb | 20 ------------------- 3 files changed, 7 insertions(+), 23 deletions(-) delete mode 100644 lib/grape-swagger/endpoint/headers_parser.rb diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 53cb6187..0f04ac76 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -10,8 +10,8 @@ module Grape class Endpoint # rubocop:disable Metrics/ClassLength REQUEST_PARAM_PARSERS = [ - GrapeSwagger::Endpoint::PathParamsParser, GrapeSwagger::Endpoint::HeaderParamsParser, + GrapeSwagger::Endpoint::PathParamsParser, GrapeSwagger::Endpoint::ContractParser, GrapeSwagger::Endpoint::ParamsParser, ].freeze diff --git a/lib/grape-swagger/endpoint/header_params_parser.rb b/lib/grape-swagger/endpoint/header_params_parser.rb index fde0be15..0c3d025c 100644 --- a/lib/grape-swagger/endpoint/header_params_parser.rb +++ b/lib/grape-swagger/endpoint/header_params_parser.rb @@ -18,12 +18,16 @@ def parse route.headers.each_with_object({}) do |(name, definition), accum| params = { - documentation: { desc: definition[:description], in: 'header' }, + documentation: { + desc: definition["description"] || definition[:description], + in: 'header' + }, } params[:type] = definition[:type].titleize if definition[:type] accum[name] = definition - .except(:description) + .symbolize_keys + .except(:description, "description") .merge(params) end end diff --git a/lib/grape-swagger/endpoint/headers_parser.rb b/lib/grape-swagger/endpoint/headers_parser.rb deleted file mode 100644 index c7d05bf0..00000000 --- a/lib/grape-swagger/endpoint/headers_parser.rb +++ /dev/null @@ -1,20 +0,0 @@ -# frozen_string_literal: true - -module GrapeSwagger - module Endpoint - class HeadersParser - 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 From c84ceb040a7b206c6190e94cc2437b29c72ef125 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 12:21:53 +0100 Subject: [PATCH 05/11] Make it easy to find a diff --- Gemfile | 1 + spec/spec_helper.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/Gemfile b/Gemfile index 7ca79e45..55430847 100644 --- a/Gemfile +++ b/Gemfile @@ -20,6 +20,7 @@ group :development, :test do gem 'pry', platforms: [:mri] gem 'pry-byebug', platforms: [:mri] gem 'dry-schema' + 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/spec/spec_helper.rb b/spec/spec_helper.rb index 9904e932..cec90907 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -22,6 +22,7 @@ require 'pry' require 'rack' require 'rack/test' +require 'super_diff/rspec' if ENV.key?('SUPER_DIFF') RSpec.configure do |config| require 'rspec/expectations' From 5e56932fea3bfda21a792cd6287d093dc814b8bf Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 12:23:08 +0100 Subject: [PATCH 06/11] Let rubocop be happy --- Gemfile | 4 ++-- .../doc_methods/build_model_definition.rb | 6 +++--- lib/grape-swagger/doc_methods/move_params.rb | 4 ++-- lib/grape-swagger/endpoint.rb | 8 ++++---- lib/grape-swagger/endpoint/contract_parser.rb | 2 +- lib/grape-swagger/endpoint/header_params_parser.rb | 10 +++++----- lib/grape-swagger/endpoint/params_parser.rb | 2 +- lib/grape-swagger/endpoint/path_params_parser.rb | 9 ++++----- spec/spec_helper.rb | 3 +-- .../support/model_parsers/custom_parsed_type_parser.rb | 6 +++--- 10 files changed, 26 insertions(+), 28 deletions(-) diff --git a/Gemfile b/Gemfile index 55430847..a3d363fd 100644 --- a/Gemfile +++ b/Gemfile @@ -16,11 +16,11 @@ gem ENV.fetch('MODEL_PARSER', nil) if ENV.key?('MODEL_PARSER') group :development, :test do gem 'bundler' + gem 'dry-schema' gem 'grape-entity' gem 'pry', platforms: [:mri] gem 'pry-byebug', platforms: [:mri] - gem 'dry-schema' - gem "super_diff", require: false + 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/doc_methods/build_model_definition.rb b/lib/grape-swagger/doc_methods/build_model_definition.rb index 8cddbaa2..f017b708 100644 --- a/lib/grape-swagger/doc_methods/build_model_definition.rb +++ b/lib/grape-swagger/doc_methods/build_model_definition.rb @@ -4,9 +4,9 @@ module GrapeSwagger module DocMethods class BuildModelDefinition class << self - OBJECT_ATTRIBUTE_KEYS = [ - :$ref, :type, - ].freeze + OBJECT_ATTRIBUTE_KEYS = %i[ + $ref type + ].freeze def build(_model, properties, required, other_def_properties = {}) definition = { type: 'object', properties: properties }.merge(other_def_properties) diff --git a/lib/grape-swagger/doc_methods/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index ca0cf56a..70ac8201 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -163,9 +163,9 @@ def object_type end def prepare_nested_names(property, params) - params.each { |x| + params.each do |x| x[:name] = x[:name].to_s.sub(property.to_s, '').sub('[', '').sub(']', '') - } + end end def unify!(params) diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 0f04ac76..86ff6930 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -10,10 +10,10 @@ module Grape class Endpoint # rubocop:disable Metrics/ClassLength REQUEST_PARAM_PARSERS = [ - GrapeSwagger::Endpoint::HeaderParamsParser, - GrapeSwagger::Endpoint::PathParamsParser, - GrapeSwagger::Endpoint::ContractParser, - GrapeSwagger::Endpoint::ParamsParser, + GrapeSwagger::Endpoint::HeaderParamsParser, + GrapeSwagger::Endpoint::PathParamsParser, + GrapeSwagger::Endpoint::ContractParser, + GrapeSwagger::Endpoint::ParamsParser ].freeze def content_types_for(target_class) diff --git a/lib/grape-swagger/endpoint/contract_parser.rb b/lib/grape-swagger/endpoint/contract_parser.rb index f4f31c3d..1e7b5117 100644 --- a/lib/grape-swagger/endpoint/contract_parser.rb +++ b/lib/grape-swagger/endpoint/contract_parser.rb @@ -31,7 +31,7 @@ def parse def contract_defined? endpoint_settings = @endpoint&.route&.app&.inheritable_setting&.namespace_stackable binding.pry - return false unless endpoint_settings + false unless endpoint_settings end end end diff --git a/lib/grape-swagger/endpoint/header_params_parser.rb b/lib/grape-swagger/endpoint/header_params_parser.rb index 0c3d025c..e34b1a5a 100644 --- a/lib/grape-swagger/endpoint/header_params_parser.rb +++ b/lib/grape-swagger/endpoint/header_params_parser.rb @@ -19,16 +19,16 @@ def parse route.headers.each_with_object({}) do |(name, definition), accum| params = { documentation: { - desc: definition["description"] || definition[:description], + desc: definition['description'] || definition[:description], in: 'header' - }, + } } params[:type] = definition[:type].titleize if definition[:type] accum[name] = definition - .symbolize_keys - .except(:description, "description") - .merge(params) + .symbolize_keys + .except(:description, 'description') + .merge(params) end end end diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/endpoint/params_parser.rb index 92923004..f1a54cef 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/endpoint/params_parser.rb @@ -49,7 +49,7 @@ def param_type_is_array?(param_type) end def public_params - params.select { |key, param| public_parameter?(param) } + params.select { |_key, param| public_parameter?(param) } end def public_parameter?(param_options) diff --git a/lib/grape-swagger/endpoint/path_params_parser.rb b/lib/grape-swagger/endpoint/path_params_parser.rb index 8337e968..1e0e733d 100644 --- a/lib/grape-swagger/endpoint/path_params_parser.rb +++ b/lib/grape-swagger/endpoint/path_params_parser.rb @@ -18,7 +18,7 @@ def initialize(route, _params, _settings, _endpoint) def parse stackable_values = route.app&.inheritable_setting&.namespace_stackable - get_params = get_path_params(stackable_values) + get_path_params(stackable_values) path_params = build_path_params(stackable_values) fulfill_params(path_params) @@ -41,7 +41,7 @@ def build_path_params(stackable_values) def fetch_inherited_params(stackable_values) return {} unless stackable_values.new_values - namespaces = stackable_values.new_values.dig(:namespace) || [] + namespaces = stackable_values.new_values[:namespace] || [] namespaces.each_with_object({}) do |namespace, params| space = namespace.space.to_s.gsub(':', '') @@ -50,18 +50,17 @@ def fetch_inherited_params(stackable_values) end def fulfill_params(path_params) - param_keys = route.params.keys + route.params.keys # Merge path params options into route params route.params.each_with_object({}) do |(param, definition), accum| value = (path_params[param] || {}).merge( - definition.is_a?(Hash) ? definition : {}, + definition.is_a?(Hash) ? definition : {} ) accum[param.to_sym] = value.empty? ? DEFAULT_PARAM_TYPE : value end end - # Iterates over namespaces recursively # to build a hash of path params with options, including type def get_path_params(stackable_values) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index cec90907..0c86e826 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,8 +11,7 @@ Dir[File.join(Dir.getwd, 'spec/support/*.rb')].each { |f| require f } require "grape-swagger/#{MODEL_PARSER}" if MODEL_PARSER != 'mock' require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_parser.rb") -require_relative './support/model_parsers/custom_parsed_type_parser' - +require_relative 'support/model_parsers/custom_parsed_type_parser' require 'grape-entity' require 'grape-swagger-entity' diff --git a/spec/support/model_parsers/custom_parsed_type_parser.rb b/spec/support/model_parsers/custom_parsed_type_parser.rb index 5c8dae52..ffe39540 100644 --- a/spec/support/model_parsers/custom_parsed_type_parser.rb +++ b/spec/support/model_parsers/custom_parsed_type_parser.rb @@ -12,15 +12,15 @@ def initialize(model, endpoint) def call { - type: "object", + type: 'object', properties: { custom: { type: 'boolean', description: "it's a custom type", - default: true, + default: true } }, - required: [], + required: [] } end end From 44d178132a43a2ae1c982a932a2f15f3773f19e7 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Thu, 13 Feb 2025 13:19:56 +0100 Subject: [PATCH 07/11] Fixes after rubocop autocorrection --- lib/grape-swagger.rb | 4 ++-- lib/grape-swagger/doc_methods/move_params.rb | 4 +--- lib/grape-swagger/endpoint.rb | 4 +--- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 7a36ea0c..575582e9 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -21,11 +21,11 @@ def model_parsers # 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 }.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/move_params.rb b/lib/grape-swagger/doc_methods/move_params.rb index 70ac8201..c8417ce5 100644 --- a/lib/grape-swagger/doc_methods/move_params.rb +++ b/lib/grape-swagger/doc_methods/move_params.rb @@ -163,9 +163,7 @@ def object_type end def prepare_nested_names(property, params) - params.each do |x| - x[:name] = x[:name].to_s.sub(property.to_s, '').sub('[', '').sub(']', '') - end + 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 86ff6930..59a81931 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -22,9 +22,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 From 286885ebdef3c830c600045e7846c24a38c4ba64 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Mon, 17 Feb 2025 15:28:03 +0100 Subject: [PATCH 08/11] Cleanup a little --- Gemfile | 1 - lib/grape-swagger.rb | 2 +- .../doc_methods/build_model_definition.rb | 22 +---------- lib/grape-swagger/endpoint.rb | 14 +++---- lib/grape-swagger/endpoint/contract_parser.rb | 38 ------------------- .../body.rb} | 5 +-- .../headers.rb} | 4 +- .../route.rb} | 11 +++--- spec/lib/endpoint_spec.rb | 4 +- .../body_spec.rb} | 6 +-- spec/spec_helper.rb | 2 - .../custom_parsed_type_parser.rb | 28 -------------- .../api_swagger_v2_param_type_body_spec.rb | 27 +------------ 13 files changed, 23 insertions(+), 141 deletions(-) delete mode 100644 lib/grape-swagger/endpoint/contract_parser.rb rename lib/grape-swagger/{endpoint/params_parser.rb => request_param_parsers/body.rb} (96%) rename lib/grape-swagger/{endpoint/header_params_parser.rb => request_param_parsers/headers.rb} (94%) rename lib/grape-swagger/{endpoint/path_params_parser.rb => request_param_parsers/route.rb} (92%) rename spec/lib/{endpoint/params_parser_spec.rb => request_param_parsers/body_spec.rb} (95%) delete mode 100644 spec/support/model_parsers/custom_parsed_type_parser.rb diff --git a/Gemfile b/Gemfile index a3d363fd..3a404967 100644 --- a/Gemfile +++ b/Gemfile @@ -16,7 +16,6 @@ gem ENV.fetch('MODEL_PARSER', nil) if ENV.key?('MODEL_PARSER') group :development, :test do gem 'bundler' - gem 'dry-schema' gem 'grape-entity' gem 'pry', platforms: [:mri] gem 'pry-byebug', platforms: [:mri] diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index 575582e9..bc99a50a 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -25,7 +25,7 @@ def model_parsers serializable_hash: Grape::Formatter::SerializableHash, json: Grape::Formatter::Json, jsonapi: Grape::Formatter::Json, - txt: Grape::Formatter::Txt, + 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/build_model_definition.rb b/lib/grape-swagger/doc_methods/build_model_definition.rb index f017b708..444b17f8 100644 --- a/lib/grape-swagger/doc_methods/build_model_definition.rb +++ b/lib/grape-swagger/doc_methods/build_model_definition.rb @@ -4,10 +4,6 @@ module GrapeSwagger module DocMethods class BuildModelDefinition class << self - OBJECT_ATTRIBUTE_KEYS = %i[ - $ref type - ].freeze - def build(_model, properties, required, other_def_properties = {}) definition = { type: 'object', properties: properties }.merge(other_def_properties) @@ -17,10 +13,6 @@ def build(_model, properties, required, other_def_properties = {}) end def parse_params_from_model(parsed_response, model, model_name) - # If the parsed response looks like a complete object (e.g., containing `$ref` or `type`), - # it uses the provided response as-is. - return parsed_response if complete_object?(parsed_response) - if parsed_response.is_a?(Hash) && parsed_response.keys.first == :allOf refs_or_models = parsed_response[:allOf] parsed = parse_refs_and_models(refs_or_models, model) @@ -62,7 +54,7 @@ def parse_properties(properties) def parse_refs_and_models(refs_or_models, model) refs_or_models.map do |ref_or_models| - if complete_object?(ref_or_models) + if ref_or_models.is_a?(Hash) && ref_or_models.keys.first == '$ref' ref_or_models else properties, required = ref_or_models @@ -70,18 +62,6 @@ def parse_refs_and_models(refs_or_models, model) end end end - - private - - # Checks if the parsed response is already a complete object. - # - # @param parsed_response [Hash] The parsed response to check. - # @return [Boolean] True if the response has object attributes. - def complete_object?(parsed_response) - return false unless parsed_response.is_a?(Hash) - - parsed_response.keys.intersect?(OBJECT_ATTRIBUTE_KEYS) - end end end end diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index 59a81931..ff69abb4 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -2,18 +2,16 @@ require 'active_support' require 'active_support/core_ext/string/inflections' -require 'grape-swagger/endpoint/params_parser' -require 'grape-swagger/endpoint/path_params_parser' -require 'grape-swagger/endpoint/contract_parser' -require 'grape-swagger/endpoint/header_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 REQUEST_PARAM_PARSERS = [ - GrapeSwagger::Endpoint::HeaderParamsParser, - GrapeSwagger::Endpoint::PathParamsParser, - GrapeSwagger::Endpoint::ContractParser, - GrapeSwagger::Endpoint::ParamsParser + GrapeSwagger::RequestParamParsers::Headers, + GrapeSwagger::RequestParamParsers::Route, + GrapeSwagger::RequestParamParsers::Body ].freeze def content_types_for(target_class) diff --git a/lib/grape-swagger/endpoint/contract_parser.rb b/lib/grape-swagger/endpoint/contract_parser.rb deleted file mode 100644 index 1e7b5117..00000000 --- a/lib/grape-swagger/endpoint/contract_parser.rb +++ /dev/null @@ -1,38 +0,0 @@ -# frozen_string_literal: true - -module GrapeSwagger - module Endpoint - class ContractParser - attr_reader :route, :params, :settings, :endpoint - - class << self - def parse(route, params, settings, endpoint) - new(route, params, settings, endpoint).parse - end - - alias parse_request_params parse - end - - def initialize(route, params, settings, endpoint) - @route = route - @params = params - @settings = settings - @endpoint = endpoint - end - - def parse - # return {} unless contract_defined? - - {} - end - - private - - def contract_defined? - endpoint_settings = @endpoint&.route&.app&.inheritable_setting&.namespace_stackable - binding.pry - false unless endpoint_settings - end - end - end -end diff --git a/lib/grape-swagger/endpoint/params_parser.rb b/lib/grape-swagger/request_param_parsers/body.rb similarity index 96% rename from lib/grape-swagger/endpoint/params_parser.rb rename to lib/grape-swagger/request_param_parsers/body.rb index f1a54cef..21d96096 100644 --- a/lib/grape-swagger/endpoint/params_parser.rb +++ b/lib/grape-swagger/request_param_parsers/body.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true module GrapeSwagger - module Endpoint - class ParamsParser + module RequestParamParsers + class Body attr_reader :route, :params, :settings, :endpoint def self.parse(route, params, settings, endpoint) @@ -29,7 +29,6 @@ def parse memo[name] = options end end - alias parse_request_params parse private diff --git a/lib/grape-swagger/endpoint/header_params_parser.rb b/lib/grape-swagger/request_param_parsers/headers.rb similarity index 94% rename from lib/grape-swagger/endpoint/header_params_parser.rb rename to lib/grape-swagger/request_param_parsers/headers.rb index e34b1a5a..65153e1e 100644 --- a/lib/grape-swagger/endpoint/header_params_parser.rb +++ b/lib/grape-swagger/request_param_parsers/headers.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true module GrapeSwagger - module Endpoint - class HeaderParamsParser + module RequestParamParsers + class Headers attr_reader :route def self.parse(route, params, settings, endpoint) diff --git a/lib/grape-swagger/endpoint/path_params_parser.rb b/lib/grape-swagger/request_param_parsers/route.rb similarity index 92% rename from lib/grape-swagger/endpoint/path_params_parser.rb rename to lib/grape-swagger/request_param_parsers/route.rb index 1e0e733d..e3b028db 100644 --- a/lib/grape-swagger/endpoint/path_params_parser.rb +++ b/lib/grape-swagger/request_param_parsers/route.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true module GrapeSwagger - module Endpoint - class PathParamsParser + module RequestParamParsers + class Route DEFAULT_PARAM_TYPE = { required: true, type: 'Integer' }.freeze attr_reader :route @@ -29,13 +29,12 @@ def parse def build_path_params(stackable_values) params = {} - loop do - break params unless stackable_values - break params unless stackable_values.is_a? Grape::Util::StackableValues - + 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) diff --git a/spec/lib/endpoint_spec.rb b/spec/lib/endpoint_spec.rb index 4230fca7..ba672448 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, nil, 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 95% rename from spec/lib/endpoint/params_parser_spec.rb rename to spec/lib/request_param_parsers/body_spec.rb index 42a80007..be1548d4 100644 --- a/spec/lib/endpoint/params_parser_spec.rb +++ b/spec/lib/request_param_parsers/body_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe GrapeSwagger::Endpoint::ParamsParser do +describe GrapeSwagger::RequestParamParsers::Body do let(:settings) { {} } let(:params) { [] } let(:endpoint) { nil } @@ -10,7 +10,7 @@ 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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0c86e826..2a27c8e1 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -11,14 +11,12 @@ Dir[File.join(Dir.getwd, 'spec/support/*.rb')].each { |f| require f } require "grape-swagger/#{MODEL_PARSER}" if MODEL_PARSER != 'mock' require File.join(Dir.getwd, "spec/support/model_parsers/#{MODEL_PARSER}_parser.rb") -require_relative 'support/model_parsers/custom_parsed_type_parser' require 'grape-entity' require 'grape-swagger-entity' Bundler.setup :default, :test -require 'pry' require 'rack' require 'rack/test' require 'super_diff/rspec' if ENV.key?('SUPER_DIFF') diff --git a/spec/support/model_parsers/custom_parsed_type_parser.rb b/spec/support/model_parsers/custom_parsed_type_parser.rb deleted file mode 100644 index ffe39540..00000000 --- a/spec/support/model_parsers/custom_parsed_type_parser.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -CustomParsedType = Class.new - -class CustomParsedTypeParser - attr_reader :model, :endpoint - - def initialize(model, endpoint) - @model = model - @endpoint = endpoint - end - - def call - { - type: 'object', - properties: { - custom: { - type: 'boolean', - description: "it's a custom type", - default: true - } - }, - required: [] - } - end -end - -GrapeSwagger.model_parsers.register(CustomParsedTypeParser, CustomParsedType) 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 5f61cab0..cf394f99 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 @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'spec_helper' -require 'dry-schema' describe 'setting of param type, such as `query`, `path`, `formData`, `body`, `header`' do include_context "#{MODEL_PARSER} swagger example" @@ -68,18 +67,6 @@ class BodyParamTypeApi < Grape::API end end - namespace :with_dry_schema_params do - contract do - required(:orders).array(:hash) do - required(:name).filled(:string) - optional(:volume).maybe(:integer, lt?: 9) - end - end - post do - { 'declared_params' => declared(params) } - end - end - add_swagger_documentation end end @@ -91,7 +78,7 @@ def app describe 'no entity given' do subject do - get '/swagger_doc/' + get '/swagger_doc/wo_entities' JSON.parse(last_response.body) end @@ -183,18 +170,6 @@ def app end end - describe 'with exceed type parser result' do - subject do - get '/swagger_doc/with_dry_schema_params' - JSON.parse(last_response.body) - end - - specify do - puts JSON.pretty_generate(subject) - expect(subject['paths']['/with_dry_schema_params']).to eq({}) - end - end - describe 'complex entity given' do let(:request_parameters_definition) do [ From 13d6065750ba792989f3e7767bb5603a62bf8e87 Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Tue, 18 Feb 2025 16:40:20 +0100 Subject: [PATCH 09/11] Fix spec --- lib/grape-swagger/request_param_parsers/route.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/grape-swagger/request_param_parsers/route.rb b/lib/grape-swagger/request_param_parsers/route.rb index e3b028db..0af25608 100644 --- a/lib/grape-swagger/request_param_parsers/route.rb +++ b/lib/grape-swagger/request_param_parsers/route.rb @@ -49,9 +49,13 @@ def fetch_inherited_params(stackable_values) end def fulfill_params(path_params) - route.params.keys # 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. + next if param.is_a?(String) && accum.key?(param.to_sym) + value = (path_params[param] || {}).merge( definition.is_a?(Hash) ? definition : {} ) From 2bf1299619746d351c6a44a06da52d9070ed9acf Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Sat, 22 Feb 2025 21:54:34 +0100 Subject: [PATCH 10/11] Add requst param parser regestry --- lib/grape-swagger.rb | 5 +++ lib/grape-swagger/endpoint.rb | 8 +--- .../request_param_parser_registry.rb | 43 +++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 lib/grape-swagger/request_param_parser_registry.rb diff --git a/lib/grape-swagger.rb b/lib/grape-swagger.rb index bc99a50a..8eb42934 100644 --- a/lib/grape-swagger.rb +++ b/lib/grape-swagger.rb @@ -10,12 +10,17 @@ 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' diff --git a/lib/grape-swagger/endpoint.rb b/lib/grape-swagger/endpoint.rb index ff69abb4..54dd8191 100644 --- a/lib/grape-swagger/endpoint.rb +++ b/lib/grape-swagger/endpoint.rb @@ -8,12 +8,6 @@ module Grape class Endpoint # rubocop:disable Metrics/ClassLength - REQUEST_PARAM_PARSERS = [ - GrapeSwagger::RequestParamParsers::Headers, - GrapeSwagger::RequestParamParsers::Route, - GrapeSwagger::RequestParamParsers::Body - ].freeze - def content_types_for(target_class) content_types = (target_class.content_types || {}).values @@ -389,7 +383,7 @@ def build_file_response(memo) end def build_request_params(route, settings) - REQUEST_PARAM_PARSERS.each_with_object({}) do |parser_klass, accum| + GrapeSwagger.request_param_parsers.each_with_object({}) do |parser_klass, accum| params = parser_klass.parse( route, accum, 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 00000000..e6cd0176 --- /dev/null +++ b/lib/grape-swagger/request_param_parser_registry.rb @@ -0,0 +1,43 @@ +# 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) + @parsers << klass + end + + def insert_before(before_klass, klass) + insert_at = @parsers.index(before_klass) + insert_at = @parsers.length - 1 if insert_at.nil? + @parsers.insert(insert_at, klass) + end + + def insert_after(after_klass, klass) + insert_at = @parsers.index(after_klass) + insert_at = @parsers.length - 1 if insert_at.nil? + @parsers.insert(insert_at + 1, klass) + end + + def each + @parsers.each do |klass| + yield klass + end + end + end +end From bffbbd40b4178c04e02376fd82530833331da49f Mon Sep 17 00:00:00 2001 From: Andrey Subbota Date: Sat, 22 Feb 2025 22:20:17 +0100 Subject: [PATCH 11/11] Refactor param parsers --- .../request_param_parser_registry.rb | 21 +++++++++------ .../request_param_parsers/body.rb | 9 +++---- .../request_param_parsers/headers.rb | 19 ++++++------- .../request_param_parsers/route.rb | 27 ++++--------------- spec/lib/request_param_parsers/body_spec.rb | 12 ++++----- 5 files changed, 36 insertions(+), 52 deletions(-) diff --git a/lib/grape-swagger/request_param_parser_registry.rb b/lib/grape-swagger/request_param_parser_registry.rb index e6cd0176..17d29201 100644 --- a/lib/grape-swagger/request_param_parser_registry.rb +++ b/lib/grape-swagger/request_param_parser_registry.rb @@ -19,25 +19,30 @@ def initialize end def register(klass) + remove_parser(klass) @parsers << klass end def insert_before(before_klass, klass) - insert_at = @parsers.index(before_klass) - insert_at = @parsers.length - 1 if insert_at.nil? + 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) - insert_at = @parsers.length - 1 if insert_at.nil? - @parsers.insert(insert_at + 1, klass) + @parsers.insert(insert_at ? insert_at + 1 : @parsers.size, klass) end - def each - @parsers.each do |klass| - yield 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/request_param_parsers/body.rb b/lib/grape-swagger/request_param_parsers/body.rb index 21d96096..b8814396 100644 --- a/lib/grape-swagger/request_param_parsers/body.rb +++ b/lib/grape-swagger/request_param_parsers/body.rb @@ -18,10 +18,9 @@ def initialize(_route, params, settings, endpoint) 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(/\[(.*)\]$/) diff --git a/lib/grape-swagger/request_param_parsers/headers.rb b/lib/grape-swagger/request_param_parsers/headers.rb index 65153e1e..6115a478 100644 --- a/lib/grape-swagger/request_param_parsers/headers.rb +++ b/lib/grape-swagger/request_param_parsers/headers.rb @@ -17,18 +17,15 @@ def parse return {} unless route.headers route.headers.each_with_object({}) do |(name, definition), accum| - params = { - documentation: { - desc: definition['description'] || definition[:description], - in: 'header' - } - } - params[:type] = definition[:type].titleize if definition[:type] + # Extract the description from any key type (string or symbol) + description = definition[:description] || definition['description'] + doc = { desc: description, in: 'header' } - accum[name] = definition - .symbolize_keys - .except(:description, 'description') - .merge(params) + 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 diff --git a/lib/grape-swagger/request_param_parsers/route.rb b/lib/grape-swagger/request_param_parsers/route.rb index 0af25608..d3d27d20 100644 --- a/lib/grape-swagger/request_param_parsers/route.rb +++ b/lib/grape-swagger/request_param_parsers/route.rb @@ -18,7 +18,6 @@ def initialize(route, _params, _settings, _endpoint) def parse stackable_values = route.app&.inheritable_setting&.namespace_stackable - get_path_params(stackable_values) path_params = build_path_params(stackable_values) fulfill_params(path_params) @@ -54,29 +53,13 @@ def fulfill_params(path_params) # 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. - next if param.is_a?(String) && accum.key?(param.to_sym) + key = param.is_a?(String) ? param.to_sym : param + next if param.is_a?(String) && accum.key?(key) - value = (path_params[param] || {}).merge( - definition.is_a?(Hash) ? definition : {} - ) - - accum[param.to_sym] = value.empty? ? DEFAULT_PARAM_TYPE : value - end - 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 || {} + defined_options = definition.is_a?(Hash) ? definition : {} + value = (path_params[param] || {}).merge(defined_options) + accum[key] = value.empty? ? DEFAULT_PARAM_TYPE : value end - inherited_params = get_path_params(stackable_values.inherited_values) - inherited_params.merge(params) end end end diff --git a/spec/lib/request_param_parsers/body_spec.rb b/spec/lib/request_param_parsers/body_spec.rb index be1548d4..79add0a8 100644 --- a/spec/lib/request_param_parsers/body_spec.rb +++ b/spec/lib/request_param_parsers/body_spec.rb @@ -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