add methods for verifying and splitting attributes, fix behavior of find_closest_providers_for to look for a parent attribute that actually has providers, create new error Ohai::Exceptions::ProviderNotFound to indicate when an attribute exists in the mapping but doesn't have any listed providers

4 files changed, 116 insertions, 53 deletions

diff --git a/lib/ohai/exception.rb b/lib/ohai/exception.rb
index 1ce964e6..144a70df 100644
--- a/lib/ohai/exception.rb
+++ b/lib/ohai/exception.rb
@@ -21,6 +21,7 @@ module Ohai
     class Exec < RuntimeError; end
     class IllegalPluginDefinition < Exception; end
     class AttributeNotFound < Exception; end
+    class ProviderNotFound < Exception; end
     class DependencyCycle < Exception; end
     class DependencyNotFound < Exception; end
     class AttributeSyntaxError < Exception; end
diff --git a/lib/ohai/provides_map.rb b/lib/ohai/provides_map.rb
index bd07c7d6..c2da2124 100644
--- a/lib/ohai/provides_map.rb
+++ b/lib/ohai/provides_map.rb
@@ -38,30 +38,52 @@ module Ohai
 
       provided_attributes.each do |attribute|
         attrs = @map
-        parts = attribute.split('/')
-        parts.shift if parts.length != 0 && parts[0].length == 0 # attribute begins with a '/'
-        unless parts.length == 0
-          parts.each do |part|
-            raise Ohai::Exceptions::AttributeSyntaxError, "Attribute contains duplicate '/' characters: #{attribute}" if part.length == 0
-            attrs[part] ||= Mash.new
-            attrs = attrs[part]
-          end
-          attrs[:_plugins] ||= []
-          attrs[:_plugins] << plugin
+        parts = normalize_and_validate(attribute)
+        parts.each do |part|
+          attrs[part] ||= Mash.new
+          attrs = attrs[part]
         end
+        attrs[:_plugins] ||= []
+        attrs[:_plugins] << plugin
       end
     end
 
+    # gather plugins providing exactly the attributes listed
     def find_providers_for(attributes)
-      find_with_search_type(attributes, :strict)
+      plugins = []
+      attributes.each do |attribute|
+        attrs = select_subtree(@map, attribute)
+        raise Ohai::Exceptions::AttributeNotFound, "No such attribute: \'#{attribute}\'" unless attrs
+        raise Ohai::Exceptions::ProviderNotFound, "Cannot find plugin providing attribute: \'#{attribute}\'" unless attrs[:_plugins]
+        plugins += attrs[:_plugins]
+      end
+      plugins.uniq
     end
 
+    # gather plugins providing each of the attributes listed along
+    # with providers of subattributes
     def deep_find_providers_for(attributes)
-      find_with_search_type(attributes, :deep)
+      plugins = []
+      attributes.each do |attribute|
+        attrs = select_subtree(@map, attribute)
+        raise Ohai::Exceptions::AttributeNotFound, "No such attribute: \'#{attribute}\'" unless attrs
+        collect_plugins_in(attrs, plugins)
+      end
+      plugins.uniq
     end
 
+    # gather plugins providing each of the attributes listed, or the
+    # plugins providing the closest parent attribute
     def find_closest_providers_for(attributes)
-      find_with_search_type(attributes, :closest)
+      plugins = []
+      attributes.each do |attribute|
+        parts = normalize_and_validate(attribute)
+        raise Ohai::Exceptions::AttributeNotFound, "No such attribute: \'#{attribute}\'" unless @map[parts[0]]
+        attrs = select_closest_subtree(@map, attribute)
+        raise Ohai::Exceptions::ProviderNotFound, "Cannot find plugin providing attribute: \'#{attribute}\'" unless attrs
+        plugins += attrs[:_plugins]
+      end
+      plugins.uniq
     end
 
     def all_plugins(attribute_filter=nil)
