V3(services): refactor binding jobs and actions

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

* WIP - route and cred bindings controller are calling the same job

* Extracted testing of create binding job to shared example

Shared example can be used to test route and credential type of bindings.
Next step in the refactoring is merging the action in one single version.

Co-Authored-By: Derik Evangelista <ederik@vmware.com>

* WIP:  Started extracting binding action into one super class

Co-Authored-By: Derik Evangelista <ederik@vmware.com>

* WIP: poll service binding creation can be used by credential binding and route binding

* WIP: Poll testing

Co-Authored-By: Derik Evangelista <ederik@vmware.com>

* wip: add unit test for polling

* wip: fix failing test

Co-authored-by: Derik Evangelista <devangelista@pivotal.io>
Co-authored-by: Derik Evangelista <ederik@vmware.com>

Author: 3 people

app/actions/service_credential_binding_create.rb

@@ -1,16 +1,18 @@
 require 'repositories/service_binding_event_repository'
 require 'services/service_brokers/service_client_provider'
+require 'actions/v3/service_binding_create'
 
 module VCAP::CloudController
   module V3
-    class ServiceCredentialBindingCreate
+    class ServiceCredentialBindingCreate < V3::ServiceBindingCreate
       class UnprocessableCreate < StandardError
       end
 
       class Unimplemented < StandardError
       end
 
       def initialize(user_audit_info, audit_hash)
+        super()
         @user_audit_info = user_audit_info
         @audit_hash = audit_hash
       end
@@ -39,37 +41,18 @@ def precursor(service_instance, app: nil, name: nil, volume_mount_services_enabl
         raise UnprocessableCreate.new(e.full_message)
       end
 
-      def bind(binding, parameters: {}, accepts_incomplete: false)
-        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
-        details = client.bind(binding, arbitrary_parameters: parameters, accepts_incomplete: accepts_incomplete)
-
-        if details[:async]
-          save_incomplete_binding(binding, details[:operation])
-        else
-          binding.save_with_new_operation(operation_succeeded, attributes: details[:binding])
-          event_repository.record_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
-        end
-      rescue => e
-        binding.save_with_new_operation({
-          type: 'create',
-          state: 'failed',
-          description: e.message,
-        })
-        raise e
-      end
-
       private
 
-      def operation_succeeded
-        { type: 'create', state: 'succeeded' }
-      end
-
-      def save_incomplete_binding(binding, operation)
-        binding.save_with_new_operation({
-          type: 'create',
-          state: 'in progress',
-          broker_provided_operation: operation
-        })
+      def complete_binding_and_save(binding, binding_details, last_operation)
+        binding.save_with_attributes_and_new_operation(
+          binding_details,
+          {
+            type: 'create',
+            state: last_operation[:state],
+            description: last_operation[:description]
+          }
+        )
+        event_repository.record_create(binding, @user_audit_info, @audit_hash, manifest_triggered: false)
       end
 
       def validate!(service_instance, app, volume_mount_services_enabled)

app/actions/service_route_binding_create.rb

@@ -1,9 +1,11 @@
 require 'services/service_brokers/service_client_provider'
+require 'actions/v3/service_binding_create'
 
 module VCAP::CloudController
   module V3
-    class ServiceRouteBindingCreate
+    class ServiceRouteBindingCreate < V3::ServiceBindingCreate
       def initialize(service_event_repository)
+        super()
         @service_event_repository = service_event_repository
       end
 
@@ -22,77 +24,10 @@ def precursor(service_instance, route)
         )
       end
 
