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

17 files changed, 84 insertions, 74 deletions

diff --git a/lib/puppet/application/interface_base.rb b/lib/puppet/application/interface_base.rb
index 654674df5..7a31ce323 100644
--- a/lib/puppet/application/interface_base.rb
+++ b/lib/puppet/application/interface_base.rb
@@ -1,4 +1,5 @@
 require 'puppet/application'
+require 'puppet/interface'
 
 class Puppet::Application::InterfaceBase < Puppet::Application
   should_parse_config
@@ -40,11 +41,6 @@ class Puppet::Application::InterfaceBase < Puppet::Application
     @exit_code || 0
   end
 
-  def initialize(*args)
-    require 'puppet/interface'
-    super
-  end
-
   def main
     # Call the method associated with the provided action (e.g., 'find').
     if result = interface.send(verb, *arguments)
diff --git a/lib/puppet/interface.rb b/lib/puppet/interface.rb
index 0ec0f803f..f82d6235c 100644
--- a/lib/puppet/interface.rb
+++ b/lib/puppet/interface.rb
@@ -3,6 +3,7 @@ require 'puppet/util/autoload'
 
 class Puppet::Interface
   require 'puppet/interface/action_manager'
+  require 'puppet/interface/interface_collection'
 
   include Puppet::Interface::ActionManager
   extend Puppet::Interface::ActionManager
@@ -19,50 +20,27 @@ class Puppet::Interface
   end
 
   def self.interfaces
-    unless @loaded
-      @loaded = true
-      $LOAD_PATH.each do |dir|
-        next unless FileTest.directory?(dir)
-        Dir.chdir(dir) do
-          Dir.glob("puppet/interface/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
-            iname = file.sub(/\.rb/, '')
-            begin
-              require iname
-            rescue Exception => detail
-              puts detail.backtrace if Puppet[:trace]
-              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
-            end
-          end
-        end
-      end
-    end
-    return @interfaces.keys
+    Puppet::Interface::InterfaceCollection.interfaces
   end
 
   def self.interface?(name)
-    name = underscorize(name)
-    require "puppet/interface/#{name}" unless @interfaces.has_key? name
-    return @interfaces.has_key? name
-  rescue LoadError
-    return false
-  end
-
-  def self.interface(name, &blk)
-    interface = interface?(name) ? @interfaces[underscorize(name)] : new(name)
-    interface.instance_eval(&blk) if block_given?
-    return interface
+    Puppet::Interface::InterfaceCollection.interface?(name)
   end
 
-  def self.register_interface(name, instance)
-    @interfaces[underscorize(name)] = instance
+  def self.register(instance)
+    Puppet::Interface::InterfaceCollection.register(instance)
   end
 
-  def self.underscorize(name)
-    unless name.to_s =~ /^[-_a-z]+$/i then
-      raise ArgumentError, "#{name.inspect} (#{name.class}) is not a valid interface name"
+  def self.interface(name, &blk)
+    if interface?(name)
+      interface = Puppet::Interface::InterfaceCollection[name]
+      interface.instance_eval(&blk) if blk
+    else
+      interface = new(name, &blk)
+      Puppet::Interface::InterfaceCollection.register(interface)
+      interface.load_actions
     end
-
-    name.to_s.downcase.split(/[-_]/).join('_').to_sym
+    return interface
   end
 
   attr_accessor :default_format
@@ -71,30 +49,16 @@ class Puppet::Interface
     self.default_format = format.to_sym
   end
 
-  # Return the interface name.
-  def name
-    @name || self.to_s.sub(/.+::/, '').downcase
-  end
-
-  attr_accessor :type, :verb, :name, :arguments, :options
+  attr_accessor :type, :verb, :arguments, :options
+  attr_reader :name
 
   def initialize(name, options = {}, &block)
-    @name = self.class.underscorize(name)
+    @name = Puppet::Interface::InterfaceCollection.underscorize(name)
 
     @default_format = :pson
     options.each { |opt, val| send(opt.to_s + "=", val) }
 
-    # We have to register before loading actions,
-    # since the actions require the registration
-    # Use the full class name, so this works with
-    # subclasses.
-    Puppet::Interface.register_interface(name, self)
-
-    load_actions
-
-    if block_given?
-      instance_eval(&block)
-    end
+    instance_eval(&block) if block
   end
 
   # Try to find actions defined in other files.
@@ -102,7 +66,7 @@ class Puppet::Interface
     path = "puppet/interface/#{name}"
 
     loaded = []
-    self.class.autoloader.search_directories.each do |dir|
+    Puppet::Interface.autoloader.search_directories.each do |dir|
       fdir = ::File.join(dir, path)
       next unless FileTest.directory?(fdir)
 
diff --git a/lib/puppet/interface/catalog.rb b/lib/puppet/interface/catalog.rb
index c0af53bac..defe32127 100644
--- a/lib/puppet/interface/catalog.rb
+++ b/lib/puppet/interface/catalog.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:catalog) do
+Puppet::Interface::Indirector.interface(:catalog) do
   action(:apply) do
     invoke do |catalog|
       report = Puppet::Transaction::Report.new("apply")
