Call provides? when resolving, reduce number of calls to provides?

in Provider

5 files changed, 70 insertions, 76 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index dd0bac3cf9..235c09c1a8 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -97,6 +97,7 @@ class Chef
     class ConflictingMembersInGroup < ArgumentError; end
     class InvalidResourceReference < RuntimeError; end
     class ResourceNotFound < RuntimeError; end
+    class ProviderNotFound < ArgumentError; end
     class VerificationNotFound < RuntimeError; end
 
     # Can't find a Resource of this type that is valid on this platform.
diff --git a/lib/chef/platform/provider_mapping.rb b/lib/chef/platform/provider_mapping.rb
index 4278b8d24f..38dd0e38af 100644
--- a/lib/chef/platform/provider_mapping.rb
+++ b/lib/chef/platform/provider_mapping.rb
@@ -176,7 +176,7 @@ class Chef
                          platform_provider(platform, version, resource_type) ||
                          resource_matching_provider(platform, version, resource_type)
 
-        raise ArgumentError, "Cannot find a provider for #{resource_type} on #{platform} version #{version}" if provider_klass.nil?
+        raise Chef::Exceptions::ProviderNotFound, "Cannot find a provider for #{resource_type} on #{platform} version #{version}" if provider_klass.nil?
 
         provider_klass
       end
@@ -197,7 +197,7 @@ class Chef
 
         def resource_matching_provider(platform, version, resource_type)
           if resource_type.kind_of?(Chef::Resource)
-            class_name = resource_type.class.name ? resource_type.class.name.split('::').last : 
+            class_name = resource_type.class.name ? resource_type.class.name.split('::').last :
               convert_to_class_name(resource_type.resource_name.to_s)
 
             begin
diff --git a/lib/chef/provider_resolver.rb b/lib/chef/provider_resolver.rb
index 5bfee343d1..d810900a03 100644
--- a/lib/chef/provider_resolver.rb
+++ b/lib/chef/provider_resolver.rb
@@ -17,7 +17,7 @@
 #
 
 require 'chef/exceptions'
-require 'chef/platform/provider_priority_map'
+require 'chef/platform/priority_map'
 
 class Chef
   #
@@ -63,7 +63,7 @@ class Chef
     end
 
     def provided_by?(provider_class)
-      prioritized_handlers.include?(provider_class)
+      potential_handlers.include?(provider_class)
     end
 
     private
@@ -78,19 +78,6 @@ class Chef
     def maybe_dynamic_provider_resolution(resource, action)
       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 = enabled_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.
@@ -114,13 +101,25 @@ class Chef
       Chef::Platform.find_provider_for_node(node, resource)
     end
 
-    def provider_priority_map
+    def priority_map
       Chef::Platform::ProviderPriorityMap.instance
     end
 
-    def prioritized_handlers
-      @prioritized_handlers ||=
-        provider_priority_map.list_handlers(node, resource.resource_name).flatten(1).uniq
+    # @api private
+    def potential_handlers
+      priority_map.list_handlers(node, resource.resource_name).flatten(1).uniq
+    end
+
+    def enabled_handlers
+      @enabled_handlers ||= potential_handlers.select { |handler| handler.method(:provides?).owner == Chef::Provider || handler.provides?(node, resource) }
+    end
+
+    # TODO deprecate this and allow actions to be passed as a filter to
+    # `provides` so we don't have to have two separate things.
+    # @api private
+    def supported_handlers
+      @supported_handlers ||=
+        enabled_handlers.select { |handler| handler.supports?(resource, action) }
     end
 
     module Deprecated
@@ -129,33 +128,28 @@ class Chef
         @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)
+        @enabled_handlers ||= begin
+          handlers = potential_handlers
+          # If there are no potential handlers for this node (nobody called provides)
+          # then we search through all classes and call provides in case someone
+          # defined a provides? method that returned true in spite of provides
+          # not being called
+          if handlers.empty?
+            warn = true
+            handlers = providers
           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.")
+          handlers.select do |handler|
+            if handler.method(:provides?).owner == Chef::Provider
+              true
+            elsif handler.provides?(node, resource)
+              if warn
+                Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides DSL #{resource.resource_name}, 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
+              true
+            end
           end
-          result
         end
       end
     end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 704490e748..9510351d16 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -1297,7 +1297,7 @@ class Chef
     end
     def self.inherited(child)
       super
-      @sorted_descendants = nil
+      @@sorted_descendants = nil
       # set resource_name automatically if it's not set
       if child.name && !child.resource_name
         if child.name =~ /^Chef::Resource::(\w+)$/
@@ -1340,8 +1340,8 @@ class Chef
       result
     end
 
-    def self.provides?(node, resource)
-      Chef::ResourceResolver.resolve(resource, node: node).provided_by?(self)
+    def self.provides?(node, resource_name)
+      Chef::ResourceResolver.new(node, resource_name).provided_by?(self)
     end
 
     # Helper for #notifies
diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb
index 9df627beb2..11121df04a 100644
--- a/lib/chef/resource_resolver.rb
+++ b/lib/chef/resource_resolver.rb
@@ -83,9 +83,9 @@ class Chef
     # @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_name} resource enabled on node include: #{prioritized_handlers}"
+      Chef::Log.debug "Resources for generic #{resource_name} resource enabled on node include: #{enabled_handlers}"
 
-      handler = prioritized_handlers.first
+      handler = enabled_handlers.first
 
       if handler
         Chef::Log.debug "Resource for #{resource_name} is #{handler}"
@@ -98,8 +98,8 @@ class Chef
 
     # @api private
     def list
-      Chef::Log.debug "Resources for generic #{resource_name} resource enabled on node include: #{prioritized_handlers}"
-      prioritized_handlers
+      Chef::Log.debug "Resources for generic #{resource_name} resource enabled on node include: #{enabled_handlers}"
+      enabled_handlers
     end
 
     #
@@ -107,7 +107,7 @@ class Chef
     #
     # @api private
     def provided_by?(resource_class)
-      !prioritized_handlers.include?(resource_class)
+      potential_handlers.include?(resource_class)
     end
 
     protected
@@ -116,41 +116,40 @@ class Chef
       Chef::Platform::ResourcePriorityMap.instance
     end
 
-    def prioritized_handlers
-      @prioritized_handlers ||=
-        priority_map.list_handlers(node, resource_name, canonical: canonical)
+    # @api private
+    def potential_handlers
+      priority_map.list_handlers(node, resource_name, canonical: canonical).flatten(1).uniq
+    end
+
+    def enabled_handlers
+      potential_handlers.select { |handler| handler.method(:provides?).owner == Chef::Resource || handler.provides?(node, resource_name) }
     end
 
     module Deprecated
       # return a deterministically sorted list of Chef::Resource subclasses
-      # @deprecated Now prioritized_handlers does its own work (more efficiently)
       def resources
         Chef::Resource.sorted_descendants
       end
 
-      # A list of all handlers
-      # @deprecated Now prioritized_handlers does its own work
       def enabled_handlers
-        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
-
-      # 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 overridden. We already
-            # know prioritized_handlers
-            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.")
+        @enabled_handlers ||= begin
+          handlers = potential_handlers
+          if handlers.empty?
+            warn = true
+            handlers = resources
+          end
+          handlers.select do |handler|
+            if handler.method(:provides?).owner == Chef::Resource
+              true
+            elsif handler.provides?(node, resource_name)
+              if warn
+                Chef::Log.deprecation("#{handler}.provides? returned true when asked if it provides 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.")
+              end
               true
             end
           end
+        end
       end
     end
     prepend Deprecated