-      def bind(precursor, parameters: {}, accepts_incomplete: false)
-        client = VCAP::Services::ServiceClientProvider.provide(instance: precursor.service_instance)
-        details = client.bind(precursor, arbitrary_parameters: parameters, accepts_incomplete: accepts_incomplete)
-
-        if details[:async]
-          not_retrievable! unless bindings_retrievable?(precursor)
-          save_incomplete_binding(precursor, details[:operation])
-        else
-          complete_binding_and_save(precursor, details[:binding][:route_service_url])
-        end
-      rescue => e
-        precursor.save_with_new_operation({}, {
-          type: 'create',
-          state: 'failed',
-          description: e.message,
-        })
-
-        raise e
-      end
-
-      def poll(binding)
-        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
-
-        details = fetch_last_operation(client, binding)
-        return PollingNotComplete.new unless details
-
-        attributes = {}
-
-        complete = details[:last_operation][:state] == 'succeeded'
-        if complete
-          params = client.fetch_service_binding(binding)
-          attributes[:route_service_url] = params[:route_service_url]
-        end
-
-        binding.save_with_new_operation(
-          attributes,
-          {
-            type: 'create',
-            state: details[:last_operation][:state],
-            description: details[:last_operation][:description],
-          }
-        )
-
-        if complete
-          binding.notify_diego
-          record_audit_event(binding)
-        end
-
-        if binding.reload.terminal_state?
-          PollingComplete.new
-        else
-          PollingNotComplete.new(details[:retry_after])
-        end
-      rescue => e
-        binding.save_with_new_operation({}, {
-          type: 'create',
-          state: 'failed',
-          description: e.message,
-        })
-        return PollingComplete.new
-      end
-
-      PollingComplete = Class.new.freeze
-      PollingNotComplete = Struct.new(:retry_after).freeze
-
       class UnprocessableCreate < StandardError; end
 
       class RouteBindingAlreadyExists < StandardError; end
 
-      class BindingNotRetrievable < StandardError; end
-
       private
 
       def validate!(service_instance, route)
@@ -107,46 +42,19 @@ def validate!(service_instance, route)
 
       attr_reader :service_event_repository
 
-      def fetch_last_operation(client, binding)
-        client.fetch_service_binding_last_operation(binding)
-      rescue VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse,
-             VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerRequestRejected,
-             HttpRequestError => e
-        binding.save_with_new_operation({}, {
-          type: 'create',
-          state: 'in progress',
-          description: e.message,
-        })
-
-        return nil
-      end
-
-      def save_incomplete_binding(precursor, operation)
-        precursor.save_with_new_operation({},
+      def complete_binding_and_save(binding, binding_details, last_operation)
+        binding.save_with_attributes_and_new_operation(
           {
-            type: 'create',
-            state: 'in progress',
-            broker_provided_operation: operation
-          }
-        )
-      end
-
-      def complete_binding_and_save(precursor, route_service_url)
-        save_with_route_service_url(precursor, route_service_url)
-        precursor.notify_diego
-        record_audit_event(precursor)
-      end
-
-      def save_with_route_service_url(precursor, route_service_url)
-        precursor.save_with_new_operation(
-          {
-            route_service_url: route_service_url
+            route_service_url: binding_details[:route_service_url]
           },
           {
             type: 'create',
-            state: 'succeeded',
+            state: last_operation[:state],
+            description: last_operation[:description],
           }
         )
+        binding.notify_diego
+        record_audit_event(binding)
       end
 
       def record_audit_event(precursor)
@@ -157,10 +65,6 @@ def record_audit_event(precursor)
         )
       end
 
-      def bindings_retrievable?(binding)
-        binding.service_instance.service.bindings_retrievable
-      end
-
       def operation_in_progress!
         raise UnprocessableCreate.new('There is an operation in progress for the service instance')
       end
@@ -188,10 +92,6 @@ def not_bindable!
       def already_exists!
         raise RouteBindingAlreadyExists.new('The route and service instance are already bound')
       end
-
-      def not_retrievable!
-        raise BindingNotRetrievable.new('The broker responded asynchronously but does not support fetching binding data')
-      end
     end
   end
 end

app/actions/v3/service_binding_create.rb

