Merge pull request #3623 from chef/jdm/simplify-lwrp1

Simplify LWRP Deprecations Proposal 1

3 files changed, 153 insertions, 184 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index b406f898da..43fbaade73 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1300,9 +1300,11 @@ class Chef
         Chef::Resource.send(:remove_const, class_name)
       end
 
-      Chef::Resource.const_set(class_name, resource_class)
+      if !Chef::Config[:treat_deprecation_warnings_as_errors]
+        Chef::Resource.const_set(class_name, resource_class)
+        deprecated_constants[class_name.to_sym] = resource_class
+      end
 
-      deprecated_constants[class_name.to_sym] = resource_class
     end
 
     def self.deprecated_constants
diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb
index 8089b585ed..ef3c2b5bba 100644
--- a/lib/chef/resource/lwrp_base.rb
+++ b/lib/chef/resource/lwrp_base.rb
@@ -52,52 +52,17 @@ class Chef
           resource_name = filename_to_qualified_string(cookbook_name, filename)
 
           # We load the class first to give it a chance to set its own name
-          deprecated_resource_class = Class.new(self).tap do |resource_class|
-            resource_class.resource_name(nil)
-            resource_class.run_context = run_context
-            resource_class.instance_eval do
-              define_method(:initialize) do |*args, &block|
-                Chef::Log::deprecation("Deprecated Thing") if chef_deprecated_access
-                super(*args, &block)
-              end
-              define_method(:chef_deprecated_access) do
-                true
-              end
-            end
-
-            resource_class.class_from_file(filename)
-
-            # Make a useful string for the class (rather than <Class:312894723894>)
-            resource_class.instance_eval do
-              define_singleton_method(:to_s) do
-                "LWRP resource #{resource_name} from cookbook #{cookbook_name}"
-              end
-              define_singleton_method(:inspect) { to_s }
-            end
-          end
-
-          resource_class = Class.new(deprecated_resource_class).tap do |resource_class|
-            resource_class.resource_name(resource_name.to_sym)
-            resource_class.run_context = run_context
-            resource_class.instance_eval do
-              define_method(:chef_deprecated_access) do
-                false
-              end
-            end
-            resource_class.class_eval do
-              define_singleton_method(:===) do |instance|
-                super(instance) || instance.class <= deprecated_resource_class
-              end
-            end
-          end
-
-          deprecated_resource_class.class_eval do
-            define_method(:kind_of?) do |klass|
-              super(klass) || klass == resource_class
-            end
-            define_method(:is_a?) do |klass|
-              super(klass) || klass == resource_class
+          resource_class = Class.new(self)
+          resource_class.resource_name resource_name.to_sym
+          resource_class.run_context = run_context
+          resource_class.class_from_file(filename)
+
+          # Make a useful string for the class (rather than <Class:312894723894>)
+          resource_class.instance_eval do
+            define_singleton_method(:to_s) do
+              "LWRP resource #{resource_name} from cookbook #{cookbook_name}"
             end
+            define_singleton_method(:inspect) { to_s }
           end
 
           Chef::Log.debug("Loaded contents of #{filename} into resource #{resource_name} (#{resource_class})")
@@ -105,7 +70,7 @@ class Chef
           LWRPBase.loaded_lwrps[filename] = true
 
           # Create the deprecated Chef::Resource::LwrpFoo class
-          Chef::Resource.register_deprecated_lwrp_class(deprecated_resource_class, convert_to_class_name(resource_name))
+          Chef::Resource.register_deprecated_lwrp_class(resource_class, convert_to_class_name(resource_name))
           resource_class
         end
 
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index 7e2ed998b8..545524eb10 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -177,20 +177,6 @@ describe "LWRP" do
       end
     end
 
-    it "allows monkey patching of the lwrp through Chef::Resource" do
-      monkey = Module.new do
-        def issue_3607
-        end
-      end
-      allow(Chef::Config).to receive(:[]).with(:treat_deprecation_warnings_as_errors).and_return(false)
-      Chef::Resource::LwrpFoo.send(:include, monkey)
-      expect { get_lwrp(:lwrp_foo).new("blah").issue_3607 }.not_to raise_error
-    end
-
-    it "should load the resource into a properly-named class and emit a warning when it is initialized" do
-      expect { Chef::Resource::LwrpFoo.new('hi') }.to raise_error(Chef::Exceptions::DeprecatedFeatureError)
-    end
-
     it "should be resolvable with Chef::ResourceResolver.resolve(:lwrp_foo)" do
       expect(Chef::ResourceResolver.resolve(:lwrp_foo, node: Chef::Node.new)).to eq(get_lwrp(:lwrp_foo))
     end
