v3(bindings): allow the binding app to be included (cloudfoundry#1794)

in the List and Get endpoints

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

Author: Derik Evangelista

app/controllers/v3/service_credential_bindings_controller.rb

@@ -2,6 +2,8 @@
 require 'fetchers/service_credential_binding_list_fetcher'
 require 'presenters/v3/service_credential_binding_presenter'
 require 'messages/service_credential_bindings_list_message'
+require 'messages/service_credential_bindings_show_message'
+require 'decorators/include_binding_app_decorator'
 
 class ServiceCredentialBindingsController < ApplicationController
   def index
@@ -15,18 +17,31 @@ def index
       paginated_result: SequelPaginator.new.get_page(results, pagination_options),
       path: '/v3' + service_credential_bindings_path,
       message: message,
+      decorators: decorators(message)
     )
 
     render status: :ok, json: presenter
   end
 
   def show
+    message = ServiceCredentialBindingsShowMessage.from_params(query_params)
+    invalid_param!(message.errors.full_messages) unless message.valid?
+
     ensure_service_credential_binding_is_accessible!
-    render status: :ok, json: serialized
+
+    render status: :ok, json: serialized(message)
   end
 
   private
 
+  AVAILABLE_DECORATORS = [
+    IncludeBindingAppDecorator
+  ].freeze
+
+  def decorators(message)
+    AVAILABLE_DECORATORS.select { |d| d.match?(message.include) }.reduce([]) { |decorators, d| decorators << d }
+  end
+
   def service_credential_binding
     @service_credential_binding ||= fetcher.fetch(hashed_params[:guid], space_guids: space_guids)
   end
@@ -44,8 +59,8 @@ def pagination_options
     ListMessage.from_params(query_params_with_order_by, []).pagination_options
   end
 
-  def serialized
-    Presenters::V3::ServiceCredentialBindingPresenter.new(service_credential_binding).to_hash
+  def serialized(message)
+    Presenters::V3::ServiceCredentialBindingPresenter.new(service_credential_binding, decorators: decorators(message)).to_hash
   end
 
   def ensure_service_credential_binding_is_accessible!

app/decorators/include_binding_app_decorator.rb

@@ -0,0 +1,23 @@
+module VCAP::CloudController
+  class IncludeBindingAppDecorator
+    class << self
+      def match?(include)
+        include&.any? { |i| %w(app).include?(i) }
+      end
+
+      def decorate(hash, bindings)
+        hash.deep_merge({
+          included: {
+            apps: apps(bindings).map { |app| Presenters::V3::AppPresenter.new(app).to_hash }
+          }
+        })
+      end
+
+      private
+
+      def apps(bindings)
+        AppModel.where(guid: bindings.map { |x| x[:app_guid] }.uniq).order(:created_at)
+      end
+    end
+  end
+end

app/messages/service_credential_bindings_list_message.rb

@@ -2,20 +2,27 @@
 
 module VCAP::CloudController
   class ServiceCredentialBindingsListMessage < ListMessage
-    register_allowed_keys [
+    ARRAY_KEYS = [
       :names,
       :service_instance_guids,
       :service_instance_names,
       :app_guids,
       :app_names,
+      :include
+    ].freeze
+
+    SINGLE_KEYS = [
       :type
-    ]
+    ].freeze
+
+    register_allowed_keys ARRAY_KEYS + SINGLE_KEYS
 
     validates_with NoAdditionalParamsValidator
     validates :type, allow_nil: true, inclusion: { in: %w(app key), message: "must be one of 'app', 'key'" }
+    validates_with IncludeParamValidator, valid_values: %w(app)
 
     def self.from_params(params)
-      super(params, %w(names service_instance_guids service_instance_names app_guids app_names))
+      super(params, ARRAY_KEYS.map(&:to_s))
     end
   end
 end

app/messages/service_credential_bindings_show_message.rb

@@ -0,0 +1,18 @@
+require 'messages/base_message'
+
+module VCAP::CloudController
+  class ServiceCredentialBindingsShowMessage < BaseMessage
+    ARRAY_KEYS = [
+      :include
+    ].freeze
+
+    register_allowed_keys ARRAY_KEYS
+
+    validates_with NoAdditionalParamsValidator
+    validates_with IncludeParamValidator, valid_values: %w(app)
+
+    def self.from_params(params)
+      super(params, ARRAY_KEYS.map(&:to_s))
+    end
+  end
+end

app/presenters/v3/service_credential_binding_presenter.rb

@@ -6,16 +6,24 @@ module Presenters
       module V3
         class ServiceCredentialBindingPresenter < BasePresenter
           def to_hash
+            base_hash.merge(extra).merge(decorations)
+          end
+
+          private
+
+          def base_hash
             {
               guid: @resource.guid,
               created_at: @resource.created_at,
               updated_at: @resource.updated_at,
               name: @resource.name,
               type: type
-            }.merge(extra)
+            }
           end
 
