[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

Daniel Pittman daniel at puppetlabs.com
Tue May 10 08:10:32 UTC 2011


The following commit has been merged in the experimental branch:
commit 5bba1a26140cd3326739b00c2d60dff9321d4044
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Thu Mar 24 13:10:27 2011 -0700

    (#6749) Implement support for options on strings and actions.
    
    We want to support both strings and actions specifying options, to support
    generic wrappers that present strings to the user across multiple distinct
    front-ends.
    
    At the moment we focus on implementation of a generic CLI providing full
    control to all the strings, but we aim to support other programmatic
    interfaces including Ruby and RPC invocation as part of the overall change.
    
    We also detect, at the time they are declared, duplicate options.  They are
    reported, like any duplicate, with an error thrown.  Specifically:
    
    It is illegal to declare a duplicate option in the same scope, such as within
    the same string, or within the same action.  This is unchanged.
    
    It is illegal to declare an option in an action that duplicates an option in
    the string, or vice-versa.  This is reported when the duplicate is declared,
    so may report on either the string or action depending on sequence.
    
    It remains legal to duplicate the same option across multiple actions, with
    different meanings.  There is no conflict, as the same option can't be passed
    to both simultaneously.
    
    Reviewed-By: Pieter van de Bruggen <pieter at puppetlabs.com>

diff --git a/lib/puppet/string.rb b/lib/puppet/string.rb
index 04db1f3..517cf45 100644
--- a/lib/puppet/string.rb
+++ b/lib/puppet/string.rb
@@ -2,12 +2,16 @@ require 'puppet'
 require 'puppet/util/autoload'
 
 class Puppet::String
-  require 'puppet/string/action_manager'
   require 'puppet/string/string_collection'
 
+  require 'puppet/string/action_manager'
   include Puppet::String::ActionManager
   extend Puppet::String::ActionManager
 
+  require 'puppet/string/option_manager'
+  include Puppet::String::OptionManager
+  extend Puppet::String::OptionManager
+
   include Puppet::Util
 
   class << self
@@ -58,7 +62,7 @@ class Puppet::String
 
   def initialize(name, version, &block)
     unless Puppet::String::StringCollection.validate_version(version)
-      raise ArgumentError, "Cannot create string with invalid version number '#{version}'!"
+      raise ArgumentError, "Cannot create string #{name.inspect} with invalid version number '#{version}'!"
     end
 
     @name = Puppet::String::StringCollection.underscorize(name)
diff --git a/lib/puppet/string/action.rb b/lib/puppet/string/action.rb
index 5a7f3f2..4219aca 100644
--- a/lib/puppet/string/action.rb
+++ b/lib/puppet/string/action.rb
@@ -1,4 +1,5 @@
 require 'puppet/string'
+require 'puppet/string/option'
 
 class Puppet::String::Action
   attr_reader :name
@@ -8,11 +9,10 @@ class Puppet::String::Action
   end
 
   def initialize(string, name, attrs = {})
-    name = name.to_s
-    raise "'#{name}' is an invalid action name" unless name =~ /^[a-z]\w*$/
-
-    @string = string
-    @name      = name
+    raise "#{name.inspect} is an invalid action name" unless name.to_s =~ /^[a-z]\w*$/
+    @string  = string
+    @name    = name.to_sym
+    @options = {}
     attrs.each do |k,v| send("#{k}=", v) end
   end
 
@@ -27,4 +27,26 @@ class Puppet::String::Action
       @string.meta_def(@name, &block)
     end
   end
+
+  def add_option(option)
+    if option? option.name then
+      raise ArgumentError, "#{option.name} duplicates an existing option on #{self}"
+    elsif @string.option? option.name then
+      raise ArgumentError, "#{option.name} duplicates an existing option on #{@string}"
+    end
+
+    @options[option.name] = option
+  end
+
+  def option?(name)
+    @options.include? name.to_sym
+  end
+
+  def options
+    (@options.keys + @string.options).sort
+  end
+
+  def get_option(name)
+    @options[name.to_sym] || @string.get_option(name)
+  end
 end
diff --git a/lib/puppet/string/action_builder.rb b/lib/puppet/string/action_builder.rb
index b3db511..fb2a749 100644
--- a/lib/puppet/string/action_builder.rb
+++ b/lib/puppet/string/action_builder.rb
@@ -5,10 +5,8 @@ class Puppet::String::ActionBuilder
   attr_reader :action
 
   def self.build(string, name, &block)
-    name = name.to_s
-    raise "Action '#{name}' must specify a block" unless block
-    builder = new(string, name, &block)
-    builder.action
+    raise "Action #{name.inspect} must specify a block" unless block
+    new(string, name, &block).action
   end
 
   def initialize(string, name, &block)
@@ -24,4 +22,9 @@ class Puppet::String::ActionBuilder
     raise "Invoke called on an ActionBuilder with no corresponding Action" unless @action
     @action.invoke = block
   end
+
+  def option(name, attrs = {}, &block)
+    option = Puppet::String::OptionBuilder.build(@action, name, attrs, &block)
+    @action.add_option(option)
+  end
 end
diff --git a/lib/puppet/string/action_manager.rb b/lib/puppet/string/action_manager.rb
index c29dbf4..c980142 100644
--- a/lib/puppet/string/action_manager.rb
+++ b/lib/puppet/string/action_manager.rb
@@ -5,20 +5,15 @@ module Puppet::String::ActionManager
   # the code to do so.
   def action(name, &block)
     @actions ||= {}
-    name = name.to_s.downcase.to_sym
-
     raise "Action #{name} already defined for #{self}" if action?(name)
-
     action = Puppet::String::ActionBuilder.build(self, name, &block)
-
-    @actions[name] = action
+    @actions[action.name] = action
   end
 
   # This is the short-form of an action definition; it doesn't use the
   # builder, just creates the action directly from the block.
   def script(name, &block)
     @actions ||= {}
-    name = name.to_s.downcase.to_sym
     raise "Action #{name} already defined for #{self}" if action?(name)
     @actions[name] = Puppet::String::Action.new(self, name, :invoke => block)
   end
@@ -36,7 +31,8 @@ module Puppet::String::ActionManager
   end
 
   def get_action(name)
-    @actions[name].dup
+    @actions ||= {}
+    @actions[name.to_sym]
   end
 
   def action?(name)
diff --git a/lib/puppet/string/option.rb b/lib/puppet/string/option.rb
new file mode 100644
index 0000000..bdc3e07
--- /dev/null
+++ b/lib/puppet/string/option.rb
@@ -0,0 +1,24 @@
+class Puppet::String::Option
+  attr_reader :name, :string
+
+  def initialize(string, name, attrs = {})
+    raise "#{name.inspect} is an invalid option name" unless name.to_s =~ /^[a-z]\w*$/
+    @string = string
+    @name   = name.to_sym
+    attrs.each do |k,v| send("#{k}=", v) end
+  end
+
+  def to_s
+    @name.to_s.tr('_', '-')
+  end
+
+  Types = [:boolean, :string]
+  def type
+    @type ||= :boolean
+  end
+  def type=(input)
+    value = begin input.to_sym rescue nil end
+    Types.include?(value) or raise ArgumentError, "#{input.inspect} is not a valid type"
+    @type = value
+  end
+end
diff --git a/lib/puppet/string/option_builder.rb b/lib/puppet/string/option_builder.rb
new file mode 100644
index 0000000..2087cbc
--- /dev/null
+++ b/lib/puppet/string/option_builder.rb
@@ -0,0 +1,25 @@
+require 'puppet/string/option'
+
+class Puppet::String::OptionBuilder
+  attr_reader :option
+
+  def self.build(string, name, attrs = {}, &block)
+    new(string, name, attrs, &block).option
+  end
+
+  private
+  def initialize(string, name, attrs, &block)
+    @string = string
+    @option = Puppet::String::Option.new(string, name, attrs)
+    block and instance_eval(&block)
+    @option
+  end
+
+  # Metaprogram the simple DSL from the option class.
+  Puppet::String::Option.instance_methods.grep(/=$/).each do |setter|
+    next if setter =~ /^=/      # special case, darn it...
+
+    dsl = setter.sub(/=$/, '')
+    define_method(dsl) do |value| @option.send(setter, value) end
+  end
+end
diff --git a/lib/puppet/string/option_manager.rb b/lib/puppet/string/option_manager.rb
new file mode 100644
index 0000000..df3ae6b
--- /dev/null
+++ b/lib/puppet/string/option_manager.rb
@@ -0,0 +1,46 @@
+require 'puppet/string/option_builder'
+
+module Puppet::String::OptionManager
+  # Declare that this app can take a specific option, and provide
+  # the code to do so.
+  def option(name, attrs = {}, &block)
+    @options ||= {}
+    raise ArgumentError, "Option #{name} already defined for #{self}" if option?(name)
+    actions.each do |action|
+      if get_action(action).option?(name) then
+        raise ArgumentError, "Option #{name} already defined on action #{action} for #{self}"
+      end
+    end
+    option = Puppet::String::OptionBuilder.build(self, name, &block)
+    @options[option.name] = option
+  end
+
+  def options
+    @options ||= {}
+    result = @options.keys
+
+    if self.is_a?(Class) and superclass.respond_to?(:options)
+      result += superclass.options
+    elsif self.class.respond_to?(:options)
+      result += self.class.options
+    end
+    result.sort
+  end
+
+  def get_option(name)
+    @options ||= {}
+    result = @options[name.to_sym]
+    unless result then
+      if self.is_a?(Class) and superclass.respond_to?(:get_option)
+        result = superclass.get_option(name)
+      elsif self.class.respond_to?(:get_option)
+        result = self.class.get_option(name)
+      end
+    end
+    return result
+  end
+
+  def option?(name)
+    options.include? name.to_sym
+  end
+end
diff --git a/spec/unit/string/action_builder_spec.rb b/spec/unit/string/action_builder_spec.rb
index c3395cf..946244c 100755
--- a/spec/unit/string/action_builder_spec.rb
+++ b/spec/unit/string/action_builder_spec.rb
@@ -9,7 +9,7 @@ describe Puppet::String::ActionBuilder do
       action = Puppet::String::ActionBuilder.build(nil,:foo) do
       end
       action.should be_a(Puppet::String::Action)
-      action.name.should == "foo"
+      action.name.should == :foo
     end
 
     it "should define a method on the string which invokes the action" do
@@ -24,7 +24,36 @@ describe Puppet::String::ActionBuilder do
     end
 
     it "should require a block" do
-      lambda { Puppet::String::ActionBuilder.build(nil,:foo) }.should raise_error("Action 'foo' must specify a block")
+      lambda { Puppet::String::ActionBuilder.build(nil,:foo) }.
+        should raise_error("Action :foo must specify a block")
+    end
+
+    describe "when handling options" do
+      let :string do Puppet::String.new(:option_handling, '0.0.1') end
+
+      it "should have a #option DSL function" do
+        method = nil
+        Puppet::String::ActionBuilder.build(string, :foo) do
+          method = self.method(:option)
+        end
+        method.should be
+      end
+
+      it "should define an option without a block" do
+        action = Puppet::String::ActionBuilder.build(string, :foo) do
+          option :bar
+        end
+        action.should be_option :bar
+      end
+
+      it "should accept an empty block" do
+        action = Puppet::String::ActionBuilder.build(string, :foo) do
+          option :bar do
+            # This space left deliberately blank.
+          end
+        end
+        action.should be_option :bar
+      end
     end
   end
 end
diff --git a/spec/unit/string/action_spec.rb b/spec/unit/string/action_spec.rb
index f4ca831..d182f0a 100755
--- a/spec/unit/string/action_spec.rb
+++ b/spec/unit/string/action_spec.rb
@@ -63,4 +63,86 @@ describe Puppet::String::Action do
       string.qux.should  == "the value of foo in baz is '25'"
     end
   end
+
+  describe "with action-level options" do
+    it "should support options without arguments" do
+      string = Puppet::String.new(:action_level_options, '0.0.1') do
+        action(:foo) do
+          option :bar
+        end
+      end
+
+      string.should_not be_option :bar
+      string.get_action(:foo).should be_option :bar
+    end
+
+    it "should support options with an empty block" do
+      string = Puppet::String.new(:action_level_options, '0.0.1') do
+        action :foo do
+          option :bar do
+            # this line left deliberately blank
+          end
+        end
+      end
+
+      string.should_not be_option :bar
+      string.get_action(:foo).should be_option :bar
+    end
+
+    it "should return only action level options when there are no string options" do
+      string = Puppet::String.new(:action_level_options, '0.0.1') do
+        action :foo do option :bar end
+      end
+
+      string.get_action(:foo).options.should =~ [:bar]
+    end
+
+    describe "with both string and action options" do
+      let :string do
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          action :foo do option :bar end
+          action :baz do option :bim end
+          option :quux
+        end
+      end
+
+      it "should return combined string and action options" do
+        string.get_action(:foo).options.should =~ [:bar, :quux]
+      end
+
+      it "should get an action option when asked" do
+        string.get_action(:foo).get_option(:bar).
+          should be_an_instance_of Puppet::String::Option
+      end
+
+      it "should get a string option when asked" do
+        string.get_action(:foo).get_option(:quux).
+          should be_an_instance_of Puppet::String::Option
+      end
+
+      it "should return options only for this action" do
+        string.get_action(:baz).options.should =~ [:bim, :quux]
+      end
+    end
+
+    it "should fail when a duplicate option is added" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          action :foo do
+            option :foo
+            option :foo
+          end
+        end
+      }.should raise_error ArgumentError, /foo duplicates an existing option/
+    end
+
+    it "should fail when a string option duplicates an action option" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          option :foo
+          action :bar do option :foo end
+        end
+      }.should raise_error ArgumentError, /duplicates an existing option .*action_level/i
+    end
+  end
 end
