3 files changed, 71 insertions, 49 deletions

diff --git a/lib/puppet/util/autoload.rb b/lib/puppet/util/autoload.rb
index 0c80f8b06..7c176779a 100644
--- a/lib/puppet/util/autoload.rb
+++ b/lib/puppet/util/autoload.rb
@@ -1,9 +1,11 @@
 require 'puppet/util/warnings'
+require 'puppet/util/cacher'
 
 # Autoload paths, either based on names or all at once.
 class Puppet::Util::Autoload
     include Puppet::Util
     include Puppet::Util::Warnings
+    include Puppet::Util::Cacher
 
     @autoloaders = {}
     @loaded = []
@@ -70,9 +72,9 @@ class Puppet::Util::Autoload
     def load(name)
         path = name.to_s + ".rb"
 
-        eachdir do |dir|
+        searchpath.each do |dir|
             file = File.join(dir, path)
-            next unless FileTest.exists?(file)
+            next unless FileTest.exist?(file)
             begin
                 Kernel.load file, @wrap
                 name = symbolize(name)
@@ -108,7 +110,7 @@ class Puppet::Util::Autoload
     # so that already-loaded files don't get reloaded unnecessarily.
     def loadall
         # Load every instance of everything we can find.
-        eachdir do |dir|
+        searchpath.each do |dir|
             Dir.glob("#{dir}/*.rb").each do |file|
                 name = File.basename(file).sub(".rb", '').intern
                 next if loaded?(name)
@@ -125,22 +127,25 @@ class Puppet::Util::Autoload
         end
     end
 
-    # Yield each subdir in turn.
-    def eachdir
-        searchpath.each do |dir|
-            subdir = File.join(dir, @path)
-            yield subdir if FileTest.directory?(subdir)
+    # The list of directories to search through for loadable plugins.
+    # We have to hard-code the ttl because this library is used by
+    # so many other classes it's hard to get the load-order such that
+    # the defaults load before this.
+    cached_attr(:searchpath, :ttl => 15) do
+        search_directories.collect { |d| File.join(d, @path) }.find_all { |d| FileTest.directory?(d) }
+    end
+
+    def module_directories
+        Puppet.settings.value(:modulepath, Puppet[:environment]).find_all do |dir|
+            FileTest.directory?(dir)
+        end.collect do |dir|
+            Dir.entries(dir)
+        end.flatten.collect { |d| [File.join(d, "plugins"), File.join(d, "lib")] }.flatten.find_all do |d|
+            FileTest.directory?(d)
         end
     end
 
-    # The list of directories to search through for loadable plugins.
-    def searchpath
-        # JJM: Search for optional lib directories in each module bundle.
-        module_lib_dirs = Puppet[:modulepath].split(":").collect do |d|
-            Dir.glob("%s/*/{plugins,lib}" % d).select do |f|
-                FileTest.directory?(f) 
-            end
-        end.flatten
-        [module_lib_dirs, Puppet[:libdir], $:].flatten
+    def search_directories
+        [module_directories, Puppet[:libdir], $:].flatten
     end
 end
diff --git a/spec/unit/util/autoload.rb b/spec/unit/util/autoload.rb
index ff717d6c5..53b90db69 100755
--- a/spec/unit/util/autoload.rb
+++ b/spec/unit/util/autoload.rb
@@ -11,20 +11,66 @@ describe Puppet::Util::Autoload do
         @autoload.stubs(:eachdir).yields "/my/dir"
     end
 
