10 files changed, 251 insertions, 123 deletions

diff --git a/lib/puppet/file_serving/mount/plugins.rb b/lib/puppet/file_serving/mount/plugins.rb
index 487bd043b..c7a3ee6ff 100644
--- a/lib/puppet/file_serving/mount/plugins.rb
+++ b/lib/puppet/file_serving/mount/plugins.rb
@@ -16,7 +16,7 @@ class Puppet::FileServing::Mount::Plugins < Puppet::FileServing::Mount
     def search(relative_path, options = {})
         # We currently only support one kind of search on plugins - return
         # them all.
-        paths = environment(options[:node]).modules.find_all { |mod|  mod.plugins? }.collect { |mod| mod.plugins }
+        paths = environment(options[:node]).modules.find_all { |mod|  mod.plugins? }.collect { |mod| mod.plugin_directory }
         return nil if paths.empty?
         return paths
     end
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index e4bdd16e6..30c2512f3 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -25,7 +25,7 @@ class Puppet::Module
 
                 yielded[name] = true
 
-                yield Puppet::Module.new(name, module_path)
+                yield Puppet::Module.new(name)
             end
         end
     end
@@ -41,49 +41,91 @@ class Puppet::Module
     # absolute, or if there is no module whose name is the first component
     # of +path+, return +nil+
     def self.find(modname, environment = nil)
+        return nil unless modname
         Puppet::Node::Environment.new(environment).module(modname)
     end
 
-    attr_reader :name, :path
-    def initialize(name, path)
+    attr_reader :name, :environment
+    def initialize(name, environment = nil)
         @name = name
-        @path = path
+        if environment.is_a?(Puppet::Node::Environment)
+            @environment = environment
+        else
+            @environment = Puppet::Node::Environment.new(environment)
+        end
     end
 
     FILETYPES.each do |type|
-        # Create a method for returning the full path to a given
-        # file type's directory.
-        define_method(type.to_s) do
-            File.join(path, type.to_s)
-        end
-
-        # Create a boolean method for testing whether our module has
-        # files of a given type.
-        define_method(type.to_s + "?") do
-            FileTest.exist?(send(type))
+        # A boolean method to let external callers determine if
+        # we have files of a given type.
+        define_method(type +'?') do
+            return false unless path
+            return false unless FileTest.exist?(subpath(type))
+            return true
         end
 
-        # Finally, a method for returning an individual file
+        # A method for returning a given file of a given type.
+        # e.g., file = mod.manifest("my/manifest.pp")
+        #
+        # If the file name is nil, then the base directory for the
+        # file type is passed; this is used for fileserving.
         define_method(type.to_s.sub(/s$/, '')) do |file|
+            return nil unless path
+
+            # If 'file' is nil then they're asking for the base path.
+            # This is used for things like fileserving.
             if file
-                path = File.join(send(type), file)
+                full_path = File.join(subpath(type), file)
             else
-                path = send(type)
+                full_path = subpath(type)
             end
-            return nil unless FileTest.exist?(path)
-            return path
+
+            return nil unless FileTest.exist?(full_path)
+            return full_path
         end
     end
 
+    def exist?
+        ! path.nil?
+    end
+
+    # Find the first 'files' directory.  This is used by the XMLRPC fileserver.
+    def file_directory
+        subpath("files")
+    end
+
     # Return the list of manifests matching the given glob pattern,
     # defaulting to 'init.pp' for empty modules.
     def match_manifests(rest)
+        return find_init_manifest unless rest # Use init.pp
+
         rest ||= "init.pp"
         p = File::join(path, MANIFESTS, rest)
-        files = Dir.glob(p).reject { |f| FileTest.directory?(f) }
-        if files.size == 0
-            files = Dir.glob(p + ".pp")
+        result = Dir.glob(p).reject { |f| FileTest.directory?(f) }
+        if result.size == 0 and rest !~ /\.pp$/
+            result = Dir.glob(p + ".pp")
         end
-        return files
+        result.flatten.compact
+    end
+
+    # Find this module in the modulepath.
+    def path
+        self.class.modulepath.collect { |path| File.join(path, name) }.find { |d| FileTest.exist?(d) }
+    end
+
+    # Find all plugin directories.  This is used by the Plugins fileserving mount.
+    def plugin_directory
+        subpath("plugins")
+    end
+
+    private
+
+    def find_init_manifest
+        return [] unless file = manifest("init.pp")
+        return [file]
+    end
+
+    def subpath(type)
+        File.join(path, type)
     end
 end
