Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add field to include broker catalog unique id to service instances #4045

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions app/decorators/field_service_instance_offering_decorator.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module VCAP::CloudController
class FieldServiceInstanceOfferingDecorator
def self.allowed
Set.new(%w[name guid description documentation_url tags relationships.service_broker])
Set.new(%w[name guid description documentation_url tags broker_catalog.id relationships.service_broker])
end

def self.match?(fields)
Expand All @@ -16,14 +16,7 @@ def decorate(hash, service_instances)
managed_service_instances = service_instances.select(&:managed_instance?)
return hash if managed_service_instances.empty?

offerings = Service.
join(:service_plans, service_id: :services__id).
join(:service_instances, service_plan_id: :service_plans__id).
where(service_instances__id: managed_service_instances.map(&:id)).
distinct.
order_by(:services__created_at, :services__guid).
select(:services__label, :services__guid, :services__description, :services__tags, :services__extra, :services__service_broker_id, :services__created_at).
all
offerings = service_offerings(managed_service_instances)

hash[:included] ||= {}
hash[:included][:service_offerings] = offerings.map do |offering|
Expand All @@ -33,6 +26,13 @@ def decorate(hash, service_instances)
offering_view[:description] = offering.description if @fields.include?('description')
offering_view[:tags] = offering.tags if @fields.include?('tags')
offering_view[:documentation_url] = extract_documentation_url(offering.extra) if @fields.include?('documentation_url')

if @fields.include?('broker_catalog.id')
offering_view[:broker_catalog] = {
id: offering.unique_id
}
end