@@ -75,42 +97,57 @@ module Ohai
 
     private
 
-    # Finds providers for each listed attribute using the given search
-    # type. Search types behave as follows:
-    #   1. :strict  => Returns a list of unique plugins explicitly
-    #                  providing one or more of the listed attributes.
-    #                  Raises Ohai::Exceptions::AttributeNotFound
-    #                  error if at least one attribute does not exists
-    #                  in the map. 
-    #   2. :deep    => For each listed attribute, gathers all the
-    #                  unique plugins providing that attribute and any
-    #                  of its subattributes.
-    #                  Raises Ohai::Exceptions::AttributeNotFound
-    #                  error if at least one attribute does not exists
-    #                  in the map.
-    #   3. :closest => For each listed attribute, gathers the
-    #                  providers for the most specific parent. If the
-    #                  attribute exists in the mapping, its providers
-    #                  are gathered. If the least specific parent does
-    #                  not exist in the map an
-    #                  Ohai::Exceptions::AttributeNotFound error is
-    #                  raised. 
-    def find_with_search_type(attributes, search_type=:strict)
-      plugins = []
-      attributes.each do |attribute|
-        attrs = @map
-        parts = attribute.split('/')
-        parts.shift if parts.length != 0 && parts[0].length == 0
-        parts.each do |part|
-          unless attrs[part]
-            break if search_type.eql?(:closest) && part != parts[0]
-            raise Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute \'#{attribute}\'"
-          end
-          attrs = attrs[part]
+    def normalize_and_validate(attribute)
+      raise Ohai::Exceptions::AttributeSyntaxError, "Attribute contains duplicate '/' characters: #{attribute}" if attribute.match(/\/\/+/)
+      raise Ohai::Exceptions::AttributeSyntaxError, "Attribute contains a trailing '/': #{attribute}" if attribute.match(/\/$/)
+
+      parts = attribute.split('/')
+      parts.shift if parts.length != 0 && parts[0].length == 0 # attribute begins with a '/'
+      parts
+    end
+
+    def select_subtree(provides_map, attribute)
+      subtree = provides_map
+      parts = normalize_and_validate(attribute)
+      parts.each do |part|
+        return nil unless subtree[part]
+        subtree = subtree[part]
+      end
+      subtree
+    end
+
+    def select_closest_subtree(provides_map, attribute)
+      attr, *rest = normalize_and_validate(attribute)
+
+      # return nil if the top-level part of the attribute is not a
+      # top-level key in the provides_map (can't search any lower, and
+      # no information to return from this level of the search)
+      return nil unless provides_map[attr]
+
+      # attr is a key in the provides_map, search for the sub
+      # attribute under attr (if attribute = attr/sub1/sub2 then we
+      # search provides_map[attr] for sub1/sub2)
+      unless rest.empty?
+        subtree = select_closest_subtree(provides_map[attr], rest.join('/'))
+      end
+
+      if subtree.nil?
+        # no subtree of provides_map[attr] either 1) has a
+        # subattribute, 2) has a plugin providing a subattribute.
+        unless provides_map[attr][:_plugins]
+          # no providers for this attribute, this subtree won't do.
+          return nil # no providers for this attribute
+        else
+          # there are providers for this attribute, return its subtree
+          # to indicate this is the closest subtree
+          return provides_map[attr]
         end
-        search_type.eql?(:deep) ? plugins += collect_plugins_in(attrs, []) : plugins += attrs[:_plugins]
       end
-      plugins.uniq
+
+      # we've already found a closest subtree or we've search all
+      # parent attributes of the original attribute and found no
+      # providers (subtree is nil in this case)
+      subtree
     end
 
     # Takes a section of the map, recursively searches for a `_plugins` key
