diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2017-03-27 17:50:51 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-03-27 17:50:51 -0700 |
commit | 11fb879cb780c004fa010fb5cacddeb92bfa5529 (patch) | |
tree | 42a01d96af9e0a75ac06bc347356ff5efddeff84 | |
parent | d3d8d0d4bf99f7a8e76bd0cfd583f583ad3de830 (diff) | |
parent | d82ac7a7c66ffcf6120092f536dd41a9c06db872 (diff) | |
download | chef-11fb879cb780c004fa010fb5cacddeb92bfa5529.tar.gz |
Merge pull request #5946 from chef/lcg/fix-action-class
Fix action class weirdness in Chef-13
-rw-r--r-- | lib/chef/provider_resolver.rb | 9 | ||||
-rw-r--r-- | lib/chef/resource.rb | 82 | ||||
-rw-r--r-- | spec/integration/recipes/resource_action_spec.rb | 61 |
3 files changed, 88 insertions, 64 deletions
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb index 439a7e9f5f..16e57b00b8 100644 --- a/lib/chef/provider_resolver.rb +++ b/lib/chef/provider_resolver.rb @@ -58,6 +58,7 @@ class Chef def resolve maybe_explicit_provider(resource) || + maybe_custom_resource(resource) || maybe_dynamic_provider_resolution(resource, action) || raise(Chef::Exceptions::ProviderNotFound, "Cannot find a provider for #{resource} on #{node["platform"]} version #{node["platform_version"]}") end @@ -105,10 +106,14 @@ class Chef end end + # if its a custom resource, just grab the action class + def maybe_custom_resource(resource) + resource.class.action_class if resource.class.custom_resource? + end + # if resource.provider is set, just return one of those objects def maybe_explicit_provider(resource) - return nil unless resource.provider - resource.provider + resource.provider if resource.provider end # try dynamically finding a provider based on querying the providers to see what they support diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index f0d816cd89..8992594933 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -752,8 +752,7 @@ class Chef else arg end - set_or_return(:provider, klass, kind_of: [ Class ]) || - self.class.action_class + set_or_return(:provider, klass, kind_of: [ Class ]) end def provider=(arg) @@ -1122,53 +1121,66 @@ class Chef end # - # The action class is an automatic `Provider` created to handle - # actions declared by `action :x do ... end`. + # The action class is a `Chef::Provider` which is created at Resource + # class evaluation time when the Custom Resource is being constructed. # - # This class will be returned by `resource.provider` if `resource.provider` - # is not set. `provider_for_action` will also use this instead of calling - # out to `Chef::ProviderResolver`. - # - # If the user has not declared actions on this class or its superclasses - # using `action :x do ... end`, then there is no need for this class and - # `action_class` will be `nil`. + # This happens the first time the ruby parser hits an `action` or an + # `action_class` method, the presence of either indiates that this is + # going to be a Chef-12.5 custom resource. If we never see one of these + # directives then we are constructing an old-style Resource+Provider or + # LWRP or whatevs. # # If a block is passed, the action_class is always created and the block is # run inside it. # - # @api private - # def self.action_class(&block) - return @action_class if @action_class && !block - # If the superclass needed one, then we need one as well. - if block || (superclass.respond_to?(:action_class) && superclass.action_class) - @action_class = declare_action_class(&block) - end + @action_class ||= declare_action_class + @action_class.class_eval(&block) if block @action_class end + # Returns true or false based on if the resource is a custom resource. The + # top-level Chef::Resource is not a chef resource. This value is inherited. + # + # @return [Boolean] if the resource is a custom_resource + def self.custom_resource? + false + end + + # This sets the resource to being a custom resource, and does so in a way + # that automatically inherits to all subclasses via defining a method on + # the class (class variables and class instance variables don't have the + # correct semantics here, this is a poor man's activesupport class_attribute) + # + # @api private + def self.is_custom_resource! + define_singleton_method :custom_resource? do + true + end + end + # # Ensure the action class actually gets created. This is called # when the user does `action :x do ... end`. # - # If a block is passed, it is run inside the action_class. - # # @api private - def self.declare_action_class(&block) - @action_class ||= begin - if superclass.respond_to?(:action_class) - base_provider = superclass.action_class - end - base_provider ||= Chef::Provider - - resource_class = self - Class.new(base_provider) do - include ActionClass - self.resource_class = resource_class - end - end - @action_class.class_eval(&block) if block - @action_class + def self.declare_action_class + @action_class ||= + begin + is_custom_resource! + base_provider = + if superclass.custom_resource? + superclass.action_class + else + Chef::Provider + end + + resource_class = self + Class.new(base_provider) do + include ActionClass + self.resource_class = resource_class + end + end end # diff --git a/spec/integration/recipes/resource_action_spec.rb b/spec/integration/recipes/resource_action_spec.rb index 7a68164d41..03331c8625 100644 --- a/spec/integration/recipes/resource_action_spec.rb +++ b/spec/integration/recipes/resource_action_spec.rb @@ -502,40 +502,28 @@ module ResourceActionSpec end end - context "When a resource declares methods in action_class and declare_action_class" do + context "When a resource declares methods in action_class" do class DeclaresActionClassMethods < Chef::Resource use_automatic_resource_name property :x - action :create do - new_resource.x = a + b + c + d - end action_class do def a 1 end end - declare_action_class do - def b - 2 - end - end - action_class do + action_class.class_eval <<-EOM def c 3 end - end - declare_action_class do - def d - 4 - end + EOM + action :create do + new_resource.x = a + c end end it "the methods are not available on the resource" do expect { DeclaresActionClassMethods.new("hi").a }.to raise_error(NameError) - expect { DeclaresActionClassMethods.new("hi").b }.to raise_error(NameError) expect { DeclaresActionClassMethods.new("hi").c }.to raise_error(NameError) - expect { DeclaresActionClassMethods.new("hi").d }.to raise_error(NameError) end it "the methods are available to the action" do @@ -543,17 +531,14 @@ module ResourceActionSpec expect_recipe do r = declares_action_class_methods "hi" end.to emit_no_warnings_or_errors - expect(r.x).to eq(10) + expect(r.x).to eq(4) end - context "And a subclass also creates a method" do + context "And a subclass overrides a method with an action_class block" do class DeclaresActionClassMethodsToo < DeclaresActionClassMethods use_automatic_resource_name - action :create do - new_resource.x a + b + c + d + e - end action_class do - def e + def a 5 end end @@ -561,10 +546,7 @@ module ResourceActionSpec it "the methods are not available on the resource" do expect { DeclaresActionClassMethods.new("hi").a }.to raise_error(NameError) - expect { DeclaresActionClassMethods.new("hi").b }.to raise_error(NameError) expect { DeclaresActionClassMethods.new("hi").c }.to raise_error(NameError) - expect { DeclaresActionClassMethods.new("hi").d }.to raise_error(NameError) - expect { DeclaresActionClassMethods.new("hi").e }.to raise_error(NameError) end it "the methods are available to the action" do @@ -572,7 +554,32 @@ module ResourceActionSpec expect_recipe do r = declares_action_class_methods_too "hi" end.to emit_no_warnings_or_errors - expect(r.x).to eq(15) + expect(r.x).to eq(8) + end + end + + context "And a subclass overrides a method with class_eval" do + # this tests inheritance with *only* an action_class accessor that does not declare a block + class DeclaresActionClassMethodsToo < DeclaresActionClassMethods + use_automatic_resource_name + action_class.class_eval <<-EOM + def a + 5 + end + EOM + end + + it "the methods are not available on the resource" do + expect { DeclaresActionClassMethods.new("hi").a }.to raise_error(NameError) + expect { DeclaresActionClassMethods.new("hi").c }.to raise_error(NameError) + end + + it "the methods are available to the action" do + r = nil + expect_recipe do + r = declares_action_class_methods_too "hi" + end.to emit_no_warnings_or_errors + expect(r.x).to eq(8) end end end |