Splitting the instance code into a Fact class.

This allows the Facter class to become a module, and it
is now much simpler.

7 files changed, 260 insertions, 231 deletions

diff --git a/lib/facter.rb b/lib/facter.rb
index 675b95d..ebc4299 100644
--- a/lib/facter.rb
+++ b/lib/facter.rb
@@ -17,8 +17,8 @@
 # 
 #--
 
-class Facter
-    require 'facter/resolution'
+module Facter
+    require 'facter/fact'
 
     include Comparable
     include Enumerable
@@ -50,8 +50,6 @@ class Facter
     RESET = ""
     @@debug = 0
 
-    attr_accessor :name, :searching, :ldapname
-
 	# module methods
 
     # Return the version of the library.
@@ -77,11 +75,10 @@ class Facter
 
     # Add a resolution mechanism for a named fact.  This does not distinguish
     # between adding a new fact and adding a new way to resolve a fact.
-    def self.add(name, &block)
-        fact = nil
-
+    def self.add(name, options = {}, &block)
         unless fact = @@facts[name]
-            fact = Facter.new(name)
+            fact = Facter::Fact.new(name, options)
+            @@facts[name] = fact
         end
 
         unless block
@@ -98,11 +95,9 @@ class Facter
         # Iterate across all of the facts.
         def each
             @@facts.each { |name,fact|
-                if fact.suitable?
-                    value = fact.value
-                    unless value.nil?
-                        yield name.to_s, fact.value
-                    end
+                value = fact.value
+                if ! value.nil?
+                    yield name.to_s, fact.value
                 end
             }
         end
@@ -132,7 +127,7 @@ class Facter
                 end
             else
                 # Else, fail like a normal missing method.
-                return super
+                raise NoMethodError, "Could not find fact '%s'" % name
             end
         end
     end
@@ -185,14 +180,12 @@ class Facter
     end
 
     # Return a hash of all of our facts.
-    def self.to_hash(*tags)
+    def self.to_hash
         @@facts.inject({}) do |h, ary|
-            if ary[1].suitable? and (tags.empty? or ary[1].tagged?(*tags))
-                value = ary[1].value
-                if value
-                    # For backwards compatibility, convert the fact name to a string.
-                    h[ary[0].to_s] = value
-                end
+            value = ary[1].value
+            if ! value.nil?
+                # For backwards compatibility, convert the fact name to a string.
+                h[ary[0].to_s] = value
             end
             h
         end
@@ -206,179 +199,6 @@ class Facter
         end
     end
 
