Replace "magic" strings with constants or StringInquirer

Rather than comparing various strings this commit shifts to using
constants and/or ActiveSupport's `StringInquirer`

Signed-off-by: Nishad Mathur <nishad.mathur@broadcom.com>

Author: aramprice

src/bosh-director/lib/bosh/director/deployment_plan/instance.rb

@@ -1,9 +1,19 @@
 require 'securerandom'
+require 'active_support/core_ext/string/inquiry'
 
 module Bosh::Director
   module DeploymentPlan
     # Represents a single Instance Group instance.
     class Instance
+      RESTART = 'restart'.freeze
+      RECREATE = 'recreate'.freeze
+      STARTED = 'started'.freeze
+
+      VIRTUAL_STATE_TO_STATE = {
+        RECREATE => STARTED,
+        RESTART => STARTED
+      }
+
       # @return [Integer] Instance index
       attr_reader :index
 
@@ -25,9 +35,6 @@ class Instance
       attr_accessor :desired_variable_set
       attr_reader :previous_variable_set
 
-      # @return [String] job state
-      attr_reader :virtual_state
-
       attr_reader :availability_zone
 
       attr_reader :existing_network_reservations
@@ -259,7 +266,7 @@ def current_packages
       end
 
       def current_job_state
-        @current_state['job_state']
+        @current_state['job_state'].to_s.inquiry
       end
 
       def current_networks
@@ -297,14 +304,11 @@ def update_variable_set
       end
 
       def state
-        case @virtual_state
-        when 'recreate'
-          'started'
-        when 'restart'
-          'started'
-        else
-          @virtual_state
-        end
+        VIRTUAL_STATE_TO_STATE.fetch(virtual_state, virtual_state)
+      end
+
+      def virtual_state
+        @virtual_state.to_s.inquiry
       end
 
       ##

src/bosh-director/lib/bosh/director/deployment_plan/instance_plan.rb

@@ -59,7 +59,7 @@ def should_create_swap_delete?
         def unresponsive_agent?
           return false if @instance.nil?
 
-          @instance.current_job_state == 'unresponsive'
+          @instance.current_job_state.unresponsive?
         end
 
         ##
@@ -118,7 +118,7 @@ def should_be_ignored?
         end
 
         def restart_requested?
-          @instance.virtual_state == 'restart'
+          @instance.virtual_state.restart?
         end
 
         def recreation_requested?
@@ -129,7 +129,7 @@ def recreation_requested?
             @logger.debug("#{__method__} instance should be recreated because of unresponsive agent")
             true
           else
-            @instance.virtual_state == 'recreate'
+            @instance.virtual_state.recreate?
           end
         end
 
@@ -186,16 +186,15 @@ def network_settings_changed?
         end
 
         def state_changed?
-          if instance.state == 'detached' &&
-             existing_instance.state != instance.state
+          if instance.state.detached? && existing_instance.state != instance.state
             @logger.debug("Instance '#{instance}' needs to be detached")
             return true
           end
 
           return true if unresponsive_agent?
 
-          if instance.state == 'stopped' && instance.current_job_state == 'running' ||
-             instance.state == 'started' && instance.current_job_state != 'running'
+          if instance.state.stopped? && instance.current_job_state.running? ||
+              instance.state.started? && !instance.current_job_state.running?
             @logger.debug("Instance state is '#{instance.state}' and agent reports '#{instance.current_job_state}'")
             return true
           end
@@ -399,15 +398,11 @@ def packages_changed?
         end
 
         def already_detached?
-          return false if new?
-
-          @existing_instance.state == 'detached'
+          new? ? false : @existing_instance.detached?
         end
 
         def needs_disk?
-          instance_group = @desired_instance.instance_group
-
-          instance_group.persistent_disk_collection.needs_disk?
+          @desired_instance.instance_group.persistent_disk_collection.needs_disk?
         end
 
         def persist_current_spec

src/bosh-director/lib/bosh/director/instance_updater/update_procedure.rb

@@ -1,6 +1,22 @@
 module Bosh::Director
   class InstanceUpdater
     class UpdateProcedure
+      STARTED = 'started'.freeze
+      STOPPED = 'stopped'.freeze
+      DETACHED = 'detached'.freeze
+
+      RECREATE = 'recreate'.freeze
+      CREATE = 'create'.freeze
+      START = 'start'.freeze
+      STOP = 'stop'.freeze
+      UPDATE = 'update'.freeze
+
+      VIRTUAL_STATE_TO_ACTION_MAPPING = {
+        STARTED => START,
+        STOPPED => STOP,
+        DETACHED => STOP,
+      }
+
       attr_reader :instance, :instance_plan, :options, :instance_report, :action, :context
 
       def initialize(instance,
@@ -178,21 +194,13 @@ def deleting_vm?
 
       def calculate_action
         if restarting?
-          names = {
-            'started' => 'start',
-            'stopped' => 'stop',
-            'detached' => 'stop',
-          }
-
-          raw_name = instance_plan.instance.virtual_state
-          return names[raw_name] if names.key? raw_name
-
-          return raw_name
+          return VIRTUAL_STATE_TO_ACTION_MAPPING.fetch(instance_plan.instance.virtual_state,
+                                                       instance_plan.instance.virtual_state)
         end
 
-        return 'create' if instance_plan.new?
+        return CREATE if instance_plan.new?
 
-        @needs_recreate ? 'recreate' : 'update'
+        @needs_recreate ? RECREATE : UPDATE
       end
 
       def restarting?

src/bosh-director/spec/unit/bosh/director/errand/errand_provider_spec.rb

@@ -29,10 +29,11 @@ module Bosh::Director
       let(:template_blob_cache) { instance_double(Bosh::Director::Core::Templates::TemplateBlobCache) }
       let(:runner) { instance_double(Errand::Runner) }
       let(:errand_step) { instance_double(Errand::LifecycleErrandStep) }
+      let(:current_job_state) { 'dummy'}
       let(:instance) do
         instance_double(
           DeploymentPlan::Instance,
-          current_job_state: double(:current_job_state),
+          current_job_state: current_job_state.inquiry,
           uuid: instance_model.uuid,
           model: instance_model,
         )

src/bosh-director/spec/unit/bosh/director/starter_spec.rb

@@ -1,4 +1,5 @@
 require 'spec_helper'
+require 'active_support/core_ext/string/inquiry'
 
 module Bosh::Director
   describe Starter do
@@ -19,7 +20,7 @@ module Bosh::Director
     let(:instance) do
       instance_double(
         DeploymentPlan::Instance,
-        current_job_state: current_job_state,
+        current_job_state: current_job_state.inquiry,
       )
     end
 

src/bosh-director/spec/unit/bosh/director/stopper_spec.rb

@@ -34,7 +34,7 @@ module Bosh::Director
         rendered_templates_archive: nil,
         configuration_hash: { 'fake-spec' => true },
         template_hashes: [],
-        current_job_state: current_job_state,
+        current_job_state: current_job_state.inquiry,
         deployment_model: deployment_model,
       )
     end