Narrow resolvers to only look at parts of the map we support

11 files changed, 155 insertions, 126 deletions

diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb
index 77646376ba..0c9d200180 100644
--- a/lib/chef/dsl/recipe.rb
+++ b/lib/chef/dsl/recipe.rb
@@ -65,17 +65,6 @@ class Chef
         #
         resource_class = Chef::ResourceResolver.new(run_context ? run_context.node : nil, method_symbol).resolve
         if resource_class
-          #
-          # If the DSL method was *not* added, this is the case where the
-          # matching class implements 'provides?' and matches resources that it
-          # never declared "provides" for (which means we would never have
-          # created DSL).  Anything where we don't create DSL is deprecated.
-          #
-          if !respond_to?(method_symbol)
-            Chef::Log.deprecation("#{resource_class} is marked as providing DSL #{method_symbol}, but provides #{method_symbol.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.")
-            Chef::DSL::Resources.add_resource_dsl(method_symbol)
-          end
           return send(method_symbol, *args, &block)
         end
 
diff --git a/lib/chef/mixin/provides.rb b/lib/chef/mixin/provides.rb
index c95cb753a8..095e273dab 100644
--- a/lib/chef/mixin/provides.rb
+++ b/lib/chef/mixin/provides.rb
@@ -8,17 +8,13 @@ class Chef
       include Chef::Mixin::DescendantsTracker
 
       def provides(short_name, opts={}, &block)
-        provides_priority_map.priority(short_name, self, opts, &block)
+        raise NotImplementedError, :provides
       end
 
       # Check whether this resource provides the resource_name DSL for the given
-      # node.
-      def provides?(node, short_name)
-        provides_priority_map.list(node, short_name).include?(self)
-      end
-
-      def provides_priority_map
-        raise NotImplementedError, :provides_priority_map
+      # node.  TODO remove this when we stop checking unregistered things.
+      def provides?(node, resource)
+        raise NotImplementedError, :provides?
       end
 
       # Get the list of recipe DSL this resource is responsible for on the given
diff --git a/lib/chef/node_map.rb b/lib/chef/node_map.rb
index 7c25cb841a..a7a0389a2a 100644
--- a/lib/chef/node_map.rb
+++ b/lib/chef/node_map.rb
@@ -71,6 +71,13 @@ class Chef
       list(node, key).first
     end
 
+    # List all matches for the given node and key from the NodeMap, from
+    # most-recently added to oldest.
+    #
+    # @param node [Chef::Node] The Chef::Node object for the run
+    # @param key [Object] Key to look up
+    # @return [Object] Value
+    #
     def list(node, key)
       # FIXME: real exception
       raise "first argument must be a Chef::Node" unless node.is_a?(Chef::Node)
diff --git a/lib/chef/platform/provider_priority_map.rb b/lib/chef/platform/provider_priority_map.rb
index 2d250b5006..0a6c8d8b4d 100644
--- a/lib/chef/platform/provider_priority_map.rb
+++ b/lib/chef/platform/provider_priority_map.rb
@@ -10,15 +10,16 @@ class Chef
       end
 
       def set_priority_array(resource_name, priority_array, *filter)
-        priority(resource_name.to_sym, Array(priority_array), *filter)
+        priority(resource_name.to_sym, priority_array, *filter)
       end
 
-      def priority(*args)
-        priority_map.set(*args)
+      def priority(resource_name, priority_array, *filter)
+        priority_map.set(resource_name.to_sym, Array(priority_array), *filter)
       end
 
-      def list(node, resource_name)
-        priority_map.list(node, resource_name).flatten(1).uniq
+      # @api private
+      def list_handlers(node, resource_name)
+        priority_map.list(node, resource_name.to_sym).flatten(1).uniq
       end
 
       private
diff --git a/lib/chef/platform/resource_priority_map.rb b/lib/chef/platform/resource_priority_map.rb
index e692cbb800..34f13529fe 100644
--- a/lib/chef/platform/resource_priority_map.rb
+++ b/lib/chef/platform/resource_priority_map.rb
@@ -17,7 +17,8 @@ class Chef
         priority_map.set(*args)
       end
 
-      def list(*args)
+      # @api private
+      def list_handlers(*args)
         priority_map.list(*args).flatten(1).uniq
       end
 
diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index 52082123cb..984ad0974a 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -175,12 +175,17 @@ class Chef
       converge_actions.add_action(descriptions, &block)
     end
 
