diff options
| -rw-r--r-- | lib/facter.rb | 62 | ||||
| -rw-r--r-- | lib/facter/util/collection.rb | 20 | ||||
| -rw-r--r-- | lib/facter/util/loader.rb | 62 | ||||
| -rwxr-xr-x | spec/integration/facter.rb | 1 | ||||
| -rwxr-xr-x | spec/unit/facter.rb | 26 | ||||
| -rwxr-xr-x | spec/unit/util/collection.rb | 33 | ||||
| -rwxr-xr-x | spec/unit/util/loader.rb | 41 |
7 files changed, 153 insertions, 92 deletions
diff --git a/lib/facter.rb b/lib/facter.rb index 0f00f7d..91588d3 100644 --- a/lib/facter.rb +++ b/lib/facter.rb @@ -77,11 +77,18 @@ module Facter end class << self - [:add, :each, :fact, :flush, :list, :to_hash, :value].each do |method| + [:add, :fact, :flush, :list, :value].each do |method| define_method(method) do |*args| collection.send(method, *args) end end + + [:list, :to_hash].each do |method| + define_method(method) do |*args| + collection.load_all + collection.send(method, *args) + end + end end @@ -91,6 +98,15 @@ module Facter collection.add(name, options, &block) end + def self.each + # Make sure all facts are loaded. + collection.load_all + + collection.each do |*args| + yield(*args) + end + end + class << self # Allow users to call fact names directly on the Facter class, # either retrieving the value or comparing it to an existing value. @@ -189,49 +205,7 @@ module Facter end end - locals = [] - - # Now find all our loadable facts - factdirs = [] # All the places to check for facts - - # See if we can find any other facts in the regular Ruby lib - # paths - $:.each do |dir| - fdir = File.join(dir, "facter") - if FileTest.exists?(fdir) and FileTest.directory?(fdir) - factdirs.push(fdir) - end - end - # Also check anything in 'FACTERLIB' - if ENV['FACTERLIB'] - ENV['FACTERLIB'].split(":").each do |fdir| - factdirs.push(fdir) - end - end - factdirs.each do |fdir| - Dir.glob("#{fdir}/*.rb").each do |file| - # Load here, rather than require, because otherwise - # the facts won't get reloaded if someone calls - # "loadfacts". Really only important in testing, but, - # well, it's important in testing. - begin - load file - rescue => detail - warn "Could not load %s: %s" % - [file, detail] - end - end - end - - - # Now try to get facts from the environment - ENV.each do |name, value| - if name =~ /^facter_?(\w+)$/i - Facter.add($1) do - setcode { value } - end - end - end + collection.load_all end Facter.loadfacts diff --git a/lib/facter/util/collection.rb b/lib/facter/util/collection.rb index f76b578..267424a 100644 --- a/lib/facter/util/collection.rb +++ b/lib/facter/util/collection.rb @@ -1,5 +1,6 @@ require 'facter' require 'facter/util/fact' +require 'facter/util/loader' # Manage which facts exist and how we access them. Largely just a wrapper # around a hash of facts. @@ -39,7 +40,11 @@ class Facter::Util::Collection # Return a fact by name. def fact(name) - @facts[canonize(name)] + name = canonize(name) + + loader.load(name) unless @facts[name] + + return @facts[name] end # Flush all cached values. @@ -56,6 +61,19 @@ class Facter::Util::Collection return @facts.keys end + # Load all known facts. + def load_all + loader.load_all + end + + # The thing that loads facts if we don't have them. + def loader + unless defined?(@loader) + @loader = Facter::Util::Loader.new + end + @loader + end + # Return a hash of all of our facts. def to_hash @facts.inject({}) do |h, ary| diff --git a/lib/facter/util/loader.rb b/lib/facter/util/loader.rb index 7031172..52b5f07 100644 --- a/lib/facter/util/loader.rb +++ b/lib/facter/util/loader.rb @@ -13,8 +13,7 @@ class Facter::Util::Loader # Load individual files file = File.join(dir, filename) - # We have to specify Kernel.load, because we have a load method. - Kernel.load(file) if FileTest.exist?(file) + load_file(file) if FileTest.exist?(file) # And load any directories matching the name factdir = File.join(dir, shortname) @@ -24,18 +23,24 @@ class Facter::Util::Loader # Load all facts from all directories. def load_all + return if defined?(@loaded_all) + load_env search_path.each do |dir| + next unless FileTest.directory?(dir) + Dir.entries(dir).each do |file| path = File.join(dir, file) if File.directory?(path) load_dir(path) elsif file =~ /\.rb$/ - Kernel.load(File.join(dir, file)) + load_file(File.join(dir, file)) end end end + + @loaded_all = true end # The list of directories we're going to search through for facts. @@ -53,56 +58,21 @@ class Facter::Util::Loader result end - def old_stuff - # See if we can find any other facts in the regular Ruby lib - # paths - $:.each do |dir| - fdir = File.join(dir, "facter") - if FileTest.exists?(fdir) and FileTest.directory?(fdir) - factdirs.push(fdir) - end - end - # Also check anything in 'FACTERLIB' - if ENV['FACTERLIB'] - ENV['FACTERLIB'].split(":").each do |fdir| - factdirs.push(fdir) - end - end - factdirs.each do |fdir| - Dir.glob("#{fdir}/*.rb").each do |file| - # Load here, rather than require, because otherwise - # the facts won't get reloaded if someone calls - # "loadfacts". Really only important in testing, but, - # well, it's important in testing. - begin - load file - rescue => detail - warn "Could not load %s: %s" % - [file, detail] - end - end - end - - - # Now try to get facts from the environment - ENV.each do |name, value| - if name =~ /^facter_?(\w+)$/i - Facter.add($1) do - setcode { value } - end - end - end - end - private def load_dir(dir) - return if dir =~ /\/util$/ + return if dir =~ /\/\.+$/ or dir =~ /\/util$/ or dir =~ /\/lib$/ + Dir.entries(dir).find_all { |f| f =~ /\.rb$/ }.each do |file| - Kernel.load(File.join(dir, file)) + load_file(File.join(dir, file)) end end + def load_file(file) + # We have to specify Kernel.load, because we have a load method. + Kernel.load(file) + end + # Load facts from the environment. If no name is provided, # all will be loaded. def load_env(fact = nil) diff --git a/spec/integration/facter.rb b/spec/integration/facter.rb index e1f2025..79a1f0f 100755 --- a/spec/integration/facter.rb +++ b/spec/integration/facter.rb @@ -5,7 +5,6 @@ require File.dirname(__FILE__) + '/../spec_helper' describe Facter do before do Facter.reset - Facter.loadfacts end after do diff --git a/spec/unit/facter.rb b/spec/unit/facter.rb index 53796ad..59d10d4 100755 --- a/spec/unit/facter.rb +++ b/spec/unit/facter.rb @@ -31,11 +31,23 @@ describe Facter do Facter.list end + it "should load all facts when listing" do + Facter.collection.expects(:load_all) + Facter.collection.stubs(:list) + Facter.list + end + it "should delegate the :to_hash method to the collection" do Facter.collection.expects(:to_hash) Facter.to_hash end + it "should load all facts when calling :to_hash" do + Facter.collection.expects(:load_all) + Facter.collection.stubs(:to_hash) + Facter.to_hash + end + it "should delegate the :value method to the collection" do Facter.collection.expects(:value) Facter.value @@ -46,6 +58,20 @@ describe Facter do Facter.each end + it "should load all facts when calling :each" do + Facter.collection.expects(:load_all) + Facter.collection.stubs(:each) + Facter.each + end + + it "should yield to the block when using :each" do + Facter.collection.stubs(:load_all) + Facter.collection.stubs(:each).yields "foo" + result = [] + Facter.each { |f| result << f } + result.should == %w{foo} + end + describe "when provided code as a string" do it "should execute the code in the shell" do Facter.add("shell_testing") do diff --git a/spec/unit/util/collection.rb b/spec/unit/util/collection.rb index e88de06..9ea2e9d 100755 --- a/spec/unit/util/collection.rb +++ b/spec/unit/util/collection.rb @@ -9,6 +9,29 @@ describe Facter::Util::Collection do Facter::Util::Collection.new.should respond_to(:add) end + it "should have a method for returning a loader" do + Facter::Util::Collection.new.should respond_to(:loader) + end + + it "should use an instance of the Loader class as its loader" do + Facter::Util::Collection.new.loader.should be_instance_of(Facter::Util::Loader) + end + + it "should cache its loader" do + coll = Facter::Util::Collection.new + coll.loader.should equal(coll.loader) + end + + it "should have a method for loading all facts" do + Facter::Util::Collection.new.should respond_to(:load_all) + end + + it "should delegate its load_all method to its loader" do + coll = Facter::Util::Collection.new + coll.loader.expects(:load_all) + coll.load_all + end + describe "when adding facts" do before do @coll = Facter::Util::Collection.new @@ -55,6 +78,16 @@ describe Facter::Util::Collection do it "should treat strings and symbols equivalently" do @coll.fact(:yayness).should equal(@fact) end + + it "should use its loader to try to load the fact if no fact can be found" do + @coll.loader.expects(:load).with(:testing) + @coll.fact("testing") + end + + it "should return nil if it cannot find or load the fact" do + @coll.loader.expects(:load).with(:testing) + @coll.fact("testing").should be_nil + end end it "should have a method for returning a fact's value" do diff --git a/spec/unit/util/loader.rb b/spec/unit/util/loader.rb index e6a7f17..690ec03 100755 --- a/spec/unit/util/loader.rb +++ b/spec/unit/util/loader.rb @@ -149,6 +149,18 @@ describe Facter::Util::Loader do before do @loader = Facter::Util::Loader.new @loader.stubs(:search_path).returns [] + + FileTest.stubs(:directory?).returns true + end + + it "should skip directories that do not exist" do + @loader.expects(:search_path).returns %w{/one/dir} + + FileTest.expects(:directory?).with("/one/dir").returns false + + Dir.expects(:entries).with("/one/dir").never + + @loader.load_all end it "should load all files in all search paths" do @@ -190,6 +202,29 @@ describe Facter::Util::Loader do @loader.load_all end + it "should not load files in a lib subdirectory" do + @loader.expects(:search_path).returns %w{/one/dir} + + Dir.expects(:entries).with("/one/dir").returns %w{lib} + + File.expects(:directory?).with("/one/dir/lib").returns true + + Dir.expects(:entries).with("/one/dir/lib").never + + @loader.load_all + end + + it "should not load files in '.' or '..'" do + @loader.expects(:search_path).returns %w{/one/dir} + + Dir.expects(:entries).with("/one/dir").returns %w{. ..} + + File.expects(:entries).with("/one/dir/.").never + File.expects(:entries).with("/one/dir/..").never + + @loader.load_all + end + it "should load all facts from the environment" do Facter.expects(:add).with('one') Facter.expects(:add).with('two') @@ -198,5 +233,11 @@ describe Facter::Util::Loader do @loader.load_all end end + + it "should only load all facts one time" do + @loader.expects(:load_env).once + @loader.load_all + @loader.load_all + end end end |