Merge pull request #796 from chef/OHAI-794/attribute-from-mash

Safely get or check the existence of attributes

2 files changed, 276 insertions, 62 deletions

diff --git a/lib/ohai/dsl/plugin.rb b/lib/ohai/dsl/plugin.rb
index e51923f0..adcb6f82 100644
--- a/lib/ohai/dsl/plugin.rb
+++ b/lib/ohai/dsl/plugin.rb
@@ -125,7 +125,9 @@ module Ohai
         @data.has_key?(name)
       end
 
-      alias :attribute? :has_key?
+      def attribute?(name, *keys)
+        !safe_get_attribute(name, *keys).nil?
+      end
 
       def set(name, *value)
         set_attribute(name, *value)
@@ -155,8 +157,8 @@ module Ohai
         @data[name]
       end
 
-      def get_attribute(name)
-        @data[name]
+      def get_attribute(name, *keys)
+        safe_get_attribute(name, *keys)
       end
 
       def hint?(name)
@@ -183,6 +185,18 @@ module Ohai
 
       private
 
+      def safe_get_attribute(*keys)
+        keys.inject(@data) do |attrs, key|
+          unless attrs.nil? || attrs.is_a?(Array) || attrs.is_a?(Hash)
+            raise TypeError.new("Expected Hash but got #{attrs.class}.")
+          end
+          attrs[key]
+        end
+      rescue NoMethodError
+        # NoMethodError occurs when trying to access a key on nil
+        nil
+      end
+
       def Array18(*args)
         return nil if args.empty?
         return args.first if args.length == 1
diff --git a/spec/unit/dsl/plugin_spec.rb b/spec/unit/dsl/plugin_spec.rb
index 698e3f1b..4b225df5 100644
--- a/spec/unit/dsl/plugin_spec.rb
+++ b/spec/unit/dsl/plugin_spec.rb
@@ -21,11 +21,11 @@ require File.expand_path("../../../spec_helper.rb", __FILE__)
 
 shared_examples "Ohai::DSL::Plugin" do
   context "#initialize" do
-    it "should set has_run? to false" do
+    it "sets has_run? to false" do
       expect(plugin.has_run?).to be false
     end
 
-    it "should set the correct plugin version" do
+    it "sets the correct plugin version" do
       expect(plugin.version).to eql(version)
     end
   end
@@ -41,12 +41,12 @@ shared_examples "Ohai::DSL::Plugin" do
         allow(Ohai.config).to receive(:[]).with(:disabled_plugins).and_return([ ])
       end
 
-      it "should run the plugin" do
+      it "runs the plugin" do
         expect(plugin).to receive(:run_plugin)
         plugin.run
       end
 
-      it "should set has_run? to true" do
+      it "sets has_run? to true" do
         plugin.run
         expect(plugin.has_run?).to be true
       end
@@ -57,17 +57,17 @@ shared_examples "Ohai::DSL::Plugin" do
         allow(Ohai.config).to receive(:[]).with(:disabled_plugins).and_return([ :TestPlugin ])
       end
 
-      it "should not run the plugin" do
+      it "does not run the plugin" do
         expect(plugin).not_to receive(:run_plugin)
         plugin.run
       end
 
-      it "should log a message to debug" do
+      it "logs a message to debug" do
         expect(Ohai::Log).to receive(:debug).with(/Skipping disabled plugin TestPlugin/)
         plugin.run
       end
 
-      it "should set has_run? to true" do
+      it "sets has_run? to true" do
         plugin.run
         expect(plugin.has_run?).to be true
       end
@@ -75,37 +75,23 @@ shared_examples "Ohai::DSL::Plugin" do
   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 values" do
+    it "takes a missing method and store the method name as a key, with its arguments as values" do
       plugin.guns_n_roses("chinese democracy")
       expect(plugin.data["guns_n_roses"]).to eql("chinese democracy")
     end
 
-    it "should return the current value of the method name" do
+    it "returns the current value of the method name" do
       expect(plugin.guns_n_roses("chinese democracy")).to eql("chinese democracy")
     end
 
