Get guid,name of service broker in service plan list request

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

Author: pivotal-marcela-campo

app/controllers/v3/service_plans_controller.rb

@@ -8,6 +8,7 @@
 require 'actions/transactional_metadata_update'
 require 'decorators/include_service_plan_space_organization_decorator'
 require 'decorators/include_service_plan_service_offering_decorator'
+require 'decorators/field_service_plan_service_broker_decorator'
 
 class ServicePlansController < ApplicationController
   include ServicePermissions
@@ -44,6 +45,7 @@ def index
     decorators = []
     decorators << IncludeServicePlanSpaceOrganizationDecorator if IncludeServicePlanSpaceOrganizationDecorator.match?(message.include)
     decorators << IncludeServicePlanServiceOfferingDecorator if IncludeServicePlanServiceOfferingDecorator.match?(message.include)
+    decorators << FieldServicePlanServiceBrokerDecorator.new(message.fields) if FieldServicePlanServiceBrokerDecorator.match?(message.fields)
 
     presenter = Presenters::V3::PaginatedListPresenter.new(
       presenter: Presenters::V3::ServicePlanPresenter,

app/decorators/field_service_plan_service_broker_decorator.rb

@@ -0,0 +1,29 @@
+module VCAP::CloudController
+  class FieldServicePlanServiceBrokerDecorator
+    def self.allowed
+      Set['name', 'guid']
+    end
+
+    def self.match?(fields)
+      fields.is_a?(Hash) && fields[:'service_offering.service_broker']&.to_set&.intersect?(self.allowed)
+    end
+
+    def initialize(fields)
+      @fields = fields[:'service_offering.service_broker'].to_set.intersection(self.class.allowed)
+    end
+
+    def decorate(hash, service_plans)
+      hash[:included] ||= {}
+      service_offerings = service_plans.map(&:service).uniq
+      service_brokers = service_offerings.map(&:service_broker).uniq
+      hash[:included][:service_brokers] = service_brokers.sort_by(&:created_at).map do |broker|
+        broker_view = {}
+        broker_view[:name] = broker.name if @fields.include?('name')
+        broker_view[:guid] = broker.guid if @fields.include?('guid')
+        broker_view
+      end
+
+      hash
+    end
+  end
+end

app/messages/service_plans_list_message.rb

@@ -17,6 +17,7 @@ class ServicePlansListMessage < MetadataListMessage
     ]
     @single_keys = [
       :available,
+      :fields
     ]
 
     register_allowed_keys(@single_keys + @array_keys)
@@ -25,8 +26,14 @@ class ServicePlansListMessage < MetadataListMessage
     validates_with IncludeParamValidator, valid_values: %w(space.organization service_offering)
     validates :available, inclusion: { in: %w(true false), message: "only accepts values 'true' or 'false'" }, allow_nil: true
 
+    validates :fields, allow_nil: true, fields: {
+      allowed: {
+        'service_offering.service_broker' => ['guid', 'name']
+      }
+    }
+
     def self.from_params(params)
-      super(params, @array_keys.map(&:to_s))
+      super(params, @array_keys.map(&:to_s), fields: %w(fields))
     end
 
     def available?

docs/v3/source/includes/experimental_resources/service_plans/_list.md.erb