diff --git a/spec/unit/provides_map_spec.rb b/spec/unit/provides_map_spec.rb
index 1b9e92d8..3db88d21 100644
--- a/spec/unit/provides_map_spec.rb
+++ b/spec/unit/provides_map_spec.rb
@@ -29,9 +29,16 @@ describe Ohai::ProvidesMap do
   let(:plugin_4) { Ohai::DSL::Plugin.new(ohai_system.data) }
 
   describe "when looking up providing plugins for a single attribute" do
-    describe "when no plugin provides the attribute" do
-      it "should raise Ohai::Exceptions::AttributeNotFound error, with inherit = false" do
-        expect{ provides_map.find_providers_for(["single"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute 'single'")
+    describe "when the attribute does not exist" do
+      it "should raise Ohai::Exceptions::AttributeNotFound error" do
+        expect{ provides_map.find_providers_for(["single"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "No such attribute: 'single'")
+      end
+    end
+
+    describe "when the attribute does not have a provider" do
+      it "should raise Ohai::Exceptions::ProviderNotFound error" do
+        provides_map.set_providers_for(plugin_1, ["first/second"])
+        expect{ provides_map.find_providers_for(["first"]) }.to raise_error(Ohai::Exceptions::ProviderNotFound, "Cannot find plugin providing attribute: 'first'")
       end
     end
 
@@ -95,6 +102,20 @@ describe Ohai::ProvidesMap do
     end
   end
 
+  describe "when setting multi-level attributes" do
+    describe "when the attribute contains //" do
+      it "should raise an Ohai::Exceptions::AttributeSyntaxError" do
+        expect{ provides_map.set_providers_for(plugin_1, ["this/plugin//is/bad"]) }.to raise_error(Ohai::Exceptions::AttributeSyntaxError, "Attribute contains duplicate '/' characters: this/plugin//is/bad")
+      end
+    end
+
+    describe "when the attribute has a trailing slash" do
+      it "should raise an Ohai::Exceptions::AttributeSyntaxError" do
+        expect{ provides_map.set_providers_for(plugin_1, ["this/plugin/is/bad/"]) }.to raise_error(Ohai::Exceptions::AttributeSyntaxError, "Attribute contains a trailing '/': this/plugin/is/bad/")
+      end
+    end
+  end
+
   describe "when looking for attribute and subattribute providers" do
     before do
       provides_map.set_providers_for(plugin_1, ["cat/whiskers"])
@@ -136,8 +157,12 @@ describe Ohai::ProvidesMap do
         expect(provides_map.find_closest_providers_for(["do/not/eat/glass"])).to eql([plugin_1])
       end
 
-      it "should raise error if the least-specific parent is not provided" do
-        expect{ provides_map.find_closest_providers_for(["please/eat/your/vegetables"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute 'please/eat/your/vegetables'")
+      it "should raise error if the least-specific parent is not an attribute" do
+        expect{ provides_map.find_closest_providers_for(["please/eat/your/vegetables"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "No such attribute: 'please/eat/your/vegetables'")
+      end
+
+      it "should raise error if no parent attribute has a provider" do
+        expect{ provides_map.find_closest_providers_for(["do/not"]) }.to raise_error(Ohai::Exceptions::ProviderNotFound, "Cannot find plugin providing attribute: 'do/not'")
       end
     end
   end
diff --git a/spec/unit/runner_spec.rb b/spec/unit/runner_spec.rb
index 36c0c0d8..0c2bea10 100644
--- a/spec/unit/runner_spec.rb
+++ b/spec/unit/runner_spec.rb
@@ -409,7 +409,7 @@ describe Ohai::Runner, "fetch_plugins" do
     describe "and no parent attribute has providers" do
       it "should raise Ohai::Exceptions::AttributeNotFound exception" do
         # provides map is empty
-        expect{ @runner.fetch_plugins(["false/attribute"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute 'false/attribute'")
+        expect{ @runner.fetch_plugins(["false/attribute"]) }.to raise_error(Ohai::Exceptions::AttributeNotFound, "No such attribute: 'false/attribute'")
       end
     end
   end