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.

7 files changed, 96 insertions, 37 deletions

diff --git a/spec/unit/node/configuration.rb b/spec/unit/node/configuration.rb
index 0a8b47fc5..0023e0f2b 100755
--- a/spec/unit/node/configuration.rb
+++ b/spec/unit/node/configuration.rb
@@ -351,6 +351,18 @@ describe Puppet::Node::Configuration, " when functioning as a resource container
         @config.vertices.find { |r| r.ref == @one.ref }.should equal(@one)
     end
 
+    it "should canonize how resources are referred to during retrieval when both type and title are provided" do
+        @config.add_resource(@one)
+
+        @config.resource("me", "one").should equal(@one)
+    end
+
+    it "should canonize how resources are referred to during retrieval when just the title is provided" do
+        @config.add_resource(@one)
+
+        @config.resource("me[one]", nil).should equal(@one)
+    end
+
     it "should not allow two resources with the same resource reference" do
         @config.add_resource(@one)
         proc { @config.add_resource(@dupe) }.should raise_error(ArgumentError)
diff --git a/spec/unit/other/transbucket.rb b/spec/unit/other/transbucket.rb
index 241529ebe..10c551752 100755
--- a/spec/unit/other/transbucket.rb
+++ b/spec/unit/other/transbucket.rb
@@ -14,7 +14,7 @@ describe Puppet::TransBucket do
         resource = nil
         proc { resource = @bucket.to_type }.should_not raise_error
         resource.should be_instance_of(Puppet::Type::Component)
-        resource.title.should == "user[luke]"
+        resource.title.should == "User[luke]"
     end
 
     it "should accept TransObjects into its children list" do
@@ -44,7 +44,14 @@ describe Puppet::TransBucket do
     it "should return the title as its reference" do
         @bucket.name = "luke"
         @bucket.type = "user"
-        @bucket.to_ref.should == "user[luke]"
+        @bucket.to_ref.should == "User[luke]"
+    end
+
+    it "should canonize resource references when the type is 'component'" do
+        @bucket.name = 'something'
+        @bucket.type = 'foo::bar'
+
+        @bucket.to_ref.should == "Foo::Bar[something]"
     end
 end
 
@@ -73,7 +80,7 @@ describe Puppet::TransBucket, " when generating a configuration" do
         @config = @top.to_configuration
 
         @users = %w{top middle bottom}
-        @fakes = %w{fake[bottom] fake[middle] fake[top]}
+        @fakes = %w{Fake[bottom] Fake[middle] Fake[top]}
     end
 
     it "should convert all transportable objects to RAL resources" do
diff --git a/spec/unit/other/transobject.rb b/spec/unit/other/transobject.rb
index eaca855db..830d4923d 100755
--- a/spec/unit/other/transobject.rb
+++ b/spec/unit/other/transobject.rb
@@ -4,6 +4,13 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/transportable'
 
+describe Puppet::TransObject do
+    it "should canonize resource references" do
+        resource = Puppet::TransObject.new("me", "foo::bar")
+        resource.ref.should == 'Foo::Bar[me]'
+    end
+end
+
 describe Puppet::TransObject, " when serializing" do
     before do
         @resource = Puppet::TransObject.new("/my/file", "file")
diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb
index 72c4c627c..c0e5f2298 100755
--- a/spec/unit/parser/collector.rb
+++ b/spec/unit/parser/collector.rb
@@ -7,7 +7,7 @@ require 'puppet/parser/collector'
 describe Puppet::Parser::Collector, "when initializing" do
     before do
         @scope = mock 'scope'
-        @resource_type = mock 'resource_type'
+        @resource_type = 'resource_type'
         @form = :exported
         @vquery = mock 'vquery'
         @equery = mock 'equery'
@@ -20,7 +20,7 @@ describe Puppet::Parser::Collector, "when initializing" do
     end
 
     it "should require a resource type" do
-        @collector.type.should equal(@resource_type)
+        @collector.type.should == 'Resource_type'
     end
 
     it "should only accept :virtual or :exported as the collector form" do
