Merge branch 'plugin-provider-cleanup'

Extract a class to maintain the provided attributes to providing plugins mapping
prein · Dec 5, 2013 · 0f40633 · 0f40633
2 parents 85bb7dd + 5fe6a47
commit 0f40633
Show file tree

Hide file tree

Showing 12 changed files with 295 additions and 220 deletions.
diff --git a/lib/ohai/dsl/plugin.rb b/lib/ohai/dsl/plugin.rb
@@ -109,12 +109,15 @@ class Plugin
 
       def initialize(controller, source)
         @controller = controller
-        @attributes = controller.attributes
         @data = controller.data
         @source = source
         @has_run = false
       end
 
+      def provides_map
+        @controller.provides_map
+      end
+
       def run
         @has_run = true
         run_plugin
@@ -227,19 +230,7 @@ def self.collect_contents(contents)
         end
 
         def provides(*paths)
-          paths.each do |path|
-            parts = path.split("/")
-            a = @attributes
-            unless parts.length == 0
-              parts.shift if parts[0].length == 0
-              parts.each do |part|
-                a[part] ||= Mash.new
-                a = a[part]
-              end
-            end
-            a[:_plugins] ||= []
-            a[:_plugins] << self
-          end
+          provides_map.set_providers_for(self, paths)
         end
 
         def require_plugin(*args)

diff --git a/lib/ohai/loader.rb b/lib/ohai/loader.rb
@@ -25,7 +25,10 @@ class Loader
 
     def initialize(controller)
       @controller = controller
-      @attributes = controller.attributes
+    end
+
+    def provides_map
+      @controller.provides_map
     end
 
     # @note: plugin_name is used only by version 6 plugins and is the
@@ -68,21 +71,7 @@ def load_plugin(plugin_path, plugin_name=nil)
 
     def collect_provides(plugin)
       plugin_provides = plugin.class.provides_attrs
-
-      plugin_provides.each do |attr|
-        parts = attr.split('/')
-        a = @attributes
-        unless parts.length == 0
-          parts.shift if parts[0].length == 0
-          parts.each do |part|
-            a[part] ||= Mash.new
-            a = a[part]
-          end
-        end
-
-        a[:_plugins] ||= []
-        a[:_plugins] << plugin
-      end
+      provides_map.set_providers_for(plugin, plugin_provides)
     end
 
   end

diff --git a/lib/ohai/provides_map.rb b/lib/ohai/provides_map.rb
@@ -0,0 +1,90 @@
+#
+# Author:: Adam Jacob (<adam@opscode.com>)
+# Author:: Daniel DeLeo (<dan@opscode.com>)
+# Copyright:: Copyright (c) 2008, 2013 Opscode, Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require 'ohai/mash'
+require 'ohai/exception'
+require 'ohai/mixin/os'
+require 'ohai/dsl/plugin'
+
+module Ohai
+  class ProvidesMap
+
+    attr_reader :map
+
+    def initialize
+      @map = Mash.new
+    end
+
+    def set_providers_for(plugin, provided_attributes)
+      unless plugin.kind_of?(Ohai::DSL::Plugin)
+        raise ArgumentError, "set_providers_for only accepts Ohai Plugin classes (got: #{plugin})"
+      end
+      provided_attributes.each do |attr|
+        parts = attr.split('/')
+        a = map
+        unless parts.length == 0
+          parts.shift if parts[0].length == 0
+          parts.each do |part|
+            a[part] ||= Mash.new
+            a = a[part]
+          end
+        end
+
+        a[:_plugins] ||= []
+        a[:_plugins] << plugin
+      end
+    end
+
+    def find_providers_for(attributes)
+      plugins = []
+      attributes.each do |attribute|
+        attrs = map
+        parts = attribute.split('/')
+        parts.each do |part|
+          next if part == Ohai::Mixin::OS.collect_os
+          raise Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute \'#{attribute}\'" unless attrs[part]
+          attrs = attrs[part]
+        end
+        plugins << attrs[:_plugins]
+        plugins.flatten!
+      end
+      plugins.uniq
+    end
+
+
+    def all_plugins
+      collected = []
+      collect_plugins_in(map, collected).uniq
+    end
+
+    private
+
+    def collect_plugins_in(provides_map, collected)
+      provides_map.keys.each do |plugin|
+        if plugin.eql?("_plugins")
+          collected.concat(provides_map[plugin])
+        else
+          collect_plugins_in(provides_map[plugin], collected)
+        end
+      end
+      collected
+    end
+  end
+end
+
diff --git a/lib/ohai/runner.rb b/lib/ohai/runner.rb
@@ -21,53 +21,45 @@
 
 module Ohai
   class Runner
