Merge pull request #3605 from coderanger/action

Rework Resource#action to match the 12.3 API.

26 files changed, 131 insertions, 51 deletions

diff --git a/lib/chef/guard_interpreter/resource_guard_interpreter.rb b/lib/chef/guard_interpreter/resource_guard_interpreter.rb
index d4b386a15a..6372b206a7 100644
--- a/lib/chef/guard_interpreter/resource_guard_interpreter.rb
+++ b/lib/chef/guard_interpreter/resource_guard_interpreter.rb
@@ -68,7 +68,8 @@ class Chef
         run_action = action || @resource.action
 
         begin
-          @resource.run_action(run_action)
+          # Coerce to an array to be safe.
+          Array(run_action).each {|action_to_run| @resource.run_action(action_to_run) }
           resource_updated = @resource.updated
         rescue Mixlib::ShellOut::ShellCommandFailed
           resource_updated = nil
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 8d2532dac4..12d4f007ba 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -169,26 +169,24 @@ class Chef
     # @param arg [Array[Symbol], Symbol] A list of actions (e.g. `:create`)
     # @return [Array[Symbol]] the list of actions.
     #
-    attr_accessor :action
     def action(arg=nil)
       if arg
-        if arg.is_a?(Array)
-          arg = arg.map { |a| a.to_sym }
-        else
-          arg = arg.to_sym
-        end
-        Array(arg).each do |action|
+        arg = Array(arg).map(&:to_sym)
+        arg.each do |action|
           validate(
             { action: action },
             { action: { kind_of: Symbol, equal_to: allowed_actions } }
           )
         end
-        self.action = arg
+        @action = arg
       else
         @action
       end
     end
 
+    # Alias for normal assigment syntax.
+    alias_method :action=, :action
+
     #
     # Sets up a notification that will run a particular action on another resource
     # if and when *this* resource is updated by an action.
@@ -1095,22 +1093,17 @@ class Chef
     # Setting default_action will automatially add the action to
     # allowed_actions, if it isn't already there.
     #
-    # Defaults to :nothing.
+    # Defaults to [:nothing].
     #
     # @param action_name [Symbol,Array<Symbol>] The default action (or series
     #   of actions) to use.
     #
-    # @return [Symbol,Array<Symbol>] The default actions for the resource.
+    # @return [Array<Symbol>] The default actions for the resource.
     #
     def self.default_action(action_name=NOT_PASSED)
       unless action_name.equal?(NOT_PASSED)
-        if action_name.is_a?(Array)
-          @default_action = action_name.map { |arg| arg.to_sym }
-        else
-          @default_action = action_name.to_sym
-        end
-
-        self.allowed_actions |= Array(@default_action)
+        @default_action = Array(action_name).map(&:to_sym)
+        self.allowed_actions |= @default_action
       end
 
       if @default_action
@@ -1118,7 +1111,7 @@ class Chef
       elsif superclass.respond_to?(:default_action)
         superclass.default_action
       else
-        :nothing
+        [:nothing]
       end
     end
     def self.default_action=(action_name)
@@ -1159,7 +1152,7 @@ class Chef
       action = action.to_sym
       new_action_provider_class.action(action, &recipe_block)
       self.allowed_actions += [ action ]
-      default_action action if default_action == :nothing
+      default_action action if Array(default_action) == [:nothing]
     end
 
     #
diff --git a/spec/functional/provider/whyrun_safe_ruby_block_spec.rb b/spec/functional/provider/whyrun_safe_ruby_block_spec.rb
index b3c2333e9a..2b582feb05 100644
--- a/spec/functional/provider/whyrun_safe_ruby_block_spec.rb
+++ b/spec/functional/provider/whyrun_safe_ruby_block_spec.rb
@@ -43,7 +43,7 @@ describe Chef::Resource::WhyrunSafeRubyBlock do
     end
 
     it "updates the evil laugh, even in why-run mode" do
