diff options
author | Tim Smith <tsmith@chef.io> | 2018-03-08 19:10:51 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-03-08 19:10:51 -0800 |
commit | b34c1b8dcef7e03ad0833ac2b208922713eb02e1 (patch) | |
tree | bc16e8f2e32bd23be18dacffeb7cced6171dad64 | |
parent | f95b79d0d2748c1b9d571d78926f73ae0131207e (diff) | |
parent | bdd124437898927df9e9f7934a08bba0da58edc5 (diff) | |
download | chef-b34c1b8dcef7e03ad0833ac2b208922713eb02e1.tar.gz |
Merge pull request #6964 from chef/tm/backport_resource_enhancements
Backport RFC-101/RFC-104 resource enhancements
-rw-r--r-- | lib/chef/mixin/params_validate.rb | 28 | ||||
-rw-r--r-- | lib/chef/mixin/properties.rb | 13 | ||||
-rw-r--r-- | lib/chef/property.rb | 22 | ||||
-rw-r--r-- | lib/chef/provider.rb | 14 | ||||
-rw-r--r-- | lib/chef/resource.rb | 25 | ||||
-rw-r--r-- | spec/unit/mixin/params_validate_spec.rb | 13 | ||||
-rw-r--r-- | spec/unit/mixin/properties_spec.rb | 14 | ||||
-rw-r--r-- | spec/unit/property/validation_spec.rb | 9 | ||||
-rw-r--r-- | spec/unit/resource_spec.rb | 31 |
9 files changed, 154 insertions, 15 deletions
diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index d90e38b916..c955dd3b12 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob (<adam@chef.io>) -# Copyright:: Copyright 2008-2017, Chef Software Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -35,6 +35,8 @@ class Chef # map options are: # # @param opts [Hash<Symbol,Object>] Validation opts. + # @option opts [String] :validation_message A custom message to return + # should validation fail. # @option opts [Object,Array] :is An object, or list of # objects, that must match the value using Ruby's `===` operator # (`opts[:is].any? { |v| v === value }`). (See #_pv_is.) @@ -91,16 +93,20 @@ class Chef raise ArgumentError, "Options must be a hash" unless opts.kind_of?(Hash) raise ArgumentError, "Validation Map must be a hash" unless map.kind_of?(Hash) + @validation_message ||= {} + map.each do |key, validation| unless key.kind_of?(Symbol) || key.kind_of?(String) raise ArgumentError, "Validation map keys must be symbols or strings!" end + case validation when true _pv_required(opts, key) when false true when Hash + @validation_message[key] = validation.delete(:validation_message) if validation.has_key?(:validation_message) validation.each do |check, carg| check_method = "_pv_#{check}" if respond_to?(check_method, true) @@ -129,6 +135,10 @@ class Chef validation.has_key?(:is) && _pv_is({ key => nil }, key, validation[:is], raise_error: false) end + def _validation_message(key, default) + @validation_message.has_key?(key) ? @validation_message[key] : default + end + # Return the value of a parameter, or nil if it doesn't exist. def _pv_opts_lookup(opts, key) if opts.has_key?(key.to_s) @@ -145,7 +155,7 @@ class Chef if is_required return true if opts.has_key?(key.to_s) && (explicitly_allows_nil || !opts[key.to_s].nil?) return true if opts.has_key?(key.to_sym) && (explicitly_allows_nil || !opts[key.to_sym].nil?) - raise Exceptions::ValidationFailed, "Required argument #{key.inspect} is missing!" + raise Exceptions::ValidationFailed, _validation_message(key, "Required argument #{key.inspect} is missing!") end true end @@ -168,7 +178,7 @@ class Chef to_be.each do |tb| return true if value == tb end - raise Exceptions::ValidationFailed, "Option #{key} must be equal to one of: #{to_be.join(", ")}! You passed #{value.inspect}." + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must be equal to one of: #{to_be.join(", ")}! You passed #{value.inspect}.") end end @@ -187,7 +197,7 @@ class Chef to_be.each do |tb| return true if value.kind_of?(tb) end - raise Exceptions::ValidationFailed, "Option #{key} must be a kind of #{to_be}! You passed #{value.inspect}." + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must be a kind of #{to_be}! You passed #{value.inspect}.") end end @@ -202,7 +212,7 @@ class Chef unless value.nil? Array(method_name_list).each do |method_name| unless value.respond_to?(method_name) - raise Exceptions::ValidationFailed, "Option #{key} must have a #{method_name} method!" + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} must have a #{method_name} method!") end end end @@ -234,7 +244,7 @@ class Chef if value.respond_to?(predicate_method) if value.send(predicate_method) - raise Exceptions::ValidationFailed, "Option #{key} cannot be #{predicate_method_base_name}" + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key} cannot be #{predicate_method_base_name}") end end end @@ -294,7 +304,7 @@ class Chef Array(regex).flatten.each do |r| return true if r.match(value.to_s) end - raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}" + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}") end end @@ -316,7 +326,7 @@ class Chef if !value.nil? callbacks.each do |message, zeproc| unless zeproc.call(value) - raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} #{message}!" + raise Exceptions::ValidationFailed, _validation_message(key, "Option #{key}'s value #{value} #{message}!") end end end @@ -428,7 +438,7 @@ class Chef unless errors.empty? message << " Errors:\n#{errors.map { |m| "- #{m}" }.join("\n")}" end - raise Exceptions::ValidationFailed, message + raise Exceptions::ValidationFailed, _validation_message(key, message) end end diff --git a/lib/chef/mixin/properties.rb b/lib/chef/mixin/properties.rb index 8ff2cc4501..6b95b87063 100644 --- a/lib/chef/mixin/properties.rb +++ b/lib/chef/mixin/properties.rb @@ -75,6 +75,8 @@ class Chef # will return if the user does not set one. If this is `lazy`, it will # be run in the context of the instance (and able to access other # properties). + # @option options [String] :description A description of the property. + # @option options [String] :introduced The release that introduced this property # @option options [Boolean] :desired_state `true` if this property is # part of desired state. Defaults to `true`. # @option options [Boolean] :identity `true` if this property @@ -301,6 +303,17 @@ class Chef raise ArgumentError, "Property #{name} is not defined in class #{self}" if !property property.reset(self) end + + # + # The description of the property + # + # @param name [Symbol] The name of the property. + # @return [String] The description of the property. + def property_description(name) + property = self.class.properties[name.to_sym] + raise ArgumentError, "Property #{name} is not defined in class #{self}" if !property + property.description + end end end end diff --git a/lib/chef/property.rb b/lib/chef/property.rb index 9d0957dcdf..be54c8bfa1 100644 --- a/lib/chef/property.rb +++ b/lib/chef/property.rb @@ -60,10 +60,12 @@ class Chef # options). # @option options [Symbol] :name The name of this property. # @option options [Class] :declared_in The class this property comes from. + # @option options [String] :description A description of the property. # @option options [Symbol] :instance_variable_name The instance variable # tied to this property. Must include a leading `@`. Defaults to `@<name>`. # `nil` means the property is opaque and not tied to a specific instance # variable. + # @option options [String] :introduced The release that introduced this property # @option options [Boolean] :desired_state `true` if this property is part of desired # state. Defaults to `true`. # @option options [Boolean] :identity `true` if this property is part of object @@ -158,6 +160,24 @@ class Chef end # + # A description of this property. + # + # @return [String] + # + def description + options[:description] + end + + # + # When this property was introduced + # + # @return [String] + # + def introduced + options[:introduced] + end + + # # The instance variable associated with this property. # # Defaults to `@<name>` @@ -252,7 +272,7 @@ class Chef # def validation_options @validation_options ||= options.reject do |k, v| - [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable, :sensitive].include?(k) + [:declared_in, :name, :instance_variable_name, :desired_state, :identity, :default, :name_property, :coerce, :required, :nillable, :sensitive, :description, :introduced].include?(k) end end diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb index 9e9013b24e..3a6aee711f 100644 --- a/lib/chef/provider.rb +++ b/lib/chef/provider.rb @@ -198,6 +198,20 @@ class Chef @requirements ||= ResourceRequirements.new(@new_resource, run_context) end + def description(description = "NOT_PASSED") + if description != "NOT_PASSED" + @description = description + end + @description + end + + def introduced(introduced = "NOT_PASSED") + if introduced != "NOT_PASSED" + @introduced = introduced + end + @introduced + end + def converge_by(descriptions, &block) converge_actions.add_action(descriptions, &block) end diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 5436e3ceb3..743619585d 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -1179,8 +1179,8 @@ class Chef # Internal Resource Interface (for Chef) # - FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider] - HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider] + FORBIDDEN_IVARS = [:@run_context, :@not_if, :@only_if, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message] + HIDDEN_IVARS = [:@allowed_actions, :@resource_name, :@source_line, :@run_context, :@name, :@not_if, :@only_if, :@elapsed_time, :@enclosing_provider, :@description, :@introduced, :@examples, :@validation_message] include Chef::Mixin::ConvertToClassName extend Chef::Mixin::ConvertToClassName @@ -1379,6 +1379,27 @@ class Chef end end + def self.description(description = "NOT_PASSED") + if description != "NOT_PASSED" + @description = description + end + @description + end + + def self.introduced(introduced = "NOT_PASSED") + if introduced != "NOT_PASSED" + @introduced = introduced + end + @introduced + end + + def self.examples(examples = "NOT_PASSED") + if examples != "NOT_PASSED" + @examples = examples + end + @examples + end + # # The cookbook in which this Resource was defined (if any). # diff --git a/spec/unit/mixin/params_validate_spec.rb b/spec/unit/mixin/params_validate_spec.rb index 0cafb925c8..7bc8a27398 100644 --- a/spec/unit/mixin/params_validate_spec.rb +++ b/spec/unit/mixin/params_validate_spec.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob (<adam@chef.io>) -# Copyright:: Copyright 2008-2016, Chef Software Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -342,6 +342,17 @@ describe Chef::Mixin::ParamsValidate do end.to raise_error(Chef::Exceptions::ValidationFailed) end + it "allows a custom validation message" do + expect do + @vo.validate({ :not_blank => "should pass" }, + { :not_blank => { :cannot_be => [ :nil, :empty ], validation_message: "my validation message" } }) + end.not_to raise_error + expect do + @vo.validate({ :not_blank => "" }, + { :not_blank => { :cannot_be => [ :nil, :empty ], validation_message: "my validation message" } }) + end.to raise_error(Chef::Exceptions::ValidationFailed, "my validation message") + end + it "should set and return a value, then return the same value" do value = "meow" expect(@vo.set_or_return(:test, value, {}).object_id).to eq(value.object_id) diff --git a/spec/unit/mixin/properties_spec.rb b/spec/unit/mixin/properties_spec.rb index 1af0bc7abd..ee0c252381 100644 --- a/spec/unit/mixin/properties_spec.rb +++ b/spec/unit/mixin/properties_spec.rb @@ -11,6 +11,7 @@ module ChefMixinPropertiesSpec property :a, "a", default: "a" property :ab, %w{a b}, default: "a" property :ac, %w{a c}, default: "a" + property :d, "d", description: "The d property", introduced: "14.0" end context "and a module B with properties b, ab and bc" do @@ -30,11 +31,20 @@ module ChefMixinPropertiesSpec end it "A.properties has a, ab, and ac with types 'a', ['a', 'b'], and ['b', 'c']" do - expect(A.properties.keys).to eq [ :a, :ab, :ac ] + expect(A.properties.keys).to eq [ :a, :ab, :ac, :d ] expect(A.properties[:a].validation_options[:is]).to eq "a" expect(A.properties[:ab].validation_options[:is]).to eq %w{a b} expect(A.properties[:ac].validation_options[:is]).to eq %w{a c} end + + it "A.properties can get the description of `d`" do + expect(A.properties[:d].description).to eq "The d property" + end + + it "A.properties can get the release that introduced `d`" do + expect(A.properties[:d].introduced).to eq "14.0" + end + it "B.properties has b, ab, and bc with types 'b', nil and ['b', 'c']" do expect(B.properties.keys).to eq [ :b, :ab, :bc ] expect(B.properties[:b].validation_options[:is]).to eq "b" @@ -42,7 +52,7 @@ module ChefMixinPropertiesSpec expect(B.properties[:bc].validation_options[:is]).to eq %w{b c} end it "C.properties has a, b, c, ac and bc with merged types" do - expect(C.properties.keys).to eq [ :a, :ab, :ac, :b, :bc, :c ] + expect(C.properties.keys).to eq [ :a, :ab, :ac, :d, :b, :bc, :c ] expect(C.properties[:a].validation_options[:is]).to eq "a" expect(C.properties[:b].validation_options[:is]).to eq "b" expect(C.properties[:c].validation_options[:is]).to eq "c" diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb index 13afcdfbc2..882ea3353b 100644 --- a/spec/unit/property/validation_spec.rb +++ b/spec/unit/property/validation_spec.rb @@ -699,4 +699,13 @@ describe "Chef::Resource.property validation" do end end end + + context "custom validation messages" do + with_property ":x, String, validation_message: 'Must be a string, fool'" do + it "raise with the correct error message" do + expect { resource.x 1 }.to raise_error Chef::Exceptions::ValidationFailed, + "Must be a string, fool" + end + end + end end diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb index 9227c8b12d..7972f2077d 100644 --- a/spec/unit/resource_spec.rb +++ b/spec/unit/resource_spec.rb @@ -387,6 +387,37 @@ describe Chef::Resource do end end + context "Documentation of resources" do + it "can have a description" do + c = Class.new(Chef::Resource) do + description "my description" + end + expect(c.description).to eq "my description" + end + + it "can say when it was introduced" do + c = Class.new(Chef::Resource) do + introduced "14.0" + end + expect(c.introduced).to eq "14.0" + end + + it "can have some examples" do + c = Class.new(Chef::Resource) do + examples <<-EOH +resource "foo" do + foo foo +end + EOH + end + expect(c.examples).to eq <<-EOH +resource "foo" do + foo foo +end + EOH + end + end + describe "self.resource_name" do context "When resource_name is not set" do it "and there are no provides lines, resource_name is nil" do |