[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5
Paul Berry
paul at puppetlabs.com
Tue May 10 07:58:50 UTC 2011
The following commit has been merged in the experimental branch:
commit 6b1dd81799a44288287d9ab0cdf46afa3aaf090a
Author: Paul Berry <paul at puppetlabs.com>
Date: Thu Aug 5 10:34:35 2010 -0700
[#4472]+[#4483] Moved type-name resolution out of Puppet::Parser::TypeLoader.
Moved type-name resolution out of Puppet::Parser::TypeLoader, and into
its primary client, Puppet::Resource::TypeCollection. TypeCollection
now always passes fully qualified type names to TypeLoader.
This avoids duplicate type-name resolution logic between TypeLoader
and TypeCollection. That in turn fixes bug 4472, which resulted
from flaws in the type-name resolution logic in TypeLoader.
In addition, it fixes bug 4483, which resulted from improper
interleaving between looking up names using the TypeCollection and the
TypeLoader.
diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index c0fd371..97d985c 100644
--- a/lib/puppet/parser/parser_support.rb
+++ b/lib/puppet/parser/parser_support.rb
@@ -103,11 +103,11 @@ class Puppet::Parser::Parser
end
def find_hostclass(namespace, name)
- known_resource_types.find_or_load(namespace, name, :hostclass)
+ known_resource_types.find_hostclass(namespace, name)
end
def find_definition(namespace, name)
- known_resource_types.find_or_load(namespace, name, :definition)
+ known_resource_types.find_definition(namespace, name)
end
def import(file)
diff --git a/lib/puppet/parser/type_loader.rb b/lib/puppet/parser/type_loader.rb
index 09aa636..516c1e3 100644
--- a/lib/puppet/parser/type_loader.rb
+++ b/lib/puppet/parser/type_loader.rb
@@ -76,50 +76,29 @@ class Puppet::Parser::TypeLoader
@imported = {}
end
- def load_until(namespaces, name)
- return nil if name == "" # special-case main.
- name2files(namespaces, name).each do |filename|
- modname = begin
- import_if_possible(filename)
- rescue Puppet::ImportError => detail
- # We couldn't load the item
- # I'm not convienced we should just drop these errors, but this
- # preserves existing behaviours.
- nil
- end
- if result = yield(filename)
- Puppet.debug "Automatically imported #{name} from #{filename} into #{environment}"
- result.module_name = modname if modname and result.respond_to?(:module_name=)
- return result
+ # Try to load the object with the given fully qualified name. For
+ # each file that was actually loaded, yield(filename, modname).
+ def try_load_fqname(fqname)
+ return nil if fqname == "" # special-case main.
+ name2files(fqname).each do |filename|
+ if not loaded?(filename)
+ modname = begin
+ import_if_possible(filename)
+ rescue Puppet::ImportError => detail
+ # We couldn't load the item
+ # I'm not convienced we should just drop these errors, but this
+ # preserves existing behaviours.
+ nil
+ end
+ yield(filename, modname)
end
end
- nil
end
def loaded?(name)
@loaded.include?(name)
end
- def name2files(namespaces, name)
- return [name.sub(/^::/, '').gsub("::", File::SEPARATOR)] if name =~ /^::/
-
- result = namespaces.inject([]) do |names_to_try, namespace|
- fullname = (namespace + "::#{name}").sub(/^::/, '')
-
- # Try to load the module init file if we're a qualified name
- names_to_try << fullname.split("::")[0] if fullname.include?("::")
-
- # Then the fully qualified name
- names_to_try << fullname
- end
-
- # Otherwise try to load the bare name on its own. This
- # is appropriate if the class we're looking for is in a
- # module that's different from our namespace.
- result << name
- result.uniq.collect { |f| f.gsub("::", File::SEPARATOR) }
- end
-
def parse_file(file)
Puppet.debug("importing '#{file}' in environment #{environment}")
parser = Puppet::Parser::Parser.new(environment)
@@ -143,4 +122,19 @@ class Puppet::Parser::TypeLoader
@loading.done_with(file)
end
end
+
+ private
+
+ # Return a list of all file basenames that should be tried in order
+ # to load the object with the given fully qualified name.
+ def name2files(fqname)
+ result = []
+ ary = fqname.split("::")
+ while ary.length > 0
+ result << ary.join(File::SEPARATOR)
+ ary.pop
+ end
+ return result
+ end
+
end
diff --git a/lib/puppet/resource/type_collection.rb b/lib/puppet/resource/type_collection.rb
index 63d1103..3a327e2 100644
--- a/lib/puppet/resource/type_collection.rb
+++ b/lib/puppet/resource/type_collection.rb
@@ -92,50 +92,8 @@ class Puppet::Resource::TypeCollection
@definitions[munge_name(name)]
end
- def find(namespaces, name, type)
- #Array("") == [] for some reason
- namespaces = [namespaces] unless namespaces.is_a?(Array)
-
- if name =~ /^::/
- return send(type, name.sub(/^::/, ''))
- end
-
- namespaces.each do |namespace|
- ary = namespace.split("::")
-
- while ary.length > 0
- tmp_namespace = ary.join("::")
- if r = find_partially_qualified(tmp_namespace, name, type)
- return r
- end
-
- # Delete the second to last object, which reduces our namespace by one.
- ary.pop
- end
-
- if result = send(type, name)
- return result
- end
- end
- nil
- end
-
- def find_or_load(namespaces, name, type)
- name = name.downcase
- namespaces = [namespaces] unless namespaces.is_a?(Array)
- namespaces = namespaces.collect { |ns| ns.downcase }
-
- # This could be done in the load_until, but the knowledge seems to
- # belong here.
- if r = find(namespaces, name, type)
- return r
- end
-
- loader.load_until(namespaces, name) { find(namespaces, name, type) }
- end
-
def find_node(namespaces, name)
- find("", name, :node)
+ @nodes[munge_name(name)]
end
def find_hostclass(namespaces, name)
@@ -198,8 +156,59 @@ class Puppet::Resource::TypeCollection
private
- def find_partially_qualified(namespace, name, type)
- send(type, [namespace, name].join("::"))
+ # Return a list of all possible fully-qualified names that might be
+ # meant by the given name, in the context of namespaces.
+ def resolve_namespaces(namespaces, name)
+ name = name.downcase
+ if name =~ /^::/
+ # name is explicitly fully qualified, so just return it, sans
+ # initial "::".
+ return [name.sub(/^::/, '')]
+ end
+ if name == ""
+ # The name "" has special meaning--it always refers to a "main"
+ # hostclass which contains all toplevel resources.
+ return [""]
+ end
+
+ namespaces = [namespaces] unless namespaces.is_a?(Array)
+ namespaces = namespaces.collect { |ns| ns.downcase }
+
+ result = []
+ namespaces.each do |namespace|
+ ary = namespace.split("::")
+
+ # Search each namespace nesting in innermost-to-outermost order.
+ while ary.length > 0
+ result << "#{ary.join("::")}::#{name}"
+ ary.pop
+ end
+
+ # Finally, search the toplevel namespace.
+ result << name
+ end
+
+ return result.uniq
+ end
+
+ # Resolve namespaces and find the given object. Autoload it if
+ # necessary.
+ def find_or_load(namespaces, name, type)
+ resolve_namespaces(namespaces, name).each do |fqname|
+ if result = send(type, fqname)
+ return result
+ end
+ loader.try_load_fqname(fqname) do |filename, modname|
+ if result = send(type, fqname)
+ Puppet.debug "Automatically imported #{name} from #{filename} into #{environment}"
+ result.module_name = modname if modname and result.respond_to?(:module_name=)
+ return result
+ end
+ end
+ end
+
+ # Nothing found.
+ return nil
end
def munge_name(name)
diff --git a/spec/unit/parser/type_loader_spec.rb b/spec/unit/parser/type_loader_spec.rb
index 8f005d5..b7e1747 100644
--- a/spec/unit/parser/type_loader_spec.rb
+++ b/spec/unit/parser/type_loader_spec.rb
@@ -28,86 +28,28 @@ describe Puppet::Parser::TypeLoader do
describe "when loading names from namespaces" do
it "should do nothing if the name to import is an empty string" do
@loader.expects(:name2files).never
- @loader.load_until(["foo"], "") { |f| false }.should be_nil
- end
-
- it "should turn the provided namespaces and name into a list of files" do
- @loader.expects(:name2files).with(["foo"], "bar").returns []
- @loader.load_until(["foo"], "bar") { |f| false }
+ @loader.try_load_fqname("") { |filename, modname| raise :should_not_occur }.should be_nil
end
it "should attempt to import each generated name" do
- @loader.expects(:name2files).returns %w{foo bar}
+ @loader.expects(:import).with("foo/bar",nil)
@loader.expects(:import).with("foo",nil)
- @loader.expects(:import).with("bar",nil)
- @loader.load_until(["foo"], "bar") { |f| false }
+ @loader.try_load_fqname("foo::bar") { |f| false }
end
it "should yield after each import" do
yielded = []
- @loader.expects(:name2files).returns %w{foo bar}
- @loader.expects(:import).with("foo",nil)
- @loader.expects(:import).with("bar",nil)
- @loader.load_until(["foo"], "bar") { |f| yielded << f; false }
- yielded.should == %w{foo bar}
- end
-
- it "should stop importing when the yielded block returns true" do
- yielded = []
- @loader.expects(:name2files).returns %w{foo bar baz}
- @loader.expects(:import).with("foo",nil)
- @loader.expects(:import).with("bar",nil)
- @loader.expects(:import).with("baz",nil).never
- @loader.load_until(["foo"], "bar") { |f| true if f == "bar" }
- end
-
- it "should return the result of the block" do
- yielded = []
- @loader.expects(:name2files).returns %w{foo bar baz}
+ @loader.expects(:import).with("foo/bar",nil)
@loader.expects(:import).with("foo",nil)
- @loader.expects(:import).with("bar",nil)
- @loader.expects(:import).with("baz",nil).never
- @loader.load_until(["foo"], "bar") { |f| 10 if f == "bar" }.should == 10
- end
-
- it "should return nil if the block never returns true" do
- @loader.expects(:name2files).returns %w{foo bar}
- @loader.expects(:import).with("foo",nil)
- @loader.expects(:import).with("bar",nil)
- @loader.load_until(["foo"], "bar") { |f| false }.should be_nil
+ @loader.try_load_fqname("foo::bar") { |filename, modname| yielded << [filename, modname]; false }
+ yielded.should == [["foo/bar", nil], ["foo", nil]]
end
it "should know when a given name has been loaded" do
- @loader.expects(:name2files).returns %w{file}
@loader.expects(:import).with("file",nil)
- @loader.load_until(["foo"], "bar") { |f| true }
+ @loader.try_load_fqname("file") { |f| true }
@loader.should be_loaded("file")
end
-
- it "should set the module name on any created resource types" do
- type = Puppet::Resource::Type.new(:hostclass, "mytype")
-
- Puppet::Parser::Files.expects(:find_manifests).returns ["modname", %w{one}]
- @loader.stubs(:parse_file)
- @loader.load_until(["foo"], "one") { |f| type }
-
- type.module_name.should == "modname"
- end
- end
-
- describe "when mapping names to files" do
- {
- [["foo"], "::bar::baz"] => %w{bar/baz},
- [[""], "foo::bar"] => %w{foo foo/bar},
- [%w{foo}, "bar"] => %w{foo foo/bar bar},
- [%w{a b}, "bar"] => %w{a a/bar b b/bar bar},
- [%w{a::b::c}, "bar"] => %w{a a/b/c/bar bar},
- [%w{a::b}, "foo::bar"] => %w{a a/b/foo/bar foo/bar}
- }.each do |inputs, outputs|
- it "should produce #{outputs.inspect} from the #{inputs[0].inspect} namespace and #{inputs[1]} name" do
- @loader.name2files(*inputs).should == outputs
- end
- end
end
describe "when importing" do
@@ -198,4 +140,15 @@ describe Puppet::Parser::TypeLoader do
@loader.known_resource_types.hostclass("foo").should be_instance_of(Puppet::Resource::Type)
end
+
+ describe "when deciding where to look for files" do
+ { 'foo' => ['foo'],
+ 'foo::bar' => ['foo/bar', 'foo'],
+ 'foo::bar::baz' => ['foo/bar/baz', 'foo/bar', 'foo']
+ }.each do |fqname, expected_paths|
+ it "should look for #{fqname.inspect} in #{expected_paths.inspect}" do
+ @loader.instance_eval { name2files(fqname) }.should == expected_paths
+ end
+ end
+ end
end
diff --git a/spec/unit/resource/type_collection_spec.rb b/spec/unit/resource/type_collection_spec.rb
index 577aea4..4bad291 100644
--- a/spec/unit/resource/type_collection_spec.rb
+++ b/spec/unit/resource/type_collection_spec.rb
@@ -89,6 +89,34 @@ describe Puppet::Resource::TypeCollection do
loader.node("node").should be_nil
end
+ describe "when resolving namespaces" do
+ [ ['', '::foo', ['foo']],
+ ['a', '::foo', ['foo']],
+ ['a::b', '::foo', ['foo']],
+ [['a::b'], '::foo', ['foo']],
+ [['a::b', 'c'], '::foo', ['foo']],
+ [['A::B', 'C'], '::Foo', ['foo']],
+ ['', '', ['']],
+ ['a', '', ['']],
+ ['a::b', '', ['']],
+ [['a::b'], '', ['']],
+ [['a::b', 'c'], '', ['']],
+ [['A::B', 'C'], '', ['']],
+ ['', 'foo', ['foo']],
+ ['a', 'foo', ['a::foo', 'foo']],
+ ['a::b', 'foo', ['a::b::foo', 'a::foo', 'foo']],
+ ['A::B', 'Foo', ['a::b::foo', 'a::foo', 'foo']],
+ [['a::b'], 'foo', ['a::b::foo', 'a::foo', 'foo']],
+ [['a', 'b'], 'foo', ['a::foo', 'foo', 'b::foo']],
+ [['a::b', 'c::d'], 'foo', ['a::b::foo', 'a::foo', 'foo', 'c::d::foo', 'c::foo']],
+ [['a::b', 'a::c'], 'foo', ['a::b::foo', 'a::foo', 'foo', 'a::c::foo']],
+ ].each do |namespaces, name, expected_result|
+ it "should resolve #{name.inspect} in namespaces #{namespaces.inspect} correctly" do
+ @code.instance_eval { resolve_namespaces(namespaces, name) }.should == expected_result
+ end
+ end
+ end
+
describe "when looking up names" do
before do
@type = Puppet::Resource::Type.new(:hostclass, "ns::klass")
@@ -107,29 +135,32 @@ describe Puppet::Resource::TypeCollection do
describe "that need to be loaded" do
it "should use the loader to load the files" do
- @code.loader.expects(:load_until).with(["ns"], "klass")
- @code.find_or_load(["ns"], "klass", :hostclass)
+ @code.loader.expects(:try_load_fqname).with("ns::klass")
+ @code.loader.expects(:try_load_fqname).with("klass")
+ @code.find_hostclass(["ns"], "klass")
end
it "should downcase the name and downcase and array-fy the namespaces before passing to the loader" do
- @code.loader.expects(:load_until).with(["ns"], "klass")
- @code.find_or_load("Ns", "Klass", :hostclass)
+ @code.loader.expects(:try_load_fqname).with("ns::klass")
+ @code.loader.expects(:try_load_fqname).with("klass")
+ @code.find_hostclass("Ns", "Klass")
end
it "should attempt to find the type when the loader yields" do
- @code.loader.expects(:load_until).yields
- @code.expects(:find).with(["ns"], "klass", :hostclass).times(2).returns(false).then.returns(true)
- @code.find_or_load("ns", "klass", :hostclass)
+ @code.loader.expects(:try_load_fqname).yields
+ @code.expects(:hostclass).with("ns::klass").times(2).returns(false).then.returns(true)
+ @code.find_hostclass("ns", "klass")
end
- it "should return the result of 'load_until'" do
- @code.loader.expects(:load_until).returns "foo"
- @code.find_or_load("Ns", "Klass", :hostclass).should == "foo"
+ it "should return nil if the name isn't found" do
+ @code.stubs(:try_load_fqname).returns(nil)
+ @code.find_hostclass("Ns", "Klass").should be_nil
end
- it "should return nil if the name isn't found" do
- @code.stubs(:load_until).returns(nil)
- @code.find_or_load("Ns", "Klass", :hostclass).should be_nil
+ it "already-loaded names at broader scopes should not shadow autoloaded names" do
+ @code.add Puppet::Resource::Type.new(:hostclass, "bar")
+ @code.loader.expects(:try_load_fqname).with("foo::bar")
+ @code.find_hostclass("foo", "bar")
end
end
end
@@ -195,68 +226,68 @@ describe Puppet::Resource::TypeCollection do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar")
loader.add instance
- loader.find("namespace", "::foo::bar", :hostclass).should equal(instance)
+ loader.find_hostclass("namespace", "::foo::bar").should equal(instance)
end
it "should return nil if the instance name is fully qualified and no such instance exists" do
loader = Puppet::Resource::TypeCollection.new("env")
- loader.find("namespace", "::foo::bar", :hostclass).should be_nil
+ loader.find_hostclass("namespace", "::foo::bar").should be_nil
end
it "should be able to find classes in the base namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo")
loader.add instance
- loader.find("", "foo", :hostclass).should equal(instance)
+ loader.find_hostclass("", "foo").should equal(instance)
end
it "should return the partially qualified object if it exists in a provided namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar::baz")
loader.add instance
- loader.find("foo", "bar::baz", :hostclass).should equal(instance)
+ loader.find_hostclass("foo", "bar::baz").should equal(instance)
end
it "should be able to find partially qualified objects in any of the provided namespaces" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar::baz")
loader.add instance
- loader.find(["nons", "foo", "otherns"], "bar::baz", :hostclass).should equal(instance)
+ loader.find_hostclass(["nons", "foo", "otherns"], "bar::baz").should equal(instance)
end
it "should return the unqualified object if it exists in a provided namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar")
loader.add instance
- loader.find("foo", "bar", :hostclass).should equal(instance)
+ loader.find_hostclass("foo", "bar").should equal(instance)
end
it "should return the unqualified object if it exists in the parent namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar")
loader.add instance
- loader.find("foo::bar::baz", "bar", :hostclass).should equal(instance)
+ loader.find_hostclass("foo::bar::baz", "bar").should equal(instance)
end
it "should should return the partially qualified object if it exists in the parent namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar::baz")
loader.add instance
- loader.find("foo::bar", "bar::baz", :hostclass).should equal(instance)
+ loader.find_hostclass("foo::bar", "bar::baz").should equal(instance)
end
it "should return the qualified object if it exists in the root namespace" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar::baz")
loader.add instance
- loader.find("foo::bar", "foo::bar::baz", :hostclass).should equal(instance)
+ loader.find_hostclass("foo::bar", "foo::bar::baz").should equal(instance)
end
it "should return nil if the object cannot be found" do
loader = Puppet::Resource::TypeCollection.new("env")
instance = Puppet::Resource::Type.new(:hostclass, "foo::bar::baz")
loader.add instance
- loader.find("foo::bar", "eh", :hostclass).should be_nil
+ loader.find_hostclass("foo::bar", "eh").should be_nil
end
describe "when topscope has a class that has the same name as a local class" do
@@ -268,11 +299,11 @@ describe Puppet::Resource::TypeCollection do
end
it "should favor the local class, if the name is unqualified" do
- @loader.find("foo", "bar", :hostclass).name.should == 'foo::bar'
+ @loader.find_hostclass("foo", "bar").name.should == 'foo::bar'
end
it "should only look in the topclass, if the name is qualified" do
- @loader.find("foo", "::bar", :hostclass).name.should == 'bar'
+ @loader.find_hostclass("foo", "::bar").name.should == 'bar'
end
end
@@ -281,15 +312,16 @@ describe Puppet::Resource::TypeCollection do
@loader = Puppet::Resource::TypeCollection.new("env")
@loader.add Puppet::Resource::Type.new(:hostclass, "foo::bar")
- @loader.find("foo", "::bar", :hostclass).should == nil
+ @loader.find_hostclass("foo", "::bar").should == nil
end
end
- it "should use the generic 'find' method with an empty namespace to find nodes" do
+ it "should be able to find nodes" do
+ node = Puppet::Resource::Type.new(:node, "bar")
loader = Puppet::Resource::TypeCollection.new("env")
- loader.expects(:find).with("", "bar", :node)
- loader.find_node(stub("ignored"), "bar")
+ loader.add(node)
+ loader.find_node(stub("ignored"), "bar").should == node
end
it "should use the 'find_or_load' method to find hostclasses" do
diff --git a/spec/unit/resource/type_spec.rb b/spec/unit/resource/type_spec.rb
index 4d3942c..ac9d381 100755
--- a/spec/unit/resource/type_spec.rb
+++ b/spec/unit/resource/type_spec.rb
@@ -514,8 +514,7 @@ describe Puppet::Resource::Type do
@compiler.add_resource @scope, @parent_resource
@type.resource_type_collection = @scope.known_resource_types
- @type.resource_type_collection.stubs(:node).with("parent").returns(@parent_type)
- @type.resource_type_collection.stubs(:node).with("Parent").returns(@parent_type)
+ @type.resource_type_collection.add(@parent_type)
end
it "should evaluate the parent's resource" do
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list