Skip to content

Commit dbfa5a5

Browse files
LeFnordLeFnord
committed
force usage of entities for response definition (ruby-grape#406)
1 parent 3f212a8 commit dbfa5a5

11 files changed

+153
-261
lines changed

‎CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
#### Features
44

5+
* [#406](https://github.com/ruby-grape/grape-swagger/pull/406): force usage of entities for response definition [issue #385](https://github.com/ruby-grape/grape-swagger/issues/385) - [@LeFnord](https://github.com/LeFnord).
6+
57
#### Fixes
68

79
* [#405](https://github.com/ruby-grape/grape-swagger/pull/405): corrects documentation of versions [issue #403](https://github.com/ruby-grape/grape-swagger/issues/403) - [@LeFnord](https://github.com/LeFnord).

‎README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
[![Gem Version](https://badge.fury.io/rb/grape-swagger.svg)](http://badge.fury.io/rb/grape-swagger)
2-
[![Build Status](https://travis-ci.org/ruby-grape/grape-swagger.svg?branch=swagger-2.0)](https://travis-ci.org/ruby-grape/grape-swagger)
2+
[![Build Status](https://travis-ci.org/ruby-grape/grape-swagger.svg?branch=master)](https://travis-ci.org/ruby-grape/grape-swagger)
33
[![Dependency Status](https://gemnasium.com/ruby-grape/grape-swagger.svg)](https://gemnasium.com/ruby-grape/grape-swagger)
44
[![Code Climate](https://codeclimate.com/github/ruby-grape/grape-swagger.svg)](https://codeclimate.com/github/ruby-grape/grape-swagger)
55

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

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@ class MoveParams
44
class << self
55
def to_definition(paths, definitions)
66
@definitions = definitions
7-
find_post_put(paths) do |path|
8-
find_definition_and_params(path)
7+
8+
find_post_put(paths) do |method_definition|
9+
verb = method_definition.keys.first
10+
method_object = method_definition[verb]
11+
12+
find_definition_and_params(method_object, verb)
913
end
1014
end
1115

@@ -16,25 +20,29 @@ def find_post_put(paths)
1620
end
1721
end
1822

19-
def find_definition_and_params(path)
20-
path.keys.each do |verb|
21-
params = path[verb][:parameters]
23+
def find_definition_and_params(path, verb)
24+
params = path[:parameters]
2225

23-
next if params.nil?
24-
next unless should_move?(params)
26+
return if params.nil?
27+
return unless should_move?(params)
2528

26-
unify!(params)
29+
unify!(params)
2730

28-
status_code = GrapeSwagger::DocMethods::StatusCodes.get[verb.to_sym][:code]
29-
response = path[verb][:responses][status_code]
30-
referenced_definition = parse_model(response[:schema]['$ref'])
31+
status_code = GrapeSwagger::DocMethods::StatusCodes.get[verb.to_sym][:code]
32+
response = path[:responses][status_code]
3133

34+
if response[:schema] && response[:schema]['$ref']
35+
referenced_definition = parse_model(response[:schema]['$ref'])
3236
name = build_definition(referenced_definition, verb)
33-
move_params_to_new(name, params)
34-
35-
@definitions[name][:description] = path[verb][:description]
36-
path[verb][:parameters] << build_body_parameter(response.dup, name)
37+
else
38+
referenced_definition = path[:operationId]
39+
name = build_definition(referenced_definition)
3740
end
41+
42+
move_params_to_new(name, params)
43+
44+
@definitions[name][:description] = path[:description]
45+
path[:parameters] << build_body_parameter(response.dup, name)
3846
end
3947

4048
def move_params_to_new(name, params)
@@ -88,9 +96,10 @@ def nested_definitions(name, params, properties)
8896
private
8997

9098
def build_body_parameter(response, name = false)
99+
entity = response[:schema] ? parse_model(response[:schema]['$ref']) : name
91100
body_param = {}
92101
body_param.tap do |x|
93-
x[:name] = parse_model(response[:schema]['$ref'])
102+
x[:name] = entity
94103
x[:in] = 'body'
95104
x[:required] = true
96105
x[:schema] = { '$ref' => response[:schema]['$ref'] } unless name

‎lib/grape-swagger/endpoint.rb

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,12 @@ def path_item(routes, options)
8989
@item, path = GrapeSwagger::DocMethods::PathString.build(route.route_path, options)
9090
@entity = route.route_entity || route.route_success
9191

92-
method = route.route_method.downcase.to_sym
93-
request_params = method_object(route, options, path)
92+
verb, method_object = method_object(route, options, path)
9493

9594
if @paths.key?(path.to_sym)
96-
@paths[path.to_sym][method] = request_params
95+
@paths[path.to_sym][verb] = method_object
9796
else
98-
@paths[path.to_sym] = { method => request_params }
97+
@paths[path.to_sym] = { verb => method_object }
9998
end
10099

101100
GrapeSwagger::DocMethods::Extensions.add(@paths[path.to_sym], @definitions, route)
@@ -112,6 +111,8 @@ def method_object(route, options, path)
112111
method[:tags] = tag_object(route, options[:version].to_s)
113112
method[:operationId] = GrapeSwagger::DocMethods::OperationId.build(route, path)
114113
method.delete_if { |_, value| value.blank? }
114+
115+
[route.route_method.downcase.to_sym, method]
115116
end
116117

117118
def description_object(route, markdown)
@@ -200,19 +201,7 @@ def partition_params(route)
200201
unless declared_params.nil? && route.route_headers.nil?
201202
request_params = parse_request_params(required)
202203
end
203-
if !exposed.empty?
204-
exposed_params = exposed.each_with_object({}) { |x, memo| memo[x.first] = x.last }
205-
properties = parse_response_params(exposed_params)
206-
else
207-
properties = parse_response_params(required)
208-
end
209204

210-
key = model_name(@entity || @item)
211-
212-
unless properties.empty? || (route.route_method == 'DELETE' && !@entity)
213-
@definitions[key] = { type: 'object', properties: properties } unless @definitions.key?(key)
214-
@definitions[key][:properties].merge!(properties) if @definitions.key?(key)
215-
end
216205
return route.route_params if route.route_params.present? && !route.route_settings[:declared_params].present?
217206
request_params || {}
218207
end

‎spec/lib/move_params_spec.rb

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,8 @@
3939
describe 'find_definition_and_params' do
4040
specify do
4141
subject.instance_variable_set(:@definitions, definitions)
42-
subject.find_definition_and_params(found_path)
43-
44-
expect(definitions.keys).to include 'InBody', 'postRequestInBody'
42+
subject.find_definition_and_params(found_path, :post)
43+
expect(definitions.keys).to include 'InBody'
4544
end
4645
end
4746

‎spec/lib/operation_id_spec.rb

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -21,33 +21,30 @@
2121
let(:method) { :get }
2222
specify { expect(subject.build(route)).to eql 'get' }
2323
end
24-
describe 'GET with path foo' do
25-
let(:method) { 'GET' }
26-
specify { expect(subject.build(route, 'foo')).to eql 'getFoo' }
27-
end
28-
describe 'GET with path /foo' do
29-
let(:method) { 'GET' }
30-
specify { expect(subject.build(route, '/foo')).to eql 'getFoo' }
31-
end
32-
describe 'GET with path bar/foo' do
33-
let(:method) { 'GET' }
34-
specify { expect(subject.build(route, 'bar/foo')).to eql 'getBarFoo' }
35-
end
36-
describe 'GET with path bar/foo{id}' do
37-
let(:method) { 'GET' }
38-
specify { expect(subject.build(route, 'bar/foo{id}')).to eql 'getBarFooId' }
39-
end
40-
describe 'GET with path /bar_foo{id}' do
41-
let(:method) { 'GET' }
42-
specify { expect(subject.build(route, '/bar_foo{id}')).to eql 'getBarFooId' }
43-
end
44-
describe 'GET with path /bar-foo{id}' do
45-
let(:method) { 'GET' }
46-
specify { expect(subject.build(route, '/bar-foo{id}')).to eql 'getBarFooId' }
47-
end
48-
describe 'GET with path /simple_test/bar-foo{id}' do
49-
let(:method) { 'GET' }
50-
specify { expect(subject.build(route, '/simple_test/bar-foo{id}')).to eql 'getSimpleTestBarFooId' }
24+
25+
describe 'path given' do
26+
let(:method) { 'GET' }
27+
it 'GET with path foo' do
28+
expect(subject.build(route, 'foo')).to eql 'getFoo'
29+
end
30+
it 'GET with path /foo' do
31+
expect(subject.build(route, '/foo')).to eql 'getFoo'
32+
end
33+
it 'GET with path bar/foo' do
34+
expect(subject.build(route, 'bar/foo')).to eql 'getBarFoo'
35+
end
36+
it 'GET with path bar/foo{id}' do
37+
expect(subject.build(route, 'bar/foo{id}')).to eql 'getBarFooId'
38+
end
39+
it 'GET with path /bar_foo{id}' do
40+
expect(subject.build(route, '/bar_foo{id}')).to eql 'getBarFooId'
41+
end
42+
it 'GET with path /bar-foo{id}' do
43+
expect(subject.build(route, '/bar-foo{id}')).to eql 'getBarFooId'
44+
end
45+
it 'GET with path /simple_test/bar-foo{id}' do
46+
expect(subject.build(route, '/simple_test/bar-foo{id}')).to eql 'getSimpleTestBarFooId'
47+
end
5148
end
5249
end
5350

‎spec/params_entity_spec.rb

Lines changed: 0 additions & 49 deletions
This file was deleted.

‎spec/support/api_swagger_v2_result.rb

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ class ApiError < Grape::Entity
9090
"operationId"=>"getV3OtherThingElements",
9191
"x-amazon-apigateway-auth"=>{"type"=>"none"},
9292
"x-amazon-apigateway-integration"=>{"type"=>"aws", "uri"=>"foo_bar_uri", "httpMethod"=>"get"}
93-
}
94-
},
93+
}},
9594
"/thing"=>{
9695
"get"=>{
9796
"description"=>"This gets Things.",
@@ -102,7 +101,7 @@ class ApiError < Grape::Entity
102101
{"in"=>"formData", "name"=>"links", "type"=>"array", "items"=>{"type"=>"link"}, "required"=>false},
103102
{"in"=>"query", "name"=>"others", "type"=>"text", "required"=>false}
104103
],
105-
"responses"=>{"200"=>{"description"=>"This gets Things.", "schema"=>{"$ref"=>"#/definitions/Thing"}}, "401"=>{"description"=>"Unauthorized", "schema"=>{"$ref"=>"#/definitions/ApiError"}}},
104+
"responses"=>{"200"=>{"description"=>"This gets Things."}, "401"=>{"description"=>"Unauthorized", "schema"=>{"$ref"=>"#/definitions/ApiError"}}},
106105
"tags"=>["thing"],
107106
"operationId"=>"getThing"
108107
},
@@ -117,14 +116,13 @@ class ApiError < Grape::Entity
117116
"responses"=>{"201"=>{"description"=>"This creates Thing.", "schema"=>{"$ref"=>"#/definitions/Something"}}, "422"=>{"description"=>"Unprocessible Entity"}},
118117
"tags"=>["thing"],
119118
"operationId"=>"postThing"
120-
}
121-
},
119+
}},
122120
"/thing/{id}"=>{
123121
"get"=>{
124122
"description"=>"This gets Thing.",
125123
"produces"=>["application/json"],
126124
"parameters"=>[{"in"=>"path", "name"=>"id", "type"=>"integer", "format"=>"int32", "required"=>true}],
127-
"responses"=>{"200"=>{"description"=>"getting a single thing", "schema"=>{"$ref"=>"#/definitions/Thing"}}, "401"=>{"description"=>"Unauthorized"}},
125+
"responses"=>{"200"=>{"description"=>"getting a single thing"}, "401"=>{"description"=>"Unauthorized"}},
128126
"tags"=>["thing"],
129127
"operationId"=>"getThingId"
130128
},
@@ -148,17 +146,15 @@ class ApiError < Grape::Entity
148146
"responses"=>{"200"=>{"description"=>"This deletes Thing.", "schema"=>{"$ref"=>"#/definitions/Something"}}},
149147
"tags"=>["thing"],
150148
"operationId"=>"deleteThingId"
151-
}
152-
},
149+
}},
153150
"/thing2"=>{
154151
"get"=>{
155152
"description"=>"This gets Things.",
156153
"produces"=>["application/json"],
157154
"responses"=>{"200"=>{"description"=>"get Horses", "schema"=>{"$ref"=>"#/definitions/Something"}}, "401"=>{"description"=>"HorsesOutError", "schema"=>{"$ref"=>"#/definitions/ApiError"}}},
158155
"tags"=>["thing2"],
159156
"operationId"=>"getThing2"
160-
}
161-
},
157+
}},
162158
"/dummy/{id}"=>{
163159
"delete"=>{
164160
"description"=>"dummy route.",
@@ -167,32 +163,32 @@ class ApiError < Grape::Entity
167163
"responses"=>{"204"=>{"description"=>"dummy route."}, "401"=>{"description"=>"Unauthorized"}},
168164
"tags"=>["dummy"],
169165
"operationId"=>"deleteDummyId"
170-
}
171-
}
172-
},
166+
}}},
173167
"definitions"=>{
174168
"QueryInput"=>{
175169
"type"=>"object",
176170
"properties"=>{"elements"=>{"type"=>"array", "items"=>{"$ref"=>"#/definitions/QueryInputElement"}, "description"=>"Set of configuration"}},
177-
"description"=>"nested route inside namespace"},
171+
"description"=>"nested route inside namespace"
172+
},
178173
"QueryInputElement"=>{
179174
"type"=>"object",
180-
"properties"=>{"key"=>{"type"=>"string", "description"=>"Name of parameter"}, "value"=>{"type"=>"string", "description"=>"Value of parameter"}}},
181-
"Thing"=>{
182-
"type"=>"object",
183-
"properties"=>{"id"=>{"type"=>"integer", "format"=>"int32"}, "text"=>{"type"=>"string"}, "links"=>{"type"=>"link"}, "others"=>{"type"=>"text"}},
184-
"description"=>"This gets Thing."},
175+
"properties"=>{"key"=>{"type"=>"string", "description"=>"Name of parameter"}, "value"=>{"type"=>"string", "description"=>"Value of parameter"}}
176+
},
185177
"ApiError"=>{
186178
"type"=>"object",
187179
"properties"=>{"code"=>{"type"=>"integer", "format"=>"int32", "description"=>"status code"}, "message"=>{"type"=>"string", "description"=>"error message"}},
188-
"description"=>"This gets Things."},
180+
"description"=>"This gets Things."
181+
},
189182
"Something"=>{
190183
"type"=>"object",
191-
"properties"=>{"id"=>{"type"=>"integer", "format"=>"int32", "description"=>"Identity of Something"}, "text"=>{"type"=>"string", "description"=>"Content of something."}, "links"=>{"type"=>"link"}, "others"=>{"type"=>"text"}},
184+
"properties"=>{
185+
"id"=>{"type"=>"integer", "format"=>"int32", "description"=>"Identity of Something"},
186+
"text"=>{"type"=>"string", "description"=>"Content of something."},
187+
"links"=>{"type"=>"link"},
188+
"others"=>{"type"=>"text"}
189+
},
192190
"description"=>"This gets Things."
193-
}
194-
}
195-
}
191+
}}}
196192
end
197193