diff --git a/lib/puppet/network/handler/fileserver.rb b/lib/puppet/network/handler/fileserver.rb
index aad3b98be..50e2614aa 100755
--- a/lib/puppet/network/handler/fileserver.rb
+++ b/lib/puppet/network/handler/fileserver.rb
@@ -254,8 +254,8 @@ class Puppet::Network::Handler
             end
 
             # And use the environment to look up the module.
-            if mod = Puppet::Node::Environment.new(env).module(module_name)
-                return @mounts[MODULES].copy(mod.name, mod.files)
+            if mod = Puppet::Node::Environment.new(env).module(module_name) and mod.files?
+                return @mounts[MODULES].copy(mod.name, mod.file_directory)
             else
                 return nil
             end
@@ -744,7 +744,7 @@ class Puppet::Network::Handler
 
             private
             def valid_modules(client)
-                Puppet::Node::Environment.new.modules.find_all { |mod| mod.plugins? }
+                Puppet::Node::Environment.new.modules.find_all { |mod| mod.exist? }
             end
             
             def add_to_filetree(f, filetree)
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 445439aa3..3d13af1f8 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -30,11 +30,9 @@ class Puppet::Node::Environment
     end
 
     def module(name)
-        Puppet::Module.each_module(modulepath) do |mod|
-            return mod if mod.name == name
-        end
-
-        return nil
+        mod = Puppet::Module.new(name, self)
+        return nil unless mod.exist?
+        return mod
     end
 
     def modulepath
diff --git a/lib/puppet/parser/files.rb b/lib/puppet/parser/files.rb
index 2cd163f97..ca4fb4f10 100644
--- a/lib/puppet/parser/files.rb
+++ b/lib/puppet/parser/files.rb
@@ -16,16 +16,16 @@ module Puppet::Parser::Files
     def find_manifests(start, options = {})
         cwd = options[:cwd] || Dir.getwd
         module_name, pattern = split_file_path(start)
-        if module_name and mod = Puppet::Module.find(module_name, options[:environment])
+        if mod = Puppet::Module.find(module_name, options[:environment])
             return mod.match_manifests(pattern)
-        else
-            abspat = File::expand_path(start, cwd)
-            files = Dir.glob(abspat).reject { |f| FileTest.directory?(f) }
-            if files.size == 0
-                files = Dir.glob(abspat + ".pp").reject { |f| FileTest.directory?(f) }
-            end
-            return files
         end
+
+        abspat = File::expand_path(start, cwd)
+        files = Dir.glob(abspat).reject { |f| FileTest.directory?(f) }
+        if files.size == 0
+            files = Dir.glob(abspat + ".pp").reject { |f| FileTest.directory?(f) }
+        end
+        return files
     end
 
     # Find the concrete file denoted by +file+. If +file+ is absolute,
diff --git a/spec/unit/file_serving/mount/plugins.rb b/spec/unit/file_serving/mount/plugins.rb
index c658b8cd6..ef842d12b 100755
--- a/spec/unit/file_serving/mount/plugins.rb
+++ b/spec/unit/file_serving/mount/plugins.rb
@@ -61,8 +61,8 @@ describe Puppet::FileServing::Mount::Plugins, "when searching for files" do
     end
 
     it "should return the plugin paths for each module that has plugins" do
-        one = stub 'module', :plugins? => true, :plugins => "/one"
-        two = stub 'module', :plugins? => true, :plugins => "/two"
+        one = stub 'module', :plugins? => true, :plugin_directory => "/one"
+        two = stub 'module', :plugins? => true, :plugin_directory => "/two"
 
         @environment.expects(:modules).returns [one, two]
         @mount.search("foo/bar").should == %w{/one /two}
diff --git a/spec/unit/module.rb b/spec/unit/module.rb
index 17c2065eb..4ff69695e 100755
--- a/spec/unit/module.rb
+++ b/spec/unit/module.rb
@@ -3,40 +3,137 @@
 require File.dirname(__FILE__) + '/../spec_helper'
 
 describe Puppet::Module do
