From 80a8b7bd1fdb3aadd53d112ce9a20b86d8ea9a62 Mon Sep 17 00:00:00 2001 From: Leonardo Rodrigues de Mello Date: Mon, 16 Sep 2013 11:08:13 -0300 Subject: [PATCH] (#20681) fix behaviour of delete_undef_values The issue #20681 describe the error of delete() function removing the elements from the origin array/hash/string. This issue affected other delete functions. Because ruby delete and delete_if functions make destructive changes to the origin array/hash. The delete_undef_values removed elements from the origin array/hash and this is not the desired behaviour. To solve this, we should dup or clone the array/hash before using the delete or delete_if ruby functions. We should also check if args[0] is not nil before using dup, since dup on nil raises exception. This fix the problem and add unit tests, so we could enforce this behaviour and prevent regressions. --- lib/puppet/parser/functions/delete_undef_values.rb | 12 ++++++------ .../parser/functions/delete_undef_values_spec.rb | 12 ++++++++++++ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/lib/puppet/parser/functions/delete_undef_values.rb b/lib/puppet/parser/functions/delete_undef_values.rb index 4eecab2..532639e 100644 --- a/lib/puppet/parser/functions/delete_undef_values.rb +++ b/lib/puppet/parser/functions/delete_undef_values.rb @@ -18,16 +18,16 @@ Would return: ['A','',false] raise(Puppet::ParseError, "delete_undef_values(): Wrong number of arguments given " + "(#{args.size})") if args.size < 1 - - result = args[0] + + unless args[0].is_a? Array or args[0].is_a? Hash + raise(Puppet::ParseError, + "delete_undef_values(): expected an array or hash, got #{args[0]} type #{args[0].class} ") + end + result = args[0].dup if result.is_a?(Hash) result.delete_if {|key, val| val.equal? :undef} elsif result.is_a?(Array) result.delete :undef - else - raise(Puppet::ParseError, - "delete_undef_values(): Wrong argument type #{args[0].class} " + - "for first argument") end result end diff --git a/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb b/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb index 0536641..404aeda 100644 --- a/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb +++ b/spec/unit/puppet/parser/functions/delete_undef_values_spec.rb @@ -26,4 +26,16 @@ describe "the delete_undef_values function" do result = scope.function_delete_undef_values([{'a'=>'A','b'=>:undef,'c'=>'C','d'=>'undef'}]) result.should(eq({'a'=>'A','c'=>'C','d'=>'undef'})) end + + it "should not change origin array passed as argument" do + origin_array = ['a',:undef,'c','undef'] + result = scope.function_delete_undef_values([origin_array]) + origin_array.should(eq(['a',:undef,'c','undef'])) + end + + it "should not change origin hash passed as argument" do + origin_hash = { 'a' => 1, 'b' => :undef, 'c' => 'undef' } + result = scope.function_delete_undef_values([origin_hash]) + origin_hash.should(eq({ 'a' => 1, 'b' => :undef, 'c' => 'undef' })) + end end