v3(services): fix job state when deleting orphans (cloudfoundry#1749)

Co-authored-by: Brian Butz <bbutz@pivotal.io>

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

Author: Derik Evangelista

app/jobs/pollable_job_wrapper.rb

@@ -13,9 +13,10 @@ def initialize(handler, existing_guid: nil)
       # use custom hook as Job does not have the guid field populated during the normal `enqueue` hook
       def after_enqueue(job)
         if existing_guid && (existing = PollableJobModel.find(guid: existing_guid))
+          state = @handler.try(:pollable_job_state) || PollableJobModel::POLLING_STATE
           existing.update(
             delayed_job_guid: job.guid,
-            state: PollableJobModel::POLLING_STATE,
+            state: state,
             operation: @handler.display_name,
             resource_guid: @handler.resource_guid,
             resource_type: @handler.resource_type

app/jobs/v3/delete_service_instance_job.rb

@@ -7,12 +7,17 @@ class DeprovisionBadResponse < StandardError
 
     class DeleteServiceInstanceJob < ServiceInstanceAsyncJob
       def initialize(guid, audit_info)
-        super(guid, audit_info)
+        super
       end
 
       def send_broker_request(client)
-        client.deprovision(service_instance, { accepts_incomplete: true })
+        deprovision_response = client.deprovision(service_instance, { accepts_incomplete: true })
+
+        @request_failed = false
+
+        deprovision_response
       rescue VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse => err
+        @request_failed = true
         raise DeprovisionBadResponse.new(err.message)
       end
 
@@ -40,6 +45,12 @@ def restart_on_failure?
         true
       end
 
+      def pollable_job_state
+        return PollableJobModel::PROCESSING_STATE if @request_failed
+
+        PollableJobModel::POLLING_STATE
+      end
+
       def restart_job(msg)
         super
         logger.info("could not complete the operation: #{msg}. Triggering orphan mitigation")

spec/request/service_instances_spec.rb

@@ -2831,7 +2831,7 @@ def check_filtered_instances(*instances)
               api_call.call(admin_headers)
               execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE)
             end
 
             it 'retries the deprovision' do
@@ -2875,7 +2875,7 @@ def check_filtered_instances(*instances)
               api_call.call(admin_headers)
               execute_all_jobs(expected_successes: 1, expected_failures: 0)
 
-              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::POLLING_STATE)
+              expect(job.state).to eq(VCAP::CloudController::PollableJobModel::PROCESSING_STATE)
             end
 
             it 'retries the deprovision' do

spec/unit/jobs/pollable_job_wrapper_spec.rb

@@ -59,6 +59,20 @@ class BigException < StandardError
           expect(job_record.resource_type).to eq('droplet')
           expect(job_record.cf_api_error).to be_nil
         end
+
+        context 'when the job defines its state' do
+          before do
+            job.define_singleton_method(:pollable_job_state) do
+              'ABRACADABRA'
+            end
+          end
+
+          it 'updates the existing job state accordingly' do
+            enqueued_job = VCAP::CloudController::Jobs::Enqueuer.new(pollable_job).enqueue
+            job_record = VCAP::CloudController::PollableJobModel.find(delayed_job_guid: enqueued_job.guid)
+            expect(job_record.state).to eq('ABRACADABRA')
+          end
+        end
       end
 
       context 'when the job fails' do

spec/unit/jobs/v3/delete_service_instance_job_spec.rb

@@ -28,6 +28,22 @@ module V3
           allow(VCAP::Services::ServiceClientProvider).to receive(:provide).and_return(client)
         end
 
+        context 'when the client succeeds' do
+          let(:r) do
+            VCAP::Services::ServiceBrokers::V2::HttpResponse.new(code: '204', body: 'all good')
+          end
+
+          before do
+            allow(client).to receive(:deprovision).and_return(r)
+          end
+
+          it 'the pollable job state is set to polling' do
+            subject.perform
+
+            expect(subject.pollable_job_state).to eq(PollableJobModel::POLLING_STATE)
+          end
+        end
+
         context 'when the client raises a ServiceBrokerBadResponse' do
           let(:r) do
             VCAP::Services::ServiceBrokers::V2::HttpResponse.new(code: '204', body: 'unexpected failure!')
@@ -48,6 +64,12 @@ module V3
             expect(subject.instance_variable_get(:@first_time)).to eq(true)
           end
 
+          it 'the pollable job state is set to processing' do
+            subject.perform
+
+            expect(subject.pollable_job_state).to eq(PollableJobModel::PROCESSING_STATE)
+          end
+
           it 'does not modify the service instance operation' do
             service_instance.save_with_new_operation(
               {},
@@ -126,6 +148,11 @@ module V3
           expect(response).to eq('some response')
         end
 
+        it 'sets the @request_failed to false' do
+          subject.send_broker_request(client)
+          expect(subject.instance_variable_get(:@request_failed)).to eq(false)
+        end
+
         context 'when the client raises a ServiceBrokerBadResponse' do
           it 'raises a DeprovisionBadResponse error' do
             r = VCAP::Services::ServiceBrokers::V2::HttpResponse.new(code: '204', body: 'unexpected failure!')