+    it "should have a class method that returns a named module from a given environment" do
+        env = mock 'module'
+        env.expects(:module).with("mymod").returns "yep"
+        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+
+        Puppet::Module.find("mymod", "myenv").should == "yep"
+    end
+
+    it "should return nil if asked for a named module that doesn't exist" do
+        env = mock 'module'
+        env.expects(:module).with("mymod").returns nil
+        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+
+        Puppet::Module.find("mymod", "myenv").should be_nil
+    end
+
+    it "should return nil if asked for a module whose name is 'nil'" do
+        Puppet::Module.find(nil, "myenv").should be_nil
+    end
+
+    it "should require a name at initialization" do
+        lambda { Puppet::Module.new }.should raise_error(ArgumentError)
+    end
+
+    it "should convert an environment name into an Environment instance" do
+        Puppet::Module.new("foo", "prod").environment.should be_instance_of(Puppet::Node::Environment)
+    end
+
+    it "should accept an environment at initialization" do
+        Puppet::Module.new("foo", :prod).environment.name.should == :prod
+    end
+
+    it "should use the default environment if none is provided" do
+        env = Puppet::Node::Environment.new
+        Puppet::Module.new("foo").environment.should equal(env)
+    end
+
+    it "should use any provided Environment instance" do
+        env = Puppet::Node::Environment.new
+        Puppet::Module.new("foo", env).environment.should equal(env)
+    end
+
+    it "should return the path to the first found instance in its module paths as its path" do
+        mod = Puppet::Module.new("foo")
+        paths = %w{/a /b /c}
+        Puppet::Module.stubs(:modulepath).returns paths
+
+        FileTest.expects(:exist?).with("/a/foo").returns false
+        FileTest.expects(:exist?).with("/b/foo").returns true
+        FileTest.expects(:exist?).with("/c/foo").never
+
+        mod.path.should == "/b/foo"
+    end
+
+    it "should be considered existent if it exists in at least one module path" do
+        mod = Puppet::Module.new("foo")
+        mod.expects(:path).returns "/a/foo"
+        mod.should be_exist
+    end
+
+    it "should be considered nonexistent if it does not exist in any of the module paths" do
+        mod = Puppet::Module.new("foo")
+        mod.expects(:path).returns nil
+        mod.should_not be_exist
+    end
+
     [:plugins, :templates, :files, :manifests].each do |filetype|
-        it "should be able to indicate whether it has #{filetype}" do
-            Puppet::Module.new("foo", "/foo/bar").should respond_to(filetype.to_s + "?")
+        it "should be able to return individual #{filetype}" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s, "my/file")
+            FileTest.expects(:exist?).with(path).returns true
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should == path
         end
 
-        it "should correctly detect when it has #{filetype}" do
-            FileTest.expects(:exist?).with("/foo/bar/#{filetype}").returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s + "?").should be_true
+        it "should consider #{filetype} to be present if their base directory exists" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(path).returns true
+            mod.send(filetype.to_s + "?").should be_true
         end
 
-        it "should correctly detect when it does not have #{filetype}" do
-            FileTest.expects(:exist?).with("/foo/bar/#{filetype}").returns false
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s + "?").should be_false
+        it "should consider #{filetype} to be absent if their base directory does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(path).returns false
+            mod.send(filetype.to_s + "?").should be_false
         end
 
-        it "should have a method for returning the full path to the #{filetype}" do
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s).should == File.join("/foo/bar", filetype.to_s)
+        it "should consider #{filetype} to be absent if the module base directory does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns nil
+            mod.send(filetype.to_s + "?").should be_false
         end
 
-        it "should be able to return individual #{filetype}" do
-            path = File.join("/foo/bar", filetype.to_s, "my/file")
-            FileTest.expects(:exist?).with(path).returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), "my/file").should == path
+        it "should return nil if asked to return individual #{filetype} that don't exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            path = File.join("/a/foo", filetype.to_s, "my/file")
+            FileTest.expects(:exist?).with(path).returns false
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
         end
 
-        it "should return nil if asked to return individual #{filetype} that don't exist" do
-            FileTest.expects(:exist?).with(File.join("/foo/bar", filetype.to_s, "my/file")).returns false
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
+        it "should return nil when asked for individual #{filetype} if the module does not exist" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns nil
+            mod.send(filetype.to_s.sub(/s$/, ''), "my/file").should be_nil
         end
 
         it "should return the base directory if asked for a nil path" do
