Skip to content

Commit fc7b5b1

Browse files
committed
Don't overwrite model description with the route destription.
1 parent 588579d commit fc7b5b1

File tree

10 files changed

+160
-97
lines changed

10 files changed

+160
-97
lines changed

CHANGELOG.md

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

99
* Your contribution here.
10-
10+
* [#804](https://github.com/ruby-grape/grape-swagger/pull/804): Don't overwrite model description with the route description. - [@Bhacaz](https://github.com/Bhacaz)
1111

1212
### 1.2.1 (July 15, 2020)
1313

README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -859,10 +859,11 @@ get '/thing', failure: [
859859
end
860860
```
861861

862-
By adding a `model` key, e.g. this would be taken.
862+
By adding a `model` key, e.g. this would be taken. Setting an empty string will act like an empty body.
863863
```ruby
864864
get '/thing', failure: [
865865
{ code: 400, message: 'General error' },
866+
{ code: 403, message: 'Forbidden error', model: '' },
866867
{ code: 422, message: 'Invalid parameter entry', model: Entities::ApiError }
867868
] do
868869
# ...

UPGRADING.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
## Upgrading Grape-swagger
22

3+
### Upgrading to >= 1.3.0
4+
5+
- The model (entity) description no longer comes from the route description. It will have a default value: `<<EntityName>> model`.
6+
37
### Upgrading to >= 1.2.0
48

59
- The entity_name class method is now called on parent classes for inherited entities. Now you can do this

lib/grape-swagger/doc_methods.rb

Lines changed: 66 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,62 @@
1717

1818
module GrapeSwagger
1919
module DocMethods
20+
21+
DEFAULTS =
22+
{
23+
info: {},
24+
models: [],
25+
doc_version: '0.0.1',
26+
target_class: nil,
27+
mount_path: '/swagger_doc',
28+
host: nil,
29+
base_path: nil,
30+
add_base_path: false,
31+
add_version: true,
32+
add_root: false,
33+
hide_documentation_path: true,
34+
format: :json,
35+
authorizations: nil,
36+
security_definitions: nil,
37+
security: nil,
38+
api_documentation: { desc: 'Swagger compatible API description' },
39+
specific_api_documentation: { desc: 'Swagger compatible API description for specific API' },
40+
endpoint_auth_wrapper: nil,
41+
swagger_endpoint_guard: nil,
42+
token_owner: nil
43+
}.freeze
44+
45+
FORMATTER_METHOD = %i[format default_format default_error_formatter].freeze
46+
47+
def self.output_path_definitions(combi_routes, endpoint, target_class, options)
48+
output = endpoint.swagger_object(
49+
target_class,
50+
endpoint.request,
51+
options
52+
)
53+
54+
paths, definitions = endpoint.path_and_definition_objects(combi_routes, options)
55+
tags = tags_from(paths, options)
56+
57+
output[:tags] = tags unless tags.empty? || paths.blank?
58+
output[:paths] = paths unless paths.blank?
59+
output[:definitions] = definitions unless definitions.blank?
60+
61+
output
62+
end
63+
64+
def self.tags_from(paths, options)
65+
tags = GrapeSwagger::DocMethods::TagNameDescription.build(paths)
66+
67+
if options[:tags]
68+
names = options[:tags].map { |t| t[:name] }
69+
tags.reject! { |t| names.include?(t[:name]) }
70+
tags += options[:tags]
71+
end
72+
73+
tags
74+
end
75+
2076
def hide_documentation_path
2177
@@hide_documentation_path
2278
end
@@ -26,54 +82,32 @@ def mount_path
2682
end
2783

2884
def setup(options)
29-
options = defaults.merge(options)
85+
options = DEFAULTS.merge(options)
3086

3187
# options could be set on #add_swagger_documentation call,
3288
# for available options see #defaults
3389
target_class = options[:target_class]
3490
guard = options[:swagger_endpoint_guard]
35-
formatter = options[:format]
3691
api_doc = options[:api_documentation].dup
3792
specific_api_doc = options[:specific_api_documentation].dup
3893

3994
class_variables_from(options)
4095

41-
if formatter
42-
%i[format default_format default_error_formatter].each do |method|
43-
send(method, formatter)
44-
end
45-
end
96+
setup_formatter(options[:format])
4697

4798
desc api_doc.delete(:desc), api_doc
4899

49-
output_path_definitions = proc do |combi_routes, endpoint|
50-
output = endpoint.swagger_object(
51-
target_class,
52-
endpoint.request,
53-
options
54-
)
55-
56-
paths, definitions = endpoint.path_and_definition_objects(combi_routes, options)
57-
tags = tags_from(paths, options)
58-
59-
output[:tags] = tags unless tags.empty? || paths.blank?
60-
output[:paths] = paths unless paths.blank?
61-
output[:definitions] = definitions unless definitions.blank?
62-
63-
output
64-
end
65-
66100
instance_eval(guard) unless guard.nil?
67101

68102
get mount_path do
69103
header['Access-Control-Allow-Origin'] = '*'
70104
header['Access-Control-Request-Method'] = '*'
71105

72-
output_path_definitions.call(target_class.combined_namespace_routes, self)
106+
GrapeSwagger::DocMethods
107+
.output_path_definitions(target_class.combined_namespace_routes, self, target_class, options)
73108
end
74109

75-
desc specific_api_doc.delete(:desc), { params:
76-
specific_api_doc.delete(:params) || {} }.merge(specific_api_doc)
110+
desc specific_api_doc.delete(:desc), { params: specific_api_doc.delete(:params) || {}, **specific_api_doc }
77111

78112
params do
79113
requires :name, type: String, desc: 'Resource name of mounted API'
@@ -88,51 +122,21 @@ def setup(options)
88122
combined_routes = target_class.combined_namespace_routes[params[:name]]
89123
error!({ error: 'named resource not exist' }, 400) if combined_routes.nil?
90124

91-
output_path_definitions.call({ params[:name] => combined_routes }, self)
125+
GrapeSwagger::DocMethods
126+
.output_path_definitions({ params[:name] => combined_routes }, self, target_class, options)
92127
end
93128
end
94129

95-
def defaults
96-
{
97-
info: {},
98-
models: [],
99-
doc_version: '0.0.1',
100-
target_class: nil,
101-
mount_path: '/swagger_doc',
102-
host: nil,
103-
base_path: nil,
104-
add_base_path: false,
105-
add_version: true,
106-
add_root: false,
107-
hide_documentation_path: true,
108-
format: :json,
109-
authorizations: nil,
110-
security_definitions: nil,
111-
security: nil,
112-
api_documentation: { desc: 'Swagger compatible API description' },
113-
specific_api_documentation: { desc: 'Swagger compatible API description for specific API' },
114-
endpoint_auth_wrapper: nil,
115-
swagger_endpoint_guard: nil,
116-
token_owner: nil
117-
}
118-
end
119-
120130
def class_variables_from(options)
121131
@@mount_path = options[:mount_path]
122132
@@class_name = options[:class_name] || options[:mount_path].delete('/')
123133
@@hide_documentation_path = options[:hide_documentation_path]
124134
end
125135

126-
def tags_from(paths, options)
127-
tags = GrapeSwagger::DocMethods::TagNameDescription.build(paths)
136+
def setup_formatter(formatter)
137+
return unless formatter
128138

129-
if options[:tags]
130-
names = options[:tags].map { |t| t[:name] }
131-
tags.reject! { |t| names.include?(t[:name]) }
132-
tags += options[:tags]
133-
end
134-
135-
tags
139+
FORMATTER_METHOD.each { |method| send(method, formatter) }
136140
end
137141
end
138142
end

lib/grape-swagger/endpoint.rb

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,11 @@ def contact_object(infos)
7878
def path_and_definition_objects(namespace_routes, options)
7979
@paths = {}
8080
@definitions = {}
81+
add_definitions_from options[:models]
8182
namespace_routes.each_value do |routes|
8283
path_item(routes, options)
8384
end
8485

85-
add_definitions_from options[:models]
8686
[@paths, @definitions]
8787
end
8888

@@ -207,19 +207,20 @@ def response_object(route, options)
207207

208208
next build_file_response(memo[value[:code]]) if file_response?(value[:model])
209209

210-
response_model = @item
211-
response_model = expose_params_from_model(value[:model]) if value[:model]
212-
213210
if memo.key?(200) && route.request_method == 'DELETE' && value[:model].nil?
214211
memo[204] = memo.delete(200)
215212
value[:code] = 204
213+
next
216214
end
217215

218-
next if value[:code] == 204
219-
next unless !response_model.start_with?('Swagger_doc') && (@definitions[response_model] || value[:model])
216+
# Explicitly request no model with { model: '' }
217+
next if value[:model] == ''
220218

221-
@definitions[response_model][:description] = description_object(route)
219+
response_model = value[:model] ? expose_params_from_model(value[:model]) : @item
220+
next unless @definitions[response_model]
221+
next if response_model.start_with?('Swagger_doc')
222222

223+
@definitions[response_model][:description] ||= "#{response_model} model"
223224
memo[value[:code]][:schema] = build_reference(route, value, response_model, options)
224225
memo[value[:code]][:examples] = value[:examples] if value[:examples]
225226
end

spec/support/model_parsers/entity_parser.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,23 +145,23 @@ class DocumentedHashAndArrayModel < Grape::Entity
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' => 'This returns something' },
148+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'description' => 'ApiError model' },
149149
'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' => 'This returns something' }
150+
'UseItemResponseAsType' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, 'responses' => { '$ref' => '#/definitions/ResponseItem' } }, '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' => 'This returns something' },
156+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } }, 'description' => 'ApiError model' },
157157
'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' => 'This returns something' }
158+
'UseResponse' => { 'type' => 'object', 'properties' => { 'description' => { 'type' => 'string' }, '$responses' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/ResponseItem' } } }, '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' => 'This returns something' }
164+
'ApiError' => { 'type' => 'object', 'properties' => { 'code' => { 'description' => 'status code', 'type' => 'integer', 'format' => 'int32' }, 'message' => { 'description' => 'error message', 'type' => 'string' } }, 'description' => 'ApiError model' }
165165
}
166166
end
167167

@@ -300,7 +300,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
300300
'type' => 'object',
301301
'required' => ['elements'],
302302
'properties' => { 'elements' => { 'type' => 'array', 'items' => { '$ref' => '#/definitions/QueryInputElement' }, 'description' => 'Set of configuration' } },
303-
'description' => 'nested route inside namespace'
303+
'description' => 'QueryInput model'
304304
},
305305
'QueryInputElement' => {
306306
'type' => 'object',
@@ -310,7 +310,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
310310
'ApiError' => {
311311
'type' => 'object',
312312
'properties' => { 'code' => { 'type' => 'integer', 'format' => 'int32', 'description' => 'status code' }, 'message' => { 'type' => 'string', 'description' => 'error message' } },
313-
'description' => 'This gets Things.'
313+
'description' => 'ApiError model'
314314
},
315315
'Something' => {
316316
'type' => 'object',
@@ -320,7 +320,7 @@ class DocumentedHashAndArrayModel < Grape::Entity
320320
'links' => { 'type' => 'array', 'items' => { 'type' => 'link' } },
321321
'others' => { 'type' => 'text' }
322322
},
323-
'description' => 'This gets Things.'
323+
'description' => 'Something model'
324324
}
325325
}
326326
}

