Theoretically, this patch is to fix #917 (which it does), but

there were enough problems fixing it that I decided something
more drastic needed to be done.

This uses the new Puppet::ResourceReference class to canonize
what a resource reference looks like and how to retrieve resources
via their references.  Specifically, it guarantees that resource types
are always capitalized, even when they include '::' in them.

While many files are modified in this commit, the majority of changes are
quite small, and most of the changes are fixing the tests to use
capitalized types.

As we look at consolidating some of our resource types, we could consolidate
the ResourceReference stuff at the same time, but at least the
Puppet::Parser::ResourceReference class subclasses the main Puppet::ResourceReference
class.

14 files changed, 89 insertions, 144 deletions

diff --git a/lib/puppet/dsl.rb b/lib/puppet/dsl.rb
index 97d06a4ec..c1b86e06d 100644
--- a/lib/puppet/dsl.rb
+++ b/lib/puppet/dsl.rb
@@ -81,6 +81,7 @@ module Puppet
             end
             bucket = Puppet::TransBucket.new(objects)
             bucket.name = "top"
+            bucket.type = "class"
 
             return bucket
         end
diff --git a/lib/puppet/metatype/attributes.rb b/lib/puppet/metatype/attributes.rb
index dc59ce6b6..b83fcdd78 100644
--- a/lib/puppet/metatype/attributes.rb
+++ b/lib/puppet/metatype/attributes.rb
@@ -428,8 +428,7 @@ class Puppet::Type
             if defined? @title and @title
                 hash[namevar] = @title
             else
-                raise Puppet::Error,
-                    "Was not passed a namevar or title"
+                raise Puppet::Error, "Was not passed a namevar or title"
             end
         end
 
diff --git a/lib/puppet/metatype/manager.rb b/lib/puppet/metatype/manager.rb
index e95b6fd07..773f30b1b 100644
--- a/lib/puppet/metatype/manager.rb
+++ b/lib/puppet/metatype/manager.rb
@@ -111,7 +111,7 @@ module Manager
     def type(name)
         @types ||= {}
 
-        name = symbolize(name)
+        name = name.to_s.downcase.to_sym
 
         if t = @types[name]
             return t
diff --git a/lib/puppet/metatype/metaparams.rb b/lib/puppet/metatype/metaparams.rb
index eb158a47d..349a2c1bb 100644
--- a/lib/puppet/metatype/metaparams.rb
+++ b/lib/puppet/metatype/metaparams.rb
@@ -276,18 +276,12 @@ class Puppet::Type
                 # we just have a name and a type, and we need to convert it
                 # to an object...
                 tname, name = value
-                object = nil
-                if type = Puppet::Type.type(tname)
-                    object = type[name]
-                else # try to treat it as a component
-                    object = Puppet::Type::Component["#{tname}[#{name}]"]
-                end
+                reference = Puppet::ResourceReference.new(tname, name)
                 
                 # Either of the two retrieval attempts could have returned
                 # nil.
-                unless object
-                    self.fail "Could not retrieve dependency '%s[%s]' of %s" %
-                        [tname.to_s.capitalize, @resource.ref, name]
+                unless object = reference.resolve
+                    self.fail "Could not retrieve dependency '%s' of %s" % [reference, @resource.ref]
                 end
 
                 # Are we requiring them, or vice versa?  See the method docs
diff --git a/lib/puppet/node/configuration.rb b/lib/puppet/node/configuration.rb
index 393f23913..5da539e5c 100644
--- a/lib/puppet/node/configuration.rb
+++ b/lib/puppet/node/configuration.rb
@@ -103,9 +103,7 @@ class Puppet::Node::Configuration < Puppet::PGraph
         rescue Puppet::Error => detail
             Puppet.err "Could not apply complete configuration: %s" % detail
         rescue => detail
-            if Puppet[:trace]
-                puts detail.backtrace
-            end
+            puts detail.backtrace if Puppet[:trace]
             Puppet.err "Got an uncaught exception of type %s: %s" % [detail.class, detail]
         ensure
             # Don't try to store state unless we're a host config
