Skip to content

Commit 65031c4

Browse files
hoffmaenhoffmaen
committed
Fix all tests, except manifest related tests
1 parent 9c630be commit 65031c4

File tree

10 files changed

+68
-45
lines changed

10 files changed

+68
-45
lines changed

‎app/actions/route_update.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ class RouteUpdate
33
def update(route:, message:)
44
Route.db.transaction do
55
if message.requested?(:options)
6-
route.options = if route.options.nil?
7-
message.options
6+
route.options = if message.options.nil?
7+
{}
88
else
99
route.options.symbolize_keys.merge(message.options).compact
1010
end

‎app/messages/route_create_message.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@ class RouteCreateMessage < MetadataBaseMessage
1414
options
1515
]
1616

17+
def self.options_requested?
18+
@options_requested ||= proc { |a| a.requested?(:options) }
19+
end
20+
1721
validates :host,
1822
allow_nil: true,
1923
string: true,
@@ -58,7 +62,7 @@ class RouteCreateMessage < MetadataBaseMessage
5862

5963
validates_with NoAdditionalKeysValidator
6064
validates_with RelationshipValidator
61-
validates_with OptionsValidator
65+
validates_with OptionsValidator, if: options_requested?
6266

6367
delegate :space_guid, to: :relationships_message
6468
delegate :domain_guid, to: :relationships_message

‎app/messages/validators.rb‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -242,15 +242,12 @@ def validate(record)
242242

243243
class OptionsValidator < ActiveModel::Validator
244244
def validate(record)
245-
# If the options key is provided, it must not be nil
246-
if record.respond_to?(:options) && record.options.nil?
247-
record.errors.add(:options, message: "'options' must not be explicitly set to null")
245+
if record.options.blank?
246+
# Route Creation with explicit 'null' options is not allowed
247+
record.errors.add(:options, message: 'is not a valid object') if record.is_a?(VCAP::CloudController::RouteCreateMessage) && record.options.nil?
248248
return
249249
end
250250

251-
# Empty option hashes are allowed, so we skip further validation
252-
record.options.blank? && return
253-
254251
unless record.options.is_a?(Hash)
255252
record.errors.add(:options, message: "'options' is not a valid object")
256253
return

‎spec/request/app_manifests_spec.rb‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,13 @@
9797
'routes' => [
9898
{
9999
'route' => "#{route.host}.#{route.domain.name}",
100-
'protocol' => 'http1'
100+
'protocol' => 'http1',
101+
'options' => {}
101102
},
102103
{
103104
'route' => "#{second_route.host}.#{second_route.domain.name}/path",
104-
'protocol' => 'http1'
105+
'protocol' => 'http1',
106+
'options' => {}
105107
}
106108
],
107109
'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } },
@@ -201,11 +203,13 @@
201203
'routes' => [
202204
{
203205
'route' => "#{route.host}.#{route.domain.name}",
204-
'protocol' => 'http1'
206+
'protocol' => 'http1',
207+
'options' => {}
205208
},
206209
{
207210
'route' => "#{second_route.host}.#{second_route.domain.name}/path",
208-
'protocol' => 'http1'
211+
'protocol' => 'http1',
212+
'options' => {}
209213
}
210214
],
211215
'metadata' => { 'labels' => { 'potato' => 'idaho' }, 'annotations' => { 'style' => 'mashed' } },
@@ -273,11 +277,13 @@
273277
'routes' => [
274278
{
275279
'route' => "#{route.host}.#{route.domain.name}",
276-
'protocol' => 'http2'
280+
'protocol' => 'http2',
281+
'options' => {}
277282
},
278283
{
279284
'route' => "#{second_route.host}.#{second_route.domain.name}/path",
280-
'protocol' => 'http1'
285+
'protocol' => 'http1',
286+
'options' => {}
281287
}
282288
]
283289
}

