v3(services): expose additional service instance fields (cloudfoundry#1720)

The documentation_url and description of the service offering are
available as fields via the service instance endpoints

[#173447953](https://www.pivotaltracker.com/story/show/173447953)

Author: blgm

app/decorators/field_service_instance_offering_decorator.rb

@@ -1,7 +1,7 @@
 module VCAP::CloudController
   class FieldServiceInstanceOfferingDecorator
     def self.allowed
-      Set['name', 'guid', 'relationships.service_broker']
+      Set.new(%w(name guid description documentation_url relationships.service_broker))
     end
 
     def self.match?(fields)
@@ -24,6 +24,8 @@ def decorate(hash, service_instances)
         offering_view = {}
         offering_view[:name] = offering.name if @fields.include?('name')
         offering_view[:guid] = offering.guid if @fields.include?('guid')
+        offering_view[:description] = offering.description if @fields.include?('description')
+        offering_view[:documentation_url] = offering.documentation_url if @fields.include?('documentation_url')
         if @fields.include?('relationships.service_broker')
           offering_view[:relationships] = {
             service_broker: {

app/messages/service_instance_show_message.rb

@@ -5,11 +5,11 @@ class ServiceInstanceShowMessage < BaseMessage
     validates_with NoAdditionalParamsValidator
     validates :fields, allow_nil: true, fields: {
       allowed: {
-        'space' => ['name', 'guid'],
-        'space.organization' => ['name', 'guid'],
-        'service_plan' => ['name', 'guid'],
-        'service_plan.service_offering' => ['name', 'guid'],
-        'service_plan.service_offering.service_broker' => ['name', 'guid']
+        'space' => %w(name guid),
+        'space.organization' => %w(name guid),
+        'service_plan' => %w(name guid),
+        'service_plan.service_offering' => %w(name guid description documentation_url),
+        'service_plan.service_offering.service_broker' => %w(name guid)
       }
     }
 

app/messages/service_instances_list_message.rb

@@ -19,17 +19,17 @@ class ServiceInstancesListMessage < MetadataListMessage
     validates_with NoAdditionalParamsValidator
 
     validates :type, allow_nil: true, inclusion: {
-        in: %w(managed user-provided),
-        message: "must be one of 'managed', 'user-provided'"
-      }
+      in: %w(managed user-provided),
+      message: "must be one of 'managed', 'user-provided'"
+    }
 
     validates :fields, allow_nil: true, fields: {
       allowed: {
-        'space' => ['guid', 'name', 'relationships.organization'],
-        'space.organization' => ['name', 'guid'],
-        'service_plan' => ['guid', 'name', 'relationships.service_offering'],
-        'service_plan.service_offering' => ['name', 'guid', 'relationships.service_broker'],
-        'service_plan.service_offering.service_broker' => ['name', 'guid']
+        '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 relationships.service_broker),
+        'service_plan.service_offering.service_broker' => %w(name guid)
       }
     }
 

docs/v3/source/includes/experimental_resources/service_instances/_get.md.erb

@@ -52,7 +52,7 @@ Resource            | Allowed Keys
 space | `guid`, `name`
 space.organization | `name`, `guid`
 service_plan| `name`, `guid`
-service_plan.service_offering| `name`, `guid`
+service_plan.service_offering| `name`, `guid`, `description`, `documentation_url`
 service_plan.service_offering.service_broker| `name`, `guid`
 
 #### Permitted roles

docs/v3/source/includes/resources/service_instances/_list.md.erb

@@ -50,7 +50,7 @@ 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`, `relationships.service_broker`
+service_plan.service_offering| `guid`, `name`, `description`, `documentation_url`, `relationships.service_broker`
 service_plan.service_offering.service_broker| `guid`, `name`
 
 #### Permitted roles

spec/request/service_instances_spec.rb

@@ -133,15 +133,18 @@
         expect({ included: parsed_response['included'] }).to match_json_response({ included: included })
       end
 
-      it 'can include the offering and broker name and guid fields' do
-        get "/v3/service_instances/#{guid}?fields[service_plan.service_offering]=name,guid&fields[service_plan.service_offering.service_broker]=name,guid", nil, admin_headers
+      it 'can include service offering and broker fields' do
+        get "/v3/service_instances/#{guid}?fields[service_plan.service_offering]=name,guid,description,documentation_url&" \
+           'fields[service_plan.service_offering.service_broker]=name,guid', nil, admin_headers
         expect(last_response).to have_status_code(200)
 
         included = {
           service_offerings: [
             {
               name: instance.service_plan.service.name,
-              guid: instance.service_plan.service.guid
+              guid: instance.service_plan.service.guid,
+              description: instance.service_plan.service.description,
+              documentation_url: instance.service_plan.service.documentation_url,
             }
           ],
           service_brokers: [
@@ -356,9 +359,9 @@ def check_filtered_instances(*instances)
         expect({ included: parsed_response['included'] }).to match_json_response({ included: included })
       end
 
-      it 'can include the service plan, offering and broker name and guid fields' do
+      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,relationships.service_broker&' \
+                'fields[service_plan.service_offering]=name,guid,description,documentation_url,relationships.service_broker&' \
                 'fields[service_plan.service_offering.service_broker]=name,guid', nil, admin_headers
 
         expect(last_response).to have_status_code(200)
@@ -403,6 +406,8 @@ def check_filtered_instances(*instances)
             {
               name: msi_1.service_plan.service.name,
               guid: msi_1.service_plan.service.guid,
+              description: msi_1.service_plan.service.description,
+              documentation_url: msi_1.service_plan.service.documentation_url,
               relationships: {
                 service_broker: {
                   data: {
@@ -414,6 +419,8 @@ def check_filtered_instances(*instances)
             {
               name: msi_2.service_plan.service.name,
               guid: msi_2.service_plan.service.guid,
+              description: msi_2.service_plan.service.description,
+              documentation_url: msi_2.service_plan.service.documentation_url,
               relationships: {
                 service_broker: {
                   data: {
@@ -425,6 +432,8 @@ def check_filtered_instances(*instances)
             {
               name: ssi.service_plan.service.name,
               guid: ssi.service_plan.service.guid,
+              description: ssi.service_plan.service.description,
+              documentation_url: ssi.service_plan.service.documentation_url,
               relationships: {
                 service_broker: {
                   data: {
@@ -2015,7 +2024,7 @@ def check_filtered_instances(*instances)
             expect(last_response).to have_status_code(422)
             expect(parsed_response['errors']).to include(
               include({ 'detail' => 'Cannot update parameters of a service instance that belongs to inaccessible plan' })
-             )
+            )
           end
         end
 

spec/unit/decorators/field_service_instance_offering_decorator_spec.rb

@@ -13,7 +13,7 @@ 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 offering name from service instances' do
+      it 'can decorate with the service offering name' do
         undecorated_hash = { foo: 'bar', included: { monkeys: %w(zach greg) } }
         decorator = described_class.new({ 'service_plan.service_offering': ['name', 'foo'] })
 
@@ -35,7 +35,7 @@ module VCAP::CloudController
         })
       end
 
-      it 'decorated the given hash with offering guids from service instances' do
+      it 'can decorate with the service offering guid' do
         undecorated_hash = { foo: 'bar', included: { monkeys: %w(zach greg) } }
         decorator = described_class.new({ 'service_plan.service_offering': ['guid', 'foo'] })
 
@@ -57,6 +57,50 @@ module VCAP::CloudController
         })
       end
 
+      it 'can decorate with the service offering description' do
+        undecorated_hash = { foo: 'bar', included: { monkeys: %w(zach greg) } }
+        decorator = described_class.new({ 'service_plan.service_offering': ['description', '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: [
+              {
+                description: offering1.description,
+              },
+              {
+                description: offering2.description,
+              }
+            ]
+          }
+        })
+      end
+
+      it 'can decorate with the service offering documentation' do
+        undecorated_hash = { foo: 'bar', included: { monkeys: %w(zach greg) } }
+        decorator = described_class.new({ 'service_plan.service_offering': ['documentation_url', '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: [
+              {
+                documentation_url: offering1.documentation_url,
+              },
+              {
+                documentation_url: offering2.documentation_url,
+              }
+            ]
+          }
+        })
+      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'] })
@@ -116,20 +160,16 @@ module VCAP::CloudController
     end
 
     describe '.match?' do
-      it 'matches hashes containing key symbol `service_plan.service_offering` and value `name`' do
-        expect(described_class.match?({ 'service_plan.service_offering': ['name'], other: ['bar'] })).to be_truthy
-      end
+      fields = %w(name guid description documentation_url relationships.service_broker)
 
-      it 'matches hashes containing key symbol `service_plan.service_offering` and value `guid`' do
-        expect(described_class.match?({ 'service_plan.service_offering': ['guid'], other: ['bar'] })).to be_truthy
-      end
-
-      it 'matches hashes containing key symbol `service_plan.service_offering` and value `relationships.service_broker`' do
-        expect(described_class.match?({ 'service_plan.service_offering': ['relationships.service_broker'], other: ['bar'] })).to be_truthy
+      fields.each do |field|
+        it "matches value `#{field}` for key symbol `service_plan.service_offering`" do
+          expect(described_class.match?({ 'service_plan.service_offering': [field], other: ['bar'] })).to be_truthy
+        end
       end
 
-      it 'matches hashes containing key symbol `service_plan.service_offering` and value `name,guid,relationships.service_broker`' do
-        expect(described_class.match?({ 'service_plan.service_offering': ['name', 'guid', 'relationships.service_broker', 'something'], other: ['bar'] })).to be_truthy
+      it 'matches all fields together for key symbol `service_plan.service_offering`' do
+        expect(described_class.match?({ 'service_plan.service_offering': fields, other: ['bar'] })).to be_truthy
       end
 
       it 'does not match other values for a valid key' do

spec/unit/messages/service_instance_show_message_spec.rb

@@ -10,7 +10,7 @@ module VCAP::CloudController
 
     it_behaves_like 'field query parameter', 'service_plan', 'name,guid'
 
-    it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid'
+    it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,documentation_url'
 
     it_behaves_like 'field query parameter', 'service_plan.service_offering.service_broker', 'name,guid'
 

spec/unit/messages/service_instances_list_message_spec.rb

@@ -6,9 +6,9 @@ module VCAP::CloudController
   RSpec.describe ServiceInstancesListMessage do
     let(:params) do
       {
-        'page'      => 1,
-        'per_page'  => 5,
-        'order_by'  => 'name',
+        'page' => 1,
+        'per_page' => 5,
+        'order_by' => 'name',
         'names' => 'rabbitmq, redis,mysql',
         'space_guids' => 'space-1, space-2, space-3',
         'label_selector' => 'key=value',
@@ -83,7 +83,7 @@ module VCAP::CloudController
 
         it_behaves_like 'field query parameter', 'service_plan', 'guid,name,relationships.service_offering'
 
-        it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,relationships.service_broker'
+        it_behaves_like 'field query parameter', 'service_plan.service_offering', 'name,guid,description,documentation_url,relationships.service_broker'
 
         it_behaves_like 'field query parameter', 'service_plan.service_offering.service_broker', 'name,guid'
       end