@@ -238,127 +224,6 @@ describe "LWRP" do
       expect(cls.node[:penguin_name]).to eql("jackass")
     end
 
-    context "resource class created" do
-      before do
-        @old_treat_deprecation_warnings_as_errors = Chef::Config[:treat_deprecation_warnings_as_errors]
-        Chef::Config[:treat_deprecation_warnings_as_errors] = false
-      end
-      after do
-        Chef::Config[:treat_deprecation_warnings_as_errors] = @old_treat_deprecation_warnings_as_errors
-      end
-
-      it "should load the resource into a properly-named class" do
-        expect(Chef::Resource::LwrpFoo).to be_kind_of(Class)
-        expect(Chef::Resource::LwrpFoo <= Chef::Resource::LWRPBase).to be_truthy
-      end
-
-      it "get_lwrp(:lwrp_foo).new is a Chef::Resource::LwrpFoo" do
-        lwrp = get_lwrp(:lwrp_foo).new('hi')
-        expect(lwrp.kind_of?(Chef::Resource::LwrpFoo)).to be_truthy
-        expect(lwrp.is_a?(Chef::Resource::LwrpFoo)).to be_truthy
-        expect(get_lwrp(:lwrp_foo) === lwrp).to be_truthy
-        expect(Chef::Resource::LwrpFoo === lwrp).to be_truthy
-      end
-
-      it "Chef::Resource::LwrpFoo.new is a get_lwrp(:lwrp_foo)" do
-        lwrp = Chef::Resource::LwrpFoo.new('hi')
-        expect(lwrp.kind_of?(get_lwrp(:lwrp_foo))).to be_truthy
-        expect(lwrp.is_a?(get_lwrp(:lwrp_foo))).to be_truthy
-        expect(get_lwrp(:lwrp_foo) === lwrp).to be_truthy
-        expect(Chef::Resource::LwrpFoo === lwrp).to be_truthy
-      end
-
-      it "works even if LwrpFoo exists in the top level" do
-        module ::LwrpFoo
-        end
-        expect(Chef::Resource::LwrpFoo).not_to eq(::LwrpFoo)
-      end
-
-      context "with a subclass of get_lwrp(:lwrp_foo)" do
-        let(:subclass) do
-          Class.new(get_lwrp(:lwrp_foo))
-        end
-
-        it "subclass.new is a subclass" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_truthy
-          expect(lwrp.is_a?(subclass)).to be_truthy
-          expect(subclass === lwrp).to be_truthy
-          expect(lwrp.class === subclass)
-        end
-        it "subclass.new is a Chef::Resource::LwrpFoo" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(Chef::Resource::LwrpFoo)).to be_truthy
-          expect(lwrp.is_a?(Chef::Resource::LwrpFoo)).to be_truthy
-          expect(Chef::Resource::LwrpFoo === lwrp).to be_truthy
-          expect(lwrp.class === Chef::Resource::LwrpFoo)
-        end
-        it "subclass.new is a get_lwrp(:lwrp_foo)" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(get_lwrp(:lwrp_foo))).to be_truthy
-          expect(lwrp.is_a?(get_lwrp(:lwrp_foo))).to be_truthy
-          expect(get_lwrp(:lwrp_foo) === lwrp).to be_truthy
-          expect(lwrp.class === get_lwrp(:lwrp_foo))
-        end
-        it "Chef::Resource::LwrpFoo.new is *not* a subclass" do
-          lwrp = Chef::Resource::LwrpFoo.new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_falsey
-          expect(lwrp.is_a?(subclass)).to be_falsey
-          expect(subclass === lwrp.class).to be_falsey
-          expect(subclass === Chef::Resource::LwrpFoo).to be_falsey
-        end
-        it "get_lwrp(:lwrp_foo).new is *not* a subclass" do
-          lwrp = get_lwrp(:lwrp_foo).new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_falsey
-          expect(lwrp.is_a?(subclass)).to be_falsey
-          expect(subclass === lwrp.class).to be_falsey
-          expect(subclass === get_lwrp(:lwrp_foo)).to be_falsey
-        end
-      end
-
-      context "with a subclass of Chef::Resource::LwrpFoo" do
-        let(:subclass) do
-          Class.new(Chef::Resource::LwrpFoo)
-        end
-
-        it "subclass.new is a subclass" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_truthy
-          expect(lwrp.is_a?(subclass)).to be_truthy
-          expect(subclass === lwrp).to be_truthy
-          expect(lwrp.class === subclass)
-        end
-        it "subclass.new is a Chef::Resource::LwrpFoo" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(Chef::Resource::LwrpFoo)).to be_truthy
-          expect(lwrp.is_a?(Chef::Resource::LwrpFoo)).to be_truthy
-          expect(Chef::Resource::LwrpFoo === lwrp).to be_truthy
-          expect(lwrp.class === Chef::Resource::LwrpFoo)
-        end
-        it "subclass.new is a get_lwrp(:lwrp_foo)" do
-          lwrp = subclass.new('hi')
-          expect(lwrp.kind_of?(get_lwrp(:lwrp_foo))).to be_truthy
-          expect(lwrp.is_a?(get_lwrp(:lwrp_foo))).to be_truthy
-          expect(get_lwrp(:lwrp_foo) === lwrp).to be_truthy
-          expect(lwrp.class === get_lwrp(:lwrp_foo))
-        end
-        it "Chef::Resource::LwrpFoo.new is *not* a subclass" do
-          lwrp = Chef::Resource::LwrpFoo.new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_falsey
-          expect(lwrp.is_a?(subclass)).to be_falsey
-          expect(subclass === lwrp.class).to be_falsey
-          expect(subclass === Chef::Resource::LwrpFoo).to be_falsey
-        end
-        it "get_lwrp(:lwrp_foo).new is *not* a subclass" do
-          lwrp = get_lwrp(:lwrp_foo).new('hi')
-          expect(lwrp.kind_of?(subclass)).to be_falsey
-          expect(lwrp.is_a?(subclass)).to be_falsey
-          expect(subclass === lwrp.class).to be_falsey
-          expect(subclass === get_lwrp(:lwrp_foo)).to be_falsey
-        end
-      end
-    end
-
     context "resource_name" do
       let(:klass) { Class.new(Chef::Resource::LWRPBase) }
 