@@ -113,21 +111,15 @@ class Puppet::Node::Configuration < Puppet::PGraph
             Puppet::Util::Storage.store if host_config?
         end
 
-        if block_given?
-            yield transaction
-        end
+        yield transaction if block_given?
         
-        if host_config and (Puppet[:report] or Puppet[:summarize])
-            transaction.send_report
-        end
+        transaction.send_report if host_config and (Puppet[:report] or Puppet[:summarize])
 
         return transaction
     ensure
         @applying = false
         cleanup()
-        if defined? transaction and transaction
-            transaction.cleanup
-        end
+        transaction.cleanup if defined? transaction and transaction
     end
 
     # Are we in the middle of applying the configuration?
@@ -213,7 +205,7 @@ class Puppet::Node::Configuration < Puppet::PGraph
         current = nil
         buckets = {}
 
-        unless main = vertices.find { |res| res.type == "class" and res.title == :main }
+        unless main = vertices.find { |res| res.type == "Class" and res.title == :main }
             raise Puppet::DevError, "Could not find 'main' class; cannot generate configuration"
         end
 
@@ -358,10 +350,15 @@ class Puppet::Node::Configuration < Puppet::PGraph
 
     # Look a resource up by its reference (e.g., File[/etc/passwd]).
     def resource(type, title = nil)
+        # Always create a resource reference, so that it always canonizes how we
+        # are referring to them.
         if title
-            ref = "%s[%s]" % [type.to_s.capitalize, title]
+            ref = Puppet::ResourceReference.new(type, title).to_s
         else
-            ref = type
+            # If they didn't provide a title, then we expect the first
+            # argument to be of the form 'Class[name]', which our
+            # Reference class canonizes for us.
+            ref = Puppet::ResourceReference.new(nil, type).to_s
         end
         if resource = @resource_table[ref]
             return resource
diff --git a/lib/puppet/parser/ast/resource_defaults.rb b/lib/puppet/parser/ast/resource_defaults.rb
index 44ec146b0..8f9c1b8df 100644
--- a/lib/puppet/parser/ast/resource_defaults.rb
+++ b/lib/puppet/parser/ast/resource_defaults.rb
@@ -10,7 +10,10 @@ class Puppet::Parser::AST
         # object type.
         def evaluate(hash)
             scope = hash[:scope]
-            type = @type.downcase
+
+            # Use a resource reference to canonize the type
+            ref = Puppet::ResourceReference.new(@type, "whatever")
+            type = ref.type
             params = @params.safeevaluate(:scope => scope)
 
             parsewrap do
diff --git a/lib/puppet/parser/collector.rb b/lib/puppet/parser/collector.rb
index cdde51bee..9dd07f31a 100644
--- a/lib/puppet/parser/collector.rb
+++ b/lib/puppet/parser/collector.rb
@@ -28,7 +28,9 @@ class Puppet::Parser::Collector
 
     def initialize(scope, type, equery, vquery, form)
         @scope = scope
-        @type = type
+
+        # Canonize the type
+        @type = Puppet::ResourceReference.new(type, "whatever").type
         @equery = equery
         @vquery = vquery
 
@@ -98,8 +100,7 @@ class Puppet::Parser::Collector
     end
 
     def collect_exported_resources
-        raise Puppet::ParseError,
-            "realize() is not yet implemented for exported resources"
+        raise Puppet::ParseError, "realize() is not yet implemented for exported resources"
     end
 
     # Collect resources directly; this is the result of using 'realize',
diff --git a/lib/puppet/parser/resource.rb b/lib/puppet/parser/resource.rb
index 2f7fff13f..3f346166e 100644
--- a/lib/puppet/parser/resource.rb
+++ b/lib/puppet/parser/resource.rb
@@ -131,8 +131,9 @@ class Puppet::Parser::Resource
         end
 
         @tags = []
