Make resource_for_short_name look up the canonical resourcejk/automatic-automatic-name

5 files changed, 204 insertions, 70 deletions

diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index ff398bc9e6..d69f0a8f11 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -162,7 +162,7 @@ class Chef
         # Chef::Resource::Blah = <resource>, a deprecation warning will be
         # emitted and the DSL method 'blah' will be added to the DSL.
         #
-        resource_class = Chef::ResourceResolver.new(run_context ? run_context.node : nil, method_symbol).resolve
+        resource_class = Chef::ResourceResolver.resolve(method_symbol, node: run_context ? run_context.node : nil)
         if resource_class
           Chef::DSL::Resources.add_resource_dsl(method_symbol)
           return send(method_symbol, *args, &block)
diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb
index f07c44192f..f547018a38 100644
--- a/lib/chef/node_map.rb
+++ b/lib/chef/node_map.rb
@@ -71,12 +71,14 @@ class Chef
     # @param node [Chef::Node] The Chef::Node object for the run, or `nil` to
     #   ignore all filters.
     # @param key [Object] Key to look up
+    # @param canonical [Boolean] `true` or `false` to match canonical or
+    #   non-canonical values only. `nil` to ignore canonicality.  Default: `nil`
     #
     # @return [Object] Value
     #
-    def get(node, key)
+    def get(node, key, canonical: nil)
       raise ArgumentError, "first argument must be a Chef::Node" unless node.is_a?(Chef::Node) || node.nil?
-      list(node, key).first
+      list(node, key, canonical: canonical).first
     end
 
     #
@@ -86,14 +88,16 @@ class Chef
     # @param node [Chef::Node] The Chef::Node object for the run, or `nil` to
     #   ignore all filters.
     # @param key [Object] Key to look up
+    # @param canonical [Boolean] `true` or `false` to match canonical or
+    #   non-canonical values only. `nil` to ignore canonicality.  Default: `nil`
     #
     # @return [Object] Value
     #
-    def list(node, key)
+    def list(node, key, canonical: nil)
       raise ArgumentError, "first argument must be a Chef::Node" unless node.is_a?(Chef::Node) || node.nil?
       return [] unless @map.has_key?(key)
       @map[key].select do |matcher|
-        !node || (filters_match?(node, matcher[:filters]) && block_matches?(node, matcher[:block]))
+        node_matches?(node, matcher) && canonical_matches?(canonical, matcher)
       end.map { |matcher| matcher[:value] }
     end
 
@@ -183,5 +187,15 @@ class Chef
       return true if block.nil?
       block.call node
     end
+
+    def node_matches?(node, matcher)
+      return true if !node
+      filters_match?(node, matcher[:filters]) && block_matches?(node, matcher[:block])
+    end
+
+    def canonical_matches?(canonical, matcher)
+      return true if canonical.nil?
+      !!canonical == !!matcher[:canonical]
+    end
   end
 end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index ed66bab916..7fe8a52d95 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -937,11 +937,6 @@ class Chef
       resource_name(name)
     end
 
-    def self.inherited(child)
-      super
-      child.resource_name
-    end
-
     #
     # The module where Chef should look for providers for this resource.
     # The provider for `MyResource` will be looked up using
@@ -1018,7 +1013,7 @@ class Chef
       end
     end
     def self.default_action=(action_name)
-      default_action action_name
+      default_action(action_name)
     end
 
     #
@@ -1113,9 +1108,10 @@ class Chef
     def self.sorted_descendants
       @@sorted_descendants ||= descendants.sort_by { |x| x.to_s }
     end
-    def self.inherited(other)
+    def self.inherited(child)
       super
-      @@sorted_descendants = nil
+      @sorted_descendants = nil
+      child.resource_name
     end
 
 
@@ -1153,7 +1149,7 @@ class Chef
     end
 
     def self.provides?(node, resource)
-      Chef::ResourceResolver.new(node, resource).provided_by?(self)
+      Chef::ResourceResolver.resolve(resource, node: node).provided_by?(self)
     end
 
     # Helper for #notifies
@@ -1285,15 +1281,13 @@ class Chef
     # === Returns
     # <Chef::Resource>:: returns the proper Chef::Resource class
     def self.resource_for_node(short_name, node)