-      new_resource.run_action(new_resource.action)
+      Array(new_resource.action).each {|action| new_resource.run_action(action) }
       expect($evil_global_evil_laugh).to eq(:mwahahaha)
       expect(new_resource).to be_updated
     end
diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb
index 4786294803..cee79133a9 100644
--- a/spec/integration/recipes/resource_action_spec.rb
+++ b/spec/integration/recipes/resource_action_spec.rb
@@ -337,7 +337,7 @@ describe "Resource.action" do
           NoActionJackson.action_was = action
         end
       }
-      expect(NoActionJackson.action_was).to eq :nothing
+      expect(NoActionJackson.action_was).to eq [:nothing]
     end
   end
 end
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index 784ff966cd..55df9cb8fa 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -163,7 +163,7 @@ describe "LWRP" do
       it "Should load the old content, and not the new" do
         resource = Chef::ResourceResolver.resolve(:lwrp_foo)
         expect(resource).to eq @original_resource
-        expect(resource.default_action).to eq(:pass_buck)
+        expect(resource.default_action).to eq([:pass_buck])
         expect(Chef.method_defined?(:method_created_by_override_lwrp_foo)).to be_falsey
       end
     end
@@ -202,7 +202,7 @@ describe "LWRP" do
     end
 
     it "should set the specified action as the default action" do
-      expect(get_lwrp(:lwrp_foo).new("blah").action).to eq(:pass_buck)
+      expect(get_lwrp(:lwrp_foo).new("blah").action).to eq([:pass_buck])
     end
 
     it "should create a method for each attribute" do
@@ -419,7 +419,7 @@ describe "LWRP" do
         end
 
         it "delegates #default_action to the parent" do
-          expect(child.default_action).to eq(:eat)
+          expect(child.default_action).to eq([:eat])
         end
       end
 
@@ -436,7 +436,7 @@ describe "LWRP" do
         end
 
         it "does not delegate #default_action to the parent" do
-          expect(child.default_action).to eq(:dont_eat)
+          expect(child.default_action).to eq([:dont_eat])
         end
       end
 
diff --git a/spec/unit/provider/deploy_spec.rb b/spec/unit/provider/deploy_spec.rb
index 63658ac601..f6bb78823f 100644
--- a/spec/unit/provider/deploy_spec.rb
+++ b/spec/unit/provider/deploy_spec.rb
@@ -622,7 +622,7 @@ describe Chef::Provider::Deploy do
 
       gems = @provider.send(:gem_packages)
 
-      expect(gems.map { |g| g.action }).to eq([:install])
+      expect(gems.map { |g| g.action }).to eq([%i{install}])
       expect(gems.map { |g| g.name }).to eq(%w{eventmachine})
       expect(gems.map { |g| g.version }).to eq(%w{0.12.9})
     end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index 17ea498fe3..511e7e9397 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -409,8 +409,8 @@ describe Chef::Recipe do
       end
 
       it "does not copy the action from the first resource" do
-        expect(original_resource.action).to eq(:score)
-        expect(duplicated_resource.action).to eq(:nothing)
+        expect(original_resource.action).to eq([:score])
+        expect(duplicated_resource.action).to eq([:nothing])
       end
 
       it "does not copy the source location of the first resource" do
@@ -505,7 +505,7 @@ describe Chef::Recipe do
       recipe.from_file(File.join(CHEF_SPEC_DATA, "recipes", "test.rb"))
       res = recipe.resources(:file => "/etc/nsswitch.conf")
       expect(res.name).to eql("/etc/nsswitch.conf")
-      expect(res.action).to eql(:create)
+      expect(res.action).to eql([:create])
       expect(res.owner).to eql("root")
       expect(res.group).to eql("root")
       expect(res.mode).to eql(0644)
diff --git a/spec/unit/resource/breakpoint_spec.rb b/spec/unit/resource/breakpoint_spec.rb
index 9c867ebcc7..88ab34d568 100644
--- a/spec/unit/resource/breakpoint_spec.rb
+++ b/spec/unit/resource/breakpoint_spec.rb
@@ -37,7 +37,7 @@ describe Chef::Resource::Breakpoint do
   end
 
   it "defaults to the break action" do