-        @tags << @ref.type.to_s
-        @tags << @ref.title.to_s if @ref.title.to_s =~ /^[-\w]+$/
+        tag(@ref.type)
+        tag(@ref.title) if @ref.title.to_s =~ /^[-\w]+$/
+
         if scope.resource
             @tags += scope.resource.tags
         end
@@ -225,15 +226,14 @@ class Puppet::Parser::Resource
     # Add a tag to our current list.  These tags will be added to all
     # of the objects contained in this scope.
     def tag(*ary)
-        ary.each { |tag|
-            tag = tag.to_s
+        ary.collect { |tag| tag.to_s.downcase }.collect { |tag| tag.split("::") }.flatten.each do |tag|
             unless tag =~ /^\w[-\w]*$/
                 fail Puppet::ParseError, "Invalid tag %s" % tag.inspect
             end
             unless @tags.include?(tag)
                 @tags << tag
             end
-        }
+        end
     end
 
     def tags
diff --git a/lib/puppet/parser/resource/reference.rb b/lib/puppet/parser/resource/reference.rb
index 543ee7195..1dd816093 100644
--- a/lib/puppet/parser/resource/reference.rb
+++ b/lib/puppet/parser/resource/reference.rb
@@ -1,9 +1,11 @@
+require 'puppet/resource_reference'
+
 # A reference to a resource.  Mostly just the type and title.
-class Puppet::Parser::Resource::Reference
+class Puppet::Parser::Resource::Reference < Puppet::ResourceReference
     include Puppet::Util::MethodHelper
     include Puppet::Util::Errors
 
-    attr_accessor :type, :title, :builtin, :file, :line, :scope
+    attr_accessor :builtin, :file, :line, :scope
 
     # Are we a builtin type?
     def builtin?
@@ -19,7 +21,7 @@ class Puppet::Parser::Resource::Reference
     end
 
     def builtintype
-        if t = Puppet::Type.type(self.type) and t.name != :component
+        if t = Puppet::Type.type(self.type.downcase) and t.name != :component
             t
         else
             nil
@@ -30,28 +32,24 @@ class Puppet::Parser::Resource::Reference
     # definitions or nodes.
     def definedtype
         unless defined? @definedtype
-            type = self.type.to_s.downcase
-            name = self.title
-            case type
-            when "class": # look for host classes
+            case self.type
+            when "Class": # look for host classes
                 if self.title == :main
                     tmp = @scope.findclass("")
                 else
                     tmp = @scope.findclass(self.title)
                 end
-            when "node": # look for node definitions
+            when "Node": # look for node definitions
                 tmp = @scope.parser.nodes[self.title]
             else # normal definitions
                 # We have to swap these variables around so the errors are right.
-                name = type
-                type = "type"
                 tmp = @scope.finddefine(self.type)
             end
 
             if tmp
                 @definedtype = tmp
             else
-                fail Puppet::ParseError, "Could not find resource %s '%s'" % [type, name]
+                fail Puppet::ParseError, "Could not find resource '%s'" % self
             end
         end
 
@@ -67,13 +65,6 @@ class Puppet::Parser::Resource::Reference
         return [type.to_s,title.to_s]
     end
 
-    def to_s
-        unless defined? @namestring
-            @namestring = "%s[%s]" % [type.split("::").collect { |s| s.capitalize }.join("::"), title]
-        end
-        @namestring
-    end
-
     def typeclass
         unless defined? @typeclass
             if tmp = builtintype || definedtype
diff --git a/lib/puppet/rails/host.rb b/lib/puppet/rails/host.rb
index e0a26bdd4..9ecacdd6f 100644
--- a/lib/puppet/rails/host.rb
+++ b/lib/puppet/rails/host.rb
@@ -29,9 +29,7 @@ class Puppet::Rails::Host < ActiveRecord::Base
 
     # Store our host in the database.
     def self.store(node, resources)
-        unless name = hash[:name]
-            raise ArgumentError, "You must specify the hostname for storage"
-        end
+        raise ArgumentError, "You must specify the hostname for storage" unless name = hash[:name]
 
         args = {}
 
