Set operation to 'delete in progress' at beginning of action

app/actions/service_credential_binding_delete.rb

@@ -17,19 +17,19 @@ def initialize(type, user_audit_info)
 
       private
 
-      def perform_delete_actions(binding)
-        binding.destroy
+      def event_repository
+        @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new(@event_repository_type)
+      end
 
+      def perform_delete_actions(binding)
         event_repository.record_delete(binding, @user_audit_info)
+
+        binding.destroy
       end
 
       def perform_start_delete_actions(binding)
         event_repository.record_start_delete(binding, @user_audit_info)
       end
-
-      def event_repository
-        @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new(@event_repository_type)
-      end
     end
   end
 end

app/actions/service_route_binding_delete.rb

@@ -7,20 +7,17 @@ class ServiceRouteBindingDelete < V3::ServiceBindingDelete
       def initialize(user_audit_info)
         super()
         @user_audit_info = user_audit_info
+        @event_repository_type = Repositories::ServiceGenericBindingEventRepository::SERVICE_ROUTE_BINDING
       end
 
       private
 
       def event_repository
-        @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new(
-          Repositories::ServiceGenericBindingEventRepository::SERVICE_ROUTE_BINDING)
+        @event_repository ||= Repositories::ServiceGenericBindingEventRepository.new(@event_repository_type)
       end
 
       def perform_delete_actions(binding)
-        event_repository.record_delete(
-          binding,
-          @user_audit_info
-        )
+        event_repository.record_delete(binding, @user_audit_info)
 
         binding.destroy
         binding.notify_diego

app/actions/v3/service_binding_delete.rb

@@ -25,18 +25,20 @@ def blocking_operation_in_progress?(binding)
       def delete(binding)
         operation_in_progress! if blocking_operation_in_progress?(binding)
 
+        update_last_operation(binding) unless binding.operation_in_progress?
+
         result = send_unbind_to_client(binding)
         if result[:finished]
           perform_delete_actions(binding)
         else
           perform_start_delete_actions(binding)
-          update_last_operation(binding, operation: result[:operation])
+          update_last_operation_with_details(binding, operation: result[:operation])
         end
 
-        return result
+        result
       rescue => e
         unless e.is_a? ConcurrencyError
-          update_last_operation(binding, state: 'failed', description: e.message)
+          update_last_operation_with_failure(binding, e.message)
         end
 
         raise e
@@ -48,7 +50,7 @@ def poll(binding)
 
         case details[:last_operation][:state]
         when 'in progress'
-          update_last_operation(
+          update_last_operation_with_details(
             binding,
             description: details[:last_operation][:description],
             operation: binding.last_operation.broker_provided_operation)
@@ -57,13 +59,13 @@ def poll(binding)
           perform_delete_actions(binding)
           return PollingFinished
         when 'failed'
-          update_last_operation(binding, state: 'failed', description: details[:last_operation][:description])
+          update_last_operation_with_failure(binding, details[:last_operation][:description])
           raise LastOperationFailedState
         end
       rescue LastOperationFailedState => e
         raise e
       rescue => e
-        update_last_operation(binding, state: 'failed', description: e.message)
+        update_last_operation_with_failure(binding, e.message)
         raise e
       end
 
@@ -86,15 +88,28 @@ def send_unbind_to_client(binding)
         unprocessable!(binding, err)
       end
 
-      def update_last_operation(binding, description: nil, state: 'in progress', operation: nil)
+      def update_last_operation(binding)
+        binding.save_with_attributes_and_new_operation({}, { type: 'delete', state: 'in progress' })
+      end
+
+      def update_last_operation_with_details(binding, description: nil, operation: nil)
+        lo = { type: 'delete', state: 'in progress' }
+        description ||= binding.last_operation&.description
+        operation ||= binding.last_operation&.broker_provided_operation
+        lo.merge!({ description: description }) unless description.nil?
+        lo.merge!({ broker_provided_operation: operation }) unless operation.nil?
+        binding.save_with_attributes_and_new_operation({}, lo)
+      end
+
+      def update_last_operation_with_failure(binding, message)
         binding.save_with_attributes_and_new_operation(
           {},
           {
             type: 'delete',
-            state: state,
-            description: description,
-            broker_provided_operation: operation || binding.last_operation&.broker_provided_operation
-          })
+            state: 'failed',
+            description: message,
+          }
+        )
       end
 
       def unprocessable!(binding, err)