-          private
+          def decorations
+            @decorators.reduce({}) { |memo, d| d.decorate(memo, [@resource]) }
+          end
 
           def type
             case @resource

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

@@ -38,6 +38,12 @@ This endpoint retrieves the service credential binding by GUID.
 #### Definition
 `GET /v3/service_service_credential_bindings/:guid`
 
+#### Query parameters
+
+Name | Type | Description
+---- | ---- | ------------
+**include** | _list of strings_ | Optionally include a list of unique related resources in the response. Valid values are: `app`
+
 #### Permitted roles
  |
 --- | ---

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

@@ -27,6 +27,8 @@ This endpoint retrieves the service credential bindings the user has access to.
 #### Definition
 `GET <%= @service_credential_binding_list_path %>`
 
+#### Query parameters
+
 Name | Type | Description
 ---- | ---- | ------------
 **names** | _list of strings_ | Comma-delimited list of service credential binding names to filter by
@@ -35,6 +37,7 @@ Name | Type | Description
 **app_guids** | _list of strings_ | Comma-delimited list of app guids to filter by
 **app_names** | _strings_ | Type of credential binding to filter by. Valid values are: `app` or `key`
 **type** | _list of strings_ | Comma-delimited list of app names to filter by
+**include** | _list of strings_ | Optionally include a list of unique related resources in the response. Valid values are: `app`
 **page** | _integer_ | Page to display; valid values are integers >= 1
 **per_page** | _integer_ | Number of results per page; <br>valid values are 1 through 5000
 **order_by** | _string_ | Value to sort by. Defaults to ascending; prepend with `-` to sort descending<br>Valid values are `created_at`, `updated_at`, and `name`

spec/request/service_credential_bindings_spec.rb

@@ -215,7 +215,8 @@ def check_filtered_bindings(*bindings)
         'names',
         'service_instance_guids', 'service_instance_names',
         'app_guids', 'app_names',
-        'type',
+        'include',
+        'type'
       ]
       }
 
@@ -229,6 +230,27 @@ def check_filtered_bindings(*bindings)
         }))
       end
     end
+
+    describe 'include' do
+      it 'can include `app`' do
+        get '/v3/service_credential_bindings?include=app', nil, admin_headers
+        expect(last_response).to have_status_code(200)
+
+        expect(parsed_response['included']['apps']).to have(2).items
+        guids = parsed_response['included']['apps'].map { |x| x['guid'] }
+        expect(guids).to contain_exactly(app_binding.app.guid, other_app_binding.app.guid)
+      end
+
+      it 'returns a 400 for invalid includes' do
+        get '/v3/service_credential_bindings?include=routes', nil, admin_headers
+        expect(last_response).to have_status_code(400)
+        expect(parsed_response['errors']).to include(include({
+          'detail' => include("Invalid included resource: 'routes'"),
+          'title' => 'CF-BadQueryParameter',
+          'code' => 10005,
+        }))
+      end
+    end
   end
 
   describe 'GET /v3/service_credential_bindings/:missing_key' do
@@ -284,6 +306,17 @@ def check_filtered_bindings(*bindings)
         it_behaves_like 'permissions for single object endpoint', LOCAL_ROLES
       end
     end
+
+    describe 'query params' do
+      describe 'include' do
+        it 'can include `app`' do
+          get "/v3/service_credential_bindings/#{key.guid}?include=app", nil, admin_headers
+          expect(last_response).to have_status_code(200)
+
+          expect(parsed_response['included']['apps']).to have(0).items
+        end
+      end
+    end
   end
 
   describe 'GET /v3/service_credential_bindings/:app_guid' do
@@ -340,6 +373,47 @@ def check_filtered_bindings(*bindings)
         end
       end
     end
+
+    describe 'include' do
+      it 'can include `app`' do
+        get "/v3/service_credential_bindings/#{app_binding.guid}?include=app", nil, admin_headers
+        expect(last_response).to have_status_code(200)
+
+        expect(parsed_response['included']['apps']).to have(1).items
+        guids = parsed_response['included']['apps'].map { |x| x['guid'] }
+        expect(guids).to contain_exactly(app_to_bind_to.guid)
+      end
+    end
+  end
+
+  describe 'GET /v3/service_credential_bindings/:guid' do
+    let(:binding) { VCAP::CloudController::ServiceBinding.make }
+
+    describe 'query params' do
+      it 'returns 400 for invalid query params' do
+        get "/v3/service_credential_bindings/#{binding.guid}?bahamas=yes-please", nil, admin_headers
+
+        expect(last_response).to have_status_code(400)
+        expect(parsed_response['errors']).to include(include({
+          'detail' => "The query parameter is invalid: Unknown query parameter(s): 'bahamas'. Valid parameters are: 'include'",
+          'title' => 'CF-BadQueryParameter',
+          'code' => 10005,
+        }))
+      end
+
+      describe 'includes' do
+        it 'returns 400 for invalid includes' do
+          get "/v3/service_credential_bindings/#{binding.guid}?include=routes", nil, admin_headers
+
+          expect(last_response).to have_status_code(400)
+          expect(parsed_response['errors']).to include(include({
+            'detail' => include("Invalid included resource: 'routes'"),
+            'title' => 'CF-BadQueryParameter',
+            'code' => 10005,
+          }))
+        end
+      end
+    end
   end
 
   def expected_json(binding)