diff --git a/lib/puppet/resource_reference.rb b/lib/puppet/resource_reference.rb
index 5e4cc649e..479de6127 100644
--- a/lib/puppet/resource_reference.rb
+++ b/lib/puppet/resource_reference.rb
@@ -11,8 +11,11 @@ class Puppet::ResourceReference
     attr_accessor :title, :configuration
 
     def initialize(type, title)
-        @title = title
-        self.type = type
+        # This will set @type if it looks like a resource reference.
+        self.title = title
+
+        # Don't override whatever was done by setting the title.
+        self.type = type if self.type.nil?
         
         @builtin_type = nil
     end
@@ -30,9 +33,24 @@ class Puppet::ResourceReference
         end
     end
 
+    # If the title has square brackets, treat it like a reference and
+    # set things appropriately; else, just set it.
+    def title=(value)
+        if value =~ /^(.+)\[(.+)\]$/
+            self.type = $1
+            @title = $2
+        else
+            @title = value
+        end
+    end
+
     # Canonize the type so we know it's always consistent.
     def type=(value)
-        @type = value.to_s.split("::").collect { |s| s.capitalize }.join("::")
+        if value.nil? or value.to_s.downcase == "component"
+            @type = "Class"
+        else
+            @type = value.to_s.split("::").collect { |s| s.capitalize }.join("::")
+        end
     end
 
     # Convert to the standard way of referring to resources.
diff --git a/lib/puppet/transportable.rb b/lib/puppet/transportable.rb
index 6a573489c..a448e28a5 100644
--- a/lib/puppet/transportable.rb
+++ b/lib/puppet/transportable.rb
@@ -1,4 +1,5 @@
 require 'puppet'
+require 'puppet/resource_reference'
 require 'yaml'
 
 module Puppet
@@ -35,13 +36,9 @@ module Puppet
 
         def ref
             unless defined? @ref
-                if @type == :component
-                    @ref = @name
-                else
-                    @ref = "%s[%s]" % [type_capitalized, name]
-                end
+                @ref = Puppet::ResourceReference.new(@type, @name)
             end
-            @ref
+            @ref.to_s
         end
 
         def tags
@@ -50,18 +47,10 @@ module Puppet
 
         # Convert a defined type into a component.
         def to_component
-            tmpname = nil
-
-            # Nodes have the same name and type
-            if self.name
-                tmpname = "%s[%s]" % [type_capitalized, self.name]
-            else
-                tmpname = @type
-            end
-            trans = TransObject.new(tmpname, :component)
+            trans = TransObject.new(ref, :component)
             @params.each { |param,value|
                 next unless Puppet::Type::Component.validattr?(param)
-                Puppet.debug "Defining %s on %s of type %s" % [param,@name,@type]
+                Puppet.debug "Defining %s on %s" % [param, ref]
                 trans[param] = value
             }
             Puppet::Type::Component.create(trans)
@@ -90,14 +79,7 @@ module Puppet
         end
 
         def to_ref
-            unless defined? @res_ref
-                if self.type and self.name
-                    @res_ref = "%s[%s]" % [type_capitalized, self.name]
-                else
-                    @res_ref = nil
-                end
-            end
-            @res_ref
+            ref
         end
 
         def to_type
@@ -110,11 +92,6 @@ module Puppet
 
             return retobj
         end
-
-        # Return the type fully capitalized correctly.
-        def type_capitalized
-            type.to_s.split("::").collect { |s| s.capitalize }.join("::")
-        end
     end
 
     # Just a linear container for objects.  Behaves mostly like an array, except
@@ -230,32 +207,21 @@ module Puppet
         end
 
         def to_ref
+            return nil unless self.type and self.name
             unless defined? @ref
-                if self.type and self.name
-                    @ref = "%s[%s]" % [self.type, self.name]
-                else
-                    @ref = nil
-                end
+                @ref = Puppet::ResourceReference.new(self.type, self.name)
             end
-            @ref
+            @ref.to_s
         end
 
         def to_type