diff --git a/spec/unit/string/option_builder_spec.rb b/spec/unit/string/option_builder_spec.rb
new file mode 100644
index 0000000..6857878
--- /dev/null
+++ b/spec/unit/string/option_builder_spec.rb
@@ -0,0 +1,57 @@
+require 'puppet/string/option_builder'
+
+describe Puppet::String::OptionBuilder do
+  let :string do Puppet::String.new(:option_builder_testing, '0.0.1') end
+
+  it "should be able to construct an option without a block" do
+    Puppet::String::OptionBuilder.build(string, :foo).
+      should be_an_instance_of Puppet::String::Option
+  end
+
+  it "should set attributes during construction" do
+    # Walk all types, since at least one of them should be non-default...
+    Puppet::String::Option::Types.each do |type|
+      option = Puppet::String::OptionBuilder.build(string, :foo, :type => type)
+      option.should be_an_instance_of Puppet::String::Option
+      option.type.should == type
+    end
+  end
+
+  describe "when using the DSL block" do
+    it "should work with an empty block" do
+      option = Puppet::String::OptionBuilder.build(string, :foo) do
+        # This block deliberately left blank.
+      end
+
+      option.should be_an_instance_of Puppet::String::Option
+    end
+
+    describe "#type" do
+      Puppet::String::Option::Types.each do |valid|
+        it "should accept #{valid.inspect}" do
+          option = Puppet::String::OptionBuilder.build(string, :foo) do
+            type valid
+          end
+          option.should be_an_instance_of Puppet::String::Option
+        end
+
+        it "should accept #{valid.inspect} as a string" do
+          option = Puppet::String::OptionBuilder.build(string, :foo) do
+            type valid.to_s
+          end
+          option.should be_an_instance_of Puppet::String::Option
+        end
+
+        [:foo, nil, true, false, 12, '12', 'whatever', ::String, URI].each do |input|
+          it "should reject #{input.inspect}" do
+            expect {
+              Puppet::String::OptionBuilder.build(string, :foo) do
+                type input
+              end
+            }.should raise_error ArgumentError, /not a valid type/
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/unit/string/option_spec.rb b/spec/unit/string/option_spec.rb
new file mode 100644
index 0000000..9bb4309
--- /dev/null
+++ b/spec/unit/string/option_spec.rb
@@ -0,0 +1,61 @@
+require 'puppet/string/option'
+
+describe Puppet::String::Option do
+  let :string do Puppet::String.new(:option_testing, '0.0.1') end
+
+  it "requires a string when created" do
+    expect { Puppet::String::Option.new }.
+      should raise_error ArgumentError, /wrong number of arguments/
+  end
+
+  it "also requires a name when created" do
+    expect { Puppet::String::Option.new(string) }.
+      should raise_error ArgumentError, /wrong number of arguments/
+  end
+
+  it "should create an instance when given a string and name" do
+    Puppet::String::Option.new(string, :foo).
+      should be_instance_of Puppet::String::Option
+  end
+
+  describe "#to_s" do
+    it "should transform a symbol into a string" do
+      Puppet::String::Option.new(string, :foo).to_s.should == "foo"
+    end
+
+    it "should use - rather than _ to separate words" do
+      Puppet::String::Option.new(string, :foo_bar).to_s.should == "foo-bar"
+    end
+  end
+
+  describe "#type" do
+    Puppet::String::Option::Types.each do |type|
+      it "should accept #{type.inspect}" do
+        Puppet::String::Option.new(string, :foo, :type => type).
+          should be_an_instance_of Puppet::String::Option
+      end
+
+      it "should accept #{type.inspect} when given as a string" do
+        Puppet::String::Option.new(string, :foo, :type => type.to_s).
+          should be_an_instance_of Puppet::String::Option
+      end
+    end
+
+    [:foo, nil, true, false, 12, '12', 'whatever', ::String, URI].each do |input|
+      it "should reject #{input.inspect}" do
+        expect { Puppet::String::Option.new(string, :foo, :type => input) }.
+          should raise_error ArgumentError, /not a valid type/
+      end
+    end
+  end
+
+
+  # name         short  value          type
+  # ca-location         CA_LOCATION    string
+  # debug        d      ----           boolean
+  # verbose      v      ----           boolean
+  # terminus            TERMINUS       string
+  # format              FORMAT         symbol
+  # mode         r      RUNMODE        limited set of symbols
+  # server              URL            URL
+end
diff --git a/spec/unit/string_spec.rb b/spec/unit/string_spec.rb
index 64d4f12..5775051 100755
--- a/spec/unit/string_spec.rb
+++ b/spec/unit/string_spec.rb
@@ -81,4 +81,93 @@ describe Puppet::String do
   end
 
   it "should be able to load all actions in all search paths"
