[MODULES-2462] Improve parseyaml function

* Add default value support
  Second argument will be returned if yaml cannot be parsed
  instead of false value
* Update tests

7 files changed, 178 insertions, 42 deletions

diff --git a/README.markdown b/README.markdown
index 8ed3d9b..2f84a24 100644
--- a/README.markdown
+++ b/README.markdown
@@ -490,10 +490,12 @@ Converts a number or a string representation of a number into a true boolean. Ze
 #### `parsejson`
 
 Converts a string of JSON into the correct Puppet structure. *Type*: rvalue.
+The optional second argument will be returned if the data was not correct.
 
 #### `parseyaml`
 
 Converts a string of YAML into the correct Puppet structure. *Type*: rvalue.
+The optional second argument will be returned if the data was not correct.
 
 #### `pick`
 
diff --git a/lib/puppet/parser/functions/parsejson.rb b/lib/puppet/parser/functions/parsejson.rb
index a9a16a4..f822fc4 100644
--- a/lib/puppet/parser/functions/parsejson.rb
+++ b/lib/puppet/parser/functions/parsejson.rb
@@ -3,21 +3,22 @@
 #
 
 module Puppet::Parser::Functions
-  newfunction(:parsejson, :type => :rvalue, :doc => <<-EOS
-This function accepts JSON as a string and converts into the correct Puppet
-structure.
-    EOS
+  newfunction(:parsejson, :type => :rvalue, :arity => -2, :doc => <<-EOS
+This function accepts JSON as a string and converts it into the correct
+Puppet structure.
+
+The optional second argument can be used to pass a default value that will
+be returned if the parsing of YAML string have failed.
+  EOS
   ) do |arguments|
+    raise ArgumentError, 'Wrong number of arguments. 1 or 2 arguments should be provided.' unless arguments.length >= 1
 
-    if (arguments.size != 1) then
-      raise(Puppet::ParseError, "parsejson(): Wrong number of arguments "+
-        "given #{arguments.size} for 1")
+    begin
+      PSON::load(arguments[0]) || arguments[1]
+    rescue Exception
+      arguments[1]
     end
 
-    json = arguments[0]
-
-    # PSON is natively available in puppet
-    PSON.load(json)
   end
 end
 
diff --git a/lib/puppet/parser/functions/parseyaml.rb b/lib/puppet/parser/functions/parseyaml.rb
index 53d54fa..d38b3ef 100644
--- a/lib/puppet/parser/functions/parseyaml.rb
+++ b/lib/puppet/parser/functions/parseyaml.rb
@@ -3,20 +3,22 @@
 #
 
 module Puppet::Parser::Functions
-  newfunction(:parseyaml, :type => :rvalue, :doc => <<-EOS
+  newfunction(:parseyaml, :type => :rvalue, :arity => -2, :doc => <<-EOS
 This function accepts YAML as a string and converts it into the correct
 Puppet structure.
-    EOS
-  ) do |arguments|
-
-    if (arguments.size != 1) then
-      raise(Puppet::ParseError, "parseyaml(): Wrong number of arguments "+
-        "given #{arguments.size} for 1")
-    end
 
+The optional second argument can be used to pass a default value that will
+be returned if the parsing of YAML string have failed.
+  EOS
+  ) do |arguments|
+    raise ArgumentError, 'Wrong number of arguments. 1 or 2 arguments should be provided.' unless arguments.length >= 1
     require 'yaml'
 
-    YAML::load(arguments[0])
+    begin
+      YAML::load(arguments[0]) || arguments[1]
+    rescue Exception
+      arguments[1]
+    end
 
   end
 end
diff --git a/spec/acceptance/parsejson_spec.rb b/spec/acceptance/parsejson_spec.rb
index 5097810..d0feabd 100755
--- a/spec/acceptance/parsejson_spec.rb
+++ b/spec/acceptance/parsejson_spec.rb
@@ -16,19 +16,28 @@ describe 'parsejson function', :unless => UNSUPPORTED_PLATFORMS.include?(fact('o
       end
     end
   end
+
   describe 'failure' do
     it 'raises error on incorrect json' do
       pp = <<-EOS
       $a = '{"hunter": "washere", "tests": "passing",}'
-      $ao = parsejson($a)
+      $ao = parsejson($a, {'tests' => 'using the default value'})
       notice(inline_template('a is <%= @ao.inspect %>'))
       EOS
 
-      apply_manifest(pp, :expect_failures => true) do |r|
-        expect(r.stderr).to match(/expected next name/)
+      apply_manifest(pp, :catch_failures => true) do |r|
+        expect(r.stdout).to match(/tests are "using the default value"/)
       end
     end
 
-    it 'raises error on incorrect number of arguments'
+    it 'raises error on incorrect number of arguments' do
+      pp = <<-EOS
+      $o = parsejson()
+      EOS
+
+      apply_manifest(pp, :expect_failures => true) do |r|
+        expect(r.stderr).to match(/wrong number of arguments/i)
+      end
+    end
   end
 end
diff --git a/spec/acceptance/parseyaml_spec.rb b/spec/acceptance/parseyaml_spec.rb
index 5819837..7946de0 100755
--- a/spec/acceptance/parseyaml_spec.rb
+++ b/spec/acceptance/parseyaml_spec.rb
@@ -16,20 +16,29 @@ describe 'parseyaml function', :unless => UNSUPPORTED_PLATFORMS.include?(fact('o
       end
     end
   end
+
   describe 'failure' do
-    it 'raises error on incorrect yaml' do
+    it 'returns the default value on incorrect yaml' do
       pp = <<-EOS
       $a = "---\nhunter: washere\ntests: passing\n:"
-      $o = parseyaml($a)
+      $o = parseyaml($a, {'tests' => 'using the default value'})
       $tests = $o['tests']
       notice(inline_template('tests are <%= @tests.inspect %>'))
       EOS
 
-      apply_manifest(pp, :expect_failures => true) do |r|
-        expect(r.stderr).to match(/(syntax error|did not find expected key)/)
+      apply_manifest(pp, :catch_failures => true) do |r|
+        expect(r.stdout).to match(/tests are "using the default value"/)
       end
     end
 
-    it 'raises error on incorrect number of arguments'
+    it 'raises error on incorrect number of arguments' do
+      pp = <<-EOS
+      $o = parseyaml()
+      EOS
+
+      apply_manifest(pp, :expect_failures => true) do |r|
+        expect(r.stderr).to match(/wrong number of arguments/i)
+      end
+    end
   end
 end
diff --git a/spec/functions/parsejson_spec.rb b/spec/functions/parsejson_spec.rb
index 436566e..5bea8af 100755
--- a/spec/functions/parsejson_spec.rb
+++ b/spec/functions/parsejson_spec.rb
@@ -1,9 +1,64 @@
 require 'spec_helper'
 
 describe 'parsejson' do
-  it { is_expected.not_to eq(nil) }
-  it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
-  it { is_expected.to run.with_params('', '').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
-  it { is_expected.to run.with_params('["one"').and_raise_error(PSON::ParserError) }
-  it { is_expected.to run.with_params('["one", "two", "three"]').and_return(['one', 'two', 'three']) }
+  it 'should exist' do
+    is_expected.not_to eq(nil)
+  end
+
+  it 'should raise an error if called without any arguments' do
+    is_expected.to run.with_params().
+                       and_raise_error(/wrong number of arguments/i)
+  end
+
+  context 'with correct JSON data' do
+
+    it 'should be able to parse a JSON data with a Hash' do
+      is_expected.to run.with_params('{"a":"1","b":"2"}').
+                         and_return({'a'=>'1', 'b'=>'2'})
+    end
+
+    it 'should be able to parse a JSON data with an Array' do
+      is_expected.to run.with_params('["a","b","c"]').
+                         and_return(['a', 'b', 'c'])
+    end
+
+    it 'should be able to parse empty JSON values' do
+      is_expected.to run.with_params('[]').
+                         and_return([])
+      is_expected.to run.with_params('{}').
+                         and_return({})
+    end
+
+    it 'should be able to parse a JSON data with a mixed structure' do
+      is_expected.to run.with_params('{"a":"1","b":2,"c":{"d":[true,false]}}').
+                         and_return({'a' =>'1', 'b' => 2, 'c' => { 'd' => [true, false] } })
+    end
+
+    it 'should not return the default value if the data was parsed correctly' do
+      is_expected.to run.with_params('{"a":"1"}', 'default_value').
+                         and_return({'a' => '1'})
+    end
+
+  end
+
+  context 'with incorrect YAML data' do
+    it 'should return "nil" if a default value should be returned but is not provided' do
+      is_expected.to run.with_params('').
+                         and_return(nil)
+    end
+
+    it 'should support a structure for a default value' do
+      is_expected.to run.with_params('', {'a' => '1'}).
+                         and_return({'a' => '1'})
+    end
+
+    ['', 1, 1.2, nil, true, false, [], {}, :yaml].each do |value|
+      it "should return the default value for an incorrect #{value.inspect} (#{value.class}) parameter" do
+        is_expected.to run.with_params(value, 'default_value').
+                           and_return('default_value')
+      end
+    end
+
+  end
+
 end
diff --git a/spec/functions/parseyaml_spec.rb b/spec/functions/parseyaml_spec.rb
index fb635f8..492a1c9 100755
--- a/spec/functions/parseyaml_spec.rb
+++ b/spec/functions/parseyaml_spec.rb
@@ -1,14 +1,72 @@
 require 'spec_helper'
 
 describe 'parseyaml' do
-  it { is_expected.not_to eq(nil) }
-  it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
-  it { is_expected.to run.with_params('', '').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
-  it { is_expected.to run.with_params('["one", "two", "three"]').and_return(['one', 'two', 'three']) }
-  context 'when running on modern rubies', :unless => RUBY_VERSION == '1.8.7' do
-    it { is_expected.to run.with_params('["one"').and_raise_error(Psych::SyntaxError) }
+  it 'should exist' do
+    is_expected.not_to eq(nil)
   end
-  context 'when running on ruby 1.8.7, which does not have Psych', :if => RUBY_VERSION == '1.8.7' do
-    it { is_expected.to run.with_params('["one"').and_raise_error(ArgumentError) }
+
+  it 'should raise an error if called without any arguments' do
+    is_expected.to run.with_params().
+                       and_raise_error(/wrong number of arguments/i)
+  end
+
+  context 'with correct YAML data' do
+    it 'should be able to parse a YAML data with a String' do
+      is_expected.to run.with_params('--- just a string').
+                         and_return('just a string')
+      is_expected.to run.with_params('just a string').
+                         and_return('just a string')
+    end
+
+    it 'should be able to parse a YAML data with a Hash' do
+      is_expected.to run.with_params("---\na: '1'\nb: '2'\n").
+                         and_return({'a' => '1', 'b' => '2'})
+    end
+
+    it 'should be able to parse a YAML data with an Array' do
+      is_expected.to run.with_params("---\n- a\n- b\n- c\n").
+                         and_return(['a', 'b', 'c'])
+    end
+
+    it 'should be able to parse a YAML data with a mixed structure' do
+      is_expected.to run.with_params("---\na: '1'\nb: 2\nc:\n  d:\n  - :a\n  - true\n  - false\n").
+                         and_return({'a' => '1', 'b' => 2, 'c' => {'d' => [:a, true, false]}})
+    end
+
+    it 'should not return the default value if the data was parsed correctly' do
+      is_expected.to run.with_params("---\na: '1'\n", 'default_value').
+                         and_return({'a' => '1'})
+    end
+
   end
+
+  context 'with incorrect YAML data' do
+    it 'should return "nil" if a default value should be returned but is not provided' do
+      is_expected.to run.with_params('').
+                         and_return(nil)
+    end
+
+    it 'should support a structure for a default value' do
+      is_expected.to run.with_params('', {'a' => '1'}).
+                         and_return({'a' => '1'})
+    end
+
+    [1, 1.2, nil, true, false, [], {}, :yaml].each do |value|
+      it "should return the default value for an incorrect #{value.inspect} (#{value.class}) parameter" do
+        is_expected.to run.with_params(value, 'default_value').
+                           and_return('default_value')
+      end
+    end
+
+    context 'when running on modern rubies', :unless => RUBY_VERSION == '1.8.7' do
+      ['---', '...', '*8', ''].each do |value|
+        it "should return the default value for an incorrect #{value.inspect} string parameter" do
+          is_expected.to run.with_params(value, 'default_value').
+                             and_return('default_value')
+        end
+      end
+    end
+
+  end
+
 end