spec/support/model_parsers/mock_parser.rb

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class ApiResponse < OpenStruct; end
112112
'description' => "it's a mock"
113113
}
114114
},
115-
'description' => 'This returns something'
115+
'description' => 'ApiError model'
116116
},
117117
'UseItemResponseAsType' => {
118118
'type' => 'object',
@@ -122,7 +122,7 @@ class ApiResponse < OpenStruct; end
122122
'description' => "it's a mock"
123123
}
124124
},
125-
'description' => 'This returns something'
125+
'description' => 'UseItemResponseAsType model'
126126
}
127127
}
128128
end
@@ -137,7 +137,7 @@ class ApiResponse < OpenStruct; end
137137
'description' => "it's a mock"
138138
}
139139
},
140-
'description' => 'This returns something'
140+
'description' => 'UseResponse model'
141141
},
142142
'ApiError' => {
143143
'type' => 'object',
@@ -147,7 +147,7 @@ class ApiResponse < OpenStruct; end
147147
'description' => "it's a mock"
148148
}
149149
},
150-
'description' => 'This returns something'
150+
'description' => 'ApiError model'
151151
}
152152
}
153153
end
@@ -162,7 +162,7 @@ class ApiResponse < OpenStruct; end
162162
'description' => "it's a mock"
163163
}
164164
},
165-
'description' => 'This returns something'
165+
'description' => 'ApiError model'
166166
}
167167
}
168168
end
@@ -296,7 +296,7 @@ class ApiResponse < OpenStruct; end
296296
'description' => "it's a mock"
297297
}
298298
},
299-
'description' => 'nested route inside namespace'
299+
'description' => 'QueryInput model'
300300
},
301301
'ApiError' => {
302302
'type' => 'object',
@@ -306,7 +306,7 @@ class ApiResponse < OpenStruct; end
306306
'description' => "it's a mock"
307307
}
308308
},
309-
'description' => 'This gets Things.'
309+
'description' => 'ApiError model'
310310
},
311311
'Something' => {
312312
'type' => 'object',
@@ -316,7 +316,7 @@ class ApiResponse < OpenStruct; end
316316
'description' => "it's a mock"
317317
}
318318
},
319-
'description' => 'This gets Things.'
319+
'description' => 'Something model'
320320
}
321321
}
322322
}

0 commit comments

Comments
 (0)