diff --git a/lib/puppet/interface/certificate.rb b/lib/puppet/interface/certificate.rb
index 52ba4e3b8..09da0a6c3 100644
--- a/lib/puppet/interface/certificate.rb
+++ b/lib/puppet/interface/certificate.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate) do
+Puppet::Interface::Indirector.interface(:certificate) do
 end
diff --git a/lib/puppet/interface/certificate_request.rb b/lib/puppet/interface/certificate_request.rb
index 77b485f8e..b85c15fef 100644
--- a/lib/puppet/interface/certificate_request.rb
+++ b/lib/puppet/interface/certificate_request.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate_request) do
+Puppet::Interface::Indirector.interface(:certificate_request) do
 end
diff --git a/lib/puppet/interface/certificate_revocation_list.rb b/lib/puppet/interface/certificate_revocation_list.rb
index ee1e6a8c4..956fb6494 100644
--- a/lib/puppet/interface/certificate_revocation_list.rb
+++ b/lib/puppet/interface/certificate_revocation_list.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:certificate_revocation_list) do
+Puppet::Interface::Indirector.interface(:certificate_revocation_list) do
 end
diff --git a/lib/puppet/interface/config.rb b/lib/puppet/interface/config.rb
index 0aecc263f..79d2ee7c1 100644
--- a/lib/puppet/interface/config.rb
+++ b/lib/puppet/interface/config.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface'
 
-Puppet::Interface.new(:config) do
+Puppet::Interface.interface(:config) do
   action(:print) do
     invoke do |*args|
       Puppet.settings[:configprint] = args.join(",")
diff --git a/lib/puppet/interface/configurer.rb b/lib/puppet/interface/configurer.rb
index 2fbde27f1..0d21c4d72 100644
--- a/lib/puppet/interface/configurer.rb
+++ b/lib/puppet/interface/configurer.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface'
 
-Puppet::Interface.new(:configurer) do
+Puppet::Interface.interface(:configurer) do
   action(:synchronize) do
     invoke do |certname|
       facts = Puppet::Interface.interface(:facts).find(certname)
diff --git a/lib/puppet/interface/facts.rb b/lib/puppet/interface/facts.rb
index 8843d297d..97e22714b 100644
--- a/lib/puppet/interface/facts.rb
+++ b/lib/puppet/interface/facts.rb
@@ -1,7 +1,7 @@
 require 'puppet/interface/indirector'
 require 'puppet/node/facts'
 
-Puppet::Interface::Indirector.new(:facts) do
+Puppet::Interface::Indirector.interface(:facts) do
   set_default_format :yaml
 
   # Upload our facts to the server
diff --git a/lib/puppet/interface/file.rb b/lib/puppet/interface/file.rb
index 859f92ca4..f38af2b92 100644
--- a/lib/puppet/interface/file.rb
+++ b/lib/puppet/interface/file.rb
@@ -1,5 +1,5 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:file) do
+Puppet::Interface::Indirector.interface(:file) do
   set_indirection_name :file_bucket_file
 end