198194
let(:http_verbs) { %w[get post put delete]}

‎spec/swagger_v2/api_swagger_v2_param_type_body_spec.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,12 @@ def app
7171

7272
specify do
7373
expect(subject['paths']['/wo_entities/in_body']['post']['parameters']).to eql([
74-
{"name"=>"InBody", "in"=>"body", "required"=>true, "schema"=>{"$ref"=>"#/definitions/postRequestInBody"}}
74+
{"name"=>"postWoEntitiesInBody", "in"=>"body", "required"=>true, "schema"=>{"$ref"=>"#/definitions/postWoEntitiesInBody"}}
7575
])
7676
end
7777

7878
specify do
79-
expect(subject['definitions']['postRequestInBody']).to eql({
79+
expect(subject['definitions']['postWoEntitiesInBody']).to eql({
8080
"description" => "post in body /wo entity",
8181
"type"=>"object",
8282
"properties"=>{
@@ -91,12 +91,12 @@ def app
9191
specify do
9292
expect(subject['paths']['/wo_entities/in_body/{key}']['put']['parameters']).to eql([
9393
{"in"=>"path", "name"=>"key", "type"=>"integer", "format"=>"int32", "required"=>true},
94-
{"name"=>"InBody", "in"=>"body", "required"=>true, "schema"=>{"$ref"=>"#/definitions/putRequestInBody"}}
94+
{"name"=>"putWoEntitiesInBodyKey", "in"=>"body", "required"=>true, "schema"=>{"$ref"=>"#/definitions/putWoEntitiesInBodyKey"}}
9595
])
9696
end
9797

9898
specify do
99-
expect(subject['definitions']['putRequestInBody']).to eql({
99+
expect(subject['definitions']['putWoEntitiesInBodyKey']).to eql({
100100
"description" => "put in body /wo entity",
101101
"type"=>"object",
102102
"properties"=>{

0 commit comments

Comments
 (0)