@@ -0,0 +1,110 @@
+require 'services/service_brokers/service_client_provider'
+
+module VCAP::CloudController
+  module V3
+    class ServiceBindingCreate
+      PollingStatus = Struct.new(:finished, :retry_after).freeze
+      PollingFinished = PollingStatus.new(true, nil).freeze
+      ContinuePolling = ->(retry_after) { PollingStatus.new(false, retry_after) }
+
+      def bind(binding, parameters: {}, accepts_incomplete: false)
+        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
+        details = client.bind(binding, arbitrary_parameters: parameters, accepts_incomplete: accepts_incomplete)
+
+        if details[:async]
+          not_retrievable! unless bindings_retrievable?(binding)
+          save_incomplete_binding(binding, details[:operation])
+        else
+          complete_binding_and_save(binding, details[:binding], { state: 'succeeded' })
+        end
+      rescue => e
+        binding.save_with_attributes_and_new_operation(
+          {},
+          {
+            type: 'create',
+            state: 'failed',
+            description: e.message,
+          }
+        )
+
+        raise e
+      end
+
+      def poll(binding)
+        client = VCAP::Services::ServiceClientProvider.provide(instance: binding.service_instance)
+        details = fetch_last_operation(client, binding)
+        return ContinuePolling.call(nil) unless details
+
+        if details[:last_operation][:state] == 'succeeded'
+          params = client.fetch_service_binding(binding)
+          complete_binding_and_save(binding, params, details[:last_operation])
+          return PollingFinished
+        end
+
+        binding.save_with_attributes_and_new_operation(
+          {},
+          {
+            type: 'create',
+            state: details[:last_operation][:state],
+            description: details[:last_operation][:description],
+          }
+        )
+
+        if binding.reload.terminal_state?
+          PollingFinished
+        else
+          ContinuePolling.call(details[:retry_after])
+        end
+      rescue => e
+        binding.save_with_attributes_and_new_operation(
+          {},
+          {
+            type: 'create',
+            state: 'failed',
+            description: e.message,
+          }
+        )
+        PollingFinished
+      end
+
+      class BindingNotRetrievable < StandardError; end
+
+      private
+
+      def save_incomplete_binding(precursor, operation)
+        precursor.save_with_attributes_and_new_operation(
+          {},
+          {
+            type: 'create',
+            state: 'in progress',
+            broker_provided_operation: operation
+          }
+        )
+      end
+
+      def bindings_retrievable?(binding)
+        binding.service_instance.service.bindings_retrievable
+      end
+
+      def not_retrievable!
+        raise BindingNotRetrievable.new('The broker responded asynchronously but does not support fetching binding data')
+      end
+
+      def fetch_last_operation(client, binding)
+        client.fetch_service_binding_last_operation(binding)
+      rescue VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse,
+             VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerRequestRejected,
+             HttpRequestError => e
+        binding.save_with_attributes_and_new_operation(
+          {},
+          {
+          type: 'create',
+          state: 'in progress',
+          description: e.message,
+        })
+
+        return nil
+      end
+    end
+  end
+end

app/controllers/v3/service_credential_bindings_controller.rb

@@ -9,7 +9,7 @@
 require 'messages/service_credential_binding_create_message'
 require 'decorators/include_binding_app_decorator'
 require 'decorators/include_binding_service_instance_decorator'
-require 'jobs/v3/create_service_credential_binding_job'
+require 'jobs/v3/create_service_credential_binding_job_actor'
 
 class ServiceCredentialBindingsController < ApplicationController
   def index
@@ -112,7 +112,8 @@ def parameters
   private
 
   def enqueue_bind_job(binding_guid, message)
-    bind_job = VCAP::CloudController::V3::CreateServiceCredentialBindingJob.new(
+    bind_job = VCAP::CloudController::V3::CreateBindingAsyncJob.new(
+      :credential,
       binding_guid,
       user_audit_info: user_audit_info,
       audit_hash: message.audit_hash,

app/controllers/v3/service_route_bindings_controller.rb

@@ -3,7 +3,7 @@
 require 'messages/service_route_bindings_list_message'
 require 'actions/service_route_binding_create'
 require 'actions/service_route_binding_delete'
-require 'jobs/v3/create_route_binding_job'
+require 'jobs/v3/create_binding_async_job'
 require 'jobs/v3/delete_route_binding_job'
 require 'presenters/v3/paginated_list_presenter'
 require 'presenters/v3/service_route_binding_presenter'
@@ -102,9 +102,11 @@ def valid_message(message_type:)
   end
 
   def enqueue_bind_job(binding_guid, parameters)
-    bind_job = VCAP::CloudController::V3::CreateRouteBindingJob.new(
+    bind_job = VCAP::CloudController::V3::CreateBindingAsyncJob.new(
+      :route,
       binding_guid,
       user_audit_info: user_audit_info,
+      audit_hash: {},
       parameters: parameters,
     )
     pollable_job = Jobs::Enqueuer.new(bind_job, queue: Jobs::Queues.generic).enqueue_pollable