maint: Implement an InterfaceCollection class to manage interfaces

Having an instance variable on class Interface is insufficient for
Interface::Indirector. This also changes the semantics of "Interface.interface"
to handle registration and loading actions, and for "Interface.new" to only
instantiate an Interface. Thus, consumers of the API should typically use
"Interface.interface", unless they have reasons to not want an interface
automatically registered.

Paired-With: Pieter van de Bruggen

15 files changed, 129 insertions, 224 deletions

diff --git a/spec/unit/interface/action_manager_spec.rb b/spec/unit/interface/action_manager_spec.rb
index 0b12db317..bf101b344 100644
--- a/spec/unit/interface/action_manager_spec.rb
+++ b/spec/unit/interface/action_manager_spec.rb
@@ -10,65 +10,60 @@ class ActionManagerTester
 end
 
 describe Puppet::Interface::ActionManager do
-  before do
-    @tester = ActionManagerTester.new
-  end
+  subject { ActionManagerTester.new }
 
   describe "when included in a class" do
     it "should be able to define an action" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to list defined actions" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
-      @tester.action(:bar) do
+      subject.action(:bar) do
         invoke { "something" }
       end
 
-      @tester.actions.should include(:bar)
-      @tester.actions.should include(:foo)
+      subject.actions.should include(:bar)
+      subject.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
 
-      @tester.should be_action(:foo)
+      subject.should be_action(:foo)
     end
   end
 
   describe "when used to extend a class" do
-    before do
-      @tester = Class.new
-      @tester.extend(Puppet::Interface::ActionManager)
-    end
+    subject { Class.new.extend(Puppet::Interface::ActionManager) }
 
     it "should be able to define an action" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something "}
       end
     end
 
     it "should be able to list defined actions" do
-      @tester.action(:foo) do
+      subject.action(:foo) do
         invoke { "something" }
       end
-      @tester.action(:bar) do
+      subject.action(:bar) do
         invoke { "something" }
       end
 
-      @tester.actions.should include(:bar)
-      @tester.actions.should include(:foo)
+      subject.actions.should include(:bar)
+      subject.actions.should include(:foo)
     end
 
     it "should be able to indicate when an action is defined" do
-      @tester.action(:foo) { "something" }
-      @tester.should be_action(:foo)
+      subject.action(:foo) { "something" }
+      subject.should be_action(:foo)
     end
   end
 
diff --git a/spec/unit/interface/catalog_spec.rb b/spec/unit/interface/catalog_spec.rb
index 8eb0040ff..78d62110f 100644
--- a/spec/unit/interface/catalog_spec.rb
+++ b/spec/unit/interface/catalog_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/catalog'
 
-describe Puppet::Interface.interface(:catalog) do
-  before do
-    @interface = Puppet::Interface.interface(:catalog)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'catalog' indirection" do
-    @interface.indirection.name.should == :catalog
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:catalog) do
 end
diff --git a/spec/unit/interface/certificate_request_spec.rb b/spec/unit/interface/certificate_request_spec.rb
index 8a613e5e5..a6ab8d17e 100644
--- a/spec/unit/interface/certificate_request_spec.rb
+++ b/spec/unit/interface/certificate_request_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_request'
 
-describe Puppet::Interface.interface(:certificate_request) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate_request)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate_request' indirection" do
-    @interface.indirection.name.should == :certificate_request
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate_request) do
 end
diff --git a/spec/unit/interface/certificate_revocation_list_spec.rb b/spec/unit/interface/certificate_revocation_list_spec.rb
index 8ee341bef..a98b48d90 100644
--- a/spec/unit/interface/certificate_revocation_list_spec.rb
+++ b/spec/unit/interface/certificate_revocation_list_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate_revocation_list'
 
-describe Puppet::Interface.interface(:certificate_revocation_list) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate_revocation_list)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate_revocation_list' indirection" do
-    @interface.indirection.name.should == :certificate_revocation_list
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate_revocation_list) do
 end
diff --git a/spec/unit/interface/certificate_spec.rb b/spec/unit/interface/certificate_spec.rb
index 47ddcb52e..6a325343f 100644
--- a/spec/unit/interface/certificate_spec.rb
+++ b/spec/unit/interface/certificate_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/certificate'
 