-    expect(@breakpoint.action).to eq(:break)
+    expect(@breakpoint.action).to eq([:break])
   end
 
   it "names itself after the line number of the file where it's created" do
diff --git a/spec/unit/resource/cron_spec.rb b/spec/unit/resource/cron_spec.rb
index 743552c1de..0978be6930 100644
--- a/spec/unit/resource/cron_spec.rb
+++ b/spec/unit/resource/cron_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::Cron do
   end
 
   it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   it "should accept create or delete for action" do
diff --git a/spec/unit/resource/directory_spec.rb b/spec/unit/resource/directory_spec.rb
index c452b2a914..e9e80806db 100644
--- a/spec/unit/resource/directory_spec.rb
+++ b/spec/unit/resource/directory_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::Directory do
   end
 
   it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   it "should accept create or delete for action" do
diff --git a/spec/unit/resource/dsc_resource_spec.rb b/spec/unit/resource/dsc_resource_spec.rb
index ae15f56eaf..06769d86ce 100644
--- a/spec/unit/resource/dsc_resource_spec.rb
+++ b/spec/unit/resource/dsc_resource_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::DscResource do
     }
 
     it "has a default action of `:run`" do
-      expect(dsc_test_resource.action).to eq(:run)
+      expect(dsc_test_resource.action).to eq([:run])
     end
 
     it "has an allowed_actions attribute with only the `:run` and `:nothing` attributes" do
diff --git a/spec/unit/resource/dsc_script_spec.rb b/spec/unit/resource/dsc_script_spec.rb
index 71103ea590..4361b35b91 100644
--- a/spec/unit/resource/dsc_script_spec.rb
+++ b/spec/unit/resource/dsc_script_spec.rb
@@ -29,7 +29,7 @@ describe Chef::Resource::DscScript do
       Chef::RunContext.new(node, {}, empty_events)
     }
     let(:dsc_test_resource) {
-      Chef::Resource::DscScript.new(dsc_test_resource_name, dsc_test_run_context) 
+      Chef::Resource::DscScript.new(dsc_test_resource_name, dsc_test_run_context)
     }
     let(:configuration_code) {'echo "This is supposed to create a configuration document."'}
     let(:configuration_path) {'c:/myconfigs/formatc.ps1'}
@@ -38,7 +38,7 @@ describe Chef::Resource::DscScript do
     let(:configuration_data_script) { 'c:/myconfigs/data/safedata.psd1' }
 
     it "has a default action of `:run`" do
-      expect(dsc_test_resource.action).to eq(:run)
+      expect(dsc_test_resource.action).to eq([:run])
     end
 
     it "has an allowed_actions attribute with only the `:run` and `:nothing` attributes" do
diff --git a/spec/unit/resource/env_spec.rb b/spec/unit/resource/env_spec.rb
index 566827a27e..9bee07c593 100644
--- a/spec/unit/resource/env_spec.rb
+++ b/spec/unit/resource/env_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::Env do
   end
 
   it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   { :create => false, :delete => false, :modify => false, :flibber => true }.each do |action,bad_value|
diff --git a/spec/unit/resource/erl_call_spec.rb b/spec/unit/resource/erl_call_spec.rb
index 008d27372a..9abf2e7812 100644
--- a/spec/unit/resource/erl_call_spec.rb
+++ b/spec/unit/resource/erl_call_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::ErlCall do
   end
 
   it "should have a default action of run" do
-    expect(@resource.action).to eql(:run)
+    expect(@resource.action).to eql([:run])
   end
 
   it "should accept run as an action" do
diff --git a/spec/unit/resource/file_spec.rb b/spec/unit/resource/file_spec.rb
index dd20f5f03a..76beaf15e1 100644
--- a/spec/unit/resource/file_spec.rb
+++ b/spec/unit/resource/file_spec.rb
@@ -29,7 +29,7 @@ describe Chef::Resource::File do
   end
 
   it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   it "should have a default content of nil" do
