Merge pull request #10952 from chef/mp/chef-10660-2

Signed-off-by: Tim Smith <tsmith@chef.io>

6 files changed, 103 insertions, 13 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index e572f0667d..15fdd87c84 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1062,6 +1062,7 @@ class Chef
     # action for the resource.
     #
     # @param name [Symbol] The action name to define.
+    # @param description [String] optional description for the action
     # @param recipe_block The recipe to run when the action is taken. This block
     #   takes no parameters, and will be evaluated in a new context containing:
     #
@@ -1071,14 +1072,37 @@ class Chef
     #
     # @return The Action class implementing the action
     #
-    def self.action(action, &recipe_block)
+    def self.action(action, description: nil, &recipe_block)
       action = action.to_sym
       declare_action_class
       action_class.action(action, &recipe_block)
       self.allowed_actions += [ action ]
+      # Accept any non-nil description, which will correctly override
+      # any specific inherited description.
+      action_descriptions[action] = description unless description.nil?
       default_action action if Array(default_action) == [:nothing]
     end
 
+    # Retrieve the description for a resource's action, if
+    # any description has been included in the definition.
+    #
+    # @param action [Symbol,String] the action name
+    # @return the description of the action provided, or nil if no description
+    # was defined
+    def self.action_description(action)
+      action_descriptions[action.to_sym]
+    end
+
+    # @api private
+    #
+    # @return existing action description hash, or newly-initialized
+    # hash containing action descriptions inherited from parent Resource,
+    # if any.
+    def self.action_descriptions
+      @action_descriptions ||=
+        superclass.respond_to?(:action_descriptions) ? superclass.action_descriptions.dup : { nothing: nil }
+    end
+
     # Define a method to load up this resource's properties with the current
     # actual values.
     #
@@ -1196,9 +1220,9 @@ class Chef
     #
 
     # FORBIDDEN_IVARS do not show up when the resource is converted to JSON (ie. hidden from data_collector and sending to the chef server via #to_json/to_h/as_json/inspect)
-    FORBIDDEN_IVARS = %i{@run_context @logger @not_if @only_if @enclosing_provider @description @introduced @examples @validation_message @deprecated @default_description @skip_docs @executed_by_runner}.freeze
+    FORBIDDEN_IVARS = %i{@run_context @logger @not_if @only_if @enclosing_provider @description @introduced @examples @validation_message @deprecated @default_description @skip_docs @executed_by_runner @action_descriptions}.freeze
     # HIDDEN_IVARS do not show up when the resource is displayed to the user as text (ie. in the error inspector output via #to_text)
-    HIDDEN_IVARS = %i{@allowed_actions @resource_name @source_line @run_context @logger @name @not_if @only_if @elapsed_time @enclosing_provider @description @introduced @examples @validation_message @deprecated @default_description @skip_docs @executed_by_runner}.freeze
+    HIDDEN_IVARS = %i{@allowed_actions @resource_name @source_line @run_context @logger @name @not_if @only_if @elapsed_time @enclosing_provider @description @introduced @examples @validation_message @deprecated @default_description @skip_docs @executed_by_runner @action_descriptions}.freeze
 
     include Chef::Mixin::ConvertToClassName
     extend Chef::Mixin::ConvertToClassName
diff --git a/lib/chef/resource_inspector.rb b/lib/chef/resource_inspector.rb
index 6d320f4202..95ae110170 100644
--- a/lib/chef/resource_inspector.rb
+++ b/lib/chef/resource_inspector.rb
@@ -41,7 +41,11 @@ class Chef
       data[:description] = resource.description
       # data[:deprecated] = resource.deprecated || false
       data[:default_action] = resource.default_action
-      data[:actions] = resource.allowed_actions
+      data[:actions] = {}
+      resource.allowed_actions.each do |action|
+        data[:actions][action] = resource.action_description(action)
+      end
+
       data[:examples] = resource.examples
       data[:introduced] = resource.introduced
       data[:preview] = resource.preview_resource
diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb
index 9bfe86c74e..febac64283 100644
--- a/spec/integration/recipes/resource_action_spec.rb
+++ b/spec/integration/recipes/resource_action_spec.rb
@@ -223,6 +223,10 @@ module ResourceActionSpec
           ActionJackson.succeeded = ActionJackson.ruby_block_converged
         end
 
+        action :test1, description: "Original description" do
+          true
+        end
+
         def foo_public
           "foo_public!"
         end
@@ -293,7 +297,12 @@ module ResourceActionSpec
             ActionJackalope.jackalope_ran = :access_attribute
             ActionJackalope.succeeded = ActionJackson.succeeded
           end
+
+          action :test1, description: "An old action with a new description" do
+            super
+          end
         end