+
+  describe "with string-level options" do
+    it "should support options without arguments" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        option :foo
+      end
+      string.should be_an_instance_of Puppet::String
+      string.should be_option :foo
+    end
+
+    it "should support options with an empty block" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        option :foo do
+          # this section deliberately left blank
+        end
+      end
+      string.should be_an_instance_of Puppet::String
+      string.should be_option :foo
+    end
+
+    it "should return all the string-level options" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        option :foo
+        option :bar
+      end
+      string.options.should =~ [:foo, :bar]
+    end
+
+    it "should not return any action-level options" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        option :foo
+        option :bar
+        action :baz do
+          option :quux
+        end
+      end
+      string.options.should =~ [:foo, :bar]
+    end
+
+    it "should fail when a duplicate option is added" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          option :foo
+          option :foo
+        end
+      }.should raise_error ArgumentError, /option foo already defined for/i
+    end
+
+    it "should fail when a string option duplicates an action option" do
+      expect {
+        Puppet::String.new(:action_level_options, '0.0.1') do
+          action :bar do option :foo end
+          option :foo
+        end
+      }.should raise_error ArgumentError, /foo already defined on action bar/i
+    end
+
+    it "should work when two actions have the same option" do
+      string = Puppet::String.new(:with_options, '0.0.1') do
+        action :foo do option :quux end
+        action :bar do option :quux end
+      end
+
+      string.get_action(:foo).options.should =~ [:quux]
+      string.get_action(:bar).options.should =~ [:quux]
+    end
+  end
+
+  describe "with inherited options" do
+    let :string do
+      parent = Class.new(Puppet::String)
+      parent.option(:inherited, :type => :string)
+      string = parent.new(:example, '0.2.1')
+      string.option(:local)
+      string
+    end
+
+    describe "#options" do
+      it "should list inherited options" do
+        string.options.should =~ [:inherited, :local]
+      end
+    end
+
+    describe "#get_option" do
+      it "should return an inherited option object" do
+        string.get_option(:inherited).should be_an_instance_of Puppet::String::Option
+      end
+    end
+  end
 end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list