-    protected
+    def self.provides(short_name, opts={}, &block)
+      priority_map = Chef::Platform::ProviderPriorityMap.instance
+      priority_map.priority(short_name, self, opts, &block)
+    end
 
-    def self.provides_priority_map
-      Chef::Platform::ResourcePriorityMap.instance
+    def self.provides?(node, resource)
+      Chef::ProviderResolver.new(node, resource, :nothing).provided_by?(self)
     end
 
+    protected
+
     def converge_actions
       @converge_actions ||= ConvergeActions.new(@new_resource, run_context, @action)
     end
@@ -240,4 +245,5 @@ end
 require 'chef/chef_class'
 require 'chef/mixin/why_run'
 require 'chef/resource_collection'
+require 'chef/platform/provider_priority_map'
 require 'chef/runner'
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb
index 7c7bac0443..ea28062bac 100644
--- a/lib/chef/provider_resolver.rb
+++ b/lib/chef/provider_resolver.rb
@@ -119,33 +119,14 @@ class Chef
       @action = action
     end
 
-    # return a deterministically sorted list of Chef::Provider subclasses
-    def providers
-      @providers ||= Chef::Provider.descendants
-    end
-
     def resolve
       maybe_explicit_provider(resource) ||
         maybe_dynamic_provider_resolution(resource, action) ||
         maybe_chef_platform_lookup(resource)
     end
 
-    # this cut looks at if the provider can handle the resource type on the node
-    def enabled_handlers
-      @enabled_handlers ||=
-        providers.select do |klass|
-          # NB: this is different from resource_resolver which must pass a resource_name
-          # FIXME: deprecate this and normalize on passing resource_name here
-          klass.provides?(node, resource)
-        end.sort {|a,b| a.to_s <=> b.to_s }
-    end
-
-    # this cut looks at if the provider can handle the specific resource and action
-    def supported_handlers
-      @supported_handlers ||=
-        enabled_handlers.select do |klass|
-          klass.supports?(resource, action)
-        end
+    def provided_by?(provider_class)
+      prioritized_handlers.include?(provider_class)
     end
 
     private
@@ -158,40 +139,37 @@ class Chef
 
     # try dynamically finding a provider based on querying the providers to see what they support
     def maybe_dynamic_provider_resolution(resource, action)
-      # log this so we know what providers will work for the generic resource on the node (early cut)
-      Chef::Log.debug "providers for generic #{resource.resource_name} resource enabled on node include: #{enabled_handlers}"
-
-      # what providers were excluded by machine state (late cut)
-      Chef::Log.debug "providers that refused resource #{resource} were: #{enabled_handlers - supported_handlers}"
-      Chef::Log.debug "providers that support resource #{resource} include: #{supported_handlers}"
-
-      # if none of the providers specifically support the resource, we still need to pick one of the providers that are
-      # enabled on the node to handle the why-run use case.
-      handlers = supported_handlers.empty? ? enabled_handlers : supported_handlers
-      Chef::Log.debug "no providers supported the resource, falling back to enabled handlers" if supported_handlers.empty?
-
-      if handlers.count >= 2
-        # this magic stack ranks the providers by where they appear in the provider_priority_map, it is mostly used
-        # to pick amongst N different ways to start init scripts on different debian/ubuntu systems.
-        priority_list = [ get_priority_array(node, resource.resource_name) ].flatten.compact
-        handlers = handlers.sort_by { |x| i = priority_list.index x; i.nil? ? Float::INFINITY : i }
-        if priority_list.index(handlers.first).nil?
-          # if we had more than one and we picked one with a precidence of infinity that means that the resource_priority_map
-          # entry for this resource is missing -- we should probably raise here and force resolution of the ambiguity.
-          Chef::Log.warn "Ambiguous provider precedence: #{handlers}, please use Chef.set_provider_priority_array to provide determinism"
-        end
-        handlers = [ handlers.first ]
+      Chef::Log.debug "Providers for generic #{resource.resource_name} resource enabled on node include: #{enabled_handlers}"
+
+      # Get all the handlers in the priority bucket
+      handlers = prioritized_handlers
+
+      # Narrow it down to handlers that return `true` to `provides?`
+      # TODO deprecate this and don't bother calling--the fact that they said
+      # `provides` should be enough.  But we need to do it right now because
+      # some classes implement additional handling.
+      enabled_handlers = prioritized_handlers.select { |handler| handler.provides?(node, resource) }
+
+      # Narrow it down to handlers that return `true` to `supports?`
+      # TODO deprecate this and allow actions to be passed as a filter to
+      # `provides` so we don't have to have two separate things.
+      supported_handlers = handlers.select { |handler| handler.supports?(resource, action) }
+      if supported_handlers.empty?
+        # if none of the providers specifically support the resource, we still need to pick one of the providers that are
+        # enabled on the node to handle the why-run use case. FIXME we should only do this in why-run mode then.
+        Chef::Log.debug "No providers responded true to `supports?` for action #{action} on resource #{resource}, falling back to enabled handlers so we can return something anyway."
+        handler = enabled_handlers.first
+      else
+        handler = supported_handlers.first
       end
 