+
         before do
           ActionJackalope.jackalope_ran = nil
           ActionJackalope.load_current_resource_ran = nil
@@ -344,6 +353,11 @@ module ResourceActionSpec
           expect(ActionJackalope.succeeded).to eq "foo!alope blarghle! bar!alope"
         end
 
+        it "allows overridden action to have a description separate from the action defined in the base resource" do
+          expect(ActionJackson.action_description(:test1)).to eql "Original description"
+          expect(ActionJackalope.action_description(:test1)).to eql "An old action with a new description"
+        end
+
         it "non-overridden actions run and can access overridden and non-overridden variables (but not necessarily new ones)" do
           converge do
             action_jackalope "hi" do
diff --git a/spec/unit/resource_inspector_spec.rb b/spec/unit/resource_inspector_spec.rb
index f3a4b2aa0a..f786018b29 100644
--- a/spec/unit/resource_inspector_spec.rb
+++ b/spec/unit/resource_inspector_spec.rb
@@ -28,7 +28,11 @@ class DummyResource < Chef::Resource
   introduced "14.0"
   property :first, String, description: "My First Property", introduced: "14.0"
 
-  action :dummy do
+  action :dummy, description: "Dummy action" do
+    return true
+  end
+
+  action :dummy_no_desc do
     return true
   end
 end
@@ -39,7 +43,8 @@ describe Chef::ResourceInspector do
 
     it "returns a hash with required data" do
       expect(subject[:description]).to eq "A dummy resource"
-      expect(subject[:actions]).to match_array %i{nothing dummy}
+      expect(subject[:actions]).to eq({ nothing: nil, dummy: "Dummy action",
+                                        dummy_no_desc: nil })
     end
 
     context "excluding built in properties" do
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 7a19e0e8e1..f7109cc680 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -1162,6 +1162,52 @@ describe Chef::Resource do
     end
   end
 
+  describe "#action_description" do
+    class TestResource < ::Chef::Resource
+      action :symbol_action, description: "a symbol test" do; end
+      action "string_action", description: "a string test" do; end
+      action :base_action0 do; end
+      action :base_action1, description: "unmodified base action 1 desc" do; end
+      action :base_action2, description: "unmodified base action 2 desc" do; end
+      action :base_action3, description: "unmodified base action 3 desc" do; end
+    end
+
+    it "returns nil when no description was provided for the action" do
+      expect(TestResource.action_description(:base_action0)).to eql(nil)
+    end
+
+    context "when action definition is a string" do
+      it "returns the description whether a symbol or string is used to look it up" do
+        expect(TestResource.action_description("string_action")).to eql("a string test")
+        expect(TestResource.action_description(:string_action)).to eql("a string test")
+      end
+    end
+
+    context "when action definition is a symbol" do
+      it "returns the description whether a symbol or string is used to look up" do
+        expect(TestResource.action_description("symbol_action")).to eql("a symbol test")
+        expect(TestResource.action_description(:symbol_action)).to eql("a symbol test")
+      end
+    end
+
+    context "when inheriting from an existing resource" do
+      class TestResourceChild < TestResource
+        action :base_action2, description: "modified base action 2 desc" do; end
+        action :base_action3 do; end
+      end
+
+      it "returns original description when a described action is not overridden in child resource" do
+        expect(TestResourceChild.action_description(:base_action1)).to eq "unmodified base action 1 desc"
+      end
+      it "returns original description when the child resource overrides an inherited action but NOT its description" do
+        expect(TestResourceChild.action_description(:base_action3)).to eq "unmodified base action 3 desc"
+      end
+      it "returns new description when the child resource overrides an inherited action and its description" do
+        expect(TestResourceChild.action_description(:base_action2)).to eq "modified base action 2 desc"
+      end
+    end
+  end
+
   describe ".default_action" do
     let(:default_action) {}
     let(:resource_class) do
diff --git a/tasks/docs.rb b/tasks/docs.rb
index b815988b8b..5d336a7b14 100755
--- a/tasks/docs.rb
+++ b/tasks/docs.rb
@@ -126,17 +126,14 @@ namespace :docs_site do
 
     #
     # Build the actions section of the resource yaml
+    # as a hash of actions to markdown descriptions.
     #
     # @return [Hash]
     #
     def action_list(actions)
-      list = {}
-      actions.sort.each do |action|
-        # nothing is a special case that sources the content from the docs site
-        list[action.to_sym] = (action == "nothing" ? { "shortcode" => "resources_common_actions_nothing.md" } : { "markdown" => nil })
-      end
-
-      list
+      actions = actions.map { |k, v| [k.to_sym, { "markdown" => v } ] }.to_h
+      actions[:nothing] = { "shortcode" => "resources_common_actions_nothing.md" }
+      actions
     end
 
     # TODO: