Refactoring the Module/Environment co-interface

This simplifies who owns what code in these two classes,
and the result should be much cleaner and simpler.

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

2 files changed, 15 insertions, 28 deletions

diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index 2b6777c43..d332392f9 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -4,6 +4,12 @@ require 'puppet/util/logging'
 class Puppet::Module
     include Puppet::Util::Logging
 
+    class InvalidName < ArgumentError
+        def message
+            "Invalid module name; module names must be alphanumeric (plus '-')"
+        end
+    end
+
     TEMPLATES = "templates"
     FILES = "files"
     MANIFESTS = "manifests"
@@ -11,28 +17,6 @@ class Puppet::Module
 
     FILETYPES = [MANIFESTS, FILES, TEMPLATES, PLUGINS]
 
-    # Search through a list of paths, yielding each found module in turn.
-    def self.each_module(*paths)
-        paths = paths.flatten.collect { |p| p.split(File::PATH_SEPARATOR) }.flatten
-
-        yielded = {}
-        paths.each do |dir|
-            next unless FileTest.directory?(dir)
-
-            Dir.entries(dir).each do |name|
-                next if name =~ /^\./
-                next if yielded.include?(name)
-
-                module_path = File.join(dir, name)
-                next unless FileTest.directory?(module_path)
-
-                yielded[name] = true
-
-                yield Puppet::Module.new(name)
-            end
-        end
-    end
-
     # Return an array of paths by splitting the +modulepath+ config
     # parameter. Only consider paths that are absolute and existing
     # directories
@@ -53,6 +37,9 @@ class Puppet::Module
 
     def initialize(name, environment = nil)
         @name = name
+
+        assert_validity()
+
         if environment.is_a?(Puppet::Node::Environment)
             @environment = environment
         else
@@ -152,4 +139,8 @@ class Puppet::Module
             return File.join(path, "lib")
         end
     end
+
+    def assert_validity
+        raise InvalidName unless name =~ /^[\w-]+$/
+    end
 end
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
index 323f2793a..133f22c77 100644
--- a/lib/puppet/node/environment.rb
+++ b/lib/puppet/node/environment.rb
@@ -69,12 +69,8 @@ class Puppet::Node::Environment
     # Return all modules from this environment.
     # Cache the list, because it can be expensive to create.
     cached_attr(:modules, :ttl => Puppet[:filetimeout]) do
-        result = []
-        Puppet::Module.each_module(modulepath) do |mod|
-            mod.environment = self
-            result << mod
-        end
-        result
+        module_names = modulepath.collect { |path| Dir.entries(path) }.flatten.uniq
+        module_names.collect { |path| Puppet::Module.new(path, self) rescue nil }.compact
     end
 
     def to_s