app/actions/v3/service_instance_delete.rb

@@ -37,20 +37,25 @@ def blocking_operation_in_progress?
       def delete
         operation_in_progress! if blocking_operation_in_progress?
 
+        update_last_operation unless service_instance.operation_in_progress?
+
         errors = remove_associations
         raise errors.first if errors.any?
 
         result = send_deprovison_to_broker
         if result[:finished]
           perform_delete_actions
         else
-          update_last_operation_with_operation_id(result[:operation])
+          update_last_operation_with_details(operation: result[:operation])
           record_start_delete_event
         end
 
-        return result
+        result
       rescue => e
-        update_last_operation_with_failure(e.message) unless service_instance.operation_in_progress?
+        unless (e.is_a? CloudController::Errors::ApiError) && e.name == 'AsyncServiceInstanceOperationInProgress'
+          update_last_operation_with_failure(e.message)
+        end
+
         raise e
       end
 
@@ -61,7 +66,7 @@ def poll
         )
         case result[:last_operation][:state]
         when 'in progress'
-          update_last_operation_with_description(result[:last_operation][:description])
+          update_last_operation_with_details(description: result[:last_operation][:description])
           ContinuePolling.call(result[:retry_after])
         when 'succeeded'
           perform_delete_actions
@@ -152,21 +157,16 @@ def unshare_all_spaces
         end
       end
 
-      def update_last_operation_with_operation_id(operation_id)
-        service_instance.save_with_new_operation(
-          {},
-          {
-            type: 'delete',
-            state: 'in progress',
-            broker_provided_operation: operation_id
-          }
-        )
+      def update_last_operation
+        service_instance.save_with_new_operation({}, { type: 'delete', state: 'in progress' })
       end
 
-      def update_last_operation_with_description(description)
-        lo = service_instance.last_operation.to_hash
-        lo[:broker_provided_operation] = service_instance.last_operation.broker_provided_operation
-        lo[:description] = description
+      def update_last_operation_with_details(description: nil, operation: nil)
+        lo = { type: 'delete', state: 'in progress' }
+        description ||= service_instance.last_operation&.description
+        operation ||= service_instance.last_operation&.broker_provided_operation
+        lo.merge!({ description: description }) unless description.nil?
+        lo.merge!({ broker_provided_operation: operation }) unless operation.nil?
         service_instance.save_with_new_operation({}, lo)
       end
 

app/jobs/v3/delete_binding_job.rb

@@ -14,11 +14,11 @@ def initialize(type, binding_guid, user_audit_info:)
       end
 
       def actor
-        DeleteServiceBindingFactory.for(@type)
+        @actor ||= DeleteServiceBindingFactory.for(@type)
       end
 
       def action
-        DeleteServiceBindingFactory.action(@type, @user_audit_info)
+        @action ||= DeleteServiceBindingFactory.action(@type, @user_audit_info)
       end
 
       def operation
@@ -79,11 +79,11 @@ def handle_timeout
       private
 
       def binding
-        actor.get_resource(resource_guid)
+        @binding ||= actor.get_resource(resource_guid)
       end
 
       def delete_in_progress?
-        binding.last_operation&.type == 'delete' &&
+        binding.reload.last_operation&.type == 'delete' &&
           binding.last_operation&.state == 'in progress'
       end
 

app/jobs/v3/delete_service_binding_job_factory.rb

@@ -38,10 +38,10 @@ def self.type_of(binding)
         case binding
         when RouteBinding
           :route
-        when ServiceKey
-          :key
         when ServiceBinding
           :credential
+        when ServiceKey
+          :key
         else
           raise InvalidType
         end

app/jobs/v3/delete_service_instance_job.rb

@@ -5,7 +5,7 @@
 module VCAP::CloudController
   module V3
     class DeleteServiceInstanceJob < VCAP::CloudController::Jobs::ReoccurringJob