+
     # safe_run: set to true if this runner will run plugins in
     # safe-mode. default false.
     def initialize(controller, safe_run = false)
-      @attributes = controller.attributes
+      @provides_map = controller.provides_map
       @safe_run = safe_run
     end
 
     # runs this plugin and any un-run dependencies. if force is set to
     # true, then this plugin and its dependencies will be run even if
     # they have been run before.
     def run_plugin(plugin, force = false)
+      unless plugin.kind_of?(Ohai::DSL::Plugin)
+        raise ArgumentError, "Invalid plugin #{plugin} (must be an Ohai::DSL::Plugin or subclass)"
+      end
       visited = [plugin]
       while !visited.empty?
-        p = visited.pop
+        next_plugin = visited.pop
 
-        next if p.has_run? unless force
+        next if next_plugin.has_run? unless force
 
-        if visited.include?(p)
+        if visited.include?(next_plugin)
           raise Ohai::Exceptions::DependencyCycle, "Dependency cycle detected. Please refer to the following plugins: #{get_cycle(visited, p).join(", ") }"
         end
 
-        dependency_providers = fetch_plugins(p.dependencies)
-        dependency_providers.delete_if { |plugin| (!force && plugin.has_run?) || plugin.eql?(p) }
+        dependency_providers = fetch_plugins(next_plugin.dependencies)
+        dependency_providers.delete_if { |dep_plugin| (!force && dep_plugin.has_run?) || dep_plugin.eql?(next_plugin) }
 
         if dependency_providers.empty?
-          @safe_run ? p.safe_run : p.run
+          @safe_run ? next_plugin.safe_run : next_plugin.run
         else
-          visited << p << dependency_providers.first
+          visited << next_plugin << dependency_providers.first
         end
       end
     end
 
     # returns a list of plugins which provide the given attributes
     def fetch_plugins(attributes)
-      plugins = []
-      attributes.each do |attribute|
-        attrs = @attributes
-        parts = attribute.split('/')
-        parts.each do |part|
-          next if part == Ohai::Mixin::OS.collect_os
-          raise Ohai::Exceptions::AttributeNotFound, "Cannot find plugin providing attribute \'#{attribute}\'" unless attrs[part]
-          attrs = attrs[part]
-        end
-        plugins << attrs[:_plugins]
-        plugins.flatten!
-      end
-      plugins.uniq
+      @provides_map.find_providers_for(attributes)
     end
 
     # given a list of plugins and the first plugin in the cycle,

diff --git a/lib/ohai/system.rb b/lib/ohai/system.rb
@@ -25,20 +25,23 @@
 require 'ohai/mixin/command'
 require 'ohai/mixin/os'
 require 'ohai/mixin/string'
+require 'ohai/provides_map'
 require 'mixlib/shellout'
 
 require 'yajl'
 
 module Ohai
+
   class System
     attr_accessor :data
-    attr_reader :attributes
+    attr_reader :provides_map
     attr_reader :hints
     attr_reader :v6_dependency_solver
 
     def initialize
       @data = Mash.new
-      @attributes = Mash.new
+      @provides_map = ProvidesMap.new
+
       @hints = Hash.new
       @v6_dependency_solver = Hash.new
       @plugin_path = ""
@@ -92,7 +95,8 @@ def run_plugins(safe = false, force = false)
       end
 
       # collect and run version 7 plugins
-      plugins = collect_plugins(@attributes)
+      plugins = @provides_map.all_plugins
+
       begin
         plugins.each { |plugin| @runner.run_plugin(plugin, force) }
       rescue Ohai::Exceptions::AttributeNotFound, Ohai::Exceptions::DependencyCycle => e
@@ -105,6 +109,7 @@ def run_plugins(safe = false, force = false)
     def collect_plugins(plugins)
       collected = []
       if plugins.is_a?(Mash)
+        # TODO: remove this branch
         plugins.keys.each do |plugin|
           if plugin.eql?("_plugins")
             collected << plugins[plugin]

diff --git a/spec/ohai/dsl/plugin_spec.rb b/spec/ohai/dsl/plugin_spec.rb
@@ -43,8 +43,7 @@
   end
 
   context "when accessing data via method_missing" do
