Make sure node are referenced by their names

This patch uses the unused AST::HostName as the only way to reference
a node in the AST nodes array.
The AST::HostName respect the hash properties of the underlying
string, to keep the O(1) hash properties.

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

3 files changed, 191 insertions, 17 deletions

diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb
index 8315b8066..38b753b28 100755
--- a/spec/unit/parser/ast/leaf.rb
+++ b/spec/unit/parser/ast/leaf.rb
@@ -136,7 +136,9 @@ end
 describe Puppet::Parser::AST::HostName do
     before :each do
         @scope = stub 'scope'
-        @value = stub 'value', :is_a? => true, :=~ => true
+        @value = stub 'value', :=~ => false
+        @value.stubs(:to_s).returns(@value)
+        @value.stubs(:downcase).returns(@value)
         @host = Puppet::Parser::AST::HostName.new( :value => @value)
     end
 
@@ -144,7 +146,48 @@ describe Puppet::Parser::AST::HostName do
         lambda { Puppet::Parser::AST::HostName.new( :value => "not an hostname!" ) }.should raise_error
     end
 
+    it "should stringify the value" do
+        value = stub 'value', :=~ => false
+
+        value.expects(:to_s).returns("test")
+
+        Puppet::Parser::AST::HostName.new(:value => value)
+    end
+
+    it "should downcase the value" do
+        value = stub 'value', :=~ => false
+        value.stubs(:to_s).returns("UPCASED")
+        host = Puppet::Parser::AST::HostName.new(:value => value)
+
+        host.value == "upcased"
+    end
+
     it "should evaluate to its value" do
         @host.evaluate(@scope).should == @value
     end
+
+    it "should implement to_classname" do
+        @host.should respond_to(:to_classname)
+    end
+
+    it "should return the downcased nodename as classname" do
+        host = Puppet::Parser::AST::HostName.new( :value => "KLASSNAME" )
+        host.to_classname.should == "klassname"
+    end
+
+    it "should delegate eql? to the underlying value if it is an HostName" do
+        @value.expects(:eql?).with("value")
+        @host.eql?("value")
+    end
+
+    it "should delegate eql? to the underlying value if it is not an HostName" do
+        value = stub 'compared', :is_a? => true, :value => "value"
+        @value.expects(:eql?).with("value")
+        @host.eql?(value)
+    end
+
+    it "should delegate hash to the underlying value" do
+        @value.expects(:hash)
+        @host.hash
+    end
 end
diff --git a/spec/unit/parser/loaded_code.rb b/spec/unit/parser/loaded_code.rb
index d2986bf2c..d33bda9a9 100644
--- a/spec/unit/parser/loaded_code.rb
+++ b/spec/unit/parser/loaded_code.rb
@@ -37,49 +37,49 @@ describe Puppet::Parser::LoadedCode do
     describe "when finding a qualified instance" do
         it "should return any found instance if the instance name is fully qualified" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar", "yay"
-            loader.find("namespace", "::foo::bar", :node).should == "yay"
+            loader.add_hostclass "foo::bar", "yay"
+            loader.find("namespace", "::foo::bar", :hostclass).should == "yay"
         end
 
         it "should return nil if the instance name is fully qualified and no such instance exists" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.find("namespace", "::foo::bar", :node).should be_nil
+            loader.find("namespace", "::foo::bar", :hostclass).should be_nil
         end
 
         it "should return the partially qualified object if it exists in the provided namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar::baz", "yay"
-            loader.find("foo", "bar::baz", :node).should == "yay"
+            loader.add_hostclass "foo::bar::baz", "yay"
+            loader.find("foo", "bar::baz", :hostclass).should == "yay"
         end
 
         it "should return the unqualified object if it exists in the provided namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar", "yay"
-            loader.find("foo", "bar", :node).should == "yay"
+            loader.add_hostclass "foo::bar", "yay"
+            loader.find("foo", "bar", :hostclass).should == "yay"
         end
 
         it "should return the unqualified object if it exists in the parent namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar", "yay"
-            loader.find("foo::bar::baz", "bar", :node).should == "yay"
+            loader.add_hostclass "foo::bar", "yay"
+            loader.find("foo::bar::baz", "bar", :hostclass).should == "yay"
         end
 
         it "should should return the partially qualified object if it exists in the parent namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar::baz", "yay"
-            loader.find("foo::bar", "bar::baz", :node).should == "yay"
+            loader.add_hostclass "foo::bar::baz", "yay"
+            loader.find("foo::bar", "bar::baz", :hostclass).should == "yay"
         end
 
         it "should return the qualified object if it exists in the root namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar::baz", "yay"
-            loader.find("foo::bar", "foo::bar::baz", :node).should == "yay"
+            loader.add_hostclass "foo::bar::baz", "yay"
+            loader.find("foo::bar", "foo::bar::baz", :hostclass).should == "yay"
         end
 
         it "should return nil if the object cannot be found" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_node "foo::bar::baz", "yay"
-            loader.find("foo::bar", "eh", :node).should be_nil
+            loader.add_hostclass "foo::bar::baz", "yay"
+            loader.find("foo::bar", "eh", :hostclass).should be_nil
         end
     end
 
