Skip to content

Commit 58af6e2

Browse files
numbatanumbata
authored
Allow empty model definitions for Swagger 2.0 (#963)
* feat: Allow empty model definitions in Swagger 2.0 This commit relaxes a validation check that prevented the creation of an empty definitions or properties object in the Swagger 2.0 schema. The previous behavior would raise a GrapeSwagger::Errors::SwaggerSpec exception when a model, such as an entity with only hidden properties, resulted in an empty definition. This new behavior is more in line with the Swagger specification, which does not forbid such schemas. The fix involves removing the redundant if properties&.any? check and its associated error, allowing the parser to correctly build an empty but valid model. * Add required properties according last grape-entity changes * Update CHANGELOG.md
1 parent cb6962a commit 58af6e2

File tree

9 files changed

+148
-18
lines changed

9 files changed

+148
-18
lines changed

‎.rspec‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
--color
22
--profile
33
--format documentation
4+
--require 'spec_helper'

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* [#958](https://github.com/ruby-grape/grape-swagger/pull/958): Drop ruby-head from test matrix - [@numbata](https://github.com/numbata).
77
* [#953](https://github.com/ruby-grape/grape-swagger/pull/953): Added `super_diff` - [@numbata](https://github.com/numbata).
88
* [#951](https://github.com/ruby-grape/grape-swagger/pull/951): Use `x-example` for non-body parameters - [@olivier-thatch](https://github.com/olivier-thatch).
9+
* [#963](https://github.com/ruby-grape/grape-swagger/pull/963): Allow empty model definitions for swagger 2.0 - [@numbata](https://github.com/numbata).
910
* Your contribution here.
1011

1112
#### Fixes

‎Gemfile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# frozen_string_literal: true
22

3-
source 'http://rubygems.org'
3+
source 'https://rubygems.org'
44

55
gemspec
66

‎lib/grape-swagger/doc_methods/build_model_definition.rb‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ def parse_params_from_model(parsed_response, model, model_name)
2222
}
2323
else
2424
properties, required = parsed_response
25-
unless properties&.any?
25+
if properties.nil?
2626
raise GrapeSwagger::Errors::SwaggerSpec,
2727
"Empty model #{model_name}, swagger 2.0 doesn't support empty definitions."
2828
end

‎spec/issues/539_array_post_body_spec.rb‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ class ArrayOfElements < Grape::Entity
7474
'id' => { 'type' => 'string' },
7575
'description' => { 'type' => 'string' },
7676
'role' => { 'type' => 'string' }
77-
}
77+
},
78+
'required' => %w[id description role]
7879
}
7980
)
8081
end

