Skip to content

Commit a343211

Browse files
committed
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
1 parent 8110e4a commit a343211

File tree

4 files changed

+61
-29
lines changed

4 files changed

+61
-29
lines changed

.rubocop_todo.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ Metrics/AbcSize:
3737
# Offense count: 2
3838
# Configuration parameters: CountComments, CountAsOne.
3939
Metrics/ClassLength:
40-
Max: 117
40+
Max: 132
4141

4242
# Offense count: 2
4343
# Configuration parameters: AllowedMethods, AllowedPatterns.

lib/grape-swagger/entity/attribute_parser.rb

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ def call(entity_options)
2020
documentation = entity_options[:documentation]
2121
return param if documentation.nil?
2222

23-
add_array_documentation(param, documentation) if documentation[:is_array]
23+
add_array_documentation(param, documentation) if array_type?(documentation)
2424

2525
add_attribute_sample(param, documentation, :default)
2626
add_attribute_sample(param, documentation, :example)
@@ -37,11 +37,19 @@ def call(entity_options)
3737
def model_from(entity_options)
3838
model = entity_options[:using] if entity_options[:using].present?
3939

40-
model ||= entity_options[:documentation][:type] if could_it_be_a_model?(entity_options[:documentation])
40+
documentation = entity_options[:documentation]
41+
model ||= documentation[:type] if could_it_be_a_model?(documentation)
4142

4243
model
4344
end
4445

46+
def array_type?(documentation)
47+
return documentation[:is_array] if documentation.key?(:is_array)
48+
return true if documentation[:type].to_s.downcase == 'array'
49+
50+
documentation[:type].to_s.match?(/\Aarray\[(?<type>.+)\]\z/i)
51+
end
52+
4553
def could_it_be_a_model?(value)
4654
return false if value.nil?
4755

@@ -57,6 +65,8 @@ def ambiguous_model_type?(type)
5765
end
5866

5967
def primitive_type?(type)
68+
return false if type.nil?
69+
6070
data_type = GrapeSwagger::DocMethods::DataType.call(type)
6171
GrapeSwagger::DocMethods::DataType.request_primitive?(data_type)
6272
end
@@ -69,7 +79,7 @@ def data_type_from(entity_options)
6979

7080
documented_data_type = document_data_type(documentation, data_type)
7181

72-
if documentation[:is_array]
82+
if array_type?(documentation)
7383
{
7484
type: :array,
7585
items: documented_data_type
@@ -97,7 +107,7 @@ def document_data_type(documentation, data_type)
97107
end
98108

99109
def entity_model_type(name, entity_options)
100-
if entity_options[:documentation] && entity_options[:documentation][:is_array]
110+
if array_type?(entity_options[:documentation])
101111
{
102112
'type' => 'array',
103113
'items' => {

lib/grape-swagger/entity/parser.rb

Lines changed: 44 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -43,37 +43,57 @@ def extract_params(exposure)
4343
def parse_grape_entity_params(params, parent_model = nil)
4444
return unless params
4545

46-
parsed = params.each_with_object({}) do |(entity_name, entity_options), memo|
47-
documentation_options = entity_options.fetch(:documentation, {})
48-
in_option = documentation_options.fetch(:in, nil).to_s
49-
hidden_option = documentation_options.fetch(:hidden, nil)
50-
next if in_option == 'header' || hidden_option == true
51-
52-
entity_name = entity_name.original if entity_name.is_a?(Alias)
53-
final_entity_name = entity_options.fetch(:as, entity_name)
54-
documentation = entity_options[:documentation]
55-
56-
memo[final_entity_name] = if entity_options[:nesting]
57-
parse_nested(entity_name, entity_options, parent_model)
58-
else
59-
attribute_parser.call(entity_options)
60-
end
61-
62-
next unless documentation
63-
64-
memo[final_entity_name][:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only]
65-
memo[final_entity_name][:description] = documentation[:desc] if documentation[:desc]
46+
required = required_params(params)
47+
parsed_params = parse_params(params, parent_model)
48+
49+
handle_discriminator(parsed_params, required)
50+
end
51+
52+
def parse_params(params, parent_model)
53+
params.each_with_object({}) do |(entity_name, entity_options), memo|
54+
next if skip_param?(entity_options)
55+
56+
original_entity_name = entity_name.is_a?(Alias) ? entity_name.original : entity_name
57+
final_entity_name = entity_options.fetch(:as, original_entity_name)
58+
59+
memo[final_entity_name] = parse_entity_options(entity_options, original_entity_name, parent_model)
60+
add_documentation_to_memo(memo[final_entity_name], entity_options[:documentation])
61+
end
62+
end
63+
64+
def skip_param?(entity_options)
65+
documentation_options = entity_options.fetch(:documentation, {})
66+
in_option = documentation_options.fetch(:in, nil).to_s
67+
hidden_option = documentation_options.fetch(:hidden, nil)
68+
69+
in_option == 'header' || hidden_option == true
70+
end
71+
72+
def parse_entity_options(entity_options, entity_name, parent_model)
73+
if entity_options[:nesting]
74+
parse_nested(entity_name, entity_options, parent_model)
75+
else
76+
attribute_parser.call(entity_options)
6677
end
78+
end
79+
80+
def add_documentation_to_memo(memo_entry, documentation)
81+
return unless documentation
82+
83+
memo_entry[:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only]
84+
memo_entry[:description] = documentation[:desc] if documentation[:desc]
85+
end
6786

87+
def handle_discriminator(parsed, required)
6888
discriminator = GrapeSwagger::Entity::Helper.discriminator(model)
6989
if discriminator
70-
respond_with_all_of(parsed, params, discriminator)
90+
respond_with_all_of(parsed, required, discriminator)
7191
else
72-
[parsed, required_params(params)]
92+
[parsed, required]
7393
end
7494
end
7595

76-
def respond_with_all_of(parsed, params, discriminator)
96+
def respond_with_all_of(parsed, required, discriminator)
7797
parent_name = GrapeSwagger::Entity::Helper.model_name(model.superclass, endpoint)
7898

7999
{
@@ -83,7 +103,7 @@ def respond_with_all_of(parsed, params, discriminator)
83103
},
84104
[
85105
add_discriminator(parsed, discriminator),
86-
required_params(params).push(discriminator.attribute)
106+
required.push(discriminator.attribute)
87107
]
88108
]
89109
}

spec/grape-swagger/entities/response_model_spec.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ def app
103103
JSON.parse(last_response.body)['definitions']
104104
end
105105

106+
include_context 'this api'
107+
106108
before :all do
107109
module TheseApi
108110
module Entities

0 commit comments

Comments
 (0)