Fixing #2596 - Node, Class, Definition are not AST

This commit extracts these three classes into a single
ResourceType class in the Parser heirarchy, now completely
independent of the AST heirarchy.

Most of the other changes are just changing the interface
to the new class, which is greatly simplified over the previous
classes.

This opens up the possibility of drastically simplifying a lot
of this other code, too -- in particular, replacing the reference
to the parser with a reference to the (soon to be renamed)
LoadedCode class.

Signed-off-by: Luke Kanies <luke@madstop.com>

1 files changed, 111 insertions, 107 deletions

diff --git a/spec/unit/parser/loaded_code.rb b/spec/unit/parser/loaded_code.rb
index 75f2bc7e2..9b72dab0d 100644
--- a/spec/unit/parser/loaded_code.rb
+++ b/spec/unit/parser/loaded_code.rb
@@ -3,23 +3,92 @@
 require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/parser/loaded_code'
+require 'puppet/parser/resource_type'
 
 describe Puppet::Parser::LoadedCode do
+    before do
+        @instance = Puppet::Parser::ResourceType.new(:hostclass, "foo")
+        @code = Puppet::Parser::LoadedCode.new
+    end
+
+    it "should be able to add a resource type" do
+        Puppet::Parser::LoadedCode.new.should respond_to(:add)
+    end
+
+    it "should consider '<<' to be an alias to 'add' but should return self" do
+        loader = Puppet::Parser::LoadedCode.new
+        loader.expects(:add).with "foo"
+        loader.expects(:add).with "bar"
+        loader << "foo" << "bar"
+    end
+
+    it "should set itself as the code collection for added resource types" do
+        loader = Puppet::Parser::LoadedCode.new
+
+        node = Puppet::Parser::ResourceType.new(:node, "foo")
+
+        @code.add(node)
+        @code.node("foo").should equal(node)
+
+        node.code_collection.should equal(@code)
+    end
+
+    it "should store node resource types as nodes" do
+        node = Puppet::Parser::ResourceType.new(:node, "foo")
+
+        @code.add(node)
+        @code.node("foo").should equal(node)
+    end
+
+    it "should store hostclasses as hostclasses" do
+        klass = Puppet::Parser::ResourceType.new(:hostclass, "foo")
+
+        @code.add(klass)
+        @code.hostclass("foo").should equal(klass)
+    end
+
+    it "should store definitions as definitions" do
+        define = Puppet::Parser::ResourceType.new(:definition, "foo")
+
+        @code.add(define)
+        @code.definition("foo").should equal(define)
+    end
+
     %w{hostclass node definition}.each do |data|
         it "should have a method for adding a #{data}" do
             Puppet::Parser::LoadedCode.new.should respond_to("add_" + data)
         end
 
+        it "should use the name of the instance to add it" do
+            loader = Puppet::Parser::LoadedCode.new
+            loader.send("add_#{data}", @instance)
+            loader.send(data, @instance.name).should equal(@instance)
+        end
+
+        it "should fail to add a #{data} when one already exists" do
+            loader = Puppet::Parser::LoadedCode.new
+            loader.add @instance
+            lambda { loader.add(@instance) }.should raise_error(Puppet::ParseError)
+        end
+
+        it "should return the added #{data}" do
+            loader = Puppet::Parser::LoadedCode.new
+
+            loader.add(@instance).should equal(@instance)
+        end
+
         it "should be able to retrieve #{data} by name" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.send("add_" + data, "foo", "bar")
-            loader.send(data, "foo").should == "bar"
+            instance = Puppet::Parser::ResourceType.new(data, "bar")
+            loader.add instance
+            loader.send(data, "bar").should equal(instance)
         end
 
         it "should retrieve #{data} insensitive to case" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.send("add_" + data, "Foo", "bar")
-            loader.send(data, "fOo").should == "bar"
+            instance = Puppet::Parser::ResourceType.new(data, "Bar")
+            loader.add instance
+            loader.send(data, "bAr").should equal(instance)
         end
 
         it "should return nil when asked for a #{data} that has not been added" do
@@ -29,16 +98,18 @@ describe Puppet::Parser::LoadedCode do
         it "should be able to retrieve all #{data}s" do
             plurals = { "hostclass" => "hostclasses", "node" => "nodes", "definition" => "definitions" }
             loader = Puppet::Parser::LoadedCode.new
-            loader.send("add_" + data , "foo", "bar")
-            loader.send(plurals[data]).should == { "foo" => "bar" }
+            instance = Puppet::Parser::ResourceType.new(data, "foo")
+            loader.add instance
+            loader.send(plurals[data]).should == { "foo" => instance }
         end
     end
 
     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_hostclass "foo::bar", "yay"
