Add an optional third parameter to redisget() to specify a default value

README.md

@@ -102,13 +102,24 @@ class { '::redis::sentinel':
 
 ## `redisget()` function
 
-`redisget()` takes two arguments that are strings. The first is the key
-to be looked up and the second is the URL to the Redis service.
+`redisget()` takes two or three arguments that are strings. The first is the key
+to be looked up, the second is the URL to the Redis service and the
+optional third argument is a default value to use if the key is not
+found or connection to the Redis service cannot be made.
+
+Example of basic usage.
 
 ```puppet
 $version = redisget('version.myapp', 'redis://redis.example.com:6379')
 ```
 
+Example with default value specified. This is useful to allow for cached
+data in case Redis is not available.
+
+```puppet
+$version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version)
+```
+
 You must have the 'redis' gem installed on your puppet master.
 
 ## Unit testing

lib/puppet/parser/functions/redisget.rb

@@ -3,26 +3,51 @@
 module Puppet::Parser::Functions
   newfunction(:redisget, :type => :rvalue, :doc => <<-DOC
 Returns the value of the key being looked up or nil if the key does not
-exist. Takes two arguments, the first being a string value of the key to be
-looked up and the second is the URL to the Redis service.
+exist. Takes two arguments with an optional third. The first being a string
+value of the key to be looked up, the second is the URL to the Redis service
+and the third optional argument is a default value to be used if the lookup
+fails.
 @param redis_key [String] The key to look up in redis.
 @param redis_uri [String] The endpoint of the Redis instance.
+@param default_value [String] The value to return if the key is not found or
+  the connection to Redis fails
 @return [String] The value of the key from redis
 @return [String] An empty string eg. `''`
 @example Calling the function.
   $version = redisget('version.myapp', 'redis://redis.example.com:6379')
+@example Calling the function with a default if failure occurs
+  $version = redisget('version.myapp', 'redis://redis.example.com:6379', $::myapp_version)
 DOC
   ) do |args|
 
-    raise(Puppet::ParseError, "redisget(): Wrong number of arguments given (#{args.size} for 2)") if args.size != 2
+    raise(Puppet::ParseError, "redisget(): Wrong number of arguments given (#{args.size} for 2 or 3)") if args.size != 2 and args.size != 3
 
     key = args[0]
     url = args[1]
 
-    raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{key.class} for String") if key.is_a?(String) == false
-    raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{url.class} for String") if url.is_a?(String) == false
+    if args.size == 3
+      default = args[2]
+      raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{default.class} for String) for arg3 (default)") if default.is_a?(String) == false
+    end
 
-    redis = Redis.new(:url => url)
-    redis.get(key)
+    raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{key.class} for String) for arg1 (key)") if key.is_a?(String) == false
+    raise(Puppet::ParseError, "redisget(): Wrong argument type given (#{url.class} for String) for arg2 (url)") if url.is_a?(String) == false
+
+    begin
+      redis = Redis.new(:url => url)
+      returned_value = redis.get(key)
+      if returned_value == nil and defined?(default) != nil
+        default
+      else
+        returned_value
+      end
+    rescue Exception => e
+      if default
+        debug "Connection to redis failed with #{e} - Returning default value of #{default}"
+        default
+      else
+        raise(Puppet::Error, "connection to redis server failed - #{e}")
+      end
+    end
   end
 end

spec/acceptance/redisget_spec.rb

