Skip to content

Commit f5a0771

Browse files
numbatanumbata
authored
Fix #44: description not rendered for non-array entity references (#90)
## Summary Fixes #44 In OpenAPI/Swagger 2.0 and 3.0.x, `$ref` cannot have sibling properties - they are ignored by tools like Swagger-UI. This caused: - `desc` (description) on non-array entity references to not be displayed - `readOnly` flags to be ignored The fix wraps `$ref` in `allOf` when `description` or `readOnly` is present: **Before** (invalid - siblings ignored): ```json { "$ref": "#/definitions/Address", "description": "Client address" } ``` **After** (valid OpenAPI): ```json { "allOf": [{ "$ref": "#/definitions/Address" }], "description": "Client address" } ``` Array entity references were not affected because the description is a sibling to `type` and `items`, not `$ref`.
1 parent 409d701 commit f5a0771

6 files changed

Lines changed: 138 additions & 24 deletions

File tree

‎.rubocop_todo.yml‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,13 @@
66
# Note that changes in the inspected code, or installation of new
77
# versions of RuboCop, may require this file to be generated again.
88

9-
# Offense count: 7
9+
# Offense count: 8
1010
# Configuration parameters: AllowedMethods.
1111
# AllowedMethods: enums
1212
Lint/ConstantDefinitionInBlock:
1313
Exclude:
1414
- 'spec/grape-swagger/entities/response_model_spec.rb'
15+
- 'spec/issues/44_desc_in_entity_type_spec.rb'
1516
- 'spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb'
1617
- 'spec/support/shared_contexts/inheritance_api.rb'
1718
- 'spec/support/shared_contexts/this_api.rb'
@@ -43,7 +44,7 @@ Metrics/AbcSize:
4344
# Offense count: 2
4445
# Configuration parameters: CountComments, CountAsOne.
4546
Metrics/ClassLength:
46-
Max: 135
47+
Max: 145
4748

4849
# Offense count: 2
4950
# Configuration parameters: AllowedMethods, AllowedPatterns.
@@ -100,7 +101,7 @@ RSpec/DescribeClass:
100101
# Offense count: 4
101102
# Configuration parameters: CountAsOne.
102103
RSpec/ExampleLength:
103-
Max: 213
104+
Max: 225
104105

105106
# Offense count: 30
106107
RSpec/LeakyConstantDeclaration:

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#### Fixes
88

