[#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.

3 files changed, 86 insertions, 83 deletions

diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb
index c0fd37178..97d985cfb 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 09aa636e1..516c1e32b 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 63d110395..3a327e264 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)