@@ -40,7 +40,20 @@ class { '::redis':
     notify{"mykey value: ${mykey}":}
     EOS
 
-    # Check output for fact string
+    # Check output for function return value
+    apply_manifest(pp, :catch_failures => true) do |r|
+      expect(r.stdout).to match(/mykey value: Hello/)
+    end
+  end
+
+  it 'should return a value from valid MyKey with the redisget() function while specifying a default' do
+    pp = <<-EOS
+    $mykey = redisget('mykey', 'redis://127.0.0.1:6379', 'default_value')
+
+    notify{"mykey value: ${mykey}":}
+    EOS
+
+    # Check output for function return value
     apply_manifest(pp, :catch_failures => true) do |r|
       expect(r.stdout).to match(/mykey value: Hello/)
     end
@@ -55,10 +68,51 @@ class { '::redis':
     }
     EOS
 
-    # Check output for fact string
+    # Check output for function return value
     apply_manifest(pp, :catch_failures => true) do |r|
       expect(r.stdout).to match(/foo_key value was empty string/)
     end
   end
 
+  it 'should return the specified default value when key not present with redisget() function' do
+    pp = <<-EOS
+    $foo_key = redisget('foo', 'redis://127.0.0.1:6379', 'default_value')
+
+    notify { $foo_key: }
+    EOS
+
+    # Check output for function return value
+    apply_manifest(pp, :catch_failures => true) do |r|
+      expect(r.stdout).to match(/default_value/)
+    end
+  end
+
+  it 'should return the specified default value when connection to redis server fails' do
+    pp = <<-EOS
+    # Bogus port for redis server
+    $foo_key = redisget('foo', 'redis://127.0.0.1:12345', 'default_value')
+
+    notify { $foo_key: }
+    EOS
+
+    # Check output for function return value
+    apply_manifest(pp, :catch_failures => true) do |r|
+      expect(r.stdout).to match(/default_value/)
+    end
+  end
+
+  it 'should return an error when specifying a non connectable redis server' do
+    pp = <<-EOS
+    # Bogus port for redis server
+    $foo_key = redisget('foo', 'redis://127.0.0.1:12345')
+
+    notify { $foo_key: }
+    EOS
+
+    # Check output for error when can't connect to bogus redis
+    apply_manifest(pp, :acceptable_exit_codes => [1]) do |r|
+      expect(r.stderr).to match(/Error connecting to Redis on 127.0.0.1:12345 \(Errno::ECONNREFUSED\)/)
+    end
+  end
+
 end

spec/functions/redisget_spec.rb

@@ -3,15 +3,31 @@
 require 'redis'
 
 REDIS_URL='redis://localhost:6379'
+BROKEN_URL='redis://redis.example.com:1234'
 
 describe 'redisget' do
-  context 'should return nil if key does not exist' do
+  context 'should error if connection to redis server cannot be made and no default is specified' do
+    it { is_expected.to run.with_params('nonexistent_key', BROKEN_URL).and_raise_error(Puppet::Error, /connection to redis server failed - getaddrinfo/) }
+  end
+
+  context 'should return default value if connection to redis server cannot be made and default is specified' do
+    it { is_expected.to run.with_params('nonexistent_key', BROKEN_URL, 'default_value').and_return('default_value') }
+  end
+
+  context 'should return nil if key does not exist and no default is specified' do
+    before {
+      mr = MockRedis.new
+      Redis.stubs(:new).returns(mr)
+    }
+    it { is_expected.to run.with_params('nonexistent_key', REDIS_URL).and_return(nil) }
+  end
+
+  context 'should return the default value if specified and key does not exist' do
     before {
       mr = MockRedis.new
       Redis.stubs(:new).returns(mr)
-      mr.set('nonexistent_key', nil)
     }
-    it { is_expected.to run.with_params('nonexistent_key', REDIS_URL).and_return('') }
+    it { is_expected.to run.with_params('nonexistent_key', REDIS_URL, 'default_value').and_return('default_value') }
   end
 
   context 'should return the value of the specified key' do
@@ -23,6 +39,15 @@
     it { is_expected.to run.with_params('key', REDIS_URL).and_return('value') }
   end
 
+  context 'should return the value of the specified key and not the default' do
+    before {
+      mr = MockRedis.new
+      Redis.stubs(:new).returns(mr)
+      mr.set('key', 'value')
+    }
+    it { is_expected.to run.with_params('key', REDIS_URL, 'default_value').and_return('value') }
+  end
+
   describe 'with incorrect arguments' do
     context 'with no argument specified' do
       it { is_expected.to run.with_params().and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
@@ -32,8 +57,8 @@
       it { is_expected.to run.with_params('some_key').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
     end
 
-    context 'with more than two arguments specified' do
-      it { is_expected.to run.with_params('too', 'many', 'args').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
+    context 'with more than three arguments specified' do
+      it { is_expected.to run.with_params('way', 'too', 'many', 'args').and_raise_error(Puppet::ParseError, /wrong number of arguments/i) }
     end
   end
 
@@ -50,6 +75,10 @@
       context "specifing second parameter as <#{p}>" do
         it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, /wrong argument type/i) }
       end
+
+      context "specifing third parameter as <#{p}>" do
+        it { is_expected.to run.with_params('valid', p).and_raise_error(Puppet::ParseError, /wrong argument type/i) }
+      end
     end
   end
 end