@@ -42,12 +42,21 @@ Name | Type | Description
 **service_instance_guids** | _list of strings_ | Comma-delimited list of Service Instance GUIDs to filter by.
 **label_selector** | _string_ | **Experimental** - A query string containing a list of [label selector](#labels-and-selectors) requirements.
 **include** | _list of strings_ | Optionally include a list of unique related resources in the response. <br>Valid values are `space.organization` and `service_offering`.
+**fields** | [_fields parameter_](#fields-parameter) | **Experimental** [_Allowed values_](#service-plans-list-fields)
 
 
 The **organization_guids** and **space_guids** filters do not filter plans that are public.
 They both act on plans that are restricted to certain Organizations, and to plans from Space-Scoped
 Service Brokers.
 
+
+##### Service Plans List Fields
+
+Resource            | Allowed Keys
+------------------- | ----
+service_offering.service_broker| `guid`, `name`
+
+
 #### Permitted roles
  |
 --- | ---

spec/field_decorator_spec_shared_examples.rb

@@ -0,0 +1,23 @@
+RSpec.shared_examples 'field decorator match?' do |resource, keys|
+  keys.each do |key|
+    it "matches hashes containing resource symbol `#{resource}` and value `#{key}`" do
+      expect(described_class.match?({ "#{resource}": [key.to_s], other: ['bar'] })).to be_truthy
+    end
+  end
+
+  it "matches hashes containing resource symbol `#{resource}` and all valid keys" do
+    expect(described_class.match?({ "#{resource}": keys, other: ['bar'] })).to be_truthy
+  end
+
+  it 'does not match other values for a valid resource' do
+    expect(described_class.match?({ "#{resource}": ['foo'] })).to be_falsey
+  end
+
+  it 'does not match other resource values' do
+    expect(described_class.match?({ other: ['bar'] })).to be_falsey
+  end
+
+  it 'does not match non-hashes' do
+    expect(described_class.match?('foo')).to be_falsey
+  end
+end

spec/request/service_plans_spec.rb

@@ -153,6 +153,7 @@
           page: 2,
           order_by: 'updated_at',
           label_selector: 'foo==bar',
+          fields: { 'service_offering.service_broker' => 'name' }
         }
       end
     end
@@ -476,6 +477,22 @@
         expect(parsed_response['included']['service_offerings'][1]['guid']).to eq(offering_2.guid)
       end
     end
+
+    describe 'fields' do
+      let!(:plan_1) { VCAP::CloudController::ServicePlan.make }
+      let!(:plan_2) { VCAP::CloudController::ServicePlan.make }
+
+      it 'can include service broker name and guid' do
+        get '/v3/service_plans?fields[service_offering.service_broker]=name,guid', nil, admin_headers
+        expect(last_response).to have_status_code(200)
+
+        expect(parsed_response['included']['service_brokers']).to have(2).elements
+        expect(parsed_response['included']['service_brokers'][0]['guid']).to eq(plan_1.service.service_broker.guid)
+        expect(parsed_response['included']['service_brokers'][0]['name']).to eq(plan_1.service.service_broker.name)
+        expect(parsed_response['included']['service_brokers'][1]['guid']).to eq(plan_2.service.service_broker.guid)
+        expect(parsed_response['included']['service_brokers'][1]['name']).to eq(plan_2.service.service_broker.name)
+      end
+    end
   end
 
   describe 'DELETE /v3/service_plans/:guid' do

spec/unit/decorators/field_service_plan_service_broker_decorator_spec.rb

@@ -0,0 +1,53 @@
+require 'spec_helper'
+require 'decorators/field_service_plan_service_broker_decorator'
+require 'field_decorator_spec_shared_examples'
+
+module VCAP::CloudController
+  RSpec.describe FieldServicePlanServiceBrokerDecorator do
+    describe '.decorate' do
+      let(:offering1) { Service.make }
+      let(:offering2) { Service.make }
+
+      let(:plan1) { ServicePlan.make(service: offering1) }
+      let(:plan2) { ServicePlan.make(service: offering2) }
+
+      it 'decorated the given hash with broker name and guid' do
+        undecorated_hash = { foo: 'bar', included: { monkeys: %w(zach greg) } }
+        decorator = described_class.new({ 'service_offering.service_broker': ['name', 'guid'] })
+
+        hash = decorator.decorate(undecorated_hash, [plan1, plan2])
+
+        expect(hash).to match({
+          foo: 'bar',
+          included: {
+            monkeys: %w(zach greg),
+            service_brokers: [
+              {
+                guid: plan1.service.service_broker.guid,
+                name: plan1.service.service_broker.name
+              },
+              {
+                guid: plan2.service.service_broker.guid,
+                name: plan2.service.service_broker.name
+              }
+            ]
+          }
+        })
+      end
+
+      context 'when plans are from the same broker' do
+        let(:plan3) { ServicePlan.make(service: offering1) }
+
+        it 'does not duplicate the broker' do
+          decorator = described_class.new({ 'service_offering.service_broker': ['name'] })
+          hash = decorator.decorate({}, [plan1, plan3])
+          expect(hash[:included][:service_brokers]).to have(1).element
+        end
+      end
+    end
+
+    describe '.match?' do
+      it_behaves_like 'field decorator match?', 'service_offering.service_broker', ['name', 'guid']
+    end
+  end
+end

spec/unit/messages/service_plans_list_message_spec.rb

@@ -1,5 +1,6 @@
 require 'spec_helper'
 require 'messages/service_plans_list_message'
+require 'field_message_spec_shared_examples'
 
 module VCAP::CloudController
   RSpec.describe ServicePlansListMessage do
@@ -17,11 +18,12 @@ module VCAP::CloudController
           'service_offering_guids' => 'offering_guid_1,offering_guid_2',
           'service_offering_names' => 'offering_name_1,offering_name_2',
           'space_guids' => 'space_guid_1,space_guid_2',
+          'fields' => { 'service_offering.service_broker' => 'guid,name' },
         }.with_indifferent_access
       end
 
       it 'returns the correct ServicePlansListMessage' do
