delete_undef_values function fix bug #20681

[lmello]

(#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.

[adrienthebo]

Thank you very much for this contribution! Could this have the commit fixed up like GH-182 and GH-183 before I merge it?

[lmello]

sure! :-)

[adrienthebo]

Sorry, one last comment - why do we alternate between result and args[0] here?

[lmello]

this is necessary because we can't use .dup on nil .
as a side effect we only attribute result value if args[0] is not nil.

If the function receives a empty or nil argument,
we won't have the result value attributed, but args[0] will always exists, so
it would be better to check the type of arg[0] instead of result.

when i implemented this i had thought about other ways of making this change:

basically we could:

1. do .dup on the result when its value is attributed (method i used)
   or
2. before running the delete or delete_if commands (in this case we would need to have
   .dup in two different places. )

as see by the following example :
result = args[0]
if result.is_a?(hash)
result = result.dup.delete_if {|key, val| val.equal? :undef}
elsif result.is_a?(Array)
result = result.dup.delete :undef
else
raise(Puppet::ParseError, .....REDACTED....

if you prefer method 2, or have other suggestion, I could change that.

[adrienthebo]

What about doing error checking on the type first?

unless args[0].is_a? Array or args[0].is_a? Hash
  raise Puppet::ParseError, "delete_undef_values(): expected an array or hash, got a #{args[0]}"
end

result = args[0].dup
# ...

[lmello]

Awesome!

[lmello]

Thanks for your suggestion, it is much better now! :-D

[adrienthebo]

The travis-ci build failed (https://travis-ci.org/puppetlabs/puppetlabs-stdlib/jobs/11480648#L927), https://github.com/puppetlabs/puppetlabs-stdlib/pull/184/files#L0R24 should be got #{args[0]} instead of got #{arg[0]}

[lmello]

@adrienthebo ... typo fixed.

[adrienthebo]

summary: merged into master in 7ccf8cf; thanks for the contribution!

[lmello]

Thanks for merging! :-)