-      klass = Chef::ResourceResolver.new(node, short_name).resolve
+      klass = Chef::ResourceResolver.resolve(short_name, node: node)
       raise Chef::Exceptions::NoSuchResourceType.new(short_name, node) if klass.nil?
       klass
     end
 
     #
-    # Returns the class of a Chef::Resource based on the short name
-    # Only returns the *canonical* class with the given name, not the one that
-    # would be picked by the ResourceResolver.
+    # Returns the class with the given resource_name.
     #
     # ==== Parameters
     # short_name<Symbol>:: short_name of the resource (ie :directory)
@@ -1301,10 +1295,8 @@ class Chef
     # === Returns
     # <Chef::Resource>:: returns the proper Chef::Resource class
     #
-    # @deprecated Chef::Resource::FooBar will no longer mean anything special in
-    #   Chef 13.  Use `resource_for_node` instead.
     def self.resource_matching_short_name(short_name)
-      Chef::ResourceResolver.get(short_name, canonical: true)
+      Chef::ResourceResolver.resolve(short_name, canonical: true)
     end
 
     # @api private
diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb
index 22fed7e587..31b39f7e24 100644
--- a/lib/chef/resource_resolver.rb
+++ b/lib/chef/resource_resolver.rb
@@ -21,64 +21,92 @@ require 'chef/platform/resource_priority_map'
 
 class Chef
   class ResourceResolver
+    #
+    # Resolve a resource by name.
+    #
+    # @param resource_name [Symbol] The resource DSL name (e.g. `:file`).
+    # @param node [Chef::Node] The node against which to resolve. `nil` causes
+    #   platform filters to be ignored.
+    #
+    def self.resolve(resource_name, node: nil, canonical: nil)
+      new(node, resource_name, canonical: canonical).resolve
+    end
+
+    #
+    # Resolve a list of all resources that implement the given DSL (in order of
+    # preference).
+    #
+    # @param resource_name [Symbol] The resource DSL name (e.g. `:file`).
+    # @param node [Chef::Node] The node against which to resolve. `nil` causes
+    #   platform filters to be ignored.
+    # @param canonical [Boolean] `true` or `false` to match canonical or
+    #   non-canonical values only. `nil` to ignore canonicality.
+    #
+    def self.list(resource_name, node: nil, canonical: nil)
+      new(node, resource_name, canonical: canonical).list
+    end
+
+
     include Chef::Mixin::ConvertToClassName
 
+    # @api private
     attr_reader :node
-    attr_reader :resource
+    # @api private
+    attr_reader :resource_name
+    # @api private
+    def resource
+      Chef::Log.deprecation("Chef::ResourceResolver.resource deprecated.  Use resource_name instead.")
+      resource_name
+    end
+    # @api private
     attr_reader :action
+    # @api private
     attr_reader :canonical
 
-    def initialize(node, resource)
+    #
+    # Create a resolver.
+    #
+    # @param node [Chef::Node] The node against which to resolve. `nil` causes
+    #   platform filters to be ignored.
+    # @param resource_name [Symbol] The resource DSL name (e.g. `:file`).
+    # @param canonical [Boolean] `true` or `false` to match canonical or
+    #   non-canonical values only. `nil` to ignore canonicality.  Default: `nil`
+    #
+    # @api private use Chef::ResourceResolver.resolve or .list instead.
+    def initialize(node, resource_name, canonical: nil)
       @node = node
-      @resource = resource.to_sym
+      @resource_name = resource_name.to_sym
+      @canonical = canonical
     end
 
+    # @api private use Chef::ResourceResolver.resolve instead.
     def resolve
       # log this so we know what resources will work for the generic resource on the node (early cut)
-      Chef::Log.debug "Resources for generic #{resource} resource enabled on node include: #{enabled_handlers}"
+      Chef::Log.debug "Resources for generic #{resource_name} resource enabled on node include: #{prioritized_handlers}"
 
       handler = prioritized_handlers.first
 
       if handler
-        Chef::Log.debug "Resource for #{resource} is #{handler}"
+        Chef::Log.debug "Resource for #{resource_name} is #{handler}"
       else