@@ -714,7 +579,144 @@ describe "LWRP" do
       end
 
     end
-
   end
 
+  context "resource class created" do
+    before(:context) do
+      @tmpdir = Dir.mktmpdir("lwrp_test")
+      resource_path = File.join(@tmpdir, "once.rb")
+      IO.write(resource_path, "default_action :create")
+
+      @old_treat_deprecation_warnings_as_errors = Chef::Config[:treat_deprecation_warnings_as_errors]
+      Chef::Config[:treat_deprecation_warnings_as_errors] = false
+      Chef::Resource::LWRPBase.build_from_file("lwrp", resource_path, nil)
+    end
+
+    after(:context) do
+      FileUtils.remove_entry @tmpdir
+      Chef::Config[:treat_deprecation_warnings_as_errors] = @old_treat_deprecation_warnings_as_errors
+    end
+
+    it "should load the resource into a properly-named class" do
+      expect(Chef::Resource::LwrpOnce).to be_kind_of(Class)
+      expect(Chef::Resource::LwrpOnce <= Chef::Resource::LWRPBase).to be_truthy
+    end
+
+    it "get_lwrp(:lwrp_once).new is a Chef::Resource::LwrpOnce" do
+      lwrp = get_lwrp(:lwrp_once).new('hi')
+      expect(lwrp.kind_of?(Chef::Resource::LwrpOnce)).to be_truthy
+      expect(lwrp.is_a?(Chef::Resource::LwrpOnce)).to be_truthy
+      expect(get_lwrp(:lwrp_once) === lwrp).to be_truthy
+      expect(Chef::Resource::LwrpOnce === lwrp).to be_truthy
+    end
+
+    it "Chef::Resource::LwrpOnce.new is a get_lwrp(:lwrp_once)" do
+      lwrp = Chef::Resource::LwrpOnce.new('hi')
+      expect(lwrp.kind_of?(get_lwrp(:lwrp_once))).to be_truthy
+      expect(lwrp.is_a?(get_lwrp(:lwrp_once))).to be_truthy
+      expect(get_lwrp(:lwrp_once) === lwrp).to be_truthy
+      expect(Chef::Resource::LwrpOnce === lwrp).to be_truthy
+    end
+
+    it "works even if LwrpOnce exists in the top level" do
+      module ::LwrpOnce
+      end
+      expect(Chef::Resource::LwrpOnce).not_to eq(::LwrpOnce)
+    end
+
+    it "allows monkey patching of the lwrp through Chef::Resource" do
+      monkey = Module.new do
+        def issue_3607
+        end
+      end
+      Chef::Resource::LwrpOnce.send(:include, monkey)
+      expect { get_lwrp(:lwrp_once).new("blah").issue_3607 }.not_to raise_error
+    end
+
+    context "with a subclass of get_lwrp(:lwrp_once)" do
+      let(:subclass) do
+        Class.new(get_lwrp(:lwrp_once))
+      end
+
+      it "subclass.new is a subclass" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_truthy
+        expect(lwrp.is_a?(subclass)).to be_truthy
+        expect(subclass === lwrp).to be_truthy
+        expect(lwrp.class === subclass)
+      end
+      it "subclass.new is a Chef::Resource::LwrpOnce" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(Chef::Resource::LwrpOnce)).to be_truthy
+        expect(lwrp.is_a?(Chef::Resource::LwrpOnce)).to be_truthy
+        expect(Chef::Resource::LwrpOnce === lwrp).to be_truthy
+        expect(lwrp.class === Chef::Resource::LwrpOnce)
+      end
+      it "subclass.new is a get_lwrp(:lwrp_once)" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(get_lwrp(:lwrp_once))).to be_truthy
+        expect(lwrp.is_a?(get_lwrp(:lwrp_once))).to be_truthy
+        expect(get_lwrp(:lwrp_once) === lwrp).to be_truthy
+        expect(lwrp.class === get_lwrp(:lwrp_once))
+      end
+      it "Chef::Resource::LwrpOnce.new is *not* a subclass" do
+        lwrp = Chef::Resource::LwrpOnce.new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_falsey
+        expect(lwrp.is_a?(subclass)).to be_falsey
+        expect(subclass === lwrp.class).to be_falsey
+        expect(subclass === Chef::Resource::LwrpOnce).to be_falsey
+      end
+      it "get_lwrp(:lwrp_once).new is *not* a subclass" do
+        lwrp = get_lwrp(:lwrp_once).new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_falsey
+        expect(lwrp.is_a?(subclass)).to be_falsey
+        expect(subclass === lwrp.class).to be_falsey
+        expect(subclass === get_lwrp(:lwrp_once)).to be_falsey
+      end
+    end
+
+    context "with a subclass of Chef::Resource::LwrpOnce" do
+      let(:subclass) do
+        Class.new(Chef::Resource::LwrpOnce)
+      end
+
+      it "subclass.new is a subclass" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_truthy
+        expect(lwrp.is_a?(subclass)).to be_truthy
+        expect(subclass === lwrp).to be_truthy
+        expect(lwrp.class === subclass)
+      end
+      it "subclass.new is a Chef::Resource::LwrpOnce" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(Chef::Resource::LwrpOnce)).to be_truthy
+        expect(lwrp.is_a?(Chef::Resource::LwrpOnce)).to be_truthy
+        expect(Chef::Resource::LwrpOnce === lwrp).to be_truthy
+        expect(lwrp.class === Chef::Resource::LwrpOnce)
+      end
+      it "subclass.new is a get_lwrp(:lwrp_once)" do
+        lwrp = subclass.new('hi')
+        expect(lwrp.kind_of?(get_lwrp(:lwrp_once))).to be_truthy
+        expect(lwrp.is_a?(get_lwrp(:lwrp_once))).to be_truthy
+        expect(get_lwrp(:lwrp_once) === lwrp).to be_truthy
+        expect(lwrp.class === get_lwrp(:lwrp_once))
+      end
+      it "Chef::Resource::LwrpOnce.new is *not* a subclass" do
+        lwrp = Chef::Resource::LwrpOnce.new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_falsey
+        expect(lwrp.is_a?(subclass)).to be_falsey
+        expect(subclass === lwrp.class).to be_falsey
+        expect(subclass === Chef::Resource::LwrpOnce).to be_falsey
+      end
+      it "get_lwrp(:lwrp_once).new is *not* a subclass" do
+        lwrp = get_lwrp(:lwrp_once).new('hi')
+        expect(lwrp.kind_of?(subclass)).to be_falsey
+        expect(lwrp.is_a?(subclass)).to be_falsey
+        expect(subclass === lwrp.class).to be_falsey
+        expect(subclass === get_lwrp(:lwrp_once)).to be_falsey
+      end
+    end
+  end
 end
+
+