summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--lib/puppet/module.rb35
-rw-r--r--lib/puppet/node/environment.rb8
-rwxr-xr-xspec/unit/module.rb91
-rwxr-xr-xspec/unit/node/environment.rb67
4 files changed, 71 insertions, 130 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
diff --git a/spec/unit/module.rb b/spec/unit/module.rb
index 29883dc26..4fa6ec5af 100755
--- a/spec/unit/module.rb
+++ b/spec/unit/module.rb
@@ -43,6 +43,10 @@ describe Puppet::Module do
mod.to_s.should == "Module foo(/a)"
end
+ it "should fail if its name is not alphanumeric" do
+ lambda { Puppet::Module.new(".something") }.should raise_error(Puppet::Module::InvalidName)
+ end
+
it "should require a name at initialization" do
lambda { Puppet::Module.new }.should raise_error(ArgumentError)
end
@@ -178,93 +182,6 @@ describe Puppet::Module do
end
end
-describe Puppet::Module, "when yielding each module in a list of directories" do
- before do
- FileTest.stubs(:directory?).returns true
- end
-
- it "should search for modules in each directory in the list" do
- Dir.expects(:entries).with("/one").returns []
- Dir.expects(:entries).with("/two").returns []
-
- Puppet::Module.each_module("/one", "/two")
- end
-
- it "should accept the list of directories as an array" do
- Dir.expects(:entries).with("/one").returns []
- Dir.expects(:entries).with("/two").returns []
-
- Puppet::Module.each_module(%w{/one /two})
- end
-
- it "should accept the list of directories joined by #{File::PATH_SEPARATOR}" do
- Dir.expects(:entries).with("/one").returns []
- Dir.expects(:entries).with("/two").returns []
-
- list = %w{/one /two}.join(File::PATH_SEPARATOR)
-
- Puppet::Module.each_module(list)
- end
-
- it "should not create modules for '.' or '..' in the provided directory list" do
- Dir.expects(:entries).with("/one").returns(%w{. ..})
-
- result = []
- Puppet::Module.each_module("/one") do |mod|
- result << mod
- end
-
- result.should be_empty
- end
-
- it "should not create modules for non-directories in the provided directory list" do
- Dir.expects(:entries).with("/one").returns(%w{notdir})
-
- FileTest.expects(:directory?).with("/one/notdir").returns false
-
- result = []
- Puppet::Module.each_module("/one") do |mod|
- result << mod
- end
-
- result.should be_empty
- end
-
- it "should yield each found module" do
- Dir.expects(:entries).with("/one").returns(%w{f1 f2})
-
- one = mock 'one'
- two = mock '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|
- result << mod
- end
-
- result.should == [one, two]
- end
-
- it "should not yield a module with the same name as a previously yielded module" do
- Dir.expects(:entries).with("/one").returns(%w{f1})
- Dir.expects(:entries).with("/two").returns(%w{f1})
-
- one = mock 'one'
-
- Puppet::Module.expects(:new).with("f1").returns one
- Puppet::Module.expects(:new).with("f1").never
-
- result = []
- Puppet::Module.each_module("/one", "/two") do |mod|
- result << mod
- end
-
- result.should == [one]
- end
-end
-
describe Puppet::Module, " when building its search path" do
it "should use the current environment's search path if no environment is specified" do
env = mock 'env'
diff --git a/spec/unit/node/environment.rb b/spec/unit/node/environment.rb
index 18747d1b7..a16b3d6e1 100755
--- a/spec/unit/node/environment.rb
+++ b/spec/unit/node/environment.rb
@@ -96,21 +96,6 @@ describe Puppet::Node::Environment do
env["myvar"].should == "myval"
end
- it "should be able to return its modules" do
- Puppet::Node::Environment.new("testing").should respond_to(:modules)
- end
-
- it "should return each module from the environment-specific module path when asked for its modules" do
- env = Puppet::Node::Environment.new("testing")
- module_path = %w{/one /two}.join(File::PATH_SEPARATOR)
- env.expects(:modulepath).returns module_path
-
- mods = [Puppet::Module.new('mod1'), Puppet::Module.new("mod2")]
- Puppet::Module.expects(:each_module).with(module_path).multiple_yields(*mods)
-
- env.modules.should == mods
- end
-
it "should be able to return an individual module that exists in its module path" do
env = Puppet::Node::Environment.new("testing")
@@ -130,5 +115,57 @@ describe Puppet::Node::Environment do
env.module("one").should be_nil
end
+
+ it "should be able to return its modules" do
+ Puppet::Node::Environment.new("testing").should respond_to(:modules)
+ end
+
+ describe ".modules" do
+ it "should return a module named for every directory in each module path" do
+ env = Puppet::Node::Environment.new("testing")
+ env.expects(:modulepath).returns %w{/a /b}
+ Dir.expects(:entries).with("/a").returns %w{foo bar}
+ Dir.expects(:entries).with("/b").returns %w{bee baz}
+
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo bar bee baz}.sort
+ end
+
+ it "should remove duplicates" do
+ env = Puppet::Node::Environment.new("testing")
+ env.expects(:modulepath).returns %w{/a /b}
+ Dir.expects(:entries).with("/a").returns %w{foo}
+ Dir.expects(:entries).with("/b").returns %w{foo}
+
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo}
+ end
+
+ it "should ignore invalid modules" do
+ env = Puppet::Node::Environment.new("testing")
+ env.expects(:modulepath).returns %w{/a}
+ Dir.expects(:entries).with("/a").returns %w{foo bar}
+
+ Puppet::Module.expects(:new).with { |name, env| name == "foo" }.returns mock("foomod", :name => "foo")
+ Puppet::Module.expects(:new).with { |name, env| name == "bar" }.raises Puppet::Module::InvalidName
+
+ env.modules.collect{|mod| mod.name}.sort.should == %w{foo}
+ end
+
+ it "should create modules with the correct environment" do
+ env = Puppet::Node::Environment.new("testing")
+ env.expects(:modulepath).returns %w{/a}
+ Dir.expects(:entries).with("/a").returns %w{foo}
+
+ env.modules.should be_all{|mod| mod.environment == "testing" }
+ end
+
+ it "should cache the module list" do
+ env = Puppet::Node::Environment.new("testing")
+ env.expects(:modulepath).once.returns %w{/a}
+ Dir.expects(:entries).once.with("/a").returns %w{foo}
+
+ env.modules
+ env.modules
+ end
+ end
end
end