@@ -34,6 +34,11 @@ describe Puppet::Parser::Collector, "when initializing" do
     it "should accept an optional exported query" do
         @collector.equery.should equal(@equery)
     end
+
+    it "should canonize the type name" do
+        @collector = Puppet::Parser::Collector.new(@scope, "resource::type", @equery, @vquery, @form)
+        @collector.type.should == "Resource::Type"
+    end
 end
 
 describe Puppet::Parser::Collector, "when collecting specific virtual resources" do
@@ -95,15 +100,15 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
         @scope = mock 'scope'
         @compile = mock 'compile'
         @scope.stubs(:compile).returns(@compile)
-        @resource_type = :mytype
+        @resource_type = "Mytype"
         @vquery = proc { |res| true }
 
         @collector = Puppet::Parser::Collector.new(@scope, @resource_type, nil, @vquery, :virtual)
     end
 
     it "should find all resources matching the vquery" do
-        one = stub 'one', :type => :mytype, :virtual? => true
-        two = stub 'two', :type => :mytype, :virtual? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true
+        two = stub 'two', :type => "Mytype", :virtual? => true
 
         one.stubs(:virtual=)
         two.stubs(:virtual=)
@@ -114,7 +119,7 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     end
 
     it "should mark all matched resources as non-virtual" do
-        one = stub 'one', :type => :mytype, :virtual? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true
 
         one.expects(:virtual=).with(false)
 
@@ -124,8 +129,8 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     end
 
     it "should return matched resources" do
-        one = stub 'one', :type => :mytype, :virtual? => true
-        two = stub 'two', :type => :mytype, :virtual? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true
+        two = stub 'two', :type => "Mytype", :virtual? => true
 
         one.stubs(:virtual=)
         two.stubs(:virtual=)
@@ -136,8 +141,8 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     end
 
     it "should return all resources of the correct type if there is no virtual query" do
-        one = stub 'one', :type => :mytype, :virtual? => true
-        two = stub 'two', :type => :mytype, :virtual? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true
+        two = stub 'two', :type => "Mytype", :virtual? => true
 
         one.expects(:virtual=).with(false)
         two.expects(:virtual=).with(false)
@@ -150,7 +155,7 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     end
 
     it "should not return or mark resources of a different type" do
-        one = stub 'one', :type => :mytype, :virtual? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true
         two = stub 'two', :type => :other, :virtual? => true
 
         one.expects(:virtual=).with(false)
@@ -162,7 +167,7 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     end
 
     it "should not return or mark non-virtual resources" do
-        one = stub 'one', :type => :mytype, :virtual? => false
+        one = stub 'one', :type => "Mytype", :virtual? => false
         two = stub 'two', :type => :other, :virtual? => false
 
         one.expects(:virtual=).never
@@ -176,8 +181,8 @@ describe Puppet::Parser::Collector, "when collecting virtual resources" do
     it "should not return or mark non-matching resources" do
         @collector.vquery = proc { |res| res.name == :one }
 
-        one = stub 'one', :name => :one, :type => :mytype, :virtual? => true
-        two = stub 'two', :name => :two, :type => :mytype, :virtual? => true
+        one = stub 'one', :name => :one, :type => "Mytype", :virtual? => true
+        two = stub 'two', :name => :two, :type => "Mytype", :virtual? => true
 
         one.expects(:virtual=).with(false)
         two.expects(:virtual=).never
@@ -195,7 +200,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         @scope = stub 'scope', :host => "myhost", :debug => nil
         @compile = mock 'compile'
         @scope.stubs(:compile).returns(@compile)
-        @resource_type = :mytype
+        @resource_type = "Mytype"
         @equery = "test = true"
         @vquery = proc { |r| true }
 
@@ -225,8 +230,8 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
     it "should return all matching resources from the current compile" do
         stub_rails(true)
 
-        one = stub 'one', :type => :mytype, :virtual? => true, :exported? => true
-        two = stub 'two', :type => :mytype, :virtual? => true, :exported? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => true
+        two = stub 'two', :type => "Mytype", :virtual? => true, :exported? => true
 
         one.stubs(:exported=)
         one.stubs(:virtual=)