‎spec/request/routes_spec.rb‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,7 @@
8989
labels: {},
9090
annotations: {}
9191
},
92+
options: {},
9293
links: {
9394
self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route_in_org.guid}} },
9495
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route_in_org.space.guid}} },
@@ -121,6 +122,7 @@
121122
labels: {},
122123
annotations: {}
123124
},
125+
options: {},
124126
links: {
125127
self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route_in_other_org.guid}} },
126128
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route_in_other_org.space.guid}} },
@@ -305,6 +307,7 @@
305307
}
306308
}
307309
},
310+
options: {},
308311
links: {
309312
self: { href: "http://api2.vcap.me/v3/routes/#{route1_domain1.guid}" },
310313
space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" },
@@ -416,6 +419,7 @@
416419
}
417420
}
418421
},
422+
options: {},
419423
links: {
420424
self: { href: 'http://api2.vcap.me/v3/routes/route-without-host' },
421425
space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" },
@@ -451,6 +455,7 @@
451455
}
452456
}
453457
},
458+
options: {},
454459
links: {
455460
self: { href: 'http://api2.vcap.me/v3/routes/route-without-host2' },
456461
space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" },
@@ -489,6 +494,7 @@
489494
}
490495
}
491496
},
497+
options: {},
492498
links: {
493499
self: { href: 'http://api2.vcap.me/v3/routes/route-without-path' },
494500
space: { href: "http://api2.vcap.me/v3/spaces/#{space.guid}" },
@@ -977,6 +983,7 @@
977983
labels: {},
978984
annotations: {}
979985
},
986+
options: {},
980987
links: {
981988
self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}} },
982989
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route.space.guid}} },
@@ -1062,6 +1069,7 @@
10621069
labels: {},
10631070
annotations: {}
10641071
},
1072+
options: {},
10651073
links: {
10661074
self: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{UUID_REGEX}} },
10671075
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route.space.guid}} },
@@ -2435,7 +2443,8 @@
24352443
potato: 'russet',
24362444
style: 'fried'
24372445
}
2438-
}
2446+
},
2447+
options: {}
24392448
}
24402449
end
24412450

@@ -2506,7 +2515,8 @@
25062515
potato: 'russet',
25072516
style: 'fried'
25082517
}
2509-
}
2518+
},
2519+
options: {}
25102520
}
25112521
end
25122522
let(:expected_codes_and_responses) do
@@ -3606,7 +3616,8 @@
36063616
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route1.space.guid}} },
36073617
destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route1.guid}/destinations} },
36083618
domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{route1.domain.guid}} }
3609-
}
3619+
},
3620+
options: {}
36103621
}
36113622
end
36123623

@@ -3649,7 +3660,8 @@
36493660
space: { href: %r{#{Regexp.escape(link_prefix)}/v3/spaces/#{route2.space.guid}} },
36503661
destinations: { href: %r{#{Regexp.escape(link_prefix)}/v3/routes/#{route2.guid}/destinations} },
36513662
domain: { href: %r{#{Regexp.escape(link_prefix)}/v3/domains/#{route2.domain.guid}} }
3652-
}
3663+
},
3664+
options: {}
36533665
}
36543666
end
36553667

‎spec/unit/lib/cloud_controller/diego/protocol/routing_info_spec.rb‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ class Protocol
4848

4949
it 'returns the mapped http routes associated with the app with protocol' do
5050
expected_http = [
51-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http2' },
52-
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' }
51+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http2', 'options' => {} },
52+
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }
5353
]
5454

5555
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -66,8 +66,8 @@ class Protocol
6666
context 'and app has no ports' do
6767
it 'returns the mapped http routes associated with the app with a default of port 8080' do
6868
expected_http = [
69-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' },
70-
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' }
69+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} },
70+
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }
7171
]
7272

7373
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -80,8 +80,8 @@ class Protocol
8080

8181
it 'uses the first port available on the app' do
8282
expected_http = [
83-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 7890, 'protocol' => 'http1' },
84-
{ 'hostname' => route_without_service.uri, 'port' => 7890, 'protocol' => 'http1' }
83+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 7890, 'protocol' => 'http1', 'options' => {} },
84+
{ 'hostname' => route_without_service.uri, 'port' => 7890, 'protocol' => 'http1', 'options' => {} }
8585
]
8686

8787
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -113,8 +113,8 @@ class Protocol
113113

114114
it 'uses 8080 as a default' do
115115
expected_http = [
116-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' },
117-
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' }
116+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} },
117+
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} }
118118
]
119119

120120
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -127,8 +127,8 @@ class Protocol
127127

128128
it 'uses the first docker port available on the app' do
129129
expected_http = [
130-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 1024, 'protocol' => 'http1' },
131-
{ 'hostname' => route_without_service.uri, 'port' => 1024, 'protocol' => 'http1' }
130+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 1024, 'protocol' => 'http1', 'options' => {} },
131+
{ 'hostname' => route_without_service.uri, 'port' => 1024, 'protocol' => 'http1', 'options' => {} }
132132
]
133133

134134
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -154,7 +154,7 @@ class Protocol
154154

155155
it 'returns the app port in routing info' do
156156
expected_http = [
157-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' }
157+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} }
158158
]
159159

160160
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -169,8 +169,8 @@ class Protocol
169169

170170
it 'returns the app port in routing info' do
171171
expected_http = [
172-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1' },
173-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' }
172+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 8080, 'protocol' => 'http1', 'options' => {} },
173+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} }
174174
]
175175