99
* Your contribution here.
10+
* [#90](https://github.com/ruby-grape/grape-swagger-entity/pull/90): Fix description not rendered for non-array entity references - [@numbata](https://github.com/numbata).
1011
* [#86](https://github.com/ruby-grape/grape-swagger-entity/pull/86): Fix Array[Object] documentation type parsing - [@numbata](https://github.com/numbata).
1112
* [#87](https://github.com/ruby-grape/grape-swagger-entity/pull/87): Remove hidden attributes from required - [@bogdan](https://github.com/bogdan).
1213
* [#88](https://github.com/ruby-grape/grape-swagger-entity/pull/88): Update danger workflows - [@numbata](https://github.com/numbata).

‎lib/grape-swagger/entity/parser.rb‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,23 @@ def parse_entity_options(entity_options, entity_name, parent_model)
8080
def add_documentation_to_memo(memo_entry, documentation)
8181
return unless documentation
8282

83-
memo_entry[:readOnly] = documentation[:read_only].to_s == 'true' if documentation[:read_only]
84-
memo_entry[:description] = documentation[:desc] if documentation[:desc]
83+
has_read_only = documentation[:read_only]
84+
has_description = documentation[:desc]
85+
86+
return unless has_read_only || has_description
87+
88+
# In OpenAPI/Swagger 2.0 and 3.0.x, $ref cannot have sibling properties - they are ignored.
89+
# To add description or readOnly to a $ref, wrap it in allOf.
90+
# See: https://swagger.io/docs/specification/using-ref/
91+
wrap_ref_in_all_of(memo_entry) if memo_entry.key?('$ref')
92+
93+
memo_entry[:readOnly] = documentation[:read_only].to_s == 'true' if has_read_only
94+
memo_entry[:description] = documentation[:desc] if has_description
95+
end
96+
97+
def wrap_ref_in_all_of(memo_entry)
98+
ref = memo_entry.delete('$ref')
99+
memo_entry['allOf'] = [{ '$ref' => ref }]
85100
end
86101

87102
def handle_discriminator(parsed, required)

‎spec/grape-swagger/entities/response_model_spec.rb‎

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,16 @@ def app
5656
{ 'text' => { 'type' => 'string', 'description' => 'Content of something.' },
5757
'colors' => { 'type' => 'array', 'items' => { 'type' => 'string' }, 'description' => 'Colors' },
5858
'created_at' => { 'type' => 'string', 'format' => 'date-time', 'description' => 'Created at the time.' },
59-
'kind' => { '$ref' => '#/definitions/ThisApi_Entities_Kind',
59+
'kind' => { 'allOf' => [{ '$ref' => '#/definitions/ThisApi_Entities_Kind' }],
6060
'description' => 'The kind of this something.' },
61-
'kind2' => { '$ref' => '#/definitions/ThisApi_Entities_Kind', 'description' => 'Secondary kind.' },
62-
'kind3' => { '$ref' => '#/definitions/ThisApi_Entities_Kind', 'description' => 'Tertiary kind.' },
61+
'kind2' => { 'allOf' => [{ '$ref' => '#/definitions/ThisApi_Entities_Kind' }],
62+
'description' => 'Secondary kind.' },
63+
'kind3' => { 'allOf' => [{ '$ref' => '#/definitions/ThisApi_Entities_Kind' }],
64+
'description' => 'Tertiary kind.' },
6365
'tags' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ThisApi_Entities_Tag' },
6466
'description' => 'Tags.' },
65-
'relation' => { '$ref' => '#/definitions/ThisApi_Entities_Relation', 'description' => 'A related model.',
66-
'x-other' => 'stuff' },
67+
'relation' => { 'allOf' => [{ '$ref' => '#/definitions/ThisApi_Entities_Relation' }],
68+
'description' => 'A related model.', 'x-other' => 'stuff' },
6769
'code' => { 'type' => 'string', 'description' => 'Error code' },
6870
'message' => { 'type' => 'string', 'description' => 'Error message' },
6971
'attr' => { 'type' => 'string', 'description' => 'Attribute' } },
@@ -409,8 +411,10 @@ def app
409411
expect(subject['TheseApi_Entities_Polymorphic']).to eql(
410412
'type' => 'object',
411413
'properties' => {
412-
'kind' => { '$ref' => '#/definitions/TheseApi_Entities_Kind', 'description' => 'Polymorphic Kind' },
413-
'values' => { '$ref' => '#/definitions/TheseApi_Entities_Values', 'description' => 'Polymorphic Values' },
414+
'kind' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Kind' }],
415+
'description' => 'Polymorphic Kind' },
416+
'values' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Values' }],
417+
'description' => 'Polymorphic Values' },
414418
'str' => { 'type' => 'string', 'description' => 'Polymorphic String' },
415419
'num' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'Polymorphic Number' }
416420
}
@@ -432,22 +436,28 @@ def app
432436
'type' => 'object',
433437
'properties' => {
434438
'text' => { 'type' => 'string', 'description' => 'Content of something.' },
435-
'kind' => { '$ref' => '#/definitions/TheseApi_Entities_Kind', 'description' => 'The kind of this something.' },
436-
'kind2' => { '$ref' => '#/definitions/TheseApi_Entities_Kind', 'description' => 'Secondary kind.' },
437-
'kind3' => { '$ref' => '#/definitions/TheseApi_Entities_Kind', 'description' => 'Tertiary kind.' },
439+
'kind' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Kind' }],
440+
'description' => 'The kind of this something.' },
441+
'kind2' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Kind' }],
442+
'description' => 'Secondary kind.' },
443+
'kind3' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Kind' }],
444+
'description' => 'Tertiary kind.' },
438445
'tags' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/TheseApi_Entities_Tag' },
439446
'description' => 'Tags.' },
440-
'relation' => { '$ref' => '#/definitions/TheseApi_Entities_Relation', 'description' => 'A related model.' },
441-
'values' => { '$ref' => '#/definitions/TheseApi_Entities_Values', 'description' => 'Tertiary kind.' },
442-
'nested' => { '$ref' => '#/definitions/TheseApi_Entities_Nested', 'description' => 'Nested object.' },
443-
'nested_child' => { '$ref' => '#/definitions/TheseApi_Entities_NestedChild',
447+
'relation' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Relation' }],
448+
'description' => 'A related model.' },
449+
'values' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Values' }],
450+
'description' => 'Tertiary kind.' },
451+
'nested' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Nested' }],
452+
'description' => 'Nested object.' },
453+
'nested_child' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_NestedChild' }],
444454
'description' => 'Nested child object.' },
445455
'code' => { 'type' => 'string', 'description' => 'Error code' },
446456
'message' => { 'type' => 'string', 'description' => 'Error message' },
447-
'polymorphic' => { '$ref' => '#/definitions/TheseApi_Entities_Polymorphic',
457+
'polymorphic' => { 'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_Polymorphic' }],
448458
'description' => 'A polymorphic model.' },
449459
'mixed' => {
450-
'$ref' => '#/definitions/TheseApi_Entities_MixedType',
460+
'allOf' => [{ '$ref' => '#/definitions/TheseApi_Entities_MixedType' }],
451461
'description' => 'A model with mix of types.'
452462
},
453463
'attr' => { 'type' => 'string', 'description' => 'Attribute' }

