From 8dad40d9189b2e478a7a2a533d51c87f0aa79213 Mon Sep 17 00:00:00 2001 From: "Marc A. Paradise" Date: Fri, 29 Jan 2021 16:28:17 -0500 Subject: Add support for resource action descriptions This will allow us to generate better documentation by including action descriptions in the resource help, and includes updates to the resource inspector and doc generation task to include action descriptions in resource action data. This change permits actions to be defined with the optional new 'description' parameter as follows: ``` action :my_action, description: "my_action changes the state of this world" ... end Action description can be looked up using the `.action_description(action)` method of the resource class. Action descriptions are inheritable and overridable. Signed-off-by: Marc A. Paradise --- lib/chef/resource.rb | 30 ++++++++++++++-- lib/chef/resource_inspector.rb | 6 +++- spec/integration/recipes/resource_action_spec.rb | 14 ++++++++ spec/unit/resource_inspector_spec.rb | 9 +++-- spec/unit/resource_spec.rb | 46 ++++++++++++++++++++++++ tasks/docs.rb | 11 +++--- 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: -- cgit v1.2.1