v3(services): cred binding list body (cloudfoundry#1771)

* v3(services): Have cred binding view return ServiceBinding and ServiceKey objects

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

Author: Brian Butz

app/models/services/service_credential_binding_view.rb

@@ -7,6 +7,7 @@ module Types
       end
 
       SERVICE_KEY_VIEW = Sequel::Model(:service_keys).select(
+        Sequel.as(:service_keys__id, :id),
         Sequel.as(:service_keys__guid, :guid),
         Sequel.as(Types::SERVICE_KEY, :type),
         Sequel.as(:spaces__guid, :space_guid),
@@ -15,19 +16,15 @@ module Types
         Sequel.as(:service_keys__name, :name),
         Sequel.as(:service_instances__guid, :service_instance_guid),
         Sequel.as(nil, :app_guid),
-        Sequel.as(nil, :last_operation_state),
-        Sequel.as(nil, :last_operation_description),
-        Sequel.as(nil, :last_operation_created_at),
-        Sequel.as(nil, :last_operation_updated_at),
-        Sequel.as(nil, :last_operation_type),
-        Sequel.as(nil, :last_operation_id)
+        Sequel.as(:service_keys__service_instance_id, :service_instance_id)
       ).join(
         :service_instances, id: Sequel[:service_keys][:service_instance_id]
       ).join(
         :spaces, id: Sequel[:service_instances][:space_id]
       ).freeze
 
       SERVICE_BINDING_VIEW = Sequel::Model(:service_bindings).select(
+        Sequel.as(:service_bindings__id, :id),
         Sequel.as(:service_bindings__guid, :guid),
         Sequel.as(Types::SERVICE_BINDING, :type),
         Sequel.as(:spaces__guid, :space_guid),
@@ -36,18 +33,11 @@ module Types
         Sequel.as(:service_bindings__name, :name),
         Sequel.as(:service_bindings__service_instance_guid, :service_instance_guid),
         Sequel.as(:service_bindings__app_guid, :app_guid),
-        Sequel.as(:service_binding_operations__state, :last_operation_state),
-        Sequel.as(:service_binding_operations__description, :last_operation_description),
-        Sequel.as(:service_binding_operations__created_at, :last_operation_created_at),
-        Sequel.as(:service_binding_operations__updated_at, :last_operation_updated_at),
-        Sequel.as(:service_binding_operations__type, :last_operation_type),
-        Sequel.as(:service_binding_operations__id, :last_operation_id)
+        Sequel.as(nil, :service_instance_id)
       ).join(
         :apps, guid: Sequel[:service_bindings][:app_guid]
       ).join(
         :spaces, guid: Sequel[:apps][:space_guid]
-      ).left_join(
-        :service_binding_operations, service_binding_id: Sequel[:service_bindings][:id]
       ).freeze
 
       VIEW = [
@@ -58,6 +48,12 @@ module Types
       end.from_self.freeze
 
       class View < Sequel::Model(VIEW)
+        plugin :single_table_inheritance,
+               :type,
+               model_map: {
+                 'app' => 'VCAP::CloudController::ServiceBinding',
+                 'key' => 'VCAP::CloudController::ServiceKey'
+               }
       end
     end
   end

app/presenters/v3/service_credential_binding_presenter.rb

@@ -8,35 +8,52 @@ class ServiceCredentialBindingPresenter < BasePresenter
           def to_hash
             {
               guid: @resource.guid,
-              type: @resource.type,
               name: @resource.name,
               created_at: @resource.created_at,
               updated_at: @resource.updated_at,
-              last_operation: last_operation,
-              relationships: build_relationships,
-              links: build_links
-            }
+              type: type
+            }.merge(extra)
           end
 
           private
 
-          def build_relationships
-            base_relationships.merge(app_relationship)
+          def type
+            case @resource
+            when VCAP::CloudController::ServiceKey
+              'key'
+            when VCAP::CloudController::ServiceBinding
+              'app'
+            end
           end
 
-          def build_links
-            base_links.merge(app_link)
+          def extra
+            case type
+            when 'app'
+              {
+                last_operation: last_operation,
+                relationships: base_relationships.merge(app_relationship),
+                links: base_links.merge(app_link)
+              }
+            when 'key'
+              {
+                last_operation: nil,
+                relationships: base_relationships,
+                links: base_links
+              }
+            end
           end
 
           def last_operation
-            return nil if @resource.last_operation_id.blank?
+            return nil if @resource.last_operation.blank?
+
+            last_operation = @resource.last_operation
 
             {
-              type: @resource.last_operation_type,
-              state: @resource.last_operation_state,
-              description: @resource.last_operation_description,
-              created_at: @resource.last_operation_created_at,
-              updated_at: @resource.last_operation_updated_at
+              type: last_operation.type,
+              state: last_operation.state,
+              description: last_operation.description,
+              created_at: last_operation.created_at,
+              updated_at: last_operation.updated_at
             }
           end
 

docs/v3/source/includes/api_resources/_service_credential_bindings.erb

@@ -10,7 +10,7 @@
         "guid": "74f7c078-0934-470f-9883-4fddss5b8f13"
       }
     },