diff --git a/spec/unit/resource/group_spec.rb b/spec/unit/resource/group_spec.rb
index bcf9205f7e..a4029fc911 100644
--- a/spec/unit/resource/group_spec.rb
+++ b/spec/unit/resource/group_spec.rb
@@ -50,7 +50,7 @@ describe Chef::Resource::Group, "initialize" do
   end
 
   it "should set action to :create" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   %w{create remove modify manage}.each do |action|
diff --git a/spec/unit/resource/link_spec.rb b/spec/unit/resource/link_spec.rb
index 51221e0472..0246fcd13b 100644
--- a/spec/unit/resource/link_spec.rb
+++ b/spec/unit/resource/link_spec.rb
@@ -36,7 +36,7 @@ describe Chef::Resource::Link do
   end
 
   it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   { :create => false, :delete => false, :blues => true }.each do |action,bad_value|
diff --git a/spec/unit/resource/mdadm_spec.rb b/spec/unit/resource/mdadm_spec.rb
index 866309ec5b..6ca99c58e5 100644
--- a/spec/unit/resource/mdadm_spec.rb
+++ b/spec/unit/resource/mdadm_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::Mdadm do
   end
 
   it "should have a default action of create" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   it "should accept create, assemble, stop as actions" do
diff --git a/spec/unit/resource/mount_spec.rb b/spec/unit/resource/mount_spec.rb
index ad95c06e04..acce26dcab 100644
--- a/spec/unit/resource/mount_spec.rb
+++ b/spec/unit/resource/mount_spec.rb
@@ -38,7 +38,7 @@ describe Chef::Resource::Mount do
   end
 
   it "should have a default action of mount" do
-    expect(@resource.action).to eql(:mount)
+    expect(@resource.action).to eql([:mount])
   end
 
   it "should accept mount, umount and remount as actions" do
diff --git a/spec/unit/resource/ohai_spec.rb b/spec/unit/resource/ohai_spec.rb
index fe29755abf..3bc21a41d2 100644
--- a/spec/unit/resource/ohai_spec.rb
+++ b/spec/unit/resource/ohai_spec.rb
@@ -34,7 +34,7 @@ describe Chef::Resource::Ohai do
   end
 
   it "should have a default action of create" do
-    expect(@resource.action).to eql(:reload)
+    expect(@resource.action).to eql([:reload])
   end
 
   it "should allow you to set the plugin attribute" do
diff --git a/spec/unit/resource/registry_key_spec.rb b/spec/unit/resource/registry_key_spec.rb
index e2a864d73a..2e2811d026 100644
--- a/spec/unit/resource/registry_key_spec.rb
+++ b/spec/unit/resource/registry_key_spec.rb
@@ -45,7 +45,7 @@ describe Chef::Resource::RegistryKey, "initialize" do
   end
 
   it "should set action to :create" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   %w{create create_if_missing delete delete_key}.each do |action|
diff --git a/spec/unit/resource/route_spec.rb b/spec/unit/resource/route_spec.rb
index ffb9304511..ec1d369932 100644
--- a/spec/unit/resource/route_spec.rb
+++ b/spec/unit/resource/route_spec.rb
@@ -35,7 +35,7 @@ describe Chef::Resource::Route do
   end
 
   it "should have a default action of 'add'" do
-    expect(@resource.action).to eql(:add)
+    expect(@resource.action).to eql([:add])
   end
 
   it "should accept add or delete for action" do
diff --git a/spec/unit/resource/ruby_block_spec.rb b/spec/unit/resource/ruby_block_spec.rb
index 5d83f7e367..8664564ac5 100644
--- a/spec/unit/resource/ruby_block_spec.rb
+++ b/spec/unit/resource/ruby_block_spec.rb
@@ -30,8 +30,8 @@ describe Chef::Resource::RubyBlock do
     expect(@resource).to be_a_kind_of(Chef::Resource::RubyBlock)
   end
 
