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 <[email protected]> --- lib/puppet/parser/ast/leaf.rb | 16 +- lib/puppet/parser/ast/node.rb | 5 - lib/puppet/parser/grammar.ra | 10 +- lib/puppet/parser/loaded_code.rb | 9 +- lib/puppet/parser/parser.rb | 1129 ++++++++++++++--------------- lib/puppet/parser/parser_support.rb | 4 +- spec/unit/parser/ast/leaf.rb | 45 ++- spec/unit/parser/loaded_code.rb | 73 ++- spec/unit/parser/parser.rb | 90 +++- test/lib/puppettest/support/collection.rb | 2 +- 10 files changed, 786 insertions(+), 597 deletions(-) diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb index de5876e..866d2b6 100644 --- a/lib/puppet/parser/ast/leaf.rb +++ b/lib/puppet/parser/ast/leaf.rb @@ -90,11 +90,25 @@ class Puppet::Parser::AST def initialize(hash) super - unless @value =~ %r{^[0-9a-zA-Z\-]+(\.[0-9a-zA-Z\-]+)*$} + @value = @value.to_s.downcase + if @value =~ /[^-\w.]/ raise Puppet::DevError, "'%s' is not a valid hostname" % @value end end + + def to_classname + return @value + end + + def eql?(value) + value = value.value if value.is_a?(HostName) + return @value.eql?(value) + end + + def hash + return @value.hash + end end # A simple variable. This object is only used during interpolation; diff --git a/lib/puppet/parser/ast/node.rb b/lib/puppet/parser/ast/node.rb index faa7fa1..b2d4044 100644 --- a/lib/puppet/parser/ast/node.rb +++ b/lib/puppet/parser/ast/node.rb @@ -11,11 +11,6 @@ class Puppet::Parser::AST::Node < Puppet::Parser::AST::HostClass def initialize(options) @parentclass = nil super - - # Do some validation on the node name - if @name =~ /[^-\w.]/ - raise Puppet::ParseError, "Invalid node name %s" % @name - end end def namespace diff --git a/lib/puppet/parser/grammar.ra b/lib/puppet/parser/grammar.ra index 072f351..ed55d21 100644 --- a/lib/puppet/parser/grammar.ra +++ b/lib/puppet/parser/grammar.ra @@ -684,11 +684,15 @@ classname: NAME { result = val[0][:value] } # Multiple hostnames, as used for node names. These are all literal # strings, not AST objects. -hostnames: hostname - | hostnames COMMA hostname { +hostnames: nodename + | hostnames COMMA nodename { result = val[0] result = [result] unless result.is_a?(Array) - result << val[2][:value] + result << val[2] +} + +nodename: hostname { + result = ast AST::HostName, :value => val[0] } hostname: NAME { result = val[0][:value] } diff --git a/lib/puppet/parser/loaded_code.rb b/lib/puppet/parser/loaded_code.rb index 6c51961..4bb03ee 100644 --- a/lib/puppet/parser/loaded_code.rb +++ b/lib/puppet/parser/loaded_code.rb @@ -14,11 +14,11 @@ class Puppet::Parser::LoadedCode end def add_node(name, code) - @nodes[munge_name(name)] = code + @nodes[check_name(name)] = code end def node(name) - @nodes[munge_name(name)] + @nodes[check_name(name)] end def nodes? @@ -88,4 +88,9 @@ class Puppet::Parser::LoadedCode def munge_name(name) name.to_s.downcase end + + def check_name(name) + name = Puppet::Parser::AST::HostName.new(:value => name) unless name.is_a?(Puppet::Parser::AST::HostName) + name + end end diff --git a/lib/puppet/parser/parser.rb b/lib/puppet/parser/parser.rb index cedc7de..dbe0be4 100644 --- a/lib/puppet/parser/parser.rb +++ b/lib/puppet/parser/parser.rb <Patch Elided> diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/parser_support.rb index e1af2fe..b13fbb4 100644 --- a/lib/puppet/parser/parser_support.rb +++ b/lib/puppet/parser/parser_support.rb @@ -354,7 +354,7 @@ class Puppet::Parser::Parser names = [names] unless names.instance_of?(Array) doc = lexer.getcomment names.collect do |name| - name = name.to_s.downcase + name = AST::HostName.new :value => name unless name.is_a?(AST::HostName) if other = @loaded_code.node(name) error("Node %s is already defined at %s:%s; cannot redefine" % [other.name, other.file, other.line]) end @@ -372,7 +372,7 @@ class Puppet::Parser::Parser args[:parentclass] = options[:parent] end node = ast(AST::Node, args) - node.classname = name + node.classname = name.to_classname @loaded_code.add_node(name, node) node end diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb index 8315b80..38b753b 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 d2986bf..d33bda9 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 44e4bed..46a5829 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 diff --git a/test/lib/puppettest/support/collection.rb b/test/lib/puppettest/support/collection.rb index 43df030..9da15ea 100644 --- a/test/lib/puppettest/support/collection.rb +++ b/test/lib/puppettest/support/collection.rb @@ -18,7 +18,7 @@ module PuppetTest::Support::Collection query = nil assert_nothing_raised("Could not parse '#{str}'") do - query = parser.parse(code).classes[""].code[0].query + query = parser.parse(code).hostclass("").code[0].query end yield str, res, query -- 1.6.0.2 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected] To unsubscribe from this group, send email to [email protected] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