-        Chef::Log.debug "Dynamic resource resolver FAILED to resolve a resource for #{resource}"
+        Chef::Log.debug "Dynamic resource resolver FAILED to resolve a resource for #{resource_name}"
       end
 
       handler
     end
 
+    # @api private
     def list
-      Chef::Log.debug "Resources for generic #{resource} resource enabled on node include: #{enabled_handlers}"
-      Chef::Log.debug "Resources for #{resource}: #{prioritized_handlers}"
+      Chef::Log.debug "Resources for generic #{resource_name} resource enabled on node include: #{prioritized_handlers}"
       prioritized_handlers
     end
 
-    def provided_by?(resource_class)
-      !prioritized_handlers.include?(resource_class)
-    end
-
     #
-    # Resolve a resource by name.
+    # Whether this DSL is provided by the given resource_class.
     #
-    # @param resource_name [Symbol] The resource DSL name (e.g. `:file`).
-    # @param node [Chef::Node] The node on which the resource will run. If not
-    #   passed, will return the first match.
-    #
-    def self.resolve(resource_name, node: nil)
-      new(node, resource_name).resolve
-    end
-
-    #
-    # Resolve a list of all resources that implement the given DSL (in order of
-    # preference).
-    #
-    # @param resource_name [Symbol] The resource DSL name (e.g. `:file`).
-    # @param node [Chef::Node] The node on which the resource will run. If not
-    #   passed, will return all resources (ignoring filters).
-    #
-    def self.list(resource_name, node: nil)
-      new(node, resource_name).list
+    # @api private
+    def provided_by?(resource_class)
+      !prioritized_handlers.include?(resource_class)
     end
 
     protected
@@ -89,7 +117,7 @@ class Chef
 
     def prioritized_handlers
       @prioritized_handlers ||=
-        priority_map.list_handlers(node, resource)
+        priority_map.list_handlers(node, resource_name, canonical: canonical)
     end
 
     module Deprecated
@@ -102,19 +130,22 @@ class Chef
       # A list of all handlers
       # @deprecated Now prioritized_handlers does its own work
       def enabled_handlers
-        resources.select { |klass| klass.provides?(node, resource) }
+        Chef::Log.deprecation("enabled_handlers is deprecated.  If you are implementing a ResourceResolver, use provided_handlers.  If you are not, use Chef::ResourceResolver.list(#{resource_name.inspect}, node: <node>)")
+        resources.select { |klass| klass.provides?(node, resource_name) }
       end
 
       protected
 
-      # If there are no providers for a DSL, we search through the
+      # A list of all handlers for the given DSL.  If there are no handlers in
+      # the map, we still check all descendants of Chef::Resource for backwards
+      # compatibility purposes.
       def prioritized_handlers
         @prioritized_handlers ||= super ||
           resources.select do |klass|
-            # Don't bother calling provides? unless it's overriden. We already
+            # Don't bother calling provides? unless it's overridden. We already
             # know prioritized_handlers
-            if klass.method(:provides?).owner != Chef::Resource && klass.provides?(node, resource)
-              Chef::Log.deprecation("Resources #{provided.join(", ")} are marked as providing DSL #{resource}, but provides #{resource.inspect} was never called!")
+            if klass.method(:provides?).owner != Chef::Resource && klass.provides?(node, resource_name)
+              Chef::Log.deprecation("Resources #{provided.join(", ")} are marked as providing DSL #{resource_name}, but provides #{resource_name.inspect} was never called!")
               Chef::Log.deprecation("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.")
               true
             end
diff --git a/spec/integration/recipes/recipe_dsl_spec.rb b/spec/integration/recipes/recipe_dsl_spec.rb
index 94bee77314..3f4bf9fd5f 100644
--- a/spec/integration/recipes/recipe_dsl_spec.rb
+++ b/spec/integration/recipes/recipe_dsl_spec.rb
@@ -86,7 +86,7 @@ describe "Recipe DSL methods" do
           expect(BaseThingy.created_provider).to eq Chef::Provider::BackcompatThingy
         end
 
