Chef-13: Chef::Resource cleanup

most of this deletes useless old code.

the change to lookup_provider_constant changes to more strictly
stop using class name based lookups and to go through the resource
resolver and provider resolver.

as a result in order to find the provider class for a given dsl name
you have to go through the resource resolver to find the resource in
order to be able to pass a resource instance through the provider
resolver.  since the provider resolver api passes resources into blocks
passed into provides api you must construct a resource instance.

that means that providers need to be associated with resources in order
to be looked up (which makes sense in Real Life(tm) use of Chef, but
breaks quite a few lazy tests we had where we constructed providers
without doing the work of wrapping them in a resource.

note that as the deploy resource shows this filters into a changed
behavior of the `provider` syntax where before `provider :revision`
would look up Chef::Provider::Deploy::Revision via class-name based
magic.  this breaks that API so that `provider :deploy_revision` is
used instead -- the symbol (or string) there is turned into a resource
first via the Chef::ResourceResolver and then looked up via the
Chef::ProviderResolver into Chef::Provider::Deploy::Revision.  this
is a breaking change but is also a bug fix so that the symbol here
goes through the same lookup that you get when you type it in the DSL.

i had considered implementing a lookup from a resource_name symbol to
a provider, but in looking at how to implement that in the
ProviderResolver the issue is that we really need to have a resource
instance order to pass to the ProviderResolver.

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

11 files changed, 53 insertions, 144 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 234cd61dd2..cf8682396e 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -130,7 +130,6 @@ class Chef
     def initialize(name, run_context = nil)
       name(name) unless name.nil?
       @run_context = run_context
-      @noop = nil
       @before = nil
       @params = Hash.new
       @provider = nil
@@ -451,11 +450,6 @@ class Chef
     #
     property :sensitive, [ TrueClass, FalseClass ], default: false, desired_state: false
 
-    # ??? TODO unreferenced.  Delete?
-    attr_reader :not_if_args
-    # ??? TODO unreferenced.  Delete?
-    attr_reader :only_if_args
-
     #
     # The time it took (in seconds) to run the most recently-run action.  Not
     # cumulative across actions.  This is set to 0 as soon as a new action starts
@@ -946,21 +940,6 @@ class Chef
     end
 
     #
-    # The DSL name of this resource (e.g. `package` or `yum_package`)
-    #
-    # @return [String] The DSL name of this resource.
-    #
-    # @deprecated Use resource_name instead.
-    #
-    def self.dsl_name
-      Chef.deprecated(:custom_resource, "Resource.dsl_name is deprecated and will be removed in Chef 13.  Use resource_name instead.")
-      if name
-        name = self.name.split("::")[-1]
-        convert_to_snake_case(name)
-      end
-    end
-
-    #
     # The display name of this resource type, for printing purposes.
     #
     # This also automatically calls "provides" to provide DSL with the given
@@ -1014,29 +993,6 @@ class Chef
     end
 
     #
-    # The module where Chef should look for providers for this resource.
-    # The provider for `MyResource` will be looked up using
-    # `provider_base::MyResource`.  Defaults to `Chef::Provider`.
-    #
-    # @param arg [Module] The module containing providers for this resource
-    # @return [Module] The module containing providers for this resource
-    #
-    # @example
-    #   class MyResource < Chef::Resource
-    #     provider_base Chef::Provider::Deploy
-    #     # ...other stuff
-    #   end
-    #
-    # @deprecated Use `provides` on the provider, or `provider` on the resource, instead.
-    #
-    def self.provider_base(arg = nil)
-      if arg
-        Chef.deprecated(:custom_resource, "Resource.provider_base is deprecated and will be removed in Chef 13. Use provides on the provider, or provider on the resource, instead.")
-      end
-      @provider_base ||= arg || Chef::Provider
-    end
-
-    #
     # The list of allowed actions for the resource.
     #
     # @param actions [Array<Symbol>] The list of actions to add to allowed_actions.
@@ -1445,24 +1401,6 @@ class Chef
       provider
     end
 
-    # ??? TODO Seems unused.  Delete?
-    def noop(tf = nil)
-      if !tf.nil?
-        raise ArgumentError, "noop must be true or false!" unless tf == true || tf == false
-        @noop = tf
-      end
-      @noop
-    end
-
-    # TODO Seems unused.  Delete?
-    def is(*args)
-      if args.size == 1
-        args.first
-      else
-        args
-      end
-    end
-
     #
     # Preface an exception message with generic Resource information.
     #
@@ -1540,13 +1478,7 @@ class Chef
 
     # @api private
     def lookup_provider_constant(name, action = :nothing)
-      self.class.provider_base.const_get(convert_to_class_name(name.to_s))
-    rescue NameError => e
-      if e.to_s =~ /#{Regexp.escape(self.class.provider_base.to_s)}/
-        raise ArgumentError, "No provider found to match '#{name}'"
-      else
-        raise e
-      end
+      self.class.resource_for_node(name, node).new("name", run_context).provider_for_action(action).class
     end
 
     module DeprecatedLWRPClass
diff --git a/spec/data/lwrp/resources/buck_passer.rb b/spec/data/lwrp/resources/buck_passer.rb
new file mode 100644
index 0000000000..7335c0aae2
--- /dev/null
+++ b/spec/data/lwrp/resources/buck_passer.rb
@@ -0,0 +1,5 @@
+
+provides :buck_passer
+
+default_action :pass_buck
+actions :pass_buck
diff --git a/spec/data/lwrp/resources/buck_passer_2.rb b/spec/data/lwrp/resources/buck_passer_2.rb
new file mode 100644
index 0000000000..c7a1a279f3
--- /dev/null
+++ b/spec/data/lwrp/resources/buck_passer_2.rb
@@ -0,0 +1,3 @@
+
+default_action :pass_buck
+actions :pass_buck
diff --git a/spec/data/lwrp/resources/embedded_resource_accesses_providers_scope.rb b/spec/data/lwrp/resources/embedded_resource_accesses_providers_scope.rb
new file mode 100644
index 0000000000..3a8ae2c19f
--- /dev/null
+++ b/spec/data/lwrp/resources/embedded_resource_accesses_providers_scope.rb
@@ -0,0 +1,3 @@
+
+default_action :twiddle_thumbs
+actions :twiddle_thumbs
diff --git a/spec/data/lwrp/resources/inline_compiler.rb b/spec/data/lwrp/resources/inline_compiler.rb
new file mode 100644
index 0000000000..fe446ddf84
--- /dev/null
+++ b/spec/data/lwrp/resources/inline_compiler.rb
@@ -0,0 +1,3 @@
+
+default_action :test
+actions :test, :no_updates
diff --git a/spec/data/lwrp/resources/monkey_name_printer.rb b/spec/data/lwrp/resources/monkey_name_printer.rb
new file mode 100644
index 0000000000..d70e2f34e3
--- /dev/null
+++ b/spec/data/lwrp/resources/monkey_name_printer.rb
@@ -0,0 +1,5 @@
+
+property :monkey
+
+default_action :twiddle_thumbs
+actions :twiddle_thumbs
diff --git a/spec/data/lwrp/resources/paint_drying_watcher.rb b/spec/data/lwrp/resources/paint_drying_watcher.rb
new file mode 100644
index 0000000000..519b7f83fd
--- /dev/null
+++ b/spec/data/lwrp/resources/paint_drying_watcher.rb
@@ -0,0 +1,3 @@
+
+default_action :prepare_eyes
+actions :prepare_eyes, :watch_paint_dry
diff --git a/spec/data/lwrp/resources/thumb_twiddler.rb b/spec/data/lwrp/resources/thumb_twiddler.rb
new file mode 100644
index 0000000000..2b5d2d803e
--- /dev/null
+++ b/spec/data/lwrp/resources/thumb_twiddler.rb
@@ -0,0 +1,3 @@
+
+default_action :prepare_thumbs
+actions :prepare_thumbs, :twiddle_thumbs
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index e167adb04c..28773a3c30 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -41,14 +41,11 @@ describe "LWRP" do
     Chef::ResourceResolver.resolve(name)
   end
 
-  def get_lwrp_provider(name)
-    old_treat_deprecation_warnings_as_errors = Chef::Config[:treat_deprecation_warnings_as_errors]
-    Chef::Config[:treat_deprecation_warnings_as_errors] = false
-    begin
-      Chef::Provider.const_get(convert_to_class_name(name.to_s))
-    ensure
-      Chef::Config[:treat_deprecation_warnings_as_errors] = old_treat_deprecation_warnings_as_errors
-    end
+  def get_dynamic_lwrp_provider(name)
+    # need a node to do dynamic lookup, so also need a run_context and a resource instance
+    node = Chef::Node.new
+    run_context = Chef::RunContext.new(node, {}, nil)
+    Chef::Resource.new("name", run_context).lookup_provider_constant(name)
   end
 
   describe "when overriding an existing class" do
@@ -386,7 +383,7 @@ describe "LWRP" do
 
     let(:runner) { Chef::Runner.new(run_context) }
 
-    let(:lwrp_cookbok_name) { "lwrp" }
+    let(:lwrp_cookbook_name) { "lwrp" }
 
     before do
       Chef::Provider::LWRPBase.class_eval { @loaded_lwrps = {} }
@@ -394,18 +391,18 @@ describe "LWRP" do
 
     before(:each) do
       Dir[File.expand_path(File.expand_path("../../data/lwrp/resources/*", __FILE__))].each do |file|
-        Chef::Resource::LWRPBase.build_from_file(lwrp_cookbok_name, file, run_context)
+        Chef::Resource::LWRPBase.build_from_file(lwrp_cookbook_name, file, run_context)
       end
 
       Dir[File.expand_path(File.expand_path("../../data/lwrp/providers/*", __FILE__))].each do |file|
-        Chef::Provider::LWRPBase.build_from_file(lwrp_cookbok_name, file, run_context)
+        Chef::Provider::LWRPBase.build_from_file(lwrp_cookbook_name, file, run_context)
       end
     end
 
     it "should properly handle a new_resource reference" do
       resource = get_lwrp(:lwrp_foo).new("morpheus", run_context)
       resource.monkey("bob")
-      resource.provider(get_lwrp_provider(:lwrp_monkey_name_printer))
+      resource.provider(get_dynamic_lwrp_provider(:lwrp_monkey_name_printer))
       provider = resource.provider_for_action(:twiddle_thumbs)
       provider.action_twiddle_thumbs
     end
@@ -426,8 +423,8 @@ describe "LWRP" do
       end
 
       it "should create a method for each action" do
-        expect(get_lwrp_provider(:lwrp_buck_passer).instance_methods).to include(:action_pass_buck)
-        expect(get_lwrp_provider(:lwrp_thumb_twiddler).instance_methods).to include(:action_twiddle_thumbs)
+        expect(get_dynamic_lwrp_provider(:lwrp_buck_passer).instance_methods).to include(:action_pass_buck)
+        expect(get_dynamic_lwrp_provider(:lwrp_thumb_twiddler).instance_methods).to include(:action_twiddle_thumbs)
       end
 
       it "sets itself as a provider for a resource of the same name" do
@@ -435,30 +432,30 @@ describe "LWRP" do
         # we bypass the per-file loading to get the file to load each time,
         # which creates the LWRP class repeatedly. New things get prepended to
         # the list of providers.
-        expect(found_providers.first).to eq(get_lwrp_provider(:lwrp_buck_passer))
+        expect(found_providers.first).to eq(get_dynamic_lwrp_provider(:lwrp_buck_passer))
       end
 
       context "with a cookbook with an underscore in the name" do
 
-        let(:lwrp_cookbok_name) { "l_w_r_p" }
+        let(:lwrp_cookbook_name) { "l_w_r_p" }
 
         it "sets itself as a provider for a resource of the same name" do
           found_providers = Chef::Platform::ProviderHandlerMap.instance.list(node, :l_w_r_p_buck_passer)
           expect(found_providers.size).to eq(1)
-          expect(found_providers.last).to eq(get_lwrp_provider(:l_w_r_p_buck_passer))
+          expect(found_providers.last).to eq(get_dynamic_lwrp_provider(:l_w_r_p_buck_passer))
         end
       end
 
       context "with a cookbook with a hypen in the name" do
 
-        let(:lwrp_cookbok_name) { "l-w-r-p" }
+        let(:lwrp_cookbook_name) { "l-w-r-p" }
 
         it "sets itself as a provider for a resource of the same name" do
           incorrect_providers = Chef::Platform::ProviderHandlerMap.instance.list(node, :'l-w-r-p_buck_passer')
           expect(incorrect_providers).to eq([])
 
           found_providers = Chef::Platform::ProviderHandlerMap.instance.list(node, :l_w_r_p_buck_passer)
-          expect(found_providers.first).to eq(get_lwrp_provider(:l_w_r_p_buck_passer))
+          expect(found_providers.first).to eq(get_dynamic_lwrp_provider(:l_w_r_p_buck_passer))
         end
       end
     end
@@ -466,7 +463,7 @@ describe "LWRP" do
     it "should insert resources embedded in the provider into the middle of the resource collection" do
       injector = get_lwrp(:lwrp_foo).new("morpheus", run_context)
       injector.action(:pass_buck)
-      injector.provider(get_lwrp_provider(:lwrp_buck_passer))
+      injector.provider(get_dynamic_lwrp_provider(:lwrp_buck_passer))
       dummy = Chef::Resource::ZenMaster.new("keanu reeves", run_context)
       dummy.provider(Chef::Provider::Easy)
       run_context.resource_collection.insert(injector)
@@ -483,11 +480,11 @@ describe "LWRP" do
     it "should insert embedded resources from multiple providers, including from the last position, properly into the resource collection" do
       injector = get_lwrp(:lwrp_foo).new("morpheus", run_context)
       injector.action(:pass_buck)
-      injector.provider(get_lwrp_provider(:lwrp_buck_passer))
+      injector.provider(get_dynamic_lwrp_provider(:lwrp_buck_passer))
 
       injector2 = get_lwrp(:lwrp_bar).new("tank", run_context)
       injector2.action(:pass_buck)
-      injector2.provider(get_lwrp_provider(:lwrp_buck_passer_2))
+      injector2.provider(get_dynamic_lwrp_provider(:lwrp_buck_passer_2))
 
       dummy = Chef::Resource::ZenMaster.new("keanu reeves", run_context)
       dummy.provider(Chef::Provider::Easy)
@@ -510,7 +507,7 @@ describe "LWRP" do
     it "should properly handle a new_resource reference" do
       resource = get_lwrp(:lwrp_foo).new("morpheus", run_context)
       resource.monkey("bob")
-      resource.provider(get_lwrp_provider(:lwrp_monkey_name_printer))
+      resource.provider(get_dynamic_lwrp_provider(:lwrp_monkey_name_printer))
 
       provider = resource.provider_for_action(:twiddle_thumbs)
       provider.action_twiddle_thumbs
@@ -521,7 +518,7 @@ describe "LWRP" do
     it "should properly handle an embedded Resource accessing the enclosing Provider's scope" do
       resource = get_lwrp(:lwrp_foo).new("morpheus", run_context)
       resource.monkey("bob")
-      resource.provider(get_lwrp_provider(:lwrp_embedded_resource_accesses_providers_scope))
+      resource.provider(get_dynamic_lwrp_provider(:lwrp_embedded_resource_accesses_providers_scope))
 
       provider = resource.provider_for_action(:twiddle_thumbs)
       #provider = @runner.build_provider(resource)
@@ -541,7 +538,7 @@ describe "LWRP" do
         @resource = get_lwrp(:lwrp_foo).new("morpheus", run_context)
         @resource.allowed_actions << :test
         @resource.action(:test)
-        @resource.provider(get_lwrp_provider(:lwrp_inline_compiler))
+        @resource.provider(get_dynamic_lwrp_provider(:lwrp_inline_compiler))
       end
 
       it "does not add interior resources to the exterior resource collection" do
diff --git a/spec/unit/resource/deploy_spec.rb b/spec/unit/resource/deploy_spec.rb
index 33f16b4a89..a758638244 100644
--- a/spec/unit/resource/deploy_spec.rb
+++ b/spec/unit/resource/deploy_spec.rb
@@ -1,6 +1,6 @@
 #
 # Author:: Daniel DeLeo (<dan@kallistec.com>)
-# Copyright:: Copyright 2008-2016, Chef Software Inc.
+# Copyright:: Copyright 2008-2017, Chef Software Inc.
 # License:: Apache License, Version 2.0
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
@@ -216,12 +216,12 @@ describe Chef::Resource::Deploy do
   end
 
   it "allows deploy providers to be set via symbol" do
-    @resource.provider :revision
+    @resource.provider :deploy_revision
     expect(@resource.provider).to eq(Chef::Provider::Deploy::Revision)
   end
 
   it "allows deploy providers to be set via string" do
-    @resource.provider "revision"
+    @resource.provider "deploy_revision"
     expect(@resource.provider).to eq(Chef::Provider::Deploy::Revision)
   end
 
diff --git a/spec/unit/resource_spec.rb b/spec/unit/resource_spec.rb
index 481a379743..df8ae3100a 100644
--- a/spec/unit/resource_spec.rb
+++ b/spec/unit/resource_spec.rb
@@ -231,14 +231,6 @@ describe Chef::Resource do
     end
   end
 
-  describe "noop" do
-    it "should accept true or false for noop" do
-      expect { resource.noop true }.not_to raise_error
-      expect { resource.noop false }.not_to raise_error
-      expect { resource.noop "eat it" }.to raise_error(ArgumentError)
-    end
-  end
-
   describe "notifies" do
     it "should make notified resources appear in the actions hash" do
       run_context.resource_collection << Chef::Resource::ZenMaster.new("coffee")
@@ -450,18 +442,6 @@ describe Chef::Resource do
     end
   end
 
-  describe "is" do
-    it "should return the arguments passed with 'is'" do
-      zm = Chef::Resource::ZenMaster.new("coffee")
-      expect(zm.is("one", "two", "three")).to eq(%w{one two three})
-    end
-
-    it "should allow arguments preceded by is to methods" do
-      resource.noop(resource.is(true))
-      expect(resource.noop).to eql(true)
-    end
-  end
-
   describe "to_json" do
     it "should serialize to json" do
       json = resource.to_json
@@ -480,7 +460,7 @@ describe Chef::Resource do
       it "should include the default in the hash" do
         expect(resource.to_hash.keys.sort).to eq([:a, :allowed_actions, :params, :provider, :updated,
           :updated_by_last_action, :before,
-          :noop, :name, :source_line,
+          :name, :source_line,
           :action, :elapsed_time,
           :default_guard_interpreter, :guard_interpreter].sort)
         expect(resource.to_hash[:name]).to eq "funk"
@@ -492,7 +472,7 @@ describe Chef::Resource do
       hash = resource.to_hash
       expected_keys = [ :allowed_actions, :params, :provider, :updated,
         :updated_by_last_action, :before,
-        :noop, :name, :source_line,
+        :name, :source_line,
         :action, :elapsed_time,
         :default_guard_interpreter, :guard_interpreter ]
       expect(hash.keys - expected_keys).to eq([])
@@ -571,31 +551,6 @@ describe Chef::Resource do
     end
   end
 
-  describe "setting the base provider class for the resource" do
-
-    it "defaults to Chef::Provider for the base class" do
-      expect(Chef::Resource.provider_base).to eq(Chef::Provider)
-    end
-
-    it "allows the base provider to be overridden" do
-      Chef::Config.treat_deprecation_warnings_as_errors(false)
-      class OverrideProviderBaseTest < Chef::Resource
-        provider_base Chef::Provider::Package
-      end
-
-      expect(OverrideProviderBaseTest.provider_base).to eq(Chef::Provider::Package)
-    end
-
-    it "warns when setting provider_base" do
-      expect do
-        class OverrideProviderBaseTest2 < Chef::Resource
-          provider_base Chef::Provider::Package
-        end
-      end.to raise_error(Chef::Exceptions::DeprecatedFeatureError)
-    end
-
-  end
-
   it "runs an action by finding its provider, loading the current resource and then running the action" do
     skip
   end