diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 729632a69c9..bf5e43443e1 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -132,7 +132,10 @@ def fix_notifier_reference(resource_collection) # Track all subclasses of Resource. This is used so names can be looked up # when attempting to deserialize from JSON. (See: json_compat) def self.resource_classes - @resource_classes ||= [] + # Using a class variable here ensures we have one variable to track + # subclasses shared by the entire class hierarchy; without this, each + # subclass would have its own list of subclasses. + @@resource_classes ||= [] end # Callback when subclass is defined. Adds subclass to list of subclasses. diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb index 8a714e75b72..370ccd8c10c 100644 --- a/lib/chef/resource/lwrp_base.rb +++ b/lib/chef/resource/lwrp_base.rb @@ -39,8 +39,13 @@ def self.build_from_file(cookbook_name, filename, run_context) # Add log entry if we override an existing light-weight resource. class_name = convert_to_class_name(rname) - overriding = Chef::Resource.const_defined?(class_name) - Chef::Log.info("#{class_name} light-weight resource already initialized -- overriding!") if overriding + if Resource.const_defined?(class_name) + old_class = Resource.send(:remove_const, class_name) + # CHEF-3432 -- Chef::Resource keeps a list of subclasses; need to + # remove old ones from the list when replacing. + resource_classes.delete(old_class) + Chef::Log.info("#{class_name} light-weight resource already initialized -- overriding!") + end resource_class = Class.new(self) diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb index a158bce4402..3e807bc06c6 100644 --- a/spec/unit/lwrp_spec.rb +++ b/spec/unit/lwrp_spec.rb @@ -18,43 +18,60 @@ require 'spec_helper' -describe "override logging" do - before :each do - $stderr.stub!(:write) - end - it "should log if attempting to load resource of same name" do - Dir[File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "lwrp", "resources", "*"))].each do |file| - Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) - end +describe "LWRP" do + before do + @original_VERBOSE = $VERBOSE + $VERBOSE = nil + end - Dir[File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "lwrp_override", "resources", "*"))].each do |file| - Chef::Log.should_receive(:info).with(/overriding/) - Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) - end + after do + $VERBOSE = @original_VERBOSE end - it "should log if attempting to load provider of same name" do - Dir[File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "lwrp", "providers", "*"))].each do |file| - Chef::Provider::LWRPBase.build_from_file("lwrp", file, nil) + describe "when overriding an existing class" do + before :each do + $stderr.stub!(:write) end - Dir[File.expand_path(File.join(File.dirname(__FILE__), "..", "data", "lwrp_override", "providers", "*"))].each do |file| - Chef::Log.should_receive(:info).with(/overriding/) - Chef::Provider::LWRPBase.build_from_file("lwrp", file, nil) + it "should log if attempting to load resource of same name" do + Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file| + Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) + end + + Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file| + Chef::Log.should_receive(:info).with(/overriding/) + Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) + end end - end -end + it "should log if attempting to load provider of same name" do + Dir[File.expand_path( "lwrp/providers/*", CHEF_SPEC_DATA)].each do |file| + Chef::Provider::LWRPBase.build_from_file("lwrp", file, nil) + end -describe "LWRP" do - before do - @original_VERBOSE = $VERBOSE - $VERBOSE = nil - end + Dir[File.expand_path( "lwrp/providers/*", CHEF_SPEC_DATA)].each do |file| + Chef::Log.should_receive(:info).with(/overriding/) + Chef::Provider::LWRPBase.build_from_file("lwrp", file, nil) + end + end + + it "removes the old LRWP resource class from the list of resource subclasses [CHEF-3432]" do + # CHEF-3432 regression test: + # Chef::Resource keeps a list of all subclasses to assist class inflation + # for json parsing (see Chef::JSONCompat). When replacing LWRP resources, + # we need to ensure the old resource class is remove from that list. + Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file| + Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) + end + first_lwr_foo_class = Chef::Resource::LwrpFoo + Chef::Resource.resource_classes.should include(first_lwr_foo_class) + Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file| + Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) + end + Chef::Resource.resource_classes.should_not include(first_lwr_foo_class) + end - after do - $VERBOSE = @original_VERBOSE end describe "Lightweight Chef::Resource" do diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb index ffcc7ae5b40..964946a3392 100644 --- a/spec/unit/resource_spec.rb +++ b/spec/unit/resource_spec.rb @@ -35,6 +35,21 @@ class ResourceTestHarness < Chef::Resource @resource = Chef::Resource.new("funk", @run_context) end + describe "when inherited", :focus do + + it "adds an entry to a list of subclasses" do + subclass = Class.new(Chef::Resource) + Chef::Resource.resource_classes.should include(subclass) + end + + it "keeps track of subclasses of subclasses" do + subclass = Class.new(Chef::Resource) + subclass_of_subclass = Class.new(subclass) + Chef::Resource.resource_classes.should include(subclass_of_subclass) + end + + end + describe "when declaring the identity attribute" do it "has no identity attribute by default" do Chef::Resource.identity_attr.should be_nil