-      Chef::Log.debug "providers that survived replacement include: #{handlers}"
-
-      raise Chef::Exceptions::AmbiguousProviderResolution.new(resource, handlers) if handlers.count >= 2
-
-      Chef::Log.debug "dynamic provider resolver FAILED to resolve a provider" if handlers.empty?
-
-      return nil if handlers.empty?
+      if handler
+        Chef::Log.debug "Provider for action #{action} on resource #{resource} is #{handler}"
+      else
+        Chef::Log.debug "Dynamic provider resolver FAILED to resolve a provider for action #{action} on resource #{resource}"
+      end
 
-      handlers[0]
+      handler
     end
 
     # try the old static lookup of providers by platform
@@ -199,13 +177,51 @@ class Chef
       Chef::Platform.find_provider_for_node(node, resource)
     end
 
-    # dep injection hooks
-    def get_priority_array(node, resource_name)
-      provider_priority_map.get_priority_array(node, resource_name)
-    end
-
     def provider_priority_map
       Chef::Platform::ProviderPriorityMap.instance
     end
+
+    def prioritized_handlers
+      @prioritized_handlers ||=
+        provider_priority_map.list_handlers(node, resource.resource_name).flatten(1).uniq
+    end
+
+    module Deprecated
+      # return a deterministically sorted list of Chef::Provider subclasses
+      def providers
+        @providers ||= Chef::Provider.descendants
+      end
+
+      # this cut looks at if the provider can handle the resource type on the node
+      def enabled_handlers
+        @enabled_handlers ||=
+          providers.select do |klass|
+            # NB: this is different from resource_resolver which must pass a resource_name
+            # FIXME: deprecate this and normalize on passing resource_name here
+            klass.provides?(node, resource)
+          end.sort {|a,b| a.to_s <=> b.to_s }
+      end
+
+      # this cut looks at if the provider can handle the specific resource and action
+      def supported_handlers
+        @supported_handlers ||=
+          enabled_handlers.select do |klass|
+            klass.supports?(resource, action)
+          end
+      end
+
+      # If there are no providers for a DSL, we search through the
+      def prioritized_handlers
+        @prioritized_handlers ||= super || begin
+          result = providers.select { |handler| handler.provides?(node, resource) }.sort_by(:name)
+          if !result.empty?
+            Chef::Log.deprecation("#{resource.resource_name.to_sym} is marked as providing DSL #{method_symbol}, but provides #{resource.resource_name.to_sym.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.")
+          end
+          result
+        end
+      end
+    end
+    prepend Deprecated
   end
 end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 686b212e49..caced28721 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1104,11 +1104,16 @@ class Chef
     end
 
     def self.provides(name, opts={}, &block)
-      result = super
+      priority_map = Chef::Platform::ResourcePriorityMap.instance
+      result = priority_map.priority(name, self, opts, &block)
       Chef::DSL::Resources.add_resource_dsl(name)
       result
     end
 
+    def self.provides?(node, resource)
+      Chef::ResourceResolver.new(node, resource).provided_by?(self)
+    end
+
     # Helper for #notifies
     def validate_resource_spec!(resource_spec)
       run_context.resource_collection.validate_lookup_spec!(resource_spec)
diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb
index 47d98154a8..da34f832eb 100644
--- a/lib/chef/resource_resolver.rb
+++ b/lib/chef/resource_resolver.rb
@@ -33,22 +33,13 @@ class Chef
       @resource = resource.to_sym
     end
 
-    # return a deterministically sorted list of Chef::Resource subclasses
-    def resources
-      @resources ||= Chef::Resource.descendants
-    end
-
     def resolve
       maybe_dynamic_resource_resolution ||
         maybe_chef_platform_lookup
     end
 