-            path = File.join("/foo/bar", filetype.to_s)
-            FileTest.expects(:exist?).with(path).returns true
-            Puppet::Module.new("foo", "/foo/bar").send(filetype.to_s.sub(/s$/, ''), nil).should == path
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+            base = File.join("/a/foo", filetype.to_s)
+            FileTest.expects(:exist?).with(base).returns true
+            mod.send(filetype.to_s.sub(/s$/, ''), nil).should == base
+        end
+    end
+
+    %w{plugins files}.each do |type|
+        short = type.sub(/s$/, '')
+        it "should be able to return the #{short} directory" do
+            Puppet::Module.new("foo").should respond_to(short + "_directory")
+        end
+
+        it "should return the path to the #{short} directory" do
+            mod = Puppet::Module.new("foo")
+            mod.stubs(:path).returns "/a/foo"
+
+            mod.send(short + "_directory").should == "/a/foo/#{type}"
         end
     end
 end
@@ -99,8 +196,8 @@ describe Puppet::Module, "when yielding each module in a list of directories" do
         one = mock 'one'
         two = mock 'two'
 
-        Puppet::Module.expects(:new).with("f1", "/one/f1").returns one
-        Puppet::Module.expects(:new).with("f2", "/one/f2").returns two
+        Puppet::Module.expects(:new).with("f1").returns one
+        Puppet::Module.expects(:new).with("f2").returns two
 
         result = []
         Puppet::Module.each_module("/one") do |mod|
@@ -116,8 +213,8 @@ describe Puppet::Module, "when yielding each module in a list of directories" do
 
         one = mock 'one'
 
-        Puppet::Module.expects(:new).with("f1", "/one/f1").returns one
-        Puppet::Module.expects(:new).with("f1", "/two/f1").never
+        Puppet::Module.expects(:new).with("f1").returns one
+        Puppet::Module.expects(:new).with("f1").never
 
         result = []
         Puppet::Module.each_module("/one", "/two") do |mod|
@@ -146,27 +243,48 @@ describe Puppet::Module, " when building its search path" do
     end
 end
 
-describe Puppet::Module, " when searching for modules" do
-    it "should use the current environment to find the specified module if no environment is provided" do
-        env = mock 'env'
-        env.expects(:module).with("foo").returns "yay"
-        Puppet::Node::Environment.expects(:new).with(nil).returns env
+describe Puppet::Module, "when finding matching manifests" do
+    before do
+        @mod = Puppet::Module.new("mymod")
+        @mod.stubs(:path).returns "/a"
+    end
+
+    it "should return all manifests matching the glob pattern" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{foo bar})
 
-        Puppet::Module.find("foo").should == "yay"
+        @mod.match_manifests("yay/*.pp").should == %w{foo bar}
     end
 
-    it "should use the specified environment to find the specified module if an environment is provided" do
-        env = mock 'env'
-        env.expects(:module).with("foo").returns "yay"
-        Puppet::Node::Environment.expects(:new).with("myenv").returns env
+    it "should not return directories" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{foo bar})
 
-        Puppet::Module.find("foo", "myenv").should == "yay"
+        FileTest.expects(:directory?).with("foo").returns false
+        FileTest.expects(:directory?).with("bar").returns true
+        @mod.match_manifests("yay/*.pp").should == %w{foo}
     end
-end
 
-describe Puppet::Module, " when returning files" do
-    it "should return the path to the module's 'files' directory" do
-        mod = Puppet::Module.send(:new, "mymod", "/my/mod")
-        mod.files.should == "/my/mod/files"
+    it "should default to the 'init.pp' file if no glob pattern is specified" do
+        FileTest.stubs(:exist?).returns true
+
+        @mod.match_manifests(nil).should == %w{/a/manifests/init.pp}
+    end
+
+    it "should return all manifests matching the glob pattern in all existing paths" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns(%w{a b})
+
+        @mod.match_manifests("yay/*.pp").should == %w{a b}
+    end
+
+    it "should match the glob pattern plus '.pp' if no results are found" do
+        Dir.expects(:glob).with("/a/manifests/yay/foo").returns([])
+        Dir.expects(:glob).with("/a/manifests/yay/foo.pp").returns(%w{yay})
+
+        @mod.match_manifests("yay/foo").should == %w{yay}
+    end
+
+    it "should return an empty array if no manifests matched" do
+        Dir.expects(:glob).with("/a/manifests/yay/*.pp").returns([])
+
+        @mod.match_manifests("yay/*.pp").should == []
     end
 end