-describe Puppet::Interface.interface(:certificate) do
-  before do
-    @interface = Puppet::Interface.interface(:certificate)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'certificate' indirection" do
-    @interface.indirection.name.should == :certificate
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:certificate) do
 end
diff --git a/spec/unit/interface/config_spec.rb b/spec/unit/interface/config_spec.rb
index 79c65f2ac..e8aafd4a3 100644
--- a/spec/unit/interface/config_spec.rb
+++ b/spec/unit/interface/config_spec.rb
@@ -4,24 +4,16 @@ require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/config'
 
 describe Puppet::Interface.interface(:config) do
-  before do
-    @interface = Puppet::Interface.interface(:config)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface)
-  end
-
   it "should use Settings#print_config_options when asked to print" do
     Puppet.settings.stubs(:puts)
     Puppet.settings.expects(:print_config_options)
-    @interface.print
+    subject.print
   end
 
   it "should set 'configprint' to all desired values and call print_config_options when a specific value is provided" do
     Puppet.settings.stubs(:puts)
     Puppet.settings.expects(:print_config_options)
-    @interface.print("libdir", "ssldir")
+    subject.print("libdir", "ssldir")
     Puppet.settings[:configprint].should == "libdir,ssldir"
   end
 end
diff --git a/spec/unit/interface/facts_spec.rb b/spec/unit/interface/facts_spec.rb
index 03d6410f9..d0f87d3a0 100644
--- a/spec/unit/interface/facts_spec.rb
+++ b/spec/unit/interface/facts_spec.rb
@@ -3,17 +3,13 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/facts'
 
-describe Puppet::Interface.interface(:facts) do
-  before do
-    @interface = Puppet::Interface.interface(:facts)
-  end
-
+describe Puppet::Interface::Indirector.interface(:facts) do
   it "should define an 'upload' fact" do
-    @interface.should be_action(:upload)
+    subject.should be_action(:upload)
   end
 
   it "should set its default format to :yaml" do
-    @interface.default_format.should == :yaml
+    subject.default_format.should == :yaml
   end
 
   describe "when uploading" do
diff --git a/spec/unit/interface/file_spec.rb b/spec/unit/interface/file_spec.rb
index fc7accf0d..54427a267 100644
--- a/spec/unit/interface/file_spec.rb
+++ b/spec/unit/interface/file_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/file'
 
-describe Puppet::Interface.interface(:file) do
-  before do
-    @interface = Puppet::Interface.interface(:file)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'file' indirection" do
-    @interface.indirection.name.should == :file_bucket_file
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:file) do
 end
diff --git a/spec/unit/interface/indirector_spec.rb b/spec/unit/interface/indirector_spec.rb
index c4d93ade3..0eb7a9a4f 100644
--- a/spec/unit/interface/indirector_spec.rb
+++ b/spec/unit/interface/indirector_spec.rb
@@ -12,10 +12,6 @@ describe Puppet::Interface::Indirector do
     @instance.stubs(:indirection).returns @indirection
   end
 
-  it "should be a subclass of Interface" do
-    Puppet::Interface::Indirector.superclass.should equal(Puppet::Interface)
-  end
-
   it "should be able to return a list of indirections" do
     Puppet::Interface::Indirector.indirections.should be_include("catalog")
   end