-    # Compare one value to another.
-    def <=>(other)
-        return self.value <=> other
-    end
-
-    # Are we the same?  Used for case statements.
-    def ===(value)
-        self.value == value
-    end
-
-    # Create a new fact, with no resolution mechanisms.
-    def initialize(name)
-        @name = name.to_s.downcase.intern
-        if @@facts.include?(@name)
-            raise ArgumentError, "A fact named %s already exists" % @name
-        else
-            @@facts[@name] = self
-        end
-
-        @resolves = []
-        @tags = []
-        @searching = false
-
-        @value = nil
-
-        @ldapname = name.to_s
-    end
-
-    # Add a new resolution mechanism.  This requires a block, which will then
-    # be evaluated in the context of the new mechanism.
-    def add(&block)
-        unless block_given?
-            raise ArgumentError, "You must pass a block to Fact<instance>.add"
-        end
-
-        resolve = Resolution.new(@name)
-
-        resolve.fact = self
-
-        resolve.instance_eval(&block)
-
-        # skip resolves that will never be suitable for us
-        unless resolve.suitable?
-            return
-        end
-
-        # insert resolves in order of number of confinements
-        inserted = false
-        @resolves.each_with_index { |r,index|
-            if resolve.length > r.length
-                @resolves.insert(index,resolve)
-                inserted = true
-                break
-            end
-        }
-
-        unless inserted
-            @resolves.push resolve
-        end
-    end
-
-    # Return a count of resolution mechanisms available.
-    def count
-        return @resolves.length
-    end
-
-    # Iterate across all of the fact resolution mechanisms and yield each in
-    # turn.  These are inserted in order of most confinements.
-    def each
-        @resolves.each { |r| yield r }
-    end
-
-    # Flush any cached values.
-    def flush
-        @value = nil
-        @suitable = nil
-    end
-
-    # Is this fact suitable for finding answers on this host?  This is used
-    # to throw away any initially unsuitable mechanisms.
-    def suitable?
-        if @resolves.length == 0
-            return false
-        end
-
-        unless defined? @suitable or (defined? @suitable and @suitable.nil?)
-            @suitable = false
-            @resolves.each { |resolve|
-                if resolve.suitable?
-                    @suitable = true
-                    break
-                end
-            }
-        end
-
-        return @suitable
-    end
-
-    # Add one ore more tags
-    def tag(*tags)
-        tags.each do |t|
-            t = t.to_s.downcase.intern
-            @tags << t unless @tags.include?(t)
-        end
-    end
-
-    # Is our fact tagged with all of the specified tags?
-    def tagged?(*tags)
-        tags.each do |t|
-            unless @tags.include? t.to_s.downcase.intern
-                return false
-            end
-        end
-
-        return true
-    end
-
-    def tags
-        @tags.dup
-    end
-
-    # Return the value for a given fact.  Searches through all of the mechanisms
-    # and returns either the first value or nil.
-    def value
-        unless @value
-            # make sure we don't get stuck in recursive dependency loops
-            if @searching
-                Facter.debug "Caught recursion on %s" % @name
-                
-                # return a cached value if we've got it
-                if @value
-                    return @value
-                else
-                    return nil
-                end
-            end
-            @value = nil
-            foundsuits = false
-
-            if @resolves.length == 0
-                Facter.debug "No resolves for %s" % @name
-                return nil
-            end
-
-            @searching = true
-            @resolves.each { |resolve|
-                #Facter.debug "Searching resolves for %s" % @name
-                if resolve.suitable?
-                    @value = resolve.value
-                    foundsuits = true
-                end
-                unless @value.nil? or @value == ""
-                    break
-                end
-            }
-            @searching = false
-
-            unless foundsuits
-                Facter.debug "Found no suitable resolves of %s for %s" %
-                    [@resolves.length,@name]
-            end
-        end
-
-        if @value.nil?
-            # nothing
-            Facter.debug("value for %s is still nil" % @name)
-            return nil
-        else
-            return @value
-        end
-    end
-
-
     # Load all of the default facts
     def self.loadfacts
         Facter.add(:facterversion) do
@@ -456,5 +276,3 @@ class Facter
 
     Facter.loadfacts
 end
-
-# $Id$
diff --git a/lib/facter/fact.rb b/lib/facter/fact.rb
new file mode 100644
index 0000000..9514cd0
--- /dev/null
+++ b/lib/facter/fact.rb
@@ -0,0 +1,102 @@
+require 'facter'
+require 'facter/resolution'
+
+class Facter::Fact
+    attr_accessor :name, :searching, :ldapname
+
+    # Create a new fact, with no resolution mechanisms.
+    def initialize(name, options = {})
+        @name = name.to_s.downcase.intern
+
+        # LAK:NOTE: This is slow for many options, but generally we won't have any and at
+        # worst we'll have one.  If we add more, this should be made more efficient.
+        options.each do |name, value|
+            case name
+            when :ldapname: self.ldapname = value
+            else
+                raise ArgumentError, "Invalid fact option '%s'" % name
+            end
+        end
+
+        @ldapname ||= @name.to_s
+
+        @resolves = []
+        @searching = false
+
+        @value = nil
+    end
+
+    # Add a new resolution mechanism.  This requires a block, which will then
+    # be evaluated in the context of the new mechanism.
+    def add(&block)
+        raise ArgumentError, "You must pass a block to Fact<instance>.add" unless block_given?
+
+        resolve = Facter::Resolution.new(@name)
+
+        resolve.instance_eval(&block)
+
+        @resolves << resolve
+
+        # Immediately sort the resolutions, so that we always have
+        # a sorted list for looking up values.
+        #  We always want to look them up in the order of number of
+        # confines, so the most restricted resolution always wins.
+        @resolves.sort! { |a, b| b.length <=> a.length }
+
+        return resolve
+    end
+
+    # Flush any cached values.
+    def flush
+        @value = nil
+        @suitable = nil
+    end
+
+    # Return the value for a given fact.  Searches through all of the mechanisms
+    # and returns either the first value or nil.
+    def value
+        unless @value
+            # make sure we don't get stuck in recursive dependency loops
+            if @searching
+                Facter.debug "Caught recursion on %s" % @name
+                
+                # return a cached value if we've got it
+                if @value
+                    return @value
+                else
+                    return nil
+                end
+            end
+            @value = nil
+
+            if @resolves.length == 0
+                Facter.debug "No resolves for %s" % @name
+                return nil
+            end
+
+            @searching = true
+            foundsuits = false
+            @value = @resolves.inject(nil) { |result, resolve|
+                next unless resolve.suitable?
+                foundsuits = true
+
+                tmp = resolve.value
+
+                break tmp unless tmp.nil? or tmp == ""
+            }
+            @searching = false
+
+            unless foundsuits
+                Facter.debug "Found no suitable resolves of %s for %s" % [@resolves.length, @name]
+            end
+        end
+
+        if @value.nil?
+            # nothing
+            Facter.debug("value for %s is still nil" % @name)
+            return nil
+        else
+            return @value
+        end
+    end
+end
diff --git a/lib/facter/networking.rb b/lib/facter/networking.rb
index cde2c46..bd804cf 100644
--- a/lib/facter/networking.rb
+++ b/lib/facter/networking.rb
@@ -77,8 +77,7 @@
                 end
             end
         end