diff --git a/spec/unit/node/environment.rb b/spec/unit/node/environment.rb
index dd6745f1e..5fac98b77 100755
--- a/spec/unit/node/environment.rb
+++ b/spec/unit/node/environment.rb
@@ -94,28 +94,24 @@ describe Puppet::Node::Environment do
             env.modules.should == %w{mod1 mod2}
         end
 
-        it "should be able to return an individual module by matching the module name" do
+        it "should be able to return an individual module that exists in its module path" do
             env = Puppet::Node::Environment.new("testing")
-            module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
-            env.expects(:modulepath).returns module_path
 
-            one = stub 'one', :name => 'one'
-            two = stub 'two', :name => 'two'
-            Puppet::Module.expects(:each_module).with(module_path).multiple_yields(one, two)
+            mod = mock 'module'
+            Puppet::Module.expects(:new).with("one", env).returns mod
+            mod.expects(:exist?).returns true
 
-            env.module("two").should equal(two)
+            env.module("one").should equal(mod)
         end
 
-        it "should return nil if asked for a module that is not in its path" do
+        it "should return nil if asked for a module that does not exist in its path" do
             env = Puppet::Node::Environment.new("testing")
-            module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
-            env.expects(:modulepath).returns module_path
 
-            one = stub 'one', :name => 'one'
-            two = stub 'two', :name => 'two'
-            Puppet::Module.expects(:each_module).with(module_path).multiple_yields(one, two)
+            mod = mock 'module'
+            Puppet::Module.expects(:new).with("one", env).returns mod
+            mod.expects(:exist?).returns false
 
-            env.module("three").should be_nil
+            env.module("one").should be_nil
         end
     end
 end
diff --git a/spec/unit/parser/files.rb b/spec/unit/parser/files.rb
index f401a902c..a7bd37b44 100644
--- a/spec/unit/parser/files.rb
+++ b/spec/unit/parser/files.rb
@@ -149,10 +149,6 @@ describe Puppet::Parser::Files do
     end
 
     describe "when searching for manifests in a found module" do
-        before do
-            @module = Puppet::Module.new("mymod", "/one")
-        end
-
         it "should return the manifests from the first found module" do
             mod = mock 'module'
             Puppet::Node::Environment.new.expects(:module).with("mymod").returns mod
@@ -167,28 +163,6 @@ describe Puppet::Parser::Files do
             Puppet::Parser::Files.find_manifests("mymod/init.pp", :environment => "myenv").should == ["/one/mymod/manifests/init.pp"]
         end
 
-        it "should return all manifests matching the glob pattern" do
-            File.stubs(:directory?).returns(true)
-            Dir.expects(:glob).with("/one/manifests/yay/*.pp").returns(%w{/one /two})
-
-            @module.match_manifests("yay/*.pp").should == %w{/one /two}
-        end
-
-        it "should not return directories" do
-            Dir.expects(:glob).with("/one/manifests/yay/*.pp").returns(%w{/one /two})
-
-            FileTest.expects(:directory?).with("/one").returns false
-            FileTest.expects(:directory?).with("/two").returns true
-
-            @module.match_manifests("yay/*.pp").should == %w{/one}
-        end
-
-        it "should default to the 'init.pp' file in the manifests directory" do
-            Dir.expects(:glob).with("/one/manifests/init.pp").returns(%w{/init.pp})
-
-            @module.match_manifests(nil).should == %w{/init.pp}
-        end
-
         after { Puppet.settings.clear }
     end
 end
diff --git a/test/language/parser.rb b/test/language/parser.rb
index 7794c42fe..01b18acd9 100755
--- a/test/language/parser.rb
+++ b/test/language/parser.rb
@@ -1222,7 +1222,7 @@ file { "/tmp/yayness":
 
         # We use an exception to cut short the processing to simplify our stubbing
         #Puppet::Module.expects(:find_manifests).with("test", {:cwd => ".", :environment => "something"}).raises(Puppet::ParseError)
-        Puppet::Module.expects(:find_manifests).with("test", {:cwd => ".", :environment => "something"}).returns([])
+        Puppet::Parser::Files.expects(:find_manifests).with("test", {:cwd => ".", :environment => "something"}).returns([])
 
         assert_raise(Puppet::ImportError) do
             parser.import("test")