(#7012) Split plumbing into Puppet::Interface

This splits out the plumbing into the Puppet::Interface namespace, and uses
Puppet::Faces for all the public-facing code.

The fault line is "what you care about if you are using or writing a face",
which is public, against "what you care about to enable either of those two",
which is the plumbing.

6 files changed, 753 insertions, 0 deletions

diff --git a/spec/unit/interface/action_builder_spec.rb b/spec/unit/interface/action_builder_spec.rb
new file mode 100755
index 000000000..ae9cc83d4
--- /dev/null
+++ b/spec/unit/interface/action_builder_spec.rb
@@ -0,0 +1,59 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+require 'puppet/interface/action_builder'
+
+describe Puppet::Interface::ActionBuilder do
+  describe "::build" do
+    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')
+      action = Puppet::Interface::ActionBuilder.build(face, :foo) do
+        when_invoked do
+          "invoked the method"
+        end
+      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
+      let :face do Puppet::Interface.new(:option_handling, '0.0.1') end
+
+      it "should have a #option DSL function" do
+        method = nil
+        Puppet::Interface::ActionBuilder.build(face, :foo) do
+          method = self.method(:option)
+        end
+        method.should be
+      end
+
+      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
+  end
+end
diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
new file mode 100755
index 000000000..50bea5f89
--- /dev/null
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -0,0 +1,233 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+
+# This is entirely an internal class for Interface, so we have to load it instead of our class.
+require 'puppet/interface'
+
+class ActionManagerTester
+  include Puppet::Interface::ActionManager
+end
+
+describe Puppet::Interface::ActionManager do
+  subject { ActionManagerTester.new }
+
+  describe "when included in a class" do
+    it "should be able to define an action" do
+      subject.action(:foo) do
+        when_invoked { "something "}
+      end
+    end
+
+    it "should be able to define a 'script' style action" do
+      subject.script :bar do
+        "a bar is where beer is found"
+      end
+    end
+
+    it "should be able to list defined actions" do
+      subject.action(:foo) do
+        when_invoked { "something" }
+      end
+      subject.action(:bar) do
+        when_invoked { "something" }
+      end
+
+      subject.actions.should =~ [:foo, :bar]
+    end
+
+    it "should list 'script' actions" do
+      subject.script :foo do "foo" end
+      subject.actions.should =~ [:foo]
+    end
+
+    it "should list both script and normal actions" do
+      subject.action :foo do
+        when_invoked do "foo" end
+      end
+      subject.script :bar do "a bar is where beer is found" end
+
+      subject.actions.should =~ [:foo, :bar]
+    end
+
+    it "should be able to indicate when an action is defined" do
+      subject.action(:foo) do
+        when_invoked { "something" }
+      end
+
+      subject.should be_action(:foo)
+    end
+
+    it "should indicate an action is defined for script actions" do
+      subject.script :foo do "foo" end
+      subject.should be_action :foo
+    end
+
+    it "should correctly treat action names specified as strings" do
+      subject.action(:foo) do
+        when_invoked { "something" }
+      end
+
+      subject.should be_action("foo")
+    end
+  end
+
+  describe "when used to extend a class" do
+    subject { Class.new.extend(Puppet::Interface::ActionManager) }
+
+    it "should be able to define an action" do
+      subject.action(:foo) do
+        when_invoked { "something "}
+      end
+    end
+
+    it "should be able to list defined actions" do
+      subject.action(:foo) do
+        when_invoked { "something" }
+      end
+      subject.action(:bar) do
+        when_invoked { "something" }
+      end
+
+      subject.actions.should include(:bar)
+      subject.actions.should include(:foo)
+    end
+
+    it "should be able to indicate when an action is defined" do
+      subject.action(:foo) { "something" }
+      subject.should be_action(:foo)
+    end
+  end
+
+  describe "when used both at the class and instance level" do
+    before do
+      @klass = Class.new do
+        include Puppet::Interface::ActionManager
+        extend Puppet::Interface::ActionManager
+      end
+      @instance = @klass.new
+    end
+
+    it "should be able to define an action at the class level" do
+      @klass.action(:foo) do
+        when_invoked { "something "}
+      end
+    end
+
+    it "should create an instance method when an action is defined at the class level" do
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.foo.should == "something"
+    end
+
+    it "should be able to define an action at the instance level" do
+      @instance.action(:foo) do
+        when_invoked { "something "}
+      end
+    end
+
+    it "should create an instance method when an action is defined at the instance level" do
+      @instance.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.foo.should == "something"
+    end
+
+    it "should be able to list actions defined at the class level" do
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @klass.action(:bar) do
+        when_invoked { "something" }
+      end
+
+      @klass.actions.should include(:bar)
+      @klass.actions.should include(:foo)
+    end
+
+    it "should be able to list actions defined at the instance level" do
+      @instance.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.action(:bar) do
+        when_invoked { "something" }
+      end
+
+      @instance.actions.should include(:bar)
+      @instance.actions.should include(:foo)
+    end
+
+    it "should be able to list actions defined at both instance and class level" do
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.action(:bar) do
+        when_invoked { "something" }
+      end
+
+      @instance.actions.should include(:bar)
+      @instance.actions.should include(:foo)
+    end
+
+    it "should be able to indicate when an action is defined at the class level" do
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.should be_action(:foo)
+    end
+
+    it "should be able to indicate when an action is defined at the instance level" do
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.should be_action(:foo)
+    end
+
+    it "should list actions defined in superclasses" do
+      @subclass = Class.new(@klass)
+      @instance = @subclass.new
+
+      @klass.action(:parent) do
+        when_invoked { "a" }
+      end
+      @subclass.action(:sub) do
+        when_invoked { "a" }
+      end
+      @instance.action(:instance) do
+        when_invoked { "a" }
+      end
+
+      @instance.should be_action(:parent)
+      @instance.should be_action(:sub)
+      @instance.should be_action(:instance)
+    end
+
+    it "should create an instance method when an action is defined in a superclass" do
+      @subclass = Class.new(@klass)
+      @instance = @subclass.new
+
+      @klass.action(:foo) do
+        when_invoked { "something" }
+      end
+      @instance.foo.should == "something"
+    end
+  end
+
+  describe "#get_action" do
+    let :parent_class do
+      parent_class = Class.new(Puppet::Interface)
+      parent_class.action(:foo) {}
+      parent_class
+    end
+
+    it "should check that we can find inherited actions when we are a class" do
+      Class.new(parent_class).get_action(:foo).name.should == :foo
+    end
+
+    it "should check that we can find inherited actions when we are an instance" do
+      instance = parent_class.new(:foo, '0.0.0')
+      instance.get_action(:foo).name.should == :foo
+    end
+  end
+end
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
new file mode 100755
index 000000000..4801a3cc8
--- /dev/null
+++ b/spec/unit/interface/action_spec.rb
@@ -0,0 +1,173 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+require 'puppet/interface/action'
+
+describe Puppet::Interface::Action do
+  describe "when validating the action name" do
+    [nil, '', 'foo bar', '-foobar'].each do |input|
+      it "should treat #{input.inspect} as an invalid name" do
+        expect { Puppet::Interface::Action.new(nil, input) }.
+          should raise_error(/is an invalid action name/)
+      end
+    end
+  end
+
+  describe "when invoking" do
+    it "should be able to call other actions on the same object" do
+      face = Puppet::Interface.new(:my_face, '0.0.1') do
+        action(:foo) do
+          when_invoked { 25 }
+        end
+
+        action(:bar) do
+          when_invoked { "the value of foo is '#{foo}'" }
+        end
+      end
+      face.foo.should == 25
+      face.bar.should == "the value of foo is '25'"
+    end
+
+    # bar is a class action calling a class action
+    # quux is a class action calling an instance action
+    # baz is an instance action calling a class action
+    # qux is an instance action calling an instance action
+    it "should be able to call other actions on the same object when defined on a class" do
+      class Puppet::Interface::MyInterfaceBaseClass < Puppet::Interface
+        action(:foo) do
+          when_invoked { 25 }
+        end
+
+        action(:bar) do
+          when_invoked { "the value of foo is '#{foo}'" }
+        end
+
+        action(:quux) do
+          when_invoked { "qux told me #{qux}" }
+        end
+      end
+
+      face = Puppet::Interface::MyInterfaceBaseClass.new(:my_inherited_face, '0.0.1') do
+        action(:baz) do
+          when_invoked { "the value of foo in baz is '#{foo}'" }
+        end
+
+        action(:qux) do
+          when_invoked { baz }
+        end
+      end
+      face.foo.should  == 25
+      face.bar.should  == "the value of foo is '25'"
+      face.quux.should == "qux told me the value of foo in baz is '25'"
+      face.baz.should  == "the value of foo in baz is '25'"
+      face.qux.should  == "the value of foo in baz is '25'"
+    end
+
+    context "when calling the Ruby API" do
+      let :face do
+        Puppet::Interface.new(:ruby_api, '1.0.0') do
+          action :bar do
+            when_invoked do |options|
+              options
+            end
+          end
+        end
+      end
+
+      it "should work when no options are supplied" do
+        options = face.bar
+        options.should == {}
+      end
+
+      it "should work when options are supplied" do
+        options = face.bar :bar => "beer"
+        options.should == { :bar => "beer" }
+      end
+    end
+  end
+
+  describe "with action-level options" do
+    it "should support options with an empty block" do
+      face = Puppet::Interface.new(:action_level_options, '0.0.1') do
+        action :foo do
+          option "--bar" do
+            # this line left deliberately blank
+          end
+        end
+      end
+
+      face.should_not be_option :bar
+      face.get_action(:foo).should be_option :bar
+    end
+
+    it "should return only action level options when there are no face options" do
+      face = Puppet::Interface.new(:action_level_options, '0.0.1') do
+        action :foo do option "--bar" end
+      end
+
+      face.get_action(:foo).options.should =~ [:bar]
+    end
+
+    describe "with both face and action options" do
+      let :face do
+        Puppet::Interface.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 face and action options" do
+        face.get_action(:foo).options.should =~ [:bar, :quux]
+      end
+
+      it "should fetch options that the face inherited" do
+        parent = Class.new(Puppet::Interface)
+        parent.option "--foo"
+        child = parent.new(:inherited_options, '0.0.1') do
+          option "--bar"
+          action :action do option "--baz" end
+        end
+
+        action = child.get_action(:action)
+        action.should be
+
+        [:baz, :bar, :foo].each do |name|
+          action.get_option(name).should be_an_instance_of Puppet::Interface::Option
+        end
+      end
+
+      it "should get an action option when asked" do
+        face.get_action(:foo).get_option(:bar).
+          should be_an_instance_of Puppet::Interface::Option
+      end
+
+      it "should get a face option when asked" do
+        face.get_action(:foo).get_option(:quux).
+          should be_an_instance_of Puppet::Interface::Option
+      end
+
+      it "should return options only for this action" do
+        face.get_action(:baz).options.should =~ [:bim, :quux]
+      end
+    end
+
+    it_should_behave_like "things that declare options" do
+      def add_options_to(&block)
+        face = Puppet::Interface.new(:with_options, '0.0.1') do
+          action(:foo, &block)
+        end
+        face.get_action(:foo)
+      end
+    end
+
+    it "should fail when a face option duplicates an action option" do
+      expect {
+        Puppet::Interface.new(:action_level_options, '0.0.1') do
+          option "--foo"
+          action :bar do option "--foo" end
+        end
+      }.should raise_error ArgumentError, /Option foo conflicts with existing option foo/i
+    end
+  end
+end
diff --git a/spec/unit/interface/face_collection_spec.rb b/spec/unit/interface/face_collection_spec.rb
new file mode 100755
index 000000000..de6d29cee
--- /dev/null
+++ b/spec/unit/interface/face_collection_spec.rb
@@ -0,0 +1,184 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+require 'tmpdir'
+
+describe Puppet::Interface::FaceCollection do
+  # To avoid cross-pollution we have to save and restore both the hash
+  # containing all the interface data, and the array used by require.  Restoring
+  # both means that we don't leak side-effects across the code. --daniel 2011-04-06
+  before :each do
+    @original_faces    = subject.instance_variable_get("@faces").dup
+    @original_required = $".dup
+    subject.instance_variable_get("@faces").clear
+  end
+
+  after :each do
+    subject.instance_variable_set("@faces", @original_faces)
+    $".clear ; @original_required.each do |item| $" << item end
+  end
+
+  describe "::faces" do
+    it "REVISIT: should have some tests here, if we describe it"
+  end
+
+  describe "::validate_version" do
+    it 'should permit three number versions' do
+      subject.validate_version('10.10.10').should == true
+    end
+
+    it 'should permit versions with appended descriptions' do
+      subject.validate_version('10.10.10beta').should == true
+    end
+
+    it 'should not permit versions with more than three numbers' do
+      subject.validate_version('1.2.3.4').should == false
+    end
+
+    it 'should not permit versions with only two numbers' do
+      subject.validate_version('10.10').should == false
+    end
+
+    it 'should not permit versions with only one number' do
+      subject.validate_version('123').should == false
+    end
+
+    it 'should not permit versions with text in any position but at the end' do
+      subject.validate_version('v1.1.1').should == false
+    end
+  end
+
+  describe "::[]" do
+    before :each do
+      subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10
+    end
+
+    before :each do
+      @dir = Dir.mktmpdir
+      @lib = FileUtils.mkdir_p(File.join @dir, 'puppet', 'faces')
+      $LOAD_PATH.push(@dir)
+    end
+
+    after :each do
+      FileUtils.remove_entry_secure @dir
+      $LOAD_PATH.pop
+    end
+
+    it "should return the faces with the given name" do
+      subject["foo", '0.0.1'].should == 10
+    end
+
+    it "should attempt to load the faces if it isn't found" do
+      subject.expects(:require).with('puppet/faces/bar')
+      subject["bar", '0.0.1']
+    end
+
+    it "should attempt to load the default faces for the specified version :current" do
+      subject.expects(:require).never # except...
+      subject.expects(:require).with('puppet/faces/fozzie')
+      subject['fozzie', :current]
+    end
+  end
+
+  describe "::face?" do
+    before :each do
+      subject.instance_variable_get("@faces")[:foo]['0.0.1'] = 10
+    end
+
+    it "should return true if the faces specified is registered" do
+      subject.face?("foo", '0.0.1').should == true
+    end
+
+    it "should attempt to require the faces if it is not registered" do
+      subject.expects(:require).with do |file|
+        subject.instance_variable_get("@faces")[:bar]['0.0.1'] = true
+        file == 'puppet/faces/bar'
+      end
+      subject.face?("bar", '0.0.1').should == true
+    end
+
+    it "should return true if requiring the faces registered it" do
+      subject.stubs(:require).with do
+        subject.instance_variable_get("@faces")[:bar]['0.0.1'] = 20
+      end
+    end
+
+    it "should return false if the faces is not registered" do
+      subject.stubs(:require).returns(true)
+      subject.face?("bar", '0.0.1').should be_false
+    end
+
+    it "should return false if the faces file itself is missing" do
+      subject.stubs(:require).
+        raises(LoadError, 'no such file to load -- puppet/faces/bar')
+      subject.face?("bar", '0.0.1').should be_false
+    end
+
+    it "should register the version loaded by `:current` as `:current`" do
+      subject.expects(:require).with do |file|
+        subject.instance_variable_get("@faces")[:huzzah]['2.0.1'] = :huzzah_faces
+        file == 'puppet/faces/huzzah'
+      end
+      subject.face?("huzzah", :current)
+      subject.instance_variable_get("@faces")[:huzzah][:current].should == :huzzah_faces
+    end
+
+    context "with something on disk" do
+      before :each do
+        write_scratch_faces :huzzah do |fh|
+          fh.puts <<EOF
+Puppet::Faces.define(:huzzah, '2.0.1') do
+  action :bar do "is where beer comes from" end
+end
+EOF
+        end
+      end
+
+      it "should register the version loaded from `puppet/faces/{name}` as `:current`" do
+        subject.should be_face "huzzah", '2.0.1'
+        subject.should be_face "huzzah", :current
+        Puppet::Faces[:huzzah, '2.0.1'].should == Puppet::Faces[:huzzah, :current]
+      end
+
+      it "should index :current when the code was pre-required" do
+        subject.instance_variable_get("@faces")[:huzzah].should_not be_key :current
+        require 'puppet/faces/huzzah'
+        subject.face?(:huzzah, :current).should be_true
+      end
+    end
+  end
+
+  describe "::register" do
+    it "should store the faces by name" do
+      faces = Puppet::Faces.new(:my_faces, '0.0.1')
+      subject.register(faces)
+      subject.instance_variable_get("@faces").should == {:my_faces => {'0.0.1' => faces}}
+    end
+  end
+
+  describe "::underscorize" do
+    faulty = [1, "#foo", "$bar", "sturm und drang", :"sturm und drang"]
+    valid  = {
+      "Foo"      => :foo,
+      :Foo       => :foo,
+      "foo_bar"  => :foo_bar,
+      :foo_bar   => :foo_bar,
+      "foo-bar"  => :foo_bar,
+      :"foo-bar" => :foo_bar,
+    }
+
+    valid.each do |input, expect|
+      it "should map #{input.inspect} to #{expect.inspect}" do
+        result = subject.underscorize(input)
+        result.should == expect
+      end
+    end
+
+    faulty.each do |input|
+      it "should fail when presented with #{input.inspect} (#{input.class})" do
+        expect { subject.underscorize(input) }.
+          should raise_error ArgumentError, /not a valid face name/
+      end
+    end
+  end
+end
diff --git a/spec/unit/interface/option_builder_spec.rb b/spec/unit/interface/option_builder_spec.rb
new file mode 100644
index 000000000..fae48324e
--- /dev/null
+++ b/spec/unit/interface/option_builder_spec.rb
@@ -0,0 +1,29 @@
+require 'puppet/interface/option_builder'
+
+describe Puppet::Interface::OptionBuilder do
+  let :face do Puppet::Interface.new(:option_builder_testing, '0.0.1') end
+
+  it "should be able to construct an option without a block" do
+    Puppet::Interface::OptionBuilder.build(face, "--foo").
+      should be_an_instance_of Puppet::Interface::Option
+  end
+
+  describe "when using the DSL block" do
+    it "should work with an empty block" do
+      option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+        # This block deliberately left blank.
+      end
+
+      option.should be_an_instance_of Puppet::Interface::Option
+    end
+
+    it "should support documentation declarations" do
+      text = "this is the description"
+      option = Puppet::Interface::OptionBuilder.build(face, "--foo") do
+        desc text
+      end
+      option.should be_an_instance_of Puppet::Interface::Option
+      option.desc.should == text
+    end
+  end
+end
diff --git a/spec/unit/interface/option_spec.rb b/spec/unit/interface/option_spec.rb
new file mode 100644
index 000000000..3bcd121e2
--- /dev/null
+++ b/spec/unit/interface/option_spec.rb
@@ -0,0 +1,75 @@
+require 'puppet/interface/option'
+
+describe Puppet::Interface::Option do
+  let :face do Puppet::Interface.new(:option_testing, '0.0.1') end
+
+  describe "#optparse_to_name" do
+    ["", "=BAR", " BAR", "=bar", " bar"].each do |postfix|
+      { "--foo" => :foo, "-f" => :f }.each do |base, expect|
+        input = base + postfix
+        it "should map #{input.inspect} to #{expect.inspect}" do
+          option = Puppet::Interface::Option.new(face, input)
+          option.name.should == expect
+        end
+      end
+    end
+
+    [:foo, 12, nil, {}, []].each do |input|
+      it "should fail sensible when given #{input.inspect}" do
+        expect { Puppet::Interface::Option.new(face, input) }.
+          should raise_error ArgumentError, /is not valid for an option argument/
+      end
+    end
+
+    ["-foo", "-foo=BAR", "-foo BAR"].each do |input|
+      it "should fail with a single dash for long option #{input.inspect}" do
+        expect { Puppet::Interface::Option.new(face, input) }.
+          should raise_error ArgumentError, /long options need two dashes \(--\)/
+      end
+    end
+  end
+
+  it "requires a face when created" do
+    expect { Puppet::Interface::Option.new }.
+      should raise_error ArgumentError, /wrong number of arguments/
+  end
+
+  it "also requires some declaration arguments when created" do
+    expect { Puppet::Interface::Option.new(face) }.
+      should raise_error ArgumentError, /No option declarations found/
+  end
+
+  it "should infer the name from an optparse string" do
+    option = Puppet::Interface::Option.new(face, "--foo")
+    option.name.should == :foo
+  end
+
+  it "should infer the name when multiple optparse string are given" do
+    option = Puppet::Interface::Option.new(face, "--foo", "-f")
+    option.name.should == :foo
+  end
+
+  it "should prefer the first long option name over a short option name" do
+    option = Puppet::Interface::Option.new(face, "-f", "--foo")
+    option.name.should == :foo
+  end
+
+  it "should create an instance when given a face and name" do
+    Puppet::Interface::Option.new(face, "--foo").
+      should be_instance_of Puppet::Interface::Option
+  end
+
+  describe "#to_s" do
+    it "should transform a symbol into a string" do
+      option = Puppet::Interface::Option.new(face, "--foo")
+      option.name.should == :foo
+      option.to_s.should == "foo"
+    end
+
+    it "should use - rather than _ to separate words in strings but not symbols" do
+      option = Puppet::Interface::Option.new(face, "--foo-bar")
+      option.name.should == :foo_bar
+      option.to_s.should == "foo-bar"
+    end
+  end
+end