-        context "And another resource 'backcompat_thingy' in BackcompatThingy with 'provides'" do
+        context "and another resource 'backcompat_thingy' in BackcompatThingy with 'provides'" do
           before(:context) {
 
             class RecipeDSLSpecNamespace::BackcompatThingy < BaseThingy
@@ -106,7 +106,7 @@ describe "Recipe DSL methods" do
         end
       end
 
-      context "With a resource named RecipeDSLSpecNamespace::Bar::Thingy" do
+      context "With a resource named RecipeDSLSpecNamespace::Bar::BarThingy" do
         before(:context) {
 
           class RecipeDSLSpecNamespace::Bar::BarThingy < BaseThingy
@@ -413,7 +413,7 @@ describe "Recipe DSL methods" do
 
         context "With a resource TwoClassesOneDsl" do
           let(:class_name) { "TwoClassesOneDsl#{Namer.current_index}" }
-          let(:dsl_method) { "two_classes_one_dsl#{Namer.current_index}" }
+          let(:dsl_method) { :"two_classes_one_dsl#{Namer.current_index}" }
 
           before {
             eval <<-EOM, nil, __FILE__, __LINE__+1
@@ -421,7 +421,7 @@ describe "Recipe DSL methods" do
               end
             EOM
           }
-          context "And resource BlahModule::TwoClassesOneDsl" do
+          context "and resource BlahModule::TwoClassesOneDsl" do
             before {
               eval <<-EOM, nil, __FILE__, __LINE__+1
                 module BlahModule
@@ -438,8 +438,11 @@ describe "Recipe DSL methods" do
               expect(recipe.logged_warnings).to eq ''
               expect(BaseThingy.created_resource).to eq eval("BlahModule::#{class_name}")
             end
+            it "resource_matching_short_name returns BlahModule::TwoClassesOneDsl" do
+              expect(Chef::Resource.resource_matching_short_name(dsl_method)).to eq eval("BlahModule::#{class_name}")
+            end
           end
-          context "And resource BlahModule::TwoClassesOneDsl with resource_name nil" do
+          context "and resource BlahModule::TwoClassesOneDsl with resource_name nil" do
             before {
               eval <<-EOM, nil, __FILE__, __LINE__+1
                 module BlahModule
@@ -457,8 +460,11 @@ describe "Recipe DSL methods" do
               expect(recipe.logged_warnings).to eq ''
               expect(BaseThingy.created_resource).to eq eval("::#{class_name}")
             end
+            it "resource_matching_short_name returns ::TwoClassesOneDsl" do
+              expect(Chef::Resource.resource_matching_short_name(dsl_method)).to eq eval("::#{class_name}")
+            end
           end
-          context "And resource BlahModule::TwoClassesOneDsl with resource_name :argh" do
+          context "and resource BlahModule::TwoClassesOneDsl with resource_name :argh" do
             before {
               eval <<-EOM, nil, __FILE__, __LINE__+1
                 module BlahModule
@@ -476,8 +482,11 @@ describe "Recipe DSL methods" do
               expect(recipe.logged_warnings).to eq ''
               expect(BaseThingy.created_resource).to eq eval("::#{class_name}")
             end
+            it "resource_matching_short_name returns ::TwoClassesOneDsl" do
+              expect(Chef::Resource.resource_matching_short_name(dsl_method)).to eq eval("::#{class_name}")
+            end
           end
-          context "And resource BlahModule::TwoClassesOneDsl with provides :two_classes_one_dsl, os: 'blarghle'" do
+          context "and resource BlahModule::TwoClassesOneDsl with provides :two_classes_one_dsl, os: 'blarghle'" do
             before {
               eval <<-EOM, nil, __FILE__, __LINE__+1
                 module BlahModule
@@ -487,6 +496,7 @@ describe "Recipe DSL methods" do
                 end
               EOM
             }
+
             it "and os = blarghle, two_classes_one_dsl resolves to BlahModule::TwoClassesOneDsl" do
               dsl_method = self.dsl_method
               recipe = converge {
@@ -498,7 +508,7 @@ describe "Recipe DSL methods" do
               expect(BaseThingy.created_resource).to eq eval("BlahModule::#{class_name}")
             end
 
-            it "and os = linux, two_classes_one_dsl resolves to BlahModule::TwoClassesOneDsl" do
+            it "and os = linux, two_classes_one_dsl resolves to ::TwoClassesOneDsl" do
               dsl_method = self.dsl_method
               recipe = converge {
                 # this is an ugly way to test, make Cheffish expose node attrs
@@ -514,7 +524,7 @@ describe "Recipe DSL methods" do
     end
 
     context "provides" do
-      context "When MySupplier provides :hemlock" do
+      context "when MySupplier provides :hemlock" do
         before(:context) {
 
           class RecipeDSLSpecNamespace::MySupplier < BaseThingy
@@ -537,7 +547,7 @@ describe "Recipe DSL methods" do
         end
       end
 
-      context "When Thingy3 provides :thingy3" do
+      context "when Thingy3 has resource_name :thingy3" do
         before(:context) {
 
           class RecipeDSLSpecNamespace::Thingy3 < BaseThingy
@@ -553,7 +563,7 @@ describe "Recipe DSL methods" do
           expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy3
         end
 
-        context "And Thingy4 provides :thingy3" do
+        context "and Thingy4 has resource_name :thingy3" do
           before(:context) {
 
             class RecipeDSLSpecNamespace::Thingy4 < BaseThingy
@@ -574,10 +584,97 @@ describe "Recipe DSL methods" do
               thingy4 'blah' do; end
             }.to raise_error(NoMethodError)
           end
+
+          it "resource_matching_short_name returns Thingy4" do
+            expect(Chef::Resource.resource_matching_short_name(:thingy3)).to eq RecipeDSLSpecNamespace::Thingy4
+          end
+        end
+      end
+
+      context "when Thingy5 has resource_name :thingy5" do
+        before(:context) {
+
+          class RecipeDSLSpecNamespace::Thingy5 < BaseThingy
+            resource_name :thingy5
+          end
+
+        }
+
+        it "thingy5 works in a recipe" do
+          expect_recipe {
+            thingy5 'blah' do; end
+          }.to emit_no_warnings_or_errors
+          expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy5
+        end
+
+        context "and Thingy6 provides :thingy5" do
+          before(:context) {
+
+            class RecipeDSLSpecNamespace::Thingy6 < BaseThingy
+              provides :thingy5
+            end
+
+          }
+
+          it "thingy6 works in a recipe and yields Thingy6" do
+            recipe = converge {
+              thingy6 'blah' do; end
+            }
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy6
+          end
+
+          it "thingy5 works in a recipe and yields Foo::Thingy6 (the later one)" do
+            recipe = converge {
+              thingy5 'blah' do; end
+            }
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy6
+          end
+
+          it "resource_matching_short_name returns Thingy5" do
+            expect(Chef::Resource.resource_matching_short_name(:thingy5)).to eq RecipeDSLSpecNamespace::Thingy5
+          end
+        end
+      end
+
+      context "when Thingy7 provides :thingy8" do
+        before(:context) {
+
+          class RecipeDSLSpecNamespace::Thingy7 < BaseThingy
+            provides :thingy8
+          end
+
+        }
+
+        context "and Thingy8 has resource_name :thingy8" do
+          before(:context) {
+
+            class RecipeDSLSpecNamespace::Thingy8 < BaseThingy
+              resource_name :thingy8
+            end
+
+          }
+
+          it "thingy7 works in a recipe and yields Thingy7" do
+            recipe = converge {
+              thingy7 'blah' do; end
+            }
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy7
+          end
+
+          it "thingy8 works in a recipe and yields Thingy8 (the later one)" do
+            recipe = converge {
+              thingy8 'blah' do; end
+            }
+            expect(BaseThingy.created_resource).to eq RecipeDSLSpecNamespace::Thingy8
+          end
+
+          it "resource_matching_short_name returns Thingy8" do
+            expect(Chef::Resource.resource_matching_short_name(:thingy8)).to eq RecipeDSLSpecNamespace::Thingy8
+          end
         end
       end
 
-      context "When Thingy5 provides :thingy5, :twizzle and :twizzle2" do
+      context "when Thingy5 provides :thingy5, :twizzle and :twizzle2" do
         before(:context) {
 
           class RecipeDSLSpecNamespace::Thingy5 < BaseThingy