-    it "should allow you to get the value of a key by calling method_missing with no arguments" do
+    it "allows you to get the value of a key by calling method_missing with no arguments" do
       plugin.guns_n_roses("chinese democracy")
       expect(plugin.guns_n_roses).to eql("chinese democracy")
     end
   end
 
-  context "when checking attribute existence" do
-    before(:each) do
-      plugin.metallica("death magnetic")
-    end
-
-    it "should return true if an attribute exists with the given name" do
-      expect(plugin.attribute?("metallica")).to eql(true)
-    end
-
-    it "should return false if an attribute does not exist with the given name" do
-      expect(plugin.attribute?("alice in chains")).to eql(false)
-    end
-  end
-
   context "when setting attributes" do
-    it "should let you set an attribute" do
+    it "lets you set an attribute" do
       plugin.set_attribute(:tea, "is soothing")
       expect(plugin.data["tea"]).to eql("is soothing")
     end
@@ -116,57 +102,273 @@ shared_examples "Ohai::DSL::Plugin" do
       plugin.set_attribute(:tea, "is soothing")
     end
 
-    it "should let you get an attribute" do
+    it "lets you get an attribute" do
       expect(plugin.get_attribute("tea")).to eql("is soothing")
     end
   end
+
+  describe "get_attribute" do
+    it "requires at least one argument" do
+      expect { plugin.get_attribute }.to raise_error(ArgumentError)
+    end
+
+    describe "a top-level attribute" do
+      before(:each) do
+        plugin.set_attribute(:tea, "is soothing")
+      end
+
+      describe "as a string" do
+        it "returns nil when the attribute does not exist" do
+          expect(plugin.get_attribute("coffee")).to be nil
+        end
+
+        it "returns the attribute when it exists" do
+          expect(plugin.get_attribute("tea")).to eql("is soothing")
+        end
+      end
+
+      describe "as a symbol" do
+        it "returns false when the attribute does not exist" do
+          expect(plugin.get_attribute(:coffee)).to be nil
+        end
+
+        it "returns true when the attribute exists" do
+          expect(plugin.get_attribute(:tea)).to eql("is soothing")
+        end
+      end
+    end
+
+    describe "a nested attribute" do
+      before(:each) do
+        plugin.set_attribute(:the_monarch, { arch_rival: "dr_venture" })
+      end
+
+      describe "as a list" do
+        describe "of strings" do
+          it "returns true when the attribute exists" do
+            expect(plugin.get_attribute("the_monarch", "arch_rival")).
+              to eql("dr_venture")
+          end
+
+          describe "when the attribute does not exist" do
+            describe "and the subkey is missing" do
+              it "returns nil" do
+                expect(
+                  plugin.get_attribute("the_monarch", "henchmen")
+                ).to be nil
+              end
+            end
+
+            describe "and an intermediate key is missing" do
+              it "returns nil" do
+                expect(
+                  plugin.get_attribute("the_monarch", "henchmen",
+                                       "corky_knightrider")
+                ).to be nil
+              end
+            end
+
+            describe "and an intermediate key is not a hash" do
+              it "raises a TypeError" do
+                expect {
+                  plugin.get_attribute("the_monarch", "arch_rival",
+                                       "dr_venture", "since")
+                }.to raise_error(TypeError,
+                                 "Expected Hash but got String.")
+              end
+            end
+          end
+        end
+
+        describe "of symbols" do
+          it "returns true when the attribute exists" do
+            expect(plugin.get_attribute(:the_monarch, :arch_rival)).
+              to eql("dr_venture")
+          end
+
+          describe "when the attribute does not exist" do
+            describe "and the subkey is missing" do
+              it "returns nil" do
+                expect(plugin.get_attribute(:the_monarch, :henchmen)).to be nil
+              end
+            end
+
+            describe "and an intermediate key is missing" do
+              it "returns nil" do
+                expect(
+                  plugin.get_attribute(:the_monarch, :henchmen,
+                                       :corky_knightrider)
+                ).to be nil
+              end
+            end
+
+            describe "and an intermediate key is not a hash" do
+              it "raises a TypeError" do
+                expect {
+                  plugin.get_attribute(:the_monarch, :arch_rival,
+                                       :dr_venture, :since)
+                }.to raise_error(TypeError,
+                                 "Expected Hash but got String.")
+              end
+            end
+          end
+        end
+      end
+    end
+  end
+
+  describe "attribute?" do
+    it "requires at least one argument" do
+      expect { plugin.attribute? }.to raise_error(ArgumentError)
+    end
+
+    describe "a top-level attribute" do
+      describe "as a string" do
+        it "returns false when the attribute does not exist" do
+          expect(plugin.attribute?("alice in chains")).to eql(false)
+        end
+
+        it "returns true if an attribute exists with the given name" do
+          plugin.metallica("death magnetic")
+          expect(plugin.attribute?("metallica")).to eql(true)
+        end
+      end
+
+      describe "as a symbol" do
+        it "returns false when the attribute does not exist" do
+          expect(plugin.attribute?(:sparkle_dream)).to be false
+        end
+
+        it "returns true when the attribute exists" do
+          plugin.set_attribute("sparkle_dream", { version: 256 })
+          expect(plugin.attribute?(:sparkle_dream)).to be true
+        end
+      end
+    end
+
+    describe "a nested attribute" do
+      before(:each) do
+        plugin.set_attribute(:the_monarch, { arch_rival: "dr_venture" })
+      end
+
+      describe "as a list" do
+        describe "of strings" do
+          it "returns true when the attribute exists" do
+            expect(plugin.attribute?("the_monarch", "arch_rival")).to be true
+          end
+
+          describe "when the attribute does not exist" do
+            describe "and the subkey is missing" do
+              it "returns false" do
+                expect(
+                  plugin.attribute?("the_monarch", "henchmen")
+                ).to be false
+              end
+            end
+
+            describe "and an intermediate key is missing" do
+              it "returns false" do
+                expect(
+                  plugin.attribute?("the_monarch", "henchmen",
+                                    "corky_knightrider")
+                ).to be false
+              end
+            end
+
+            describe "and an intermediate key is not a hash" do
+              it "raises a TypeError" do
+                expect {
+                  plugin.attribute?("the_monarch", "arch_rival",
+                                    "dr_venture", "since")
+                }.to raise_error(TypeError,
+                                 "Expected Hash but got String.")
+              end
+            end
+          end
+        end
+
+        describe "of symbols" do
+          it "returns true when the attribute exists" do
+            expect(plugin.attribute?(:the_monarch, :arch_rival)).to be true
+          end
+
+          describe "when the attribute does not exist" do
+            describe "and the subkey is missing" do
+              it "returns false" do
+                expect(plugin.attribute?(:the_monarch, :henchmen)).to be false
+              end
+            end
+
+            describe "and an intermediate key is missing" do
+              it "returns false" do
+                expect(
+                  plugin.attribute?(:the_monarch, :henchmen,
+                                    :corky_knightrider)
+                ).to be false
+              end
+            end
+
+            describe "and an intermediate key is not a hash" do
+              it "raises a TypeError" do
+                expect {
+                  plugin.attribute?(:the_monarch, :arch_rival,
+                                    :dr_venture, :since)
+                }.to raise_error(TypeError,
+                                 "Expected Hash but got String.")
+              end
+            end
+          end
+        end
+      end
+    end
+  end
 end
 
 describe Ohai::DSL::Plugin::VersionVII do