+    it "should use the Cacher module" do
+        Puppet::Util::Autoload.ancestors.should be_include(Puppet::Util::Cacher)
+    end
+
+    it "should use a ttl of 15 for the search path" do
+        Puppet::Util::Autoload.attr_ttl(:searchpath).should == 15
+    end
+
+    describe "when building the search path" do
+        it "should collect all of the plugins and lib directories that exist in the current environment's module path" do
+            Puppet.settings.expects(:value).with(:environment).returns "foo"
+            Puppet.settings.expects(:value).with(:modulepath, "foo").returns %w{/a /b /c}
+            Dir.expects(:entries).with("/a").returns %w{/a/one /a/two}
+            Dir.expects(:entries).with("/b").returns %w{/b/one /b/two}
+
+            FileTest.stubs(:directory?).returns false
+            FileTest.expects(:directory?).with("/a").returns true
+            FileTest.expects(:directory?).with("/b").returns true
+            %w{/a/one/plugins /a/two/lib /b/one/plugins /b/two/lib}.each do |d|
+                FileTest.expects(:directory?).with(d).returns true
+            end
+
+            @autoload.module_directories.should == %w{/a/one/plugins /a/two/lib /b/one/plugins /b/two/lib}
+        end
+
+        it "should include the module directories, the Puppet libdir, and all of the Ruby load directories" do
+            @autoload.expects(:module_directories).returns %w{/one /two}
+            @autoload.search_directories.should == ["/one", "/two", Puppet[:libdir], $:].flatten
+        end
+
+        it "should include in its search path all of the search directories that have a subdirectory matching the autoload path" do
+            @autoload = Puppet::Util::Autoload.new("foo", "loaddir")
+            @autoload.expects(:search_directories).returns %w{/one /two /three}
+            FileTest.expects(:directory?).with("/one/loaddir").returns true
+            FileTest.expects(:directory?).with("/two/loaddir").returns false
+            FileTest.expects(:directory?).with("/three/loaddir").returns true
+            @autoload.searchpath.should == ["/one/loaddir", "/three/loaddir"]
+        end
+    end
+
     describe "when loading a file" do
+        before do
+            @autoload.stubs(:searchpath).returns %w{/a}
+        end
+
         [RuntimeError, LoadError, SyntaxError].each do |error|
             it "should not die an if a #{error.to_s} exception is thrown" do
-                FileTest.stubs(:exists?).returns true
+                FileTest.stubs(:directory?).returns true
+                FileTest.stubs(:exist?).returns true
 
                 Kernel.expects(:load).raises error
 
-                lambda { @autoload.load("foo") }.should_not raise_error
+                @autoload.load("foo")
             end
         end
     end
 
     describe "when loading all files" do
         before do
+            @autoload.stubs(:searchpath).returns %w{/a}
             Dir.stubs(:glob).returns "file.rb"
         end
 
diff --git a/test/util/autoload.rb b/test/util/autoload.rb
index de503ab99..6aaf32995 100755
--- a/test/util/autoload.rb
+++ b/test/util/autoload.rb
@@ -93,16 +93,6 @@ TestAutoload.newthing(:#{name.to_s})
         end
     end
 
-    # Make sure that autoload dynamically modifies $: with the libdir as
-    # appropriate.
-    def test_searchpath
-        dir = Puppet[:libdir]
-
-        loader = Puppet::Util::Autoload.new(self, "testing")
-
-        assert(loader.send(:searchpath).include?(dir), "searchpath does not include the libdir")
-    end
-
     # This tests #1027, which was caused by using the unqualified
     # path for requires, which was initially done so that the kernel
     # would keep track of which files got loaded.
@@ -112,7 +102,7 @@ TestAutoload.newthing(:#{name.to_s})
 
         basedir = "/some/dir"
         dir = File.join(basedir, loadname)
-        loader.expects(:eachdir).yields(dir)
+        loader.expects(:searchpath).returns(dir)
 
         subname = "instance"
 
@@ -123,23 +113,4 @@ TestAutoload.newthing(:#{name.to_s})
         Kernel.expects(:require).with(file)
         loader.loadall
     end
-
-    def test_searchpath_includes_plugin_dirs
-        moddir = "/what/ever"
-        libdir = "/other/dir"
-        Puppet.settings.stubs(:value).with(:modulepath).returns(moddir)
-        Puppet.settings.stubs(:value).with(:libdir).returns(libdir)
-
-        loadname = "testing"
-        loader = Puppet::Util::Autoload.new(self.class, loadname)
-
-        # Currently, include both plugins and libs.
-        paths = %w{plugins lib}.inject({}) { |hash, d| hash[d] = File.join(moddir, "testing", d); FileTest.stubs(:directory?).with(hash[d]).returns(true); hash  }
-        Dir.expects(:glob).with("#{moddir}/*/{plugins,lib}").returns(paths.values)
-
-        searchpath = loader.searchpath
-        paths.each do |dir, path|
-            assert(searchpath.include?(path), "search path did not include path for %s" % dir)
-        end
-    end
 end