‎spec/issues/962_polymorphic_entity_with_custom_documentation_spec.rb‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
# frozen_string_literal: true
2+
3+
describe '#962 polymorphic entity with custom documentation' do
4+
let(:app) do
5+
Class.new(Grape::API) do
6+
namespace :issue_962 do
7+
module Issue962
8+
class EmptyEntity < Grape::Entity
9+
end
10+
11+
class EntityWithHiddenProperty < Grape::Entity
12+
expose :hidden_prop, documentation: { hidden: true, desc: 'This property is not exposed.' }
13+
end
14+
15+
class EntityWithNestedEmptyEntity < Grape::Entity
16+
expose :array_of_empty_entities,
17+
as: :empty_items,
18+
using: Issue962::EmptyEntity,
19+
documentation: {
20+
is_array: true,
21+
desc: 'This is a nested empty entity.'
22+
}
23+
expose :array_of_hidden_entities,
24+
as: :hidden_items,
25+
using: Issue962::EntityWithHiddenProperty,
26+
documentation: {
27+
is_array: true,
28+
desc: 'This is a nested entity with hidden props'
29+
}
30+
end
31+
end
32+
33+
desc 'Get a report',
34+
success: Issue962::EntityWithNestedEmptyEntity
35+
get '/' do
36+
present({ foo: [] }, with: Issue962::EntityWithNestedEmptyEntity)
37+
end
38+
end
39+
40+
add_swagger_documentation format: :json
41+
end
42+
end
43+
44+
subject do
45+
get '/swagger_doc'
46+
JSON.parse(last_response.body)
47+
end
48+
49+
let(:definitions) { subject['definitions'] }
50+
let(:entity_definition) { definitions['Issue962_EntityWithNestedEmptyEntity'] }
51+
let(:empty_items_property) { entity_definition['properties']['empty_items'] }
52+
let(:hidden_items_property) { entity_definition['properties']['hidden_items'] }
53+
let(:empty_entity_definition) { definitions['Issue962_EmptyEntity'] }
54+
let(:hidden_entity_definition) { definitions['Issue962_EntityWithHiddenProperty'] }
55+
56+
specify 'should generate swagger documentation without error' do
57+
expect { subject }.not_to raise_error
58+
end
59+
60+
specify do
61+
expect(definitions.keys).to include(
62+
'Issue962_EntityWithNestedEmptyEntity',
63+
'Issue962_EntityWithHiddenProperty',
64+
'Issue962_EmptyEntity'
65+
)
66+
end
67+
68+
specify do
69+
expect(empty_items_property).to eql({
70+
'type' => 'array',
71+
'description' => 'This is a nested empty entity.',
72+
'items' => {
73+
'$ref' => '#/definitions/Issue962_EmptyEntity'
74+
}
75+
})
76+
end
77+
78+
specify do
79+
expect(hidden_items_property).to eql({
80+
'type' => 'array',
81+
'description' => 'This is a nested entity with hidden props',
82+
'items' => {
83+
'$ref' => '#/definitions/Issue962_EntityWithHiddenProperty'
84+
}
85+
})
86+
end
87+
88+
specify do
89+
expect(empty_entity_definition).to eql({
90+
'type' => 'object',
91+
'properties' => {}
92+
})
93+
end
94+
95+
specify do
96+
expect(hidden_entity_definition).to eql({
97+
'type' => 'object',
98+
'properties' => {},
99+
'required' => ['hidden_prop']
100+
})
101+
end
102+
103+
let(:response_schema) { subject['paths']['/issue_962']['get']['responses']['200']['schema'] }
104+
105+
specify do
106+
expect(response_schema).to eql({
107+
'$ref' => '#/definitions/Issue962_EntityWithNestedEmptyEntity'
108+
})
109+
end
110+
end

‎spec/support/model_parsers/entity_parser.rb‎

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -135,33 +135,44 @@ class DocumentedHashAndArrayModel < Grape::Entity
135135

136136
let(:swagger_definitions_models) do
137137
{
138-
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } } },
139-
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } } },
140-
'UseResponse' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, '$responses' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ResponseItem' } } } },
141-
'RecursiveModel' => { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'The name.' }, 'children' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/RecursiveModel' }, 'description' => 'The child nodes.' } } },
142-
'DocumentedHashAndArrayModel' => { 'type' => 'object', 'properties' => { 'raw_hash' => { 'type' => 'object', 'description' => 'Example Hash.' }, 'raw_array' => { 'type' => 'array', 'description' => 'Example Array' } } }
138+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'required' => %w[code message] },
139+
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } }, 'required' => %w[id name] },
140+
'UseResponse' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, '$responses' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ResponseItem' } } }, 'required' => ['description', '$responses'] },
141+
'RecursiveModel' => { 'type' => 'object', 'properties' => { 'name' => { 'type' => 'string', 'description' => 'The name.' }, 'children' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/RecursiveModel' }, 'description' => 'The child nodes.' } }, 'required' => %w[name children] },
142+
'DocumentedHashAndArrayModel' => { 'type' => 'object', 'properties' => { 'raw_hash' => { 'type' => 'object', 'description' => 'Example Hash.' }, 'raw_array' => { 'type' => 'array', 'description' => 'Example Array' } }, 'required' => %w[raw_hash raw_array] }
143143
}
144144
end
145145

146146
let(:swagger_nested_type) do
147147
{
148-
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'description' => 'ApiError model' },
149-
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } } },
150-
'UseItemResponseAsType' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, 'responses' => { '$ref' => '#/definitions/ResponseItem' } }, 'description' => 'UseItemResponseAsType model' }
148+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'required' => %w[code message], 'description' => 'ApiError model' },
149+
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } }, 'required' => %w[id name] },
150+
'UseItemResponseAsType' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, 'responses' => { '$ref' => '#/definitions/ResponseItem' } }, 'required' => %w[description responses], 'description' => 'UseItemResponseAsType model' }
151151
}
152152
end
153153