-  it "should not modify the plugin name when the plugin is named correctly" do
+  it "does not modify the plugin name when the plugin is named correctly" do
     plugin = Ohai.plugin(:FunkyVALIDpluginName) {}.new({})
     expect(plugin.name).to eql(:FunkyVALIDpluginName)
   end
 
   describe "when the plugin is named incorrectly" do
     context "because the plugin name doesn't start with a capital letter" do
-      it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
+      it "raises an Ohai::Exceptions::InvalidPluginName exception" do
         expect { Ohai.plugin(:badName) {} }.to raise_error(Ohai::Exceptions::InvalidPluginName, /badName is not a valid plugin name/)
       end
     end
 
     context "because the plugin name contains an underscore" do
-      it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
+      it "raises an Ohai::Exceptions::InvalidPluginName exception" do
         expect { Ohai.plugin(:Bad_Name) {} }.to raise_error(Ohai::Exceptions::InvalidPluginName, /Bad_Name is not a valid plugin name/)
       end
     end
 
     context "because the plugin name isn't a symbol" do
-      it "should raise an Ohai::Exceptions::InvalidPluginName exception" do
+      it "raises an Ohai::Exceptions::InvalidPluginName exception" do
         expect { Ohai.plugin(1138) {} }.to raise_error(Ohai::Exceptions::InvalidPluginName, /1138 is not a valid plugin name/)
       end
     end
   end
 
   describe "#version" do
