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.

2 files changed, 42 insertions, 33 deletions

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
-