Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .rspec
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
--format documentation
--color
--require "spec_helper"
16 changes: 12 additions & 4 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@
# Note that changes in the inspected code, or installation of new
# versions of RuboCop, may require this file to be generated again.

# Offense count: 3
# Offense count: 7
# Configuration parameters: AllowedMethods.
# AllowedMethods: enums
Lint/ConstantDefinitionInBlock:
Exclude:
- 'spec/grape-swagger/entities/response_model_spec.rb'
- 'spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb'
- 'spec/support/shared_contexts/inheritance_api.rb'
- 'spec/support/shared_contexts/this_api.rb'

Expand All @@ -21,6 +22,11 @@ Lint/EmptyBlock:
Exclude:
- 'spec/grape-swagger/entity/attribute_parser_spec.rb'

# Offense count: 1
Lint/EmptyClass:
Exclude:
- 'spec/support/shared_contexts/custom_type_parser.rb'

# Offense count: 2
# This cop supports unsafe autocorrection (--autocorrect-all).
# Configuration parameters: AllowedMethods.
Expand All @@ -37,7 +43,7 @@ Metrics/AbcSize:
# Offense count: 2
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 120
Max: 135

# Offense count: 2
# Configuration parameters: AllowedMethods, AllowedPatterns.
Expand Down Expand Up @@ -79,11 +85,12 @@ RSpec/ContextWording:
- 'spec/support/shared_contexts/inheritance_api.rb'
- 'spec/support/shared_contexts/this_api.rb'

# Offense count: 2
# Offense count: 3
# Configuration parameters: IgnoredMetadata.
RSpec/DescribeClass:
Exclude:
- '**/spec/features/**/*'
- '**/spec/issues/**/*'
- '**/spec/requests/**/*'
- '**/spec/routing/**/*'
- '**/spec/system/**/*'
Expand All @@ -95,10 +102,11 @@ RSpec/DescribeClass:
RSpec/ExampleLength:
Max: 213

# Offense count: 26
# Offense count: 30
RSpec/LeakyConstantDeclaration:
Exclude:
- 'spec/grape-swagger/entities/response_model_spec.rb'
- '**/spec/issues/**/*'
- 'spec/support/shared_contexts/inheritance_api.rb'
- 'spec/support/shared_contexts/this_api.rb'

Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#### Fixes

* Your contribution here.
* [#86](https://github.com/ruby-grape/grape-swagger-entity/pull/86): Fix Array[Object] documentation type parsing - [@numbata](https://github.com/numbata).
* [#87](https://github.com/ruby-grape/grape-swagger-entity/pull/87): Remove hidden attributes from required - [@bogdan](https://github.com/bogdan).
* [#88](https://github.com/ruby-grape/grape-swagger-entity/pull/88): Update danger workflows - [@numbata](https://github.com/numbata).
* [#89](https://github.com/ruby-grape/grape-swagger-entity/pull/89): Remove ruby-grape-danger - [@dblock](https://github.com/dblock).
Expand Down
27 changes: 23 additions & 4 deletions lib/grape-swagger/entity/attribute_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -37,11 +37,26 @@ 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

# Checks if documentation indicates an array type that needs items wrapping.
# Returns true for:
# - is_array: true (explicit flag)
# - type: 'Array[Something]' (array with element type)
# Returns false for:
# - type: 'Array' (bare array - already an array type, no wrapping needed)
def array_type?(documentation)
return false if documentation.nil?
return documentation[:is_array] if documentation.key?(:is_array)

# Only match Array[ElementType] syntax, not bare 'Array'
documentation[:type].to_s.match?(/\Aarray\[.+\]\z/i)
end

def could_it_be_a_model?(value)
return false if value.nil?

Expand All @@ -57,6 +72,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
Expand All @@ -69,7 +86,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
Expand Down Expand Up @@ -97,7 +114,9 @@ def document_data_type(documentation, data_type)
end

def entity_model_type(name, entity_options)
if entity_options[:documentation] && entity_options[:documentation][:is_array]
documentation = entity_options[:documentation]

if array_type?(documentation)
{
'type' => 'array',
'items' => {
Expand Down
70 changes: 45 additions & 25 deletions lib/grape-swagger/entity/parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

{
Expand All @@ -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)
]
]
}
Expand Down Expand Up @@ -111,7 +131,7 @@ def parse_nested(entity_name, entity_options, parent_model = nil)
.map(&:nested_exposures)
.flatten
.each_with_object({}) do |value, memo|
memo[value.attribute] = value.send(:options)
memo[value.attribute] = value.send(:options)
end

properties, required = parse_grape_entity_params(params, nested_entities.last)
Expand Down
2 changes: 2 additions & 0 deletions spec/grape-swagger/entities/response_model_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ def app
JSON.parse(last_response.body)['definitions']
end

include_context 'this api'

before :all do
module TheseApi
module Entities
Expand Down
30 changes: 30 additions & 0 deletions spec/grape-swagger/entity/attribute_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,29 @@ def self.to_s
it { is_expected.to include(type: :array) }
it { is_expected.to include(items: { type: 'string' }) }

context 'when using Array[Type] syntax' do
# Array[Type] syntax for primitives preserves the type string as-is.
# For entity models, use the `using:` option with Array[Object] syntax.
let(:entity_options) { { documentation: { type: 'Array[String]', desc: 'Colors' } } }

it { is_expected.to include(type: :array) }
it { is_expected.to include(items: { type: 'Array[String]' }) }
end

context 'when using lowercase array[type] syntax' do
let(:entity_options) { { documentation: { type: 'array[integer]', desc: 'Numbers' } } }

it { is_expected.to include(type: :array) }
it { is_expected.to include(items: { type: 'array[integer]' }) }
end

context 'when is_array is explicitly false' do
let(:entity_options) { { documentation: { type: 'string', desc: 'Single value', is_array: false } } }

it { is_expected.to include(type: 'string') }
it { is_expected.not_to include(:items) }
end

context 'when it contains example' do
let(:entity_options) do
{ documentation: { type: 'string', desc: 'Colors', is_array: true, example: %w[green blue] } }
Expand Down Expand Up @@ -262,6 +285,13 @@ def self.to_s
it { is_expected.not_to include('$ref') }
end

context 'when using bare Array type' do
let(:entity_options) { { documentation: { type: 'Array', desc: 'Generic array' } } }

it { is_expected.to include(type: 'array') }
it { is_expected.not_to include(:items) }
end

context 'when it is exposed as a Boolean class' do
let(:entity_options) do
{ documentation: { type: Grape::API::Boolean, example: example_value, default: example_value } }
Expand Down
Loading