-    it "should save the plugin version as :version7" do
+    it "saves the plugin version as :version7" do
       plugin = Ohai.plugin(:Test) {}
       expect(plugin.version).to eql(:version7)
     end
   end
 
   describe "#provides" do
-    it "should collect a single attribute" do
+    it "collects a single attribute" do
       plugin = Ohai.plugin(:Test) { provides("one") }
       expect(plugin.provides_attrs).to eql(["one"])
     end
 
-    it "should collect a list of attributes" do
+    it "collects a list of attributes" do
       plugin = Ohai.plugin(:Test) { provides("one", "two", "three") }
       expect(plugin.provides_attrs).to eql(%w{one two three})
     end
 
-    it "should collect from multiple provides statements" do
+    it "collects from multiple provides statements" do
       plugin = Ohai.plugin(:Test) {
         provides("one")
         provides("two", "three")
@@ -175,13 +377,13 @@ describe Ohai::DSL::Plugin::VersionVII do
       expect(plugin.provides_attrs).to eql(%w{one two three four})
     end
 
-    it "should collect attributes across multiple plugin files" do
+    it "collects attributes across multiple plugin files" do
       plugin = Ohai.plugin(:Test) { provides("one") }
       plugin = Ohai.plugin(:Test) { provides("two", "three") }
       expect(plugin.provides_attrs).to eql(%w{one two three})
     end
 
-    it "should collect unique attributes" do
+    it "collects unique attributes" do
       plugin = Ohai.plugin(:Test) { provides("one") }
       plugin = Ohai.plugin(:Test) { provides("one", "two") }
       expect(plugin.provides_attrs).to eql(%w{one two})
@@ -189,17 +391,17 @@ describe Ohai::DSL::Plugin::VersionVII do
   end
 
   describe "#depends" do
-    it "should collect a single dependency" do
+    it "collects a single dependency" do
       plugin = Ohai.plugin(:Test) { depends("one") }
       expect(plugin.depends_attrs).to eql(["one"])
     end
 
-    it "should collect a list of dependencies" do
+    it "collects a list of dependencies" do
       plugin = Ohai.plugin(:Test) { depends("one", "two", "three") }
       expect(plugin.depends_attrs).to eql(%w{one two three})
     end
 
-    it "should collect from multiple depends statements" do
+    it "collects from multiple depends statements" do
       plugin = Ohai.plugin(:Test) {
         depends("one")
         depends("two", "three")
@@ -208,13 +410,13 @@ describe Ohai::DSL::Plugin::VersionVII do
       expect(plugin.depends_attrs).to eql(%w{one two three four})
     end
 
-    it "should collect dependencies across multiple plugin files" do
+    it "collects dependencies across multiple plugin files" do
       plugin = Ohai.plugin(:Test) { depends("one") }
       plugin = Ohai.plugin(:Test) { depends("two", "three") }
       expect(plugin.depends_attrs).to eql(%w{one two three})
     end
 
-    it "should collect unique attributes" do
+    it "collects unique attributes" do
       plugin = Ohai.plugin(:Test) { depends("one") }
       plugin = Ohai.plugin(:Test) { depends("one", "two") }
       expect(plugin.depends_attrs).to eql(%w{one two})
@@ -222,24 +424,24 @@ describe Ohai::DSL::Plugin::VersionVII do
   end
 
   describe "#collect_data" do
-    it "should save as :default if no platform is given" do
+    it "saves as :default if no platform is given" do
       plugin = Ohai.plugin(:Test) { collect_data {} }
       expect(plugin.data_collector).to have_key(:default)
     end
 
-    it "should save a single given platform" do
+    it "saves a single given platform" do
       plugin = Ohai.plugin(:Test) { collect_data(:ubuntu) {} }
       expect(plugin.data_collector).to have_key(:ubuntu)
     end
 
-    it "should save a list of platforms" do
+    it "saves a list of platforms" do
       plugin = Ohai.plugin(:Test) { collect_data(:freebsd, :netbsd, :openbsd) {} }
       [:freebsd, :netbsd, :openbsd].each do |platform|
         expect(plugin.data_collector).to have_key(platform)
       end
     end
 
-    it "should save multiple collect_data blocks" do
+    it "saves multiple collect_data blocks" do
       plugin = Ohai.plugin(:Test) {
         collect_data {}
         collect_data(:windows) {}
@@ -250,7 +452,7 @@ describe Ohai::DSL::Plugin::VersionVII do
       end
     end
 
-    it "should save platforms across multiple plugins" do
+    it "saves platforms across multiple plugins" do
       plugin = Ohai.plugin(:Test) { collect_data {} }
       plugin = Ohai.plugin(:Test) { collect_data(:aix, :sigar) {} }
       [:aix, :default, :sigar].each do |platform|
@@ -258,7 +460,7 @@ describe Ohai::DSL::Plugin::VersionVII do
       end
     end
 
-    it "should fail a platform has already been defined in the same plugin" do
+    it "fails a platform has already been defined in the same plugin" do
       expect {
         Ohai.plugin(:Test) {
           collect_data {}
@@ -267,7 +469,7 @@ describe Ohai::DSL::Plugin::VersionVII do
       }.to raise_error(Ohai::Exceptions::IllegalPluginDefinition, /collect_data already defined/)
     end
 
-    it "should fail if a platform has already been defined in another plugin file" do
+    it "fails if a platform has already been defined in another plugin file" do
       Ohai.plugin(:Test) { collect_data {} }
       expect {
         Ohai.plugin(:Test) {
@@ -278,7 +480,7 @@ describe Ohai::DSL::Plugin::VersionVII do
   end
 
   describe "#provides (deprecated)" do
-    it "should log a warning" do
+    it "logs a warning" do
       plugin = Ohai::DSL::Plugin::VersionVII.new(Mash.new)
       expect(Ohai::Log).to receive(:warn).with(/\[UNSUPPORTED OPERATION\]/)
       plugin.provides("attribute")
@@ -286,7 +488,7 @@ describe Ohai::DSL::Plugin::VersionVII do
   end
 
   describe "#require_plugin (deprecated)" do
-    it "should log a warning" do
+    it "logs a warning" do
       plugin = Ohai::DSL::Plugin::VersionVII.new(Mash.new)
       expect(Ohai::Log).to receive(:warn).with(/\[UNSUPPORTED OPERATION\]/)
       plugin.require_plugin("plugin")
@@ -375,27 +577,25 @@ end
 
 describe Ohai::DSL::Plugin::VersionVI do
   describe "#version" do
-    it "should save the plugin version as :version6" do
+    it "saves the plugin version as :version6" do
       plugin = Class.new(Ohai::DSL::Plugin::VersionVI) {}
       expect(plugin.version).to eql(:version6)
     end
   end
 
   describe "#provides" do
-    before(:each) do
-      @ohai = Ohai::System.new
-    end
+    let(:ohai) { Ohai::System.new }
 
-    it "should log a debug message when provides is used" do
+    it "logs a debug message when provides is used" do
       expect(Ohai::Log).to receive(:debug).with(/Skipping provides/)
-      plugin = Ohai::DSL::Plugin::VersionVI.new(@ohai, "/some/plugin/path.rb", "/some/plugin")
+      plugin = Ohai::DSL::Plugin::VersionVI.new(ohai, "/some/plugin/path.rb", "/some/plugin")
       plugin.provides("attribute")
     end
 
-    it "should not update the provides map for version 6 plugins." do
-      plugin = Ohai::DSL::Plugin::VersionVI.new(@ohai, "/some/plugin/path.rb", "/some/plugin")
+    it "does not update the provides map for version 6 plugins." do
+      plugin = Ohai::DSL::Plugin::VersionVI.new(ohai, "/some/plugin/path.rb", "/some/plugin")
       plugin.provides("attribute")
-      expect(@ohai.provides_map.map).to be_empty
+      expect(ohai.provides_map.map).to be_empty
     end
 
   end