From 8110e4a0e31f0631013904feca62ee6a427c753a Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Wed, 20 Aug 2025 22:50:28 +0200 Subject: [PATCH 1/2] Add spec for issue: polymorphic entity with custom documentation - Reproduces "Empty model" error when using Grape::Entity with all hidden properties - Tests custom documentation override with Array[Object] type - Includes working example showing expected behavior - Follows existing issue spec patterns in the codebase --- .rspec | 1 + ...c_entity_with_custom_documentation_spec.rb | 122 ++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100644 spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb diff --git a/.rspec b/.rspec index 8c18f1a..740e6ee 100644 --- a/.rspec +++ b/.rspec @@ -1,2 +1,3 @@ --format documentation --color +--require "spec_helper" \ No newline at end of file diff --git a/spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb b/spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb new file mode 100644 index 0000000..61a7cee --- /dev/null +++ b/spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb @@ -0,0 +1,122 @@ +# frozen_string_literal: true + +describe '#962 empty entity with custom documentation type' do + context "when entity has no properties" do + let(:app) do + Class.new(Grape::API) do + namespace :issue962 do + class Foo < Grape::Entity + end + + class Report < Grape::Entity + expose :foo, + as: :bar, + using: Foo, + documentation: { + type: 'Array[object]', + desc: 'The bar in your report', + example: { + 'id' => 'string', + 'status' => 'string', + } + } + end + + desc 'Get a report', success: Report + get '/' do + present({ foo: [] }, with: Report) + end + end + + add_swagger_documentation format: :json + end + end + + subject(:swagger_doc) do + get '/swagger_doc' + JSON.parse(last_response.body) + end + + specify do + expect(swagger_doc['definitions']['Report']['properties']['bar']).to eql({ + 'type' => 'array', + 'description' => 'The bar in your report', + 'items' => { + '$ref' => '#/definitions/Foo' + }, + 'example' => { + 'id' => 'string', + 'status' => 'string' + } + }) + end + + specify do + expect(swagger_doc['definitions']['Foo']).to eql({ + 'type' => 'object', + 'properties' => {}, + }) + end + end + + context "when entity has only hidden properties" do + let(:app) do + Class.new(Grape::API) do + namespace :issue962 do + class Foo < Grape::Entity + expose :required_prop, documentation: { hidden: true } + expose :optional_prop, documentation: { hidden: true }, if: ->() { true } + end + + class Report < Grape::Entity + expose :foo, + as: :bar, + using: Foo, + documentation: { + type: 'Array[object]', + desc: 'The bar in your report', + example: { + 'id' => 'string', + 'status' => 'string', + } + } + end + + desc 'Get a report', success: Report + get '/' do + present({ foo: [] }, with: Report) + end + end + + add_swagger_documentation format: :json + end + end + + subject(:swagger_doc) do + get '/swagger_doc' + JSON.parse(last_response.body) + end + + specify do + expect(swagger_doc['definitions']['Report']['properties']['bar']).to eql({ + 'type' => 'array', + 'description' => 'The bar in your report', + 'items' => { + '$ref' => '#/definitions/Foo' + }, + 'example' => { + 'id' => 'string', + 'status' => 'string' + } + }) + end + + it "hides optional properties only" do + expect(swagger_doc['definitions']['Foo']).to eql({ + 'type' => 'object', + 'properties' => {}, + 'required' => ['required_prop'], + }) + end + end +end From a343211d0f3062bcf5650630f9cc55e2a72c36d1 Mon Sep 17 00:00:00 2001 From: Andrei Subbota Date: Sun, 24 Aug 2025 15:34:26 +0200 Subject: [PATCH 2/2] Refactor entity parser and fix array documentation bug This commit addresses a long-standing issue where array types in documentation were not correctly parsed unless the explicit `is_array` option was used. The primary changes are: - A new `array_type?` method is introduced in `attribute_parser.rb` to intelligently check for `is_array`, `type: 'array'`, or `type: 'Array[Object]'`. - The `parse_grape_entity_params` method in `parser.rb` is refactored to improve readability and reduce complexity --- .rubocop_todo.yml | 2 +- lib/grape-swagger/entity/attribute_parser.rb | 18 +++-- lib/grape-swagger/entity/parser.rb | 68 ++++++++++++------- .../entities/response_model_spec.rb | 2 + 4 files changed, 61 insertions(+), 29 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 78d143c..496a371 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -37,7 +37,7 @@ Metrics/AbcSize: # Offense count: 2 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: - Max: 117 + Max: 132 # Offense count: 2 # Configuration parameters: AllowedMethods, AllowedPatterns. diff --git a/lib/grape-swagger/entity/attribute_parser.rb b/lib/grape-swagger/entity/attribute_parser.rb index 9da9e3c..f9f654f 100644 --- a/lib/grape-swagger/entity/attribute_parser.rb +++ b/lib/grape-swagger/entity/attribute_parser.rb @@ -20,7 +20,7 @@ def call(entity_options) documentation = entity_options[:documentation] return param if documentation.nil? - add_array_documentation(param, documentation) if documentation[:is_array] + add_array_documentation(param, documentation) if array_type?(documentation) add_attribute_sample(param, documentation, :default) add_attribute_sample(param, documentation, :example) @@ -37,11 +37,19 @@ def call(entity_options) def model_from(entity_options) model = entity_options[:using] if entity_options[:using].present? - model ||= entity_options[:documentation][:type] if could_it_be_a_model?(entity_options[:documentation]) + documentation = entity_options[:documentation] + model ||= documentation[:type] if could_it_be_a_model?(documentation) model end + def array_type?(documentation) + return documentation[:is_array] if documentation.key?(:is_array) + return true if documentation[:type].to_s.downcase == 'array' + + documentation[:type].to_s.match?(/\Aarray\[(?.+)\]\z/i) + end + def could_it_be_a_model?(value) return false if value.nil? @@ -57,6 +65,8 @@ def ambiguous_model_type?(type) end def primitive_type?(type) + return false if type.nil? + data_type = GrapeSwagger::DocMethods::DataType.call(type) GrapeSwagger::DocMethods::DataType.request_primitive?(data_type) end @@ -69,7 +79,7 @@ def data_type_from(entity_options) documented_data_type = document_data_type(documentation, data_type) - if documentation[:is_array] + if array_type?(documentation) { type: :array, items: documented_data_type @@ -97,7 +107,7 @@ def document_data_type(documentation, data_type) end def entity_model_type(name, entity_options) - if entity_options[:documentation] && entity_options[:documentation][:is_array] + if array_type?(entity_options[:documentation]) { 'type' => 'array', 'items' => { diff --git a/lib/grape-swagger/entity/parser.rb b/lib/grape-swagger/entity/parser.rb index 34c0b56..c821922 100644 --- a/lib/grape-swagger/entity/parser.rb +++ b/lib/grape-swagger/entity/parser.rb @@ -43,37 +43,57 @@ def extract_params(exposure) def parse_grape_entity_params(params, parent_model = nil) return unless params - parsed = params.each_with_object({}) do |(entity_name, entity_options), memo| - documentation_options = entity_options.fetch(:documentation, {}) - in_option = documentation_options.fetch(:in, nil).to_s - hidden_option = documentation_options.fetch(:hidden, nil) - next if in_option == 'header' || hidden_option == true - - entity_name = entity_name.original if entity_name.is_a?(Alias) - final_entity_name = entity_options.fetch(:as, entity_name) - documentation = entity_options[:documentation] - - memo[final_entity_name] = if entity_options[:nesting] - parse_nested(entity_name, entity_options, parent_model) - else - attribute_parser.call(entity_options) - end - - next unless documentation - - memo[final_entity_name][:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only] - memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc] + required = required_params(params) + parsed_params = parse_params(params, parent_model) + + handle_discriminator(parsed_params, required) + end + + def parse_params(params, parent_model) + params.each_with_object({}) do |(entity_name, entity_options), memo| + next if skip_param?(entity_options) + + original_entity_name = entity_name.is_a?(Alias) ? entity_name.original : entity_name + final_entity_name = entity_options.fetch(:as, original_entity_name) + + memo[final_entity_name] = parse_entity_options(entity_options, original_entity_name, parent_model) + add_documentation_to_memo(memo[final_entity_name], entity_options[:documentation]) + end + end + + def skip_param?(entity_options) + documentation_options = entity_options.fetch(:documentation, {}) + in_option = documentation_options.fetch(:in, nil).to_s + hidden_option = documentation_options.fetch(:hidden, nil) + + in_option == 'header' || hidden_option == true + end + + def parse_entity_options(entity_options, entity_name, parent_model) + if entity_options[:nesting] + parse_nested(entity_name, entity_options, parent_model) + else + attribute_parser.call(entity_options) end + end + + def add_documentation_to_memo(memo_entry, documentation) + return unless documentation + + memo_entry[:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only] + memo_entry[:description] = documentation[:desc] if documentation[:desc] + end + def handle_discriminator(parsed, required) discriminator = GrapeSwagger::Entity::Helper.discriminator(model) if discriminator - respond_with_all_of(parsed, params, discriminator) + respond_with_all_of(parsed, required, discriminator) else - [parsed, required_params(params)] + [parsed, required] end end - def respond_with_all_of(parsed, params, discriminator) + def respond_with_all_of(parsed, required, discriminator) parent_name = GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint) { @@ -83,7 +103,7 @@ def respond_with_all_of(parsed, params, discriminator) }, [ add_discriminator(parsed, discriminator), - required_params(params).push(discriminator.attribute) + required.push(discriminator.attribute) ] ] } diff --git a/spec/grape-swagger/entities/response_model_spec.rb b/spec/grape-swagger/entities/response_model_spec.rb index 2709a26..00040ef 100644 --- a/spec/grape-swagger/entities/response_model_spec.rb +++ b/spec/grape-swagger/entities/response_model_spec.rb @@ -103,6 +103,8 @@ def app JSON.parse(last_response.body)['definitions'] end + include_context 'this api' + before :all do module TheseApi module Entities