-        Facter.add(:hostname) do
-            setldapname "cn"
+        Facter.add(:hostname, :ldapname => "cn") do
             setcode do
                 hostname = nil
                 name = Facter::Resolution.exec('hostname') or nil
@@ -109,8 +108,7 @@
             end
         end
 
-        Facter.add(:ipaddress) do
-            setldapname "iphostnumber"
+        Facter.add(:ipaddress, :ldapname => "iphostnumber") do
             setcode do
                 require 'resolv'
 
diff --git a/lib/facter/resolution.rb b/lib/facter/resolution.rb
index 7fc1e24..4df55b7 100644
--- a/lib/facter/resolution.rb
+++ b/lib/facter/resolution.rb
@@ -6,7 +6,7 @@
 require 'facter/confine'
 
 class Facter::Resolution
-    attr_accessor :interpreter, :code, :name, :fact
+    attr_accessor :interpreter, :code, :name
 
     def self.have_which
         if @have_which.nil?
@@ -80,12 +80,6 @@ class Facter::Resolution
         end
     end
 
-    # Set the name by which this parameter is known in LDAP.  The default
-    # is just the fact name.
-    def setldapname(name)
-        fact.ldapname = name.to_s
-    end
-
     # Is this resolution mechanism suitable on the system in question?
     def suitable?
         unless defined? @suitable
