+1 On Sep 16, 2009, at 11:19 AM, Brice Figureau wrote:
> > When we are checking if a node exists before creating a new one > we were also trying to match with regex node names, finding matches > where in fact there is no equality. > > Signed-off-by: Brice Figureau <[email protected]> > --- > lib/puppet/parser/loaded_code.rb | 9 +++++-- > lib/puppet/parser/parser_support.rb | 8 +++++- > spec/unit/parser/loaded_code.rb | 37 ++++++++++++++++++++++++ > +--------- > spec/unit/parser/parser.rb | 4 +- > 4 files changed, 41 insertions(+), 17 deletions(-) > > diff --git a/lib/puppet/parser/loaded_code.rb b/lib/puppet/parser/ > loaded_code.rb > index 065fcb9..80a7497 100644 > --- a/lib/puppet/parser/loaded_code.rb > +++ b/lib/puppet/parser/loaded_code.rb > @@ -22,7 +22,7 @@ class Puppet::Parser::LoadedCode > @nodes[name] = code > end > > - def node(name) > + def node_matching(name) > name = check_name(name) > > if node = @nodes[name] > @@ -30,12 +30,15 @@ class Puppet::Parser::LoadedCode > end > > @node_list.each do |nodename| > - n = @nodes[nodename] > - return n if nodename.regex? and nodename.match(name) > + return @nodes[nodename] if nodename.regex? and > nodename.match(name) > end > nil > end > > + def node_named(name) > + @nodes[check_name(name)] > + end > + > def nodes? > @nodes.length > 0 > end > diff --git a/lib/puppet/parser/parser_support.rb b/lib/puppet/parser/ > parser_support.rb > index dfc14e0..41aa9b3 100644 > --- a/lib/puppet/parser/parser_support.rb > +++ b/lib/puppet/parser/parser_support.rb > @@ -98,12 +98,16 @@ class Puppet::Parser::Parser > end > end > > - [:hostclass, :definition, :node, :nodes?].each do |method| > + [:hostclass, :definition, :nodes?].each do |method| > define_method(method) do |*args| > @loaded_code.send(method, *args) > end > end > > + def node(name) > + @loaded_code.node_matching(name) > + end > + > def find_hostclass(namespace, name) > find_or_load(namespace, name, :hostclass) > end > @@ -376,7 +380,7 @@ class Puppet::Parser::Parser > doc = lexer.getcomment > names.collect do |name| > name = AST::HostName.new :value => name unless name.is_a? > (AST::HostName) > - if other = @loaded_code.node(name) > + if other = @loaded_code.node_named(name) > error("Node %s is already defined at %s:%s; cannot > redefine" % [other.name, other.file, other.line]) > end > name = name.to_s if name.is_a?(Symbol) > diff --git a/spec/unit/parser/loaded_code.rb b/spec/unit/parser/ > loaded_code.rb > index ca4b0aa..0972a22 100644 > --- a/spec/unit/parser/loaded_code.rb > +++ b/spec/unit/parser/loaded_code.rb > @@ -5,7 +5,7 @@ require File.dirname(__FILE__) + '/../../spec_helper' > require 'puppet/parser/loaded_code' > > describe Puppet::Parser::LoadedCode do > - %w{hostclass node definition}.each do |data| > + { "hostclass" => "hostclass", "node" => "node_matching", > "definition" => "definition" }.each do |data, method| > it "should have a method for adding a #{data}" do > Puppet::Parser::LoadedCode.new.should respond_to("add_" > + data) > end > @@ -13,17 +13,17 @@ describe Puppet::Parser::LoadedCode do > 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" > + loader.send(method, "foo").should == "bar" > 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" > + loader.send(method, "fOo").should == "bar" > end > > it "should return nil when asked for a #{data} that has not > been added" do > - Puppet::Parser::LoadedCode.new.send(data, "foo").should > be_nil > + Puppet::Parser::LoadedCode.new.send(method, > "foo").should be_nil > end > > it "should be able to retrieve all #{data}s" do > @@ -140,7 +140,7 @@ describe Puppet::Parser::LoadedCode do > it "should create an HostName if nodename is a string" do > Puppet::Parser::AST::HostName.expects(:new).with(:value > => "foo") > > - @loader.node("foo") > + @loader.node_matching("foo") > end > > it "should not create an HostName if nodename is an > HostName" do > @@ -148,7 +148,7 @@ describe Puppet::Parser::LoadedCode do > > Puppet::Parser::AST::HostName.expects(:new).with(:value > => "foo").never > > - @loader.node(name) > + @loader.node_matching(name) > end > > it "should be able to find node by HostName" do > @@ -156,7 +156,24 @@ describe Puppet::Parser::LoadedCode do > nameout = Puppet::Parser::AST::HostName.new(:value => > "foo") > > @loader.add_node(namein, "bar") > - @loader.node(nameout) == "bar" > + @loader.node_matching(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") > + > + @loader.add_node(namein, "bar") > + @loader.node_named(nameout).should == "bar" > + 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/)) > + > + @loader.add_node(name1, "bar") > + @loader.add_node(name2, "baz") > + @loader.node_named(name1).should == "bar" > end > > it "should return the first matching regex nodename" do > @@ -168,7 +185,7 @@ describe Puppet::Parser::LoadedCode do > @loader.add_node(@nodename1, @node1) > @loader.add_node(@nodename2, @node2) > > - @loader.node("test").should == @node1 > + @loader.node_matching("test").should == @node1 > end > > it "should not scan non-regex node" do > @@ -180,7 +197,7 @@ describe Puppet::Parser::LoadedCode do > @loader.add_node(@nodename1,@node1) > @loader.add_node(@nodename2,@node2) > > - @loader.node("test") > + @loader.node_matching("test") > end > > it "should prefer non-regex nodes to regex nodes" do > @@ -192,7 +209,7 @@ describe Puppet::Parser::LoadedCode do > @loader.add_node(@nodename1,@node1) > @loader.add_node(@nodename2,@node2) > > - @loader.node(@nodename1) > + @loader.node_matching(@nodename1) > end > end > end > diff --git a/spec/unit/parser/parser.rb b/spec/unit/parser/parser.rb > index 9127599..d993c1e 100755 > --- a/spec/unit/parser/parser.rb > +++ b/spec/unit/parser/parser.rb > @@ -253,7 +253,7 @@ describe Puppet::Parser do > end > > it "should raise an error if the node already exists" do > - @loaded_code.stubs(:node).with(@nodename).returns(@node) > + > @loaded_code.stubs(:node_named).with(@nodename).returns(@node) > > lambda { @parser.newnode(@nodename) }.should raise_error > end > @@ -291,7 +291,7 @@ describe Puppet::Parser do > > describe "when retrieving a specific node" do > it "should delegate to the loaded_code node" do > - @loaded_code.expects(:node).with("node") > + @loaded_code.expects(:node_matching).with("node") > > @parser.node("node") > end > -- > 1.6.4 > > > > -- An expert is a person who has made all the mistakes that can be made in a very narrow field. - Niels Bohr --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
