Skip to content

Commit 64ebd4c

Browse files
committed
Tighten variant-type order, pop_desc nil semantics, document recovery scope
route.rb - Convert the recovered VariantCollectionCoercer @types via #to_a so a Set-backed coercer yields a deterministic order (insertion order from the user's declaration), matching what `parse_multi_type` will pick via `.first` downstream. - Guard scope.full_name with `respond_to?` so a future CoerceValidator built without @scope (custom subclasses, test doubles) degrades to skipping that validator rather than crashing the whole swagger doc generation with NoMethodError. - Expand the leading comment to note that on Grape < 3.3 the recovery is a no-op (the legacy `"[A, B]"` string is already handled by parse_multi_type's regex branch), so a reader doesn't conclude the defined? guard is silently overriding working behaviour. doc_methods.rb (pop_desc) - Use `key?` instead of `||` so an explicit `desc: nil` is respected rather than silently falling through to `:description`. Document the precedence inline (`:desc` wins when both keys are supplied). specs - Cover the new public-facing `:desc`/`:description`/string-key semantics in api_documentation_spec so a future refactor can't regress them silently.
1 parent 0942165 commit 64ebd4c

3 files changed

Lines changed: 54 additions & 8 deletions

File tree

lib/grape-swagger/doc_methods.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,9 @@ def setup_formatter(formatter)
138138
end
139139

140140
def pop_desc(doc)
141-
doc.delete(:desc) || doc.delete(:description)
141+
# :desc takes precedence over :description; explicit nil under :desc wins
142+
# (don't fall through on nil — that would silently substitute :description).
143+
doc.key?(:desc) ? doc.delete(:desc) : doc.delete(:description)
142144
end
143145
end
144146
end

lib/grape-swagger/request_param_parsers/route.rb

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,16 @@ def fetch_inherited_params(stackable_values)
4848
end
4949
end
5050

51-
# In Grape >= 3.3 `type: [A, B]` is wrapped in VariantCollectionCoercer and
52-
# Grape's documentation pipeline serialises it via `#to_s`, so the original
53-
# type list is lost in route.params. The live coercer is still reachable
54-
# through the CoerceValidator's @converter, so we rebuild a name => types
55-
# map keyed by the fully-qualified param name (e.g. "group[inner]") to match
56-
# route.params keys and avoid clobbering same-named params at outer scopes.
51+
# On Grape >= 3.3 `type: [A, B]` is documented in route.params via the
52+
# VariantCollectionCoercer's `#to_s`, which loses the original type list.
53+
# On earlier versions the same param appears as the stringified Array
54+
# `"[A, B]"` and the existing regex in DataType.parse_multi_type already
55+
# handles it; the recovery here is a no-op for those versions.
56+
#
57+
# The live coercer is still reachable through the CoerceValidator's
58+
# @converter, so we rebuild a name => types map keyed by the fully-qualified
59+
# param name (e.g. "group[inner]") to match route.params keys and avoid
60+
# clobbering same-named params at outer scopes.
5761
def collect_variant_types(stackable_values)
5862
variant_types = {}
5963
return variant_types unless defined?(Grape::Validations::Types::VariantCollectionCoercer) &&
@@ -66,8 +70,12 @@ def collect_variant_types(stackable_values)
6670
converter = validator.instance_variable_get(:@converter)
6771
next unless converter.is_a?(Grape::Validations::Types::VariantCollectionCoercer)
6872

69-
types = Array(converter.instance_variable_get(:@types))
73+
# `.to_a` preserves the user-declared order for both Array and Set
74+
# storage shapes; `DataType.parse_multi_type` uses `.first` downstream.
75+
types = converter.instance_variable_get(:@types).to_a
7076
scope = validator.instance_variable_get(:@scope)
77+
next unless scope.respond_to?(:full_name)
78+
7179
validator.attrs.each do |attr|
7280
variant_types[scope.full_name(attr)] = types
7381
end

spec/swagger_v2/api_documentation_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,40 @@
2323
'Swagger compatible API description for specific API'
2424
]
2525
end
26+
27+
context 'when api_documentation is string-keyed' do
28+
let(:api) do
29+
Class.new(Grape::API) do
30+
add_swagger_documentation api_documentation: { 'desc' => 'String-keyed description' }
31+
end
32+
end
33+
34+
it 'accepts string keys' do
35+
expect(subject.first[:description]).to eq('String-keyed description')
36+
end
37+
end
38+
39+
context 'when api_documentation uses :description instead of :desc' do
40+
let(:api) do
41+
Class.new(Grape::API) do
42+
add_swagger_documentation api_documentation: { description: 'Via description key' }
43+
end
44+
end
45+
46+
it 'falls back to :description' do
47+
expect(subject.first[:description]).to eq('Via description key')
48+
end
49+
end
50+
51+
context 'when both :desc and :description are supplied' do
52+
let(:api) do
53+
Class.new(Grape::API) do
54+
add_swagger_documentation api_documentation: { desc: 'Desc wins', description: 'Description loses' }
55+
end
56+
end
57+
58+
it ':desc takes precedence' do
59+
expect(subject.first[:description]).to eq('Desc wins')
60+
end
61+
end
2662
end

0 commit comments

Comments
 (0)