-  it "should have a default action of 'create'" do
-    expect(@resource.action).to eql(:run)
+  it "should have a default action of 'run'" do
+    expect(@resource.action).to eql([:run])
   end
 
   it "should have a resource name of :ruby_block" do
@@ -46,7 +46,7 @@ describe Chef::Resource::RubyBlock do
 
   it "allows the action to be 'create'" do
     @resource.action :create
-    expect(@resource.action).to eq(:create)
+    expect(@resource.action).to eq([:create])
   end
 
   describe "when it has been initialized with block code" do
diff --git a/spec/unit/resource/user_spec.rb b/spec/unit/resource/user_spec.rb
index f05de94fe0..3bf7e6187b 100644
--- a/spec/unit/resource/user_spec.rb
+++ b/spec/unit/resource/user_spec.rb
@@ -43,7 +43,7 @@ describe Chef::Resource::User, "initialize" do
   end
 
   it "should set action to :create" do
-    expect(@resource.action).to eql(:create)
+    expect(@resource.action).to eql([:create])
   end
 
   it "should set supports[:manage_home] to false" do
diff --git a/spec/unit/resource/windows_service_spec.rb b/spec/unit/resource/windows_service_spec.rb
index 8866cad1bf..45a295c24e 100644
--- a/spec/unit/resource/windows_service_spec.rb
+++ b/spec/unit/resource/windows_service_spec.rb
@@ -44,6 +44,6 @@ describe Chef::Resource::WindowsService, "initialize" do
 
   it "allows the action to be 'configure_startup'" do
     resource.action :configure_startup
-    expect(resource.action).to eq(:configure_startup)
+    expect(resource.action).to eq([:configure_startup])
   end
 end
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 8b0baff921..353c5ec129 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -977,4 +977,90 @@ describe Chef::Resource do
     end
 
   end
+
+  describe "#action" do
+    let(:resource_class) do
+      Class.new(described_class) do
+        allowed_actions(%i{one two})
+      end
+    end
+    let(:resource) { resource_class.new('test', nil) }
+    subject { resource.action }
+
+    context "with a no action" do
+      it { is_expected.to eq [:nothing] }
+    end
+
+    context "with a default action" do
+      let(:resource_class) do
+        Class.new(described_class) do
+          default_action(:one)
+        end
+      end
+      it { is_expected.to eq [:one] }
+    end
+
+    context "with a symbol action" do
+      before { resource.action(:one) }
+      it { is_expected.to eq [:one] }
+    end
+
+    context "with a string action" do
+      before { resource.action('two') }
+      it { is_expected.to eq [:two] }
+    end
+
+    context "with an array action" do
+      before { resource.action([:two, :one]) }
+      it { is_expected.to eq [:two, :one] }
+    end
+
+    context "with an assignment" do
+      before { resource.action = :one }
+      it { is_expected.to eq [:one] }
+    end
+
+    context "with an array assignment" do
+      before { resource.action = [:two, :one] }
+      it { is_expected.to eq [:two, :one] }
+    end
+
+    context "with an invalid action" do
+      it { expect { resource.action(:three) }.to raise_error Chef::Exceptions::ValidationFailed }
+    end
+
+    context "with an invalid assignment action" do
+      it { expect { resource.action = :three }.to raise_error Chef::Exceptions::ValidationFailed }
+    end
+  end
+
+  describe ".default_action" do
+    let(:default_action) { }
+    let(:resource_class) do
+      actions = default_action
+      Class.new(described_class) do
+        default_action(actions) if actions
+      end
+    end
+    subject { resource_class.default_action }
+
+    context "with no default actions" do
+      it { is_expected.to eq [:nothing] }
+    end
+
+    context "with a symbol default action" do
+      let(:default_action) { :one }
+      it { is_expected.to eq [:one] }
+    end
+
+    context "with a string default action" do
+      let(:default_action) { 'one' }
+      it { is_expected.to eq [:one] }
+    end
+
+    context "with an array default action" do
+      let(:default_action) { [:two, :one] }
+      it { is_expected.to eq [:two, :one] }
+    end
+  end
 end