if @fields.include?('relationships.service_broker')
offering_view[:relationships] = {
service_broker: {
Expand All @@ -51,6 +51,17 @@ def decorate(hash, service_instances)

private

def service_offerings(managed_service_instances)
Service.join(:service_plans, service_id: :services__id).
join(:service_instances, service_plan_id: :service_plans__id).
where(service_instances__id: managed_service_instances.map(&:id)).
distinct.
order_by(:services__created_at, :services__guid).
select(:services__label, :services__guid, :services__description, :services__tags, :services__extra,
:services__service_broker_id, :services__created_at, :services__unique_id).
all
end

def extract_documentation_url(extra)
metadata = Oj.load(extra)
metadata['documentationUrl']
Expand Down
7 changes: 6 additions & 1 deletion app/decorators/field_service_instance_plan_decorator.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module VCAP::CloudController
class FieldServiceInstancePlanDecorator
def self.allowed
Set['guid', 'name', 'relationships.service_offering']
Set['guid', 'name', 'broker_catalog.id', 'relationships.service_offering']
end

def self.match?(fields)
Expand All @@ -23,6 +23,11 @@ def decorate(hash, service_instances)
plan_view = {}
plan_view[:guid] = plan.guid if @fields.include?('guid')
plan_view[:name] = plan.name if @fields.include?('name')
if @fields.include?('broker_catalog.id')
plan_view[:broker_catalog] = {
id: plan.unique_id
}
end
if @fields.include?('relationships.service_offering')
plan_view[:relationships] = {
service_offering: {
Expand Down
4 changes: 2 additions & 2 deletions app/messages/service_instance_show_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ class ServiceInstanceShowMessage < BaseMessage
allowed: {
'space' => %w[name guid],
'space.organization' => %w[name guid],
'service_plan' => %w[name guid],
'service_plan.service_offering' => %w[name guid description tags documentation_url],
'service_plan' => %w[name guid broker_catalog.id],
'service_plan.service_offering' => %w[name guid description tags documentation_url broker_catalog.id],
'service_plan.service_offering.service_broker' => %w[name guid]
}
}
Expand Down
4 changes: 2 additions & 2 deletions app/messages/service_instances_list_message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ class ServiceInstancesListMessage < MetadataListMessage
allowed: {
'space' => %w[guid name relationships.organization],
'space.organization' => %w[name guid],
'service_plan' => %w[guid name relationships.service_offering],
'service_plan.service_offering' => %w[name guid description documentation_url tags relationships.service_broker],
'service_plan' => %w[guid name broker_catalog.id relationships.service_offering],
'service_plan.service_offering' => %w[name guid description documentation_url tags broker_catalog.id relationships.service_broker],
'service_plan.service_offering.service_broker' => %w[name guid]
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Resource | Allowed Keys
------------------- | ----
space | `guid`, `name`
space.organization | `name`, `guid`
service_plan| `name`, `guid`
service_plan.service_offering| `name`, `guid`, `description`, `documentation_url`, `tags`
service_plan| `name`, `guid`, `broker_catalog.id`
service_plan.service_offering| `name`, `guid`, `description`, `documentation_url`, `tags`, `broker_catalog.id`
service_plan.service_offering.service_broker| `name`, `guid`

#### Permitted roles
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ Resource | Allowed Keys
------------------- | ----
space | `guid`, `name`, `relationships.organization`
space.organization| `guid`, `name`
service_plan| `guid`, `name`, `relationships.service_offering`
service_plan.service_offering| `guid`, `name`, `description`, `documentation_url`, `tags`, `relationships.service_broker`
service_plan| `guid`, `name`, `broker_catalog.id`, `relationships.service_offering`
service_plan.service_offering| `guid`, `name`, `description`, `documentation_url`, `tags`, `broker_catalog.id`, `relationships.service_broker`
service_plan.service_offering.service_broker| `guid`, `name`

#### Permitted roles
Expand Down
38 changes: 31 additions & 7 deletions spec/request/service_instances_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,19 @@
expect({ included: parsed_response['included'] }).to match_json_response({ included: })
end

it 'can include service plan guid and name fields' do
get "/v3/service_instances/#{guid}?fields[service_plan]=guid,name", nil, admin_headers
it 'can include service plan guid, name and broker catalog fields' do
get "/v3/service_instances/#{guid}?fields[service_plan]=guid,name,broker_catalog.id", nil, admin_headers

expect(last_response).to have_status_code(200)

included = {
service_plans: [
{
guid: instance.service_plan.guid,
name: instance.service_plan.name
name: instance.service_plan.name,
broker_catalog: {
id: instance.service_plan.unique_id
}
}
]
}
Expand All @@ -114,7 +117,7 @@
end

it 'can include service offering and broker fields' do
get "/v3/service_instances/#{guid}?fields[service_plan.service_offering]=name,guid,description,documentation_url&" \
get "/v3/service_instances/#{guid}?fields[service_plan.service_offering]=name,guid,description,documentation_url,broker_catalog.id&" \
'fields[service_plan.service_offering.service_broker]=name,guid', nil, admin_headers
expect(last_response).to have_status_code(200)

Expand All @@ -124,7 +127,10 @@
name: instance.service_plan.service.name,
guid: instance.service_plan.service.guid,
description: instance.service_plan.service.description,
documentation_url: 'https://some.url.for.docs/'
documentation_url: 'https://some.url.for.docs/',
broker_catalog: {
id: instance.service_plan.service.unique_id
}
}
],
service_brokers: [
Expand Down Expand Up @@ -386,8 +392,8 @@ def check_filtered_instances(*instances)
end

it 'can include the service plan, offering and broker fields' do
get '/v3/service_instances?fields[service_plan]=guid,name,relationships.service_offering&' \
'fields[service_plan.service_offering]=name,guid,description,documentation_url,relationships.service_broker&' \
get '/v3/service_instances?fields[service_plan]=guid,name,broker_catalog.id,relationships.service_offering&' \
'fields[service_plan.service_offering]=name,guid,description,documentation_url,broker_catalog.id,relationships.service_broker&' \
'fields[service_plan.service_offering.service_broker]=name,guid', nil, admin_headers

expect(last_response).to have_status_code(200)
Expand All @@ -397,6 +403,9 @@ def check_filtered_instances(*instances)
{
guid: msi_1.service_plan.guid,
name: msi_1.service_plan.name,
broker_catalog: {
id: msi_1.service_plan.unique_id
},
relationships: {
service_offering: {
data: {
Expand All @@ -408,6 +417,9 @@ def check_filtered_instances(*instances)
{
guid: msi_2.service_plan.guid,
name: msi_2.service_plan.name,
broker_catalog: {
id: msi_2.service_plan.unique_id
},
relationships: {
service_offering: {
data: {
Expand All @@ -419,6 +431,9 @@ def check_filtered_instances(*instances)
{
guid: ssi.service_plan.guid,
name: ssi.service_plan.name,
broker_catalog: {
id: ssi.service_plan.unique_id
},
relationships: {
service_offering: {
data: {
Expand All @@ -434,6 +449,9 @@ def check_filtered_instances(*instances)
guid: msi_1.service_plan.service.guid,
description: msi_1.service_plan.service.description,
documentation_url: 'https://some.url.for.docs/',
broker_catalog: {
id: msi_1.service_plan.service.unique_id
},
relationships: {
service_broker: {
data: {
Expand All @@ -447,6 +465,9 @@ def check_filtered_instances(*instances)
guid: msi_2.service_plan.service.guid,
description: msi_2.service_plan.service.description,
documentation_url: 'https://some.url.for.docs/',
broker_catalog: {
id: msi_2.service_plan.service.unique_id
},
relationships: {
service_broker: {
data: {
Expand All @@ -460,6 +481,9 @@ def check_filtered_instances(*instances)
guid: ssi.service_plan.service.guid,
description: ssi.service_plan.service.description,
documentation_url: 'https://some.url.for.docs/',
broker_catalog: {
id: ssi.service_plan.service.unique_id
},
relationships: {
service_broker: {
data: {
Expand Down
1 change: 1 addition & 0 deletions spec/support/fakes/blueprints.rb
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,7 @@ module VCAP::CloudController
unique_id { SecureRandom.uuid }
active { true }
maintenance_info {}
unique_id { SecureRandom.uuid }
end

ServicePlan.blueprint(:routing) do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,32 @@ module VCAP::CloudController
})
end

it 'can decorate with the service offering broker_catalog_id' do
undecorated_hash = { foo: 'bar', included: { monkeys: %w[zach greg] } }
decorator = described_class.new({ 'service_plan.service_offering': %w[broker_catalog_id foo] })

hash = decorator.decorate(undecorated_hash, [service_instance_1, service_instance_2])

expect(hash).to match({
foo: 'bar',
included: {
monkeys: %w[zach greg],
service_offerings: [
{
broker_catalog: {
id: offering1.unique_id
}
},
{
broker_catalog: {
id: offering2.unique_id
}
}
]
}
})
end

it 'decorated the given hash with offering relationship to broker from service instances' do
undecorated_hash = { foo: 'bar', included: { monkeys: %w[zach greg] } }
decorator = described_class.new({ 'service_plan.service_offering': ['relationships.service_broker', 'foo'] })
Expand Down Expand Up @@ -182,7 +208,7 @@ module VCAP::CloudController
end

describe '.match?' do
fields = %w[name guid description documentation_url tags relationships.service_broker]
fields = %w[name guid description documentation_url tags broker_catalog.id relationships.service_broker]

fields.each do |field|
it "matches value `#{field}` for key symbol `service_plan.service_offering`" do
Expand Down
16 changes: 11 additions & 5 deletions spec/unit/decorators/field_service_instance_plan_decorator_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ module VCAP::CloudController
let(:service_instance_1) { ManagedServiceInstance.make(service_plan: plan1) }
let(:service_instance_2) { ManagedServiceInstance.make(service_plan: plan2) }

it 'decorated the given hash with plan guid and name from service instances' do
it 'decorated the given hash with plan guid, name and broker_catalog.id from service instances' do
undecorated_hash = { foo: 'bar', included: { monkeys: %w[zach greg] } }
decorator = described_class.new({ service_plan: %w[guid name foo] })
decorator = described_class.new({ service_plan: %w[guid name broker_catalog.id foo] })

hash = decorator.decorate(undecorated_hash, [service_instance_1, service_instance_2])

Expand All @@ -24,11 +24,17 @@ module VCAP::CloudController
service_plans: [
{
guid: plan1.guid,
name: plan1.name
name: plan1.name,
broker_catalog: {
id: plan1.unique_id
}
},
{
guid: plan2.guid,
name: plan2.name
name: plan2.name,
broker_catalog: {
id: plan2.unique_id
}
}
]
}
Expand Down Expand Up @@ -93,7 +99,7 @@ module VCAP::CloudController
end

describe '.match?' do
it_behaves_like 'field decorator match?', 'service_plan', ['name', 'guid', 'relationships.service_offering']
it_behaves_like 'field decorator match?', 'service_plan', ['name', 'guid', 'broker_catalog.id', 'relationships.service_offering']
end
end
end
4 changes: 2 additions & 2 deletions spec/unit/messages/service_instance_show_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ module VCAP::CloudController

it_behaves_like 'field query parameter', 'space.organization', 'name,guid'

it_behaves_like 'field query parameter', 'service_plan', 'name,guid'
it_behaves_like 'field query parameter', 'service_plan', 'name,guid,broker_catalog.id'

it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,tags,documentation_url'
it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,tags,documentation_url,broker_catalog.id'

it_behaves_like 'field query parameter', 'service_plan.service_offering.service_broker', 'name,guid'

Expand Down
4 changes: 2 additions & 2 deletions spec/unit/messages/service_instances_list_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ module VCAP::CloudController

it_behaves_like 'field query parameter', 'space.organization', 'name,guid'

it_behaves_like 'field query parameter', 'service_plan', 'guid,name,relationships.service_offering'
it_behaves_like 'field query parameter', 'service_plan', 'guid,name,broker_catalog.id,relationships.service_offering'

it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,documentation_url,tags,relationships.service_broker'
it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,documentation_url,tags,broker_catalog.id,relationships.service_broker'

it_behaves_like 'field query parameter', 'service_plan.service_offering.service_broker', 'name,guid'
end
Expand Down
Loading