154154
let(:swagger_entity_as_response_object) do
155155
{
156-
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'description' => 'ApiError model' },
157-
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } } },
158-
'UseResponse' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, '$responses' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ResponseItem' } } }, 'description' => 'UseResponse model' }
156+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'required' => %w[code message], 'description' => 'ApiError model' },
157+
'ResponseItem' => { 'type' => 'object', 'properties' => { 'id' => { 'type' => 'integer', 'format' => 'int32' }, 'name' => { 'type' => 'string' } }, 'required' => %w[id name] },
158+
'UseResponse' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, '$responses' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ResponseItem' } } }, 'required' => ['description', '$responses'], 'description' => 'UseResponse model' }
159159
}
160160
end
161161

162162
let(:swagger_params_as_response_object) do
163163
{
164-
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'description' => 'status code', 'type' => 'integer', 'format' => 'int32' }, 'message' => { 'description' => 'error message', 'type' => 'string' } }, 'description' => 'ApiError model' }
164+
'ApiError' => {
165+
'type' => 'object',
166+
'properties' => {
167+
'code' => { 'description' => 'status code', 'type' => 'integer', 'format' => 'int32' },
168+
'message' => { 'description' => 'error message', 'type' => 'string' }
169+
},
170+
'required' => %w[
171+
code
172+
message
173+
],
174+
'description' => 'ApiError model'
175+
}
165176
}
166177
end
167178

@@ -310,6 +321,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
310321
'ApiError' => {
311322
'type' => 'object',
312323
'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } },
324+
'required' => %w[code message],
313325
'description' => 'ApiError model'
314326
},
315327
'Something' => {
@@ -320,6 +332,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
320332
'links' => { 'type' => 'array', 'items' => { 'type' => 'link' } },
321333
'others' => { 'type' => 'text' }
322334
},
335+
'required' => %w[id text links others],
323336
'description' => 'Something model'
324337
}
325338
}

‎spec/swagger_v2/errors_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@
3434
end
3535
end
3636

37-
it 'should raise SwaggerSpec exception' do
38-
expect { get '/v3/swagger_doc' }.to raise_error(GrapeSwagger::Errors::SwaggerSpec, "Empty model EmptyClass, swagger 2.0 doesn't support empty definitions.")
37+
it 'should not raise SwaggerSpec exception' do
38+
expect { get '/v3/swagger_doc' }.not_to raise_error(GrapeSwagger::Errors::SwaggerSpec)
3939
end
4040
end
4141

‎spec/swagger_v2/reference_entity_spec.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,9 @@ def app
9090

9191
expect(subject['definitions'].keys).to include 'SomethingCustom'
9292
expect(subject['definitions']['SomethingCustom']).to eq(
93-
'type' => 'object', 'properties' => { 'text' => { 'type' => 'string', 'description' => 'Content of something.' } }
93+
'type' => 'object',
94+
'properties' => { 'text' => { 'type' => 'string', 'description' => 'Content of something.' } },
95+
'required' => ['text']
9496
)
9597

9698
expect(subject['definitions'].keys).to include 'KindCustom'
@@ -103,6 +105,7 @@ def app
103105
'description' => 'Something interesting.'
104106
}
105107
},
108+
'required' => %w[title something],
106109
'description' => 'KindCustom model'
107110
)
108111
end
@@ -122,6 +125,7 @@ def app
122125
'title' => { 'type' => 'string', 'description' => 'Title of the parent.' },
123126
'child' => { 'type' => 'string', 'description' => 'Child property.' }
124127
},
128+
'required' => %w[title child],
125129
'description' => 'MyAPI::Child model'
126130
)
127131
end

0 commit comments

Comments
 (0)