@@ -241,7 +246,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
     it "should mark all returned resources as not exported" do
         stub_rails(true)
 
-        one = stub 'one', :type => :mytype, :virtual? => true, :exported? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => true
 
         one.expects(:exported=).with(false)
         one.stubs(:virtual=)
@@ -254,7 +259,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
     it "should mark all returned resources as not virtual" do
         stub_rails(true)
 
-        one = stub 'one', :type => :mytype, :virtual? => true, :exported? => true
+        one = stub 'one', :type => "Mytype", :virtual? => true, :exported? => true
 
         one.stubs(:exported=)
         one.expects(:virtual=).with(false)
@@ -268,7 +273,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        one = stub 'one', :restype => :mytype, :title => "one", :virtual? => true, :exported? => true
+        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => true, :exported? => true
         Puppet::Rails::Resource.stubs(:find).returns([one])
 
         resource = mock 'resource'
@@ -288,7 +293,7 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        one = stub 'one', :restype => :mytype, :title => "one", :virtual? => true, :exported? => true
+        one = stub 'one', :restype => "Mytype", :title => "one", :virtual? => true, :exported? => true
         Puppet::Rails::Resource.stubs(:find).returns([one])
 
         resource = mock 'resource'
@@ -308,8 +313,8 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        rails = stub 'one', :restype => :mytype, :title => "one", :virtual? => true, :exported? => true, :id => 1, :ref => "yay"
-        inmemory = stub 'one', :type => :mytype, :virtual? => true, :exported? => true, :rails_id => 2
+        rails = stub 'one', :restype => "Mytype", :title => "one", :virtual? => true, :exported? => true, :id => 1, :ref => "yay"
+        inmemory = stub 'one', :type => "Mytype", :virtual? => true, :exported? => true, :rails_id => 2
 
         Puppet::Rails::Resource.stubs(:find).returns([rails])
 
@@ -327,8 +332,8 @@ describe Puppet::Parser::Collector, "when collecting exported resources" do
         stub_rails()
         Puppet::Rails::Host.stubs(:find_by_name).returns(nil)
 
-        rails = stub 'one', :restype => :mytype, :title => "one", :virtual? => true, :exported? => true, :id => 1, :ref => "yay"
-        inmemory = stub 'one', :type => :mytype, :virtual? => true, :exported? => true, :rails_id => 1
+        rails = stub 'one', :restype => "Mytype", :title => "one", :virtual? => true, :exported? => true, :id => 1, :ref => "yay"
+        inmemory = stub 'one', :type => "Mytype", :virtual? => true, :exported? => true, :rails_id => 1
 
         Puppet::Rails::Resource.stubs(:find).returns([rails])
 
@@ -350,7 +355,7 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co
         @scope = stub 'scope', :host => "myhost", :debug => nil
         @compile = mock 'compile'
         @scope.stubs(:compile).returns(@compile)
-        @resource_type = :mytype
+        @resource_type = "Mytype"
         @equery = nil
         @vquery = proc { |r| true }
 
@@ -390,7 +395,7 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co
     it "should only search for exported resources with the matching type" do
         Puppet::Rails::Resource.stubs(:find).with { |*arguments|
             options = arguments[3]
-            options[:conditions][0].include?("(exported=? AND restype=?)") and options[:conditions][1] == true and options[:conditions][2] == :mytype
+            options[:conditions][0].include?("(exported=? AND restype=?)") and options[:conditions][1] == true and options[:conditions][2] == "Mytype"
         }.returns([])
     end
 
diff --git a/spec/unit/parser/resource/reference.rb b/spec/unit/parser/resource/reference.rb
index 45af3d938..24b70d088 100755
--- a/spec/unit/parser/resource/reference.rb
+++ b/spec/unit/parser/resource/reference.rb
@@ -15,7 +15,7 @@ describe Puppet::Parser::Resource::Reference do
         proc { @type.new(:type => "file") }.should raise_error(Puppet::DevError)
     end
 
-    it "should know when it models a builtin type" do
+    it "should know when it refers to a builtin type" do
         ref = @type.new(:type => "file", :title => "/tmp/yay")
         ref.builtin?.should be_true
         ref.builtintype.should equal(Puppet::Type.type(:file))