spec/unit/decorators/include_binding_app_decorator_spec.rb

@@ -0,0 +1,37 @@
+require 'spec_helper'
+require 'decorators/include_binding_app_decorator'
+
+module VCAP::CloudController
+  RSpec.describe IncludeBindingAppDecorator do
+    subject(:decorator) { IncludeBindingAppDecorator }
+    let(:bindings) { [ServiceBinding.make, ServiceBinding.make, ServiceBinding.make] }
+    let(:apps) {
+      bindings.map { |b| Presenters::V3::AppPresenter.new(b.app).to_hash }
+    }
+
+    it 'decorates the given hash with organizations from apps' do
+      dict = { foo: 'bar' }
+      hash = subject.decorate(dict, bindings)
+      expect(hash[:foo]).to eq('bar')
+      expect(hash[:included][:apps]).to match_array(apps)
+    end
+
+    it 'does not overwrite other included fields' do
+      dict = { foo: 'bar', included: { fruits: ['tomato', 'banana'] } }
+      hash = subject.decorate(dict, bindings)
+      expect(hash[:foo]).to eq('bar')
+      expect(hash[:included][:apps]).to match_array(apps)
+      expect(hash[:included][:fruits]).to match_array(['tomato', 'banana'])
+    end
+
+    describe '.match?' do
+      it 'matches include arrays containing "app"' do
+        expect(decorator.match?(['potato', 'app', 'turnip'])).to be_truthy
+      end
+
+      it 'does not match other include arrays' do
+        expect(decorator.match?(['potato', 'turnip'])).to be_falsey
+      end
+    end
+  end
+end

spec/unit/messages/service_credential_bindings_list_message_spec.rb

@@ -15,7 +15,8 @@ module VCAP::CloudController
         'names' => 'name1, name2',
         'app_guids' => 'app-1-guid, app-2-guid, app-3-guid',
         'app_names' => 'app-1-name, app-2-name, app-3-name',
-        'type' => 'app'
+        'type' => 'app',
+        'include' => 'app'
       }
     end
 
@@ -31,6 +32,7 @@ module VCAP::CloudController
         expect(message.app_guids).to match_array(['app-1-guid', 'app-2-guid', 'app-3-guid'])
         expect(message.app_names).to match_array(['app-1-name', 'app-2-name', 'app-3-name'])
         expect(message.type).to eq('app')
+        expect(message.include).to match_array(['app'])
       end
 
       it 'converts requested keys to symbols' do
@@ -43,6 +45,7 @@ module VCAP::CloudController
         expect(message.requested?(:app_guids)).to be_truthy
         expect(message.requested?(:app_names)).to be_truthy
         expect(message.requested?(:type)).to be_truthy
+        expect(message.requested?(:include)).to be_truthy
       end
     end
 
@@ -58,20 +61,35 @@ module VCAP::CloudController
       end
 
       it 'returns false for invalid fields' do
-        message = described_class.from_params({ foobar: 'pants' })
+        message = described_class.from_params({ 'foobar' => 'pants' })
         expect(message).not_to be_valid
         expect(message.errors[:base][0]).to include("Unknown query parameter(s): 'foobar'")
       end
 
-      it 'returns true for valid types' do
-        expect(described_class.from_params({ type: 'app' })).to be_valid
-        expect(described_class.from_params({ type: 'key' })).to be_valid
+      context 'type' do
+        it 'returns true for valid types' do
+          expect(described_class.from_params({ 'type' => 'app' })).to be_valid
+          expect(described_class.from_params({ 'type' => 'key' })).to be_valid
+        end
+
+        it 'returns false for invalid types' do
+          message = described_class.from_params({ 'type' => 'route' })
+          expect(message).not_to be_valid
+          expect(message.errors[:type]).to include("must be one of 'app', 'key'")
+        end
       end
 
-      it 'returns false for invalid types' do
-        message = described_class.from_params({ type: 'route' })
-        expect(message).not_to be_valid
-        expect(message.errors[:type]).to include("must be one of 'app', 'key'")
+      context 'include' do
+        it 'returns false for arbitrary values' do
+          message = described_class.from_params({ 'include' => 'route' })
+          expect(message).not_to be_valid
+          expect(message.errors[:base]).to include(include("Invalid included resource: 'route'"))
+        end
+
+        it 'returns true for valid values' do
+          message = described_class.from_params({ 'include' => 'app' })
+          expect(message).to be_valid
+        end
       end
     end
   end