From 86801b580101315706b1b02a00a36840eabd75cd Mon Sep 17 00:00:00 2001
From: Daniel Pittman <daniel@puppetlabs.com>
Date: Mon, 18 Apr 2011 13:29:47 -0700
Subject: (#7013) Support 'when_rendering' and 'render_as' in actions.

These define the API used by folks writing actions that supports their
rendering hooks.  'when_rendering' defines a helper method on the interface,
which runs the users code in their expected context.

'render_as' just sets the default rendering format; by default this is
:for_humans.

Reviewed-By: Max Martin <max@puppetlabs.com>
---
 lib/puppet/interface.rb                    |   4 +-
 lib/puppet/interface/action.rb             |  57 ++++++++-
 lib/puppet/interface/action_builder.rb     |  44 ++++++-
 lib/puppet/interface/option.rb             |   4 +-
 lib/puppet/interface/option_builder.rb     |   2 +-
 spec/unit/interface/action_builder_spec.rb | 194 ++++++++++++++++++++++-------
 spec/unit/interface/action_spec.rb         |   9 ++
 7 files changed, 256 insertions(+), 58 deletions(-)

diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 5e9355061..51ae0cdc1 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -148,11 +148,11 @@ class Puppet::Interface
     end
   end
 
-  def __decorate(type, name, proc)
+  def __add_method(name, proc)
     meta_def(name, &proc)
     method(name).unbind
   end
-  def self.__decorate(type, name, proc)
+  def self.__add_method(name, proc)
     define_method(name, proc)
     instance_method(name)
   end
diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index efe7b1f50..bdd42b197 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -10,6 +10,8 @@ class Puppet::Interface::Action
     attrs.each do |k, v| send("#{k}=", v) end
 
     @options = {}
+    @when_rendering = {}
+    @render_as = :for_humans
   end
 
   # This is not nice, but it is the easiest way to make us behave like the
@@ -21,9 +23,9 @@ class Puppet::Interface::Action
     return bound_version
   end
 
-  attr_reader :name
   def to_s() "#{@face}##{@name}" end
 
+  attr_reader   :name
   attr_accessor :default
   def default?
     !!@default
@@ -31,6 +33,55 @@ class Puppet::Interface::Action
 
   attr_accessor :summary
 
+
+  ########################################################################
+  # Support for rendering formats and all.
+  def when_rendering(type)
+    unless type.is_a? Symbol
+      raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
+    end
+    @when_rendering[type]
+  end
+  def set_rendering_method_for(type, proc)
+    unless proc.is_a? Proc
+      msg = "The second argument to set_rendering_method_for must be a Proc"
+      msg += ", not #{proc.class.name}" unless proc.nil?
+      raise ArgumentError, msg
+    end
+    if proc.arity != 1 then
+      msg = "when_rendering methods take one argument, the result, not "
+      if proc.arity < 0 then
+        msg += "a variable number"
+      else
+        msg += proc.arity.to_s
+      end
+      raise ArgumentError, msg
+    end
+    unless type.is_a? Symbol
+      raise ArgumentError, "The rendering format must be a symbol, not #{type.class.name}"
+    end
+    if @when_rendering.has_key? type then
+      raise ArgumentError, "You can't define a rendering method for #{type} twice"
+    end
+    # Now, the ugly bit.  We add the method to our interface object, and
+    # retrieve it, to rotate through the dance of getting a suitable method
+    # object out of the whole process. --daniel 2011-04-18
+    @when_rendering[type] =
+      @face.__send__( :__add_method, __render_method_name_for(type), proc)
+  end
+
+  def __render_method_name_for(type)
+    :"#{name}_when_rendering_#{type}"
+  end
+  private :__render_method_name_for
+
+
+  attr_accessor :render_as
+  def render_as=(value)
+    @render_as = value.to_sym
+  end
+
+
   # Initially, this was defined to allow the @action.invoke pattern, which is
   # a very natural way to invoke behaviour given our introspection
   # capabilities.   Heck, our initial plan was to have the faces delegate to
@@ -161,7 +212,7 @@ WRAPPER
   # 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
+  def __add_method(name, proc)
+    @face.__send__ :__add_method, name, proc
   end
 end
diff --git a/lib/puppet/interface/action_builder.rb b/lib/puppet/interface/action_builder.rb
index 639d8fc7f..2ffa38709 100644
--- a/lib/puppet/interface/action_builder.rb
+++ b/lib/puppet/interface/action_builder.rb
@@ -20,20 +20,54 @@ class Puppet::Interface::ActionBuilder
   # method on the face would defer to it, but we can't get scope correct, so
   # we stick with this. --daniel 2011-03-24
   def when_invoked(&block)
-    raise "when_invoked on an ActionBuilder with no corresponding Action" unless @action
     @action.when_invoked = block
   end
 
+  def when_rendering(type = nil, &block)
+    if type.nil? then           # the default error message sucks --daniel 2011-04-18
+      raise ArgumentError, 'You must give a rendering format to when_rendering'
+    end
+    if block.nil? then
+      raise ArgumentError, 'You must give a block to when_rendering'
+    end
+    @action.set_rendering_method_for(type, block)
+  end
+
   def option(*declaration, &block)
     option = Puppet::Interface::OptionBuilder.build(@action, *declaration, &block)
     @action.add_option(option)
   end
 
-  def default
-    @action.default = true
+  def default(value = true)
+    @action.default = !!value
+  end
+
+  def render_as(value = nil)
+    value.nil? and raise ArgumentError, "You must give a rendering format to render_as"
+
+    formats = Puppet::Network::FormatHandler.formats << :for_humans
+    unless formats.include? value
+      raise ArgumentError, "#{value.inspect} is not a valid rendering format: #{formats.sort.join(", ")}"
+    end
+
+    @action.render_as = value
   end
 
-  def summary(text)
-    @action.summary = text
+  # Metaprogram the simple DSL from the target class.
+  Puppet::Interface::Action.instance_methods.grep(/=$/).each do |setter|
+    next if setter =~ /^=/
+    dsl = setter.sub(/=$/, '')
+
+    unless private_instance_methods.include? dsl
+      # Using eval because the argument handling semantics are less awful than
+      # when we use the define_method/block version.  The later warns on older
+      # Ruby versions if you pass the wrong number of arguments, but carries
+      # on, which is totally not what we want. --daniel 2011-04-18
+      eval <<METHOD
+def #{dsl}(value)
+  @action.#{dsl} = value
+end
+METHOD
+    end
   end
 end
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 1971926d8..f4c56cb2c 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -84,14 +84,14 @@ class Puppet::Interface::Option
   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)
