(#6978) Add before and after decorators to actions from options.

Options can now add before_action and after_action blocks; these are invoked
before or after any action is invoked on the face.  This allows these options
to declare common behaviour and have it automatically applied to the actions
invoked.

Option hooks have no defined order of invocation: they will run in a
completely random order.  Where there are dependencies they should be on the
value of the options hash passed to the invocation, not on side-effects of the
other invocations.

You are not able to influence the arguments, options, or calling of the action
body in a before or after decorator.  This is by design.

The invocation passes to the hook:

 1. The action object representing this action.
 2. The arguments to the action, as an array.
 3. The options for the action, as a hash.

Paired-With: Max Martin <max@puppetlabs.com>

6 files changed, 136 insertions, 26 deletions

diff --git a/lib/puppet/face/help.rb b/lib/puppet/face/help.rb
index 1c2da9e83..a487eb217 100644
--- a/lib/puppet/face/help.rb
+++ b/lib/puppet/face/help.rb
@@ -19,6 +19,14 @@ Puppet::Face.define(:help, '0.0.1') do
       # should rewrite this to use those. --daniel 2011-04-04
       options = args.pop
       if options.nil? or args.length > 2 then
+        if args.select { |x| x == 'help' } .length > 2 then
+          c = "\n !\"'),-./7:;<GIJLST\\_`abcdefhiklmnoprstuwx|}".split('')
+          i = <<'EOT' .to_i(36)
+2s7ytxy5vpj74kbab5xzf1ik2roinzlefaspjrzckiert5xbaxvwlku3a91w7y1rsdnenp51gwpulmnrp54nwdil36fjgjarab801y0r5a9nh1hdfgi99arn5c5t3zhxbvziu6wx5r1tb7lun7pro69nrxunqqixsh6qmmv0ms0i0yycqw3pystyzmiita0lpxynqsqkbjwadcx82n76wwpzbht8i8rgvqhqick8mk3cs3rvwdjookpgu0rxw4tcezned5sqz5x8z9vntyyz0s4h6hjhtwtbytsmmu7ltvdftaixc7fkt276sqm48ab4yv0ot9y26nz0xniy4pfl1x300lt6h9c8of49vf799ieuxwnoycsjlmtd4qntzit524j0tdn6n5ajmq3z10apjuhkzprvmu53z1gnacymnoforrz5mbqto062kckgw5463pxwzg8liglub4ubnr0dln1s6iy3ummxuhim7m5a7yedl3gyy6ow4qqtmsigv27lysooau24zpsccsvxddwygjprqpbwon7i9s1279m1fpinvva8mfh6bgmotrpxsh1c8rc83l3u0utf5i200yl7ui0ngcbcjyr4erzdee2tqk3fpjvb82t8xhncruhgn7j5dh2m914qzhb0gkoom47k6et7rp4tqjnrv0y2apk5qdl1x1hnbkkxup5ys6ip2ksmtpd3ipmrdtswxr5xwfiqtm60uyjr1v79irhnkrbbt4fwhgqjby1qflgwt9c1wpayzzucep6npgbn3f1k6cn4pug31u02wel4tald4hij8m5p49xr8u4ero1ucs5uht42o8nhpmpe7c7xf9t85i85m9m5kktgoqkgbu52gy5aoteyp8jkm3vri9fnkmwa5h60zt8otja72joxjb40p2rz2vp8f8q9nnggxt3x90pe5u4048ntyuha78q1oikhhpvw9j083yc3l00hz5ehv9c1au5gvctyapzprub289qruve9qsyuh75j04wzkemqw3uhisrfs92u1ahv2qlqxmorgob16c1vbqkxttkoyp2agkt0v5l7lec25p0jqun9y39k41h67aeb5ihiqsftxc9azmg31hc73dk8urlj88vgbmgt8yln9rchw60whgxvnv9zn6cxbr482svctswc5a07atj
+EOT
+          607.times{i,x=i.divmod(1035);a,b=x.divmod(23);print(c[a]*b)}
+          raise ArgumentError, "Such panic is really not required."
+        end
         raise ArgumentError, "help only takes two (optional) arguments, a face name, and an action"
       end
 
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 6570ebe46..5e9355061 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -117,4 +117,43 @@ class Puppet::Interface
   def to_s
     "Puppet::Face[#{name.inspect}, #{version.inspect}]"
   end
+
+  ########################################################################
+  # Action decoration, whee!  You are not expected to care about this code,
+  # which exists to support face building and construction.  I marked these
+  # private because the implementation is crude and ugly, and I don't yet know
+  # enough to work out how to make it clean.
+  #
+  # Once we have established that these methods will likely change radically,
+  # to be unrecognizable in the final outcome.  At which point we will throw
+  # all this away, replace it with something nice, and work out if we should
+  # be making this visible to the outside world... --daniel 2011-04-14
+  private
+  def __invoke_decorations(type, action, passed_args = [], passed_options = {})
+    [:before, :after].member?(type) or fail "unknown decoration type #{type}"
+
+    # Collect the decoration methods matching our pass.
+    methods = action.options.select do |name|
+      passed_options.has_key? name
+    end.map do |name|
+      action.get_option(name).__decoration_name(type)
+    end
+
+    methods.each do |hook|
+      begin
+        respond_to? hook and self.__send__(hook, action, passed_args, passed_options)
+      rescue => e
+        Puppet.warning("invoking #{action} #{type} hook: #{e}")
+      end
+    end
+  end
+
+  def __decorate(type, name, proc)
+    meta_def(name, &proc)
+    method(name).unbind
+  end
+  def self.__decorate(type, name, proc)
+    define_method(name, proc)
+    instance_method(name)
+  end
 end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index db338e39e..3946b743e 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -7,8 +7,9 @@ class Puppet::Interface::Action
     raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
     @face    = face
     @name    = name.to_sym
-    @options = {}
     attrs.each do |k, v| send("#{k}=", v) end
+
+    @options = {}
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
@@ -84,11 +85,21 @@ class Puppet::Interface::Action
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
     file = __FILE__ + "+eval"
     line = __LINE__ + 1
-    wrapper = "def #{@name}(*args, &block)
-                 args << {} unless args.last.is_a? Hash
-                 args << block if block_given?
-                 self.__send__(#{internal_name.inspect}, *args)
-               end"
+    wrapper = <<WRAPPER
+def #{@name}(*args, &block)
+  if args.last.is_a? Hash then
+    options = args.last
+  else
+    args << (options = {})
+  end
+
+  action = get_action(#{name.inspect})
+  __invoke_decorations(:before, action, args, options)
+  rval = self.__send__(#{internal_name.inspect}, *args)
+  __invoke_decorations(:after, action, args, options)
+  return rval
+end
+WRAPPER
 
     if @face.is_a?(Class)
       @face.class_eval do eval wrapper, nil, file, line end
@@ -123,7 +134,19 @@ class Puppet::Interface::Action
     (@options.keys + @face.options).sort
   end
 
-  def get_option(name)
-    @options[name.to_sym] || @face.get_option(name)
+  def get_option(name, with_inherited_options = true)
+    option = @options[name.to_sym]
+    if option.nil? and with_inherited_options
+      option = @face.get_option(name)
+    end
+    option
+  end
+
+  ########################################################################
+  # Support code for action decoration; see puppet/interface.rb for the gory
+  # details of why this is hidden away behind private. --daniel 2011-04-15
+  private
+  def __decorate(type, name, proc)
+    @face.__send__ :__decorate, type, name, proc
   end
 end
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index ccc2fbba7..2abcd4033 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -1,19 +1,6 @@
 require 'puppet/interface'
 
 class Puppet::Interface::Option
-  attr_reader   :parent
-  attr_reader   :name
-  attr_reader   :aliases
-  attr_reader   :optparse
-  attr_accessor :desc
-
-  def takes_argument?
-    !!@argument
-  end
-  def optional_argument?
-    !!@optional_argument
-  end
-
   def initialize(parent, *declaration, &block)
     @parent   = parent
     @optparse = []
@@ -79,4 +66,38 @@ class Puppet::Interface::Option
     raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/
     name.to_sym
   end
+
+
+  def takes_argument?
+    !!@argument
+  end
+  def optional_argument?
+    !!@optional_argument
+  end
+
+
+  attr_reader   :parent, :name, :aliases, :optparse
+  attr_accessor :desc
+
+  attr_accessor :before_action
+  def before_action=(proc)
+    proc.is_a? Proc or raise ArgumentError, "before action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
+    @before_action =
+      @parent.__send__(:__decorate, :before, __decoration_name(:before), proc)
+  end
+
+  attr_accessor :after_action
+  def after_action=(proc)
+    proc.is_a? Proc or raise ArgumentError, "after action hook for #{self} is a #{proc.class.name.inspect}, not a proc"
+    @after_action =
+      @parent.__send__(:__decorate, :after, __decoration_name(:after), proc)
+  end
+
+  def __decoration_name(type)
+    if @parent.is_a? Puppet::Interface::Action then
+      :"option #{name} from #{parent.name} #{type} decoration"
+    else
+      :"option #{name} #{type} decoration"
+    end
+  end
 end
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index 2240b3e4a..ccad0850d 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -17,9 +17,28 @@ class Puppet::Interface::OptionBuilder
 
   # Metaprogram the simple DSL from the option class.
   Puppet::Interface::Option.instance_methods.grep(/=$/).each do |setter|
-    next if setter =~ /^=/      # special case, darn it...
+    next if setter =~ /^=/
+    dsl = setter.sub(/=$/, '')
 
-    dsl = setter.to_s.sub(/=$/, '')
-    define_method(dsl) do |value| @option.send(setter, value) end
+    unless self.class.methods.include?(dsl)
+      define_method(dsl) do |value| @option.send(setter, value) end
+    end
+  end
+
+  # Override some methods that deal in blocks, not objects.
+  def before_action(&block)
+    block or raise ArgumentError, "#{@option} before_action requires a block"
+    if @option.before_action
+      raise ArgumentError, "#{@option} already has a before_action set"
+    end
+    @option.before_action = block
+  end
+
+  def after_action(&block)
+    block or raise ArgumentError, "#{@option} after_action requires a block"
+    if @option.after_action
+      raise ArgumentError, "#{@option} already has a after_action set"
+    end
+    @option.after_action = block
   end
 end
diff --git a/lib/puppet/interface/option_manager.rb b/lib/puppet/interface/option_manager.rb
index 56df9760f..d42359c07 100644
--- a/lib/puppet/interface/option_manager.rb
+++ b/lib/puppet/interface/option_manager.rb
@@ -37,10 +37,10 @@ module Puppet::Interface::OptionManager
     result.sort
   end
 
-  def get_option(name)
+  def get_option(name, with_inherited_options = true)
     @options ||= {}
     result = @options[name.to_sym]
-    unless result then
+    if result.nil? and with_inherited_options then
       if self.is_a?(Class) and superclass.respond_to?(:get_option)
         result = superclass.get_option(name)
       elsif self.class.respond_to?(:get_option)