diff --git a/spec/unit/fact.rb b/spec/unit/fact.rb
new file mode 100755
index 0000000..3772d0f
--- /dev/null
+++ b/spec/unit/fact.rb
@@ -0,0 +1,129 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../spec_helper'
+
+require 'facter/fact'
+
+describe Facter::Fact do
+    it "should require a name" do
+        lambda { Facter::Fact.new }.should raise_error(ArgumentError)
+    end
+
+    it "should always downcase the name and convert it to a symbol" do
+        Facter::Fact.new("YayNess").name.should == :yayness
+    end
+
+    it "should default to its name converted to a string as its ldapname" do
+        Facter::Fact.new("YayNess").ldapname.should == "yayness"
+    end
+
+    it "should allow specifying the ldap name at initialization" do
+        Facter::Fact.new("YayNess", :ldapname => "fooness").ldapname.should == "fooness"
+    end
+
+    it "should fail if an unknown option is provided" do
+        lambda { Facter::Fact.new('yay', :foo => :bar) }.should raise_error(ArgumentError)
+    end
+
+    it "should have a method for adding resolution mechanisms" do
+        Facter::Fact.new("yay").should respond_to(:add)
+    end
+
+    describe "when adding resolution mechanisms" do
+        before do
+            @fact = Facter::Fact.new("yay")
+
+            @resolution = mock 'resolution'
+            @resolution.stub_everything
+
+        end
+
+        it "should fail if no block is given" do
+            lambda { @fact.add }.should raise_error(ArgumentError)
+        end
+
+        it "should create a new resolution instance" do
+            Facter::Resolution.expects(:new).returns @resolution
+
+            @fact.add { }
+        end
+
+        it "should instance_eval the passed block on the new resolution" do
+            @resolution.expects(:instance_eval)
+
+            Facter::Resolution.stubs(:new).returns @resolution
+
+            @fact.add { }
+        end
+
+        it "should re-sort the resolutions by length, so the most restricted resolutions are first" do
+            r1 = stub 'r1', :length => 1
+            r2 = stub 'r2', :length => 2
+            r3 = stub 'r3', :length => 0
+            Facter::Resolution.expects(:new).times(3).returns(r1).returns(r2).returns(r3)
+            @fact.add { }
+            @fact.add { }
+            @fact.add { }
+
+            @fact.instance_variable_get("@resolves").should == [r2, r1, r3]
+        end
+    end
+
+    it "should be able to return a value" do
+        Facter::Fact.new("yay").should respond_to(:value)
+    end
+
+    describe "when returning a value" do
+        before do
+            @fact = Facter::Fact.new("yay")
+        end
+
+        it "should return nil if there are no resolutions" do
+            Facter::Fact.new("yay").value.should be_nil
+        end
+
+        it "should return the first value returned by a resolution" do
+            r1 = stub 'r1', :length => 2, :value => nil, :suitable? => true
+            r2 = stub 'r2', :length => 1, :value => "yay", :suitable? => true
+            r3 = stub 'r3', :length => 0, :value => "foo", :suitable? => true
+            Facter::Resolution.expects(:new).times(3).returns(r1).returns(r2).returns(r3)
+            @fact.add { }
+            @fact.add { }
+            @fact.add { }
+
+            @fact.value.should == "yay"
+        end
+
+        it "should short-cut returning the value once one is found" do
+            r1 = stub 'r1', :length => 2, :value => "foo", :suitable? => true
+            r2 = stub 'r2', :length => 1, :suitable? => true # would fail if 'value' were asked for
+            Facter::Resolution.expects(:new).times(2).returns(r1).returns(r2)
+            @fact.add { }
+            @fact.add { }
+
+            @fact.value
+        end
+
+        it "should skip unsuitable resolutions" do
+            r1 = stub 'r1', :length => 2, :suitable? => false # would fail if 'value' were asked for'
+            r2 = stub 'r2', :length => 1, :value => "yay", :suitable? => true
+            Facter::Resolution.expects(:new).times(2).returns(r1).returns(r2)
+            @fact.add { }
+            @fact.add { }
+
+            @fact.value.should == "yay"
+        end
+
+        it "should return nil if the value is the empty string" do
+            r1 = stub 'r1', :suitable? => true, :value => ""
+            Facter::Resolution.expects(:new).returns r1
+            @fact.add { }
+
+            @fact.value.should be_nil
+        end
+    end
+
+    it "should have a method for flushing the cached fact" do
+        Facter::Fact.new(:foo).should respond_to(:flush)
+    end
+end
diff --git a/spec/unit/facter.rb b/spec/unit/facter.rb
index 8ca160c..e794281 100755
--- a/spec/unit/facter.rb
+++ b/spec/unit/facter.rb
@@ -47,6 +47,18 @@ describe Facter do
         end
     end
 
+    describe Facter[:hostname] do
+        it "should have its ldapname set to 'cn'" do
+            Facter[:hostname].ldapname.should == "cn"
+        end
+    end
+
+    describe Facter[:ipaddress] do
+        it "should have its ldapname set to 'iphostnumber'" do
+            Facter[:ipaddress].ldapname.should == "iphostnumber"
+        end
+    end
+
     def test_onetrueconfine
         assert_nothing_raised {
             Facter.add("required") {
diff --git a/spec/unit/resolution.rb b/spec/unit/resolution.rb
index d4dc7e1..a54b872 100755
--- a/spec/unit/resolution.rb
+++ b/spec/unit/resolution.rb
@@ -155,30 +155,6 @@ describe Facter::Resolution do
         end
     end
 
-    it "should have a method for setting its ldap name" do
-        Facter::Resolution.new("yay").should respond_to(:setldapname)
-    end
-
-    it "should set the ldap name on the associated fact" do
-        @resolve = Facter::Resolution.new("yay")
-        fact = mock 'fact'
-        @resolve.stubs(:fact).returns fact
-
-        fact.expects(:ldapname=).with "ldapness"
-
-        @resolve.setldapname "ldapness"
-    end
-
-    it "should always convert the ldap name to a string" do
-        @resolve = Facter::Resolution.new("yay")
-        fact = mock 'fact'
-        @resolve.stubs(:fact).returns fact
-
-        fact.expects(:ldapname=).with "ldapness"
-
-        @resolve.setldapname :ldapness
-    end
-
     it "should have a class method for executing code" do
         Facter::Resolution.should respond_to(:exec)
     end