+      @parent.__send__(:__add_method, __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)
+      @parent.__send__(:__add_method, __decoration_name(:after), proc)
   end
 
   def __decoration_name(type)
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index 7c2ab89de..8f358c222 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -20,7 +20,7 @@ class Puppet::Interface::OptionBuilder
     next if setter =~ /^=/
     dsl = setter.sub(/=$/, '')
 
-    unless self.class.methods.include?(dsl)
+    unless private_instance_methods.include? dsl
       define_method(dsl) do |value| @option.send(setter, value) end
     end
   end
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index 8f8e8d1d5..38a23a64e 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -1,81 +1,185 @@
 #!/usr/bin/env rspec
 require 'spec_helper'
 require 'puppet/interface/action_builder'
+require 'puppet/network/format_handler'
 
 describe Puppet::Interface::ActionBuilder do
-  describe "::build" do
-    it "should build an action" do
-      action = Puppet::Interface::ActionBuilder.build(nil, :foo) do
+  let :face do Puppet::Interface.new(:puppet_interface_actionbuilder, '0.0.1') end
+
+  it "should build an action" do
+    action = Puppet::Interface::ActionBuilder.build(nil, :foo) do
+    end
+    action.should be_a(Puppet::Interface::Action)
+    action.name.should == :foo
+  end
+
+  it "should define a method on the face which invokes the action" do
+    face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1') do
+      action(:foo) { when_invoked { "invoked the method" } }
+    end
+
+    face.foo.should == "invoked the method"
+  end
+
+  it "should require a block" do
+    expect { Puppet::Interface::ActionBuilder.build(nil, :foo) }.
+      should raise_error("Action :foo must specify a block")
+  end
+
+  describe "when handling options" do
+    it "should have a #option DSL function" do
+      method = nil
+      Puppet::Interface::ActionBuilder.build(face, :foo) do
+        method = self.method(:option)
       end
-      action.should be_a(Puppet::Interface::Action)
-      action.name.should == :foo
+      method.should be
     end
 
-    it "should define a method on the face which invokes the action" do
-      face = Puppet::Interface.new(:action_builder_test_interface, '0.0.1') do
-        action(:foo) { when_invoked { "invoked the method" } }
+    it "should define an option without a block" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        option "--bar"
+      end
+      action.should be_option :bar
+    end
+
+    it "should accept an empty block" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        option "--bar" do
+          # This space left deliberately blank.
+        end
       end
+      action.should be_option :bar
+    end
+  end
 
-      face.foo.should == "invoked the method"
+  context "inline documentation" do
+    it "should set the summary" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        summary "this is some text"
+      end
+      action.summary.should == "this is some text"
     end
+  end
 
-    it "should require a block" do
-      expect { Puppet::Interface::ActionBuilder.build(nil, :foo) }.
-        should raise_error("Action :foo must specify a block")
+  context "action defaulting" do
+    it "should set the default to true" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        default
+      end
+      action.default.should be_true
     end
 