-        message = ServicePlansListMessage.from_params(params)
+        message = described_class.from_params(params)
 
         expect(message).to be_valid
         expect(message).to be_a(ServicePlansListMessage)
@@ -36,6 +38,7 @@ module VCAP::CloudController
         expect(message.service_offering_guids).to contain_exactly('offering_guid_1', 'offering_guid_2')
         expect(message.service_offering_names).to contain_exactly('offering_name_1', 'offering_name_2')
         expect(message.space_guids).to contain_exactly('space_guid_1', 'space_guid_2')
+        expect(message.fields).to match({ 'service_offering.service_broker': ['guid', 'name'] })
       end
 
       it 'converts requested keys to symbols' do
@@ -55,55 +58,73 @@ module VCAP::CloudController
       end
 
       it 'accepts an empty set' do
-        message = ServicePlansListMessage.from_params({})
+        message = described_class.from_params({})
         expect(message).to be_valid
       end
 
       it 'does not accept arbitrary fields' do
-        message = ServicePlansListMessage.from_params({ foobar: 'pants' }.with_indifferent_access)
+        message = described_class.from_params({ foobar: 'pants' }.with_indifferent_access)
 
         expect(message).not_to be_valid
         expect(message.errors[:base][0]).to include("Unknown query parameter(s): 'foobar'")
       end
 
       describe 'available' do
         it 'accepts `true`' do
-          message = ServicePlansListMessage.from_params({ available: 'true' }.with_indifferent_access)
+          message = described_class.from_params({ available: 'true' }.with_indifferent_access)
           expect(message).to be_valid
           expect(message.requested?(:available)).to be_truthy
           expect(message.available).to eq('true')
           expect(message.available?).to be(true)
         end
 
         it 'accepts `false`' do
-          message = ServicePlansListMessage.from_params({ available: 'false' }.with_indifferent_access)
+          message = described_class.from_params({ available: 'false' }.with_indifferent_access)
           expect(message).to be_valid
           expect(message.requested?(:available)).to be_truthy
           expect(message.available).to eq('false')
           expect(message.available?).to be(false)
         end
 
         it 'is false by default' do
-          message = ServicePlansListMessage.from_params({})
+          message = described_class.from_params({})
           expect(message).to be_valid
           expect(message.requested?(:available)).to be_falsey
           expect(message.available?).to be(false)
         end
 
         it 'does not accept other values' do
-          message = ServicePlansListMessage.from_params({ available: 'nope' }.with_indifferent_access)
+          message = described_class.from_params({ available: 'nope' }.with_indifferent_access)
           expect(message).not_to be_valid
           expect(message.errors[:available]).to include("only accepts values 'true' or 'false'")
         end
       end
 
       describe 'include' do
         it 'does not accept other values' do
-          message = ServicePlansListMessage.from_params({ include: 'space' }.with_indifferent_access)
+          message = described_class.from_params({ include: 'space' }.with_indifferent_access)
           expect(message).not_to be_valid
           expect(message.errors[:base]).to include(include("Invalid included resource: 'space'"))
         end
       end
+
+      context 'fields' do
+        it 'validates `fields` is a hash' do
+          message = described_class.from_params({ 'fields' => 'foo' }.with_indifferent_access)
+          expect(message).not_to be_valid
+          expect(message.errors[:fields][0]).to include('must be an object')
+        end
+
+        it_behaves_like 'field query parameter', 'service_offering.service_broker', 'guid,name'
+
+        it 'does not accept fields resources that are not allowed' do
+          message = described_class.from_params({ 'fields' => { 'space.foo': 'name' } })
+          expect(message).not_to be_valid
+          expect(message.errors[:fields]).to include(
+            "[space.foo] valid resources are: 'service_offering.service_broker'"
+          )
+        end
+      end
     end
   end
 end