diff --git a/lib/puppet/interface/interface_collection.rb b/lib/puppet/interface/interface_collection.rb
new file mode 100644
index 000000000..47ed702aa
--- /dev/null
+++ b/lib/puppet/interface/interface_collection.rb
@@ -0,0 +1,50 @@
+require 'puppet/interface'
+
+module Puppet::Interface::InterfaceCollection
+  @interfaces = {}
+
+  def self.interfaces
+    unless @loaded
+      @loaded = true
+      $LOAD_PATH.each do |dir|
+        next unless FileTest.directory?(dir)
+        Dir.chdir(dir) do
+          Dir.glob("puppet/interface/*.rb").collect { |f| f.sub(/\.rb/, '') }.each do |file|
+            iname = file.sub(/\.rb/, '')
+            begin
+              require iname
+            rescue Exception => detail
+              puts detail.backtrace if Puppet[:trace]
+              raise "Could not load #{iname} from #{dir}/#{file}: #{detail}"
+            end
+          end
+        end
+      end
+    end
+    return @interfaces.keys
+  end
+
+  def self.[](name)
+    @interfaces[underscorize(name)] if interface?(name)
+  end
+
+  def self.interface?(name)
+    name = underscorize(name)
+    require "puppet/interface/#{name}" unless @interfaces.has_key? name
+    return @interfaces.has_key? name
+  rescue LoadError
+    return false
+  end
+
+  def self.register(interface)
+    @interfaces[underscorize(interface.name)] = interface
+  end
+
+  def self.underscorize(name)
+    unless name.to_s =~ /^[-_a-z]+$/i then
+      raise ArgumentError, "#{name.inspect} (#{name.class}) is not a valid interface name"
+    end
+
+    name.to_s.downcase.split(/[-_]/).join('_').to_sym
+  end
+end
diff --git a/lib/puppet/interface/key.rb b/lib/puppet/interface/key.rb
index 9343891d0..57519883d 100644
--- a/lib/puppet/interface/key.rb
+++ b/lib/puppet/interface/key.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:key) do
+Puppet::Interface::Indirector.interface(:key) do
 end
diff --git a/lib/puppet/interface/node.rb b/lib/puppet/interface/node.rb
index 0a0f57a1e..8940fd7dd 100644
--- a/lib/puppet/interface/node.rb
+++ b/lib/puppet/interface/node.rb
@@ -1,5 +1,5 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:node) do
+Puppet::Interface::Indirector.interface(:node) do
   set_default_format :yaml
 end
diff --git a/lib/puppet/interface/report.rb b/lib/puppet/interface/report.rb
index e785ae22d..56a58f6aa 100644
--- a/lib/puppet/interface/report.rb
+++ b/lib/puppet/interface/report.rb
@@ -1,6 +1,6 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:report) do
+Puppet::Interface::Indirector.interface(:report) do
   action(:submit) do
     invoke do |report|
       begin
diff --git a/lib/puppet/interface/resource.rb b/lib/puppet/interface/resource.rb
index 65f2dec7a..130f40fce 100644
--- a/lib/puppet/interface/resource.rb
+++ b/lib/puppet/interface/resource.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:resource) do
+Puppet::Interface::Indirector.interface(:resource) do
 end
diff --git a/lib/puppet/interface/resource_type.rb b/lib/puppet/interface/resource_type.rb
index bf16652a8..70bf3b95a 100644
--- a/lib/puppet/interface/resource_type.rb
+++ b/lib/puppet/interface/resource_type.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:resource_type) do
+Puppet::Interface::Indirector.interface(:resource_type) do
 end
diff --git a/lib/puppet/interface/status.rb b/lib/puppet/interface/status.rb
index 1a1d349d1..432d1ce54 100644
--- a/lib/puppet/interface/status.rb
+++ b/lib/puppet/interface/status.rb
@@ -1,4 +1,4 @@
 require 'puppet/interface/indirector'
 
-Puppet::Interface::Indirector.new(:status) do
+Puppet::Interface::Indirector.interface(:status) do
 end