diff --git a/spec/unit/interface/interface_collection_spec.rb b/spec/unit/interface/interface_collection_spec.rb
new file mode 100644
index 000000000..536e694fd
--- /dev/null
+++ b/spec/unit/interface/interface_collection_spec.rb
@@ -0,0 +1,97 @@
+require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
+
+require 'puppet/interface/interface_collection'
+
+describe Puppet::Interface::InterfaceCollection do
+  before :each do
+    subject.instance_variable_set("@interfaces", {})
+  end
+
+  after :all do
+    subject.instance_variable_set("@interfaces", {})
+  end
+
+  describe "::interfaces" do
+  end
+
+  describe "::[]" do
+    before :each do
+      subject.instance_variable_set("@interfaces", {:foo => 10})
+    end
+
+    it "should return the interface with the given name" do
+      subject["foo"].should == 10
+    end
+
+    it "should attempt to load the interface if it isn't found" do
+      subject.expects(:require).with('puppet/interface/bar')
+      subject["bar"]
+    end
+  end
+
+  describe "::interface?" do
+    before :each do
+      subject.instance_variable_set("@interfaces", {:foo => 10})
+    end
+
+    it "should return true if the interface specified is registered" do
+      subject.interface?("foo").should == true
+    end
+
+    it "should attempt to require the interface if it is not registered" do
+      subject.expects(:require).with('puppet/interface/bar')
+      subject.interface?("bar")
+    end
+
+    it "should return true if requiring the interface registered it" do
+      subject.stubs(:require).with do
+        subject.instance_variable_set("@interfaces", {:bar => 20})
+      end
+      subject.interface?("bar").should == true
+    end
+
+    it "should return false if the interface is not registered" do
+      subject.stubs(:require).returns(true)
+      subject.interface?("bar").should == false
+    end
+
+    it "should return false if there is a LoadError requiring the interface" do
+      subject.stubs(:require).raises(LoadError)
+      subject.interface?("bar").should == false
+    end
+  end
+
+  describe "::register" do
+    it "should store the interface by name" do
+      interface = Puppet::Interface.new(:my_interface)
+      subject.register(interface)
+      subject.instance_variable_get("@interfaces").should == {:my_interface => interface}
+    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 interface name/
+      end
+    end
+  end
+end
diff --git a/spec/unit/interface/key_spec.rb b/spec/unit/interface/key_spec.rb
index 93a7c937c..4b331d169 100644
--- a/spec/unit/interface/key_spec.rb
+++ b/spec/unit/interface/key_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/key'
 
-describe Puppet::Interface.interface(:key) do
-  before do
-    @interface = Puppet::Interface.interface(:key)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'key' indirection" do
-    @interface.indirection.name.should == :key
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:key) do
 end
diff --git a/spec/unit/interface/node_spec.rb b/spec/unit/interface/node_spec.rb
index 63109308d..b1b4ad421 100644
--- a/spec/unit/interface/node_spec.rb
+++ b/spec/unit/interface/node_spec.rb
@@ -3,26 +3,8 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/node'
 
-describe Puppet::Interface.interface(:node) do
-  before do
-    @interface = Puppet::Interface.interface(:node)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
+describe Puppet::Interface::Indirector.interface(:node) do
   it "should set its default format to :yaml" do
-    @interface.default_format.should == :yaml
-  end
-
-  it "should refer to the 'node' indirection" do
-    @interface.indirection.name.should == :node
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
+    subject.default_format.should == :yaml
   end
 end
diff --git a/spec/unit/interface/report_spec.rb b/spec/unit/interface/report_spec.rb
index b5bee1a62..c424880a9 100644
--- a/spec/unit/interface/report_spec.rb
+++ b/spec/unit/interface/report_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/report'
 
-describe Puppet::Interface.interface(:report) do
-  before do
-    @interface = Puppet::Interface.interface(:report)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'report' indirection" do
-    @interface.indirection.name.should == :report
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:report) do
 end
diff --git a/spec/unit/interface/resource_spec.rb b/spec/unit/interface/resource_spec.rb
index cad45b66b..aab2753b1 100644
--- a/spec/unit/interface/resource_spec.rb
+++ b/spec/unit/interface/resource_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource'
 
-describe Puppet::Interface.interface(:resource) do
-  before do
-    @interface = Puppet::Interface.interface(:resource)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'resource' indirection" do
-    @interface.indirection.name.should == :resource
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:resource) do
 end
diff --git a/spec/unit/interface/resource_type_spec.rb b/spec/unit/interface/resource_type_spec.rb
index 6c437c475..6e973c98b 100644
--- a/spec/unit/interface/resource_type_spec.rb
+++ b/spec/unit/interface/resource_type_spec.rb
@@ -3,22 +3,5 @@
 require File.expand_path(File.dirname(__FILE__) + '/../../spec_helper.rb')
 require 'puppet/interface/resource_type'
 
-describe Puppet::Interface.interface(:resource_type) do
-  before do
-    @interface = Puppet::Interface.interface(:resource_type)
-  end
-
-  it "should be a subclass of 'Indirection'" do
-    @interface.should be_instance_of(Puppet::Interface::Indirector)
-  end
-
-  it "should refer to the 'resource_type' indirection" do
-    @interface.indirection.name.should == :resource_type
-  end
-
-  [:find, :save, :search, :save].each do |method|
-    it "should have  #{method} action defined" do
-      @interface.should be_action(method)
-    end
-  end
+describe Puppet::Interface::Indirector.interface(:resource_type) do
 end