Converge actions all together instead of in action :x (fixes #4124)jk/4124

2 files changed, 108 insertions, 57 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index d4cce075e5..3ddf33844f 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -351,10 +351,20 @@ class Chef
     # @api private
     module InlineResources
 
-      # Our run context is a child of the main run context; that gives us a
-      # whole new resource collection and notification set.
-      def initialize(resource, run_context)
-        super(resource, run_context.create_child)
+      # Create a child run_context, compile the block, and converge it.
+      #
+      # @api private
+      def compile_and_converge_action(&block)
+        old_run_context = run_context
+        @run_context = run_context.create_child
+        return_value = instance_eval(&block)
+        Chef::Runner.new(run_context).converge
+        return_value
+      ensure
+        if run_context.resource_collection.any? { |r| r.updated? }
+          new_resource.updated_by_last_action(true)
+        end
+        @run_context = old_run_context
       end
 
       # Class methods for InlineResources. Overrides the `action` DSL method
@@ -366,36 +376,7 @@ class Chef
         # compile the resources, converging them, and then checking if any
         # were updated (and updating new-resource if so)
         def action(name, &block)
-          # We first try to create the method using "def method_name", which is
-          # preferred because it actually shows up in stack traces. If that
-          # fails, we try define_method.
-          begin
-            class_eval <<-EOM, __FILE__, __LINE__+1
-              def action_#{name}
-                return_value = compile_action_#{name}
-                Chef::Runner.new(run_context).converge
-                return_value
-              ensure
-                if run_context.resource_collection.any? {|r| r.updated? }
-                  new_resource.updated_by_last_action(true)
-                end
-              end
-            EOM
-          rescue SyntaxError
-            define_method("action_#{name}") do
-              begin
-                return_value = send("compile_action_#{name}")
-                Chef::Runner.new(run_context).converge
-                return_value
-              ensure
-                if run_context.resource_collection.any? {|r| r.updated? }
-                  new_resource.updated_by_last_action(true)
-                end
-              end
-            end
-          end
-          # We put the action in its own method so that super() works.
-          define_method("compile_action_#{name}", &block)
+          define_method("action_#{name}") { compile_and_converge_action(&block) }
         end
       end
 
diff --git a/spec/integration/recipes/lwrp_inline_resources_spec.rb b/spec/integration/recipes/lwrp_inline_resources_spec.rb
index e70605d3d3..73b7d428f0 100644
--- a/spec/integration/recipes/lwrp_inline_resources_spec.rb
+++ b/spec/integration/recipes/lwrp_inline_resources_spec.rb
@@ -18,35 +18,105 @@ describe "LWRPs with inline resources" do
   # cf. CHEF-4914
   let(:chef_client) { "ruby '#{chef_dir}/chef-client' --minimal-ohai" }
 
+  context "with a use_inline_resources provider with 'def action_a' instead of action :a" do
+    class LwrpInlineResourcesTest < Chef::Resource::LWRPBase
+      resource_name :lwrp_inline_resources_test
+      actions :a, :nothing
+      default_action :a
+      property :ran_a
+      class Provider < Chef::Provider::LWRPBase
+        provides :lwrp_inline_resources_test
+        use_inline_resources
+        def action_a
+          r = new_resource
+          ruby_block 'run a' do
+            block { r.ran_a "ran a" }
+          end
+        end
+      end
+    end
+
+    it "this is totally a bug, but for backcompat purposes, it adds the resources to the main resource collection and does not get marked updated" do
+      r = nil
+      expect_recipe {
+        r = lwrp_inline_resources_test 'hi'
+      }.to have_updated('ruby_block[run a]', :run)
+      expect(r.ran_a).to eq "ran a"
+    end
+  end
+
+  context "with an inline_resources provider with two actions, one calling the other" do
+    class LwrpInlineResourcesTest2 < Chef::Resource::LWRPBase
+      resource_name :lwrp_inline_resources_test2
+      actions :a, :b, :nothing
+      default_action :b
+      property :ran_a
+      property :ran_b
+      class Provider < Chef::Provider::LWRPBase
+        provides :lwrp_inline_resources_test2
+        use_inline_resources
+
+        action :a do
+          r = new_resource
+          ruby_block 'run a' do
+            block { r.ran_a "ran a" }
+          end
+        end
+
+        action :b do
+          action_a
+          r = new_resource
+          # Grab ran_a right now, before we converge
+          ran_a = r.ran_a
+          ruby_block 'run b' do
+            block { r.ran_b "ran b: ran_a value was #{ran_a.inspect}" }
+          end
+        end
+      end
+    end
+
+    it "resources declared in b are executed immediately inline", :focus do
+      r = nil
+      expect_recipe {
+        r = lwrp_inline_resources_test2 'hi' do
+          action :b
+        end
+      }.to have_updated('lwrp_inline_resources_test2[hi]', :b).
+       and have_updated('ruby_block[run a]', :run).
+       and have_updated('ruby_block[run b]', :run)
+      expect(r.ran_b).to eq "ran b: ran_a value was \"ran a\""
+    end
+  end
+
   when_the_repository "has a cookbook with a nested LWRP" do
     before do
       directory 'cookbooks/x' do
 
-        file 'resources/do_nothing.rb', <<EOM
-actions :create, :nothing
-default_action :create
-EOM
-        file 'providers/do_nothing.rb', <<EOM
-action :create do
-end
-EOM
+        file 'resources/do_nothing.rb', <<-EOM
+          actions :create, :nothing
+          default_action :create
+        EOM
+        file 'providers/do_nothing.rb', <<-EOM
+          action :create do
+          end
+        EOM
 
-        file 'resources/my_machine.rb', <<EOM
-actions :create, :nothing
-default_action :create
-EOM
-        file 'providers/my_machine.rb', <<EOM
-use_inline_resources
-action :create do
-  x_do_nothing 'a'
-  x_do_nothing 'b'
-end
-EOM
+        file 'resources/my_machine.rb', <<-EOM
+          actions :create, :nothing
+          default_action :create
+        EOM
+        file 'providers/my_machine.rb', <<-EOM
+          use_inline_resources
+          action :create do
+            x_do_nothing 'a'
+            x_do_nothing 'b'
+          end
+        EOM
 
-        file 'recipes/default.rb', <<EOM
-x_my_machine "me"
-x_my_machine "you"
-EOM
+        file 'recipes/default.rb', <<-EOM
+          x_my_machine "me"
+          x_my_machine "you"
+        EOM
 
       end # directory 'cookbooks/x'
     end