-    it "should take a missing method and store the method name as a key, with its arguments as value\
-s" do
+    it "should take a missing method and store the method name as a key, with its arguments as values" do
       plugin.guns_n_roses("chinese democracy")
       plugin.data["guns_n_roses"].should eql("chinese democracy")
     end
@@ -223,24 +222,24 @@
   describe "#provides (deprecated)" do
     it "should log a warning" do
       plugin = Ohai::DSL::Plugin::VersionVII.new(Ohai::System.new, "")
-      Ohai::Log.should_receive(:warn).with(/[UNSUPPORTED OPERATION]/)
+      Ohai::Log.should_receive(:warn).with(/\[UNSUPPORTED OPERATION\]/)
       plugin.provides("attribute")
     end
   end
 
   describe "#require_plugin (deprecated)" do
     it "should log a warning" do
       plugin = Ohai::DSL::Plugin::VersionVII.new(Ohai::System.new, "")
-      Ohai::Log.should_receive(:warn).with(/[UNSUPPORTED OPERATION]/)
+      Ohai::Log.should_receive(:warn).with(/\[UNSUPPORTED OPERATION\]/)
       plugin.require_plugin("plugin")
     end
   end
 
   it_behaves_like "Ohai::DSL::Plugin" do
-    let (:ohai) { Ohai::System.new }
-    let (:source) { "path/plugin.rb" }
-    let (:plugin) { Ohai::DSL::Plugin::VersionVII.new(ohai, source) }
-    let (:version) { :version7 }
+    let(:ohai) { Ohai::System.new }
+    let(:source) { "path/plugin.rb" }
+    let(:plugin) { Ohai::DSL::Plugin::VersionVII.new(ohai, source) }
+    let(:version) { :version7 }
   end
 end
 
@@ -272,24 +271,23 @@
       plugin = Ohai::DSL::Plugin::VersionVI.new(@ohai, "")
       plugin.provides("attribute")
 
-      @ohai.attributes.should have_key(:attribute)
+      @ohai.provides_map.find_providers_for(["attribute"]).should eq([plugin])
     end
 
     it "should collect a list of attributes" do
       plugin = Ohai::DSL::Plugin::VersionVI.new(@ohai, "")
       plugin.provides("attr1", "attr2", "attr3")
 
-      [:attr1, :attr2, :attr3].each do |attr|
-        @ohai.attributes.should have_key(attr)
+      %w[attr1 attr2 attr3].each do |attr|
+        @ohai.provides_map.find_providers_for([attr]).should eq([plugin])
       end
     end
 
     it "should collect subattributes of an attribute" do
       plugin = Ohai::DSL::Plugin::VersionVI.new(@ohai, "")
       plugin.provides("attr/subattr")
 
-      @ohai.attributes.should have_key(:attr)
-      @ohai.attributes[:attr].should have_key(:subattr)
+      @ohai.provides_map.find_providers_for(["attr/subattr"]).should eq([plugin])
     end
 
     it "should collect all unique providers for an attribute" do
@@ -300,14 +298,14 @@
         plugins << p
       end
 
-      @ohai.attributes[:attribute][:_plugins].should eql(plugins)
+      @ohai.provides_map.find_providers_for(["attribute"]).should =~ plugins
     end
   end
 
   it_behaves_like "Ohai::DSL::Plugin" do
-    let (:ohai) { Ohai::System.new }
-    let (:source) { "path/plugin.rb" }
-    let (:plugin) { Ohai::DSL::Plugin::VersionVI.new(ohai, source) }
-    let (:version) { :version6 }
+    let(:ohai) { Ohai::System.new }
+    let(:source) { "path/plugin.rb" }
+    let(:plugin) { Ohai::DSL::Plugin::VersionVI.new(ohai, source) }
+    let(:version) { :version6 }
   end
 end
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -117,13 +117,17 @@ def from_file(filename)
 RSpec.configure do |config|
   config.treat_symbols_as_metadata_keys_with_true_values = true
 
+  config.filter_run :focus => true
+
   config.filter_run_excluding :windows_only => true unless windows?
   config.filter_run_excluding :unix_only => true unless unix?
   config.filter_run_excluding :ruby_18_only => true unless ruby_18?
   config.filter_run_excluding :ruby_19_only => true unless ruby_19?
   config.filter_run_excluding :requires_root => true unless ENV['USER'] == 'root'
   config.filter_run_excluding :requires_unprivileged_user => true if ENV['USER'] == 'root'
 
+  config.run_all_when_everything_filtered = true
+
   config.before :each do
     Ohai::Config.reset
   end