-            unless defined? @type
-                Puppet.debug "TransBucket '%s' has no type" % @name
-            end
+            Puppet.debug("TransBucket '%s' has no type" % @name) unless defined? @type
 
             # Nodes have the same name and type
-            if self.name
-                tmpname = "%s[%s]" % [@type, self.name]
-            else
-                tmpname = @type
-            end
-            trans = TransObject.new(tmpname, :component)
+            trans = TransObject.new(to_ref, :component)
             if defined? @parameters
                 @parameters.each { |param,value|
-                    Puppet.debug "Defining %s on %s of type %s" %
-                        [param,@name,@type]
+                    Puppet.debug "Defining %s on %s" % [param, to_ref]
                     trans[param] = value
                 }
             end
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index 39a26f1fa..31ffe3bfb 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -10,6 +10,7 @@ require 'puppet/metatype/manager'
 require 'puppet/util/errors'
 require 'puppet/util/log_paths'
 require 'puppet/util/logging'
+require 'puppet/resource_reference'
 
 # see the bottom of the file for the rest of the inclusions
 
diff --git a/lib/puppet/type/component.rb b/lib/puppet/type/component.rb
index 7aa24a302..6dc90596d 100644
--- a/lib/puppet/type/component.rb
+++ b/lib/puppet/type/component.rb
@@ -48,17 +48,6 @@ Puppet::Type.newtype(:component) do
         @children.each { |child| yield child }
     end
 
-    # flatten all children, sort them, and evaluate them in order
-    # this is only called on one component over the whole system
-    # this also won't work with scheduling, but eh
-    def evaluate
-        raise "Component#evaluate is deprecated"
-        self.finalize unless self.finalized?
-        transaction = Puppet::Transaction.new(self)
-        transaction.component = self
-        return transaction
-    end
-
     # Do all of the polishing off, mostly doing autorequires and making
     # dependencies.  This will get run once on the top-level component,
     # and it will do everything necessary.
@@ -97,10 +86,10 @@ Puppet::Type.newtype(:component) do
         @children = []
         super
 
-        # If the title isn't a full resource reference, assume
-        # we're a class and make an alias for that.
-        unless @title.to_s.include?("[")
-            self.class.alias("class[%s]" % @title, self)
+        @reference = Puppet::ResourceReference.new(:component, @title)
+
+        unless self.class[@reference.to_s]
+            self.class.alias(@reference.to_s, self)
         end
     end
 
@@ -123,16 +112,16 @@ Puppet::Type.newtype(:component) do
     
     # Component paths are special because they function as containers.
     def pathbuilder
-        tmp = []
-        myname = ""
-        if self.title =~ /^class\[(.+)\]$/i
+        if @reference.type == "Class"
             # 'main' is the top class, so we want to see '//' instead of
             # its name.
-            unless $1 == "main"
-                myname = $1
+            if @reference.title == "main"
+                myname = ""
+            else
+                myname = @reference.title
             end
         else
-            myname = self.title
+            myname = @reference.to_s
         end
         if p = self.parent
             return [p.pathbuilder, myname]
@@ -142,21 +131,12 @@ Puppet::Type.newtype(:component) do
     end
 
     def ref
-        title
+        @reference.to_s
     end
 
-    # We have a different way of setting the title
+    # We want our title to just be the whole reference, rather than @title.
     def title
-        unless defined? @title
-            if self[:type] == self[:name] # this is the case for classes
-                @title = self[:type]
-            elsif self[:name] =~ /\[.+\]/ # most components already have ref info in the name
-                @title = self[:name]
-            else # else, set it up
-                @title = "%s[%s]" % [self[:type].capitalize, self[:name]]
-            end
-        end
-        return @title
+        @reference.to_s
     end
 
     def refresh
@@ -169,10 +149,6 @@ Puppet::Type.newtype(:component) do
     end
 
     def to_s
-        if self.title =~ /\[/
-            return self.title
-        else
-            return "component(%s)" % self.title
-        end
+        @reference.to_s
     end
 end