@@ -110,4 +110,47 @@ describe Puppet::Parser::LoadedCode do
     it "should indicate whether no nodes are defined" do
         Puppet::Parser::LoadedCode.new.should_not be_nodes
     end
+
+    describe "when adding nodes" do
+        it "should create an HostName if nodename is a string" do
+            Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo")
+            loader = Puppet::Parser::LoadedCode.new
+            loader.add_node("foo", "bar")
+        end
+
+        it "should not create an HostName if nodename is an HostName" do
+            name = Puppet::Parser::AST::HostName.new(:value => "foo")
+
+            Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo").never
+
+            loader = Puppet::Parser::LoadedCode.new
+            loader.add_node(name, "bar")
+        end
+    end
+
+    describe "when finding nodes" do
+        it "should create an HostName if nodename is a string" do
+            Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo")
+            loader = Puppet::Parser::LoadedCode.new
+            loader.node("foo")
+        end
+
+        it "should not create an HostName if nodename is an HostName" do
+            name = Puppet::Parser::AST::HostName.new(:value => "foo")
+
+            Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo").never
+
+            loader = Puppet::Parser::LoadedCode.new
+            loader.node(name)
+        end
+
+        it "should be able to find nobe by HostName" do
+            namein = Puppet::Parser::AST::HostName.new(:value => "foo")
+            nameout = Puppet::Parser::AST::HostName.new(:value => "foo")
+            loader = Puppet::Parser::LoadedCode.new
+
+            loader.add_node(namein, "bar")
+            loader.node(nameout) == "bar"
+        end
+    end
 end
diff --git a/spec/unit/parser/parser.rb b/spec/unit/parser/parser.rb
index 44e4bedea..46a582923 100755
--- a/spec/unit/parser/parser.rb
+++ b/spec/unit/parser/parser.rb
@@ -7,7 +7,8 @@ describe Puppet::Parser do
     ast = Puppet::Parser::AST
 
     before :each do
-        @parser = Puppet::Parser::Parser.new :environment => "development"
+        @loaded_code = Puppet::Parser::LoadedCode.new
+        @parser = Puppet::Parser::Parser.new :environment => "development", :loaded_code => @loaded_code
         @true_ast = Puppet::Parser::AST::Boolean.new :value => true
     end
 
@@ -225,4 +226,91 @@ describe Puppet::Parser do
             @parser.ast(Puppet::Parser::AST::Definition)
         end
     end
+
+    describe "when creating a node" do
+        before :each do
+            @lexer = stub 'lexer'
+            @lexer.stubs(:getcomment)
+            @parser.stubs(:lexer).returns(@lexer)
+            @node = stub_everything 'node'
+            @parser.stubs(:ast).returns(@node)
+            @parser.stubs(:node).returns(nil)
+
+            @nodename = stub 'nodename', :is_a? => false, :to_classname => "node"
+            @nodename.stubs(:is_a?).with(Puppet::Parser::AST::HostName).returns(true)
+        end
+
+        it "should get the lexer stacked comments" do
+            @lexer.expects(:getcomment)
+
+            @parser.newnode(@nodename)
+        end
+
+        it "should create an HostName if needed" do
+            Puppet::Parser::AST::HostName.expects(:new).with(:value => "node").returns(@nodename)
+
+            @parser.newnode("node")
+        end
+
+        it "should raise an error if the node already exists" do
+            @loaded_code.stubs(:node).with(@nodename).returns(@node)
+
+            lambda { @parser.newnode(@nodename) }.should raise_error
+        end
+
+        it "should store the created node in the loaded code" do
+            @loaded_code.expects(:add_node).with(@nodename, @node)
+
+            @parser.newnode(@nodename)
+        end
+
+        it "should create the node with code if provided" do
+            @parser.stubs(:ast).with { |*args| args[1][:code] == :code }.returns(@node)
+
+            @parser.newnode(@nodename, :code => :code)
+        end
+
+        it "should create the node with a parentclass if provided" do
+            @parser.stubs(:ast).with { |*args| args[1][:parent] == :parent }.returns(@node)
+
+            @parser.newnode(@nodename, :parent => :parent)
+        end
+
+        it "should set the node classname from the HostName" do
+            @nodename.stubs(:to_classname).returns(:classname)
+
+            @node.expects(:classname=).with(:classname)
+
+            @parser.newnode(@nodename)
+        end
+
+        it "should return an array of nodes" do
+            @parser.newnode(@nodename).should == [@node]
+        end
+    end
+
+    describe "when retrieving a specific node" do
+        it "should delegate to the loaded_code node" do
+            @loaded_code.expects(:node).with("node")
+
+            @parser.node("node")
+        end
+    end
+
+    describe "when retrieving a specific class" do
+        it "should delegate to the loaded code" do
+            @loaded_code.expects(:hostclass).with("class")
+
+            @parser.hostclass("class")
+        end
+    end
+
+    describe "when retrieving a specific definitions" do
+        it "should delegate to the loaded code" do
+            @loaded_code.expects(:definition).with("define")
+
+            @parser.definition("define")
+        end
+    end
+
  end