-    describe "when handling options" do
-      let :face do Puppet::Interface.new(:option_handling, '0.0.1') end
+    it "should not be default by, er, default. *cough*" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do end
+      action.default.should be_false
+    end
+  end
 
-      it "should have a #option DSL function" do
-        method = nil
+  context "#when_rendering" do
+    it "should fail if no rendering format is given" do
+      expect {
         Puppet::Interface::ActionBuilder.build(face, :foo) do
-          method = self.method(:option)
+          when_rendering do true end
         end
-        method.should be
-      end
+      }.to raise_error ArgumentError, /must give a rendering format to when_rendering/
+    end
 
-      it "should define an option without a block" do
-        action = Puppet::Interface::ActionBuilder.build(face, :foo) do
-          option "--bar"
+    it "should fail if no block is given" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_rendering :json
         end
-        action.should be_option :bar
-      end
+      }.to raise_error ArgumentError, /must give a block to when_rendering/
+    end
 
-      it "should accept an empty block" do
-        action = Puppet::Interface::ActionBuilder.build(face, :foo) do
-          option "--bar" do
-            # This space left deliberately blank.
-          end
+    it "should fail if the block takes no arguments" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_rendering :json do true end
         end
-        action.should be_option :bar
-      end
+      }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/
     end
 
-    context "inline documentation" do
-      let :face do Puppet::Interface.new(:inline_action_docs, '0.0.1') end
+    it "should fail if the block takes more than one argument" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_rendering :json do |a, b, c| true end
+        end
+      }.to raise_error ArgumentError, /when_rendering methods take one argument, the result, not/
+    end
 
-      it "should set the summary" do
-        action = Puppet::Interface::ActionBuilder.build(face, :foo) do
-          summary "this is some text"
+    it "should fail if the block takes a variable number of arguments" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_rendering :json do |*args| true end
+        end
+      }.to raise_error(ArgumentError,
+                       /when_rendering methods take one argument, the result, not/)
+    end
+
+    it "should stash a rendering block" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_rendering :json do |a| true end
+      end
+      action.when_rendering(:json).should be_an_instance_of UnboundMethod
+    end
+
+    it "should fail if you try to set the same rendering twice" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          when_rendering :json do |a| true end
+          when_rendering :json do |a| true end
         end
-        action.summary.should == "this is some text"
+      }.to raise_error ArgumentError, /You can't define a rendering method for json twice/
+    end
+
+    it "should work if you set two different renderings" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_rendering :json do |a| true end
+        when_rendering :yaml do |a| true end
       end
+      action.when_rendering(:json).should be_an_instance_of UnboundMethod
+      action.when_rendering(:yaml).should be_an_instance_of UnboundMethod
     end
+  end
 
-    context "action defaulting" do
-      let :face do Puppet::Interface.new(:default_action, '0.0.1') end
+  context "#render_as" do
+    it "should default to :for_humans" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do end
+      action.render_as.should == :for_humans
+    end
+
+    it "should fail if not rendering format is given" do
+      expect {
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          render_as
+        end
+      }.to raise_error ArgumentError, /must give a rendering format to render_as/
+    end
 
-      it "should set the default to true" do
+    Puppet::Network::FormatHandler.formats.each do |name|
+      it "should accept #{name.inspect} format" do
         action = Puppet::Interface::ActionBuilder.build(face, :foo) do
-          default
+          render_as name
         end
-        action.default.should be_true
+        action.render_as.should == name
       end
+    end
 
-      it "should not be default by, er, default. *cough*" do
-        action = Puppet::Interface::ActionBuilder.build(face, :foo) do end
-        action.default.should be_false
+    it "should accept :for_humans format" do
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        render_as :for_humans
+      end
+      action.render_as.should == :for_humans
+    end
+
+    [:if_you_define_this_format_you_frighten_me, "json", 12].each do |input|
+      it "should fail if given #{input.inspect}" do
+        expect {
+          Puppet::Interface::ActionBuilder.build(face, :foo) do
+            render_as input
+          end
+        }.to raise_error ArgumentError, /#{input.inspect} is not a valid rendering format/
       end
     end
   end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index fe2409b35..07853e7b7 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -371,4 +371,13 @@ describe Puppet::Interface::Action do
       end
     end
   end
+
+  context "#when_rendering" do
+    it "should fail if no type is given when_rendering"
+    it "should accept a when_rendering block"
+    it "should accept multiple when_rendering blocks"
+    it "should fail if when_rendering gets a non-symbol identifier"
+    it "should fail if a second block is given for the same type"
+    it "should return the block if asked"
+  end
 end
-- 
cgit