Add field to include broker catalog unique id to service instances

Author: Samze

Diff for: app/decorators/field_service_instance_offering_decorator.rb

@@ -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)
@@ -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(:services__created_at, :services__guid).
-                  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|
@@ -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: {
@@ -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(:services__created_at, :services__guid).
+        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']

Diff for: app/decorators/field_service_instance_plan_decorator.rb

@@ -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)
@@ -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: {

Diff for: app/messages/service_instance_show_message.rb

@@ -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]
       }
     }

Diff for: app/messages/service_instances_list_message.rb

@@ -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]
       }
     }

Diff for: docs/v3/source/includes/resources/service_instances/_get.md.erb

@@ -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

Diff for: docs/v3/source/includes/resources/service_instances/_list.md.erb

@@ -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

Diff for: spec/request/service_instances_spec.rb

@@ -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
+              }
             }
           ]
         }
@@ -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)
 
@@ -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: [
@@ -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)
@@ -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: {
@@ -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: {
@@ -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: {
@@ -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: {
@@ -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: {
@@ -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: {

Diff for: spec/support/fakes/blueprints.rb

@@ -604,6 +604,7 @@ module VCAP::CloudController
     unique_id         { SecureRandom.uuid }
     active            { true }
     maintenance_info  {}
+    unique_id { SecureRandom.uuid }
   end
 
   ServicePlan.blueprint(:routing) do

Diff for: spec/unit/decorators/field_service_instance_offering_decorator_spec.rb

@@ -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'] })
@@ -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

Diff for: spec/unit/decorators/field_service_instance_plan_decorator_spec.rb

@@ -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])
 
@@ -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
+                                      }
                                     }
                                   ]
                                 }
@@ -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

Diff for: spec/unit/messages/service_instance_show_message_spec.rb

@@ -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'
 

Diff for: spec/unit/messages/service_instances_list_message_spec.rb

@@ -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