176176
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -185,8 +185,8 @@ class Protocol
185185

186186
it 'returns the app port in routing info' do
187187
expected_http = [
188-
{ 'hostname' => route_without_service.uri, 'port' => 9090, 'protocol' => 'http1' },
189-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' }
188+
{ 'hostname' => route_without_service.uri, 'port' => 9090, 'protocol' => 'http1', 'options' => {} },
189+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} }
190190
]
191191

192192
expect(ri.keys).to match_array %w[http_routes internal_routes]
@@ -201,8 +201,8 @@ class Protocol
201201

202202
it 'returns the app port in routing info' do
203203
expected_http = [
204-
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' },
205-
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1' }
204+
{ 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {} },
205+
{ 'hostname' => route_with_service.uri, 'route_service_url' => route_with_service.route_service_url, 'port' => 9090, 'protocol' => 'http1', 'options' => {} }
206206
]
207207
expect(ri.keys).to match_array %w[http_routes internal_routes]
208208
expect(ri['http_routes']).to match_array expected_http
@@ -216,7 +216,7 @@ class Protocol
216216
let!(:route_mapping2) { RouteMappingModel.make(app: process2.app, route: route_without_service) }
217217

218218
it 'returns only one route without duplications' do
219-
expected_http = { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1' }
219+
expected_http = { 'hostname' => route_without_service.uri, 'port' => 8080, 'protocol' => 'http1', 'options' => {}}
220220

221221
expect(ri.keys).to match_array %w[http_routes internal_routes]
222222
expect(ri['http_routes']).to contain_exactly expected_http
@@ -232,7 +232,7 @@ class Protocol
232232
it 'returns the router group guid in the http routing info' do
233233
expect(ri.keys).to contain_exactly('http_routes', 'internal_routes')
234234
hr = ri['http_routes'][0]
235-
expect(hr.keys).to contain_exactly('router_group_guid', 'port', 'hostname', 'protocol')
235+
expect(hr.keys).to contain_exactly('router_group_guid', 'port', 'hostname', 'protocol', 'options')
236236
expect(hr['router_group_guid']).to eql(domain.router_group_guid)
237237
expect(hr['port']).to eql(http_route.port)
238238
expect(hr['hostname']).to match(/host-[0-9]+\.#{domain.name}/)

‎spec/unit/messages/manifest_routes_update_message_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ module VCAP::CloudController
234234
] }
235235
end
236236

237-
it 'returns true' do
237+
it 'returns false' do
238238
msg = ManifestRoutesUpdateMessage.new(body)
239239

240240
expect(msg.valid?).to be(false)
@@ -268,7 +268,7 @@ module VCAP::CloudController
268268
] }
269269
end
270270

271-
it 'returns true' do
271+
it 'returns false' do
272272
msg = ManifestRoutesUpdateMessage.new(body)
273273

274274
expect(msg.valid?).to be(false)

‎spec/unit/messages/route_update_message_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ module VCAP::CloudController
2828
expect(message).to be_valid
2929
end
3030

31-
it 'does not accept options: nil' do
31+
it 'accepts options: nil to unset options' do
3232
message = RouteUpdateMessage.new(params.merge(options: nil))
33-
expect(message).not_to be_valid
33+
expect(message).to be_valid
3434
end
3535

3636
it 'accepts lb_algo: nil to unset load-balancing algorithm' do

‎spec/unit/presenters/v3/app_manifest_presenter_spec.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ module VCAP::CloudController::Presenters::V3
111111
service_instance2.name
112112
])
113113
expect(application[:routes]).to eq([
114-
{ route: route.uri, protocol: 'http1' },
115-
{ route: route2.uri, protocol: 'http1' }
114+
{ route: route.uri, protocol: 'http1', options: {} },
115+
{ route: route2.uri, protocol: 'http1', options: {} }
116116
])
117117
expect(application[:env]).to match({ 'one' => 'potato', 'two' => 'tomato' })
118118
expect(application[:metadata]).to match({ labels: { 'potato' => 'idaho' }, annotations: { 'style' => 'mashed' } })

‎spec/unit/presenters/v3/route_presenter_spec.rb‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,11 @@ module VCAP::CloudController::Presenters::V3
139139
end
140140

141141
it 'does not output options' do
142-
expect(subject[:options]).to be_nil
142+
k = '5'
143+
expect(k).to eql(k)
144+
expect(k).to eql('5')
145+
expect(5).to eql(5)
146+
expect(subject[:options]).to eq({})
143147
end
144148
end
145149

0 commit comments

Comments
 (0)