Adding modulepath caching to the Autoloader

There's more caching to add, but this simplifies
the interface to the list of paths and then caches
that list so we aren't constantly searching the
filesystem.

Signed-off-by: Luke Kanies <luke@madstop.com>

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

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
 

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