(#7013) Add support for required options.

This adds another hook into the generated wrapper, which invokes a method to
validate arguments.  This is used to raise an exception when required options
have not been passed to the method.

Reviewed-By: Daniel Pittman <daniel@puppetlabs.com>

7 files changed, 64 insertions, 13 deletions

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index b94298963..412e39449 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -83,10 +83,10 @@ class Puppet::Interface::Action
     # idea how motivated we were to make this cleaner.  Sorry. --daniel 2011-03-31
 
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
-    file = __FILE__ + "+eval"
-    line = __LINE__ + 1
+    file    = __FILE__ + "+eval"
+    line    = __LINE__ + 1
     wrapper = <<WRAPPER
-def #{@name}(*args, &block)
+def #{@name}(*args)
   if args.last.is_a? Hash then
     options = args.last
   else
@@ -94,6 +94,7 @@ def #{@name}(*args, &block)
   end
 
   action = get_action(#{name.inspect})
+  action.validate_args(args)
   __invoke_decorations(:before, action, args, options)
   rval = self.__send__(#{internal_name.inspect}, *args)
   __invoke_decorations(:after, action, args, options)
@@ -142,6 +143,15 @@ WRAPPER
     option
   end
 
+  def validate_args(args)
+    required = options.map do |name|
+      get_option(name)
+    end.select(&:required?).collect(&:name) - args.last.keys
+
+    return if required.empty?
+    raise ArgumentError, "missing required options (#{required.join(', ')})"
+  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
diff --git a/lib/puppet/interface/option.rb b/lib/puppet/interface/option.rb
index 2abcd4033..c04c2bf67 100644
--- a/lib/puppet/interface/option.rb
+++ b/lib/puppet/interface/option.rb
@@ -74,10 +74,12 @@ class Puppet::Interface::Option
   def optional_argument?
     !!@optional_argument
   end
-
+  def required?
+    !!@required
+  end
 
   attr_reader   :parent, :name, :aliases, :optparse
-  attr_accessor :desc
+  attr_accessor :required, :desc
 
   attr_accessor :before_action
   def before_action=(proc)
diff --git a/lib/puppet/interface/option_builder.rb b/lib/puppet/interface/option_builder.rb
index d4e59a4df..7c2ab89de 100644
--- a/lib/puppet/interface/option_builder.rb
+++ b/lib/puppet/interface/option_builder.rb
@@ -11,7 +11,7 @@ class Puppet::Interface::OptionBuilder
   def initialize(face, *declaration, &block)
     @face   = face
     @option = Puppet::Interface::Option.new(face, *declaration)
-    block and instance_eval(&block)
+    instance_eval(&block) if block_given?
     @option
   end
 
@@ -47,4 +47,8 @@ class Puppet::Interface::OptionBuilder
     end
     @option.after_action = block
   end
+
+  def required(value = true)
+    @option.required = value
+  end
 end
diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
index aecaf198f..50784cc40 100755
--- a/spec/unit/interface/action_builder_spec.rb
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -13,11 +13,7 @@ describe Puppet::Interface::ActionBuilder do
 
     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 do
-          when_invoked do
-            "invoked the method"
-          end
-        end
+        action(:foo) { when_invoked { "invoked the method" } }
       end
 
       face.foo.should == "invoked the method"
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index 6a6c254d4..07d517c18 100755
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -105,6 +105,7 @@ describe Puppet::Interface::ActionManager do
         include Puppet::Interface::ActionManager
         extend Puppet::Interface::ActionManager
         def __invoke_decorations(*args) true end
+        def options() [] end
       end
       @instance = @klass.new
     end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 4be6a1c55..fe2409b35 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -66,8 +66,8 @@ describe Puppet::Interface::Action do
       let :face do
         Puppet::Interface.new(:ruby_api, '1.0.0') do
           action :bar do
-            when_invoked do |options|
-              options
+            when_invoked do |*args|
+              args.last
             end
           end
         end
@@ -82,6 +82,11 @@ describe Puppet::Interface::Action do
         options = face.bar(:bar => "beer")
         options.should == { :bar => "beer" }
       end
+
+      it "should call #validate_args on the action when invoked" do
+        face.get_action(:bar).expects(:validate_args).with([1, :two, 'three', {}])
+        face.bar 1, :two, 'three'
+      end
     end
   end
 
@@ -168,6 +173,24 @@ describe Puppet::Interface::Action do
         end
       }.should raise_error ArgumentError, /Option foo conflicts with existing option foo/i
     end
+
+    it "should fail when a required action option is not provided" do
+      face = Puppet::Interface.new(:required_action_option, '0.0.1') do
+        action(:bar) do
+          option('--foo') { required }
+          when_invoked { }
+        end
+      end
+      expect { face.bar }.to raise_error ArgumentError, /missing required options \(foo\)/
+    end
+
+    it "should fail when a required face option is not provided" do
+      face = Puppet::Interface.new(:required_face_option, '0.0.1') do
+        option('--foo') { required }
+        action(:bar) { when_invoked { } }
+      end
+      expect { face.bar }.to raise_error ArgumentError, /missing required options \(foo\)/
+    end
   end
 
   context "with action decorators" do
diff --git a/spec/unit/interface/option_builder_spec.rb b/spec/unit/interface/option_builder_spec.rb
index b32b316f6..e9346852c 100755
--- a/spec/unit/interface/option_builder_spec.rb
+++ b/spec/unit/interface/option_builder_spec.rb
@@ -56,5 +56,20 @@ describe Puppet::Interface::OptionBuilder do
         end
       end.to raise_error ArgumentError, /takes three arguments/
     end
+
+    it "should support simple required declarations" do
+      opt = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+        required
+      end
+      opt.should be_required
+    end
+
+    it "should support arguments to the required property" do
+      opt = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+        required(false)
+      end
+      opt.should_not be_required
+    end
+
   end
 end