Fix issue with displaying Tasks that no longer have a Droplet (#4469)

- Although a Task must have a Droplet associated with it when it is
  created, these Droplets will be eventually cleaned up as the app is
  pushed with newer code over time. This change updates the presenter
  to return null when a Task's user can no longer be looked up because
  its droplet has been deleted.
- Fixes #4467

Author: tcdowney

app/models/runtime/task_model.rb

@@ -56,8 +56,13 @@ def run_action_user
       end
     end
 
-    delegate :docker?, to: :droplet
-    delegate :cnb?, to: :droplet
+    def docker?
+      !!droplet&.docker?
+    end
+
+    def cnb?
+      !!droplet&.cnb?
+    end
 
     private
 

app/presenters/v3/task_presenter.rb

@@ -15,7 +15,7 @@ def to_hash
                          sequence_id: task.sequence_id,
                          name: task.name,
                          command: task.command,
-                         user: task.run_action_user,
+                         user: presented_user(task),
                          state: task.state,
                          memory_in_mb: task.memory_in_mb,
                          disk_in_mb: task.disk_in_mb,
@@ -37,6 +37,14 @@ def task
           @resource
         end
 
+        def presented_user(task)
+          if task.droplet.present?
+            task.run_action_user
+          else
+            task.user
+          end
+        end
+
         def build_links
           {
             self: { href: url_builder.build_url(path: "/v3/tasks/#{task.guid}") },

spec/unit/models/runtime/task_model_spec.rb

@@ -166,12 +166,12 @@ module VCAP::CloudController
 
     describe '#run_action_user' do
       let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
-      let(:droplet) { DropletModel.make }
+      let(:droplet) { DropletModel.make(app: parent_app) }
 
       context 'when the task belongs to a CNB lifecycle app' do
         let(:parent_app) { AppModel.make(:cnb) }
         let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
-        let(:droplet) { DropletModel.make(:cnb) }
+        let(:droplet) { DropletModel.make(:cnb, app: parent_app) }
 
         context 'when the task has a user specified' do
           before do
@@ -197,8 +197,8 @@ module VCAP::CloudController
       context 'when the task belongs to a Docker lifecycle app' do
         let(:parent_app) { AppModel.make(:docker) }
         let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
-        let(:droplet) { DropletModel.make(:docker) }
-        let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"cnb"}' }
+        let(:droplet) { DropletModel.make(:docker, app: parent_app) }
+        let(:droplet_execution_metadata) { '{"entrypoint":["/image-entrypoint.sh"],"user":"some-user"}' }
 
         before do
           task.droplet.update(execution_metadata: droplet_execution_metadata)
@@ -216,7 +216,7 @@ module VCAP::CloudController
 
         context 'when the droplet execution metadata specifies a user' do
           it 'returns the specified user' do
-            expect(task.run_action_user).to eq('cnb')
+            expect(task.run_action_user).to eq('some-user')
           end
         end
 
@@ -268,6 +268,91 @@ module VCAP::CloudController
           it 'returns the default "vcap" user' do
             expect(task.run_action_user).to eq('vcap')
           end
+
+          context 'when there is no droplet for the task' do
+            before do
+              task.droplet.delete
+              task.reload
+            end
+
+            it 'returns vcap' do
+              expect(task.run_action_user).to eq('vcap')
+            end
+          end
+        end
+      end
+    end
+
+    describe 'docker?' do
+      context 'when there is a droplet and it has the docker lifecycle' do
+        let(:parent_app) { AppModel.make(:docker) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:docker, app: parent_app) }
+
+        it 'returns true' do
+          expect(task.docker?).to be(true)
+        end
+      end
+
+      context 'when there is a droplet and it does not have the docker lifecycle' do
+        let(:parent_app) { AppModel.make(:docker) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:buildpack, app: parent_app) }
+
+        it 'returns false' do
+          expect(task.docker?).to be(false)
+        end
+      end
+
+      context 'when there is not a droplet for the task' do
+        let(:parent_app) { AppModel.make(:docker) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:docker, app: parent_app) }
+
+        before do
+          task.droplet.delete
+          task.reload
+        end
+
+        it 'returns false' do
+          expect(task.docker?).to be(false)
+        end
+      end
+    end
+
+    describe 'cnb?' do
+      context 'when there is a droplet and it has the cnb lifecycle' do
+        let(:parent_app) { AppModel.make(:cnb) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:cnb, app: parent_app) }
+
+        it 'returns true' do
+          expect(task.cnb?).to be(true)
+        end
+      end
+
+      context 'when there is a droplet and it does not have the cnb lifecycle' do
+        let(:parent_app) { AppModel.make(:cnb) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:buildpack, app: parent_app) }
+
+        it 'returns false' do
+          expect(task.cnb?).to be(false)
+        end
+      end
+
+      context 'when there is not a droplet for the task' do
+        let(:parent_app) { AppModel.make(:cnb) }
+        let(:task) { TaskModel.make(app: parent_app, droplet: droplet, state: TaskModel::SUCCEEDED_STATE) }
+        let(:droplet) { DropletModel.make(:cnb, app: parent_app) }
+
+        before do
+          task.droplet.delete
+          task.reload
+        end
+
+        it 'returns false' do
+          expect(task.cnb?).to be(false)
         end
       end
     end

spec/unit/presenters/v3/task_presenter_spec.rb

@@ -4,9 +4,11 @@
 module VCAP::CloudController::Presenters::V3
   RSpec.describe TaskPresenter do
     subject(:presenter) { TaskPresenter.new(task) }
+    let(:task_user) { nil }
     let(:task) do
       task = VCAP::CloudController::TaskModel.make(
         failure_reason: 'sup dawg',
+        user: task_user,
         memory_in_mb: 2048,
         disk_in_mb: 4048,
         log_rate_limit: 1024,
@@ -83,6 +85,27 @@ module VCAP::CloudController::Presenters::V3
           expect(result).not_to have_key(:command)
         end
       end
+
+      describe 'user' do
+        context 'when the droplet for the task has been deleted' do
+          before do
+            task.droplet.delete
+            task.reload
+          end
+
+          it 'returns the user as nil' do
+            expect(result[:user]).to be_nil
+          end
+
+          context 'when the task has an explicit user set' do
+            let(:task_user) { 'TestUser' }
+
+            it 'returns the user' do
+              expect(result[:user]).to eq('TestUser')
+            end
+          end
+        end
+      end
     end
   end
 end