Skip to content

Commit 565f8e9

Browse files
BhacazBhacaz
authored
Don't overwrite model description with the route destription. (#804)
1 parent 588579d commit 565f8e9

File tree

10 files changed

+159
-97
lines changed

10 files changed

+159
-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: 65 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,61 @@
1717

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

2883
def setup(options)
29-
options = defaults.merge(options)
84+
options = DEFAULTS.merge(options)
3085

3186
# options could be set on #add_swagger_documentation call,
3287
# for available options see #defaults
3388
target_class = options[:target_class]
3489
guard = options[:swagger_endpoint_guard]
35-
formatter = options[:format]
3690
api_doc = options[:api_documentation].dup
3791
specific_api_doc = options[:specific_api_documentation].dup
3892

3993
class_variables_from(options)
4094

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

4797
desc api_doc.delete(:desc), api_doc
4898

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-
6699
instance_eval(guard) unless guard.nil?
67100

68101
get mount_path do
69102
header['Access-Control-Allow-Origin'] = '*'
70103
header['Access-Control-Request-Method'] = '*'
71104

72-
output_path_definitions.call(target_class.combined_namespace_routes, self)
105+
GrapeSwagger::DocMethods
106+
.output_path_definitions(target_class.combined_namespace_routes, self, target_class, options)
73107
end
74108

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

78111
params do
79112
requires :name, type: String, desc: 'Resource name of mounted API'
@@ -88,51 +121,21 @@ def setup(options)
88121
combined_routes = target_class.combined_namespace_routes[params[:name]]
89122
error!({ error: 'named resource not exist' }, 400) if combined_routes.nil?
90123

91-
output_path_definitions.call({ params[:name] => combined_routes }, self)
124+
GrapeSwagger::DocMethods
125+
.output_path_definitions({ params[:name] => combined_routes }, self, target_class, options)
92126
end
93127
end
94128

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-
120129
def class_variables_from(options)
121130
@@mount_path = options[:mount_path]
122131
@@class_name = options[:class_name] || options[:mount_path].delete('/')
123132
@@hide_documentation_path = options[:hide_documentation_path]
124133
end
125134

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

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
138+
FORMATTER_METHOD.each { |method| send(method, formatter) }
136139
end
137140
end
138141
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)