From 2b57e065d2220be4f172ae429190bd116ddbdaf1 Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Thu, 22 Oct 2009 20:09:27 +0200 Subject: Fix #2691 - Collection AR request should not include params if querying with tags f9516d introduced a change in the way the user tags are persisted to the database: user tags are now treated as regular tags (they are stored to the tags table). Thus this commit changed the AR collector request to also look at the tags tables when collecting. Unfortunately this added a performance regression since tag request were still importing the resources parameters tables and AR was issuing a large request which was returning all the resource parameters joined with the tags. This commit fixes the AR request to join to the needed table, instead of doing an include. Including (ie eager loading) parameter values was not working for resource parameters anyway since at least 0.24 because searching by parameter add a constraint to the joins and only the searched parameter was returned instead of all parameter for a given exported resource. So on a performance standpoint this new code should be as fast 0.24 was. Signed-off-by: Brice Figureau --- spec/unit/parser/collector.rb | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/collector.rb b/spec/unit/parser/collector.rb index 926033c68..7f88bf754 100755 --- a/spec/unit/parser/collector.rb +++ b/spec/unit/parser/collector.rb @@ -498,35 +498,46 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co Puppet::Rails::Host.expects(:find_by_name).with(@scope.host).returns(@host) Puppet::Rails::Resource.stubs(:find).with { |*arguments| - options = arguments[3] + options = arguments[1] options[:conditions][0] =~ /^host_id != \?/ and options[:conditions][1] == 5 }.returns([@resource]) @collector.evaluate.should == [@resource] end - it "should return parameter names, parameter values when querying ActiveRecord" do + it "should join with parameter names, parameter values when querying ActiveRecord" do + @collector.equery = "param_names.name = title" Puppet::Rails::Resource.stubs(:find).with { |*arguments| - options = arguments[3] - options[:include] == {:param_values => :param_name} + options = arguments[1] + options[:joins] == {:param_values => :param_name} }.returns([@resource]) @collector.evaluate.should == [@resource] end - it "should return tags when querying ActiveRecord with a tag exported query" do + it "should join with tag tables when querying ActiveRecord with a tag exported query" do @collector.equery = "puppet_tags.name = test" Puppet::Rails::Resource.stubs(:find).with { |*arguments| - options = arguments[3] - options[:include] == {:param_values => :param_name, :puppet_tags => :resource_tags} + options = arguments[1] + options[:joins] == {:resource_tags => :puppet_tag} }.returns([@resource]) @collector.evaluate.should == [@resource] end + it "should not join parameters when querying ActiveRecord with a tag exported query" do + @collector.equery = "puppet_tags.name = test" + Puppet::Rails::Resource.stubs(:find).with { |*arguments| + options = arguments[1] + options[:joins] == {:param_values => :param_name} + }.returns([@resource]) + + @collector.evaluate.should be_false + end + it "should only search for exported resources with the matching type" do Puppet::Rails::Resource.stubs(:find).with { |*arguments| - options = arguments[3] + options = arguments[1] options[:conditions][0].include?("(exported=? AND restype=?)") and options[:conditions][1] == true and options[:conditions][2] == "Mytype" }.returns([@resource]) @@ -536,7 +547,7 @@ describe Puppet::Parser::Collector, "when building its ActiveRecord query for co it "should include the export query if one is provided" do @collector.equery = "test = true" Puppet::Rails::Resource.stubs(:find).with { |*arguments| - options = arguments[3] + options = arguments[1] options[:conditions][0].include?("test = true") }.returns([@resource]) -- cgit From 73d04c6b15e1b626cd7dea1f963a5ca02a810137 Mon Sep 17 00:00:00 2001 From: Jesse Wolfe Date: Mon, 26 Oct 2009 00:43:29 -0700 Subject: Bug #2534 Raise error if property appears twice This patch changes Puppet::Parser::Resource to check if it has been passed two Puppet::Parser::Resource::Param objects with the same name. Signed-off-by: Jesse Wolfe --- spec/unit/parser/resource.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/resource.rb b/spec/unit/parser/resource.rb index 0a67c4b54..3f08de958 100755 --- a/spec/unit/parser/resource.rb +++ b/spec/unit/parser/resource.rb @@ -25,7 +25,7 @@ describe Puppet::Parser::Resource do params = args[:params] || {:one => "yay", :three => "rah"} if args[:params] == :none args.delete(:params) - else + elsif not args[:params].is_a? Array args[:params] = paramify(args[:source], params) end @@ -483,5 +483,18 @@ describe Puppet::Parser::Resource do result = @parser_resource.to_resource result[:fee].should == ["a", Puppet::Resource::Reference.new(:file, "/my/file1"), Puppet::Resource::Reference.new(:file, "/my/file2")] end + + it "should fail if the same param is declared twice" do + lambda do + @parser_resource = mkresource :source => @source, :params => [ + Puppet::Parser::Resource::Param.new( + :name => :foo, :value => "bar", :source => @source + ), + Puppet::Parser::Resource::Param.new( + :name => :foo, :value => "baz", :source => @source + ) + ] + end.should raise_error(Puppet::ParseError) + end end end -- cgit From 6846c327e120b88334853bcc947f522b2cd9e377 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 26 Oct 2009 20:36:01 -0700 Subject: Fixing some recently broken Scope tests Signed-off-by: Luke Kanies --- spec/unit/parser/scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/scope.rb b/spec/unit/parser/scope.rb index 0859eadb4..d7800e4b3 100755 --- a/spec/unit/parser/scope.rb +++ b/spec/unit/parser/scope.rb @@ -43,7 +43,7 @@ describe Puppet::Parser::Scope do describe "and the variable is qualified" do before do @parser = Puppet::Parser::Parser.new() - @compiler = Puppet::Parser::Compiler.new(stub("node", :name => "foonode"), @parser) + @compiler = Puppet::Parser::Compiler.new(stub("node", :name => "foonode", :classes => []), @parser) @scope.compiler = @compiler @scope.parser = @parser end -- cgit From 09fb3f707dfce31a11eda2f35bd77e65c911c15f Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Mon, 26 Oct 2009 20:39:41 -0700 Subject: Fixing #2752 - "require" loads "include" Signed-off-by: Luke Kanies --- spec/unit/parser/functions/require.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/functions/require.rb b/spec/unit/parser/functions/require.rb index 24c9ecc64..577a52a42 100755 --- a/spec/unit/parser/functions/require.rb +++ b/spec/unit/parser/functions/require.rb @@ -31,6 +31,12 @@ describe "the require function" do @scope.function_require("myclass") end + it "should verify the 'include' function is loaded" do + Puppet::Parser::Functions.expects(:function).with(:include).returns(:function_include) + @scope.stubs(:function_include) + @scope.function_require("myclass") + end + it "should include the class but not add a dependency if used on a client not at least version 0.25" do @resource.expects(:metaparam_compatibility_mode?).returns true @scope.expects(:warning) -- cgit From 38ec9fcc5f3965942a74c8d7b7dfd1cf1796c0df Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Tue, 10 Nov 2009 18:19:59 +0100 Subject: Fix #2796 - Fix puppetdoc rdoc selector parsing This patch fix this bug by adding more to_s methods to ast member so that puppetdoc can just to_s the AST to reconstruct the original puppet code. Of course this is not perfect, but should work most of the time. Signed-off-by: Brice Figureau --- spec/unit/parser/ast/leaf.rb | 9 +++++++++ spec/unit/parser/ast/selector.rb | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/ast/leaf.rb b/spec/unit/parser/ast/leaf.rb index e9681503d..fecfba386 100755 --- a/spec/unit/parser/ast/leaf.rb +++ b/spec/unit/parser/ast/leaf.rb @@ -72,6 +72,15 @@ describe Puppet::Parser::AST::String do end end +describe Puppet::Parser::AST::Variable do + describe "when converting to string" do + it "should transform its value to a variable" do + value = stub 'value', :is_a? => true, :to_s => "myvar" + Puppet::Parser::AST::Variable.new( :value => value ).to_s.should == "\$myvar" + end + end +end + describe Puppet::Parser::AST::Regex do before :each do @scope = stub 'scope' diff --git a/spec/unit/parser/ast/selector.rb b/spec/unit/parser/ast/selector.rb index 8b0057784..2ba83ad7b 100755 --- a/spec/unit/parser/ast/selector.rb +++ b/spec/unit/parser/ast/selector.rb @@ -151,6 +151,13 @@ describe Puppet::Parser::AST::Selector do @selector.evaluate(@scope) end end - + end + describe "when converting to string" do + it "should produce a string version of this selector" do + values = Puppet::Parser::AST::ASTArray.new :children => [ Puppet::Parser::AST::ResourceParam.new(:param => "type", :value => "value", :add => false) ] + param = Puppet::Parser::AST::Variable.new :value => "myvar" + selector = Puppet::Parser::AST::Selector.new :param => param, :values => values + selector.to_s.should == "$myvar ? { type => value }" + end end end -- cgit From ca56aa7e5849a5489e8d38e29b25ea934caafcd7 Mon Sep 17 00:00:00 2001 From: Markus Roberts Date: Mon, 26 Oct 2009 23:09:07 -0700 Subject: Least kludgy patch for #2675 This makes parameters responsible for the canonicalization of their values and provides a default (passthrough) implementation. It changes munge to pre- canonicalize the value and resource references to builtin types to canonicalize titles (which map to resorce namevars) with the corresponding parameter's classes's canonicalization. It adds a canonicalization routine to file paths that normalizes the behaviour (trailing slashes are ignored) and DRYs up the related code. Signed-off-by: Markus Roberts --- spec/unit/parser/resource/reference.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'spec/unit/parser') diff --git a/spec/unit/parser/resource/reference.rb b/spec/unit/parser/resource/reference.rb index 6284e67cc..a703f9284 100755 --- a/spec/unit/parser/resource/reference.rb +++ b/spec/unit/parser/resource/reference.rb @@ -40,10 +40,15 @@ describe Puppet::Parser::Resource::Reference do ref.to_s.should == "File[/tmp/yay]" end - it "should canonize resource references" do + it "should canonize resource reference types" do ref = @type.new(:type => "foo::bar", :title => "/tmp/yay") ref.to_s.should == "Foo::Bar[/tmp/yay]" end + + it "should canonize resource reference values" do + ref = @type.new(:type => "file", :title => "/tmp/yay/") + ref.to_s.should == "File[/tmp/yay]" + end end describe Puppet::Parser::Resource::Reference, " when modeling defined types" do -- cgit