‎spec/grape-swagger/entity/parser_spec.rb‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@
1616
let(:endpoint) { nil }
1717

1818
it 'parses the model with the correct :using definition' do
19-
expect(properties[:kind]['$ref']).to eq('#/definitions/Kind')
20-
expect(properties[:kind2]['$ref']).to eq('#/definitions/Kind')
21-
expect(properties[:kind3]['$ref']).to eq('#/definitions/Kind')
19+
# $ref is wrapped in allOf when description is present (OpenAPI compliance)
20+
expect(properties[:kind]['allOf']).to eq([{ '$ref' => '#/definitions/Kind' }])
21+
expect(properties[:kind2]['allOf']).to eq([{ '$ref' => '#/definitions/Kind' }])
22+
expect(properties[:kind3]['allOf']).to eq([{ '$ref' => '#/definitions/Kind' }])
2223
end
2324

2425
it 'does not mark hidden attributes as required' do

‎spec/issues/44_desc_in_entity_type_spec.rb‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
# frozen_string_literal: true
2+
3+
describe '#44 desc not rendered for non-array entity references' do
4+
subject(:swagger_doc) do
5+
get '/swagger_doc'
6+
JSON.parse(last_response.body)
7+
end
8+
9+
let(:app) do
10+
Class.new(Grape::API) do
11+
namespace :issue44 do
12+
class Address < Grape::Entity
13+
expose :street, documentation: { type: 'string', desc: 'Street' }
14+
end
15+
16+
class Client < Grape::Entity
17+
expose :name, documentation: { type: 'string', desc: 'Name' }
18+
expose :address,
19+
using: Address,
20+
documentation: { type: 'Address', desc: 'Client address', is_array: false }
21+
expose :billing_address,
22+
using: Address,
23+
documentation: { type: 'Address', read_only: true }
24+
end
25+
26+
class ClientWithArrayAddress < Grape::Entity
27+
expose :name, documentation: { type: 'string', desc: 'Name' }
28+
expose :addresses,
29+
using: Address,
30+
documentation: { type: 'Address', desc: 'Client addresses', is_array: true }
31+
end
32+
33+
desc 'Get a client', success: Client
34+
get '/client' do
35+
present({ name: 'John', address: { street: '123 Main St' } }, with: Client)
36+
end
37+
38+
desc 'Get a client with addresses', success: ClientWithArrayAddress
39+
get '/client_with_addresses' do
40+
present({ name: 'John', addresses: [{ street: '123 Main St' }] }, with: ClientWithArrayAddress)
41+
end
42+
end
43+
44+
add_swagger_documentation format: :json
45+
end
46+
end
47+
48+
describe 'non-array entity reference' do
49+
subject(:address_property) { swagger_doc['definitions']['Client']['properties']['address'] }
50+
51+
it 'includes description using allOf wrapper for OpenAPI compatibility' do
52+
# In OpenAPI/Swagger, $ref cannot have sibling properties.
53+
# To add description to a $ref, it must be wrapped in allOf.
54+
# See: https://swagger.io/docs/specification/using-ref/#syntax
55+
expect(address_property).to eq({
56+
'allOf' => [{ '$ref' => '#/definitions/Address' }],
57+
'description' => 'Client address'
58+
})
59+
end
60+
end
61+
62+
describe 'non-array entity reference with readOnly' do
63+
subject(:billing_property) { swagger_doc['definitions']['Client']['properties']['billing_address'] }
64+
65+
it 'includes readOnly using allOf wrapper for OpenAPI compatibility' do
66+
# readOnly is also a sibling property that needs allOf wrapping
67+
expect(billing_property).to eq({
68+
'allOf' => [{ '$ref' => '#/definitions/Address' }],
69+
'readOnly' => true
70+
})
71+
end
72+
end
73+
74+
describe 'array entity reference' do
75+
subject(:addresses_property) { swagger_doc['definitions']['ClientWithArrayAddress']['properties']['addresses'] }
76+
77+
it 'includes description directly since array wrapper allows siblings' do
78+
# For arrays, the description can be a sibling to 'type' and 'items'
79+
expect(addresses_property).to eq({
80+
'type' => 'array',
81+
'items' => { '$ref' => '#/definitions/Address' },
82+
'description' => 'Client addresses'
83+
})
84+
end
85+
end
86+
end

0 commit comments

Comments
 (0)