@@ -23,13 +23,18 @@ describe Puppet::Parser::Resource::Reference do
 
     it "should return a relationship-style resource reference when asked" do
         ref = @type.new(:type => "file", :title => "/tmp/yay")
-        ref.to_ref.should == ["file", "/tmp/yay"]
+        ref.to_ref.should == ["File", "/tmp/yay"]
     end
 
     it "should return a resource reference string when asked" do
         ref = @type.new(:type => "file", :title => "/tmp/yay")
         ref.to_s.should == "File[/tmp/yay]"
     end
+
+    it "should canonize resource references" do
+        ref = @type.new(:type => "foo::bar", :title => "/tmp/yay")
+        ref.to_s.should == "Foo::Bar[/tmp/yay]"
+    end
 end
 
 describe Puppet::Parser::Resource::Reference, " when modeling defined types" do
@@ -45,22 +50,21 @@ describe Puppet::Parser::Resource::Reference, " when modeling defined types" do
         @compile = Puppet::Parser::Compile.new(@node, @parser)
     end
 
-    it "should be able to model definitions" do
+    it "should be able to find defined types" do
         ref = @type.new(:type => "mydefine", :title => "/tmp/yay", :scope => @compile.topscope)
         ref.builtin?.should be_false
         ref.definedtype.should equal(@definition)
     end
 
-    it "should be able to model classes" do
+    it "should be able to find classes" do
         ref = @type.new(:type => "class", :title => "myclass", :scope => @compile.topscope)
         ref.builtin?.should be_false
         ref.definedtype.should equal(@class)
     end
 
-    it "should be able to model nodes" do
+    it "should be able to find nodes" do
         ref = @type.new(:type => "node", :title => "mynode", :scope => @compile.topscope)
         ref.builtin?.should be_false
         ref.definedtype.object_id.should  == @nodedef.object_id
     end
 end
-
diff --git a/spec/unit/ral/type.rb b/spec/unit/ral/type.rb
index adb40595e..60b99eeb8 100755
--- a/spec/unit/ral/type.rb
+++ b/spec/unit/ral/type.rb
@@ -20,7 +20,7 @@ describe Puppet::Type, " when in a configuration" do
     end
 
     it "should set its parent to its in edge" do
-        @one.parent.ref.should equal(@container.ref)
+        @one.parent.ref.should == @container.ref
     end
 
     after do
diff --git a/spec/unit/resource_reference.rb b/spec/unit/resource_reference.rb
index dad33866c..93eeaa5b8 100755
--- a/spec/unit/resource_reference.rb
+++ b/spec/unit/resource_reference.rb
@@ -16,6 +16,30 @@ describe Puppet::ResourceReference do
     it "should canonize qualified types so all strings are capitalized" do
         Puppet::ResourceReference.new("foo::bar", "foo").type.should == "Foo::Bar"
     end
+
+    it "should set its type to 'Class' and its title to the passed title if the passed type is :component and the title has no square brackets in it" do
+        ref = Puppet::ResourceReference.new(:component, "foo")
+        ref.type.should == "Class"
+        ref.title.should == "foo"
+    end
+
+    it "should interpret the title as a reference and assign appropriately if the type is :component and the title contains square brackets" do
+        ref = Puppet::ResourceReference.new(:component, "foo::bar[yay]")
+        ref.type.should == "Foo::Bar"
+        ref.title.should == "yay"
+    end
+
+    it "should set the type to 'Class' if it is nil and the title contains no square brackets" do
+        ref = Puppet::ResourceReference.new(nil, "yay")
+        ref.type.should == "Class"
+        ref.title.should == "yay"
+    end
+
+    it "should interpret the title as a reference and assign appropriately if the type is nil and the title contains square brackets" do
+        ref = Puppet::ResourceReference.new(nil, "foo::bar[yay]")
+        ref.type.should == "Foo::Bar"
+        ref.title.should == "yay"
+    end
 end
 
 describe Puppet::ResourceReference, "when resolving resources without a configuration" do