-    "service_instances": {
+    "service_instance": {
       "data": {
         "guid": "8bfe4c1b-9e18-45b1-83be-124163f31f9e"
       }
@@ -37,7 +37,7 @@
   "created_at": "2015-11-13T17:02:56Z",
   "updated_at": "2016-06-08T16:41:26Z",
   "relationships": {
-    "service_instances": {
+    "service_instance": {
       "data": {
         "guid": "8bfe4c1b-9e18-45b1-83be-124163f31f9e"
       }
@@ -89,7 +89,7 @@
             "guid": "74f7c078-0934-470f-9883-4fddss5b8f13"
           }
         },
-        "service_instances": {
+        "service_instance": {
           "data": {
             "guid": "8bfe4c1b-9e18-45b1-83be-124163f31f9e"
           }
@@ -114,7 +114,7 @@
       "updated_at": "2016-06-08T16:41:26Z",
       "last_operation": null,
       "relationships": {
-        "service_instances": {
+        "service_instance": {
           "data": {
             "guid": "8bfe4c1b-9e18-45b1-83be-124163f31f9e"
           }

spec/request/service_credential_bindings_spec.rb

@@ -13,7 +13,11 @@
     let(:other_instance) { VCAP::CloudController::ManagedServiceInstance.make(space: other_space) }
     let!(:key_binding) { VCAP::CloudController::ServiceKey.make(service_instance: instance, created_at: now - 4.seconds) }
     let!(:other_key_binding) { VCAP::CloudController::ServiceKey.make(service_instance: other_instance, created_at: now - 3.seconds) }
-    let!(:app_binding) { VCAP::CloudController::ServiceBinding.make(service_instance: instance, created_at: now - 2.seconds) }
+    let!(:app_binding) do
+      VCAP::CloudController::ServiceBinding.make(service_instance: instance, created_at: now - 2.seconds).tap do |binding|
+        operate_on(binding)
+      end
+    end
     let!(:other_app_binding) { VCAP::CloudController::ServiceBinding.make(service_instance: other_instance, created_at: now - 1.second) }
 
     describe 'permissions' do
@@ -117,15 +121,11 @@
 
   describe 'GET /v3/service_credential_bindings/:app_guid' do
     let(:app_to_bind_to) { VCAP::CloudController::AppModel.make(space: space) }
-    let(:app_binding) {
-      b = VCAP::CloudController::ServiceBinding.make(service_instance: instance, app: app_to_bind_to)
-      b.save_with_new_operation({
-        type: 'create',
-        state: 'succeeded',
-        description: 'some description'
-      })
-      b
-    }
+    let(:app_binding) do
+      VCAP::CloudController::ServiceBinding.make(service_instance: instance, app: app_to_bind_to).tap do |binding|
+        operate_on(binding)
+      end
+    end
     let(:instance) { VCAP::CloudController::ManagedServiceInstance.make(space: space) }
     let(:api_call) { ->(user_headers) { get "/v3/service_credential_bindings/#{app_binding.guid}", nil, user_headers } }
     let(:expected_object) { expected_json(app_binding) }
@@ -207,18 +207,9 @@ def extra(binding)
         type: 'key',
       }
     when VCAP::CloudController::ServiceBinding
-      if binding.last_operation.present?
-        last_operation = {
-                            type: binding.last_operation.type,
-                            state: binding.last_operation.state,
-                            description: binding.last_operation.description,
-                            created_at: iso8601,
-                            updated_at: iso8601
-                         }
-      end
       {
         type: 'app',
-        last_operation: last_operation,
+        last_operation: last_operation(binding),
         relationships: {
           app: {
             data: {
@@ -236,4 +227,26 @@ def extra(binding)
       raise 'not a binding'
     end
   end
+
+  def last_operation(binding)
+    if binding.last_operation.present?
+      {
+        type: binding.last_operation.type,
+        state: binding.last_operation.state,
+        description: binding.last_operation.description,
+        created_at: iso8601,
+        updated_at: iso8601
+      }
+    end
+  end
+
+  def operate_on(binding)
+    binding.save_with_new_operation(
+      {
+        type: 'create',
+        state: 'succeeded',
+        description: 'some description'
+      }
+    )
+  end
 end

spec/unit/fetchers/service_credential_binding_fetcher_spec.rb

@@ -16,7 +16,6 @@ module CloudController
       end
 
       describe 'service keys' do
-        let(:type) { 'key' }
         let(:space) { Space.make }
         let(:service_instance) { ManagedServiceInstance.make(space: space) }
         let!(:service_key) { ServiceKey.make(service_instance: service_instance) }
@@ -26,13 +25,11 @@ module CloudController
             credential_binding = fetcher.fetch(service_key.guid, space_guids: [space.guid])
 
             expect(credential_binding).not_to be_nil
-            expect(credential_binding.type).to eql(type)
+            expect(credential_binding).to an_instance_of(VCAP::CloudController::ServiceKey)
             expect(credential_binding.name).to eql(service_key.name)
             expect(credential_binding.created_at).to eql(service_key.created_at)
             expect(credential_binding.updated_at).to eql(service_key.updated_at)
             expect(credential_binding.service_instance_guid).to eql(service_instance.guid)
-            expect(credential_binding.app_guid).to be_nil
-            expect(credential_binding.last_operation_id).to be_nil
           end
         end
 
@@ -47,8 +44,6 @@ module CloudController
       end
 
       describe 'app bindings' do
-        let(:type) { 'app' }
-
         describe 'managed services' do
           let(:space) { Space.make }
           let(:service_instance) { ManagedServiceInstance.make(space: space) }
@@ -58,13 +53,13 @@ module CloudController
             credential_binding = fetcher.fetch(app_binding.guid, space_guids: [space.guid])
 
             expect(credential_binding).not_to be_nil
-            expect(credential_binding.type).to eql(type)
+            expect(credential_binding).to an_instance_of(VCAP::CloudController::ServiceBinding)
             expect(credential_binding.name).to eql('some-name')
             expect(credential_binding.created_at).to eql(app_binding.created_at)
             expect(credential_binding.updated_at).to eql(app_binding.updated_at)
             expect(credential_binding.service_instance_guid).to eql(service_instance.guid)
             expect(credential_binding.app_guid).to eql(app_binding.app_guid)
-            expect(credential_binding.last_operation_id).to be_nil
+            expect(credential_binding.last_operation).to be_nil
           end
 
           describe 'when not in the space' do
@@ -86,13 +81,13 @@ module CloudController
             credential_binding = fetcher.fetch(app_binding.guid, space_guids: [space.guid])
 
             expect(credential_binding).not_to be_nil
-            expect(credential_binding.type).to eql(type)
+            expect(credential_binding).to an_instance_of(VCAP::CloudController::ServiceBinding)
             expect(credential_binding.name).to eql('some-name')
             expect(credential_binding.created_at).to eql(app_binding.created_at)
             expect(credential_binding.updated_at).to eql(app_binding.updated_at)
             expect(credential_binding.service_instance_guid).to eql(service_instance.guid)
             expect(credential_binding.app_guid).to eql(app_binding.app_guid)
-            expect(credential_binding.last_operation_id).to be_nil
+            expect(credential_binding.last_operation).to be_nil
           end
 
           describe 'when not in the space' do
@@ -112,15 +107,19 @@ module CloudController
             binding.save_with_new_operation({ state: 'succeeded', type: 'create', description: 'radical avocado' })
             binding
           }
+
           it 'fetches the last operation' do
             credential_binding = fetcher.fetch(app_binding.guid, space_guids: [service_instance.space.guid])
 
-            expect(credential_binding.last_operation_id).to_not be_nil
-            expect(credential_binding.last_operation_type).to eql('create')
-            expect(credential_binding.last_operation_state).to eql('succeeded')
-            expect(credential_binding.last_operation_description).to eql('radical avocado')
-            expect(credential_binding.last_operation_created_at).to eql(app_binding.last_operation.created_at)
-            expect(credential_binding.last_operation_updated_at).to eql(app_binding.last_operation.updated_at)
+            expect(credential_binding.last_operation).to_not be_nil
+
+            last_operation = credential_binding.last_operation
+
+            expect(last_operation.type).to eql('create')
+            expect(last_operation.state).to eql('succeeded')
+            expect(last_operation.description).to eql('radical avocado')
+            expect(last_operation.created_at).to eql(app_binding.last_operation.created_at)
+            expect(last_operation.updated_at).to eql(app_binding.last_operation.updated_at)
           end
         end
       end

spec/unit/fetchers/service_credential_binding_list_fetcher_spec.rb

@@ -41,6 +41,31 @@ module CloudController
           end
         end
       end
+
+      describe 'fetching app bindings' do
+        let!(:app_binding) { VCAP::CloudController::ServiceBinding.make }
+
+        it 'allows the last operation to be accessed' do
+          app_binding.save_with_new_operation(
+            {
+              type: 'create',
+              state: 'succeeded',
+              description: 'some description'
+            }
+          )
+
+          credential_binding = fetcher.fetch(space_guids: :all).first
+          last_operation = credential_binding.last_operation
+
+          expect(last_operation).to be_present
+
+          expect(last_operation.type).to eql 'create'
+          expect(last_operation.state).to eql 'succeeded'
+          expect(last_operation.description).to eql 'some description'
+          expect(last_operation.created_at).to be_present
+          expect(last_operation.updated_at).to be_present
+        end
+      end
     end
   end
 end