-    # this cut looks at if the resource can handle the resource type on the node
-    def enabled_handlers
-      @enabled_handlers ||=
-        resources.select do |klass|
-          klass.provides?(node, resource)
-        end.sort {|a,b| a.to_s <=> b.to_s }
+    def provided_by?(resource_class)
+      !prioritized_handlers.include?(resource_class)
     end
 
     #
@@ -61,32 +52,22 @@ class Chef
       new(node, resource_name).resolve
     end
 
-    private
+    protected
 
     # try dynamically finding a resource based on querying the resources to see what they support
-    def maybe_dynamic_resource_resolution      # 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}"
-
-      # if none of the resources specifically support the resource, we still need to pick one of the resources that are
-      # enabled on the node to handle the why-run use case.
-      handlers = enabled_handlers
-
-      if handlers.size >= 2
-        # this magic stack ranks the resources by where they appear in the resource_priority_map
-        priority_list = [ get_priority_array(node, resource) ].flatten.compact
-        handlers = handlers.sort_by { |x| i = priority_list.index x; i.nil? ? Float::INFINITY : i }
-        handlers = handlers[0..0]
-      end
-
-      Chef::Log.debug "resources that survived replacement include: #{handlers}"
+    def maybe_dynamic_resource_resolution
+      # 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}"
 
-      raise Chef::Exceptions::AmbiguousResourceResolution.new(resource, handlers) if handlers.count >= 2
+      handler = prioritized_handlers.first
 
-      Chef::Log.debug "dynamic resource resolver FAILED to resolve a resource" if handlers.empty?
-
-      return nil if handlers.empty?
+      if handler
+        Chef::Log.debug "Resource for #{resource} is #{handler}"
+      else
+        Chef::Log.debug "Dynamic resource resolver FAILED to resolve a resource for #{resource}"
+      end
 
-      handlers[0]
+      handler
     end
 
     # try the old static lookup of resources by mangling name to resource klass
@@ -94,13 +75,42 @@ class Chef
       Chef::Resource.resource_matching_short_name(resource)
     end
 
-    # dep injection hooks
-    def get_priority_array(node, resource_name)
-      resource_priority_map.get_priority_array(node, resource_name)
+    def priority_map
+      Chef::Platform::ResourcePriorityMap.instance
     end
 
-    def resource_priority_map
-      Chef::Platform::ResourcePriorityMap.instance
+    def prioritized_handlers
+      @prioritized_handlers ||=
+        priority_map.list_handlers(node, resource)
+    end
+
+    module Deprecated
+      # return a deterministically sorted list of Chef::Resource subclasses
+      def resources
+        @resources ||= Chef::Resource.descendants
+      end
+
+      # this cut looks at if the resource can handle the resource type on the node
+      def enabled_handlers
+        @enabled_handlers ||=
+          resources.select do |klass|
+            klass.provides?(node, resource)
+          end.sort {|a,b| a.to_s <=> b.to_s }
+      end
+
+      protected
+
+      # If there are no providers for a DSL, we search through the
+      def prioritized_handlers
+        @prioritized_handlers ||= super || begin
+          if !enabled_handlers.empty?
+            Chef::Log.deprecation("#{resource} is marked as providing DSL #{resource}, but provides #{resource.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.")
+          end
+          enabled_handlers
+        end
+      end
     end
+    prepend Deprecated
   end
 end
diff --git a/spec/functional/resource/template_spec.rb b/spec/functional/resource/template_spec.rb
index 7cd33ed403..35c5166e31 100644
--- a/spec/functional/resource/template_spec.rb
+++ b/spec/functional/resource/template_spec.rb
@@ -31,8 +31,6 @@ describe Chef::Resource::Template do
 
   let(:node) do
     node = Chef::Node.new
-    node.normal[:os] = 'linux'
-    node.normal[:os_version] = '1.0.0'
     node.normal[:slappiness] = "a warm gun"
     node
   end
diff --git a/spec/integration/recipes/recipe_dsl_spec.rb b/spec/integration/recipes/recipe_dsl_spec.rb
index c79c20cd7d..9d6b15a478 100644
--- a/spec/integration/recipes/recipe_dsl_spec.rb
+++ b/spec/integration/recipes/recipe_dsl_spec.rb
@@ -70,7 +70,7 @@ describe "Recipe DSL methods" do
 
         }
 
-        it "backcompat_thingy creates a Chef::Resource::BackcompatThingy" do
+        it "backcompat_thingy creates a Chef::Resource::BackcompatThingy", :focus do
           recipe = converge {
             backcompat_thingy 'blah' do; end
           }