-            loader.find("namespace", "::foo::bar", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar")
+            loader.add instance
+            loader.find("namespace", "::foo::bar", :hostclass).should equal(instance)
         end
 
         it "should return nil if the instance name is fully qualified and no such instance exists" do
@@ -48,37 +119,43 @@ describe Puppet::Parser::LoadedCode do
 
         it "should return the partially qualified object if it exists in the provided namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar::baz", "yay"
-            loader.find("foo", "bar::baz", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar::baz")
+            loader.add instance
+            loader.find("foo", "bar::baz", :hostclass).should equal(instance)
         end
 
         it "should return the unqualified object if it exists in the provided namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar", "yay"
-            loader.find("foo", "bar", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar")
+            loader.add instance
+            loader.find("foo", "bar", :hostclass).should equal(instance)
         end
 
         it "should return the unqualified object if it exists in the parent namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar", "yay"
-            loader.find("foo::bar::baz", "bar", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar")
+            loader.add instance
+            loader.find("foo::bar::baz", "bar", :hostclass).should equal(instance)
         end
 
         it "should should return the partially qualified object if it exists in the parent namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar::baz", "yay"
-            loader.find("foo::bar", "bar::baz", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar::baz")
+            loader.add instance
+            loader.find("foo::bar", "bar::baz", :hostclass).should equal(instance)
         end
 
         it "should return the qualified object if it exists in the root namespace" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar::baz", "yay"
-            loader.find("foo::bar", "foo::bar::baz", :hostclass).should == "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar::baz")
+            loader.add instance
+            loader.find("foo::bar", "foo::bar::baz", :hostclass).should equal(instance)
         end
 
         it "should return nil if the object cannot be found" do
             loader = Puppet::Parser::LoadedCode.new
-            loader.add_hostclass "foo::bar::baz", "yay"
+            instance = Puppet::Parser::ResourceType.new(:hostclass, "foo::bar::baz")
+            loader.add instance
             loader.find("foo::bar", "eh", :hostclass).should be_nil
         end
     end
@@ -103,7 +180,7 @@ describe Puppet::Parser::LoadedCode do
 
     it "should indicate whether any nodes are defined" do
         loader = Puppet::Parser::LoadedCode.new
-        loader.add_node("foo", "bar")
+        loader.add_node(Puppet::Parser::ResourceType.new(:node, "foo"))
         loader.should be_nodes
     end
 
@@ -111,105 +188,32 @@ describe Puppet::Parser::LoadedCode 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
         before :each do
             @loader = Puppet::Parser::LoadedCode.new
-
-            @nodename1 = stub 'nodename1', :is_a? => true
-            @node1 = stub 'node1'
-            @nodename2 = stub 'nodename2', :is_a? => true
-            @node2 = stub 'node2'
-        end
-        it "should create an HostName if nodename is a string" do
-            Puppet::Parser::AST::HostName.expects(:new).with(:value => "foo")
-
-            @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.node(name)
-        end
-
-        it "should be able to find node by HostName" do
-            namein = Puppet::Parser::AST::HostName.new(:value => "foo")
-            nameout = Puppet::Parser::AST::HostName.new(:value => "foo")
-
-            @loader.add_node(namein, "bar")
-            @loader.node(nameout).should == "bar"
         end
 
-        it "should be able to find node by HostName strict equality" do
-            namein = Puppet::Parser::AST::HostName.new(:value => "foo")
-            nameout = Puppet::Parser::AST::HostName.new(:value => "foo")
+        it "should return any node whose name exactly matches the provided node name" do
+            node = Puppet::Parser::ResourceType.new(:node, "foo")
+            @loader << node
 
-            @loader.add_node(namein, "bar")
-            @loader.node_exists?(nameout).should == "bar"
+            @loader.node("foo").should equal(node)
         end
 
-        it "should not use node name matching when finding with strict node HostName" do
-            name1 = Puppet::Parser::AST::HostName.new(:value => "foo")
-            name2 = Puppet::Parser::AST::HostName.new(:value => Puppet::Parser::AST::Regex.new(:value => /foo/))
+        it "should return the first regex node whose regex matches the provided node name" do
+            node1 = Puppet::Parser::ResourceType.new(:node, /\w/)
+            node2 = Puppet::Parser::ResourceType.new(:node, /\d/)
+            @loader << node1 << node2
 
-            @loader.add_node(name1, "bar")
-            @loader.add_node(name2, "baz")
-            @loader.node_exists?(name1).should == "bar"
+            @loader.node("foo10").should equal(node1)
         end
 
-        it "should return the first matching regex nodename" do
-            @nodename1.stubs(:regex?).returns(true)
-            @nodename1.stubs(:match).returns(true)
-            @nodename2.stubs(:regex?).returns(true)
-            @nodename2.stubs(:match).returns(true)
-
-            @loader.add_node(@nodename1, @node1)
-            @loader.add_node(@nodename2, @node2)
-
-            @loader.node("test").should == @node1
-        end
-
-        it "should not scan non-regex node" do
-            @nodename1.stubs(:regex?).returns(true)
-            @nodename1.stubs(:match).returns(false)
-            @nodename2.stubs(:regex?).returns(false)
-            @nodename2.expects(:match).never
-
-            @loader.add_node(@nodename1,@node1)
-            @loader.add_node(@nodename2,@node2)
-
-            @loader.node("test")
-        end
-
-        it "should prefer non-regex nodes to regex nodes" do
-            @nodename1.stubs(:regex?).returns(false)
-            @nodename1.expects(:match).never
-            @nodename2.stubs(:regex?).returns(true)
-            @nodename2.expects(:match).never
-
-            @loader.add_node(@nodename1,@node1)
-            @loader.add_node(@nodename2,@node2)
+        it "should preferentially return a node whose name is string-equal over returning a node whose regex matches a provided name" do
+            node1 = Puppet::Parser::ResourceType.new(:node, /\w/)
+            node2 = Puppet::Parser::ResourceType.new(:node, "foo")
+            @loader << node1 << node2
 
-            @loader.node(@nodename1)
+            @loader.node("foo").should equal(node2)
         end
     end
 end