-      attr_reader :resource_guid
+      attr_reader :resource_guid, :user_audit_info
 
       def initialize(guid, user_audit_info)
         super()
@@ -55,19 +55,17 @@ def display_name
 
       private
 
-      attr_reader :user_audit_info
-
       def service_instance
-        ManagedServiceInstance.first(guid: resource_guid)
+        @service_instance ||= ManagedServiceInstance.first(guid: resource_guid)
       end
 
       def delete_in_progress?
-        service_instance.last_operation&.type == 'delete' &&
+        service_instance.reload.last_operation&.type == 'delete' &&
           service_instance.last_operation&.state == 'in progress'
       end
 
       def action
-        ServiceInstanceDelete.new(service_instance, Repositories::ServiceEventRepository.new(user_audit_info))
+        @action ||= ServiceInstanceDelete.new(service_instance, Repositories::ServiceEventRepository.new(user_audit_info))
       end
     end
   end

app/jobs/v3/delete_service_route_binding_job_actor.rb

@@ -1,6 +1,3 @@
-require 'jobs/reoccurring_job'
-require 'cloud_controller/errors/api_error'
-
 module VCAP::CloudController
   module V3
     class DeleteServiceRouteBindingJobActor

lib/cloud_controller/jobs.rb

@@ -37,7 +37,6 @@
 require 'jobs/runtime/prune_completed_builds'
 require 'jobs/runtime/prune_excess_app_revisions'
 
-require 'jobs/v2/services/legacy_jobs/service_instance_deletion'
 require 'jobs/v2/services/service_usage_events_cleanup'
 
 require 'jobs/v2/upload_droplet_from_user'

spec/unit/actions/service_credential_binding_delete_spec.rb

@@ -17,11 +17,19 @@ module V3
         allow(binding_event_repo).to receive(:record_start_delete)
       end
 
-      RSpec.shared_examples 'successful credential binding delete' do |klass|
+      RSpec.shared_examples 'successful credential binding delete' do |klass, klass_operation|
         it 'deletes the binding' do
           action.delete(binding)
 
           expect(klass.all).to be_empty
+          expect(klass_operation.all).to be_empty
+        end
+
+        it "sets the operation to 'delete in progress'" do
+          expect_any_instance_of(klass).to receive(:save_with_attributes_and_new_operation).with({}, { type: 'delete', state: 'in progress' }).and_call_original
+          expect_any_instance_of(klass).to receive(:save_with_attributes_and_new_operation).and_call_original
+
+          action.delete(binding)
         end
 
         it 'creates an audit event' do
@@ -40,7 +48,7 @@ module V3
         end
       end
 
-      RSpec.shared_examples 'managed service instance binding delete' do |klass|
+      RSpec.shared_examples 'managed service instance binding delete' do |klass, klass_operation|
         context 'managed service instance' do
           let(:service_instance) { ManagedServiceInstance.make(space: space) }
 
@@ -54,7 +62,7 @@ module V3
               allow(VCAP::Services::ServiceBrokers::V2::Client).to receive(:new).and_return(broker_client)
             end
 
-            it_behaves_like 'successful credential binding delete', klass
+            it_behaves_like 'successful credential binding delete', klass, klass_operation
           end
 
           context 'async unbinding' do
@@ -212,12 +220,12 @@ module V3
         end
 
         describe '#delete' do
-          it_behaves_like 'managed service instance binding delete', ServiceBinding
+          it_behaves_like 'managed service instance binding delete', ServiceBinding, ServiceBindingOperation
 
           context 'user-provided service instance' do
             let(:service_instance) { UserProvidedServiceInstance.make(space: space) }
 
-            it_behaves_like 'successful credential binding delete', ServiceBinding
+            it_behaves_like 'successful credential binding delete', ServiceBinding, ServiceBindingOperation
           end
         end
 
@@ -245,7 +253,7 @@ module V3
         end
 
         describe '#delete' do
-          it_behaves_like 'managed service instance binding delete', ServiceKey
+          it_behaves_like 'managed service instance binding delete', ServiceKey, ServiceKeyOperation
         end
 
         describe '#poll' do