Fixing the state class so that blocks are optional for values. This is useful for cases where you want to specify values for validation but you want a method called on the provider instead.

git-svn-id: https://reductivelabs.com/svn/puppet/trunk@1857 980ebf18-57e1-0310-9a29-db15c13687c0

2 files changed, 103 insertions, 32 deletions

diff --git a/lib/puppet/type/state.rb b/lib/puppet/type/state.rb
index cf400f613..a85ea801f 100644
--- a/lib/puppet/type/state.rb
+++ b/lib/puppet/type/state.rb
@@ -38,6 +38,9 @@ class State < Puppet::Parameter
 
     # Only retrieve the event, don't autogenerate one.
     def self.event(value)
+        if value.is_a?(String)
+            value = symbolize(value)
+        end
         if hash = @parameteroptions[value]
             hash[:event]
         else
@@ -70,6 +73,10 @@ class State < Puppet::Parameter
             paramopts[symbolize(opt)] = symbolize(val)
         end
 
+        # If there was no block given, we still want to store the information
+        # for validation, but we won't be defining a method
+        block ||= true
+
         case name
         when Symbol
             if @parametervalues.include?(name)
@@ -77,9 +84,11 @@ class State < Puppet::Parameter
             end
             @parametervalues[name] = block
 
-            method = "set_" + name.to_s
-            settor = paramopts[:settor] || (self.name.to_s + "=")
-            define_method(method, &block)
+            if block_given?
+                method = "set_" + name.to_s
+                settor = paramopts[:settor] || (self.name.to_s + "=")
+                define_method(method, &block)
+            end
         when Regexp
             # The regexes are handled in parameter.rb
             @parameterregexes[name] = block
@@ -109,6 +118,31 @@ class State < Puppet::Parameter
                 [self.name, detail]
         end
     end
+
+    # Figure out which event to return.
+    # not specified.
+    def event(value, event = nil)
+        if setevent = self.class.event(value)
+            return setevent
+        else
+            if event and event.is_a?(Symbol)
+                if event == :nochange
+                    return nil
+                else
+                    return event
+                end
+            else
+                event = case self.should
+                when :present: (@parent.class.name.to_s + "_created").intern
+                when :absent: (@parent.class.name.to_s + "_removed").intern
+                else
+                    (@parent.class.name.to_s + "_changed").intern
+                end
+            end
+        end
+
+        return event
+    end
     
     # initialize our state
     def initialize(hash)
@@ -250,6 +284,9 @@ class State < Puppet::Parameter
         if value.nil?
             self.devfail "Got a nil value for should"
         end
+
+        # Set a name for looking up associated options like the event.
+        name = symbolize(value)
         method = "set_" + value.to_s
         event = nil
         if self.respond_to?(method)
@@ -268,11 +305,19 @@ class State < Puppet::Parameter
                 error.set_backtrace detail.backtrace
                 raise error
             end
-        elsif ary = self.class.match?(value)
+        elsif ary = self.class.match?(value) and ary[1].is_a?(Proc)
             # FIXME It'd be better here to define a method, so that
             # the blocks could return values.
-            event = self.instance_eval(&ary[1])
+            # If the regex was defined with no associated block, then just pass
+            # through and the correct event will be passed back.
+            if ary[1].is_a?(Proc)
+                event = self.instance_eval(&ary[1])
+            end
         else
+            # This will get set if the regex matches but has no proc
+            if ary
+                name = ary[0]
+            end
             if @parent.provider
                 begin
                     provider.send(self.class.name.to_s + "=", self.should)
@@ -286,30 +331,7 @@ class State < Puppet::Parameter
             end
         end
 
-        if setevent = self.class.event(value)
-            return setevent
-        else
-            if event and event.is_a?(Symbol)
-                if event == :nochange
-                    return nil
-                else
-                    return event
-                end
-            else
-                # Return the appropriate event.
-                event = case self.should
-                when :present: (@parent.class.name.to_s + "_created").intern
-                when :absent: (@parent.class.name.to_s + "_removed").intern
-                else
-                    (@parent.class.name.to_s + "_changed").intern
-                end
-
-                #self.log "made event %s because 'should' is %s, 'is' is %s" %
-                #    [event, self.should.inspect, self.is.inspect]
-
-                return event
-            end
-        end
+        return event(name, event)
     end
 
     # Only return the first value
diff --git a/test/types/state.rb b/test/types/state.rb
index be4e8ae8c..e079c5c41 100755
--- a/test/types/state.rb
+++ b/test/types/state.rb
@@ -8,17 +8,21 @@ require 'puppettest'
 class TestState < Test::Unit::TestCase
 	include PuppetTest
 
-    def newinst(state)
+    def newinst(state, parent = nil)
         inst = nil
+        unless parent
+            parent = "fakeparent"
+            parent.meta_def(:path) do self.to_s end
+        end
         assert_nothing_raised {
-            return state.new(:parent => nil)
+            return state.new(:parent => parent)
         }
     end
     
     def newstate(name = :fakestate)
         assert_nothing_raised {
             state = Class.new(Puppet::State) do
-                @name = :fakeparam
+                @name = name
             end
             state.initvars
 
@@ -124,6 +128,51 @@ class TestState < Test::Unit::TestCase
                      "Event did not get returned correctly")
     end
 
+    # We want to support values with no blocks, either regexes or strings.
+    # If there's no block provided, then we should call the provider mechanism
+    # like we would normally.
+    def test_newvalue_with_no_block
+        state = newstate(:mystate)
+
+        assert_nothing_raised {
+            state.newvalue(:value, :event => :matched_value)
+        }
+        assert_nothing_raised {
+            state.newvalue(/^\d+$/, :event => :matched_number)
+        }
+
+        # Create an object that responds to mystate as an attr
+        provklass = Class.new { attr_accessor :mystate }
+        prov = provklass.new
+
+        klass = Class.new { attr_accessor :provider, :path }
+        klassinst = klass.new
+        klassinst.path = "instpath"
+        klassinst.provider = prov
+
+        inst = newinst(state, klassinst)
+
+        # Now make sure we can set the values, they get validated as normal,
+        # and they set the values on the parent rathe than trying to call
+        # a method
+        {:value => :matched_value, "27" => :matched_number}.each do |value, event|
+            assert_nothing_raised do
+                inst.should = value
+            end
+            ret = nil
+            assert_nothing_raised do
+                ret = inst.sync
+            end
+            assert_equal(event, ret, "Did not return correct event for %s" % value)
+            assert_equal(value, prov.mystate, "%s was not set right" % value)
+        end
+
+        # And make sure we still fail validations
+        assert_raise(ArgumentError) do
+            inst.should = "invalid"
+        end
+    end
+
     def test_tags
         obj = "yay"
         metaobj = class << obj; self; end