Merge branch 'CHEF-3432'

4 files changed, 70 insertions, 30 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 729632a69c..bf5e43443e 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -132,7 +132,10 @@ F
     # 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 8a714e75b7..370ccd8c10 100644
--- a/lib/chef/resource/lwrp_base.rb
+++ b/lib/chef/resource/lwrp_base.rb
@@ -39,8 +39,13 @@ class Chef
 
         # 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 a158bce440..3e807bc06c 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 ffcc7ae5b4..964946a339 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -35,6 +35,21 @@ describe Chef::Resource do
     @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