CR feedback: move command evaluation to guard interpreter

2 files changed, 38 insertions, 37 deletions

diff --git a/lib/chef/guard_interpreter/default_guard_interpreter.rb b/lib/chef/guard_interpreter/default_guard_interpreter.rb
index f7d039c1cf..df91c2b1ad 100644
--- a/lib/chef/guard_interpreter/default_guard_interpreter.rb
+++ b/lib/chef/guard_interpreter/default_guard_interpreter.rb
@@ -19,16 +19,22 @@
 class Chef
   class GuardInterpreter
     class DefaultGuardInterpreter
+      include Chef::Mixin::ShellOut
 
       protected
 
-      def initialize
+      def initialize(command, opts)
+        @command = command
+        @command_opts = opts
       end
 
       public
 
-      def translate_command_block(command, opts, &block)
-        [command, block]
+      def evaluate
+        shell_out(@command, @command_opts).status.success?
+      rescue Chef::Exceptions::CommandTimeout
+        Chef::Log.warn "Command '#{@command}' timed out"
+        false
       end
     end
   end
diff --git a/lib/chef/guard_interpreter/resource_guard_interpreter.rb b/lib/chef/guard_interpreter/resource_guard_interpreter.rb
index c4b6d33a16..c2a2b1d6b4 100644
--- a/lib/chef/guard_interpreter/resource_guard_interpreter.rb
+++ b/lib/chef/guard_interpreter/resource_guard_interpreter.rb
@@ -22,34 +22,13 @@ class Chef
   class GuardInterpreter
     class ResourceGuardInterpreter < DefaultGuardInterpreter
 
-      def translate_command_block(command, opts, &block)
-        merge_inherited_attributes
-
-        if command && ! block_given?
-          block_attributes = opts.merge({:code => command})
-
-          # Handles cases like powershell_script where default
-          # attributes are different when used in a guard vs. not. For
-          # powershell_script in particular, this will go away when
-          # the one attribue that causes this changes its default to be
-          # the same after some period to prepare for deprecation
-          if @resource.class.respond_to?(:get_default_attributes)
-            block_attributes = @resource.class.send(:get_default_attributes, opts).merge(block_attributes)
-          end
-
-          translated_block = to_block(block_attributes)
-          [nil, translated_block]
-        else
-          super
-        end
-      end
-
-      def initialize(resource_symbol, parent_resource)
+      def initialize(parent_resource, command, opts, &block)
+        super(command, opts)
         @parent_resource = parent_resource
 
-        resource_class = get_resource_class(parent_resource, resource_symbol)
+        resource_class = get_resource_class(parent_resource)
 
-        raise ArgumentError, "Specified guard_interpreter resource #{resource_symbol.to_s} unknown for this platform" if resource_class.nil?
+        raise ArgumentError, "Specified guard_interpreter resource #{parent_resource.guard_interpreter.to_s} unknown for this platform" if resource_class.nil?
 
         empty_events = Chef::EventDispatch::Dispatcher.new
         anonymous_run_context = Chef::RunContext.new(parent_resource.node, {}, empty_events)
@@ -61,6 +40,29 @@ class Chef
         end
       end
 
+      def evaluate
+        # Add attributes inherited from the parent class
+        # to the resource
+        merge_inherited_attributes
+
+        # Script resources have a code attribute, which is
+        # what is used to execute the command, so include
+        # that with attributes specified by caller in opts
+        block_attributes = @command_opts.merge({:code => @command})
+
+        # Handles cases like powershell_script where default
+        # attributes are different when used in a guard vs. not. For
+        # powershell_script in particular, this will go away when
+        # the one attribue that causes this changes its default to be
+        # the same after some period to prepare for deprecation
+        if @resource.class.respond_to?(:get_default_attributes)
+          block_attributes = @resource.class.send(:get_default_attributes, @command_opts).merge(block_attributes)
+        end
+
+        resource_block = block_from_attributes(block_attributes)
+        evaluate_action(nil, &resource_block)
+      end
+
       protected
 
       def evaluate_action(action=nil, &block)
@@ -78,18 +80,11 @@ class Chef
         resource_updated
       end
 
-      def to_block(attributes, action=nil)
-        resource_block = block_from_attributes(attributes)
-        Proc.new do
-          evaluate_action(action, &resource_block)
-        end
-      end
-
-      def get_resource_class(parent_resource, resource_symbol)
+      def get_resource_class(parent_resource)
         if parent_resource.nil? || parent_resource.node.nil?
           raise ArgumentError, "Node for guard resource parent must not be nil"
         end
-        Chef::Resource.resource_for_node(resource_symbol, parent_resource.node)
+        Chef::Resource.resource_for_node(parent_resource.guard_interpreter, parent_resource.node)
       end
 
       def block_from_attributes(attributes)